Specify proposer when constructing validator set in light client (#706)

* Specify proposer when constructing validator set in light client * Update changelog
informalsystems · Dec 14, 2020 · 56d7e49 · 56d7e49
1 parent a2fc126
commit 56d7e49
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 40 deletions.
diff --git 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
 

diff --git 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<LightBlock, IoError> {
             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<TMValidatorSet, IoError> {
+        fn fetch_validator_set(
+            &self,
+            height: AtHeight,
+            proposer_address: Option<TMAccountId>,
+        ) -> Result<TMValidatorSet, IoError> {
             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/src/predicates.rs b/light-client/src/predicates.rs
@@ -579,7 +579,7 @@ mod tests {
                 .generate()
                 .expect("Failed to generate validator"),
         );
-        let val_set_with_faulty_signer = Set::new_simple(bad_vals);
+        let val_set_with_faulty_signer = Set::without_proposer(bad_vals);
 
         // reset signatures
         signed_header.commit.signatures = signatures;
@@ -675,7 +675,7 @@ mod tests {
                 .generate()
                 .unwrap(),
         );
-        let bad_valset = Set::new_simple(vals);
+        let bad_valset = Set::without_proposer(vals);
 
         trust_threshold = TrustThreshold::new(2, 3).expect("Cannot make trust threshold");
 

diff --git a/light-client/tests/model_based.rs b/light-client/tests/model_based.rs
@@ -244,7 +244,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"), LiteVerdict::Invalid)
@@ -260,7 +260,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;
         (
@@ -540,10 +540,11 @@ 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
@@ -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
@@ -15,9 +15,7 @@ use tendermint_proto::Protobuf;
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq)]
 pub struct Set {
     validators: Vec<Info>,
-
     proposer: Option<Info>,
-
     total_voting_power: vote::Power,
 }
 
@@ -35,7 +33,7 @@ impl TryFrom<RawValidatorSet> 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<Set> for RawValidatorSet {
 }
 
 impl Set {
-    /// constructor
+    /// Constructor
     pub fn new(
         validators: Vec<Info>,
         proposer: Option<Info>,
-        total_voting_power: vote::Power,
+        total_voting_power: Option<vote::Power>,
     ) -> 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::<u64>()
+                .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<Info>) -> Set {
-        Self::new(validators, None, vote::Power::default())
+    pub fn without_proposer(validators: Vec<Info>) -> Set {
+        Self::new(validators, None, None)
+    }
+
+    /// Convenience constructor for cases where there is a proposer
+    pub fn with_proposer(
+        validators: Vec<Info>,
+        proposer_address: account::Id,
+    ) -> Result<Self, Error> {
+        // 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
@@ -102,9 +102,9 @@ impl Generator<block::Header> 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
@@ -176,17 +176,17 @@ impl Generator<TMLightBlock> 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(),
         };
 

diff --git a/testgen/src/validator_set.rs b/testgen/src/validator_set.rs
@@ -49,7 +49,7 @@ impl Generator<validator::Set> for ValidatorSet {
 
     fn generate(&self) -> Result<validator::Set, SimpleError> {
         let vals = generate_validators(&self.validators.as_ref().unwrap())?;
-        Ok(validator::Set::new_simple(vals))
+        Ok(validator::Set::without_proposer(vals))
     }
 }
 
@@ -65,7 +65,7 @@ mod tests {
             Validator::new("b"),
             Validator::new("c"),
         ];
-        let valset2 = validator::Set::new_simple(generate_validators(&vals1).unwrap());
+        let valset2 = validator::Set::without_proposer(generate_validators(&vals1).unwrap());
 
         assert_eq!(valset1.hash(), valset2.hash());
 
@@ -82,7 +82,7 @@ mod tests {
             Validator::new("b"),
             Validator::new("c"),
         ];
-        let valset5 = validator::Set::new_simple(generate_validators(&vals2).unwrap());
+        let valset5 = validator::Set::without_proposer(generate_validators(&vals2).unwrap());
         assert_ne!(valset2.hash(), valset5.hash());
     }
 }