From 8fe4b42b9d6646ff3eb128542f60783ed28b08d0 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Wed, 11 Oct 2023 16:08:55 +0200 Subject: [PATCH 1/4] add reproducible test --- .../ics02_client/handler/update_client.rs | 64 ++++++++++++++++ crates/ibc/src/mock/context.rs | 73 +++++++++++++++++++ crates/ibc/src/mock/host.rs | 37 +++++++++- 3 files changed, 173 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index ebdada405..7fa6a8119 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/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,69 @@ 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, + &[ + 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(); diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index cf2b61b5f..2c19eeb85 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -13,6 +13,7 @@ use derive_more::{From, TryInto}; 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 @@ impl MockContext { } } + /// Same as ::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], + 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 diff --git a/crates/ibc/src/mock/host.rs b/crates/ibc/src/mock/host.rs index 5b782a431..18950335d 100644 --- a/crates/ibc/src/mock/host.rs +++ b/crates/ibc/src/mock/host.rs @@ -7,7 +7,10 @@ use ibc_proto::ibc::lightclients::tendermint::v1::Header as RawHeader; 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 @@ impl HostBlock { } } + /// 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, From fc941151e4bcd1bef991d3cf73e395d9ac08dc91 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Wed, 11 Oct 2023 16:16:35 +0200 Subject: [PATCH 2/4] fix doc lints --- crates/ibc/src/core/ics02_client/handler/update_client.rs | 3 +++ crates/ibc/src/mock/context.rs | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index 7fa6a8119..736fdcb54 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -281,6 +281,9 @@ mod tests { 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), diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index 2c19eeb85..e28808d44 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -314,10 +314,10 @@ impl MockContext { } } - /// Same as ::new() but with custom validator sets for each block. + /// 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. + /// `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, From 85a73777e30c6f52f8bee3fbc91c8952d5c185b3 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Wed, 11 Oct 2023 16:17:05 +0200 Subject: [PATCH 3/4] rm incorrect validation check --- crates/ibc/src/clients/ics07_tendermint/header.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/header.rs b/crates/ibc/src/clients/ics07_tendermint/header.rs index 614f4dd5a..da0167e3e 100644 --- a/crates/ibc/src/clients/ics07_tendermint/header.rs +++ b/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(()) } } From 7c91b754c71aced7a36f15e9415e7cf0ca6bb9cb Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Wed, 11 Oct 2023 16:24:57 +0200 Subject: [PATCH 4/4] changelog entry --- .../bug/911-fix-header-validation-during-tmclient-update.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/bug/911-fix-header-validation-during-tmclient-update.md diff --git a/.changelog/unreleased/bug/911-fix-header-validation-during-tmclient-update.md b/.changelog/unreleased/bug/911-fix-header-validation-during-tmclient-update.md new file mode 100644 index 000000000..ba6cdb634 --- /dev/null +++ b/.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))