From 9c67dc2500f31272eced97ba7ddda0c6c87c1217 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 29 Mar 2023 13:22:43 -0400 Subject: [PATCH 01/62] fix --- crates/ibc/src/core/ics26_routing/msgs.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/core/ics26_routing/msgs.rs b/crates/ibc/src/core/ics26_routing/msgs.rs index bad168e67..36c5c3384 100644 --- a/crates/ibc/src/core/ics26_routing/msgs.rs +++ b/crates/ibc/src/core/ics26_routing/msgs.rs @@ -2,7 +2,9 @@ use crate::prelude::*; use ibc_proto::google::protobuf::Any; -use crate::core::ics02_client::msgs::{create_client, update_client, upgrade_client, ClientMsg}; +use crate::core::ics02_client::msgs::{ + create_client, misbehaviour, update_client, upgrade_client, ClientMsg, +}; use crate::core::ics03_connection::msgs::{ conn_open_ack, conn_open_confirm, conn_open_init, conn_open_try, ConnectionMsg, }; @@ -44,6 +46,11 @@ impl TryFrom for MsgEnvelope { .map_err(RouterError::MalformedMessageBytes)?; Ok(MsgEnvelope::Client(ClientMsg::UpgradeClient(domain_msg))) } + misbehaviour::TYPE_URL => { + let domain_msg = misbehaviour::MsgSubmitMisbehaviour::decode_vec(&any_msg.value) + .map_err(RouterError::MalformedMessageBytes)?; + Ok(MsgEnvelope::Client(ClientMsg::Misbehaviour(domain_msg))) + } // ICS03 conn_open_init::TYPE_URL => { From 904946fbbb4ad2f00bb4c136837804dff4bdab27 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 29 Mar 2023 13:25:18 -0400 Subject: [PATCH 02/62] changelog --- .changelog/unreleased/bug/578-misbehaviour-msgenvelope.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/bug/578-misbehaviour-msgenvelope.md diff --git a/.changelog/unreleased/bug/578-misbehaviour-msgenvelope.md b/.changelog/unreleased/bug/578-misbehaviour-msgenvelope.md new file mode 100644 index 000000000..31ed78f49 --- /dev/null +++ b/.changelog/unreleased/bug/578-misbehaviour-msgenvelope.md @@ -0,0 +1,2 @@ +- Properly convert from `Any` to `MsgEnvelope` + ([#578](https://github.com/cosmos/ibc-rs/issues/578)) From 3a1cc0c669b17bf783bb3f985ed27c8d5c385ae3 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 29 Mar 2023 16:34:49 -0400 Subject: [PATCH 03/62] verify_header contains a FIXME to double check --- .../clients/ics07_tendermint/client_state.rs | 87 +++++++++++++++++++ .../ibc/src/core/ics02_client/client_state.rs | 10 +++ .../core/ics02_client/msgs/update_client.rs | 9 ++ crates/ibc/src/mock/client_state.rs | 10 +++ 4 files changed, 116 insertions(+) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 1febe6022..8db00b081 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -1,3 +1,4 @@ +use crate::core::ics02_client::msgs::update_client::UpdateClientKind; use crate::prelude::*; use core::convert::{TryFrom, TryInto}; @@ -216,6 +217,74 @@ impl ClientState { }) } + fn verify_header( + &self, + ctx: &dyn ValidationContext, + client_id: ClientId, + header: TmHeader, + ) -> Result<(), ClientError> { + let trusted_state = { + let trusted_client_cons_state_path = + ClientConsensusStatePath::new(&client_id, &header.trusted_height); + let trusted_consensus_state = downcast_tm_consensus_state( + ctx.consensus_state(&trusted_client_cons_state_path) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })? + .as_ref(), + )?; + + TrustedBlockState { + chain_id: &self.chain_id.clone().into(), + header_time: trusted_consensus_state.timestamp, + height: header + .trusted_height + .revision_height() + .try_into() + .map_err(|_| ClientError::ClientSpecific { + description: Error::InvalidHeaderHeight { + height: header.trusted_height.revision_height(), + } + .to_string(), + })?, + next_validators: &header.trusted_validator_set, + next_validators_hash: trusted_consensus_state.next_validators_hash, + } + }; + + let untrusted_state = UntrustedBlockState { + signed_header: &header.signed_header, + validators: &header.validator_set, + // NB: This will skip the + // VerificationPredicates::next_validators_match check for the + // untrusted state. + next_validators: None, + }; + + let options = self.as_light_client_options()?; + let now = ctx + .host_timestamp() + .map_err(|e| ClientError::Other { + description: e.to_string(), + })? + .into_tm_time() + .unwrap(); + + self.verifier + .verify(untrusted_state, trusted_state, &options, now) + .into_result()?; + + // FIXME (BEFORE MERGE): + // do we need the checks under the comments "Monotonicity checks for timestamps ..." + // and "(cs-trusted, cs-prev, cs-new)"? + // Aren't these covered in the `self.verifier.verify()` call? + + Ok(()) + } + fn check_header_validator_set( trusted_consensus_state: &TmConsensusState, header: &Header, @@ -271,6 +340,9 @@ impl ClientState { ) .into_result()?; + // FIXME (BEFORE MERGE): + // + Ok(()) } @@ -342,6 +414,21 @@ impl Ics2ClientState for ClientState { TmConsensusState::try_from(consensus_state).map(TmConsensusState::into_box) } + fn verify_client_message( + &self, + ctx: &dyn ValidationContext, + client_id: ClientId, + client_message: UpdateClientKind, + ) -> Result<(), ClientError> { + match client_message { + UpdateClientKind::UpdateHeader(header) => { + let header = TmHeader::try_from(header)?; + self.verify_header(ctx, client_id, header) + } + UpdateClientKind::Misbehaviour(_) => todo!(), + } + } + fn check_misbehaviour_and_update_state( &self, ctx: &dyn ValidationContext, diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index cf03ad747..8088a48c0 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -19,6 +19,7 @@ use crate::prelude::*; use crate::Height; use super::consensus_state::ConsensusState; +use super::msgs::update_client::UpdateClientKind; use crate::core::{ContextError, ValidationContext}; @@ -67,6 +68,15 @@ pub trait ClientState: fn initialise(&self, consensus_state: Any) -> Result, ClientError>; + /// checks the that the structure of a ClientMessage is correct and that all + /// authentication data provided is valid. + fn verify_client_message( + &self, + ctx: &dyn ValidationContext, + client_id: ClientId, + client_message: UpdateClientKind, + ) -> Result<(), ClientError>; + fn check_header_and_update_state( &self, ctx: &dyn ValidationContext, diff --git a/crates/ibc/src/core/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index 57d470273..99f31274e 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -13,6 +13,15 @@ use crate::tx_msg::Msg; pub const TYPE_URL: &str = "/ibc.core.client.v1.MsgUpdateClient"; +pub enum UpdateClientKind { + /// this is the typical scenario where a new header is submitted to the client + /// to update the client + UpdateHeader(Any), + /// this is the scenario where misbehaviour is submitted to the client + /// (e.g 2 headers with the same height in Tendermint) + Misbehaviour(Any), +} + /// A type of message that triggers the update of an on-chain (IBC) client with new headers. #[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgUpdateClient { diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index ad3e3f580..695c770ac 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -1,3 +1,4 @@ +use crate::core::ics02_client::msgs::update_client::UpdateClientKind; use crate::prelude::*; use alloc::collections::btree_map::BTreeMap as HashMap; @@ -285,6 +286,15 @@ impl ClientState for MockClientState { ) -> Result<(), ClientError> { Ok(()) } + + fn verify_client_message( + &self, + _ctx: &dyn ValidationContext, + _client_id: ClientId, + _client_message: UpdateClientKind, + ) -> Result<(), ClientError> { + Ok(()) + } } impl From for MockClientState { From 550ac794504ebce5ad14e2a8498a46f86913c04e Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 30 Mar 2023 13:59:33 -0400 Subject: [PATCH 04/62] fix header.trusted_next_validator_set hash bug --- .../clients/ics07_tendermint/client_state.rs | 21 +++++++++++++++---- .../src/clients/ics07_tendermint/header.rs | 15 +++++++------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 8db00b081..b8c3f6c02 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -237,6 +237,19 @@ impl ClientState { .as_ref(), )?; + // `header.trusted_validator_set` was given to us by the relayer. Thus, we need to + // ensure that the relayer gave us the right set, according to the hash we have + // stored on chain. + if header.trusted_next_validator_set.hash() + != trusted_consensus_state.next_validators_hash + { + return Err(ClientError::HeaderVerificationFailure { + reason: + "header trusted next validator set hash does not match hash stored on chain" + .to_string(), + }); + } + TrustedBlockState { chain_id: &self.chain_id.clone().into(), header_time: trusted_consensus_state.timestamp, @@ -250,7 +263,7 @@ impl ClientState { } .to_string(), })?, - next_validators: &header.trusted_validator_set, + next_validators: &header.trusted_next_validator_set, next_validators_hash: trusted_consensus_state.next_validators_hash, } }; @@ -289,11 +302,11 @@ impl ClientState { trusted_consensus_state: &TmConsensusState, header: &Header, ) -> Result<(), ClientError> { - let trusted_val_hash = header.trusted_validator_set.hash(); + let trusted_val_hash = header.trusted_next_validator_set.hash(); if trusted_consensus_state.next_validators_hash != trusted_val_hash { return Err(Error::MisbehaviourTrustedValidatorHashMismatch { - trusted_validator_set: header.trusted_validator_set.validators().clone(), + trusted_validator_set: header.trusted_next_validator_set.validators().clone(), next_validators_hash: trusted_consensus_state.next_validators_hash, trusted_val_hash, } @@ -584,7 +597,7 @@ impl Ics2ClientState for ClientState { } .to_string(), })?, - next_validators: &header.trusted_validator_set, + next_validators: &header.trusted_next_validator_set, next_validators_hash: trusted_consensus_state.next_validators_hash, }; diff --git a/crates/ibc/src/clients/ics07_tendermint/header.rs b/crates/ibc/src/clients/ics07_tendermint/header.rs index 33667db92..5290583db 100644 --- a/crates/ibc/src/clients/ics07_tendermint/header.rs +++ b/crates/ibc/src/clients/ics07_tendermint/header.rs @@ -29,8 +29,7 @@ pub struct Header { pub signed_header: SignedHeader, // contains the commitment root pub validator_set: ValidatorSet, // the validator set that signed Header pub trusted_height: Height, // the height of a trusted header seen by client less than or equal to Header - // TODO(thane): Rename this to trusted_next_validator_set? - pub trusted_validator_set: ValidatorSet, // the last trusted validator set at trusted height + pub trusted_next_validator_set: ValidatorSet, // the last trusted validator set at trusted height } impl core::fmt::Debug for Header { @@ -41,7 +40,7 @@ impl core::fmt::Debug for Header { impl Display for Header { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { - write!(f, "Header {{ signed_header: {}, validator_set: {}, trusted_height: {}, trusted_validator_set: {} }}", PrettySignedHeader(&self.signed_header), PrettyValidatorSet(&self.validator_set), self.trusted_height, PrettyValidatorSet(&self.trusted_validator_set)) + write!(f, "Header {{ signed_header: {}, validator_set: {}, trusted_height: {}, trusted_validator_set: {} }}", PrettySignedHeader(&self.signed_header), PrettyValidatorSet(&self.validator_set), self.trusted_height, PrettyValidatorSet(&self.trusted_next_validator_set)) } } @@ -81,7 +80,7 @@ impl Header { .map_err(|_| Error::InvalidHeaderHeight { height: self.trusted_height.revision_height(), })?, - next_validators: &self.trusted_validator_set, + next_validators: &self.trusted_next_validator_set, next_validators_hash: consensus_state.next_validators_hash, }) } @@ -141,7 +140,7 @@ impl TryFrom for Header { .trusted_height .and_then(|raw_height| raw_height.try_into().ok()) .ok_or(Error::MissingTrustedHeight)?, - trusted_validator_set: raw + trusted_next_validator_set: raw .trusted_validators .ok_or(Error::MissingTrustedValidatorSet)? .try_into() @@ -196,7 +195,7 @@ impl From
for RawHeader { signed_header: Some(value.signed_header.into()), validator_set: Some(value.validator_set.into()), trusted_height: Some(value.trusted_height.into()), - trusted_validators: Some(value.trusted_validator_set.into()), + trusted_validators: Some(value.trusted_next_validator_set.into()), } } } @@ -263,7 +262,7 @@ pub mod test_util { signed_header: shdr, validator_set: vs.clone(), trusted_height: Height::new(0, 1).unwrap(), - trusted_validator_set: vs, + trusted_next_validator_set: vs, } } @@ -277,7 +276,7 @@ pub mod test_util { signed_header: light_block.signed_header, validator_set: light_block.validators, trusted_height, - trusted_validator_set: light_block.next_validators, + trusted_next_validator_set: light_block.next_validators, } } } From 39d96898d45c976488acbde77eb292f4ba3a0b2f Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 30 Mar 2023 13:59:38 -0400 Subject: [PATCH 05/62] fmt --- crates/ibc/src/core/ics02_client/msgs/update_client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index 99f31274e..accdc6409 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -15,9 +15,9 @@ pub const TYPE_URL: &str = "/ibc.core.client.v1.MsgUpdateClient"; pub enum UpdateClientKind { /// this is the typical scenario where a new header is submitted to the client - /// to update the client + /// to update the client UpdateHeader(Any), - /// this is the scenario where misbehaviour is submitted to the client + /// this is the scenario where misbehaviour is submitted to the client /// (e.g 2 headers with the same height in Tendermint) Misbehaviour(Any), } From 43832143420d3685bb6b188fd2dc762b1db58b53 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 30 Mar 2023 14:33:45 -0400 Subject: [PATCH 06/62] remove bad comment --- crates/ibc/src/clients/ics07_tendermint/client_state.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index b8c3f6c02..28b43ebc7 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -353,9 +353,6 @@ impl ClientState { ) .into_result()?; - // FIXME (BEFORE MERGE): - // - Ok(()) } From ce1d0f3e90347121a75cfb2886983779f2690ac8 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 30 Mar 2023 16:55:58 -0400 Subject: [PATCH 07/62] add timestamp monotonicity checks to `verify_header()` --- .../clients/ics07_tendermint/client_state.rs | 62 +++++++++++++++++-- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 28b43ebc7..70d0a8e5d 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -290,10 +290,64 @@ impl ClientState { .verify(untrusted_state, trusted_state, &options, now) .into_result()?; - // FIXME (BEFORE MERGE): - // do we need the checks under the comments "Monotonicity checks for timestamps ..." - // and "(cs-trusted, cs-prev, cs-new)"? - // Aren't these covered in the `self.verifier.verify()` call? + // The final 2 checks ensure that: + // 1. for all headers, the new header has a larger timestamp than the “previous header” + // 2. if a header comes in and is not the “last” header, then we also ensure that its timestamp + // is less than the “next header” + // Together, these 2 checks ensure the monotonicity of timestamps. + let maybe_prev_cs = ctx + .prev_consensus_state(&client_id, &header.height()) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })? + .as_ref() + .map(|cs| downcast_tm_consensus_state(cs.as_ref())) + .transpose()?; + + if let Some(prev_cs) = maybe_prev_cs { + // New (untrusted) header timestamp cannot occur before the + // previous consensus state's height + if header.signed_header.header().time < prev_cs.timestamp { + return Err(ClientError::ClientSpecific { + description: Error::HeaderTimestampTooLow { + actual: header.signed_header.header().time.to_string(), + min: prev_cs.timestamp.to_string(), + } + .to_string(), + }); + } + } + + if header.height() < self.latest_height() { + let maybe_next_cs = ctx + .next_consensus_state(&client_id, &header.height()) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })? + .as_ref() + .map(|cs| downcast_tm_consensus_state(cs.as_ref())) + .transpose()?; + + if let Some(next_cs) = maybe_next_cs { + // New (untrusted) header timestamp cannot occur after next + // consensus state's height + if header.signed_header.header().time > next_cs.timestamp { + return Err(ClientError::ClientSpecific { + description: Error::HeaderTimestampTooHigh { + actual: header.signed_header.header().time.to_string(), + max: next_cs.timestamp.to_string(), + } + .to_string(), + }); + } + } + } Ok(()) } From 7b29b2a2bc57a872dd7f1a7659ce6bdff6c389c4 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 30 Mar 2023 17:03:01 -0400 Subject: [PATCH 08/62] clean up verify_header --- .../clients/ics07_tendermint/client_state.rs | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 70d0a8e5d..43fdef112 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -286,6 +286,7 @@ impl ClientState { .into_tm_time() .unwrap(); + // main header verification, delegated to the tendermint-light-client crate. self.verifier .verify(untrusted_state, trusted_state, &options, now) .into_result()?; @@ -295,32 +296,36 @@ impl ClientState { // 2. if a header comes in and is not the “last” header, then we also ensure that its timestamp // is less than the “next header” // Together, these 2 checks ensure the monotonicity of timestamps. - let maybe_prev_cs = ctx - .prev_consensus_state(&client_id, &header.height()) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })? - .as_ref() - .map(|cs| downcast_tm_consensus_state(cs.as_ref())) - .transpose()?; - - if let Some(prev_cs) = maybe_prev_cs { - // New (untrusted) header timestamp cannot occur before the - // previous consensus state's height - if header.signed_header.header().time < prev_cs.timestamp { - return Err(ClientError::ClientSpecific { - description: Error::HeaderTimestampTooLow { - actual: header.signed_header.header().time.to_string(), - min: prev_cs.timestamp.to_string(), - } - .to_string(), - }); + + // 1. + { + let maybe_prev_cs = ctx + .prev_consensus_state(&client_id, &header.height()) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })?; + + if let Some(prev_cs) = maybe_prev_cs { + // New header timestamp cannot occur *before* the + // previous consensus state's height + let prev_cs = downcast_tm_consensus_state(prev_cs.as_ref())?; + + if header.signed_header.header().time < prev_cs.timestamp { + return Err(ClientError::ClientSpecific { + description: Error::HeaderTimestampTooLow { + actual: header.signed_header.header().time.to_string(), + min: prev_cs.timestamp.to_string(), + } + .to_string(), + }); + } } } + // 2. if header.height() < self.latest_height() { let maybe_next_cs = ctx .next_consensus_state(&client_id, &header.height()) @@ -329,14 +334,13 @@ impl ClientState { _ => ClientError::Other { description: e.to_string(), }, - })? - .as_ref() - .map(|cs| downcast_tm_consensus_state(cs.as_ref())) - .transpose()?; + })?; if let Some(next_cs) = maybe_next_cs { - // New (untrusted) header timestamp cannot occur after next + // New (untrusted) header timestamp cannot occur *after* next // consensus state's height + let next_cs = downcast_tm_consensus_state(next_cs.as_ref())?; + if header.signed_header.header().time > next_cs.timestamp { return Err(ClientError::ClientSpecific { description: Error::HeaderTimestampTooHigh { From 4a8dfc7cc24c8eaeca47c1c7f271d960d1045827 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 30 Mar 2023 17:14:02 -0400 Subject: [PATCH 09/62] add revision number check in verify_header --- .../clients/ics07_tendermint/client_state.rs | 133 ++++++++++-------- 1 file changed, 74 insertions(+), 59 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 43fdef112..e34934d7b 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -223,73 +223,88 @@ impl ClientState { client_id: ClientId, header: TmHeader, ) -> Result<(), ClientError> { - let trusted_state = { - let trusted_client_cons_state_path = - ClientConsensusStatePath::new(&client_id, &header.trusted_height); - let trusted_consensus_state = downcast_tm_consensus_state( - ctx.consensus_state(&trusted_client_cons_state_path) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })? - .as_ref(), - )?; - - // `header.trusted_validator_set` was given to us by the relayer. Thus, we need to - // ensure that the relayer gave us the right set, according to the hash we have - // stored on chain. - if header.trusted_next_validator_set.hash() - != trusted_consensus_state.next_validators_hash - { - return Err(ClientError::HeaderVerificationFailure { + // The tendermint-light-client crate though works on heights that are assumed + // to have the same revision number. We ensure this here. + if header.height().revision_number() != self.chain_id().version() { + return Err(ClientError::ClientSpecific { + description: Error::MismatchedRevisions { + current_revision: self.chain_id().version(), + update_revision: header.height().revision_number(), + } + .to_string(), + }); + } + + // Call into tendermint-light-client, which contains (almost) all the verification we need + { + let trusted_state = + { + let trusted_client_cons_state_path = + ClientConsensusStatePath::new(&client_id, &header.trusted_height); + let trusted_consensus_state = downcast_tm_consensus_state( + ctx.consensus_state(&trusted_client_cons_state_path) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })? + .as_ref(), + )?; + + // `header.trusted_validator_set` was given to us by the relayer. Thus, we need to + // ensure that the relayer gave us the right set, according to the hash we have + // stored on chain. + if header.trusted_next_validator_set.hash() + != trusted_consensus_state.next_validators_hash + { + return Err(ClientError::HeaderVerificationFailure { reason: "header trusted next validator set hash does not match hash stored on chain" .to_string(), }); - } + } - TrustedBlockState { - chain_id: &self.chain_id.clone().into(), - header_time: trusted_consensus_state.timestamp, - height: header - .trusted_height - .revision_height() - .try_into() - .map_err(|_| ClientError::ClientSpecific { - description: Error::InvalidHeaderHeight { - height: header.trusted_height.revision_height(), - } - .to_string(), - })?, - next_validators: &header.trusted_next_validator_set, - next_validators_hash: trusted_consensus_state.next_validators_hash, - } - }; + TrustedBlockState { + chain_id: &self.chain_id.clone().into(), + header_time: trusted_consensus_state.timestamp, + height: header.trusted_height.revision_height().try_into().map_err( + |_| ClientError::ClientSpecific { + description: Error::InvalidHeaderHeight { + height: header.trusted_height.revision_height(), + } + .to_string(), + }, + )?, + next_validators: &header.trusted_next_validator_set, + next_validators_hash: trusted_consensus_state.next_validators_hash, + } + }; + + let untrusted_state = UntrustedBlockState { + signed_header: &header.signed_header, + validators: &header.validator_set, + // NB: This will skip the + // VerificationPredicates::next_validators_match check for the + // untrusted state. + next_validators: None, + }; - let untrusted_state = UntrustedBlockState { - signed_header: &header.signed_header, - validators: &header.validator_set, - // NB: This will skip the - // VerificationPredicates::next_validators_match check for the - // untrusted state. - next_validators: None, - }; + let options = self.as_light_client_options()?; + let now = ctx + .host_timestamp() + .map_err(|e| ClientError::Other { + description: e.to_string(), + })? + .into_tm_time() + .unwrap(); - let options = self.as_light_client_options()?; - let now = ctx - .host_timestamp() - .map_err(|e| ClientError::Other { - description: e.to_string(), - })? - .into_tm_time() - .unwrap(); + // main header verification, delegated to the tendermint-light-client crate. + self.verifier + .verify(untrusted_state, trusted_state, &options, now) + .into_result()?; + } - // main header verification, delegated to the tendermint-light-client crate. - self.verifier - .verify(untrusted_state, trusted_state, &options, now) - .into_result()?; // The final 2 checks ensure that: // 1. for all headers, the new header has a larger timestamp than the “previous header” From be78970437ecfac3297ca06cd4b0daee99b9d4c5 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 30 Mar 2023 17:28:38 -0400 Subject: [PATCH 10/62] header height extra check --- .../src/clients/ics07_tendermint/client_state.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index e34934d7b..70a109877 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -235,6 +235,20 @@ impl ClientState { }); } + // We also need to ensure that the trusted height (representing the + // height of the header already on chain for which this client update is + // based on) must be smaller than height of the new header that we're + // installing. + if header.height() <= header.trusted_height { + return Err(ClientError::HeaderVerificationFailure { + reason: format!( + "header height <= header trusted height ({} <= {})", + header.height(), + header.trusted_height + ), + }); + } + // Call into tendermint-light-client, which contains (almost) all the verification we need { let trusted_state = @@ -305,7 +319,6 @@ impl ClientState { .into_result()?; } - // The final 2 checks ensure that: // 1. for all headers, the new header has a larger timestamp than the “previous header” // 2. if a header comes in and is not the “last” header, then we also ensure that its timestamp From e92697cac31edb9cf909a298e1139ed414652420 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 31 Mar 2023 09:05:06 -0400 Subject: [PATCH 11/62] clarifying comment --- crates/ibc/src/clients/ics07_tendermint/client_state.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 70a109877..de6e0e2b3 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -249,7 +249,8 @@ impl ClientState { }); } - // Call into tendermint-light-client, which contains (almost) all the verification we need + // Delegate to tendermint-light-client, which contains the required checks + // of the new header against the trusted consensus state. { let trusted_state = { From 487195cb9692c4ddf45e031fda957dd589e548d9 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 31 Mar 2023 10:38:48 -0400 Subject: [PATCH 12/62] check_misbehaviour_header scaffolding --- .../clients/ics07_tendermint/client_state.rs | 163 +++++++++++++----- 1 file changed, 119 insertions(+), 44 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index de6e0e2b3..df9f37923 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -217,6 +217,26 @@ impl ClientState { }) } + // `header.trusted_validator_set` was given to us by the relayer. Thus, we + // need to ensure that the relayer gave us the right set, i.e. by ensuring + // that it matches the hash we have stored on chain. + fn check_header_trusted_next_validator_set( + &self, + header: &TmHeader, + trusted_consensus_state: &TmConsensusState, + ) -> Result<(), ClientError> { + if header.trusted_next_validator_set.hash() == trusted_consensus_state.next_validators_hash + { + Ok(()) + } else { + Err(ClientError::HeaderVerificationFailure { + reason: + "header trusted next validator set hash does not match hash stored on chain" + .to_string(), + }) + } + } + fn verify_header( &self, ctx: &dyn ValidationContext, @@ -249,52 +269,42 @@ impl ClientState { }); } - // Delegate to tendermint-light-client, which contains the required checks + // Delegate to tendermint-light-client, which contains the required checks // of the new header against the trusted consensus state. { - let trusted_state = - { - let trusted_client_cons_state_path = - ClientConsensusStatePath::new(&client_id, &header.trusted_height); - let trusted_consensus_state = downcast_tm_consensus_state( - ctx.consensus_state(&trusted_client_cons_state_path) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })? - .as_ref(), - )?; - - // `header.trusted_validator_set` was given to us by the relayer. Thus, we need to - // ensure that the relayer gave us the right set, according to the hash we have - // stored on chain. - if header.trusted_next_validator_set.hash() - != trusted_consensus_state.next_validators_hash - { - return Err(ClientError::HeaderVerificationFailure { - reason: - "header trusted next validator set hash does not match hash stored on chain" - .to_string(), - }); - } - - TrustedBlockState { - chain_id: &self.chain_id.clone().into(), - header_time: trusted_consensus_state.timestamp, - height: header.trusted_height.revision_height().try_into().map_err( - |_| ClientError::ClientSpecific { - description: Error::InvalidHeaderHeight { - height: header.trusted_height.revision_height(), - } - .to_string(), + let trusted_state = { + let trusted_client_cons_state_path = + ClientConsensusStatePath::new(&client_id, &header.trusted_height); + let trusted_consensus_state = downcast_tm_consensus_state( + ctx.consensus_state(&trusted_client_cons_state_path) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), }, - )?, - next_validators: &header.trusted_next_validator_set, - next_validators_hash: trusted_consensus_state.next_validators_hash, - } - }; + })? + .as_ref(), + )?; + + self.check_header_trusted_next_validator_set(&header, &trusted_consensus_state)?; + + TrustedBlockState { + chain_id: &self.chain_id.clone().into(), + header_time: trusted_consensus_state.timestamp, + height: header + .trusted_height + .revision_height() + .try_into() + .map_err(|_| ClientError::ClientSpecific { + description: Error::InvalidHeaderHeight { + height: header.trusted_height.revision_height(), + } + .to_string(), + })?, + next_validators: &header.trusted_next_validator_set, + next_validators_hash: trusted_consensus_state.next_validators_hash, + } + }; let untrusted_state = UntrustedBlockState { signed_header: &header.signed_header, @@ -385,6 +395,68 @@ impl ClientState { Ok(()) } + // verify_misbehaviour determines whether or not two conflicting headers at + // the same height would have convinced the light client. + fn verify_misbehaviour( + &self, + ctx: &dyn ValidationContext, + client_id: ClientId, + misbehaviour: TmMisbehaviour, + ) -> Result<(), ClientError> { + let header_1 = misbehaviour.header1(); + let trusted_consensus_state_1 = { + let consensus_state_path = + ClientConsensusStatePath::new(&client_id, &header_1.trusted_height); + let consensus_state = + ctx.consensus_state(&consensus_state_path) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })?; + + downcast_tm_consensus_state(consensus_state.as_ref()) + }?; + + let header_2 = misbehaviour.header2(); + let trusted_consensus_state_2 = { + let consensus_state_path = + ClientConsensusStatePath::new(&client_id, &header_2.trusted_height); + let consensus_state = + ctx.consensus_state(&consensus_state_path) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })?; + + downcast_tm_consensus_state(consensus_state.as_ref()) + }?; + + let current_timestamp = ctx.host_timestamp().map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })?; + + self.check_misbehaviour_header(header_1, &trusted_consensus_state_1, current_timestamp)?; + self.check_misbehaviour_header(header_2, &trusted_consensus_state_2, current_timestamp) + } + + fn check_misbehaviour_header( + &self, + header: &TmHeader, + trusted_consensus_state: &TmConsensusState, + _current_timestamp: Timestamp, + ) -> Result<(), ClientError> { + self.check_header_trusted_next_validator_set(header, trusted_consensus_state)?; + + todo!() + } + fn check_header_validator_set( trusted_consensus_state: &TmConsensusState, header: &Header, @@ -522,7 +594,10 @@ impl Ics2ClientState for ClientState { let header = TmHeader::try_from(header)?; self.verify_header(ctx, client_id, header) } - UpdateClientKind::Misbehaviour(_) => todo!(), + UpdateClientKind::Misbehaviour(misbehaviour) => { + let misbehaviour = TmMisbehaviour::try_from(misbehaviour)?; + self.verify_misbehaviour(ctx, client_id, misbehaviour) + } } } From d4f22b8b6be718b9b2c3fd73e29c8fe12fb437de Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 31 Mar 2023 10:51:06 -0400 Subject: [PATCH 13/62] remove duplicate methods --- .../clients/ics07_tendermint/client_state.rs | 61 ++++++------------- 1 file changed, 20 insertions(+), 41 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index df9f37923..419e49c37 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -217,26 +217,6 @@ impl ClientState { }) } - // `header.trusted_validator_set` was given to us by the relayer. Thus, we - // need to ensure that the relayer gave us the right set, i.e. by ensuring - // that it matches the hash we have stored on chain. - fn check_header_trusted_next_validator_set( - &self, - header: &TmHeader, - trusted_consensus_state: &TmConsensusState, - ) -> Result<(), ClientError> { - if header.trusted_next_validator_set.hash() == trusted_consensus_state.next_validators_hash - { - Ok(()) - } else { - Err(ClientError::HeaderVerificationFailure { - reason: - "header trusted next validator set hash does not match hash stored on chain" - .to_string(), - }) - } - } - fn verify_header( &self, ctx: &dyn ValidationContext, @@ -286,7 +266,7 @@ impl ClientState { .as_ref(), )?; - self.check_header_trusted_next_validator_set(&header, &trusted_consensus_state)?; + check_header_trusted_next_validator_set(&header, &trusted_consensus_state)?; TrustedBlockState { chain_id: &self.chain_id.clone().into(), @@ -452,36 +432,18 @@ impl ClientState { trusted_consensus_state: &TmConsensusState, _current_timestamp: Timestamp, ) -> Result<(), ClientError> { - self.check_header_trusted_next_validator_set(header, trusted_consensus_state)?; + check_header_trusted_next_validator_set(header, trusted_consensus_state)?; todo!() } - fn check_header_validator_set( - trusted_consensus_state: &TmConsensusState, - header: &Header, - ) -> Result<(), ClientError> { - let trusted_val_hash = header.trusted_next_validator_set.hash(); - - if trusted_consensus_state.next_validators_hash != trusted_val_hash { - return Err(Error::MisbehaviourTrustedValidatorHashMismatch { - trusted_validator_set: header.trusted_next_validator_set.validators().clone(), - next_validators_hash: trusted_consensus_state.next_validators_hash, - trusted_val_hash, - } - .into()); - } - - Ok(()) - } - fn check_header_and_validator_set( &self, header: &Header, consensus_state: &TmConsensusState, current_timestamp: Timestamp, ) -> Result<(), ClientError> { - Self::check_header_validator_set(consensus_state, header)?; + check_header_trusted_next_validator_set(header,consensus_state)?; let duration_since_consensus_state = current_timestamp .duration_since(&consensus_state.timestamp()) @@ -1055,6 +1017,23 @@ impl Ics2ClientState for ClientState { } } +// `header.trusted_validator_set` was given to us by the relayer. Thus, we +// need to ensure that the relayer gave us the right set, i.e. by ensuring +// that it matches the hash we have stored on chain. +fn check_header_trusted_next_validator_set( + header: &TmHeader, + trusted_consensus_state: &TmConsensusState, +) -> Result<(), ClientError> { + if header.trusted_next_validator_set.hash() == trusted_consensus_state.next_validators_hash { + Ok(()) + } else { + Err(ClientError::HeaderVerificationFailure { + reason: "header trusted next validator set hash does not match hash stored on chain" + .to_string(), + }) + } +} + fn downcast_tm_client_state(cs: &dyn Ics2ClientState) -> Result<&ClientState, ClientError> { cs.as_any() .downcast_ref::() From bc267c858a178f1121f227fecdc9ea3f92786977 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 31 Mar 2023 11:01:36 -0400 Subject: [PATCH 14/62] verify_misbehaviour done as ibc-go --- .../clients/ics07_tendermint/client_state.rs | 89 +++++++++++-------- 1 file changed, 54 insertions(+), 35 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 419e49c37..3eff5fc5f 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -252,39 +252,38 @@ impl ClientState { // Delegate to tendermint-light-client, which contains the required checks // of the new header against the trusted consensus state. { - let trusted_state = { - let trusted_client_cons_state_path = - ClientConsensusStatePath::new(&client_id, &header.trusted_height); - let trusted_consensus_state = downcast_tm_consensus_state( - ctx.consensus_state(&trusted_client_cons_state_path) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), + let trusted_state = + { + let trusted_client_cons_state_path = + ClientConsensusStatePath::new(&client_id, &header.trusted_height); + let trusted_consensus_state = downcast_tm_consensus_state( + ctx.consensus_state(&trusted_client_cons_state_path) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })? + .as_ref(), + )?; + + check_header_trusted_next_validator_set(&header, &trusted_consensus_state)?; + + TrustedBlockState { + chain_id: &self.chain_id.clone().into(), + header_time: trusted_consensus_state.timestamp, + height: header.trusted_height.revision_height().try_into().map_err( + |_| ClientError::ClientSpecific { + description: Error::InvalidHeaderHeight { + height: header.trusted_height.revision_height(), + } + .to_string(), }, - })? - .as_ref(), - )?; - - check_header_trusted_next_validator_set(&header, &trusted_consensus_state)?; - - TrustedBlockState { - chain_id: &self.chain_id.clone().into(), - header_time: trusted_consensus_state.timestamp, - height: header - .trusted_height - .revision_height() - .try_into() - .map_err(|_| ClientError::ClientSpecific { - description: Error::InvalidHeaderHeight { - height: header.trusted_height.revision_height(), - } - .to_string(), - })?, - next_validators: &header.trusted_next_validator_set, - next_validators_hash: trusted_consensus_state.next_validators_hash, - } - }; + )?, + next_validators: &header.trusted_next_validator_set, + next_validators_hash: trusted_consensus_state.next_validators_hash, + } + }; let untrusted_state = UntrustedBlockState { signed_header: &header.signed_header, @@ -430,11 +429,31 @@ impl ClientState { &self, header: &TmHeader, trusted_consensus_state: &TmConsensusState, - _current_timestamp: Timestamp, + current_timestamp: Timestamp, ) -> Result<(), ClientError> { + // ensure correctness of the trusted next validator set provided by the relayer check_header_trusted_next_validator_set(header, trusted_consensus_state)?; - todo!() + // ensure header timestamp is within trusted period from the trusted consensus state + { + let duration_since_consensus_state = current_timestamp + .duration_since(&trusted_consensus_state.timestamp()) + .ok_or_else(|| ClientError::InvalidConsensusStateTimestamp { + time1: trusted_consensus_state.timestamp(), + time2: current_timestamp, + })?; + + if duration_since_consensus_state >= self.trusting_period { + return Err(Error::ConsensusStateTimestampGteTrustingPeriod { + duration_since_consensus_state, + trusting_period: self.trusting_period, + } + .into()); + } + } + + // ensure that 2/3 of trusted validators have signed the new header + self.verify_header_commit_against_trusted(header, trusted_consensus_state) } fn check_header_and_validator_set( @@ -443,7 +462,7 @@ impl ClientState { consensus_state: &TmConsensusState, current_timestamp: Timestamp, ) -> Result<(), ClientError> { - check_header_trusted_next_validator_set(header,consensus_state)?; + check_header_trusted_next_validator_set(header, consensus_state)?; let duration_since_consensus_state = current_timestamp .duration_since(&consensus_state.timestamp()) From cb9d53fdf488fe5d2fbb02e85b270996791cfa88 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 31 Mar 2023 12:01:13 -0400 Subject: [PATCH 15/62] fix chain ID before commit verification --- .../clients/ics07_tendermint/client_state.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 3eff5fc5f..bbf103070 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -453,7 +453,23 @@ impl ClientState { } // ensure that 2/3 of trusted validators have signed the new header - self.verify_header_commit_against_trusted(header, trusted_consensus_state) + { + let untrusted_state = header.as_untrusted_block_state(); + let chain_id = self + .chain_id + .clone() + .with_version(header.height().revision_number()) + .into(); + let trusted_state = + Header::as_trusted_block_state(header, trusted_consensus_state, &chain_id)?; + let options = self.as_light_client_options()?; + + self.verifier + .verify_commit_against_trusted(&untrusted_state, &trusted_state, &options) + .into_result()?; + } + + Ok(()) } fn check_header_and_validator_set( From a4af3475db2a02ae699b3659367347ef3733513a Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 31 Mar 2023 15:53:13 -0400 Subject: [PATCH 16/62] finish verify_misbehavior --- .../clients/ics07_tendermint/client_state.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index bbf103070..3563196d1 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -469,6 +469,25 @@ impl ClientState { .into_result()?; } + // run the verification checks on the header based on the trusted + // consensus state + { + let untrusted_state = header.as_untrusted_block_state(); + let chain_id = self.chain_id.clone().into(); + let trusted_state = + header.as_trusted_block_state(trusted_consensus_state, &chain_id)?; + let options = self.as_light_client_options()?; + + self.verifier + .validate_against_trusted( + &untrusted_state, + &trusted_state, + &options, + current_timestamp.into_tm_time().unwrap(), + ) + .into_result()?; + } + Ok(()) } From 6c68a05f9d84d92b09fc7672de2245c56da24e60 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 3 Apr 2023 12:01:45 -0400 Subject: [PATCH 17/62] `check_for_misbehaviour` header variant implementation --- .../clients/ics07_tendermint/client_state.rs | 46 +++++++++++++++++++ .../ibc/src/core/ics02_client/client_state.rs | 8 ++++ crates/ibc/src/mock/client_state.rs | 11 ++++- 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 3563196d1..98663fc79 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -617,6 +617,52 @@ impl Ics2ClientState for ClientState { } } + // The misbehaviour checked for depends on the kind of message submitted: + // + For a submitted `Header`, detects duplicate height misbehaviour and BFT time violation misbehaviour + // + For a submitted `Misbehaviour`, verifies the correctness of the message + fn check_for_misbehaviour( + &self, + ctx: &dyn ValidationContext, + client_id: ClientId, + client_message: UpdateClientKind, + ) -> Result { + match client_message { + UpdateClientKind::UpdateHeader(header) => { + let header = TmHeader::try_from(header)?; + + let header_consensus_state = TmConsensusState::from(header.clone()); + + let maybe_existing_consensus_state = { + let path_at_header_height = + ClientConsensusStatePath::new(&client_id, &header.height()); + + ctx.consensus_state(&path_at_header_height).ok() + }; + + match maybe_existing_consensus_state { + Some(existing_consensus_state) => { + let existing_consensus_state = + downcast_tm_consensus_state(existing_consensus_state.as_ref())?; + + // check if there is already a consensus state stored for the + // submitted header. If there is, there is no misbehaviour to report. + // Otherwise, we just confirmed that 2 headers exist with the same + // height, which is evidence of misbehaviour. + Ok(existing_consensus_state != header_consensus_state) + } + None => { + // FIXME timestamp monotonicity checks ? + // awaiting confirmation of whether we should do them here or in `check_client_message` + todo!() + } + } + } + UpdateClientKind::Misbehaviour(_misbehaviour) => { + todo!() + } + } + } + fn check_misbehaviour_and_update_state( &self, ctx: &dyn ValidationContext, diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 8088a48c0..a49601d7f 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -77,6 +77,14 @@ pub trait ClientState: client_message: UpdateClientKind, ) -> Result<(), ClientError>; + /// Check whether misbehaviour has occured + fn check_for_misbehaviour( + &self, + ctx: &dyn ValidationContext, + client_id: ClientId, + client_message: UpdateClientKind, + ) -> Result; + fn check_header_and_update_state( &self, ctx: &dyn ValidationContext, diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 695c770ac..0072bd8a5 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -293,7 +293,16 @@ impl ClientState for MockClientState { _client_id: ClientId, _client_message: UpdateClientKind, ) -> Result<(), ClientError> { - Ok(()) + todo!() + } + + fn check_for_misbehaviour( + &self, + _ctx: &dyn ValidationContext, + _client_id: ClientId, + _client_message: UpdateClientKind, + ) -> Result { + todo!() } } From 5f912bba964cbe0c4fe1715bf9b729ebbe8e0094 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 3 Apr 2023 12:34:23 -0400 Subject: [PATCH 18/62] check_for_misbehaviour, misbehaviour variant --- .../clients/ics07_tendermint/client_state.rs | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 98663fc79..b4097bc38 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -657,8 +657,28 @@ impl Ics2ClientState for ClientState { } } } - UpdateClientKind::Misbehaviour(_misbehaviour) => { - todo!() + UpdateClientKind::Misbehaviour(misbehaviour) => { + let misbehaviour = TmMisbehaviour::try_from(misbehaviour)?; + let header_1 = misbehaviour.header1(); + let header_2 = misbehaviour.header2(); + + if header_1.height() == header_2.height() { + // when the height of the 2 headers are equal, we only have evidence + // of misbehaviour in the case where the headers are different + // (otherwise, the same header was added twice in the message, + // and this is evidence of nothing) + Ok(header_1.signed_header.commit.block_id.hash + != header_2.signed_header.commit.block_id.hash) + } else { + // FIXME BEFORE MERGE: ibc-go ensures that header_1.height > header_2.height + // in Misbehaviour.ValidateBasic(). We are missing all these checks. + + // header_1 is at greater height than header_2, therefore + // header_1 time must be less than or equal to + // header_2 time in order to be valid misbehaviour (violation of + // monotonic time). + Ok(header_1.signed_header.header.time <= header_2.signed_header.header.time) + } } } } From c9651058198f2e5b8bb83bdaf6e364efb25832ed Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 3 Apr 2023 13:40:45 -0400 Subject: [PATCH 19/62] monotonicity of timestamps in update_client --- .../clients/ics07_tendermint/client_state.rs | 114 ++++++++---------- 1 file changed, 49 insertions(+), 65 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index b4097bc38..2b5e765be 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -309,68 +309,6 @@ impl ClientState { .into_result()?; } - // The final 2 checks ensure that: - // 1. for all headers, the new header has a larger timestamp than the “previous header” - // 2. if a header comes in and is not the “last” header, then we also ensure that its timestamp - // is less than the “next header” - // Together, these 2 checks ensure the monotonicity of timestamps. - - // 1. - { - let maybe_prev_cs = ctx - .prev_consensus_state(&client_id, &header.height()) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })?; - - if let Some(prev_cs) = maybe_prev_cs { - // New header timestamp cannot occur *before* the - // previous consensus state's height - let prev_cs = downcast_tm_consensus_state(prev_cs.as_ref())?; - - if header.signed_header.header().time < prev_cs.timestamp { - return Err(ClientError::ClientSpecific { - description: Error::HeaderTimestampTooLow { - actual: header.signed_header.header().time.to_string(), - min: prev_cs.timestamp.to_string(), - } - .to_string(), - }); - } - } - } - - // 2. - if header.height() < self.latest_height() { - let maybe_next_cs = ctx - .next_consensus_state(&client_id, &header.height()) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })?; - - if let Some(next_cs) = maybe_next_cs { - // New (untrusted) header timestamp cannot occur *after* next - // consensus state's height - let next_cs = downcast_tm_consensus_state(next_cs.as_ref())?; - - if header.signed_header.header().time > next_cs.timestamp { - return Err(ClientError::ClientSpecific { - description: Error::HeaderTimestampTooHigh { - actual: header.signed_header.header().time.to_string(), - max: next_cs.timestamp.to_string(), - } - .to_string(), - }); - } - } - } - Ok(()) } @@ -651,9 +589,55 @@ impl Ics2ClientState for ClientState { Ok(existing_consensus_state != header_consensus_state) } None => { - // FIXME timestamp monotonicity checks ? - // awaiting confirmation of whether we should do them here or in `check_client_message` - todo!() + // If no header was previously installed, we ensure the monotonicity of timestamps. + + // 1. for all headers, the new header needs to have a larger timestamp than + // the “previous header” + { + let maybe_prev_cs = ctx + .prev_consensus_state(&client_id, &header.height()) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })?; + + if let Some(prev_cs) = maybe_prev_cs { + // New header timestamp cannot occur *before* the + // previous consensus state's height + let prev_cs = downcast_tm_consensus_state(prev_cs.as_ref())?; + + if header.signed_header.header().time < prev_cs.timestamp { + return Ok(true); + } + } + } + + // 2. if a header comes in and is not the “last” header, then we also ensure + // that its timestamp is less than the “next header” + if header.height() < self.latest_height() { + let maybe_next_cs = ctx + .next_consensus_state(&client_id, &header.height()) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })?; + + if let Some(next_cs) = maybe_next_cs { + // New (untrusted) header timestamp cannot occur *after* next + // consensus state's height + let next_cs = downcast_tm_consensus_state(next_cs.as_ref())?; + + if header.signed_header.header().time > next_cs.timestamp { + return Ok(true); + } + } + } + + Ok(false) } } } From b2935a8904246641e7e37a5e2a7f398dfb602409 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 3 Apr 2023 14:10:28 -0400 Subject: [PATCH 20/62] update_state_on_misbehaviour --- .../clients/ics07_tendermint/client_state.rs | 25 +++++++++++++++++-- .../ibc/src/core/ics02_client/client_state.rs | 8 +++++- crates/ibc/src/mock/client_state.rs | 8 ++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 2b5e765be..a4557edb9 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -34,7 +34,7 @@ use crate::core::ics23_commitment::commitment::{ use crate::core::ics23_commitment::merkle::{apply_prefix, MerkleProof}; use crate::core::ics23_commitment::specs::ProofSpecs; use crate::core::ics24_host::identifier::{ChainId, ClientId}; -use crate::core::ics24_host::path::{ClientConsensusStatePath, ClientUpgradePath}; +use crate::core::ics24_host::path::{ClientConsensusStatePath, ClientStatePath, ClientUpgradePath}; use crate::core::ics24_host::Path; use crate::timestamp::{Timestamp, ZERO_DURATION}; use crate::Height; @@ -43,7 +43,7 @@ use super::client_type as tm_client_type; use crate::core::context::ContextError; -use crate::core::ValidationContext; +use crate::core::{ExecutionContext, ValidationContext}; pub const TENDERMINT_CLIENT_STATE_TYPE_URL: &str = "/ibc.lightclients.tendermint.v1.ClientState"; @@ -667,6 +667,27 @@ impl Ics2ClientState for ClientState { } } + fn update_state_on_misbehaviour( + &self, + ctx: &mut dyn ExecutionContext, + client_id: ClientId, + ) -> Result<(), ClientError> { + let frozen_client_state = self + .clone() + .with_frozen_height(Height::new(0, 1).unwrap()) + .into_box(); + + ctx.store_client_state(ClientStatePath::new(&client_id), frozen_client_state) + .map_err(|e| match e { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: e.to_string(), + }, + })?; + + Ok(()) + } + fn check_misbehaviour_and_update_state( &self, ctx: &dyn ValidationContext, diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index a49601d7f..3d0e936eb 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -21,7 +21,7 @@ use crate::Height; use super::consensus_state::ConsensusState; use super::msgs::update_client::UpdateClientKind; -use crate::core::{ContextError, ValidationContext}; +use crate::core::{ContextError, ExecutionContext, ValidationContext}; pub trait ClientState: AsAny @@ -85,6 +85,12 @@ pub trait ClientState: client_message: UpdateClientKind, ) -> Result; + fn update_state_on_misbehaviour( + &self, + ctx: &mut dyn ExecutionContext, + client_id: ClientId, + ) -> Result<(), ClientError>; + fn check_header_and_update_state( &self, ctx: &dyn ValidationContext, diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 0072bd8a5..1d753d9e7 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -304,6 +304,14 @@ impl ClientState for MockClientState { ) -> Result { todo!() } + + fn update_state_on_misbehaviour( + &self, + _ctx: &mut dyn crate::core::ExecutionContext, + _client_id: ClientId, + ) -> Result<(), ClientError> { + todo!() + } } impl From for MockClientState { From 9811af7160aba28010354bd24b9ec1f85ad9a0da Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 3 Apr 2023 15:11:35 -0400 Subject: [PATCH 21/62] implement update_state in tendermint light client --- .../clients/ics07_tendermint/client_state.rs | 70 +++++++++++++------ .../ibc/src/core/ics02_client/client_state.rs | 7 ++ crates/ibc/src/core/ics02_client/error.rs | 12 ++++ crates/ibc/src/mock/client_state.rs | 13 +++- 4 files changed, 78 insertions(+), 24 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index a4557edb9..e8a02e3fc 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -1,6 +1,7 @@ use crate::core::ics02_client::msgs::update_client::UpdateClientKind; use crate::prelude::*; +use core::cmp::max; use core::convert::{TryFrom, TryInto}; use core::time::Duration; @@ -178,15 +179,9 @@ impl ClientState { }) } - pub fn with_header(self, h: TmHeader) -> Result { + pub fn with_header(self, header: TmHeader) -> Result { Ok(ClientState { - latest_height: Height::new( - self.latest_height.revision_number(), - h.signed_header.header.height.into(), - ) - .map_err(|_| Error::InvalidHeaderHeight { - height: h.signed_header.header.height.value(), - })?, + latest_height: max(header.height(), self.latest_height), ..self }) } @@ -666,7 +661,50 @@ impl Ics2ClientState for ClientState { } } } + fn update_state( + &self, + ctx: &mut dyn ExecutionContext, + client_id: ClientId, + header: Any, + ) -> Result<(), ClientError> { + let header = TmHeader::try_from(header)?; + + let maybe_existing_consensus_state = { + let path_at_header_height = ClientConsensusStatePath::new(&client_id, &header.height()); + ctx.consensus_state(&path_at_header_height).ok() + }; + + if maybe_existing_consensus_state.is_some() { + // if we already had the header installed by a previous relayer + // then this is a no-op. + Ok(()) + } else { + let new_consensus_state = TmConsensusState::from(header.clone()).into_box(); + let new_client_state = self.clone().with_header(header)?.into_box(); + + ctx.store_update_time( + client_id.clone(), + new_client_state.latest_height(), + ctx.host_timestamp()?, + )?; + ctx.store_update_height( + client_id.clone(), + new_client_state.latest_height(), + ctx.host_height()?, + )?; + + ctx.store_consensus_state( + ClientConsensusStatePath::new(&client_id, &new_client_state.latest_height()), + new_consensus_state, + )?; + ctx.store_client_state(ClientStatePath::new(&client_id), new_client_state)?; + + Ok(()) + } + } + + // TODO: make self mut and don't clone fn update_state_on_misbehaviour( &self, ctx: &mut dyn ExecutionContext, @@ -820,13 +858,7 @@ impl Ics2ClientState for ClientState { let trusted_client_cons_state_path = ClientConsensusStatePath::new(&client_id, &header.trusted_height); let trusted_consensus_state = downcast_tm_consensus_state( - ctx.consensus_state(&trusted_client_cons_state_path) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })? + ctx.consensus_state(&trusted_client_cons_state_path)? .as_ref(), )?; @@ -857,13 +889,7 @@ impl Ics2ClientState for ClientState { }; let options = client_state.as_light_client_options()?; - let now = ctx - .host_timestamp() - .map_err(|e| ClientError::Other { - description: e.to_string(), - })? - .into_tm_time() - .unwrap(); + let now = ctx.host_timestamp()?.into_tm_time().unwrap(); self.verifier .verify(untrusted_state, trusted_state, &options, now) diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 3d0e936eb..8dde3cb6e 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -85,6 +85,13 @@ pub trait ClientState: client_message: UpdateClientKind, ) -> Result; + fn update_state( + &self, + ctx: &mut dyn ExecutionContext, + client_id: ClientId, + header: Any, + ) -> Result<(), ClientError>; + fn update_state_on_misbehaviour( &self, ctx: &mut dyn ExecutionContext, diff --git a/crates/ibc/src/core/ics02_client/error.rs b/crates/ibc/src/core/ics02_client/error.rs index 8f6580991..8a6090c45 100644 --- a/crates/ibc/src/core/ics02_client/error.rs +++ b/crates/ibc/src/core/ics02_client/error.rs @@ -1,3 +1,4 @@ +use crate::core::ContextError; use crate::prelude::*; use displaydoc::Display; @@ -113,6 +114,17 @@ pub enum ClientError { Other { description: String }, } +impl From for ClientError { + fn from(context_error: ContextError) -> Self { + match context_error { + ContextError::ClientError(e) => e, + _ => ClientError::Other { + description: context_error.to_string(), + }, + } + } +} + #[cfg(feature = "std")] impl std::error::Error for ClientError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 1d753d9e7..e432cc0c4 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -25,7 +25,7 @@ use crate::mock::misbehaviour::Misbehaviour; use crate::Height; -use crate::core::{ContextError, ValidationContext}; +use crate::core::{ContextError, ExecutionContext, ValidationContext}; pub const MOCK_CLIENT_STATE_TYPE_URL: &str = "/ibc.mock.ClientState"; @@ -307,11 +307,20 @@ impl ClientState for MockClientState { fn update_state_on_misbehaviour( &self, - _ctx: &mut dyn crate::core::ExecutionContext, + _ctx: &mut dyn ExecutionContext, _client_id: ClientId, ) -> Result<(), ClientError> { todo!() } + + fn update_state( + &self, + _ctx: &mut dyn ExecutionContext, + _client_id: ClientId, + _header: Any, + ) -> Result<(), ClientError> { + todo!() + } } impl From for MockClientState { From 3d4b1e305c871d1bc8989dcc2ffd0afe71c25e8d Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 3 Apr 2023 15:15:07 -0400 Subject: [PATCH 22/62] cleanup `map_err`s --- .../clients/ics07_tendermint/client_state.rs | 86 +++---------------- 1 file changed, 12 insertions(+), 74 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index e8a02e3fc..0a3b379a3 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -252,13 +252,7 @@ impl ClientState { let trusted_client_cons_state_path = ClientConsensusStatePath::new(&client_id, &header.trusted_height); let trusted_consensus_state = downcast_tm_consensus_state( - ctx.consensus_state(&trusted_client_cons_state_path) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })? + ctx.consensus_state(&trusted_client_cons_state_path)? .as_ref(), )?; @@ -290,13 +284,7 @@ impl ClientState { }; let options = self.as_light_client_options()?; - let now = ctx - .host_timestamp() - .map_err(|e| ClientError::Other { - description: e.to_string(), - })? - .into_tm_time() - .unwrap(); + let now = ctx.host_timestamp()?.into_tm_time().unwrap(); // main header verification, delegated to the tendermint-light-client crate. self.verifier @@ -319,14 +307,7 @@ impl ClientState { let trusted_consensus_state_1 = { let consensus_state_path = ClientConsensusStatePath::new(&client_id, &header_1.trusted_height); - let consensus_state = - ctx.consensus_state(&consensus_state_path) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })?; + let consensus_state = ctx.consensus_state(&consensus_state_path)?; downcast_tm_consensus_state(consensus_state.as_ref()) }?; @@ -335,24 +316,12 @@ impl ClientState { let trusted_consensus_state_2 = { let consensus_state_path = ClientConsensusStatePath::new(&client_id, &header_2.trusted_height); - let consensus_state = - ctx.consensus_state(&consensus_state_path) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })?; + let consensus_state = ctx.consensus_state(&consensus_state_path)?; downcast_tm_consensus_state(consensus_state.as_ref()) }?; - let current_timestamp = ctx.host_timestamp().map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })?; + let current_timestamp = ctx.host_timestamp()?; self.check_misbehaviour_header(header_1, &trusted_consensus_state_1, current_timestamp)?; self.check_misbehaviour_header(header_2, &trusted_consensus_state_2, current_timestamp) @@ -589,14 +558,8 @@ impl Ics2ClientState for ClientState { // 1. for all headers, the new header needs to have a larger timestamp than // the “previous header” { - let maybe_prev_cs = ctx - .prev_consensus_state(&client_id, &header.height()) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })?; + let maybe_prev_cs = + ctx.prev_consensus_state(&client_id, &header.height())?; if let Some(prev_cs) = maybe_prev_cs { // New header timestamp cannot occur *before* the @@ -612,14 +575,8 @@ impl Ics2ClientState for ClientState { // 2. if a header comes in and is not the “last” header, then we also ensure // that its timestamp is less than the “next header” if header.height() < self.latest_height() { - let maybe_next_cs = ctx - .next_consensus_state(&client_id, &header.height()) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })?; + let maybe_next_cs = + ctx.next_consensus_state(&client_id, &header.height())?; if let Some(next_cs) = maybe_next_cs { // New (untrusted) header timestamp cannot occur *after* next @@ -704,7 +661,6 @@ impl Ics2ClientState for ClientState { } } - // TODO: make self mut and don't clone fn update_state_on_misbehaviour( &self, ctx: &mut dyn ExecutionContext, @@ -715,13 +671,7 @@ impl Ics2ClientState for ClientState { .with_frozen_height(Height::new(0, 1).unwrap()) .into_box(); - ctx.store_client_state(ClientStatePath::new(&client_id), frozen_client_state) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })?; + ctx.store_client_state(ClientStatePath::new(&client_id), frozen_client_state)?; Ok(()) } @@ -911,13 +861,7 @@ impl Ics2ClientState for ClientState { // (cs-new, cs-next, cs-latest) if header.height() < client_state.latest_height() { let maybe_next_cs = ctx - .next_consensus_state(&client_id, &header.height()) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })? + .next_consensus_state(&client_id, &header.height())? .as_ref() .map(|cs| downcast_tm_consensus_state(cs.as_ref())) .transpose()?; @@ -940,13 +884,7 @@ impl Ics2ClientState for ClientState { // (cs-trusted, cs-prev, cs-new) if header.trusted_height < header.height() { let maybe_prev_cs = ctx - .prev_consensus_state(&client_id, &header.height()) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })? + .prev_consensus_state(&client_id, &header.height())? .as_ref() .map(|cs| downcast_tm_consensus_state(cs.as_ref())) .transpose()?; From b689bde11fa0bcb3647589dd7c20c6774d5d24cc Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 08:12:19 -0400 Subject: [PATCH 23/62] Rename MsgUpdateClient::client_message --- crates/ibc/src/core/handler.rs | 8 ++++---- .../core/ics02_client/handler/update_client.rs | 18 +++++++++--------- .../core/ics02_client/msgs/update_client.rs | 8 ++++---- crates/ibc/src/mock/ics18_relayer/context.rs | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/ibc/src/core/handler.rs b/crates/ibc/src/core/handler.rs index 13e7c13b0..6f5e3f828 100644 --- a/crates/ibc/src/core/handler.rs +++ b/crates/ibc/src/core/handler.rs @@ -262,7 +262,7 @@ mod tests { name: "Client update successful".to_string(), msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), - header: MockHeader::new(update_client_height) + client_message: MockHeader::new(update_client_height) .with_timestamp(Timestamp::now()) .into(), signer: default_signer.clone(), @@ -275,7 +275,7 @@ mod tests { name: "Client update fails due to stale header".to_string(), msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), - header: MockHeader::new(update_client_height).into(), + client_message: MockHeader::new(update_client_height).into(), signer: default_signer.clone(), })) .into(), @@ -350,7 +350,7 @@ mod tests { name: "Client update successful #2".to_string(), msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), - header: MockHeader::new(update_client_height_after_send) + client_message: MockHeader::new(update_client_height_after_send) .with_timestamp(Timestamp::now()) .into(), signer: default_signer.clone(), @@ -395,7 +395,7 @@ mod tests { name: "Client update successful".to_string(), msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), - header: MockHeader::new(update_client_height_after_second_send).into(), + client_message: MockHeader::new(update_client_height_after_second_send).into(), signer: default_signer.clone(), })) .into(), 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 f8d842348..ddf89f697 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -22,7 +22,7 @@ where { let MsgUpdateClient { client_id, - header, + client_message: header, signer: _, } = msg; @@ -75,7 +75,7 @@ where { let MsgUpdateClient { client_id, - header, + client_message: header, signer: _, } = msg; @@ -161,7 +161,7 @@ mod tests { let height = Height::new(0, 46).unwrap(); let msg = MsgUpdateClient { client_id, - header: MockHeader::new(height).with_timestamp(timestamp).into(), + client_message: MockHeader::new(height).with_timestamp(timestamp).into(), signer, }; @@ -187,7 +187,7 @@ mod tests { let msg = MsgUpdateClient { client_id: ClientId::from_str("nonexistingclient").unwrap(), - header: MockHeader::new(Height::new(0, 46).unwrap()).into(), + client_message: MockHeader::new(Height::new(0, 46).unwrap()).into(), signer, }; @@ -227,7 +227,7 @@ mod tests { let latest_header_height = block.height(); let msg = MsgUpdateClient { client_id, - header: block.into(), + client_message: block.into(), signer, }; @@ -274,7 +274,7 @@ mod tests { let latest_header_height = block.height(); let msg = MsgUpdateClient { client_id, - header: block.into(), + client_message: block.into(), signer, }; @@ -335,7 +335,7 @@ mod tests { let latest_header_height = block.height(); let msg = MsgUpdateClient { client_id, - header: block.into(), + client_message: block.into(), signer, }; @@ -386,7 +386,7 @@ mod tests { let msg = MsgUpdateClient { client_id, - header: block_ref.clone().into(), + client_message: block_ref.clone().into(), signer, }; @@ -406,7 +406,7 @@ mod tests { let header: Any = MockHeader::new(height).with_timestamp(timestamp).into(); let msg = MsgUpdateClient { client_id: client_id.clone(), - header: header.clone(), + client_message: header.clone(), signer, }; diff --git a/crates/ibc/src/core/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index accdc6409..b382aef60 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -26,7 +26,7 @@ pub enum UpdateClientKind { #[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgUpdateClient { pub client_id: ClientId, - pub header: Any, + pub client_message: Any, pub signer: Signer, } @@ -49,7 +49,7 @@ impl TryFrom for MsgUpdateClient { .client_id .parse() .map_err(ClientError::InvalidMsgUpdateClientId)?, - header: raw.header.ok_or(ClientError::MissingRawHeader)?, + client_message: raw.header.ok_or(ClientError::MissingRawHeader)?, signer: raw.signer.parse().map_err(ClientError::Signer)?, }) } @@ -59,7 +59,7 @@ impl From for RawMsgUpdateClient { fn from(ics_msg: MsgUpdateClient) -> Self { RawMsgUpdateClient { client_id: ics_msg.client_id.to_string(), - header: Some(ics_msg.header), + header: Some(ics_msg.client_message), signer: ics_msg.signer.to_string(), } } @@ -83,7 +83,7 @@ mod tests { pub fn new(client_id: ClientId, header: Any, signer: Signer) -> Self { MsgUpdateClient { client_id, - header, + client_message: header, signer, } } diff --git a/crates/ibc/src/mock/ics18_relayer/context.rs b/crates/ibc/src/mock/ics18_relayer/context.rs index 8282f5ae2..83aecca1b 100644 --- a/crates/ibc/src/mock/ics18_relayer/context.rs +++ b/crates/ibc/src/mock/ics18_relayer/context.rs @@ -86,7 +86,7 @@ mod tests { // Client on destination chain can be updated. Ok(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), - header: src_header.clone_into(), + client_message: src_header.clone_into(), signer: dest.signer(), })) } From 9a7cd1af9304108fbb8c2f58eff537b3ea2fdfcd Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 08:20:26 -0400 Subject: [PATCH 24/62] impl raw misbehaviour -> MsgUpdateClient --- .../core/ics02_client/msgs/update_client.rs | 32 +++++++++++++++++++ crates/ibc/src/core/ics26_routing/msgs.rs | 6 +++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/core/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index b382aef60..6dd3a9683 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -3,6 +3,7 @@ use crate::prelude::*; use ibc_proto::google::protobuf::Any; +use ibc_proto::ibc::core::client::v1::MsgSubmitMisbehaviour as RawMsgSubmitMisbehaviour; use ibc_proto::ibc::core::client::v1::MsgUpdateClient as RawMsgUpdateClient; use ibc_proto::protobuf::Protobuf; @@ -65,6 +66,37 @@ impl From for RawMsgUpdateClient { } } +impl Protobuf for MsgUpdateClient {} + +impl TryFrom for MsgUpdateClient { + type Error = ClientError; + + fn try_from(raw: RawMsgSubmitMisbehaviour) -> Result { + let raw_misbehaviour = raw + .misbehaviour + .ok_or(ClientError::MissingRawMisbehaviour)?; + + Ok(MsgUpdateClient { + client_id: raw + .client_id + .parse() + .map_err(ClientError::InvalidRawMisbehaviour)?, + client_message: raw_misbehaviour, + signer: raw.signer.parse().map_err(ClientError::Signer)?, + }) + } +} + +impl From for RawMsgSubmitMisbehaviour { + fn from(ics_msg: MsgUpdateClient) -> Self { + RawMsgSubmitMisbehaviour { + client_id: ics_msg.client_id.to_string(), + misbehaviour: Some(ics_msg.client_message), + signer: ics_msg.signer.to_string(), + } + } +} + #[cfg(test)] mod tests { diff --git a/crates/ibc/src/core/ics26_routing/msgs.rs b/crates/ibc/src/core/ics26_routing/msgs.rs index 36c5c3384..f40c24a19 100644 --- a/crates/ibc/src/core/ics26_routing/msgs.rs +++ b/crates/ibc/src/core/ics26_routing/msgs.rs @@ -1,6 +1,7 @@ use crate::prelude::*; use ibc_proto::google::protobuf::Any; +use ibc_proto::ibc::core::client::v1::MsgUpdateClient as RawMsgUpdateClient; use crate::core::ics02_client::msgs::{ create_client, misbehaviour, update_client, upgrade_client, ClientMsg, @@ -37,7 +38,10 @@ impl TryFrom for MsgEnvelope { Ok(MsgEnvelope::Client(ClientMsg::CreateClient(domain_msg))) } update_client::TYPE_URL => { - let domain_msg = update_client::MsgUpdateClient::decode_vec(&any_msg.value) + let domain_msg = + >::decode_vec( + &any_msg.value, + ) .map_err(RouterError::MalformedMessageBytes)?; Ok(MsgEnvelope::Client(ClientMsg::UpdateClient(domain_msg))) } From f8dd700e73dfc6c35789e5004ebe788e53579496 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 08:46:20 -0400 Subject: [PATCH 25/62] change updateClientKind enum --- .../clients/ics07_tendermint/client_state.rs | 27 ++++++++++--------- crates/ibc/src/core/handler.rs | 5 ++++ .../ibc/src/core/ics02_client/client_state.rs | 7 +++-- .../ics02_client/handler/update_client.rs | 11 +++++++- .../core/ics02_client/msgs/update_client.rs | 10 +++++-- crates/ibc/src/mock/client_state.rs | 25 +++++++++-------- crates/ibc/src/mock/ics18_relayer/context.rs | 3 ++- 7 files changed, 59 insertions(+), 29 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 0a3b379a3..c5eaf80df 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -505,15 +505,16 @@ impl Ics2ClientState for ClientState { &self, ctx: &dyn ValidationContext, client_id: ClientId, - client_message: UpdateClientKind, + client_message: Any, + update_kind: UpdateClientKind, ) -> Result<(), ClientError> { - match client_message { - UpdateClientKind::UpdateHeader(header) => { - let header = TmHeader::try_from(header)?; + match update_kind { + UpdateClientKind::UpdateHeader => { + let header = TmHeader::try_from(client_message)?; self.verify_header(ctx, client_id, header) } - UpdateClientKind::Misbehaviour(misbehaviour) => { - let misbehaviour = TmMisbehaviour::try_from(misbehaviour)?; + UpdateClientKind::Misbehaviour => { + let misbehaviour = TmMisbehaviour::try_from(client_message)?; self.verify_misbehaviour(ctx, client_id, misbehaviour) } } @@ -526,11 +527,12 @@ impl Ics2ClientState for ClientState { &self, ctx: &dyn ValidationContext, client_id: ClientId, - client_message: UpdateClientKind, + client_message: Any, + update_kind: UpdateClientKind, ) -> Result { - match client_message { - UpdateClientKind::UpdateHeader(header) => { - let header = TmHeader::try_from(header)?; + match update_kind { + UpdateClientKind::UpdateHeader => { + let header = TmHeader::try_from(client_message)?; let header_consensus_state = TmConsensusState::from(header.clone()); @@ -593,8 +595,8 @@ impl Ics2ClientState for ClientState { } } } - UpdateClientKind::Misbehaviour(misbehaviour) => { - let misbehaviour = TmMisbehaviour::try_from(misbehaviour)?; + UpdateClientKind::Misbehaviour => { + let misbehaviour = TmMisbehaviour::try_from(client_message)?; let header_1 = misbehaviour.header1(); let header_2 = misbehaviour.header2(); @@ -665,6 +667,7 @@ impl Ics2ClientState for ClientState { &self, ctx: &mut dyn ExecutionContext, client_id: ClientId, + _misbehaviour: Any, ) -> Result<(), ClientError> { let frozen_client_state = self .clone() diff --git a/crates/ibc/src/core/handler.rs b/crates/ibc/src/core/handler.rs index 6f5e3f828..bf48d7d9f 100644 --- a/crates/ibc/src/core/handler.rs +++ b/crates/ibc/src/core/handler.rs @@ -43,6 +43,7 @@ mod tests { msgs::transfer::test_util::get_dummy_msg_transfer, msgs::transfer::MsgTransfer, packet::PacketData, PrefixedCoin, MODULE_ID_STR, }; + use crate::core::ics02_client::msgs::update_client::UpdateClientKind; use crate::core::ics02_client::msgs::{ create_client::MsgCreateClient, update_client::MsgUpdateClient, upgrade_client::MsgUpgradeClient, ClientMsg, @@ -265,6 +266,7 @@ mod tests { client_message: MockHeader::new(update_client_height) .with_timestamp(Timestamp::now()) .into(), + update_kind: UpdateClientKind::UpdateHeader, signer: default_signer.clone(), })) .into(), @@ -276,6 +278,7 @@ mod tests { msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), client_message: MockHeader::new(update_client_height).into(), + update_kind: UpdateClientKind::UpdateHeader, signer: default_signer.clone(), })) .into(), @@ -353,6 +356,7 @@ mod tests { client_message: MockHeader::new(update_client_height_after_send) .with_timestamp(Timestamp::now()) .into(), + update_kind: UpdateClientKind::UpdateHeader, signer: default_signer.clone(), })) .into(), @@ -396,6 +400,7 @@ mod tests { msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), client_message: MockHeader::new(update_client_height_after_second_send).into(), + update_kind: UpdateClientKind::UpdateHeader, signer: default_signer.clone(), })) .into(), diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 8dde3cb6e..8b0072861 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -74,7 +74,8 @@ pub trait ClientState: &self, ctx: &dyn ValidationContext, client_id: ClientId, - client_message: UpdateClientKind, + client_message: Any, + update_kind: UpdateClientKind, ) -> Result<(), ClientError>; /// Check whether misbehaviour has occured @@ -82,7 +83,8 @@ pub trait ClientState: &self, ctx: &dyn ValidationContext, client_id: ClientId, - client_message: UpdateClientKind, + client_message: Any, + update_kind: UpdateClientKind, ) -> Result; fn update_state( @@ -96,6 +98,7 @@ pub trait ClientState: &self, ctx: &mut dyn ExecutionContext, client_id: ClientId, + misbehaviour: Any, ) -> Result<(), ClientError>; fn check_header_and_update_state( 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 ddf89f697..a86d5d2bf 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -23,6 +23,7 @@ where let MsgUpdateClient { client_id, client_message: header, + update_kind: _update_kind, signer: _, } = msg; @@ -76,6 +77,7 @@ where let MsgUpdateClient { client_id, client_message: header, + update_kind: _update_kind, signer: _, } = msg; @@ -136,7 +138,7 @@ mod tests { use crate::core::ics02_client::client_state::ClientState; use crate::core::ics02_client::consensus_state::downcast_consensus_state; use crate::core::ics02_client::handler::update_client::{execute, validate}; - use crate::core::ics02_client::msgs::update_client::MsgUpdateClient; + use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateClientKind}; use crate::core::ics24_host::identifier::{ChainId, ClientId}; use crate::core::ValidationContext; use crate::events::{IbcEvent, IbcEventType}; @@ -162,6 +164,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: MockHeader::new(height).with_timestamp(timestamp).into(), + update_kind: UpdateClientKind::UpdateHeader, signer, }; @@ -188,6 +191,7 @@ mod tests { let msg = MsgUpdateClient { client_id: ClientId::from_str("nonexistingclient").unwrap(), client_message: MockHeader::new(Height::new(0, 46).unwrap()).into(), + update_kind: UpdateClientKind::UpdateHeader, signer, }; @@ -228,6 +232,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: block.into(), + update_kind: UpdateClientKind::UpdateHeader, signer, }; @@ -275,6 +280,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: block.into(), + update_kind: UpdateClientKind::UpdateHeader, signer, }; @@ -336,6 +342,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: block.into(), + update_kind: UpdateClientKind::UpdateHeader, signer, }; @@ -387,6 +394,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: block_ref.clone().into(), + update_kind: UpdateClientKind::UpdateHeader, signer, }; @@ -407,6 +415,7 @@ mod tests { let msg = MsgUpdateClient { client_id: client_id.clone(), client_message: header.clone(), + update_kind: UpdateClientKind::UpdateHeader, signer, }; diff --git a/crates/ibc/src/core/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index 6dd3a9683..d7f88257d 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -14,13 +14,14 @@ use crate::tx_msg::Msg; pub const TYPE_URL: &str = "/ibc.core.client.v1.MsgUpdateClient"; +#[derive(Clone, Debug, PartialEq, Eq)] pub enum UpdateClientKind { /// this is the typical scenario where a new header is submitted to the client /// to update the client - UpdateHeader(Any), + UpdateHeader, /// this is the scenario where misbehaviour is submitted to the client /// (e.g 2 headers with the same height in Tendermint) - Misbehaviour(Any), + Misbehaviour, } /// A type of message that triggers the update of an on-chain (IBC) client with new headers. @@ -28,6 +29,7 @@ pub enum UpdateClientKind { pub struct MsgUpdateClient { pub client_id: ClientId, pub client_message: Any, + pub update_kind: UpdateClientKind, pub signer: Signer, } @@ -51,6 +53,7 @@ impl TryFrom for MsgUpdateClient { .parse() .map_err(ClientError::InvalidMsgUpdateClientId)?, client_message: raw.header.ok_or(ClientError::MissingRawHeader)?, + update_kind: UpdateClientKind::UpdateHeader, signer: raw.signer.parse().map_err(ClientError::Signer)?, }) } @@ -82,6 +85,7 @@ impl TryFrom for MsgUpdateClient { .parse() .map_err(ClientError::InvalidRawMisbehaviour)?, client_message: raw_misbehaviour, + update_kind: UpdateClientKind::Misbehaviour, signer: raw.signer.parse().map_err(ClientError::Signer)?, }) } @@ -99,6 +103,7 @@ impl From for RawMsgSubmitMisbehaviour { #[cfg(test)] mod tests { + use super::*; use test_log::test; @@ -116,6 +121,7 @@ mod tests { MsgUpdateClient { client_id, client_message: header, + update_kind: UpdateClientKind::UpdateHeader, signer, } } diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index e432cc0c4..01ee819ea 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -287,37 +287,40 @@ impl ClientState for MockClientState { Ok(()) } - fn verify_client_message( + fn update_state( &self, - _ctx: &dyn ValidationContext, + _ctx: &mut dyn ExecutionContext, _client_id: ClientId, - _client_message: UpdateClientKind, + _header: Any, ) -> Result<(), ClientError> { todo!() } - fn check_for_misbehaviour( + fn verify_client_message( &self, _ctx: &dyn ValidationContext, _client_id: ClientId, - _client_message: UpdateClientKind, - ) -> Result { + _client_message: Any, + _update_kind: UpdateClientKind, + ) -> Result<(), ClientError> { todo!() } - fn update_state_on_misbehaviour( + fn check_for_misbehaviour( &self, - _ctx: &mut dyn ExecutionContext, + _ctx: &dyn ValidationContext, _client_id: ClientId, - ) -> Result<(), ClientError> { + _client_message: Any, + _update_kind: UpdateClientKind, + ) -> Result { todo!() } - fn update_state( + fn update_state_on_misbehaviour( &self, _ctx: &mut dyn ExecutionContext, _client_id: ClientId, - _header: Any, + _misbehaviour: Any, ) -> Result<(), ClientError> { todo!() } diff --git a/crates/ibc/src/mock/ics18_relayer/context.rs b/crates/ibc/src/mock/ics18_relayer/context.rs index 83aecca1b..3a4c194bc 100644 --- a/crates/ibc/src/mock/ics18_relayer/context.rs +++ b/crates/ibc/src/mock/ics18_relayer/context.rs @@ -32,7 +32,7 @@ pub trait RelayerContext { mod tests { use crate::clients::ics07_tendermint::client_type as tm_client_type; use crate::core::ics02_client::header::{downcast_header, Header}; - use crate::core::ics02_client::msgs::update_client::MsgUpdateClient; + use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateClientKind}; use crate::core::ics02_client::msgs::ClientMsg; use crate::core::ics24_host::identifier::{ChainId, ClientId}; use crate::core::ics26_routing::msgs::MsgEnvelope; @@ -87,6 +87,7 @@ mod tests { Ok(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), client_message: src_header.clone_into(), + update_kind: UpdateClientKind::UpdateHeader, signer: dest.signer(), })) } From 0ba0c05b21d7a43028e0daed0f349a8707f5c21e Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 09:04:42 -0400 Subject: [PATCH 26/62] move misbehaviour tests to update_client --- .../ics02_client/handler/update_client.rs | 203 ++++++++++++++++++ 1 file changed, 203 insertions(+) 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 a86d5d2bf..1d5610123 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -135,7 +135,10 @@ mod tests { use crate::clients::ics07_tendermint::client_type as tm_client_type; use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; + use crate::clients::ics07_tendermint::header::Header as TmHeader; + use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; use crate::core::ics02_client::client_state::ClientState; + use crate::core::ics02_client::client_type::ClientType; use crate::core::ics02_client::consensus_state::downcast_consensus_state; use crate::core::ics02_client::handler::update_client::{execute, validate}; use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateClientKind}; @@ -146,6 +149,7 @@ mod tests { use crate::mock::client_state::MockClientState; use crate::mock::context::MockContext; use crate::mock::header::MockHeader; + use crate::mock::misbehaviour::Misbehaviour as MockMisbehaviour; use crate::mock::host::{HostBlock, HostType}; use crate::test_utils::get_dummy_account_id; use crate::timestamp::Timestamp; @@ -434,4 +438,203 @@ mod tests { assert_eq!(update_client_event.consensus_heights(), &vec![height]); assert_eq!(update_client_event.header(), &header); } + + fn ensure_misbehaviour(ctx: &MockContext, client_id: &ClientId, client_type: &ClientType) { + let client_state = ctx.client_state(client_id).unwrap(); + + assert!(client_state.confirm_not_frozen().is_err()); + + // check events + assert_eq!(ctx.events.len(), 2); + assert!(matches!( + ctx.events[0], + IbcEvent::Message(IbcEventType::ClientMisbehaviour), + )); + let misbehaviour_client_event = + downcast!(&ctx.events[1] => IbcEvent::ClientMisbehaviour).unwrap(); + assert_eq!(misbehaviour_client_event.client_id(), client_id); + assert_eq!(misbehaviour_client_event.client_type(), client_type); + } + + /// Tests misbehaviour handling for the mock client. + /// Misbehaviour evidence consists of identical headers - mock misbehaviour handler considers it + /// a valid proof of misbehaviour + #[test] + fn test_misbehaviour_client_ok() { + let client_id = ClientId::default(); + let timestamp = Timestamp::now(); + let height = Height::new(0, 46).unwrap(); + let msg = MsgUpdateClient { + client_id: client_id.clone(), + client_message: MockMisbehaviour { + client_id: client_id.clone(), + header1: MockHeader::new(height).with_timestamp(timestamp), + header2: MockHeader::new(height).with_timestamp(timestamp), + } + .into(), + update_kind: UpdateClientKind::UpdateHeader, + signer: get_dummy_account_id(), + }; + + let mut ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); + + let res = validate(&ctx, msg.clone()); + assert!(res.is_ok()); + let res = execute(&mut ctx, msg); + assert!(res.is_ok()); + + ensure_misbehaviour(&ctx, &client_id, &mock_client_type()); + } + + /// Tests misbehaviour handling failure for a non-existent client + #[test] + fn test_misbehaviour_nonexisting_client() { + let client_id = ClientId::from_str("mockclient1").unwrap(); + let height = Height::new(0, 46).unwrap(); + let msg = MsgUpdateClient { + client_id: ClientId::from_str("nonexistingclient").unwrap(), + client_message: MockMisbehaviour { + client_id: client_id.clone(), + header1: MockHeader::new(height), + header2: MockHeader::new(height), + } + .into(), + update_kind: UpdateClientKind::Misbehaviour, + signer: get_dummy_account_id(), + }; + + let ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); + let res = validate(&ctx, msg); + assert!(res.is_err()); + } + + /// Tests misbehaviour handling for the synthetic Tendermint client. + /// Misbehaviour evidence consists of equivocal headers. + #[test] + fn test_misbehaviour_synthetic_tendermint_equivocation() { + let client_id = ClientId::new(tm_client_type(), 0).unwrap(); + let client_height = Height::new(1, 20).unwrap(); + let misbehaviour_height = Height::new(1, 21).unwrap(); + let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1); + + // Create a mock context for chain-A with a synthetic tendermint light client for chain-B + let mut ctx_a = MockContext::new( + ChainId::new("mockgaiaA".to_string(), 1), + 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()), + Some(client_height), + ); + + // Create a mock context for chain-B + let ctx_b = MockContext::new( + chain_id_b.clone(), + HostType::SyntheticTendermint, + 5, + misbehaviour_height, + ); + + // Get chain-B's header at `misbehaviour_height` + let header1: TmHeader = { + let mut block = ctx_b.host_block(&misbehaviour_height).unwrap().clone(); + block.set_trusted_height(client_height); + block.try_into_tm_block().unwrap().into() + }; + + // Generate an equivocal header for chain-B at `misbehaviour_height` + let header2 = { + let mut tm_block = HostBlock::generate_tm_block( + chain_id_b, + misbehaviour_height.revision_height(), + Timestamp::now(), + ); + tm_block.trusted_height = client_height; + tm_block.into() + }; + + let msg = MsgUpdateClient { + client_id: client_id.clone(), + client_message: TmMisbehaviour::new(client_id.clone(), header1, header2) + .unwrap() + .into(), + update_kind: UpdateClientKind::Misbehaviour, + signer: get_dummy_account_id(), + }; + + let res = validate(&ctx_a, msg.clone()); + assert!(res.is_ok()); + let res = execute(&mut ctx_a, msg); + assert!(res.is_ok()); + ensure_misbehaviour(&ctx_a, &client_id, &tm_client_type()); + } + + #[test] + fn test_misbehaviour_synthetic_tendermint_bft_time() { + let client_id = ClientId::new(tm_client_type(), 0).unwrap(); + let client_height = Height::new(1, 20).unwrap(); + let misbehaviour_height = Height::new(1, 21).unwrap(); + let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1); + + // Create a mock context for chain-A with a synthetic tendermint light client for chain-B + let mut ctx_a = MockContext::new( + ChainId::new("mockgaiaA".to_string(), 1), + 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()), + Some(client_height), + ); + + // Generate `header1` for chain-B + let header1 = { + let mut tm_block = HostBlock::generate_tm_block( + chain_id_b.clone(), + misbehaviour_height.revision_height(), + Timestamp::now(), + ); + tm_block.trusted_height = client_height; + tm_block + }; + + // Generate `header2` for chain-B which is identical to `header1` but with a conflicting + // timestamp + let header2 = { + let timestamp = + Timestamp::from_nanoseconds(Timestamp::now().nanoseconds() + 1_000_000_000) + .unwrap(); + let mut tm_block = HostBlock::generate_tm_block( + chain_id_b, + misbehaviour_height.revision_height(), + timestamp, + ); + tm_block.trusted_height = client_height; + tm_block + }; + + let msg = MsgUpdateClient { + client_id: client_id.clone(), + client_message: TmMisbehaviour::new(client_id.clone(), header1.into(), header2.into()) + .unwrap() + .into(), + update_kind: UpdateClientKind::Misbehaviour, + signer: get_dummy_account_id(), + }; + + let res = validate(&ctx_a, msg.clone()); + assert!(res.is_ok()); + let res = execute(&mut ctx_a, msg); + assert!(res.is_ok()); + ensure_misbehaviour(&ctx_a, &client_id, &tm_client_type()); + } } From 9b6b80d330c2c2fd2344cc56961d68792c993464 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 09:08:08 -0400 Subject: [PATCH 27/62] move misbehaviour type url --- .../ibc/src/core/ics02_client/msgs/update_client.rs | 12 ++---------- crates/ibc/src/core/ics26_routing/msgs.rs | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index d7f88257d..de61db0a7 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -10,9 +10,9 @@ use ibc_proto::protobuf::Protobuf; use crate::core::ics02_client::error::ClientError; use crate::core::ics24_host::identifier::ClientId; use crate::signer::Signer; -use crate::tx_msg::Msg; -pub const TYPE_URL: &str = "/ibc.core.client.v1.MsgUpdateClient"; +pub const UPDATE_CLIENT_TYPE_URL: &str = "/ibc.core.client.v1.MsgUpdateClient"; +pub const MISBEHAVIOUR_TYPE_URL: &str = "/ibc.core.client.v1.MsgSubmitMisbehaviour"; #[derive(Clone, Debug, PartialEq, Eq)] pub enum UpdateClientKind { @@ -33,14 +33,6 @@ pub struct MsgUpdateClient { pub signer: Signer, } -impl Msg for MsgUpdateClient { - type Raw = RawMsgUpdateClient; - - fn type_url(&self) -> String { - TYPE_URL.to_string() - } -} - impl Protobuf for MsgUpdateClient {} impl TryFrom for MsgUpdateClient { diff --git a/crates/ibc/src/core/ics26_routing/msgs.rs b/crates/ibc/src/core/ics26_routing/msgs.rs index f40c24a19..397286192 100644 --- a/crates/ibc/src/core/ics26_routing/msgs.rs +++ b/crates/ibc/src/core/ics26_routing/msgs.rs @@ -37,7 +37,7 @@ impl TryFrom for MsgEnvelope { .map_err(RouterError::MalformedMessageBytes)?; Ok(MsgEnvelope::Client(ClientMsg::CreateClient(domain_msg))) } - update_client::TYPE_URL => { + update_client::UPDATE_CLIENT_TYPE_URL => { let domain_msg = >::decode_vec( &any_msg.value, From 951a3bfde3b8217fd55a728aac99f9e332f691f9 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 09:12:40 -0400 Subject: [PATCH 28/62] remove misbehaviour msg and handler --- crates/ibc/src/core/context.rs | 4 +- crates/ibc/src/core/ics02_client/handler.rs | 1 - .../core/ics02_client/handler/misbehaviour.rs | 286 ------------------ .../ics02_client/handler/update_client.rs | 2 +- crates/ibc/src/core/ics02_client/msgs.rs | 3 - .../core/ics02_client/msgs/misbehaviour.rs | 62 ---- crates/ibc/src/core/ics26_routing/msgs.rs | 15 +- 7 files changed, 10 insertions(+), 363 deletions(-) delete mode 100644 crates/ibc/src/core/ics02_client/handler/misbehaviour.rs delete mode 100644 crates/ibc/src/core/ics02_client/msgs/misbehaviour.rs diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 74d116b84..a9d4f1fe9 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -54,7 +54,7 @@ use crate::core::ics24_host::path::{ use crate::core::ics26_routing::context::{Module, ModuleId}; use crate::core::{ ics02_client::{ - handler::{create_client, misbehaviour, update_client, upgrade_client}, + handler::{create_client, update_client, upgrade_client}, msgs::ClientMsg, }, ics03_connection::{ @@ -174,7 +174,6 @@ pub trait ValidationContext: Router { MsgEnvelope::Client(msg) => match msg { ClientMsg::CreateClient(msg) => create_client::validate(self, msg), ClientMsg::UpdateClient(msg) => update_client::validate(self, msg), - ClientMsg::Misbehaviour(msg) => misbehaviour::validate(self, msg), ClientMsg::UpgradeClient(msg) => upgrade_client::validate(self, msg), } .map_err(RouterError::ContextError), @@ -383,7 +382,6 @@ pub trait ExecutionContext: ValidationContext { MsgEnvelope::Client(msg) => match msg { ClientMsg::CreateClient(msg) => create_client::execute(self, msg), ClientMsg::UpdateClient(msg) => update_client::execute(self, msg), - ClientMsg::Misbehaviour(msg) => misbehaviour::execute(self, msg), ClientMsg::UpgradeClient(msg) => upgrade_client::execute(self, msg), } .map_err(RouterError::ContextError), diff --git a/crates/ibc/src/core/ics02_client/handler.rs b/crates/ibc/src/core/ics02_client/handler.rs index d9a165dfd..65c5e59c8 100644 --- a/crates/ibc/src/core/ics02_client/handler.rs +++ b/crates/ibc/src/core/ics02_client/handler.rs @@ -1,6 +1,5 @@ //! This module implements the processing logic for ICS2 (client abstractions and functions) msgs. pub mod create_client; -pub mod misbehaviour; pub mod update_client; pub mod upgrade_client; diff --git a/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs b/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs deleted file mode 100644 index 2c0911b90..000000000 --- a/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs +++ /dev/null @@ -1,286 +0,0 @@ -//! Protocol logic specific to processing ICS2 messages of type `MsgSubmitMisbehaviour`. -//! -use crate::prelude::*; - -use crate::core::ics02_client::error::ClientError; -use crate::core::ics02_client::events::ClientMisbehaviour; -use crate::core::ics02_client::msgs::misbehaviour::MsgSubmitMisbehaviour; -use crate::events::IbcEvent; - -use crate::core::ics24_host::path::ClientStatePath; - -use crate::core::{ContextError, ExecutionContext, ValidationContext}; - -pub(crate) fn validate(ctx: &Ctx, msg: MsgSubmitMisbehaviour) -> Result<(), ContextError> -where - Ctx: ValidationContext, -{ - let MsgSubmitMisbehaviour { - client_id, - misbehaviour, - signer: _, - } = msg; - - // Read client state from the host chain store. - let client_state = ctx.client_state(&client_id)?; - - client_state.confirm_not_frozen()?; - - let _ = client_state - .check_misbehaviour_and_update_state(ctx, client_id.clone(), misbehaviour) - .map_err(|e| ClientError::MisbehaviourHandlingFailure { - reason: e.to_string(), - })?; - - Ok(()) -} - -pub(crate) fn execute(ctx: &mut Ctx, msg: MsgSubmitMisbehaviour) -> Result<(), ContextError> -where - Ctx: ExecutionContext, -{ - let MsgSubmitMisbehaviour { - client_id, - misbehaviour, - signer: _, - } = msg; - - // Read client state from the host chain store. - let client_state = ctx.client_state(&client_id)?; - - let client_state = client_state - .check_misbehaviour_and_update_state(ctx, client_id.clone(), misbehaviour) - .map_err(|e| ClientError::MisbehaviourHandlingFailure { - reason: e.to_string(), - })?; - - let event = IbcEvent::ClientMisbehaviour(ClientMisbehaviour::new( - client_id.clone(), - client_state.client_type(), - )); - ctx.emit_ibc_event(IbcEvent::Message(event.event_type())); - ctx.emit_ibc_event(event); - - ctx.store_client_state(ClientStatePath::new(&client_id), client_state) -} - -#[cfg(test)] -mod tests { - use core::str::FromStr; - use test_log::test; - - use crate::clients::ics07_tendermint::client_type as tm_client_type; - use crate::clients::ics07_tendermint::header::Header as TmHeader; - use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; - use crate::core::ics02_client::client_type::ClientType; - use crate::core::ics02_client::handler::misbehaviour::execute; - use crate::core::ics02_client::handler::misbehaviour::validate; - use crate::core::ics02_client::msgs::misbehaviour::MsgSubmitMisbehaviour; - use crate::core::ics24_host::identifier::{ChainId, ClientId}; - use crate::core::ValidationContext; - use crate::events::{IbcEvent, IbcEventType}; - use crate::mock::client_state::client_type as mock_client_type; - use crate::mock::context::MockContext; - use crate::mock::header::MockHeader; - use crate::mock::host::{HostBlock, HostType}; - use crate::mock::misbehaviour::Misbehaviour as MockMisbehaviour; - use crate::test_utils::get_dummy_account_id; - use crate::timestamp::Timestamp; - use crate::Height; - use crate::{downcast, prelude::*}; - - fn ensure_misbehaviour(ctx: &MockContext, client_id: &ClientId, client_type: &ClientType) { - let client_state = ctx.client_state(client_id).unwrap(); - - assert!(client_state.confirm_not_frozen().is_err()); - - // check events - assert_eq!(ctx.events.len(), 2); - assert!(matches!( - ctx.events[0], - IbcEvent::Message(IbcEventType::ClientMisbehaviour), - )); - let misbehaviour_client_event = - downcast!(&ctx.events[1] => IbcEvent::ClientMisbehaviour).unwrap(); - assert_eq!(misbehaviour_client_event.client_id(), client_id); - assert_eq!(misbehaviour_client_event.client_type(), client_type); - } - - /// Tests misbehaviour handling for the mock client. - /// Misbehaviour evidence consists of identical headers - mock misbehaviour handler considers it - /// a valid proof of misbehaviour - #[test] - fn test_misbehaviour_client_ok() { - let client_id = ClientId::default(); - let timestamp = Timestamp::now(); - let height = Height::new(0, 46).unwrap(); - let msg = MsgSubmitMisbehaviour { - client_id: client_id.clone(), - misbehaviour: MockMisbehaviour { - client_id: client_id.clone(), - header1: MockHeader::new(height).with_timestamp(timestamp), - header2: MockHeader::new(height).with_timestamp(timestamp), - } - .into(), - signer: get_dummy_account_id(), - }; - - let mut ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); - - let res = validate(&ctx, msg.clone()); - assert!(res.is_ok()); - let res = execute(&mut ctx, msg); - assert!(res.is_ok()); - - ensure_misbehaviour(&ctx, &client_id, &mock_client_type()); - } - - /// Tests misbehaviour handling failure for a non-existent client - #[test] - fn test_misbehaviour_nonexisting_client() { - let client_id = ClientId::from_str("mockclient1").unwrap(); - let height = Height::new(0, 46).unwrap(); - let msg = MsgSubmitMisbehaviour { - client_id: ClientId::from_str("nonexistingclient").unwrap(), - misbehaviour: MockMisbehaviour { - client_id: client_id.clone(), - header1: MockHeader::new(height), - header2: MockHeader::new(height), - } - .into(), - signer: get_dummy_account_id(), - }; - - let ctx = MockContext::default().with_client(&client_id, Height::new(0, 42).unwrap()); - let res = validate(&ctx, msg); - assert!(res.is_err()); - } - - /// Tests misbehaviour handling for the synthetic Tendermint client. - /// Misbehaviour evidence consists of equivocal headers. - #[test] - fn test_misbehaviour_synthetic_tendermint_equivocation() { - let client_id = ClientId::new(tm_client_type(), 0).unwrap(); - let client_height = Height::new(1, 20).unwrap(); - let misbehaviour_height = Height::new(1, 21).unwrap(); - let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1); - - // Create a mock context for chain-A with a synthetic tendermint light client for chain-B - let mut ctx_a = MockContext::new( - ChainId::new("mockgaiaA".to_string(), 1), - 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()), - Some(client_height), - ); - - // Create a mock context for chain-B - let ctx_b = MockContext::new( - chain_id_b.clone(), - HostType::SyntheticTendermint, - 5, - misbehaviour_height, - ); - - // Get chain-B's header at `misbehaviour_height` - let header1: TmHeader = { - let mut block = ctx_b.host_block(&misbehaviour_height).unwrap().clone(); - block.set_trusted_height(client_height); - block.try_into_tm_block().unwrap().into() - }; - - // Generate an equivocal header for chain-B at `misbehaviour_height` - let header2 = { - let mut tm_block = HostBlock::generate_tm_block( - chain_id_b, - misbehaviour_height.revision_height(), - Timestamp::now(), - ); - tm_block.trusted_height = client_height; - tm_block.into() - }; - - let msg = MsgSubmitMisbehaviour { - client_id: client_id.clone(), - misbehaviour: TmMisbehaviour::new(client_id.clone(), header1, header2) - .unwrap() - .into(), - signer: get_dummy_account_id(), - }; - - let res = validate(&ctx_a, msg.clone()); - assert!(res.is_ok()); - let res = execute(&mut ctx_a, msg); - assert!(res.is_ok()); - ensure_misbehaviour(&ctx_a, &client_id, &tm_client_type()); - } - - #[test] - fn test_misbehaviour_synthetic_tendermint_bft_time() { - let client_id = ClientId::new(tm_client_type(), 0).unwrap(); - let client_height = Height::new(1, 20).unwrap(); - let misbehaviour_height = Height::new(1, 21).unwrap(); - let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1); - - // Create a mock context for chain-A with a synthetic tendermint light client for chain-B - let mut ctx_a = MockContext::new( - ChainId::new("mockgaiaA".to_string(), 1), - 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()), - Some(client_height), - ); - - // Generate `header1` for chain-B - let header1 = { - let mut tm_block = HostBlock::generate_tm_block( - chain_id_b.clone(), - misbehaviour_height.revision_height(), - Timestamp::now(), - ); - tm_block.trusted_height = client_height; - tm_block - }; - - // Generate `header2` for chain-B which is identical to `header1` but with a conflicting - // timestamp - let header2 = { - let timestamp = - Timestamp::from_nanoseconds(Timestamp::now().nanoseconds() + 1_000_000_000) - .unwrap(); - let mut tm_block = HostBlock::generate_tm_block( - chain_id_b, - misbehaviour_height.revision_height(), - timestamp, - ); - tm_block.trusted_height = client_height; - tm_block - }; - - let msg = MsgSubmitMisbehaviour { - client_id: client_id.clone(), - misbehaviour: TmMisbehaviour::new(client_id.clone(), header1.into(), header2.into()) - .unwrap() - .into(), - signer: get_dummy_account_id(), - }; - - let res = validate(&ctx_a, msg.clone()); - assert!(res.is_ok()); - let res = execute(&mut ctx_a, msg); - assert!(res.is_ok()); - ensure_misbehaviour(&ctx_a, &client_id, &tm_client_type()); - } -} 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 1d5610123..4ddce4fc7 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -149,8 +149,8 @@ mod tests { use crate::mock::client_state::MockClientState; use crate::mock::context::MockContext; use crate::mock::header::MockHeader; - use crate::mock::misbehaviour::Misbehaviour as MockMisbehaviour; use crate::mock::host::{HostBlock, HostType}; + use crate::mock::misbehaviour::Misbehaviour as MockMisbehaviour; use crate::test_utils::get_dummy_account_id; use crate::timestamp::Timestamp; use crate::Height; diff --git a/crates/ibc/src/core/ics02_client/msgs.rs b/crates/ibc/src/core/ics02_client/msgs.rs index 476f34cb6..ba66213fd 100644 --- a/crates/ibc/src/core/ics02_client/msgs.rs +++ b/crates/ibc/src/core/ics02_client/msgs.rs @@ -5,12 +5,10 @@ //! . use crate::core::ics02_client::msgs::create_client::MsgCreateClient; -use crate::core::ics02_client::msgs::misbehaviour::MsgSubmitMisbehaviour; use crate::core::ics02_client::msgs::update_client::MsgUpdateClient; use crate::core::ics02_client::msgs::upgrade_client::MsgUpgradeClient; pub mod create_client; -pub mod misbehaviour; pub mod update_client; pub mod upgrade_client; @@ -19,6 +17,5 @@ pub mod upgrade_client; pub enum ClientMsg { CreateClient(MsgCreateClient), UpdateClient(MsgUpdateClient), - Misbehaviour(MsgSubmitMisbehaviour), UpgradeClient(MsgUpgradeClient), } diff --git a/crates/ibc/src/core/ics02_client/msgs/misbehaviour.rs b/crates/ibc/src/core/ics02_client/msgs/misbehaviour.rs deleted file mode 100644 index a2e6a5e32..000000000 --- a/crates/ibc/src/core/ics02_client/msgs/misbehaviour.rs +++ /dev/null @@ -1,62 +0,0 @@ -use crate::prelude::*; - -use ibc_proto::google::protobuf::Any as ProtoAny; -use ibc_proto::ibc::core::client::v1::MsgSubmitMisbehaviour as RawMsgSubmitMisbehaviour; -use ibc_proto::protobuf::Protobuf; - -use crate::core::ics02_client::error::ClientError; -use crate::core::ics24_host::identifier::ClientId; -use crate::signer::Signer; -use crate::tx_msg::Msg; - -pub const TYPE_URL: &str = "/ibc.core.client.v1.MsgSubmitMisbehaviour"; - -/// A type of message that submits client misbehaviour proof. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct MsgSubmitMisbehaviour { - /// client unique identifier - pub client_id: ClientId, - /// misbehaviour used for freezing the light client - pub misbehaviour: ProtoAny, - /// signer address - pub signer: Signer, -} - -impl Msg for MsgSubmitMisbehaviour { - type Raw = RawMsgSubmitMisbehaviour; - - fn type_url(&self) -> String { - TYPE_URL.to_string() - } -} - -impl Protobuf for MsgSubmitMisbehaviour {} - -impl TryFrom for MsgSubmitMisbehaviour { - type Error = ClientError; - - fn try_from(raw: RawMsgSubmitMisbehaviour) -> Result { - let raw_misbehaviour = raw - .misbehaviour - .ok_or(ClientError::MissingRawMisbehaviour)?; - - Ok(MsgSubmitMisbehaviour { - client_id: raw - .client_id - .parse() - .map_err(ClientError::InvalidRawMisbehaviour)?, - misbehaviour: raw_misbehaviour, - signer: raw.signer.parse().map_err(ClientError::Signer)?, - }) - } -} - -impl From for RawMsgSubmitMisbehaviour { - fn from(ics_msg: MsgSubmitMisbehaviour) -> Self { - RawMsgSubmitMisbehaviour { - client_id: ics_msg.client_id.to_string(), - misbehaviour: Some(ics_msg.misbehaviour), - signer: ics_msg.signer.to_string(), - } - } -} diff --git a/crates/ibc/src/core/ics26_routing/msgs.rs b/crates/ibc/src/core/ics26_routing/msgs.rs index 397286192..41a25a499 100644 --- a/crates/ibc/src/core/ics26_routing/msgs.rs +++ b/crates/ibc/src/core/ics26_routing/msgs.rs @@ -1,11 +1,10 @@ use crate::prelude::*; use ibc_proto::google::protobuf::Any; +use ibc_proto::ibc::core::client::v1::MsgSubmitMisbehaviour as RawMsgSubmitMisbehaviour; use ibc_proto::ibc::core::client::v1::MsgUpdateClient as RawMsgUpdateClient; -use crate::core::ics02_client::msgs::{ - create_client, misbehaviour, update_client, upgrade_client, ClientMsg, -}; +use crate::core::ics02_client::msgs::{create_client, update_client, upgrade_client, ClientMsg}; use crate::core::ics03_connection::msgs::{ conn_open_ack, conn_open_confirm, conn_open_init, conn_open_try, ConnectionMsg, }; @@ -50,10 +49,12 @@ impl TryFrom for MsgEnvelope { .map_err(RouterError::MalformedMessageBytes)?; Ok(MsgEnvelope::Client(ClientMsg::UpgradeClient(domain_msg))) } - misbehaviour::TYPE_URL => { - let domain_msg = misbehaviour::MsgSubmitMisbehaviour::decode_vec(&any_msg.value) - .map_err(RouterError::MalformedMessageBytes)?; - Ok(MsgEnvelope::Client(ClientMsg::Misbehaviour(domain_msg))) + update_client::MISBEHAVIOUR_TYPE_URL => { + let domain_msg = >::decode_vec(&any_msg.value) + .map_err(RouterError::MalformedMessageBytes)?; + Ok(MsgEnvelope::Client(ClientMsg::UpdateClient(domain_msg))) } // ICS03 From de42ab61613b079e4840fa13c1669d0b24d105f3 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 13:52:08 -0400 Subject: [PATCH 29/62] update_client::validate --- .../ics02_client/handler/update_client.rs | 44 +++---------------- 1 file changed, 5 insertions(+), 39 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 4ddce4fc7..1a6afeaa3 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -1,7 +1,5 @@ //! Protocol logic specific to processing ICS2 messages of type `MsgUpdateAnyClient`. -use tracing::debug; - use crate::prelude::*; use crate::core::ics02_client::client_state::UpdatedState; @@ -22,8 +20,8 @@ where { let MsgUpdateClient { client_id, - client_message: header, - update_kind: _update_kind, + client_message, + update_kind, signer: _, } = msg; @@ -33,41 +31,9 @@ where client_state.confirm_not_frozen()?; - // Read consensus state from the host chain store. - let latest_client_cons_state_path = - ClientConsensusStatePath::new(&client_id, &client_state.latest_height()); - let latest_consensus_state = ctx - .consensus_state(&latest_client_cons_state_path) - .map_err(|_| ClientError::ConsensusStateNotFound { - client_id: client_id.clone(), - height: client_state.latest_height(), - })?; - - debug!("latest consensus state: {:?}", latest_consensus_state); - - let now = ctx.host_timestamp()?; - let duration = now - .duration_since(&latest_consensus_state.timestamp()) - .ok_or_else(|| ClientError::InvalidConsensusStateTimestamp { - time1: latest_consensus_state.timestamp(), - time2: now, - })?; - - if client_state.expired(duration) { - return Err(ClientError::HeaderNotWithinTrustPeriod { - latest_time: latest_consensus_state.timestamp(), - update_time: now, - } - .into()); - } - - let _ = client_state - .check_header_and_update_state(ctx, client_id.clone(), header) - .map_err(|e| ClientError::HeaderVerificationFailure { - reason: e.to_string(), - })?; - - Ok(()) + client_state + .verify_client_message(ctx, client_id, client_message, update_kind) + .map_err(ContextError::from) } pub(crate) fn execute(ctx: &mut Ctx, msg: MsgUpdateClient) -> Result<(), ContextError> From 2856ce104f101fc54361822523f966d5f4250994 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 14:15:13 -0400 Subject: [PATCH 30/62] implement part of update_client::execute --- .../clients/ics07_tendermint/client_state.rs | 50 +++++++++-------- .../ibc/src/core/ics02_client/client_state.rs | 18 +++--- .../ics02_client/handler/update_client.rs | 56 ++++++++----------- crates/ibc/src/mock/client_state.rs | 32 ++++++----- 4 files changed, 76 insertions(+), 80 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index c5eaf80df..69d4eff1a 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -215,7 +215,7 @@ impl ClientState { fn verify_header( &self, ctx: &dyn ValidationContext, - client_id: ClientId, + client_id: &ClientId, header: TmHeader, ) -> Result<(), ClientError> { // The tendermint-light-client crate though works on heights that are assumed @@ -250,7 +250,7 @@ impl ClientState { let trusted_state = { let trusted_client_cons_state_path = - ClientConsensusStatePath::new(&client_id, &header.trusted_height); + ClientConsensusStatePath::new(client_id, &header.trusted_height); let trusted_consensus_state = downcast_tm_consensus_state( ctx.consensus_state(&trusted_client_cons_state_path)? .as_ref(), @@ -300,13 +300,13 @@ impl ClientState { fn verify_misbehaviour( &self, ctx: &dyn ValidationContext, - client_id: ClientId, + client_id: &ClientId, misbehaviour: TmMisbehaviour, ) -> Result<(), ClientError> { let header_1 = misbehaviour.header1(); let trusted_consensus_state_1 = { let consensus_state_path = - ClientConsensusStatePath::new(&client_id, &header_1.trusted_height); + ClientConsensusStatePath::new(client_id, &header_1.trusted_height); let consensus_state = ctx.consensus_state(&consensus_state_path)?; downcast_tm_consensus_state(consensus_state.as_ref()) @@ -315,7 +315,7 @@ impl ClientState { let header_2 = misbehaviour.header2(); let trusted_consensus_state_2 = { let consensus_state_path = - ClientConsensusStatePath::new(&client_id, &header_2.trusted_height); + ClientConsensusStatePath::new(client_id, &header_2.trusted_height); let consensus_state = ctx.consensus_state(&consensus_state_path)?; downcast_tm_consensus_state(consensus_state.as_ref()) @@ -504,9 +504,9 @@ impl Ics2ClientState for ClientState { fn verify_client_message( &self, ctx: &dyn ValidationContext, - client_id: ClientId, + client_id: &ClientId, client_message: Any, - update_kind: UpdateClientKind, + update_kind: &UpdateClientKind, ) -> Result<(), ClientError> { match update_kind { UpdateClientKind::UpdateHeader => { @@ -526,9 +526,9 @@ impl Ics2ClientState for ClientState { fn check_for_misbehaviour( &self, ctx: &dyn ValidationContext, - client_id: ClientId, + client_id: &ClientId, client_message: Any, - update_kind: UpdateClientKind, + update_kind: &UpdateClientKind, ) -> Result { match update_kind { UpdateClientKind::UpdateHeader => { @@ -538,7 +538,7 @@ impl Ics2ClientState for ClientState { let maybe_existing_consensus_state = { let path_at_header_height = - ClientConsensusStatePath::new(&client_id, &header.height()); + ClientConsensusStatePath::new(client_id, &header.height()); ctx.consensus_state(&path_at_header_height).ok() }; @@ -561,7 +561,7 @@ impl Ics2ClientState for ClientState { // the “previous header” { let maybe_prev_cs = - ctx.prev_consensus_state(&client_id, &header.height())?; + ctx.prev_consensus_state(client_id, &header.height())?; if let Some(prev_cs) = maybe_prev_cs { // New header timestamp cannot occur *before* the @@ -578,7 +578,7 @@ impl Ics2ClientState for ClientState { // that its timestamp is less than the “next header” if header.height() < self.latest_height() { let maybe_next_cs = - ctx.next_consensus_state(&client_id, &header.height())?; + ctx.next_consensus_state(client_id, &header.height())?; if let Some(next_cs) = maybe_next_cs { // New (untrusted) header timestamp cannot occur *after* next @@ -608,9 +608,6 @@ impl Ics2ClientState for ClientState { Ok(header_1.signed_header.commit.block_id.hash != header_2.signed_header.commit.block_id.hash) } else { - // FIXME BEFORE MERGE: ibc-go ensures that header_1.height > header_2.height - // in Misbehaviour.ValidateBasic(). We are missing all these checks. - // header_1 is at greater height than header_2, therefore // header_1 time must be less than or equal to // header_2 time in order to be valid misbehaviour (violation of @@ -623,13 +620,17 @@ impl Ics2ClientState for ClientState { fn update_state( &self, ctx: &mut dyn ExecutionContext, - client_id: ClientId, - header: Any, + client_id: &ClientId, + client_message: Any, + _update_kind: &UpdateClientKind, ) -> Result<(), ClientError> { - let header = TmHeader::try_from(header)?; + // we only expect headers to make it here; if a TmMisbehaviour makes it to here, + // then it is not a valid misbehaviour (check_for_misbehaviour returned false), + // and so transaction should fail. + let header = TmHeader::try_from(client_message)?; let maybe_existing_consensus_state = { - let path_at_header_height = ClientConsensusStatePath::new(&client_id, &header.height()); + let path_at_header_height = ClientConsensusStatePath::new(client_id, &header.height()); ctx.consensus_state(&path_at_header_height).ok() }; @@ -654,10 +655,10 @@ impl Ics2ClientState for ClientState { )?; ctx.store_consensus_state( - ClientConsensusStatePath::new(&client_id, &new_client_state.latest_height()), + ClientConsensusStatePath::new(client_id, &new_client_state.latest_height()), new_consensus_state, )?; - ctx.store_client_state(ClientStatePath::new(&client_id), new_client_state)?; + ctx.store_client_state(ClientStatePath::new(client_id), new_client_state)?; Ok(()) } @@ -666,15 +667,16 @@ impl Ics2ClientState for ClientState { fn update_state_on_misbehaviour( &self, ctx: &mut dyn ExecutionContext, - client_id: ClientId, - _misbehaviour: Any, + client_id: &ClientId, + _client_message: Any, + _update_kind: &UpdateClientKind, ) -> Result<(), ClientError> { let frozen_client_state = self .clone() .with_frozen_height(Height::new(0, 1).unwrap()) .into_box(); - ctx.store_client_state(ClientStatePath::new(&client_id), frozen_client_state)?; + ctx.store_client_state(ClientStatePath::new(client_id), frozen_client_state)?; Ok(()) } diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 8b0072861..6892d7e65 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -73,32 +73,34 @@ pub trait ClientState: fn verify_client_message( &self, ctx: &dyn ValidationContext, - client_id: ClientId, + client_id: &ClientId, client_message: Any, - update_kind: UpdateClientKind, + update_kind: &UpdateClientKind, ) -> Result<(), ClientError>; /// Check whether misbehaviour has occured fn check_for_misbehaviour( &self, ctx: &dyn ValidationContext, - client_id: ClientId, + client_id: &ClientId, client_message: Any, - update_kind: UpdateClientKind, + update_kind: &UpdateClientKind, ) -> Result; fn update_state( &self, ctx: &mut dyn ExecutionContext, - client_id: ClientId, - header: Any, + client_id: &ClientId, + client_message: Any, + update_kind: &UpdateClientKind, ) -> Result<(), ClientError>; fn update_state_on_misbehaviour( &self, ctx: &mut dyn ExecutionContext, - client_id: ClientId, - misbehaviour: Any, + client_id: &ClientId, + client_message: Any, + update_kind: &UpdateClientKind, ) -> Result<(), ClientError>; fn check_header_and_update_state( 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 1a6afeaa3..00c0aeda9 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -2,16 +2,12 @@ use crate::prelude::*; -use crate::core::ics02_client::client_state::UpdatedState; -use crate::core::ics02_client::error::ClientError; use crate::core::ics02_client::events::UpdateClient; use crate::core::ics02_client::msgs::update_client::MsgUpdateClient; use crate::events::IbcEvent; use crate::core::context::ContextError; -use crate::core::ics24_host::path::{ClientConsensusStatePath, ClientStatePath}; - use crate::core::{ExecutionContext, ValidationContext}; pub(crate) fn validate(ctx: &Ctx, msg: MsgUpdateClient) -> Result<(), ContextError> @@ -32,7 +28,7 @@ where client_state.confirm_not_frozen()?; client_state - .verify_client_message(ctx, client_id, client_message, update_kind) + .verify_client_message(ctx, &client_id, client_message, &update_kind) .map_err(ContextError::from) } @@ -42,40 +38,34 @@ where { let MsgUpdateClient { client_id, - client_message: header, - update_kind: _update_kind, + client_message, + update_kind, signer: _, } = msg; - // Read client type from the host chain store. The client should already exist. - // Read client state from the host chain store. let client_state = ctx.client_state(&client_id)?; - let UpdatedState { - client_state, - consensus_state, - } = client_state - .check_header_and_update_state(ctx, client_id.clone(), header.clone()) - .map_err(|e| ClientError::HeaderVerificationFailure { - reason: e.to_string(), - })?; - - ctx.store_client_state(ClientStatePath::new(&client_id), client_state.clone())?; - ctx.store_consensus_state( - ClientConsensusStatePath::new(&client_id, &client_state.latest_height()), - consensus_state, - )?; - ctx.store_update_time( - client_id.clone(), - client_state.latest_height(), - ctx.host_timestamp()?, - )?; - ctx.store_update_height( - client_id.clone(), - client_state.latest_height(), - ctx.host_height()?, + let found_misbehaviour = client_state.check_for_misbehaviour( + ctx, + &client_id, + client_message.clone(), + &update_kind, )?; + if found_misbehaviour { + client_state.update_state_on_misbehaviour( + ctx, + &client_id, + client_message.clone(), + &update_kind, + )?; + } else { + client_state.update_state(ctx, &client_id, client_message.clone(), &update_kind)?; + } + + // TODO: fix events stuff + // + misbehaviour or update_client + // + use return consensus_heights from update_state { let consensus_height = client_state.latest_height(); @@ -84,7 +74,7 @@ where client_state.client_type(), consensus_height, vec![consensus_height], - header, + client_message, )); ctx.emit_ibc_event(IbcEvent::Message(event.event_type())); ctx.emit_ibc_event(event); diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 01ee819ea..81c746d48 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -287,21 +287,12 @@ impl ClientState for MockClientState { Ok(()) } - fn update_state( - &self, - _ctx: &mut dyn ExecutionContext, - _client_id: ClientId, - _header: Any, - ) -> Result<(), ClientError> { - todo!() - } - fn verify_client_message( &self, _ctx: &dyn ValidationContext, - _client_id: ClientId, + _client_id: &ClientId, _client_message: Any, - _update_kind: UpdateClientKind, + _update_kind: &UpdateClientKind, ) -> Result<(), ClientError> { todo!() } @@ -309,18 +300,29 @@ impl ClientState for MockClientState { fn check_for_misbehaviour( &self, _ctx: &dyn ValidationContext, - _client_id: ClientId, + _client_id: &ClientId, _client_message: Any, - _update_kind: UpdateClientKind, + _update_kind: &UpdateClientKind, ) -> Result { todo!() } + fn update_state( + &self, + _ctx: &mut dyn ExecutionContext, + _client_id: &ClientId, + _client_message: Any, + _update_kind: &UpdateClientKind, + ) -> Result<(), ClientError> { + todo!() + } + fn update_state_on_misbehaviour( &self, _ctx: &mut dyn ExecutionContext, - _client_id: ClientId, - _misbehaviour: Any, + _client_id: &ClientId, + _client_message: Any, + _update_kind: &UpdateClientKind, ) -> Result<(), ClientError> { todo!() } From 72eb2168f687f25e1a6e073f7679b539db5b6242 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 14:18:07 -0400 Subject: [PATCH 31/62] separate events emitted --- .../core/ics02_client/handler/update_client.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 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 00c0aeda9..4f55dfe75 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -2,7 +2,7 @@ use crate::prelude::*; -use crate::core::ics02_client::events::UpdateClient; +use crate::core::ics02_client::events::{ClientMisbehaviour, UpdateClient}; use crate::core::ics02_client::msgs::update_client::MsgUpdateClient; use crate::events::IbcEvent; @@ -59,14 +59,17 @@ where client_message.clone(), &update_kind, )?; + + let event = IbcEvent::ClientMisbehaviour(ClientMisbehaviour::new( + client_id.clone(), + client_state.client_type(), + )); + ctx.emit_ibc_event(IbcEvent::Message(event.event_type())); + ctx.emit_ibc_event(event); } else { client_state.update_state(ctx, &client_id, client_message.clone(), &update_kind)?; - } - // TODO: fix events stuff - // + misbehaviour or update_client - // + use return consensus_heights from update_state - { + // TODO: fix events stuff let consensus_height = client_state.latest_height(); let event = IbcEvent::UpdateClient(UpdateClient::new( From 2cfd32b7868e6444f931898aca373dd08a40f1ba Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 14:36:34 -0400 Subject: [PATCH 32/62] finish update_client::execute --- .../clients/ics07_tendermint/client_state.rs | 13 ++++++++----- .../ibc/src/core/ics02_client/client_state.rs | 19 +++++++++++++++---- .../ics02_client/handler/update_client.rs | 7 +------ crates/ibc/src/mock/client_state.rs | 2 +- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 69d4eff1a..ad3070a0f 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -623,14 +623,15 @@ impl Ics2ClientState for ClientState { client_id: &ClientId, client_message: Any, _update_kind: &UpdateClientKind, - ) -> Result<(), ClientError> { + ) -> Result, ClientError> { // we only expect headers to make it here; if a TmMisbehaviour makes it to here, // then it is not a valid misbehaviour (check_for_misbehaviour returned false), // and so transaction should fail. let header = TmHeader::try_from(client_message)?; + let header_height = header.height(); let maybe_existing_consensus_state = { - let path_at_header_height = ClientConsensusStatePath::new(client_id, &header.height()); + let path_at_header_height = ClientConsensusStatePath::new(client_id, &header_height); ctx.consensus_state(&path_at_header_height).ok() }; @@ -638,7 +639,8 @@ impl Ics2ClientState for ClientState { if maybe_existing_consensus_state.is_some() { // if we already had the header installed by a previous relayer // then this is a no-op. - Ok(()) + // + // Do nothing. } else { let new_consensus_state = TmConsensusState::from(header.clone()).into_box(); let new_client_state = self.clone().with_header(header)?.into_box(); @@ -659,9 +661,10 @@ impl Ics2ClientState for ClientState { new_consensus_state, )?; ctx.store_client_state(ClientStatePath::new(client_id), new_client_state)?; - - Ok(()) } + + let updated_heights = vec![header_height]; + Ok(updated_heights) } fn update_state_on_misbehaviour( diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 6892d7e65..4129d9de2 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -68,8 +68,12 @@ pub trait ClientState: fn initialise(&self, consensus_state: Any) -> Result, ClientError>; - /// checks the that the structure of a ClientMessage is correct and that all - /// authentication data provided is valid. + /// verify_client_message must verify a client_message. A client_message + /// could be a Header, Misbehaviour. It must handle each type of + /// client_message appropriately. Calls to check_for_misbehaviour, + /// update_state, and update_state_on_misbehaviour will assume that the + /// content of the client_message has been verified and can be trusted. An + /// error should be returned if the client_message fails to verify. fn verify_client_message( &self, ctx: &dyn ValidationContext, @@ -78,7 +82,8 @@ pub trait ClientState: update_kind: &UpdateClientKind, ) -> Result<(), ClientError>; - /// Check whether misbehaviour has occured + /// Checks for evidence of a misbehaviour in Header or Misbehaviour type. It + /// assumes the client_message has already been verified. fn check_for_misbehaviour( &self, ctx: &dyn ValidationContext, @@ -87,14 +92,20 @@ pub trait ClientState: update_kind: &UpdateClientKind, ) -> Result; + /// Updates and stores as necessary any associated information for an IBC + /// client, such as the ClientState and corresponding ConsensusState. Upon + /// successful update, a list of consensus heights is returned. It assumes + /// the client_message has already been verified. fn update_state( &self, ctx: &mut dyn ExecutionContext, client_id: &ClientId, client_message: Any, update_kind: &UpdateClientKind, - ) -> Result<(), ClientError>; + ) -> Result, ClientError>; + /// update_state_on_misbehaviour should perform appropriate state changes on + /// a client state given that misbehaviour has been detected and verified fn update_state_on_misbehaviour( &self, ctx: &mut dyn ExecutionContext, 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 4f55dfe75..3431c61d8 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -53,12 +53,7 @@ where )?; if found_misbehaviour { - client_state.update_state_on_misbehaviour( - ctx, - &client_id, - client_message.clone(), - &update_kind, - )?; + client_state.update_state_on_misbehaviour(ctx, &client_id, client_message, &update_kind)?; let event = IbcEvent::ClientMisbehaviour(ClientMisbehaviour::new( client_id.clone(), diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 81c746d48..0b8af9b4c 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -313,7 +313,7 @@ impl ClientState for MockClientState { _client_id: &ClientId, _client_message: Any, _update_kind: &UpdateClientKind, - ) -> Result<(), ClientError> { + ) -> Result, ClientError> { todo!() } From 9b9eab156a2fee69556e44e4e1129535903e4604 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 4 Apr 2023 14:37:44 -0400 Subject: [PATCH 33/62] remove TODO --- crates/ibc/src/core/ics02_client/handler/update_client.rs | 1 - 1 file changed, 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 3431c61d8..785b02d5f 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -64,7 +64,6 @@ where } else { client_state.update_state(ctx, &client_id, client_message.clone(), &update_kind)?; - // TODO: fix events stuff let consensus_height = client_state.latest_height(); let event = IbcEvent::UpdateClient(UpdateClient::new( From 2f35259a42dbae6e87d5cae34ca41a617c0ec83c Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 08:56:25 -0400 Subject: [PATCH 34/62] implement mock client state --- .../ics02_client/handler/update_client.rs | 2 +- crates/ibc/src/mock/client_state.rs | 73 +++++++++++++++---- 2 files changed, 61 insertions(+), 14 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 785b02d5f..76a91bc36 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -425,7 +425,7 @@ mod tests { header2: MockHeader::new(height).with_timestamp(timestamp), } .into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateClientKind::Misbehaviour, signer: get_dummy_account_id(), }; diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 0b8af9b4c..537584733 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -1,4 +1,5 @@ use crate::core::ics02_client::msgs::update_client::UpdateClientKind; +use crate::core::ics24_host::path::{ClientConsensusStatePath, ClientStatePath}; use crate::prelude::*; use alloc::collections::btree_map::BTreeMap as HashMap; @@ -291,40 +292,86 @@ impl ClientState for MockClientState { &self, _ctx: &dyn ValidationContext, _client_id: &ClientId, - _client_message: Any, - _update_kind: &UpdateClientKind, + client_message: Any, + update_kind: &UpdateClientKind, ) -> Result<(), ClientError> { - todo!() + match update_kind { + UpdateClientKind::UpdateHeader => { + let _header = MockHeader::try_from(client_message)?; + } + UpdateClientKind::Misbehaviour => { + let _misbehaviour = Misbehaviour::try_from(client_message)?; + } + } + + Ok(()) } + /// We consider the following to be misbehaviour: + /// + UpdateHeader: new header is at a lower height + /// + Misbehaviour: headers are at same height and are in the future fn check_for_misbehaviour( &self, _ctx: &dyn ValidationContext, _client_id: &ClientId, - _client_message: Any, - _update_kind: &UpdateClientKind, + client_message: Any, + update_kind: &UpdateClientKind, ) -> Result { - todo!() + match update_kind { + UpdateClientKind::UpdateHeader => { + let header = MockHeader::try_from(client_message)?; + + Ok(self.latest_height() < header.height()) + } + UpdateClientKind::Misbehaviour => { + let misbehaviour = Misbehaviour::try_from(client_message)?; + let header_1 = misbehaviour.header1; + let header_2 = misbehaviour.header2; + + let header_heights_equal = header_1.height() != header_2.height(); + let headers_are_in_future = self.latest_height() < header_1.height(); + + Ok(header_heights_equal && headers_are_in_future) + } + } } fn update_state( &self, - _ctx: &mut dyn ExecutionContext, - _client_id: &ClientId, - _client_message: Any, + ctx: &mut dyn ExecutionContext, + client_id: &ClientId, + client_message: Any, _update_kind: &UpdateClientKind, ) -> Result, ClientError> { - todo!() + let header = MockHeader::try_from(client_message)?; + let header_height = header.height; + + let new_client_state = MockClientState::new(header).into_box(); + let new_consensus_state = MockConsensusState::new(header).into_box(); + + ctx.store_consensus_state( + ClientConsensusStatePath::new(client_id, &new_client_state.latest_height()), + new_consensus_state, + )?; + ctx.store_client_state(ClientStatePath::new(client_id), new_client_state)?; + + Ok(vec![header_height]) } fn update_state_on_misbehaviour( &self, - _ctx: &mut dyn ExecutionContext, - _client_id: &ClientId, + ctx: &mut dyn ExecutionContext, + client_id: &ClientId, _client_message: Any, _update_kind: &UpdateClientKind, ) -> Result<(), ClientError> { - todo!() + let frozen_client_state = self + .with_frozen_height(Height::new(0, 1).unwrap()) + .into_box(); + + ctx.store_client_state(ClientStatePath::new(client_id), frozen_client_state)?; + + Ok(()) } } From 4eb7fb323bf2bb6f0d2d14ee337bd13eae22ec39 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 09:11:55 -0400 Subject: [PATCH 35/62] fix mockclientstate --- crates/ibc/src/mock/client_state.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 537584733..8a1e22ea9 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -297,7 +297,14 @@ impl ClientState for MockClientState { ) -> Result<(), ClientError> { match update_kind { UpdateClientKind::UpdateHeader => { - let _header = MockHeader::try_from(client_message)?; + let header = MockHeader::try_from(client_message)?; + + if self.latest_height() >= header.height() { + return Err(ClientError::LowHeaderHeight { + header_height: header.height(), + latest_height: self.latest_height(), + }); + } } UpdateClientKind::Misbehaviour => { let _misbehaviour = Misbehaviour::try_from(client_message)?; @@ -308,7 +315,7 @@ impl ClientState for MockClientState { } /// We consider the following to be misbehaviour: - /// + UpdateHeader: new header is at a lower height + /// + UpdateHeader: no misbehaviour possible /// + Misbehaviour: headers are at same height and are in the future fn check_for_misbehaviour( &self, @@ -318,11 +325,7 @@ impl ClientState for MockClientState { update_kind: &UpdateClientKind, ) -> Result { match update_kind { - UpdateClientKind::UpdateHeader => { - let header = MockHeader::try_from(client_message)?; - - Ok(self.latest_height() < header.height()) - } + UpdateClientKind::UpdateHeader => Ok(false), UpdateClientKind::Misbehaviour => { let misbehaviour = Misbehaviour::try_from(client_message)?; let header_1 = misbehaviour.header1; From f31d773634a27d0960ce5396098328b6bf76981a Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 09:27:28 -0400 Subject: [PATCH 36/62] fix event created on update_client --- crates/ibc/src/core/ics02_client/client_state.rs | 3 +++ .../src/core/ics02_client/handler/update_client.rs | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 4129d9de2..0c364167c 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -96,6 +96,9 @@ pub trait ClientState: /// client, such as the ClientState and corresponding ConsensusState. Upon /// successful update, a list of consensus heights is returned. It assumes /// the client_message has already been verified. + /// + /// Post-condition: on success, the return value MUST contain at least one + /// height. fn update_state( &self, ctx: &mut dyn ExecutionContext, 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 76a91bc36..719b5530b 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -1,5 +1,6 @@ //! Protocol logic specific to processing ICS2 messages of type `MsgUpdateAnyClient`. +use crate::core::ics02_client::error::ClientError; use crate::prelude::*; use crate::core::ics02_client::events::{ClientMisbehaviour, UpdateClient}; @@ -62,15 +63,18 @@ where ctx.emit_ibc_event(IbcEvent::Message(event.event_type())); ctx.emit_ibc_event(event); } else { - client_state.update_state(ctx, &client_id, client_message.clone(), &update_kind)?; + let consensus_heights = + client_state.update_state(ctx, &client_id, client_message.clone(), &update_kind)?; - let consensus_height = client_state.latest_height(); + let consensus_height = consensus_heights.get(0).ok_or(ClientError::Other { + description: "client update state returned no updated height".to_string(), + })?; let event = IbcEvent::UpdateClient(UpdateClient::new( client_id, client_state.client_type(), - consensus_height, - vec![consensus_height], + *consensus_height, + consensus_heights, client_message, )); ctx.emit_ibc_event(IbcEvent::Message(event.event_type())); From 023b6315d35f6fb8643f1b66ebbb19526d98cc84 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 09:34:07 -0400 Subject: [PATCH 37/62] fix mock --- crates/ibc/src/mock/client_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 8a1e22ea9..17f900ddf 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -331,7 +331,7 @@ impl ClientState for MockClientState { let header_1 = misbehaviour.header1; let header_2 = misbehaviour.header2; - let header_heights_equal = header_1.height() != header_2.height(); + let header_heights_equal = header_1.height() == header_2.height(); let headers_are_in_future = self.latest_height() < header_1.height(); Ok(header_heights_equal && headers_are_in_future) From c0d72e61a0e8ac8e7e30c835c0fe40605d0738d2 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 09:36:39 -0400 Subject: [PATCH 38/62] fix mock --- crates/ibc/src/mock/client_state.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 17f900ddf..b4b2d62d5 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -352,6 +352,16 @@ impl ClientState for MockClientState { let new_client_state = MockClientState::new(header).into_box(); let new_consensus_state = MockConsensusState::new(header).into_box(); + ctx.store_update_time( + client_id.clone(), + new_client_state.latest_height(), + ctx.host_timestamp()?, + )?; + ctx.store_update_height( + client_id.clone(), + new_client_state.latest_height(), + ctx.host_height()?, + )?; ctx.store_consensus_state( ClientConsensusStatePath::new(client_id, &new_client_state.latest_height()), new_consensus_state, From 9181e52dba78990e7e1bab9b48f2b009b9b3220e Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 10:05:34 -0400 Subject: [PATCH 39/62] test var names --- .../core/ics02_client/handler/update_client.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 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 719b5530b..be6453d0d 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -261,13 +261,13 @@ mod tests { let client_id = ClientId::new(tm_client_type(), 0).unwrap(); let client_height = Height::new(1, 20).unwrap(); - let chain_start_height = Height::new(1, 11).unwrap(); + let ctx_a_start_height = Height::new(1, 11).unwrap(); - let mut ctx = MockContext::new( + let mut ctx_a = MockContext::new( ChainId::new("mockgaiaA".to_string(), 1), HostType::Mock, 5, - chain_start_height, + ctx_a_start_height, ) .with_client_parametrized( &client_id, @@ -288,11 +288,11 @@ mod tests { let block = ctx_b.host_block(&client_height).unwrap().clone(); let block = match block { HostBlock::SyntheticTendermint(mut theader) => { - let cons_state = ctx.latest_consensus_states(&client_id, &client_height); + let cons_state = ctx_a.latest_consensus_states(&client_id, &client_height); if let Some(tcs) = downcast_consensus_state::(cons_state.as_ref()) { theader.light_block.signed_header.header.time = tcs.timestamp; - theader.trusted_height = Height::new(1, 11).unwrap(); + theader.trusted_height = ctx_a_start_height; } HostBlock::SyntheticTendermint(theader) } @@ -307,16 +307,16 @@ mod tests { signer, }; - let res = validate(&ctx, msg.clone()); + let res = validate(&ctx_a, msg.clone()); assert!(res.is_ok()); - let res = execute(&mut ctx, msg.clone()); + let res = execute(&mut ctx_a, msg.clone()); assert!(res.is_ok(), "result: {res:?}"); - let client_state = ctx.client_state(&msg.client_id).unwrap(); + let client_state = ctx_a.client_state(&msg.client_id).unwrap(); assert!(client_state.confirm_not_frozen().is_ok()); assert_eq!(client_state.latest_height(), latest_header_height); - assert_eq!(client_state, ctx.latest_client_states(&msg.client_id)); + assert_eq!(client_state, ctx_a.latest_client_states(&msg.client_id)); } #[test] From f0384e6a0f3b87fa0f4d3e890dd87ad4e3e2f7e5 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 11:49:50 -0400 Subject: [PATCH 40/62] fix parts of the problem with test --- .../clients/ics07_tendermint/client_state.rs | 6 +- .../ics02_client/handler/update_client.rs | 68 ++++++++++++------- crates/ibc/src/mock/context.rs | 5 ++ 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index ad3070a0f..2e5bf60ff 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -548,10 +548,8 @@ impl Ics2ClientState for ClientState { let existing_consensus_state = downcast_tm_consensus_state(existing_consensus_state.as_ref())?; - // check if there is already a consensus state stored for the - // submitted header. If there is, there is no misbehaviour to report. - // Otherwise, we just confirmed that 2 headers exist with the same - // height, which is evidence of misbehaviour. + // There is evidence of misbehaviour if the stored consensus state + // is different from the new one we received. Ok(existing_consensus_state != header_consensus_state) } None => { 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 be6453d0d..4e6652364 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -91,12 +91,11 @@ mod tests { use test_log::test; use crate::clients::ics07_tendermint::client_type as tm_client_type; - use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::clients::ics07_tendermint::header::Header as TmHeader; use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; use crate::core::ics02_client::client_state::ClientState; use crate::core::ics02_client::client_type::ClientType; - use crate::core::ics02_client::consensus_state::downcast_consensus_state; + use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::handler::update_client::{execute, validate}; use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateClientKind}; use crate::core::ics24_host::identifier::{ChainId, ClientId}; @@ -261,23 +260,44 @@ mod tests { let client_id = ClientId::new(tm_client_type(), 0).unwrap(); let client_height = Height::new(1, 20).unwrap(); - let ctx_a_start_height = Height::new(1, 11).unwrap(); + let ctx_a_chain_id = ChainId::new("mockgaiaA".to_string(), 1); + let ctx_b_chain_id = ChainId::new("mockgaiaB".to_string(), 1); + let start_height = Height::new(1, 11).unwrap(); + + let mut ctx_a = MockContext::new(ctx_a_chain_id.clone(), HostType::Mock, 5, start_height) + .with_client_parametrized_with_chain_id( + ctx_b_chain_id.clone(), + &client_id, + client_height, + Some(tm_client_type()), // The target host chain (B) is synthetic TM. + Some(start_height), + ); - let mut ctx_a = MockContext::new( - ChainId::new("mockgaiaA".to_string(), 1), - HostType::Mock, - 5, - ctx_a_start_height, - ) - .with_client_parametrized( - &client_id, - client_height, - Some(tm_client_type()), // The target host chain (B) is synthetic TM. - Some(client_height), - ); + // Update the client height to `client_height` + // + // Note: The current MockContext interface doesn't allow us to + // do this without a major redesign. + { + let consensus_state: Box = { + let light_block = HostBlock::generate_tm_block( + ctx_b_chain_id.clone(), + client_height.revision_height(), + Timestamp::now(), + ); + + light_block.into() + }; + + let mut ibc_store = ctx_a.ibc_store.lock(); + let client_record = ibc_store.clients.get_mut(&client_id).unwrap(); + + client_record + .consensus_states + .insert(client_height, consensus_state); + } let ctx_b = MockContext::new( - ChainId::new("mockgaiaB".to_string(), 1), + ctx_b_chain_id, HostType::SyntheticTendermint, 5, client_height, @@ -286,14 +306,16 @@ mod tests { let signer = get_dummy_account_id(); let block = ctx_b.host_block(&client_height).unwrap().clone(); + + // Update the trusted height of the header to point to the previous height + // (`start_height` in this case). + // + // Note: The current MockContext interface doesn't allow us to + // do this without a major redesign. let block = match block { HostBlock::SyntheticTendermint(mut theader) => { - let cons_state = ctx_a.latest_consensus_states(&client_id, &client_height); - if let Some(tcs) = downcast_consensus_state::(cons_state.as_ref()) - { - theader.light_block.signed_header.header.time = tcs.timestamp; - theader.trusted_height = ctx_a_start_height; - } + theader.trusted_height = start_height; + HostBlock::SyntheticTendermint(theader) } _ => block, @@ -308,7 +330,7 @@ mod tests { }; let res = validate(&ctx_a, msg.clone()); - assert!(res.is_ok()); + assert!(res.is_ok(), "result: {res:?}"); let res = execute(&mut ctx_a, msg.clone()); assert!(res.is_ok(), "result: {res:?}"); diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index 554925abe..585544c6d 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -211,7 +211,12 @@ impl MockContext { client_type: Option, consensus_state_height: Option, ) -> Self { + // NOTE: this is wrong; the client chain ID is supposed to represent + // the chain ID of the counterparty chain. But at this point this is + // too ingrained in our tests; `with_client()` is called everywhere, + // which delegates to this. let client_chain_id = self.host_chain_id.clone(); + self.with_client_parametrized_with_chain_id( client_chain_id, client_id, From 2f6903fc0d97ba8644663438a67f707395acdc76 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 13:59:05 -0400 Subject: [PATCH 41/62] fix test --- .../ics02_client/handler/update_client.rs | 50 ++++++++++--------- 1 file changed, 27 insertions(+), 23 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 4e6652364..669ac9f9c 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -90,6 +90,7 @@ mod tests { use ibc_proto::google::protobuf::Any; use test_log::test; + use crate::clients::ics07_tendermint::client_state::test_util::get_dummy_tendermint_client_state; use crate::clients::ics07_tendermint::client_type as tm_client_type; use crate::clients::ics07_tendermint::header::Header as TmHeader; use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; @@ -273,29 +274,6 @@ mod tests { Some(start_height), ); - // Update the client height to `client_height` - // - // Note: The current MockContext interface doesn't allow us to - // do this without a major redesign. - { - let consensus_state: Box = { - let light_block = HostBlock::generate_tm_block( - ctx_b_chain_id.clone(), - client_height.revision_height(), - Timestamp::now(), - ); - - light_block.into() - }; - - let mut ibc_store = ctx_a.ibc_store.lock(); - let client_record = ibc_store.clients.get_mut(&client_id).unwrap(); - - client_record - .consensus_states - .insert(client_height, consensus_state); - } - let ctx_b = MockContext::new( ctx_b_chain_id, HostType::SyntheticTendermint, @@ -314,6 +292,9 @@ mod tests { // do this without a major redesign. let block = match block { HostBlock::SyntheticTendermint(mut theader) => { + // current problem: the timestamp of the new header doesn't match the timestamp of + // the stored consensus state. If we hack them to match, then commit check fails. + // FIXME: figure out why they don't match. theader.trusted_height = start_height; HostBlock::SyntheticTendermint(theader) @@ -321,6 +302,29 @@ mod tests { _ => block, }; + // Update the client height to `client_height` + // + // Note: The current MockContext interface doesn't allow us to + // do this without a major redesign. + { + // FIXME: idea: we need to update the light client with the latest block from + // chain B + let consensus_state: Box = block.clone().into(); + + let tm_block = downcast!(block.clone() => HostBlock::SyntheticTendermint).unwrap(); + let client_state = + get_dummy_tendermint_client_state(tm_block.header().clone()).into_box(); + + let mut ibc_store = ctx_a.ibc_store.lock(); + let client_record = ibc_store.clients.get_mut(&client_id).unwrap(); + + client_record + .consensus_states + .insert(client_height, consensus_state); + + client_record.client_state = Some(client_state); + } + let latest_header_height = block.height(); let msg = MsgUpdateClient { client_id, From 589ec9838018c4b26e49b355943537f4ad14b174 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 13:59:37 -0400 Subject: [PATCH 42/62] clippy --- crates/ibc/src/core/ics02_client/client_state.rs | 4 ++-- crates/ibc/src/core/ics02_client/handler/update_client.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 0c364167c..9680a28b4 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -96,8 +96,8 @@ pub trait ClientState: /// client, such as the ClientState and corresponding ConsensusState. Upon /// successful update, a list of consensus heights is returned. It assumes /// the client_message has already been verified. - /// - /// Post-condition: on success, the return value MUST contain at least one + /// + /// Post-condition: on success, the return value MUST contain at least one /// height. fn update_state( &self, 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 669ac9f9c..5c46c3568 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -265,7 +265,7 @@ mod tests { let ctx_b_chain_id = ChainId::new("mockgaiaB".to_string(), 1); let start_height = Height::new(1, 11).unwrap(); - let mut ctx_a = MockContext::new(ctx_a_chain_id.clone(), HostType::Mock, 5, start_height) + let mut ctx_a = MockContext::new(ctx_a_chain_id, HostType::Mock, 5, start_height) .with_client_parametrized_with_chain_id( ctx_b_chain_id.clone(), &client_id, From e5551dd085087fb2b713a30e1d24b1d85fd70516 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 14:01:41 -0400 Subject: [PATCH 43/62] remove old methods --- .../clients/ics07_tendermint/client_state.rs | 293 ------------------ .../ibc/src/core/ics02_client/client_state.rs | 16 +- crates/ibc/src/mock/client_state.rs | 51 +-- 3 files changed, 2 insertions(+), 358 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 2e5bf60ff..9311adb22 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -42,8 +42,6 @@ use crate::Height; use super::client_type as tm_client_type; -use crate::core::context::ContextError; - use crate::core::{ExecutionContext, ValidationContext}; pub const TENDERMINT_CLIENT_STATE_TYPE_URL: &str = "/ibc.lightclients.tendermint.v1.ClientState"; @@ -392,63 +390,6 @@ impl ClientState { Ok(()) } - - fn check_header_and_validator_set( - &self, - header: &Header, - consensus_state: &TmConsensusState, - current_timestamp: Timestamp, - ) -> Result<(), ClientError> { - check_header_trusted_next_validator_set(header, consensus_state)?; - - let duration_since_consensus_state = current_timestamp - .duration_since(&consensus_state.timestamp()) - .ok_or_else(|| ClientError::InvalidConsensusStateTimestamp { - time1: consensus_state.timestamp(), - time2: current_timestamp, - })?; - - if duration_since_consensus_state >= self.trusting_period { - return Err(Error::ConsensusStateTimestampGteTrustingPeriod { - duration_since_consensus_state, - trusting_period: self.trusting_period, - } - .into()); - } - - let untrusted_state = header.as_untrusted_block_state(); - let chain_id = self.chain_id.clone().into(); - let trusted_state = header.as_trusted_block_state(consensus_state, &chain_id)?; - let options = self.as_light_client_options()?; - - self.verifier - .validate_against_trusted( - &untrusted_state, - &trusted_state, - &options, - current_timestamp.into_tm_time().unwrap(), - ) - .into_result()?; - - Ok(()) - } - - fn verify_header_commit_against_trusted( - &self, - header: &Header, - consensus_state: &TmConsensusState, - ) -> Result<(), ClientError> { - let untrusted_state = header.as_untrusted_block_state(); - let chain_id = self.chain_id.clone().into(); - let trusted_state = Header::as_trusted_block_state(header, consensus_state, &chain_id)?; - let options = self.as_light_client_options()?; - - self.verifier - .verify_commit_against_trusted(&untrusted_state, &trusted_state, &options) - .into_result()?; - - Ok(()) - } } impl Ics2ClientState for ClientState { @@ -682,240 +623,6 @@ impl Ics2ClientState for ClientState { Ok(()) } - fn check_misbehaviour_and_update_state( - &self, - ctx: &dyn ValidationContext, - client_id: ClientId, - misbehaviour: Any, - ) -> Result, ContextError> { - let misbehaviour = TmMisbehaviour::try_from(misbehaviour)?; - let header_1 = misbehaviour.header1(); - let header_2 = misbehaviour.header2(); - - if header_1.height() == header_2.height() { - // Fork - if header_1.signed_header.commit.block_id.hash - == header_2.signed_header.commit.block_id.hash - { - return Err(ContextError::ClientError( - Error::MisbehaviourHeadersBlockHashesEqual.into(), - )); - } - } else { - // BFT time violation - if header_1.signed_header.header.time > header_2.signed_header.header.time { - return Err(ContextError::ClientError( - Error::MisbehaviourHeadersNotAtSameHeight.into(), - )); - } - } - let client_cons_state_path_1 = - ClientConsensusStatePath::new(&client_id, &header_1.trusted_height); - let consensus_state_1 = { - let cs = ctx.consensus_state(&client_cons_state_path_1)?; - downcast_tm_consensus_state(cs.as_ref()) - }?; - - let client_cons_state_path_2 = - ClientConsensusStatePath::new(&client_id, &header_2.trusted_height); - let consensus_state_2 = { - let cs = ctx.consensus_state(&client_cons_state_path_2)?; - downcast_tm_consensus_state(cs.as_ref()) - }?; - - let chain_id = self - .chain_id - .clone() - .with_version(header_1.height().revision_number()); - if !misbehaviour.chain_id_matches(&chain_id) { - return Err(ContextError::ClientError( - Error::MisbehaviourHeadersChainIdMismatch { - header_chain_id: header_1.signed_header.header.chain_id.to_string(), - chain_id: self.chain_id.to_string(), - } - .into(), - )); - } - - let current_timestamp = ctx.host_timestamp()?; - - self.check_header_and_validator_set(header_1, &consensus_state_1, current_timestamp)?; - self.check_header_and_validator_set(header_2, &consensus_state_2, current_timestamp)?; - - self.verify_header_commit_against_trusted(header_1, &consensus_state_1)?; - self.verify_header_commit_against_trusted(header_2, &consensus_state_2)?; - - let client_state = downcast_tm_client_state(self)?.clone(); - Ok(client_state - .with_frozen_height(Height::new(0, 1).unwrap()) - .into_box()) - } - - fn check_header_and_update_state( - &self, - ctx: &dyn ValidationContext, - client_id: ClientId, - header: Any, - ) -> Result { - fn maybe_consensus_state( - ctx: &dyn ValidationContext, - client_cons_state_path: &ClientConsensusStatePath, - ) -> Result>, ClientError> { - match ctx.consensus_state(client_cons_state_path) { - Ok(cs) => Ok(Some(cs)), - Err(e) => match e { - ContextError::ClientError(ClientError::ConsensusStateNotFound { - client_id: _, - height: _, - }) => Ok(None), - ContextError::ClientError(e) => Err(e), - _ => Err(ClientError::Other { - description: e.to_string(), - }), - }, - } - } - - let client_state = downcast_tm_client_state(self)?.clone(); - let header = TmHeader::try_from(header)?; - - if header.height().revision_number() != client_state.chain_id().version() { - return Err(ClientError::ClientSpecific { - description: Error::MismatchedRevisions { - current_revision: client_state.chain_id().version(), - update_revision: header.height().revision_number(), - } - .to_string(), - }); - } - - // Check if a consensus state is already installed; if so it should - // match the untrusted header. - let header_consensus_state = TmConsensusState::from(header.clone()); - let client_cons_state_path = ClientConsensusStatePath::new(&client_id, &header.height()); - let existing_consensus_state = match maybe_consensus_state(ctx, &client_cons_state_path)? { - Some(cs) => { - let cs = downcast_tm_consensus_state(cs.as_ref())?; - // If this consensus state matches, skip verification - // (optimization) - if cs == header_consensus_state { - // Header is already installed and matches the incoming - // header (already verified) - return Ok(UpdatedState { - client_state: client_state.into_box(), - consensus_state: cs.into_box(), - }); - } - Some(cs) - } - None => None, - }; - - let trusted_client_cons_state_path = - ClientConsensusStatePath::new(&client_id, &header.trusted_height); - let trusted_consensus_state = downcast_tm_consensus_state( - ctx.consensus_state(&trusted_client_cons_state_path)? - .as_ref(), - )?; - - let trusted_state = TrustedBlockState { - chain_id: &self.chain_id.clone().into(), - header_time: trusted_consensus_state.timestamp, - height: header - .trusted_height - .revision_height() - .try_into() - .map_err(|_| ClientError::ClientSpecific { - description: Error::InvalidHeaderHeight { - height: header.trusted_height.revision_height(), - } - .to_string(), - })?, - next_validators: &header.trusted_next_validator_set, - next_validators_hash: trusted_consensus_state.next_validators_hash, - }; - - let untrusted_state = UntrustedBlockState { - signed_header: &header.signed_header, - validators: &header.validator_set, - // NB: This will skip the - // VerificationPredicates::next_validators_match check for the - // untrusted state. - next_validators: None, - }; - - let options = client_state.as_light_client_options()?; - let now = ctx.host_timestamp()?.into_tm_time().unwrap(); - - self.verifier - .verify(untrusted_state, trusted_state, &options, now) - .into_result()?; - - // If the header has verified, but its corresponding consensus state - // differs from the existing consensus state for that height, freeze the - // client and return the installed consensus state. - if let Some(cs) = existing_consensus_state { - if cs != header_consensus_state { - return Ok(UpdatedState { - client_state: client_state.with_frozen_height(header.height()).into_box(), - consensus_state: cs.into_box(), - }); - } - } - - // Monotonicity checks for timestamps for in-the-middle updates - // (cs-new, cs-next, cs-latest) - if header.height() < client_state.latest_height() { - let maybe_next_cs = ctx - .next_consensus_state(&client_id, &header.height())? - .as_ref() - .map(|cs| downcast_tm_consensus_state(cs.as_ref())) - .transpose()?; - - if let Some(next_cs) = maybe_next_cs { - // New (untrusted) header timestamp cannot occur after next - // consensus state's height - if header.signed_header.header().time > next_cs.timestamp { - return Err(ClientError::ClientSpecific { - description: Error::HeaderTimestampTooHigh { - actual: header.signed_header.header().time.to_string(), - max: next_cs.timestamp.to_string(), - } - .to_string(), - }); - } - } - } - - // (cs-trusted, cs-prev, cs-new) - if header.trusted_height < header.height() { - let maybe_prev_cs = ctx - .prev_consensus_state(&client_id, &header.height())? - .as_ref() - .map(|cs| downcast_tm_consensus_state(cs.as_ref())) - .transpose()?; - - if let Some(prev_cs) = maybe_prev_cs { - // New (untrusted) header timestamp cannot occur before the - // previous consensus state's height - if header.signed_header.header().time < prev_cs.timestamp { - return Err(ClientError::ClientSpecific { - description: Error::HeaderTimestampTooLow { - actual: header.signed_header.header().time.to_string(), - min: prev_cs.timestamp.to_string(), - } - .to_string(), - }); - } - } - } - - Ok(UpdatedState { - client_state: client_state.with_header(header.clone())?.into_box(), - consensus_state: TmConsensusState::from(header).into_box(), - }) - } - /// Perform client-specific verifications and check all data in the new /// client state to be the same across all valid Tendermint clients for the /// new chain. diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 9680a28b4..3194589df 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -21,7 +21,7 @@ use crate::Height; use super::consensus_state::ConsensusState; use super::msgs::update_client::UpdateClientKind; -use crate::core::{ContextError, ExecutionContext, ValidationContext}; +use crate::core::{ExecutionContext, ValidationContext}; pub trait ClientState: AsAny @@ -117,20 +117,6 @@ pub trait ClientState: update_kind: &UpdateClientKind, ) -> Result<(), ClientError>; - fn check_header_and_update_state( - &self, - ctx: &dyn ValidationContext, - client_id: ClientId, - header: Any, - ) -> Result; - - fn check_misbehaviour_and_update_state( - &self, - ctx: &dyn ValidationContext, - client_id: ClientId, - misbehaviour: Any, - ) -> Result, ContextError>; - /// Verify the upgraded client and consensus states and validate proofs /// against the given root. /// diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index b4b2d62d5..22f764f9f 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -26,7 +26,7 @@ use crate::mock::misbehaviour::Misbehaviour; use crate::Height; -use crate::core::{ContextError, ExecutionContext, ValidationContext}; +use crate::core::{ExecutionContext, ValidationContext}; pub const MOCK_CLIENT_STATE_TYPE_URL: &str = "/ibc.mock.ClientState"; @@ -186,55 +186,6 @@ impl ClientState for MockClientState { MockConsensusState::try_from(consensus_state).map(MockConsensusState::into_box) } - fn check_header_and_update_state( - &self, - _ctx: &dyn ValidationContext, - _client_id: ClientId, - header: Any, - ) -> Result { - let header = MockHeader::try_from(header)?; - - if self.latest_height() >= header.height() { - return Err(ClientError::LowHeaderHeight { - header_height: header.height(), - latest_height: self.latest_height(), - }); - } - - Ok(UpdatedState { - client_state: MockClientState::new(header).into_box(), - consensus_state: MockConsensusState::new(header).into_box(), - }) - } - - fn check_misbehaviour_and_update_state( - &self, - _ctx: &dyn ValidationContext, - _client_id: ClientId, - misbehaviour: Any, - ) -> Result, ContextError> { - let misbehaviour = Misbehaviour::try_from(misbehaviour)?; - let header_1 = misbehaviour.header1; - let header_2 = misbehaviour.header2; - - if header_1.height() != header_2.height() { - return Err(ClientError::InvalidHeight.into()); - } - - if self.latest_height() >= header_1.height() { - return Err(ClientError::LowHeaderHeight { - header_height: header_1.height(), - latest_height: self.latest_height(), - } - .into()); - } - - let new_state = - MockClientState::new(header_1).with_frozen_height(Height::new(0, 1).unwrap()); - - Ok(new_state.into_box()) - } - fn verify_upgrade_client( &self, upgraded_client_state: Any, From c70b29cef063a81c082f5d8e80182dcadbd38bbe Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 14:34:11 -0400 Subject: [PATCH 44/62] clean Misbehaviour::new() --- .../clients/ics07_tendermint/client_state.rs | 29 +++++++++++++++- .../clients/ics07_tendermint/misbehaviour.rs | 34 +------------------ 2 files changed, 29 insertions(+), 34 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 9311adb22..550e33d73 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -319,12 +319,39 @@ impl ClientState { downcast_tm_consensus_state(consensus_state.as_ref()) }?; - let current_timestamp = ctx.host_timestamp()?; + self.check_misbehaviour_headers_consistency(header_1, header_2)?; + let current_timestamp = ctx.host_timestamp()?; self.check_misbehaviour_header(header_1, &trusted_consensus_state_1, current_timestamp)?; self.check_misbehaviour_header(header_2, &trusted_consensus_state_2, current_timestamp) } + fn check_misbehaviour_headers_consistency( + &self, + header_1: &TmHeader, + header_2: &TmHeader, + ) -> Result<(), ClientError> { + if header_1.signed_header.header.chain_id != header_2.signed_header.header.chain_id { + return Err(Error::InvalidRawMisbehaviour { + reason: "headers must have identical chain_ids".to_owned(), + } + .into()); + } + + if header_1.height() < header_2.height() { + return Err(Error::InvalidRawMisbehaviour { + reason: format!( + "headers1 height is less than header2 height ({} < {})", + header_1.height(), + header_2.height() + ), + } + .into()); + } + + Ok(()) + } + fn check_misbehaviour_header( &self, header: &TmHeader, diff --git a/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs index f980dc2f0..b54dc7493 100644 --- a/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs +++ b/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs @@ -5,9 +5,8 @@ use ibc_proto::google::protobuf::Any; use ibc_proto::ibc::lightclients::tendermint::v1::Misbehaviour as RawMisbehaviour; use ibc_proto::protobuf::Protobuf; use prost::Message; -use tendermint_light_client_verifier::ProdVerifier; -use crate::clients::ics07_tendermint::error::{Error, IntoResult}; +use crate::clients::ics07_tendermint::error::Error; use crate::clients::ics07_tendermint::header::Header; use crate::core::ics02_client::error::ClientError; use crate::core::ics24_host::identifier::{ChainId, ClientId}; @@ -25,37 +24,6 @@ pub struct Misbehaviour { impl Misbehaviour { pub fn new(client_id: ClientId, header1: Header, header2: Header) -> Result { - if header1.signed_header.header.chain_id != header2.signed_header.header.chain_id { - return Err(Error::InvalidRawMisbehaviour { - reason: "headers must have identical chain_ids".to_owned(), - }); - } - - if header1.height() < header2.height() { - return Err(Error::InvalidRawMisbehaviour { - reason: format!( - "headers1 height is less than header2 height ({} < {})", - header1.height(), - header2.height() - ), - }); - } - - let untrusted_state_1 = header1.as_untrusted_block_state(); - let untrusted_state_2 = header2.as_untrusted_block_state(); - - let verifier = ProdVerifier::default(); - - verifier - .verify_validator_sets(&untrusted_state_1) - .into_result()?; - verifier - .verify_validator_sets(&untrusted_state_2) - .into_result()?; - - verifier.verify_commit(&untrusted_state_1).into_result()?; - verifier.verify_commit(&untrusted_state_2).into_result()?; - Ok(Self { client_id, header1, From ef1ba51f7288428c2e81572ca6f9faa3da9286e9 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 14:35:53 -0400 Subject: [PATCH 45/62] cleanup unused --- .../src/clients/ics07_tendermint/header.rs | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/header.rs b/crates/ibc/src/clients/ics07_tendermint/header.rs index 5290583db..c87d4dc95 100644 --- a/crates/ibc/src/clients/ics07_tendermint/header.rs +++ b/crates/ibc/src/clients/ics07_tendermint/header.rs @@ -1,5 +1,4 @@ use alloc::string::ToString; -use core::cmp::Ordering; use core::fmt::{Display, Error as FmtError, Formatter}; use bytes::Buf; @@ -53,10 +52,6 @@ impl Header { .expect("malformed tendermint header domain type has an illegal height of 0") } - pub fn compatible_with(&self, other_header: &Header) -> bool { - headers_compatible(&self.signed_header, &other_header.signed_header) - } - pub(crate) fn as_untrusted_block_state(&self) -> UntrustedBlockState<'_> { UntrustedBlockState { signed_header: &self.signed_header, @@ -86,26 +81,6 @@ impl Header { } } -pub fn headers_compatible(header: &SignedHeader, other: &SignedHeader) -> bool { - let ibc_client_height = other.header.height; - let self_header_height = header.header.height; - - match self_header_height.cmp(&ibc_client_height) { - Ordering::Equal => { - // 1 - fork - header.commit.block_id == other.commit.block_id - } - Ordering::Greater => { - // 2 - BFT time violation - header.header.time > other.header.time - } - Ordering::Less => { - // 3 - BFT time violation - header.header.time < other.header.time - } - } -} - impl crate::core::ics02_client::header::Header for Header { fn height(&self) -> Height { self.height() From b5d6b2f8d214a044d077cf6599013307c814f3a2 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 14:47:55 -0400 Subject: [PATCH 46/62] move `update_client` method to submodule --- .../clients/ics07_tendermint/client_state.rs | 88 +---------------- .../client_state/update_client.rs | 98 +++++++++++++++++++ 2 files changed, 101 insertions(+), 85 deletions(-) create mode 100644 crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 550e33d73..bdcc7c65b 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -1,3 +1,5 @@ +mod update_client; + use crate::core::ics02_client::msgs::update_client::UpdateClientKind; use crate::prelude::*; @@ -16,8 +18,7 @@ use prost::Message; use tendermint::chain::id::MAX_LENGTH as MaxChainIdLen; use tendermint::trust_threshold::TrustThresholdFraction as TendermintTrustThresholdFraction; use tendermint_light_client_verifier::options::Options; -use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState}; -use tendermint_light_client_verifier::{ProdVerifier, Verifier}; +use tendermint_light_client_verifier::ProdVerifier; use crate::clients::ics07_tendermint::client_state::ClientState as TmClientState; use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; @@ -210,89 +211,6 @@ impl ClientState { }) } - fn verify_header( - &self, - ctx: &dyn ValidationContext, - client_id: &ClientId, - header: TmHeader, - ) -> Result<(), ClientError> { - // The tendermint-light-client crate though works on heights that are assumed - // to have the same revision number. We ensure this here. - if header.height().revision_number() != self.chain_id().version() { - return Err(ClientError::ClientSpecific { - description: Error::MismatchedRevisions { - current_revision: self.chain_id().version(), - update_revision: header.height().revision_number(), - } - .to_string(), - }); - } - - // We also need to ensure that the trusted height (representing the - // height of the header already on chain for which this client update is - // based on) must be smaller than height of the new header that we're - // installing. - if header.height() <= header.trusted_height { - return Err(ClientError::HeaderVerificationFailure { - reason: format!( - "header height <= header trusted height ({} <= {})", - header.height(), - header.trusted_height - ), - }); - } - - // Delegate to tendermint-light-client, which contains the required checks - // of the new header against the trusted consensus state. - { - let trusted_state = - { - let trusted_client_cons_state_path = - ClientConsensusStatePath::new(client_id, &header.trusted_height); - let trusted_consensus_state = downcast_tm_consensus_state( - ctx.consensus_state(&trusted_client_cons_state_path)? - .as_ref(), - )?; - - check_header_trusted_next_validator_set(&header, &trusted_consensus_state)?; - - TrustedBlockState { - chain_id: &self.chain_id.clone().into(), - header_time: trusted_consensus_state.timestamp, - height: header.trusted_height.revision_height().try_into().map_err( - |_| ClientError::ClientSpecific { - description: Error::InvalidHeaderHeight { - height: header.trusted_height.revision_height(), - } - .to_string(), - }, - )?, - next_validators: &header.trusted_next_validator_set, - next_validators_hash: trusted_consensus_state.next_validators_hash, - } - }; - - let untrusted_state = UntrustedBlockState { - signed_header: &header.signed_header, - validators: &header.validator_set, - // NB: This will skip the - // VerificationPredicates::next_validators_match check for the - // untrusted state. - next_validators: None, - }; - - let options = self.as_light_client_options()?; - let now = ctx.host_timestamp()?.into_tm_time().unwrap(); - - // main header verification, delegated to the tendermint-light-client crate. - self.verifier - .verify(untrusted_state, trusted_state, &options, now) - .into_result()?; - } - - Ok(()) - } - // verify_misbehaviour determines whether or not two conflicting headers at // the same height would have convinced the light client. fn verify_misbehaviour( diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs new file mode 100644 index 000000000..ba6d8657e --- /dev/null +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs @@ -0,0 +1,98 @@ +use tendermint_light_client_verifier::Verifier; +use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState}; + +use crate::prelude::*; + +use crate::clients::ics07_tendermint::error::{Error, IntoResult}; +use crate::clients::ics07_tendermint::header::Header as TmHeader; +use crate::core::ics02_client::error::ClientError; +use crate::core::ics02_client::client_state::ClientState as Ics2ClientState; +use crate::core::ics24_host::path::ClientConsensusStatePath; +use crate::core::{ics24_host::identifier::ClientId, ValidationContext}; + +use super::{ClientState, downcast_tm_consensus_state, check_header_trusted_next_validator_set}; + +impl ClientState { + pub fn verify_header( + &self, + ctx: &dyn ValidationContext, + client_id: &ClientId, + header: TmHeader, + ) -> Result<(), ClientError> { + // The tendermint-light-client crate though works on heights that are assumed + // to have the same revision number. We ensure this here. + if header.height().revision_number() != self.chain_id().version() { + return Err(ClientError::ClientSpecific { + description: Error::MismatchedRevisions { + current_revision: self.chain_id().version(), + update_revision: header.height().revision_number(), + } + .to_string(), + }); + } + + // We also need to ensure that the trusted height (representing the + // height of the header already on chain for which this client update is + // based on) must be smaller than height of the new header that we're + // installing. + if header.height() <= header.trusted_height { + return Err(ClientError::HeaderVerificationFailure { + reason: format!( + "header height <= header trusted height ({} <= {})", + header.height(), + header.trusted_height + ), + }); + } + + // Delegate to tendermint-light-client, which contains the required checks + // of the new header against the trusted consensus state. + { + let trusted_state = + { + let trusted_client_cons_state_path = + ClientConsensusStatePath::new(client_id, &header.trusted_height); + let trusted_consensus_state = downcast_tm_consensus_state( + ctx.consensus_state(&trusted_client_cons_state_path)? + .as_ref(), + )?; + + check_header_trusted_next_validator_set(&header, &trusted_consensus_state)?; + + TrustedBlockState { + chain_id: &self.chain_id.clone().into(), + header_time: trusted_consensus_state.timestamp, + height: header.trusted_height.revision_height().try_into().map_err( + |_| ClientError::ClientSpecific { + description: Error::InvalidHeaderHeight { + height: header.trusted_height.revision_height(), + } + .to_string(), + }, + )?, + next_validators: &header.trusted_next_validator_set, + next_validators_hash: trusted_consensus_state.next_validators_hash, + } + }; + + let untrusted_state = UntrustedBlockState { + signed_header: &header.signed_header, + validators: &header.validator_set, + // NB: This will skip the + // VerificationPredicates::next_validators_match check for the + // untrusted state. + next_validators: None, + }; + + let options = self.as_light_client_options()?; + let now = ctx.host_timestamp()?.into_tm_time().unwrap(); + + // main header verification, delegated to the tendermint-light-client crate. + self.verifier + .verify(untrusted_state, trusted_state, &options, now) + .into_result()?; + } + + Ok(()) + } +} From 96058301c9e1a8eb8ecb9adf30de810ab4efea1f Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 14:55:04 -0400 Subject: [PATCH 47/62] move misbehaviour to submodule --- .../clients/ics07_tendermint/client_state.rs | 132 +---------------- .../client_state/misbehaviour.rs | 140 ++++++++++++++++++ .../client_state/update_client.rs | 10 +- 3 files changed, 149 insertions(+), 133 deletions(-) create mode 100644 crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index bdcc7c65b..78590ff7d 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -1,3 +1,4 @@ +mod misbehaviour; mod update_client; use crate::core::ics02_client::msgs::update_client::UpdateClientKind; @@ -22,8 +23,8 @@ use tendermint_light_client_verifier::ProdVerifier; use crate::clients::ics07_tendermint::client_state::ClientState as TmClientState; use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; -use crate::clients::ics07_tendermint::error::{Error, IntoResult}; -use crate::clients::ics07_tendermint::header::{Header as TmHeader, Header}; +use crate::clients::ics07_tendermint::error::Error; +use crate::clients::ics07_tendermint::header::Header as TmHeader; use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; use crate::core::ics02_client::client_state::{ClientState as Ics2ClientState, UpdatedState}; use crate::core::ics02_client::client_type::ClientType; @@ -38,7 +39,7 @@ use crate::core::ics23_commitment::specs::ProofSpecs; use crate::core::ics24_host::identifier::{ChainId, ClientId}; use crate::core::ics24_host::path::{ClientConsensusStatePath, ClientStatePath, ClientUpgradePath}; use crate::core::ics24_host::Path; -use crate::timestamp::{Timestamp, ZERO_DURATION}; +use crate::timestamp::ZERO_DURATION; use crate::Height; use super::client_type as tm_client_type; @@ -210,131 +211,6 @@ impl ClientState { clock_drift: self.max_clock_drift, }) } - - // verify_misbehaviour determines whether or not two conflicting headers at - // the same height would have convinced the light client. - fn verify_misbehaviour( - &self, - ctx: &dyn ValidationContext, - client_id: &ClientId, - misbehaviour: TmMisbehaviour, - ) -> Result<(), ClientError> { - let header_1 = misbehaviour.header1(); - let trusted_consensus_state_1 = { - let consensus_state_path = - ClientConsensusStatePath::new(client_id, &header_1.trusted_height); - let consensus_state = ctx.consensus_state(&consensus_state_path)?; - - downcast_tm_consensus_state(consensus_state.as_ref()) - }?; - - let header_2 = misbehaviour.header2(); - let trusted_consensus_state_2 = { - let consensus_state_path = - ClientConsensusStatePath::new(client_id, &header_2.trusted_height); - let consensus_state = ctx.consensus_state(&consensus_state_path)?; - - downcast_tm_consensus_state(consensus_state.as_ref()) - }?; - - self.check_misbehaviour_headers_consistency(header_1, header_2)?; - - let current_timestamp = ctx.host_timestamp()?; - self.check_misbehaviour_header(header_1, &trusted_consensus_state_1, current_timestamp)?; - self.check_misbehaviour_header(header_2, &trusted_consensus_state_2, current_timestamp) - } - - fn check_misbehaviour_headers_consistency( - &self, - header_1: &TmHeader, - header_2: &TmHeader, - ) -> Result<(), ClientError> { - if header_1.signed_header.header.chain_id != header_2.signed_header.header.chain_id { - return Err(Error::InvalidRawMisbehaviour { - reason: "headers must have identical chain_ids".to_owned(), - } - .into()); - } - - if header_1.height() < header_2.height() { - return Err(Error::InvalidRawMisbehaviour { - reason: format!( - "headers1 height is less than header2 height ({} < {})", - header_1.height(), - header_2.height() - ), - } - .into()); - } - - Ok(()) - } - - fn check_misbehaviour_header( - &self, - header: &TmHeader, - trusted_consensus_state: &TmConsensusState, - current_timestamp: Timestamp, - ) -> Result<(), ClientError> { - // ensure correctness of the trusted next validator set provided by the relayer - check_header_trusted_next_validator_set(header, trusted_consensus_state)?; - - // ensure header timestamp is within trusted period from the trusted consensus state - { - let duration_since_consensus_state = current_timestamp - .duration_since(&trusted_consensus_state.timestamp()) - .ok_or_else(|| ClientError::InvalidConsensusStateTimestamp { - time1: trusted_consensus_state.timestamp(), - time2: current_timestamp, - })?; - - if duration_since_consensus_state >= self.trusting_period { - return Err(Error::ConsensusStateTimestampGteTrustingPeriod { - duration_since_consensus_state, - trusting_period: self.trusting_period, - } - .into()); - } - } - - // ensure that 2/3 of trusted validators have signed the new header - { - let untrusted_state = header.as_untrusted_block_state(); - let chain_id = self - .chain_id - .clone() - .with_version(header.height().revision_number()) - .into(); - let trusted_state = - Header::as_trusted_block_state(header, trusted_consensus_state, &chain_id)?; - let options = self.as_light_client_options()?; - - self.verifier - .verify_commit_against_trusted(&untrusted_state, &trusted_state, &options) - .into_result()?; - } - - // run the verification checks on the header based on the trusted - // consensus state - { - let untrusted_state = header.as_untrusted_block_state(); - let chain_id = self.chain_id.clone().into(); - let trusted_state = - header.as_trusted_block_state(trusted_consensus_state, &chain_id)?; - let options = self.as_light_client_options()?; - - self.verifier - .validate_against_trusted( - &untrusted_state, - &trusted_state, - &options, - current_timestamp.into_tm_time().unwrap(), - ) - .into_result()?; - } - - Ok(()) - } } impl Ics2ClientState for ClientState { diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs new file mode 100644 index 000000000..4177a9d3b --- /dev/null +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs @@ -0,0 +1,140 @@ +use crate::core::ics02_client::consensus_state::ConsensusState; +use crate::prelude::*; + +use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; +use crate::clients::ics07_tendermint::error::{Error, IntoResult}; +use crate::clients::ics07_tendermint::header::Header as TmHeader; +use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; +use crate::core::ics02_client::error::ClientError; +use crate::core::ics24_host::path::ClientConsensusStatePath; +use crate::core::{ics24_host::identifier::ClientId, ValidationContext}; +use crate::timestamp::Timestamp; + +use super::{check_header_trusted_next_validator_set, downcast_tm_consensus_state, ClientState}; + +impl ClientState { + // verify_misbehaviour determines whether or not two conflicting headers at + // the same height would have convinced the light client. + pub fn verify_misbehaviour( + &self, + ctx: &dyn ValidationContext, + client_id: &ClientId, + misbehaviour: TmMisbehaviour, + ) -> Result<(), ClientError> { + let header_1 = misbehaviour.header1(); + let trusted_consensus_state_1 = { + let consensus_state_path = + ClientConsensusStatePath::new(client_id, &header_1.trusted_height); + let consensus_state = ctx.consensus_state(&consensus_state_path)?; + + downcast_tm_consensus_state(consensus_state.as_ref()) + }?; + + let header_2 = misbehaviour.header2(); + let trusted_consensus_state_2 = { + let consensus_state_path = + ClientConsensusStatePath::new(client_id, &header_2.trusted_height); + let consensus_state = ctx.consensus_state(&consensus_state_path)?; + + downcast_tm_consensus_state(consensus_state.as_ref()) + }?; + + self.check_misbehaviour_headers_consistency(header_1, header_2)?; + + let current_timestamp = ctx.host_timestamp()?; + self.check_misbehaviour_header(header_1, &trusted_consensus_state_1, current_timestamp)?; + self.check_misbehaviour_header(header_2, &trusted_consensus_state_2, current_timestamp) + } + + pub fn check_misbehaviour_headers_consistency( + &self, + header_1: &TmHeader, + header_2: &TmHeader, + ) -> Result<(), ClientError> { + if header_1.signed_header.header.chain_id != header_2.signed_header.header.chain_id { + return Err(Error::InvalidRawMisbehaviour { + reason: "headers must have identical chain_ids".to_owned(), + } + .into()); + } + + if header_1.height() < header_2.height() { + return Err(Error::InvalidRawMisbehaviour { + reason: format!( + "headers1 height is less than header2 height ({} < {})", + header_1.height(), + header_2.height() + ), + } + .into()); + } + + Ok(()) + } + + pub fn check_misbehaviour_header( + &self, + header: &TmHeader, + trusted_consensus_state: &TmConsensusState, + current_timestamp: Timestamp, + ) -> Result<(), ClientError> { + // ensure correctness of the trusted next validator set provided by the relayer + check_header_trusted_next_validator_set(header, trusted_consensus_state)?; + + // ensure header timestamp is within trusted period from the trusted consensus state + { + let duration_since_consensus_state = current_timestamp + .duration_since(&trusted_consensus_state.timestamp()) + .ok_or_else(|| ClientError::InvalidConsensusStateTimestamp { + time1: trusted_consensus_state.timestamp(), + time2: current_timestamp, + })?; + + if duration_since_consensus_state >= self.trusting_period { + return Err(Error::ConsensusStateTimestampGteTrustingPeriod { + duration_since_consensus_state, + trusting_period: self.trusting_period, + } + .into()); + } + } + + // ensure that 2/3 of trusted validators have signed the new header + { + let untrusted_state = header.as_untrusted_block_state(); + let chain_id = self + .chain_id + .clone() + .with_version(header.height().revision_number()) + .into(); + let trusted_state = + TmHeader::as_trusted_block_state(header, trusted_consensus_state, &chain_id)?; + let options = self.as_light_client_options()?; + + self.verifier + .verify_commit_against_trusted(&untrusted_state, &trusted_state, &options) + .into_result()?; + } + + // run the verification checks on the header based on the trusted + // consensus state + { + let untrusted_state = header.as_untrusted_block_state(); + let chain_id = self.chain_id.clone().into(); + let trusted_state = + header.as_trusted_block_state(trusted_consensus_state, &chain_id)?; + let options = self.as_light_client_options()?; + + self.verifier + .validate_against_trusted( + &untrusted_state, + &trusted_state, + &options, + current_timestamp.into_tm_time().unwrap(), + ) + .into_result()?; + } + + Ok(()) + } +} diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs index ba6d8657e..1f9214bb7 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs @@ -1,16 +1,16 @@ -use tendermint_light_client_verifier::Verifier; -use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState}; - use crate::prelude::*; +use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState}; +use tendermint_light_client_verifier::Verifier; + use crate::clients::ics07_tendermint::error::{Error, IntoResult}; use crate::clients::ics07_tendermint::header::Header as TmHeader; -use crate::core::ics02_client::error::ClientError; use crate::core::ics02_client::client_state::ClientState as Ics2ClientState; +use crate::core::ics02_client::error::ClientError; use crate::core::ics24_host::path::ClientConsensusStatePath; use crate::core::{ics24_host::identifier::ClientId, ValidationContext}; -use super::{ClientState, downcast_tm_consensus_state, check_header_trusted_next_validator_set}; +use super::{check_header_trusted_next_validator_set, downcast_tm_consensus_state, ClientState}; impl ClientState { pub fn verify_header( From 2188355befcf3fd14badfe8a4abaa507b85fa2fd Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 14:58:41 -0400 Subject: [PATCH 48/62] move implementation to function --- .../clients/ics07_tendermint/client_state.rs | 60 +---------------- .../client_state/update_client.rs | 64 +++++++++++++++++++ 2 files changed, 65 insertions(+), 59 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 78590ff7d..df7c76c10 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -295,65 +295,7 @@ impl Ics2ClientState for ClientState { match update_kind { UpdateClientKind::UpdateHeader => { let header = TmHeader::try_from(client_message)?; - - let header_consensus_state = TmConsensusState::from(header.clone()); - - let maybe_existing_consensus_state = { - let path_at_header_height = - ClientConsensusStatePath::new(client_id, &header.height()); - - ctx.consensus_state(&path_at_header_height).ok() - }; - - match maybe_existing_consensus_state { - Some(existing_consensus_state) => { - let existing_consensus_state = - downcast_tm_consensus_state(existing_consensus_state.as_ref())?; - - // There is evidence of misbehaviour if the stored consensus state - // is different from the new one we received. - Ok(existing_consensus_state != header_consensus_state) - } - None => { - // If no header was previously installed, we ensure the monotonicity of timestamps. - - // 1. for all headers, the new header needs to have a larger timestamp than - // the “previous header” - { - let maybe_prev_cs = - ctx.prev_consensus_state(client_id, &header.height())?; - - if let Some(prev_cs) = maybe_prev_cs { - // New header timestamp cannot occur *before* the - // previous consensus state's height - let prev_cs = downcast_tm_consensus_state(prev_cs.as_ref())?; - - if header.signed_header.header().time < prev_cs.timestamp { - return Ok(true); - } - } - } - - // 2. if a header comes in and is not the “last” header, then we also ensure - // that its timestamp is less than the “next header” - if header.height() < self.latest_height() { - let maybe_next_cs = - ctx.next_consensus_state(client_id, &header.height())?; - - if let Some(next_cs) = maybe_next_cs { - // New (untrusted) header timestamp cannot occur *after* next - // consensus state's height - let next_cs = downcast_tm_consensus_state(next_cs.as_ref())?; - - if header.signed_header.header().time > next_cs.timestamp { - return Ok(true); - } - } - } - - Ok(false) - } - } + self.check_for_misbehaviour_update_client(ctx, client_id, header) } UpdateClientKind::Misbehaviour => { let misbehaviour = TmMisbehaviour::try_from(client_message)?; diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs index 1f9214bb7..bcb238932 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs @@ -5,6 +5,7 @@ use tendermint_light_client_verifier::Verifier; use crate::clients::ics07_tendermint::error::{Error, IntoResult}; use crate::clients::ics07_tendermint::header::Header as TmHeader; +use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::core::ics02_client::client_state::ClientState as Ics2ClientState; use crate::core::ics02_client::error::ClientError; use crate::core::ics24_host::path::ClientConsensusStatePath; @@ -95,4 +96,67 @@ impl ClientState { Ok(()) } + + pub fn check_for_misbehaviour_update_client( + &self, + ctx: &dyn ValidationContext, + client_id: &ClientId, + header: TmHeader, + ) -> Result { + let header_consensus_state = TmConsensusState::from(header.clone()); + + let maybe_existing_consensus_state = { + let path_at_header_height = ClientConsensusStatePath::new(client_id, &header.height()); + + ctx.consensus_state(&path_at_header_height).ok() + }; + + match maybe_existing_consensus_state { + Some(existing_consensus_state) => { + let existing_consensus_state = + downcast_tm_consensus_state(existing_consensus_state.as_ref())?; + + // There is evidence of misbehaviour if the stored consensus state + // is different from the new one we received. + Ok(existing_consensus_state != header_consensus_state) + } + None => { + // If no header was previously installed, we ensure the monotonicity of timestamps. + + // 1. for all headers, the new header needs to have a larger timestamp than + // the “previous header” + { + let maybe_prev_cs = ctx.prev_consensus_state(client_id, &header.height())?; + + if let Some(prev_cs) = maybe_prev_cs { + // New header timestamp cannot occur *before* the + // previous consensus state's height + let prev_cs = downcast_tm_consensus_state(prev_cs.as_ref())?; + + if header.signed_header.header().time < prev_cs.timestamp { + return Ok(true); + } + } + } + + // 2. if a header comes in and is not the “last” header, then we also ensure + // that its timestamp is less than the “next header” + if header.height() < self.latest_height() { + let maybe_next_cs = ctx.next_consensus_state(client_id, &header.height())?; + + if let Some(next_cs) = maybe_next_cs { + // New (untrusted) header timestamp cannot occur *after* next + // consensus state's height + let next_cs = downcast_tm_consensus_state(next_cs.as_ref())?; + + if header.signed_header.header().time > next_cs.timestamp { + return Ok(true); + } + } + } + + Ok(false) + } + } + } } From c291e01cbb956d90aeba234b49fcff8a9a6bdee8 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 15:01:35 -0400 Subject: [PATCH 49/62] move implementation to function --- .../clients/ics07_tendermint/client_state.rs | 18 +-------------- .../client_state/misbehaviour.rs | 23 +++++++++++++++++++ .../client_state/update_client.rs | 2 +- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index df7c76c10..6c4ea8176 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -299,23 +299,7 @@ impl Ics2ClientState for ClientState { } UpdateClientKind::Misbehaviour => { let misbehaviour = TmMisbehaviour::try_from(client_message)?; - let header_1 = misbehaviour.header1(); - let header_2 = misbehaviour.header2(); - - if header_1.height() == header_2.height() { - // when the height of the 2 headers are equal, we only have evidence - // of misbehaviour in the case where the headers are different - // (otherwise, the same header was added twice in the message, - // and this is evidence of nothing) - Ok(header_1.signed_header.commit.block_id.hash - != header_2.signed_header.commit.block_id.hash) - } else { - // header_1 is at greater height than header_2, therefore - // header_1 time must be less than or equal to - // header_2 time in order to be valid misbehaviour (violation of - // monotonic time). - Ok(header_1.signed_header.header.time <= header_2.signed_header.header.time) - } + self.check_for_misbehaviour_misbehavior(&misbehaviour) } } } diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs index 4177a9d3b..65ab761a3 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs @@ -137,4 +137,27 @@ impl ClientState { Ok(()) } + + pub fn check_for_misbehaviour_misbehavior( + &self, + misbehaviour: &TmMisbehaviour, + ) -> Result { + let header_1 = misbehaviour.header1(); + let header_2 = misbehaviour.header2(); + + if header_1.height() == header_2.height() { + // when the height of the 2 headers are equal, we only have evidence + // of misbehaviour in the case where the headers are different + // (otherwise, the same header was added twice in the message, + // and this is evidence of nothing) + Ok(header_1.signed_header.commit.block_id.hash + != header_2.signed_header.commit.block_id.hash) + } else { + // header_1 is at greater height than header_2, therefore + // header_1 time must be less than or equal to + // header_2 time in order to be valid misbehaviour (violation of + // monotonic time). + Ok(header_1.signed_header.header.time <= header_2.signed_header.header.time) + } + } } diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs index bcb238932..251595cb9 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs @@ -3,9 +3,9 @@ use crate::prelude::*; use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState}; use tendermint_light_client_verifier::Verifier; +use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::clients::ics07_tendermint::error::{Error, IntoResult}; use crate::clients::ics07_tendermint::header::Header as TmHeader; -use crate::clients::ics07_tendermint::consensus_state::ConsensusState as TmConsensusState; use crate::core::ics02_client::client_state::ClientState as Ics2ClientState; use crate::core::ics02_client::error::ClientError; use crate::core::ics24_host::path::ClientConsensusStatePath; From e37bf3e5e8b77282599d9cf7ba78e1f57ee3bb4b Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 5 Apr 2023 15:15:44 -0400 Subject: [PATCH 50/62] changelog --- .../unreleased/breaking-changes/535-check-and-update-state.md | 3 +++ .../unreleased/bug/583-missing-trusted-validator-set-check.md | 3 +++ .changelog/unreleased/bug/585-missing-trusted-height-check.md | 3 +++ .changelog/unreleased/bug/589-commit-verification-fix.md | 3 +++ .../bug/598-missing-timestamp-monotonicity-checks.md | 4 ++++ .changelog/unreleased/bug/601-client-state-latest-height.md | 3 +++ 6 files changed, 19 insertions(+) create mode 100644 .changelog/unreleased/breaking-changes/535-check-and-update-state.md create mode 100644 .changelog/unreleased/bug/583-missing-trusted-validator-set-check.md create mode 100644 .changelog/unreleased/bug/585-missing-trusted-height-check.md create mode 100644 .changelog/unreleased/bug/589-commit-verification-fix.md create mode 100644 .changelog/unreleased/bug/598-missing-timestamp-monotonicity-checks.md create mode 100644 .changelog/unreleased/bug/601-client-state-latest-height.md diff --git a/.changelog/unreleased/breaking-changes/535-check-and-update-state.md b/.changelog/unreleased/breaking-changes/535-check-and-update-state.md new file mode 100644 index 000000000..a0f8876c0 --- /dev/null +++ b/.changelog/unreleased/breaking-changes/535-check-and-update-state.md @@ -0,0 +1,3 @@ +- `ClientState`: Split `check_misbehaviour_and_update_state` + and `check_header_and_update_state` + ([#535](https://github.com/cosmos/ibc-rs/issues/535)) diff --git a/.changelog/unreleased/bug/583-missing-trusted-validator-set-check.md b/.changelog/unreleased/bug/583-missing-trusted-validator-set-check.md new file mode 100644 index 000000000..ec84f007c --- /dev/null +++ b/.changelog/unreleased/bug/583-missing-trusted-validator-set-check.md @@ -0,0 +1,3 @@ +- Tendermint light client: fix missing trusted_validator_set + hash check + ([#583](https://github.com/cosmos/ibc-rs/issues/583)) diff --git a/.changelog/unreleased/bug/585-missing-trusted-height-check.md b/.changelog/unreleased/bug/585-missing-trusted-height-check.md new file mode 100644 index 000000000..b20d65f1b --- /dev/null +++ b/.changelog/unreleased/bug/585-missing-trusted-height-check.md @@ -0,0 +1,3 @@ +- Tendermint light client: fix missing `Header.height()` + vs `Header.trusted_height` check + ([#585](https://github.com/cosmos/ibc-rs/issues/585)) diff --git a/.changelog/unreleased/bug/589-commit-verification-fix.md b/.changelog/unreleased/bug/589-commit-verification-fix.md new file mode 100644 index 000000000..c55fceb98 --- /dev/null +++ b/.changelog/unreleased/bug/589-commit-verification-fix.md @@ -0,0 +1,3 @@ +- Tendermint light client: ensure that we use the correct + chain ID in commit verification + ([#589](https://github.com/cosmos/ibc-rs/issues/589)) diff --git a/.changelog/unreleased/bug/598-missing-timestamp-monotonicity-checks.md b/.changelog/unreleased/bug/598-missing-timestamp-monotonicity-checks.md new file mode 100644 index 000000000..410329630 --- /dev/null +++ b/.changelog/unreleased/bug/598-missing-timestamp-monotonicity-checks.md @@ -0,0 +1,4 @@ +- Tendermint light client: add check that ensure that + the consensus state timestamps are monotonic, otherwise + freeze the client + ([#598](https://github.com/cosmos/ibc-rs/issues/598)) diff --git a/.changelog/unreleased/bug/601-client-state-latest-height.md b/.changelog/unreleased/bug/601-client-state-latest-height.md new file mode 100644 index 000000000..ccc0d380f --- /dev/null +++ b/.changelog/unreleased/bug/601-client-state-latest-height.md @@ -0,0 +1,3 @@ +- Tendermint light client: fix how the client's latest + height is updated + ([#601](https://github.com/cosmos/ibc-rs/issues/601)) From 478ed8ce12a4f2aba5f04facb49ec7dbfcfda1d3 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 10 Apr 2023 09:41:50 -0400 Subject: [PATCH 51/62] add clarifying comment --- crates/ibc/src/core/handler.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/ibc/src/core/handler.rs b/crates/ibc/src/core/handler.rs index bf48d7d9f..68807af3e 100644 --- a/crates/ibc/src/core/handler.rs +++ b/crates/ibc/src/core/handler.rs @@ -6,6 +6,13 @@ use super::{ }; /// Entrypoint which only performs message validation +/// +/// If a transaction contains `n` messages `m_1` ... `m_n`, then +/// they MUST be processed as follows: +/// validate(m_1), execute(m_1), ..., validate(m_n), execute(m_n) +/// That is, the state transition of message `i` must be applied before +/// message `i+1` is validated. This is equivalent to calling +/// `dispatch()` on each successively. pub fn validate(ctx: &Ctx, message: Any) -> Result<(), RouterError> where Ctx: ValidationContext, From b43507c95f64d850b537e4a13b934e081c4e0685 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 10 Apr 2023 09:45:23 -0400 Subject: [PATCH 52/62] fmt --- crates/ibc/src/core/handler.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/core/handler.rs b/crates/ibc/src/core/handler.rs index 68807af3e..46317232d 100644 --- a/crates/ibc/src/core/handler.rs +++ b/crates/ibc/src/core/handler.rs @@ -6,7 +6,7 @@ use super::{ }; /// Entrypoint which only performs message validation -/// +/// /// If a transaction contains `n` messages `m_1` ... `m_n`, then /// they MUST be processed as follows: /// validate(m_1), execute(m_1), ..., validate(m_n), execute(m_n) From e363ebdf5aeeaaa1df1870d866c9a7d7219a199b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Laferri=C3=A8re?= Date: Mon, 10 Apr 2023 10:02:24 -0400 Subject: [PATCH 53/62] Update crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Anca Zamfir Signed-off-by: Philippe Laferrière --- .../src/clients/ics07_tendermint/client_state/misbehaviour.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs index 65ab761a3..cef5f92c5 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs @@ -99,7 +99,7 @@ impl ClientState { } } - // ensure that 2/3 of trusted validators have signed the new header + // ensure the client trust_threshold overlap between the validator sets of the trusted and untrusted states. { let untrusted_state = header.as_untrusted_block_state(); let chain_id = self From b795337ac266513c1bbbf38df252cf09c9b03b0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Laferri=C3=A8re?= Date: Mon, 10 Apr 2023 12:07:48 -0400 Subject: [PATCH 54/62] Update crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Anca Zamfir Signed-off-by: Philippe Laferrière --- .../src/clients/ics07_tendermint/client_state/update_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs index 251595cb9..e94ec0d41 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs @@ -149,7 +149,7 @@ impl ClientState { // consensus state's height let next_cs = downcast_tm_consensus_state(next_cs.as_ref())?; - if header.signed_header.header().time > next_cs.timestamp { + if header.signed_header.header().time >= next_cs.timestamp { return Ok(true); } } From 0be640f5a477f748806e34d141954fce7be1a6e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Laferri=C3=A8re?= Date: Tue, 11 Apr 2023 08:52:22 -0400 Subject: [PATCH 55/62] Update crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Anca Zamfir Signed-off-by: Philippe Laferrière --- .../src/clients/ics07_tendermint/client_state/misbehaviour.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs index cef5f92c5..6231a42d1 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs @@ -81,7 +81,7 @@ impl ClientState { // ensure correctness of the trusted next validator set provided by the relayer check_header_trusted_next_validator_set(header, trusted_consensus_state)?; - // ensure header timestamp is within trusted period from the trusted consensus state + // ensure trusted consensus state is within trusting period { let duration_since_consensus_state = current_timestamp .duration_since(&trusted_consensus_state.timestamp()) From 1cffe09dad8b0f167252b089b33bea340a267ed6 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 12 Apr 2023 09:56:50 -0400 Subject: [PATCH 56/62] fix misbehaviour ctor --- crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs | 8 ++++---- crates/ibc/src/core/ics02_client/handler/update_client.rs | 5 +---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs index b54dc7493..19825777d 100644 --- a/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs +++ b/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs @@ -23,12 +23,12 @@ pub struct Misbehaviour { } impl Misbehaviour { - pub fn new(client_id: ClientId, header1: Header, header2: Header) -> Result { - Ok(Self { + pub fn new(client_id: ClientId, header1: Header, header2: Header) -> Self { + Self { client_id, header1, header2, - }) + } } pub fn client_id(&self) -> &ClientId { @@ -88,7 +88,7 @@ impl TryFrom for Misbehaviour { })? .try_into()?; - Self::new(client_id, header1, header2) + Ok(Self::new(client_id, header1, header2)) } } 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 5c46c3568..0135d5e6e 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -543,9 +543,7 @@ mod tests { let msg = MsgUpdateClient { client_id: client_id.clone(), - client_message: TmMisbehaviour::new(client_id.clone(), header1, header2) - .unwrap() - .into(), + client_message: TmMisbehaviour::new(client_id.clone(), header1, header2).into(), update_kind: UpdateClientKind::Misbehaviour, signer: get_dummy_account_id(), }; @@ -608,7 +606,6 @@ mod tests { let msg = MsgUpdateClient { client_id: client_id.clone(), client_message: TmMisbehaviour::new(client_id.clone(), header1.into(), header2.into()) - .unwrap() .into(), update_kind: UpdateClientKind::Misbehaviour, signer: get_dummy_account_id(), From ea2d588b84ef630858aea948920d3d758e1665d8 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 12 Apr 2023 17:20:24 -0400 Subject: [PATCH 57/62] improve documentation around `UpdateKind` --- .../clients/ics07_tendermint/client_state.rs | 18 +++++++------- crates/ibc/src/core/handler.rs | 10 ++++---- .../ibc/src/core/ics02_client/client_state.rs | 10 ++++---- .../ics02_client/handler/update_client.rs | 24 +++++++++---------- .../core/ics02_client/msgs/update_client.rs | 24 ++++++++++++------- crates/ibc/src/mock/client_state.rs | 18 +++++++------- crates/ibc/src/mock/ics18_relayer/context.rs | 4 ++-- 7 files changed, 57 insertions(+), 51 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 6c4ea8176..265f60dd2 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -1,7 +1,7 @@ mod misbehaviour; mod update_client; -use crate::core::ics02_client::msgs::update_client::UpdateClientKind; +use crate::core::ics02_client::msgs::update_client::UpdateKind; use crate::prelude::*; use core::cmp::max; @@ -268,14 +268,14 @@ impl Ics2ClientState for ClientState { ctx: &dyn ValidationContext, client_id: &ClientId, client_message: Any, - update_kind: &UpdateClientKind, + update_kind: &UpdateKind, ) -> Result<(), ClientError> { match update_kind { - UpdateClientKind::UpdateHeader => { + UpdateKind::UpdateClient => { let header = TmHeader::try_from(client_message)?; self.verify_header(ctx, client_id, header) } - UpdateClientKind::Misbehaviour => { + UpdateKind::SubmitMisbehaviour => { let misbehaviour = TmMisbehaviour::try_from(client_message)?; self.verify_misbehaviour(ctx, client_id, misbehaviour) } @@ -290,14 +290,14 @@ impl Ics2ClientState for ClientState { ctx: &dyn ValidationContext, client_id: &ClientId, client_message: Any, - update_kind: &UpdateClientKind, + update_kind: &UpdateKind, ) -> Result { match update_kind { - UpdateClientKind::UpdateHeader => { + UpdateKind::UpdateClient => { let header = TmHeader::try_from(client_message)?; self.check_for_misbehaviour_update_client(ctx, client_id, header) } - UpdateClientKind::Misbehaviour => { + UpdateKind::SubmitMisbehaviour => { let misbehaviour = TmMisbehaviour::try_from(client_message)?; self.check_for_misbehaviour_misbehavior(&misbehaviour) } @@ -308,7 +308,7 @@ impl Ics2ClientState for ClientState { ctx: &mut dyn ExecutionContext, client_id: &ClientId, client_message: Any, - _update_kind: &UpdateClientKind, + _update_kind: &UpdateKind, ) -> Result, ClientError> { // we only expect headers to make it here; if a TmMisbehaviour makes it to here, // then it is not a valid misbehaviour (check_for_misbehaviour returned false), @@ -358,7 +358,7 @@ impl Ics2ClientState for ClientState { ctx: &mut dyn ExecutionContext, client_id: &ClientId, _client_message: Any, - _update_kind: &UpdateClientKind, + _update_kind: &UpdateKind, ) -> Result<(), ClientError> { let frozen_client_state = self .clone() diff --git a/crates/ibc/src/core/handler.rs b/crates/ibc/src/core/handler.rs index 46317232d..c45791ef2 100644 --- a/crates/ibc/src/core/handler.rs +++ b/crates/ibc/src/core/handler.rs @@ -50,7 +50,7 @@ mod tests { msgs::transfer::test_util::get_dummy_msg_transfer, msgs::transfer::MsgTransfer, packet::PacketData, PrefixedCoin, MODULE_ID_STR, }; - use crate::core::ics02_client::msgs::update_client::UpdateClientKind; + use crate::core::ics02_client::msgs::update_client::UpdateKind; use crate::core::ics02_client::msgs::{ create_client::MsgCreateClient, update_client::MsgUpdateClient, upgrade_client::MsgUpgradeClient, ClientMsg, @@ -273,7 +273,7 @@ mod tests { client_message: MockHeader::new(update_client_height) .with_timestamp(Timestamp::now()) .into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer: default_signer.clone(), })) .into(), @@ -285,7 +285,7 @@ mod tests { msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), client_message: MockHeader::new(update_client_height).into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer: default_signer.clone(), })) .into(), @@ -363,7 +363,7 @@ mod tests { client_message: MockHeader::new(update_client_height_after_send) .with_timestamp(Timestamp::now()) .into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer: default_signer.clone(), })) .into(), @@ -407,7 +407,7 @@ mod tests { msg: MsgEnvelope::Client(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), client_message: MockHeader::new(update_client_height_after_second_send).into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer: default_signer.clone(), })) .into(), diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 3194589df..04f689306 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -19,7 +19,7 @@ use crate::prelude::*; use crate::Height; use super::consensus_state::ConsensusState; -use super::msgs::update_client::UpdateClientKind; +use super::msgs::update_client::UpdateKind; use crate::core::{ExecutionContext, ValidationContext}; @@ -79,7 +79,7 @@ pub trait ClientState: ctx: &dyn ValidationContext, client_id: &ClientId, client_message: Any, - update_kind: &UpdateClientKind, + update_kind: &UpdateKind, ) -> Result<(), ClientError>; /// Checks for evidence of a misbehaviour in Header or Misbehaviour type. It @@ -89,7 +89,7 @@ pub trait ClientState: ctx: &dyn ValidationContext, client_id: &ClientId, client_message: Any, - update_kind: &UpdateClientKind, + update_kind: &UpdateKind, ) -> Result; /// Updates and stores as necessary any associated information for an IBC @@ -104,7 +104,7 @@ pub trait ClientState: ctx: &mut dyn ExecutionContext, client_id: &ClientId, client_message: Any, - update_kind: &UpdateClientKind, + update_kind: &UpdateKind, ) -> Result, ClientError>; /// update_state_on_misbehaviour should perform appropriate state changes on @@ -114,7 +114,7 @@ pub trait ClientState: ctx: &mut dyn ExecutionContext, client_id: &ClientId, client_message: Any, - update_kind: &UpdateClientKind, + update_kind: &UpdateKind, ) -> Result<(), ClientError>; /// Verify the upgraded client and consensus states and validate proofs 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 0135d5e6e..e199b389d 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -98,7 +98,7 @@ mod tests { use crate::core::ics02_client::client_type::ClientType; use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::handler::update_client::{execute, validate}; - use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateClientKind}; + use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateKind}; use crate::core::ics24_host::identifier::{ChainId, ClientId}; use crate::core::ValidationContext; use crate::events::{IbcEvent, IbcEventType}; @@ -125,7 +125,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: MockHeader::new(height).with_timestamp(timestamp).into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer, }; @@ -152,7 +152,7 @@ mod tests { let msg = MsgUpdateClient { client_id: ClientId::from_str("nonexistingclient").unwrap(), client_message: MockHeader::new(Height::new(0, 46).unwrap()).into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer, }; @@ -193,7 +193,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: block.into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer, }; @@ -241,7 +241,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: block.into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer, }; @@ -329,7 +329,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: block.into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer, }; @@ -381,7 +381,7 @@ mod tests { let msg = MsgUpdateClient { client_id, client_message: block_ref.clone().into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer, }; @@ -402,7 +402,7 @@ mod tests { let msg = MsgUpdateClient { client_id: client_id.clone(), client_message: header.clone(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer, }; @@ -455,7 +455,7 @@ mod tests { header2: MockHeader::new(height).with_timestamp(timestamp), } .into(), - update_kind: UpdateClientKind::Misbehaviour, + update_kind: UpdateKind::SubmitMisbehaviour, signer: get_dummy_account_id(), }; @@ -482,7 +482,7 @@ mod tests { header2: MockHeader::new(height), } .into(), - update_kind: UpdateClientKind::Misbehaviour, + update_kind: UpdateKind::SubmitMisbehaviour, signer: get_dummy_account_id(), }; @@ -544,7 +544,7 @@ mod tests { let msg = MsgUpdateClient { client_id: client_id.clone(), client_message: TmMisbehaviour::new(client_id.clone(), header1, header2).into(), - update_kind: UpdateClientKind::Misbehaviour, + update_kind: UpdateKind::SubmitMisbehaviour, signer: get_dummy_account_id(), }; @@ -607,7 +607,7 @@ mod tests { client_id: client_id.clone(), client_message: TmMisbehaviour::new(client_id.clone(), header1.into(), header2.into()) .into(), - update_kind: UpdateClientKind::Misbehaviour, + update_kind: UpdateKind::SubmitMisbehaviour, signer: get_dummy_account_id(), }; diff --git a/crates/ibc/src/core/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index de61db0a7..36bc1ac38 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -14,22 +14,28 @@ use crate::signer::Signer; pub const UPDATE_CLIENT_TYPE_URL: &str = "/ibc.core.client.v1.MsgUpdateClient"; pub const MISBEHAVIOUR_TYPE_URL: &str = "/ibc.core.client.v1.MsgSubmitMisbehaviour"; +/// `UpdateKind` represents the 2 ways that a client can be updated +/// in IBC: either through a `MsgUpdateClient`, or a `MsgSubmitMisbehaviour`. #[derive(Clone, Debug, PartialEq, Eq)] -pub enum UpdateClientKind { +pub enum UpdateKind { /// this is the typical scenario where a new header is submitted to the client - /// to update the client - UpdateHeader, + /// to update the client. Note that light clients are free to define the type + /// of the object used to update them (e.g. could be a list of headers). + UpdateClient, /// this is the scenario where misbehaviour is submitted to the client /// (e.g 2 headers with the same height in Tendermint) - Misbehaviour, + SubmitMisbehaviour, } -/// A type of message that triggers the update of an on-chain (IBC) client with new headers. +/// Represents the message that triggers the update of an on-chain (IBC) client +/// either with new headers, or evidence of misbehaviour. +/// Note that some types of misbehaviour can be detected when a headers +/// are updated (`UpdateKind::UpdateClient`). #[derive(Clone, Debug, PartialEq, Eq)] pub struct MsgUpdateClient { pub client_id: ClientId, pub client_message: Any, - pub update_kind: UpdateClientKind, + pub update_kind: UpdateKind, pub signer: Signer, } @@ -45,7 +51,7 @@ impl TryFrom for MsgUpdateClient { .parse() .map_err(ClientError::InvalidMsgUpdateClientId)?, client_message: raw.header.ok_or(ClientError::MissingRawHeader)?, - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer: raw.signer.parse().map_err(ClientError::Signer)?, }) } @@ -77,7 +83,7 @@ impl TryFrom for MsgUpdateClient { .parse() .map_err(ClientError::InvalidRawMisbehaviour)?, client_message: raw_misbehaviour, - update_kind: UpdateClientKind::Misbehaviour, + update_kind: UpdateKind::SubmitMisbehaviour, signer: raw.signer.parse().map_err(ClientError::Signer)?, }) } @@ -113,7 +119,7 @@ mod tests { MsgUpdateClient { client_id, client_message: header, - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer, } } diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 22f764f9f..63c0f19ce 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -1,4 +1,4 @@ -use crate::core::ics02_client::msgs::update_client::UpdateClientKind; +use crate::core::ics02_client::msgs::update_client::UpdateKind; use crate::core::ics24_host::path::{ClientConsensusStatePath, ClientStatePath}; use crate::prelude::*; @@ -244,10 +244,10 @@ impl ClientState for MockClientState { _ctx: &dyn ValidationContext, _client_id: &ClientId, client_message: Any, - update_kind: &UpdateClientKind, + update_kind: &UpdateKind, ) -> Result<(), ClientError> { match update_kind { - UpdateClientKind::UpdateHeader => { + UpdateKind::UpdateClient => { let header = MockHeader::try_from(client_message)?; if self.latest_height() >= header.height() { @@ -257,7 +257,7 @@ impl ClientState for MockClientState { }); } } - UpdateClientKind::Misbehaviour => { + UpdateKind::SubmitMisbehaviour => { let _misbehaviour = Misbehaviour::try_from(client_message)?; } } @@ -273,11 +273,11 @@ impl ClientState for MockClientState { _ctx: &dyn ValidationContext, _client_id: &ClientId, client_message: Any, - update_kind: &UpdateClientKind, + update_kind: &UpdateKind, ) -> Result { match update_kind { - UpdateClientKind::UpdateHeader => Ok(false), - UpdateClientKind::Misbehaviour => { + UpdateKind::UpdateClient => Ok(false), + UpdateKind::SubmitMisbehaviour => { let misbehaviour = Misbehaviour::try_from(client_message)?; let header_1 = misbehaviour.header1; let header_2 = misbehaviour.header2; @@ -295,7 +295,7 @@ impl ClientState for MockClientState { ctx: &mut dyn ExecutionContext, client_id: &ClientId, client_message: Any, - _update_kind: &UpdateClientKind, + _update_kind: &UpdateKind, ) -> Result, ClientError> { let header = MockHeader::try_from(client_message)?; let header_height = header.height; @@ -327,7 +327,7 @@ impl ClientState for MockClientState { ctx: &mut dyn ExecutionContext, client_id: &ClientId, _client_message: Any, - _update_kind: &UpdateClientKind, + _update_kind: &UpdateKind, ) -> Result<(), ClientError> { let frozen_client_state = self .with_frozen_height(Height::new(0, 1).unwrap()) diff --git a/crates/ibc/src/mock/ics18_relayer/context.rs b/crates/ibc/src/mock/ics18_relayer/context.rs index 3a4c194bc..ae27f6023 100644 --- a/crates/ibc/src/mock/ics18_relayer/context.rs +++ b/crates/ibc/src/mock/ics18_relayer/context.rs @@ -32,7 +32,7 @@ pub trait RelayerContext { mod tests { use crate::clients::ics07_tendermint::client_type as tm_client_type; use crate::core::ics02_client::header::{downcast_header, Header}; - use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateClientKind}; + use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateKind}; use crate::core::ics02_client::msgs::ClientMsg; use crate::core::ics24_host::identifier::{ChainId, ClientId}; use crate::core::ics26_routing::msgs::MsgEnvelope; @@ -87,7 +87,7 @@ mod tests { Ok(ClientMsg::UpdateClient(MsgUpdateClient { client_id: client_id.clone(), client_message: src_header.clone_into(), - update_kind: UpdateClientKind::UpdateHeader, + update_kind: UpdateKind::UpdateClient, signer: dest.signer(), })) } From 009a5cb010e89f6d80ab81c70d442ac04ff2fcfe Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 12 Apr 2023 17:26:30 -0400 Subject: [PATCH 58/62] fmt --- crates/ibc/src/core/ics02_client/msgs/update_client.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index 36bc1ac38..16695f6ca 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -19,8 +19,8 @@ pub const MISBEHAVIOUR_TYPE_URL: &str = "/ibc.core.client.v1.MsgSubmitMisbehavio #[derive(Clone, Debug, PartialEq, Eq)] pub enum UpdateKind { /// this is the typical scenario where a new header is submitted to the client - /// to update the client. Note that light clients are free to define the type - /// of the object used to update them (e.g. could be a list of headers). + /// to update the client. Note that light clients are free to define the type + /// of the object used to update them (e.g. could be a list of headers). UpdateClient, /// this is the scenario where misbehaviour is submitted to the client /// (e.g 2 headers with the same height in Tendermint) From af109786f551da630fb3b6ae9a03944e064c9a7f Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 12 Apr 2023 17:31:06 -0400 Subject: [PATCH 59/62] remove unused misbehaviour trait --- .../clients/ics07_tendermint/misbehaviour.rs | 11 ----- .../ibc/src/core/ics02_client/misbehaviour.rs | 45 ------------------- crates/ibc/src/core/ics02_client/mod.rs | 1 - crates/ibc/src/mock/misbehaviour.rs | 11 ----- 4 files changed, 68 deletions(-) delete mode 100644 crates/ibc/src/core/ics02_client/misbehaviour.rs diff --git a/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs index 19825777d..2f23859eb 100644 --- a/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs +++ b/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs @@ -10,7 +10,6 @@ use crate::clients::ics07_tendermint::error::Error; use crate::clients::ics07_tendermint::header::Header; use crate::core::ics02_client::error::ClientError; use crate::core::ics24_host::identifier::{ChainId, ClientId}; -use crate::Height; pub const TENDERMINT_MISBEHAVIOUR_TYPE_URL: &str = "/ibc.lightclients.tendermint.v1.Misbehaviour"; @@ -53,16 +52,6 @@ impl Misbehaviour { } } -impl crate::core::ics02_client::misbehaviour::Misbehaviour for Misbehaviour { - fn client_id(&self) -> &ClientId { - &self.client_id - } - - fn height(&self) -> Height { - self.header1.height() - } -} - impl Protobuf for Misbehaviour {} impl TryFrom for Misbehaviour { diff --git a/crates/ibc/src/core/ics02_client/misbehaviour.rs b/crates/ibc/src/core/ics02_client/misbehaviour.rs deleted file mode 100644 index be2acfc42..000000000 --- a/crates/ibc/src/core/ics02_client/misbehaviour.rs +++ /dev/null @@ -1,45 +0,0 @@ -use dyn_clone::DynClone; - -use crate::dynamic_typing::AsAny; -use crate::prelude::*; - -use crate::core::ics24_host::identifier::ClientId; -use crate::Height; - -pub trait Misbehaviour: - AsAny + sealed::ErasedPartialEqMisbehaviour + DynClone + core::fmt::Debug + Send + Sync -{ - /// The type of client (eg. Tendermint) - fn client_id(&self) -> &ClientId; - - /// The height of the consensus state - fn height(&self) -> Height; -} - -// Implements `Clone` for `Box` -dyn_clone::clone_trait_object!(Misbehaviour); - -impl PartialEq for dyn Misbehaviour { - fn eq(&self, other: &Self) -> bool { - self.eq_misbehaviour(other) - } -} -mod sealed { - use super::*; - - pub trait ErasedPartialEqMisbehaviour { - fn eq_misbehaviour(&self, other: &dyn Misbehaviour) -> bool; - } - - impl ErasedPartialEqMisbehaviour for H - where - H: Misbehaviour + PartialEq, - { - fn eq_misbehaviour(&self, other: &dyn Misbehaviour) -> bool { - other - .as_any() - .downcast_ref::() - .map_or(false, |h| self == h) - } - } -} diff --git a/crates/ibc/src/core/ics02_client/mod.rs b/crates/ibc/src/core/ics02_client/mod.rs index 6033015f2..33ed6674d 100644 --- a/crates/ibc/src/core/ics02_client/mod.rs +++ b/crates/ibc/src/core/ics02_client/mod.rs @@ -8,6 +8,5 @@ pub mod events; pub mod handler; pub mod header; pub mod height; -pub mod misbehaviour; pub mod msgs; pub mod trust_threshold; diff --git a/crates/ibc/src/mock/misbehaviour.rs b/crates/ibc/src/mock/misbehaviour.rs index 522ac9f50..b6f88b626 100644 --- a/crates/ibc/src/mock/misbehaviour.rs +++ b/crates/ibc/src/mock/misbehaviour.rs @@ -8,7 +8,6 @@ use ibc_proto::protobuf::Protobuf; use crate::core::ics02_client::error::ClientError; use crate::core::ics24_host::identifier::ClientId; use crate::mock::header::MockHeader; -use crate::Height; pub const MOCK_MISBEHAVIOUR_TYPE_URL: &str = "/ibc.mock.Misbehavior"; @@ -20,16 +19,6 @@ pub struct Misbehaviour { pub header2: MockHeader, } -impl crate::core::ics02_client::misbehaviour::Misbehaviour for Misbehaviour { - fn client_id(&self) -> &ClientId { - &self.client_id - } - - fn height(&self) -> Height { - self.header1.height() - } -} - impl Protobuf for Misbehaviour {} impl TryFrom for Misbehaviour { From d4395749ddd011f8fea0b23cd721dbf0dfe61232 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 12 Apr 2023 17:51:13 -0400 Subject: [PATCH 60/62] fix misbehaviour header checks --- .../client_state/misbehaviour.rs | 55 ++++++------------- 1 file changed, 18 insertions(+), 37 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs index 6231a42d1..d9387750d 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs @@ -1,3 +1,5 @@ +use tendermint_light_client_verifier::Verifier; + use crate::core::ics02_client::consensus_state::ConsensusState; use crate::prelude::*; @@ -42,8 +44,8 @@ impl ClientState { self.check_misbehaviour_headers_consistency(header_1, header_2)?; let current_timestamp = ctx.host_timestamp()?; - self.check_misbehaviour_header(header_1, &trusted_consensus_state_1, current_timestamp)?; - self.check_misbehaviour_header(header_2, &trusted_consensus_state_2, current_timestamp) + self.verify_misbehaviour_header(header_1, &trusted_consensus_state_1, current_timestamp)?; + self.verify_misbehaviour_header(header_2, &trusted_consensus_state_2, current_timestamp) } pub fn check_misbehaviour_headers_consistency( @@ -72,7 +74,7 @@ impl ClientState { Ok(()) } - pub fn check_misbehaviour_header( + pub fn verify_misbehaviour_header( &self, header: &TmHeader, trusted_consensus_state: &TmConsensusState, @@ -99,41 +101,20 @@ impl ClientState { } } - // ensure the client trust_threshold overlap between the validator sets of the trusted and untrusted states. - { - let untrusted_state = header.as_untrusted_block_state(); - let chain_id = self - .chain_id - .clone() - .with_version(header.height().revision_number()) - .into(); - let trusted_state = - TmHeader::as_trusted_block_state(header, trusted_consensus_state, &chain_id)?; - let options = self.as_light_client_options()?; - - self.verifier - .verify_commit_against_trusted(&untrusted_state, &trusted_state, &options) - .into_result()?; - } + // main header verification, delegated to the tendermint-light-client crate. + let untrusted_state = header.as_untrusted_block_state(); - // run the verification checks on the header based on the trusted - // consensus state - { - let untrusted_state = header.as_untrusted_block_state(); - let chain_id = self.chain_id.clone().into(); - let trusted_state = - header.as_trusted_block_state(trusted_consensus_state, &chain_id)?; - let options = self.as_light_client_options()?; - - self.verifier - .validate_against_trusted( - &untrusted_state, - &trusted_state, - &options, - current_timestamp.into_tm_time().unwrap(), - ) - .into_result()?; - } + let chain_id = self.chain_id.clone().into(); + let trusted_state = header.as_trusted_block_state(trusted_consensus_state, &chain_id)?; + + let options = self.as_light_client_options()?; + let current_timestamp = current_timestamp.into_tm_time().ok_or(ClientError::Other { + description: "host timestamp must not be zero".to_string(), + })?; + + self.verifier + .verify(untrusted_state, trusted_state, &options, current_timestamp) + .into_result()?; Ok(()) } From 8ea9ee49d7ef7317ececdb1d43b8dee7376cd8d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Laferri=C3=A8re?= Date: Wed, 12 Apr 2023 17:53:30 -0400 Subject: [PATCH 61/62] Update crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Anca Zamfir Signed-off-by: Philippe Laferrière --- .../src/clients/ics07_tendermint/client_state/update_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs index e94ec0d41..fcd1fb401 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs @@ -133,7 +133,7 @@ impl ClientState { // previous consensus state's height let prev_cs = downcast_tm_consensus_state(prev_cs.as_ref())?; - if header.signed_header.header().time < prev_cs.timestamp { + if header.signed_header.header().time <= prev_cs.timestamp { return Ok(true); } } From f6c375a8f133cec44bfaa28b3006d2aab40e9b12 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 12 Apr 2023 18:24:08 -0400 Subject: [PATCH 62/62] fix test after merge --- .../ics02_client/handler/update_client.rs | 38 +++++++++++++++++-- 1 file changed, 35 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 e199b389d..4b185b009 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -87,10 +87,11 @@ where #[cfg(test)] mod tests { use core::str::FromStr; + use core::time::Duration; use ibc_proto::google::protobuf::Any; use test_log::test; - use crate::clients::ics07_tendermint::client_state::test_util::get_dummy_tendermint_client_state; + use crate::clients::ics07_tendermint::client_state::ClientState as TmClientState; use crate::clients::ics07_tendermint::client_type as tm_client_type; use crate::clients::ics07_tendermint::header::Header as TmHeader; use crate::clients::ics07_tendermint::misbehaviour::Misbehaviour as TmMisbehaviour; @@ -99,6 +100,7 @@ mod tests { use crate::core::ics02_client::consensus_state::ConsensusState; use crate::core::ics02_client::handler::update_client::{execute, validate}; use crate::core::ics02_client::msgs::update_client::{MsgUpdateClient, UpdateKind}; + use crate::core::ics23_commitment::specs::ProofSpecs; use crate::core::ics24_host::identifier::{ChainId, ClientId}; use crate::core::ValidationContext; use crate::events::{IbcEvent, IbcEventType}; @@ -112,6 +114,7 @@ mod tests { use crate::timestamp::Timestamp; use crate::Height; use crate::{downcast, prelude::*}; + use ibc_proto::ibc::lightclients::tendermint::v1::{ClientState as RawTmClientState, Fraction}; #[test] fn test_update_client_ok() { @@ -312,8 +315,37 @@ mod tests { let consensus_state: Box = block.clone().into(); let tm_block = downcast!(block.clone() => HostBlock::SyntheticTendermint).unwrap(); - let client_state = - get_dummy_tendermint_client_state(tm_block.header().clone()).into_box(); + + let client_state = { + #[allow(deprecated)] + let raw_client_state = RawTmClientState { + chain_id: ChainId::from(tm_block.header().chain_id.clone()).to_string(), + trust_level: Some(Fraction { + numerator: 1, + denominator: 3, + }), + trusting_period: Some(Duration::from_secs(64000).into()), + unbonding_period: Some(Duration::from_secs(128000).into()), + max_clock_drift: Some(Duration::from_millis(3000).into()), + latest_height: Some( + Height::new( + ChainId::chain_version(tm_block.header().chain_id.as_str()), + u64::from(tm_block.header().height), + ) + .unwrap() + .into(), + ), + proof_specs: ProofSpecs::default().into(), + upgrade_path: Default::default(), + frozen_height: None, + allow_update_after_expiry: false, + allow_update_after_misbehaviour: false, + }; + + let client_state = TmClientState::try_from(raw_client_state).unwrap(); + + client_state.into_box() + }; let mut ibc_store = ctx_a.ibc_store.lock(); let client_record = ibc_store.clients.get_mut(&client_id).unwrap();