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)) diff --git a/crates/ibc/src/clients/ics07_tendermint/header.rs b/crates/ibc/src/clients/ics07_tendermint/header.rs index d24cf409c..b8c2b627a 100644 --- a/crates/ibc/src/clients/ics07_tendermint/header.rs +++ b/crates/ibc/src/clients/ics07_tendermint/header.rs @@ -127,13 +127,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(()) } } 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 cad4bde1d..82c7e99ec 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -162,6 +162,7 @@ mod tests { use crate::test_utils::get_dummy_account_id; use crate::Height; use ibc_proto::ibc::lightclients::tendermint::v1::{ClientState as RawTmClientState, Fraction}; + use tendermint_testgen::Validator as TestgenValidator; #[test] fn test_update_client_ok() { @@ -258,6 +259,71 @@ 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, + &[ + // Rano: 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, + header: 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/core/ics04_channel/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/acknowledgement.rs index 3b3ebb844..9af5a7fca 100644 --- a/crates/ibc/src/core/ics04_channel/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/acknowledgement.rs @@ -51,8 +51,7 @@ impl TryFrom> for Acknowledgement { } /// Defines a convenience type for IBC applications to construct an -/// [`Acknowledgement`](super::acknowledgement::Acknowledgement) based on the -/// success or failure of processing a received packet. +/// [`Acknowledgement`] based on the success or failure of processing a received packet. #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq)] pub enum AcknowledgementStatus { @@ -67,8 +66,7 @@ pub enum AcknowledgementStatus { } /// A wrapper type that guards variants of -/// [`AcknowledgementStatus`](crate::core::ics04_channel::acknowledgement::AcknowledgementStatus) -/// against being constructed with an empty value. +/// [`AcknowledgementStatus`] against being constructed with an empty value. #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Clone, Debug, PartialEq, Eq)] pub struct StatusValue(String); diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index ff7de4c93..1dbb8d822 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -18,6 +18,7 @@ use core::time::Duration; use derive_more::{From, TryInto}; use ibc_proto::protobuf::Protobuf; use parking_lot::Mutex; +use tendermint_testgen::Validator as TestgenValidator; use ibc_proto::google::protobuf::Any; use tracing::debug; @@ -316,6 +317,78 @@ impl MockContext { } } + /// 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], + 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 6a964ea90..d1cb7d880 100644 --- a/crates/ibc/src/mock/host.rs +++ b/crates/ibc/src/mock/host.rs @@ -8,7 +8,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::{ + Commit as TestgenCommit, Generator, Header as TestgenHeader, LightBlock as TestgenLightBlock, + Validator as TestgenValidator, +}; use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::clients::ics07_tendermint::header::TENDERMINT_HEADER_TYPE_URL; @@ -103,6 +106,40 @@ 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 => { + let header = TestgenHeader::new(validators) + .height(height) + .chain_id(&chain_id.to_string()) + .next_validators(next_validators) + .time(timestamp.into_tm_time().expect("Never fails")); + + let commit = TestgenCommit::new(header.clone(), 1); + + HostBlock::SyntheticTendermint(Box::new(SyntheticTmBlock { + trusted_height: Height::new(chain_id.revision_number(), 1) + .expect("Never fails"), + light_block: TestgenLightBlock::new(header, commit) + .generate() + .expect("Never fails"), + })) + } + } + } + pub fn generate_tm_block( chain_id: ChainId, height: u64,