fix: header validation during tendermint client update

.changelog/unreleased/bug/911-fix-header-validation-during-tmclient-update.md

@@ -0,0 +1,2 @@
+- Remove an incorrect validation during tendermint client update
+  ([\#911](https://github.com/cosmos/ibc-rs/issues/911))

crates/ibc/src/clients/ics07_tendermint/header.rs

@@ -126,13 +126,6 @@ impl Header {
             });
         }
 
-        if self.trusted_next_validator_set.hash() != self.signed_header.header.next_validators_hash
-        {
-            return Err(Error::MismatchValidatorsHashes {
-                signed_header_validators_hash: self.signed_header.header.next_validators_hash,
-                validators_hash: self.trusted_next_validator_set.hash(),
-            });
-        }
         Ok(())
     }
 }

crates/ibc/src/core/ics02_client/handler/update_client.rs

@@ -137,6 +137,7 @@ mod tests {
 
     use ibc_proto::google::protobuf::Any;
     use ibc_proto::ibc::lightclients::tendermint::v1::{ClientState as RawTmClientState, Fraction};
+    use tendermint_testgen::Validator as TestgenValidator;
     use test_log::test;
 
     use super::*;
@@ -255,6 +256,72 @@ mod tests {
         assert_eq!(client_state.latest_height(), latest_header_height);
     }
 
+    #[test]
+    fn test_update_synthetic_tendermint_client_validator_change_ok() {
+        let client_id = ClientId::new(tm_client_type(), 0).unwrap();
+        let client_height = Height::new(1, 20).unwrap();
+        let update_height = Height::new(1, 21).unwrap();
+        let chain_id_b = ChainId::new("mockgaiaB", 1).unwrap();
+
+        let mut ctx = MockContext::new(
+            ChainId::new("mockgaiaA", 1).unwrap(),
+            HostType::Mock,
+            5,
+            Height::new(1, 1).unwrap(),
+        )
+        .with_client_parametrized_with_chain_id(
+            chain_id_b.clone(),
+            &client_id,
+            client_height,
+            Some(tm_client_type()), // The target host chain (B) is synthetic TM.
+            Some(client_height),
+        );
+
+        let ctx_b = MockContext::new_with_validator_history(
+            chain_id_b,
+            HostType::SyntheticTendermint,
+            &[
+                // TODO(rano): the validator set params during setups.
+                // Here I picked the default validator set which is
+                // used at host side client creation.
+                vec![
+                    TestgenValidator::new("1").voting_power(50),
+                    TestgenValidator::new("2").voting_power(50),
+                ],
+                vec![
+                    TestgenValidator::new("1").voting_power(60),
+                    TestgenValidator::new("2").voting_power(40),
+                ],
+            ],
+            update_height,
+        );
+
+        let signer = get_dummy_account_id();
+
+        let mut block = ctx_b.host_block(&update_height).unwrap().clone();
+        block.set_trusted_height(client_height);
+
+        let latest_header_height = block.height();
+        let msg = MsgUpdateClient {
+            client_id,
+            client_message: block.into(),
+            signer,
+        };
+
+        let res = validate(&ctx, MsgUpdateOrMisbehaviour::UpdateClient(msg.clone()));
+        assert!(res.is_ok());
+
+        let res = execute(&mut ctx, MsgUpdateOrMisbehaviour::UpdateClient(msg.clone()));
+        assert!(res.is_ok(), "result: {res:?}");
+
+        let client_state = ctx.client_state(&msg.client_id).unwrap();
+        assert!(client_state
+            .status(&ctx, &msg.client_id)
+            .unwrap()
+            .is_active());
+        assert_eq!(client_state.latest_height(), latest_header_height);
+    }
+
     #[test]
     fn test_update_synthetic_tendermint_client_non_adjacent_ok() {
         let client_id = ClientId::new(tm_client_type(), 0).unwrap();

crates/ibc/src/mock/context.rs

@@ -13,6 +13,7 @@
 use ibc_proto::google::protobuf::Any;
 use ibc_proto::protobuf::Protobuf;
 use parking_lot::Mutex;
+use tendermint_testgen::Validator as TestgenValidator;
 use tracing::debug;
 
 use super::client_state::{MOCK_CLIENT_STATE_TYPE_URL, MOCK_CLIENT_TYPE};
@@ -313,6 +314,78 @@
         }
     }
 
