From c5fe2a88c80d9336bd14a9bcbf575e7a62f3a93b Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 26 Nov 2020 13:22:48 +0100 Subject: [PATCH 1/2] Specify proposer when constructing validator set in light client --- light-client/src/components/io.rs | 32 +++++++++++++----- light-client/tests/model_based.rs | 8 ++--- tendermint/src/error.rs | 6 ++++ tendermint/src/validator.rs | 55 +++++++++++++++++++++---------- testgen/src/header.rs | 4 +-- testgen/src/light_block.rs | 6 ++-- 6 files changed, 76 insertions(+), 35 deletions(-) diff --git a/light-client/src/components/io.rs b/light-client/src/components/io.rs index fd869ee17..8aeef4c0a 100644 --- a/light-client/src/components/io.rs +++ b/light-client/src/components/io.rs @@ -40,6 +40,10 @@ pub enum IoError { #[error("invalid height: {0}")] InvalidHeight(String), + /// Fetched validator set is invalid + #[error("fetched validator set is invalid: {0}")] + InvalidValidatorSet(String), + /// Task timed out. #[error("task timed out after {} ms", .0.as_millis())] Timeout(Duration), @@ -84,6 +88,7 @@ mod prod { use crate::types::PeerId; use crate::utils::block_on; + use tendermint::account::Id as TMAccountId; use tendermint::block::signed_header::SignedHeader as TMSignedHeader; use tendermint::validator::Set as TMValidatorSet; @@ -100,9 +105,10 @@ mod prod { fn fetch_light_block(&self, height: AtHeight) -> Result { let signed_header = self.fetch_signed_header(height)?; let height = signed_header.header.height; + let proposer_address = signed_header.header.proposer_address; - let validator_set = self.fetch_validator_set(height.into())?; - let next_validator_set = self.fetch_validator_set(height.increment().into())?; + let validator_set = self.fetch_validator_set(height.into(), Some(proposer_address))?; + let next_validator_set = self.fetch_validator_set(height.increment().into(), None)?; let light_block = LightBlock::new( signed_header, @@ -147,7 +153,11 @@ mod prod { } } - fn fetch_validator_set(&self, height: AtHeight) -> Result { + fn fetch_validator_set( + &self, + height: AtHeight, + proposer_address: Option, + ) -> Result { let height = match height { AtHeight::Highest => bail!(IoError::InvalidHeight( "given height must be greater than 0".to_string() @@ -156,12 +166,18 @@ mod prod { }; let client = self.rpc_client.clone(); - let res = block_on(self.timeout, async move { client.validators(height).await })?; + let response = block_on(self.timeout, async move { client.validators(height).await })? + .map_err(IoError::RpcError)?; - match res { - Ok(response) => Ok(TMValidatorSet::new_simple(response.validators)), - Err(err) => Err(IoError::RpcError(err)), - } + let validator_set = match proposer_address { + Some(proposer_address) => { + TMValidatorSet::with_proposer(response.validators, proposer_address) + .map_err(|e| IoError::InvalidValidatorSet(e.to_string()))? + } + None => TMValidatorSet::without_proposer(response.validators), + }; + + Ok(validator_set) } } } diff --git a/light-client/tests/model_based.rs b/light-client/tests/model_based.rs index 33b4a39ca..428550ebf 100644 --- a/light-client/tests/model_based.rs +++ b/light-client/tests/model_based.rs @@ -231,7 +231,7 @@ impl SingleStepTestFuzzer for HeaderValHashFuzzer { Validator::new("2"), Validator::new("3"), ]; - let valset = ValidatorSet::new_simple(generate_validators(&vals).unwrap()); + let valset = ValidatorSet::without_proposer(generate_validators(&vals).unwrap()); input.block.validators = valset; (String::from("header validators_hash"), true) @@ -246,7 +246,7 @@ impl SingleStepTestFuzzer for HeaderNextValHashFuzzer { Validator::new("2"), Validator::new("3"), ]; - let valset = ValidatorSet::new_simple(generate_validators(&vals).unwrap()); + let valset = ValidatorSet::without_proposer(generate_validators(&vals).unwrap()); input.block.next_validators = valset; (String::from("header next_validators_hash"), true) @@ -452,10 +452,10 @@ fn single_step_test( // Below is a temporary work around to get rid of bug-gy validator sorting // which was making all the tests fail let current_vals = input.block.validators.clone(); - let current_resorted = Set::new_simple(current_vals.validators().to_vec()); + let current_resorted = Set::without_proposer(current_vals.validators().to_vec()); let current_next_vals = input.block.next_validators.clone(); - let current_next_resorted = Set::new_simple(current_next_vals.validators().to_vec()); + let current_next_resorted = Set::without_proposer(current_next_vals.validators().to_vec()); let mut mutated_block = input.block.clone(); mutated_block.validators = current_resorted; diff --git a/tendermint/src/error.rs b/tendermint/src/error.rs index 0dab4701f..b1230e2dc 100644 --- a/tendermint/src/error.rs +++ b/tendermint/src/error.rs @@ -3,6 +3,8 @@ use anomaly::{BoxError, Context}; use thiserror::Error; +use crate::account; + /// Error type pub type Error = BoxError; @@ -176,6 +178,10 @@ pub enum Kind { /// Missing max_age_duration in evidence parameters #[error("missing max_age_duration")] MissingMaxAgeDuration, + + /// Proposer not found in validator set + #[error("proposer with address '{}' not found in validator set", _0)] + ProposerNotFound(account::Id), } impl Kind { diff --git a/tendermint/src/validator.rs b/tendermint/src/validator.rs index 96f276677..888fb2c7d 100644 --- a/tendermint/src/validator.rs +++ b/tendermint/src/validator.rs @@ -15,9 +15,7 @@ use tendermint_proto::Protobuf; #[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] pub struct Set { validators: Vec, - proposer: Option, - total_voting_power: vote::Power, } @@ -35,7 +33,7 @@ impl TryFrom for Set { Ok(Self::new( unsorted_validators_result?, value.proposer.map(TryInto::try_into).transpose()?, - value.total_voting_power.try_into()?, + Some(value.total_voting_power.try_into()?), )) } } @@ -51,14 +49,25 @@ impl From for RawValidatorSet { } impl Set { - /// constructor + /// Constructor pub fn new( validators: Vec, proposer: Option, - total_voting_power: vote::Power, + total_voting_power: Option, ) -> Set { let mut validators = validators; Self::sort_validators(&mut validators); + + // Compute the total voting power + let total_voting_power = total_voting_power.unwrap_or_else(|| { + validators + .iter() + .map(|v| v.voting_power.value()) + .sum::() + .try_into() + .unwrap() + }); + Set { validators, proposer, @@ -67,8 +76,25 @@ impl Set { } /// Convenience constructor for cases where there is no proposer - pub fn new_simple(validators: Vec) -> Set { - Self::new(validators, None, vote::Power::default()) + pub fn without_proposer(validators: Vec) -> Set { + Self::new(validators, None, None) + } + + /// Convenience constructor for cases where there is a proposer + pub fn with_proposer( + validators: Vec, + proposer_address: account::Id, + ) -> Result { + // Get the proposer. + let proposer = validators + .iter() + .find(|v| v.address == proposer_address) + .cloned() + .ok_or(Kind::ProposerNotFound(proposer_address))?; + + // Create the validator set with the given proposer. + // This is required by IBC on-chain validation. + Ok(Self::new(validators, Some(proposer), None)) } /// Get Info of the underlying validators. @@ -82,8 +108,8 @@ impl Set { } /// Get total voting power - pub fn total_voting_power(&self) -> &vote::Power { - &self.total_voting_power + pub fn total_voting_power(&self) -> vote::Power { + self.total_voting_power } /// Sort the validators according to the current Tendermint requirements @@ -110,13 +136,6 @@ impl Set { Hash::Sha256(merkle::simple_hash_from_byte_vectors(validator_bytes)) } - - /// Compute the total voting power within this validator set - pub fn total_power(&self) -> u64 { - self.validators().iter().fold(0u64, |total, val_info| { - total + val_info.voting_power.value() - }) - } } /// Validator information @@ -382,7 +401,7 @@ mod tests { 22, 57, 84, 71, 122, 200, 169, 192, 252, 41, 148, 223, 180, ]; - let val_set = Set::new_simple(vec![v1, v2, v3]); + let val_set = Set::without_proposer(vec![v1, v2, v3]); let hash = val_set.hash(); assert_eq!(hash_expect, hash.as_bytes().to_vec()); @@ -399,7 +418,7 @@ mod tests { assert_eq!(val_set.validator(v3.address).unwrap(), v3); assert_eq!(val_set.validator(not_in_set.address), None); assert_eq!( - val_set.total_power(), + val_set.total_voting_power().value(), 148_151_478_422_287_875 + 158_095_448_483_785_107 + 770_561_664_770_006_272 ); } diff --git a/testgen/src/header.rs b/testgen/src/header.rs index 323ee14cb..4285d0172 100644 --- a/testgen/src/header.rs +++ b/testgen/src/header.rs @@ -102,9 +102,9 @@ impl Generator for Header { } else { Validator::new("a").generate().unwrap().address }; - let valset = validator::Set::new_simple(vals); + let valset = validator::Set::without_proposer(vals); let next_valset = match &self.next_validators { - Some(next_vals) => validator::Set::new_simple(generate_validators(next_vals)?), + Some(next_vals) => validator::Set::without_proposer(generate_validators(next_vals)?), None => valset.clone(), }; let chain_id = match chain::Id::from_str( diff --git a/testgen/src/light_block.rs b/testgen/src/light_block.rs index 4cceb4b42..b179eb977 100644 --- a/testgen/src/light_block.rs +++ b/testgen/src/light_block.rs @@ -173,17 +173,17 @@ impl Generator for LightBlock { generate_signed_header(header, commit).expect("Could not generate signed header"); let validators = match &self.validators { - None => validator::Set::new_simple(generate_validators( + None => validator::Set::without_proposer(generate_validators( header .validators .as_ref() .expect("missing validators in header"), )?), - Some(vals) => validator::Set::new_simple(generate_validators(vals)?), + Some(vals) => validator::Set::without_proposer(generate_validators(vals)?), }; let next_validators = match &self.next_validators { - Some(next_vals) => validator::Set::new_simple(generate_validators(next_vals)?), + Some(next_vals) => validator::Set::without_proposer(generate_validators(next_vals)?), None => validators.clone(), }; From 8de481758a85dd5696dda2bdc1afcb7fcc0a462f Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Mon, 14 Dec 2020 16:15:07 +0100 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d43f60c03..0f7a0dd9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,14 @@ key/value pairs decoded from base64, where previously tag key/value pairs were Base64-encoded ([#717]) - `[tendermint]` (Since v0.17.0-rc3) Bech32 encoding fix ([#690]) +- `[tendermint, light-client]` Specify the proposer in the validator set of fetched light blocks ([#705]) [#425]: https://github.com/informalsystems/tendermint-rs/issues/425 [#646]: https://github.com/informalsystems/tendermint-rs/pull/646 [#690]: https://github.com/informalsystems/tendermint-rs/issues/690 [#701]: https://github.com/informalsystems/tendermint-rs/pull/701 [#717]: https://github.com/informalsystems/tendermint-rs/issues/717 +[#705]: https://github.com/informalsystems/tendermint-rs/issues/705 ## v0.17.0-rc3