From 2322146a3fad5f5ddcda25f9c3f9576b0934f5e8 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 22 Jul 2022 13:45:50 +0200 Subject: [PATCH 01/11] Register / update signed blocks at each block --- contracts/tgrade-valset/src/contract.rs | 23 +++++++++++++++++++---- contracts/tgrade-valset/src/state.rs | 4 ++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index aa2b5ef8..9eddcb22 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -5,8 +5,8 @@ use std::convert::TryInto; #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - to_binary, Addr, Binary, BlockInfo, CustomQuery, Decimal, Deps, DepsMut, Env, MessageInfo, - Order, QueryRequest, Reply, StdError, StdResult, Timestamp, WasmMsg, + from_binary, to_binary, Addr, Binary, BlockInfo, CustomQuery, Decimal, Deps, DepsMut, Env, + MessageInfo, Order, QueryRequest, Reply, StdError, StdResult, Timestamp, WasmMsg, }; use cw2::set_contract_version; @@ -33,8 +33,8 @@ use crate::msg::{ use crate::rewards::pay_block_rewards; use crate::state::{ export, import, operators, Config, EpochInfo, OperatorInfo, ValidatorInfo, ValidatorSlashing, - ValsetState, CONFIG, EPOCH, JAIL, PENDING_VALIDATORS, VALIDATORS, VALIDATOR_SLASHING, - VALIDATOR_START_HEIGHT, + ValsetState, BLOCK_SIGNERS, CONFIG, EPOCH, JAIL, PENDING_VALIDATORS, VALIDATORS, + VALIDATOR_SLASHING, VALIDATOR_START_HEIGHT, }; // version info for migration info @@ -652,6 +652,21 @@ fn is_genesis_block(block: &BlockInfo) -> bool { fn end_block(deps: DepsMut, env: Env) -> Result { let cfg = CONFIG.load(deps.storage)?; + // At each block, update the block signers map + if cfg.verify_validators { + let votes = deps + .querier + .query::(&QueryRequest::Custom(TgradeQuery::ValidatorVotes {}))? + .votes; + let height = env.block.height; + for vote in votes { + let addr = Addr::unchecked(from_binary::(&vote.address)?); + if vote.voted { + BLOCK_SIGNERS.save(deps.storage, &addr, &height)?; + } + } + } + // check if needed and quit early if we didn't hit epoch boundary let mut epoch = EPOCH.load(deps.storage)?; let cur_epoch = env.block.time.nanos() / (1_000_000_000 * epoch.epoch_length); diff --git a/contracts/tgrade-valset/src/state.rs b/contracts/tgrade-valset/src/state.rs index 5086cef1..9aafd173 100644 --- a/contracts/tgrade-valset/src/state.rs +++ b/contracts/tgrade-valset/src/state.rs @@ -108,6 +108,10 @@ pub const VALIDATORS: Item> = Item::new("validators"); /// to verify they're online. pub const PENDING_VALIDATORS: Item> = Item::new("pending"); +/// A map of validators to block heights they had last signed a block. +/// To verify they're online / active. +pub const BLOCK_SIGNERS: Map<&Addr, u64> = Map::new("block_signers"); + /// Map of operator addr to block height it initially became a validator. If operator doesn't /// appear in this map, he was never in the validator set. pub const VALIDATOR_START_HEIGHT: Map<&Addr, u64> = Map::new("start_height"); From c5387183bc9d1e5c7fdcc0ef50c2f0491eb2341c Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 22 Jul 2022 21:49:10 +0200 Subject: [PATCH 02/11] Jail validators that haven't signed any blocks in a given period --- contracts/tgrade-valset/src/contract.rs | 90 ++++++++++++++----------- contracts/tgrade-valset/src/state.rs | 3 +- 2 files changed, 53 insertions(+), 40 deletions(-) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index 9eddcb22..c467e705 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -1,12 +1,12 @@ use std::cmp::{max, min}; use std::collections::BTreeSet; -use std::convert::TryInto; +use std::convert::{TryFrom, TryInto}; #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - from_binary, to_binary, Addr, Binary, BlockInfo, CustomQuery, Decimal, Deps, DepsMut, Env, - MessageInfo, Order, QueryRequest, Reply, StdError, StdResult, Timestamp, WasmMsg, + to_binary, Addr, Binary, BlockInfo, CustomQuery, Decimal, Deps, DepsMut, Env, MessageInfo, + Order, QueryRequest, Reply, StdError, StdResult, Timestamp, WasmMsg, }; use cw2::set_contract_version; @@ -43,6 +43,9 @@ pub(crate) const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); const REWARDS_INIT_REPLY_ID: u64 = 1; +/// Number of missed blocks a validator can be jailed for. +const MISSED_BLOCKS: u64 = 1000; + /// We use this custom message everywhere pub type Response = cosmwasm_std::Response; pub type SubMsg = cosmwasm_std::SubMsg; @@ -660,9 +663,9 @@ fn end_block(deps: DepsMut, env: Env) -> Result(&vote.address)?); if vote.voted { - BLOCK_SIGNERS.save(deps.storage, &addr, &height)?; + let pubkey = vote.address.as_slice(); + BLOCK_SIGNERS.save(deps.storage, &pubkey, &height)?; } } } @@ -686,25 +689,34 @@ fn end_block(deps: DepsMut, env: Env) -> Result(&QueryRequest::Custom( - TgradeQuery::ValidatorVotes {}, - ))? - .votes; - - let expiration = JailingPeriod::from_duration( - JailingDuration::Duration(cfg.offline_jail_duration), - &env.block, - ); - - for val in pending { - let vote = votes - .iter() - .find(|vote| vote.address.as_slice() == val.1.to_address()); - if vote.is_none() || !vote.unwrap().voted { - JAIL.save(deps.storage, &val.0, &expiration)?; + let expiration = JailingPeriod::from_duration( + JailingDuration::Duration(cfg.offline_jail_duration), + &env.block, + ); + + for validator in VALIDATORS.load(deps.storage)? { + let operator_addr = validator.operator; + let ed25519_pubkey = match Ed25519Pubkey::try_from(validator.validator_pubkey) { + Ok(pubkey) => pubkey, + Err(_) => { + // Silently ignore wrong / different type pubkeys + continue; + } + }; + let validator_addr = ed25519_pubkey.to_address(); + let mut height = BLOCK_SIGNERS.may_load(deps.storage, &validator_addr)?; + if height.is_none() { + // Not a block signer yet, check their validator start height instead + height = VALIDATOR_START_HEIGHT.may_load(deps.storage, &operator_addr)?; + } + match height { + Some(h) if h > env.block.height - MISSED_BLOCKS => { + // Validator is still active or recent, no need to jail + continue; + } + _ => { + // validator is inactive for more than MISSED_BLOCKS, jail! + JAIL.save(deps.storage, &operator_addr, &expiration)?; } } } @@ -1172,12 +1184,12 @@ mod test { vec![ ValidatorUpdate { pubkey: Pubkey::Ed25519(b"pubkey1".into()), - power: 1 + power: 1, }, ValidatorUpdate { pubkey: Pubkey::Ed25519(b"pubkey2".into()), - power: 2 - } + power: 2, + }, ], diff.diffs ); @@ -1190,12 +1202,12 @@ mod test { vec![ ValidatorUpdate { pubkey: Pubkey::Ed25519(b"pubkey1".into()), - power: 0 + power: 0, }, ValidatorUpdate { pubkey: Pubkey::Ed25519(b"pubkey2".into()), - power: 0 - } + power: 0, + }, ], diff.diffs ); @@ -1215,7 +1227,7 @@ mod test { assert_eq!( vec![ValidatorUpdate { pubkey: Pubkey::Ed25519(b"pubkey3".into()), - power: 3 + power: 3, },], diff.diffs ); @@ -1230,7 +1242,7 @@ mod test { assert_eq!( vec![ValidatorUpdate { pubkey: Pubkey::Ed25519(b"pubkey1".into()), - power: 1 + power: 1, },], diff.diffs ); @@ -1244,7 +1256,7 @@ mod test { assert_eq!( vec![ValidatorUpdate { pubkey: Pubkey::Ed25519(b"pubkey2".into()), - power: 0 + power: 0, },], diff.diffs ); @@ -1258,7 +1270,7 @@ mod test { assert_eq!( vec![ValidatorUpdate { pubkey: Pubkey::Ed25519(b"pubkey1".into()), - power: 0 + power: 0, },], diff.diffs ); @@ -1286,7 +1298,7 @@ mod test { .iter() .map(|vi| ValidatorUpdate { pubkey: vi.validator_pubkey.clone(), - power: vi.power + power: vi.power, }) .collect() }, @@ -1310,7 +1322,7 @@ mod test { .iter() .map(|vi| ValidatorUpdate { pubkey: vi.validator_pubkey.clone(), - power: 0 + power: 0, }) .collect() }, @@ -1333,7 +1345,7 @@ mod test { ValidatorDiff { diffs: vec![ValidatorUpdate { pubkey: cur.last().as_ref().unwrap().validator_pubkey.clone(), - power: (VALIDATORS + 1) as u64 + power: (VALIDATORS + 1) as u64, }] }, diff @@ -1359,7 +1371,7 @@ mod test { .take(VALIDATORS - 1) .map(|vi| ValidatorUpdate { pubkey: vi.validator_pubkey.clone(), - power: vi.power + power: vi.power, }) .collect() }, @@ -1403,7 +1415,7 @@ mod test { .take(VALIDATORS - 1) .map(|vi| ValidatorUpdate { pubkey: vi.validator_pubkey.clone(), - power: 0 + power: 0, }) .collect() }, diff --git a/contracts/tgrade-valset/src/state.rs b/contracts/tgrade-valset/src/state.rs index 9aafd173..49a2c8ce 100644 --- a/contracts/tgrade-valset/src/state.rs +++ b/contracts/tgrade-valset/src/state.rs @@ -110,7 +110,8 @@ pub const PENDING_VALIDATORS: Item> = Item::new("pend /// A map of validators to block heights they had last signed a block. /// To verify they're online / active. -pub const BLOCK_SIGNERS: Map<&Addr, u64> = Map::new("block_signers"); +/// The key are the first 20 bytes of the SHA-256 hashed validator pubkey (from Cosmos SDK). +pub const BLOCK_SIGNERS: Map<&[u8], u64> = Map::new("block_signers"); /// Map of operator addr to block height it initially became a validator. If operator doesn't /// appear in this map, he was never in the validator set. From bdf79adca56940642fa93d5c92795d7188ae80c7 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 22 Jul 2022 21:57:50 +0200 Subject: [PATCH 03/11] Remove superseded PENDING_VALIDATORS --- contracts/tgrade-valset/src/contract.rs | 35 +++---------------------- contracts/tgrade-valset/src/state.rs | 4 --- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index c467e705..4eccd7df 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -33,8 +33,8 @@ use crate::msg::{ use crate::rewards::pay_block_rewards; use crate::state::{ export, import, operators, Config, EpochInfo, OperatorInfo, ValidatorInfo, ValidatorSlashing, - ValsetState, BLOCK_SIGNERS, CONFIG, EPOCH, JAIL, PENDING_VALIDATORS, VALIDATORS, - VALIDATOR_SLASHING, VALIDATOR_START_HEIGHT, + ValsetState, BLOCK_SIGNERS, CONFIG, EPOCH, JAIL, VALIDATORS, VALIDATOR_SLASHING, + VALIDATOR_START_HEIGHT, }; // version info for migration info @@ -664,8 +664,7 @@ fn end_block(deps: DepsMut, env: Env) -> Result, env: Env) -> Result, env: Env) -> Result = add - .iter() - .filter_map(|m| { - let addr = Addr::unchecked(&m.addr); - if let Ok(op) = operators().load(deps.storage, &addr) { - if !op.active_validator { - Some((addr, op.pubkey)) - } else { - None - } - } else { - None - } - }) - .collect(); - PENDING_VALIDATORS.save(deps.storage, &pending)?; - for v in &mut validators { - if pending.iter().any(|(addr, _)| *addr == v.operator) { - v.power = cfg.min_points; - } - } - } - VALIDATORS.save(deps.storage, &validators)?; // update operators list with info about whether or not they're active validators diff --git a/contracts/tgrade-valset/src/state.rs b/contracts/tgrade-valset/src/state.rs index 49a2c8ce..fc7f2762 100644 --- a/contracts/tgrade-valset/src/state.rs +++ b/contracts/tgrade-valset/src/state.rs @@ -104,10 +104,6 @@ pub const EPOCH: Item = Item::new("epoch"); /// This will be empty only on the first run. pub const VALIDATORS: Item> = Item::new("validators"); -/// A list of validators who have just became active and have yet to sign a block -/// to verify they're online. -pub const PENDING_VALIDATORS: Item> = Item::new("pending"); - /// A map of validators to block heights they had last signed a block. /// To verify they're online / active. /// The key are the first 20 bytes of the SHA-256 hashed validator pubkey (from Cosmos SDK). From 0565686b686cca57ca56e0043c1ae55f7cd3c648 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Sat, 23 Jul 2022 16:48:14 +0200 Subject: [PATCH 04/11] Fix: saturating subtraction --- contracts/tgrade-valset/src/contract.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index 4eccd7df..1500cb20 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -43,8 +43,8 @@ pub(crate) const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); const REWARDS_INIT_REPLY_ID: u64 = 1; -/// Number of missed blocks a validator can be jailed for. -const MISSED_BLOCKS: u64 = 1000; +/// Missed blocks interval a validator can be jailed for. +pub const MISSED_BLOCKS: u64 = 1000; /// We use this custom message everywhere pub type Response = cosmwasm_std::Response; @@ -709,7 +709,7 @@ fn end_block(deps: DepsMut, env: Env) -> Result env.block.height - MISSED_BLOCKS => { + Some(h) if h > env.block.height.saturating_sub(MISSED_BLOCKS) => { // Validator is still active or recent, no need to jail continue; } From 38e101e41810b29ec77226e47006a16852164cbc Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Sat, 23 Jul 2022 18:29:14 +0200 Subject: [PATCH 05/11] Adapt validator verification tests --- .../tgrade-valset/src/multitest/suite.rs | 7 +++ .../src/multitest/verify_online.rs | 43 +++++++++++++------ 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/contracts/tgrade-valset/src/multitest/suite.rs b/contracts/tgrade-valset/src/multitest/suite.rs index 70942d98..cd5ec1ff 100644 --- a/contracts/tgrade-valset/src/multitest/suite.rs +++ b/contracts/tgrade-valset/src/multitest/suite.rs @@ -494,6 +494,13 @@ impl Suite { Ok(diff) } + pub fn advance_blocks(&mut self, blocks: u64) -> AnyResult> { + self.app.advance_blocks(blocks); + let (_, diff) = self.app.end_block()?; + self.app.begin_block(vec![])?; + Ok(diff) + } + /// Timestamp of current block pub fn timestamp(&self) -> Timestamp { self.app.block_info().time diff --git a/contracts/tgrade-valset/src/multitest/verify_online.rs b/contracts/tgrade-valset/src/multitest/verify_online.rs index c7e95e64..69bc9f12 100644 --- a/contracts/tgrade-valset/src/multitest/verify_online.rs +++ b/contracts/tgrade-valset/src/multitest/verify_online.rs @@ -1,9 +1,11 @@ use std::convert::TryInto; +use crate::contract::MISSED_BLOCKS; use cosmwasm_std::Binary; use tg_bindings::{Ed25519Pubkey, ToAddress, ValidatorVote}; use crate::multitest::helpers::assert_active_validators; +use crate::multitest::suite::Suite; use super::{ helpers::{addr_to_pubkey, members_init}, @@ -28,7 +30,6 @@ fn verify_validators_works() { let mut suite = SuiteBuilder::new() .with_operators(&members) .with_engagement(&members_init(&members, &[2, 3])) - .with_min_points(1) .with_verify_validators(600) .build(); @@ -53,10 +54,10 @@ fn verify_validators_works() { assert!(info2.jailed_until.is_none()); assert!(info1.active_validator); assert!(info2.active_validator); - // Validators have min power before they're verified + // Validators have full power from the beginning assert_active_validators( &suite.list_active_validators(None, None).unwrap(), - &[(members[0], 1), (members[1], 1)], + &[(members[0], 2), (members[1], 3)], ); suite.advance_epoch().unwrap(); @@ -80,7 +81,7 @@ fn verify_validators_jailing() { "member2member2member2member2memb", ]; - let mut suite = SuiteBuilder::new() + let mut suite: Suite = SuiteBuilder::new() .with_operators(&members) .with_engagement(&members_init(&members, &[2, 3])) .with_verify_validators(600) @@ -94,13 +95,26 @@ fn verify_validators_jailing() { }]) .unwrap(); + // Initially no jailed validators + let info1 = suite.validator(members[0]).unwrap().validator.unwrap(); + let info2 = suite.validator(members[1]).unwrap().validator.unwrap(); + assert!(info1.jailed_until.is_none()); + assert!(info2.jailed_until.is_none()); + + // Advance before the missed blocks interval + suite.advance_epoch().unwrap(); + + // Still no jailed validators let info1 = suite.validator(members[0]).unwrap().validator.unwrap(); let info2 = suite.validator(members[1]).unwrap().validator.unwrap(); assert!(info1.jailed_until.is_none()); assert!(info2.jailed_until.is_none()); + // Advance after the missed blocks interval + suite.advance_blocks(MISSED_BLOCKS).unwrap(); suite.advance_epoch().unwrap(); + // Validator who didn't vote for all the missed blocks period is now jailed let info1 = suite.validator(members[0]).unwrap().validator.unwrap(); let info2 = suite.validator(members[1]).unwrap().validator.unwrap(); assert!(info1.jailed_until.is_none()); @@ -109,7 +123,7 @@ fn verify_validators_jailing() { } #[test] -fn validator_needs_to_verify_if_unjailed() { +fn validator_needs_to_verify_if_unjailed_1() { let members = vec![ "member1member1member1member1memb", "member2member2member2member2memb", @@ -118,7 +132,6 @@ fn validator_needs_to_verify_if_unjailed() { let mut suite = SuiteBuilder::new() .with_operators(&members) .with_engagement(&members_init(&members, &[2, 3])) - .with_min_points(2) .with_verify_validators(600) .with_epoch_length(600) .build(); @@ -140,9 +153,10 @@ fn validator_needs_to_verify_if_unjailed() { .is_none()); assert_active_validators( &suite.list_active_validators(None, None).unwrap(), - &[(members[0], 2), (members[1], 2)], + &[(members[0], 2), (members[1], 3)], ); + suite.advance_blocks(MISSED_BLOCKS).unwrap(); suite.advance_epoch().unwrap(); // Validator 2 failed verification, so is jailed @@ -173,12 +187,12 @@ fn validator_needs_to_verify_if_unjailed() { .is_none()); assert_active_validators( &suite.list_active_validators(None, None).unwrap(), - &[(members[0], 2), (members[1], 2)], + &[(members[0], 2), (members[1], 3)], ); - // Validator should be PENDING after being re-added to the valset, - // so if they fail to sign a block to prove they're online, they get + // If validator fails to sign a block to prove they're online, they get // jailed -again- + suite.advance_blocks(MISSED_BLOCKS).unwrap(); suite.advance_epoch().unwrap(); assert!(suite .validator(members[1]) @@ -203,9 +217,8 @@ fn validator_needs_to_verify_if_unjailed_by_auto_unjail() { let mut suite = SuiteBuilder::new() .with_operators(&members) .with_engagement(&members_init(&members, &[2, 3])) - .with_min_points(2) .with_auto_unjail() - .with_verify_validators(600) + .with_verify_validators(1200) .with_epoch_length(600) .build(); @@ -226,9 +239,11 @@ fn validator_needs_to_verify_if_unjailed_by_auto_unjail() { .is_none()); assert_active_validators( &suite.list_active_validators(None, None).unwrap(), - &[(members[0], 2), (members[1], 2)], + &[(members[0], 2), (members[1], 3)], ); + // Advance after the missed blocks interval + suite.advance_blocks(MISSED_BLOCKS).unwrap(); suite.advance_epoch().unwrap(); // Validator 2 failed verification, so is jailed @@ -256,7 +271,7 @@ fn validator_needs_to_verify_if_unjailed_by_auto_unjail() { .is_none()); assert_active_validators( &suite.list_active_validators(None, None).unwrap(), - &[(members[0], 2), (members[1], 2)], + &[(members[0], 2), (members[1], 3)], ); // Validator should be PENDING after being re-added to the valset, From 391f016f0f7e9aef0214937214ca44215ee42bdf Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Sat, 23 Jul 2022 18:52:44 +0200 Subject: [PATCH 06/11] Use iterator for updating the block signers height --- contracts/tgrade-valset/src/contract.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index 1500cb20..31f5db14 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -655,18 +655,16 @@ fn is_genesis_block(block: &BlockInfo) -> bool { fn end_block(deps: DepsMut, env: Env) -> Result { let cfg = CONFIG.load(deps.storage)?; - // At each block, update the block signers map if cfg.verify_validators { - let votes = deps - .querier + // Update the block signers height at each block + deps.querier .query::(&QueryRequest::Custom(TgradeQuery::ValidatorVotes {}))? - .votes; - let height = env.block.height; - for vote in votes { - if vote.voted { - BLOCK_SIGNERS.save(deps.storage, vote.address.as_slice(), &height)?; - } - } + .votes + .iter() + .filter(|&v| v.voted) + .try_for_each(|v| { + BLOCK_SIGNERS.save(deps.storage, v.address.as_slice(), &env.block.height) + })?; } // check if needed and quit early if we didn't hit epoch boundary From d4141d561a7db1946c4da8fac6e3eeae8873f809 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Sun, 24 Jul 2022 08:39:34 +0200 Subject: [PATCH 07/11] Use iterator for jailing block signer offenders --- contracts/tgrade-valset/src/contract.rs | 45 ++++++++++++------------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index 31f5db14..963f3f48 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -691,32 +691,29 @@ fn end_block(deps: DepsMut, env: Env) -> Result pubkey, - Err(_) => { - // Silently ignore wrong / different type pubkeys - continue; - } - }; - let validator_addr = ed25519_pubkey.to_address(); - let mut height = BLOCK_SIGNERS.may_load(deps.storage, &validator_addr)?; - if height.is_none() { - // Not a block signer yet, check their validator start height instead - height = VALIDATOR_START_HEIGHT.may_load(deps.storage, &operator_addr)?; - } - match height { - Some(h) if h > env.block.height.saturating_sub(MISSED_BLOCKS) => { - // Validator is still active or recent, no need to jail - continue; + VALIDATORS + .load(deps.storage)? + .iter() + .flat_map(|v| match Ed25519Pubkey::try_from(&v.validator_pubkey) { + Ok(pubkey) => Some((v, pubkey)), + _ => None, // Silently ignore wrong / different type pubkeys + }) + .try_for_each(|(v, ed25519_pubkey)| { + let operator_addr = &v.operator; + let validator_addr = ed25519_pubkey.to_address(); + let mut height = BLOCK_SIGNERS.may_load(deps.storage, &validator_addr)?; + if height.is_none() { + // Not a block signer yet, check their validator start height instead + height = VALIDATOR_START_HEIGHT.may_load(deps.storage, operator_addr)?; } - _ => { - // validator is inactive for more than MISSED_BLOCKS, jail! - JAIL.save(deps.storage, &operator_addr, &expiration)?; + match height { + Some(h) if h > env.block.height.saturating_sub(MISSED_BLOCKS) => Ok(()), + _ => { + // validator is inactive for at least MISSED_BLOCKS, jail! + JAIL.save(deps.storage, operator_addr, &expiration) + } } - } - } + })?; } // calculate and store new validator set From 4c9103beb25c8f95c1f3f51fd2d41c37a5699977 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 3 Aug 2022 09:22:08 +0200 Subject: [PATCH 08/11] Improve doc comments in passing --- contracts/tg4-engagement/src/msg.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/contracts/tg4-engagement/src/msg.rs b/contracts/tg4-engagement/src/msg.rs index 73de0399..6cdbc6c8 100644 --- a/contracts/tg4-engagement/src/msg.rs +++ b/contracts/tg4-engagement/src/msg.rs @@ -98,27 +98,27 @@ pub enum QueryMsg { Hooks {}, /// Return the current number of preauths. Returns PreauthResponse. Preauths {}, - /// Return how much rewards are assigned for withdrawal to given address. Returns + /// Return how many rewards are assigned for withdrawal from the given address. Returns /// `RewardsResponse`. WithdrawableRewards { owner: String }, - /// Return how much rewards were distributed in total by this contract. Returns + /// Return how many rewards were distributed in total by this contract. Returns /// `RewardsResponse`. DistributedRewards {}, - /// Return how much funds were send to this contract since last `ExecuteMsg::DistribtueFunds`, - /// and wait for distribution. Returns `RewardsResponse`. + /// Return how many funds were sent to this contract since last `ExecuteMsg::DistributeFunds`, + /// and await for distribution. Returns `RewardsResponse`. UndistributedRewards {}, - /// Returns address allowed for withdrawal funds assigned to owner. Returns `DelegateResponse` + /// Return address allowed for withdrawal of the funds assigned to owner. Returns `DelegateResponse` Delegated { owner: String }, - /// Returns information about the halflife, including the duration in seconds, the last - /// and the next occurence. + /// Returns information about the half-life, including the duration in seconds, the last + /// and the next occurrence. Halflife {}, - /// Returns information (bool) whether given address is an active slasher + /// Returns information (bool) about whether the given address is an active slasher IsSlasher { addr: String }, - /// Returns all active slashers as vector of addresses + /// Returns all active slashers as a vector of addresses ListSlashers {}, /// Returns rewards distribution data DistributionData {}, - /// Returns withdraw adjustment + /// Returns withdraw adjustment data WithdrawAdjustmentData { addr: String }, } From 82880529c05046e9039d8e89e8d6ab5934cf9018 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 3 Aug 2022 11:55:45 +0200 Subject: [PATCH 09/11] Update verify_validators doc comment --- contracts/tgrade-valset/src/state.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/contracts/tgrade-valset/src/state.rs b/contracts/tgrade-valset/src/state.rs index fc7f2762..3ed7f80f 100644 --- a/contracts/tgrade-valset/src/state.rs +++ b/contracts/tgrade-valset/src/state.rs @@ -57,11 +57,8 @@ pub struct Config { /// Address of contract for validator group voting. pub validator_group: Addr, - /// When a validator joins the valset, verify they sign the first block since joining - /// or jail them for a period otherwise. - /// - /// The verification happens every time the validator becomes an active validator, - /// including when they are unjailed or when they just gain enough power to participate. + /// If this is enabled, signed blocks are watched for, and if a validator fails to sign any blocks + /// in a string of a number of blocks (typically 1000 blocks), they are jailed. pub verify_validators: bool, /// The duration to jail a validator for in case they don't sign their first epoch From 784a7964dc47c3854c6f085010ff8155bf7afa3b Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 3 Aug 2022 16:42:56 +0200 Subject: [PATCH 10/11] Add verify_validators option to migration msg --- contracts/tgrade-valset/src/contract.rs | 3 +++ contracts/tgrade-valset/src/msg.rs | 1 + contracts/tgrade-valset/src/multitest/migration.rs | 1 + 3 files changed, 5 insertions(+) diff --git a/contracts/tgrade-valset/src/contract.rs b/contracts/tgrade-valset/src/contract.rs index 963f3f48..e4e2ad79 100644 --- a/contracts/tgrade-valset/src/contract.rs +++ b/contracts/tgrade-valset/src/contract.rs @@ -943,6 +943,9 @@ pub fn migrate( if let Some(max_validators) = msg.max_validators { cfg.max_validators = max_validators; } + if let Some(verify_validators) = msg.verify_validators { + cfg.verify_validators = verify_validators; + } Ok(cfg) })?; diff --git a/contracts/tgrade-valset/src/msg.rs b/contracts/tgrade-valset/src/msg.rs index ed3c9723..fcb91908 100644 --- a/contracts/tgrade-valset/src/msg.rs +++ b/contracts/tgrade-valset/src/msg.rs @@ -508,6 +508,7 @@ pub struct InstantiateResponse { pub struct MigrateMsg { pub min_points: Option, pub max_validators: Option, + pub verify_validators: Option, } #[cfg(test)] diff --git a/contracts/tgrade-valset/src/multitest/migration.rs b/contracts/tgrade-valset/src/multitest/migration.rs index 8f0e2a3a..ce6d0d37 100644 --- a/contracts/tgrade-valset/src/multitest/migration.rs +++ b/contracts/tgrade-valset/src/multitest/migration.rs @@ -19,6 +19,7 @@ fn migration_can_alter_cfg() { &MigrateMsg { min_points: Some(5), max_validators: Some(10), + verify_validators: Some(true), }, ) .unwrap(); From e409500ca1c0f83e03bf04d417c9e784421fa1b6 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Thu, 25 Aug 2022 10:16:47 +0200 Subject: [PATCH 11/11] Rename test Co-authored-by: Jakub Bogucki --- contracts/tgrade-valset/src/multitest/verify_online.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/tgrade-valset/src/multitest/verify_online.rs b/contracts/tgrade-valset/src/multitest/verify_online.rs index 69bc9f12..224ec9d6 100644 --- a/contracts/tgrade-valset/src/multitest/verify_online.rs +++ b/contracts/tgrade-valset/src/multitest/verify_online.rs @@ -123,7 +123,7 @@ fn verify_validators_jailing() { } #[test] -fn validator_needs_to_verify_if_unjailed_1() { +fn validator_needs_to_verify_if_unjailed() { let members = vec![ "member1member1member1member1memb", "member2member2member2member2memb",