+    /// Same as [Self::new] but with custom validator sets for each block.
+    /// Note: the validator history is used accordingly for current validator set and next validator set.
+    /// `validator_history[i]` and `validator_history[i+1]` is i'th block's current and next validator set.
+    /// The number of blocks will be `validator_history.len() - 1` due to the above.
+    pub fn new_with_validator_history(
+        host_id: ChainId,
+        host_type: HostType,
+        validator_history: &[Vec<TestgenValidator>],
+        latest_height: Height,
+    ) -> Self {
+        let max_history_size = validator_history.len() - 1;
+
+        assert_ne!(
+            max_history_size, 0,
+            "The chain must have a non-zero max_history_size"
+        );
+
+        assert_ne!(
+            latest_height.revision_height(),
+            0,
+            "The chain must have a non-zero revision_height"
+        );
+
+        assert!(
+            max_history_size as u64 <= latest_height.revision_height(),
+            "The number of blocks must be greater than the number of validator set histories"
+        );
+
+        assert_eq!(
+            host_id.revision_number(),
+            latest_height.revision_number(),
+            "The version in the chain identifier must match the version in the latest height"
+        );
+
+        let block_time = Duration::from_secs(DEFAULT_BLOCK_TIME_SECS);
+        let next_block_timestamp = Timestamp::now().add(block_time).expect("Never fails");
+
+        let history = (0..max_history_size)
+            .rev()
+            .map(|i| {
+                // generate blocks with timestamps -> N, N - BT, N - 2BT, ...
+                // where N = now(), BT = block_time
+                HostBlock::generate_block_with_validators(
+                    host_id.clone(),
+                    host_type,
+                    latest_height
+                        .sub(i as u64)
+                        .expect("Never fails")
+                        .revision_height(),
+                    next_block_timestamp
+                        .sub(Duration::from_secs(
+                            DEFAULT_BLOCK_TIME_SECS * (i as u64 + 1),
+                        ))
+                        .expect("Never fails"),
+                    &validator_history[i],
+                    &validator_history[i + 1],
+                )
+            })
+            .collect();
+
+        MockContext {
+            host_chain_type: host_type,
+            host_chain_id: host_id.clone(),
+            max_history_size,
+            history,
+            block_time,
+            ibc_store: Arc::new(Mutex::new(MockIbcStore::default())),
+            events: Vec::new(),
+            logs: Vec::new(),
+        }
+    }
+
     /// Associates a client record to this context.
     /// Given a client id and a height, registers a new client in the context and also associates
     /// to this client a mock client state and a mock consensus state for height `height`. The type

crates/ibc/src/mock/host.rs

@@ -7,7 +7,10 @@
 use ibc_proto::protobuf::Protobuf as ErasedProtobuf;
 use tendermint::block::Header as TmHeader;
 use tendermint_testgen::light_block::TmLightBlock;
-use tendermint_testgen::{Generator, LightBlock as TestgenLightBlock};
+use tendermint_testgen::{
+    Generator, Header as TestgenHeader, LightBlock as TestgenLightBlock,
+    Validator as TestgenValidator,
+};
 
 use super::context::AnyConsensusState;
 use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState;
@@ -101,6 +104,38 @@
         }
     }
 
+    /// Generates a new block at `height` for the given chain identifier, chain type and validator sets.
+    pub fn generate_block_with_validators(
+        chain_id: ChainId,
+        chain_type: HostType,
+        height: u64,
+        timestamp: Timestamp,
+        validators: &[TestgenValidator],
+        next_validators: &[TestgenValidator],
+    ) -> HostBlock {
+        match chain_type {
+            HostType::Mock => HostBlock::Mock(Box::new(MockHeader {
+                height: Height::new(chain_id.revision_number(), height).expect("Never fails"),
+                timestamp,
+            })),
+            HostType::SyntheticTendermint => {
+                HostBlock::SyntheticTendermint(Box::new(SyntheticTmBlock {
+                    trusted_height: Height::new(chain_id.revision_number(), 1)
+                        .expect("Never fails"),
+                    light_block: TestgenLightBlock::new_default_with_header(
+                        TestgenHeader::new(validators)
+                            .height(height)
+                            .chain_id(&chain_id.to_string())
+                            .next_validators(next_validators)
+                            .time(timestamp.into_tm_time().expect("Never fails")),
+                    )
+                    .generate()
+                    .expect("Never fails"),
+                }))
+            }
+        }
+    }
+
     pub fn generate_tm_block(
         chain_id: ChainId,
         height: u64,