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)) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 9dadd0306..22ba8fb86 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -1,5 +1,10 @@ +mod misbehaviour; +mod update_client; + +use crate::core::ics02_client::msgs::update_client::UpdateKind; use crate::prelude::*; +use core::cmp::max; use core::convert::{TryFrom, TryInto}; use core::time::Duration; @@ -14,13 +19,12 @@ 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; -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; @@ -33,16 +37,14 @@ 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::timestamp::ZERO_DURATION; use crate::Height; 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"; @@ -176,15 +178,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 }) } @@ -214,81 +210,6 @@ impl ClientState { clock_drift: self.max_clock_drift, }) } - - fn check_header_validator_set( - trusted_consensus_state: &TmConsensusState, - header: &Header, - ) -> Result<(), ClientError> { - let trusted_val_hash = header.trusted_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(), - 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)?; - - 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 { @@ -341,262 +262,111 @@ impl Ics2ClientState for ClientState { TmConsensusState::try_from(consensus_state).map(TmConsensusState::into_box) } - fn check_misbehaviour_and_update_state( + fn verify_client_message( &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(), - )); + client_id: &ClientId, + client_message: Any, + update_kind: &UpdateKind, + ) -> Result<(), ClientError> { + match update_kind { + UpdateKind::UpdateClient => { + let header = TmHeader::try_from(client_message)?; + self.verify_header(ctx, client_id, header) } - } else { - // BFT time violation - if header_1.signed_header.header.time > header_2.signed_header.header.time { - return Err(ContextError::ClientError( - Error::MisbehaviourHeadersNotAtSameHeight.into(), - )); + UpdateKind::SubmitMisbehaviour => { + let misbehaviour = TmMisbehaviour::try_from(client_message)?; + self.verify_misbehaviour(ctx, client_id, misbehaviour) } } - 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( + // 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, - 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(), - }), - }, + client_id: &ClientId, + client_message: Any, + update_kind: &UpdateKind, + ) -> Result { + match update_kind { + UpdateKind::UpdateClient => { + let header = TmHeader::try_from(client_message)?; + self.check_for_misbehaviour_update_client(ctx, client_id, header) } - } - - 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) + UpdateKind::SubmitMisbehaviour => { + let misbehaviour = TmMisbehaviour::try_from(client_message)?; + self.check_for_misbehaviour_misbehavior(&misbehaviour) } - 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) - .map_err(|e| match e { - ContextError::ClientError(e) => e, - _ => ClientError::Other { - description: e.to_string(), - }, - })? - .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_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, + } + } + fn update_state( + &self, + ctx: &mut dyn ExecutionContext, + client_id: &ClientId, + client_message: Any, + _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), + // 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); + + ctx.consensus_state(&path_at_header_height).ok() }; - 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(); - - 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(), - }); - } + if maybe_existing_consensus_state.is_some() { + // if we already had the header installed by a previous relayer + // then this is a no-op. + // + // Do nothing. + } 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)?; } - // 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()) - .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(), - }); - } - } - } + let updated_heights = vec![header_height]; + Ok(updated_heights) + } - // (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(), - }, - })? - .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(), - }); - } - } - } + fn update_state_on_misbehaviour( + &self, + ctx: &mut dyn ExecutionContext, + client_id: &ClientId, + _client_message: Any, + _update_kind: &UpdateKind, + ) -> Result<(), ClientError> { + let frozen_client_state = self + .clone() + .with_frozen_height(Height::new(0, 1).unwrap()) + .into_box(); - Ok(UpdatedState { - client_state: client_state.with_header(header.clone())?.into_box(), - consensus_state: TmConsensusState::from(header).into_box(), - }) + ctx.store_client_state(ClientStatePath::new(client_id), frozen_client_state)?; + + Ok(()) } /// Perform client-specific verifications and check all data in the new @@ -791,6 +561,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::() 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..d9387750d --- /dev/null +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/misbehaviour.rs @@ -0,0 +1,144 @@ +use tendermint_light_client_verifier::Verifier; + +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.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( + &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 verify_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 trusted consensus state is within trusting period + { + 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()); + } + } + + // main header verification, delegated to the tendermint-light-client crate. + 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()?; + 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(()) + } + + 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 new file mode 100644 index 000000000..fcd1fb401 --- /dev/null +++ b/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs @@ -0,0 +1,162 @@ +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::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::{check_header_trusted_next_validator_set, downcast_tm_consensus_state, ClientState}; + +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(()) + } + + 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) + } + } + } +} diff --git a/crates/ibc/src/clients/ics07_tendermint/header.rs b/crates/ibc/src/clients/ics07_tendermint/header.rs index 33667db92..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; @@ -29,8 +28,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 +39,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)) } } @@ -54,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, @@ -81,32 +75,12 @@ 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, }) } } -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() @@ -141,7 +115,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 +170,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 +237,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 +251,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, } } } diff --git a/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs b/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs index f980dc2f0..2f23859eb 100644 --- a/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs +++ b/crates/ibc/src/clients/ics07_tendermint/misbehaviour.rs @@ -5,13 +5,11 @@ 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}; -use crate::Height; pub const TENDERMINT_MISBEHAVIOUR_TYPE_URL: &str = "/ibc.lightclients.tendermint.v1.Misbehaviour"; @@ -24,43 +22,12 @@ 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 { + pub fn new(client_id: ClientId, header1: Header, header2: Header) -> Self { + Self { client_id, header1, header2, - }) + } } pub fn client_id(&self) -> &ClientId { @@ -85,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 { @@ -120,7 +77,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/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/handler.rs b/crates/ibc/src/core/handler.rs index 13e7c13b0..c45791ef2 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, @@ -43,6 +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::UpdateKind; use crate::core::ics02_client::msgs::{ create_client::MsgCreateClient, update_client::MsgUpdateClient, upgrade_client::MsgUpgradeClient, ClientMsg, @@ -262,9 +270,10 @@ 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(), + update_kind: UpdateKind::UpdateClient, signer: default_signer.clone(), })) .into(), @@ -275,7 +284,8 @@ 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(), + update_kind: UpdateKind::UpdateClient, signer: default_signer.clone(), })) .into(), @@ -350,9 +360,10 @@ 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(), + update_kind: UpdateKind::UpdateClient, signer: default_signer.clone(), })) .into(), @@ -395,7 +406,8 @@ 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(), + 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 cf03ad747..04f689306 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -19,8 +19,9 @@ use crate::prelude::*; use crate::Height; use super::consensus_state::ConsensusState; +use super::msgs::update_client::UpdateKind; -use crate::core::{ContextError, ValidationContext}; +use crate::core::{ExecutionContext, ValidationContext}; pub trait ClientState: AsAny @@ -67,19 +68,54 @@ pub trait ClientState: fn initialise(&self, consensus_state: Any) -> Result, ClientError>; - fn check_header_and_update_state( + /// 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, - client_id: ClientId, - header: Any, - ) -> Result; + client_id: &ClientId, + client_message: Any, + update_kind: &UpdateKind, + ) -> Result<(), ClientError>; - fn check_misbehaviour_and_update_state( + /// 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, - client_id: ClientId, - misbehaviour: Any, - ) -> Result, ContextError>; + client_id: &ClientId, + client_message: Any, + update_kind: &UpdateKind, + ) -> 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. + /// + /// Post-condition: on success, the return value MUST contain at least one + /// height. + fn update_state( + &self, + ctx: &mut dyn ExecutionContext, + client_id: &ClientId, + client_message: Any, + update_kind: &UpdateKind, + ) -> 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, + client_id: &ClientId, + client_message: Any, + update_kind: &UpdateKind, + ) -> Result<(), ClientError>; /// Verify the upgraded client and consensus states and validate proofs /// against the given root. diff --git a/crates/ibc/src/core/ics02_client/error.rs b/crates/ibc/src/core/ics02_client/error.rs index 3579776b3..95d45eaa7 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; @@ -115,6 +116,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/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 f8d842348..4b185b009 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -1,19 +1,14 @@ //! Protocol logic specific to processing ICS2 messages of type `MsgUpdateAnyClient`. -use tracing::debug; - +use crate::core::ics02_client::error::ClientError; 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::events::{ClientMisbehaviour, 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> @@ -22,7 +17,8 @@ where { let MsgUpdateClient { client_id, - header, + client_message, + update_kind, signer: _, } = msg; @@ -32,41 +28,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> @@ -75,48 +39,43 @@ where { let MsgUpdateClient { client_id, - header, + 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, )?; - { - let consensus_height = client_state.latest_height(); + if found_misbehaviour { + client_state.update_state_on_misbehaviour(ctx, &client_id, client_message, &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 { + let consensus_heights = + client_state.update_state(ctx, &client_id, client_message.clone(), &update_kind)?; + + 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], - header, + *consensus_height, + consensus_heights, + client_message, )); ctx.emit_ibc_event(IbcEvent::Message(event.event_type())); ctx.emit_ibc_event(event); @@ -128,15 +87,20 @@ 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::ClientState as TmClientState; 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::consensus_state::downcast_consensus_state; + 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; + 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}; @@ -145,10 +109,12 @@ mod tests { 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::*}; + use ibc_proto::ibc::lightclients::tendermint::v1::{ClientState as RawTmClientState, Fraction}; #[test] fn test_update_client_ok() { @@ -161,7 +127,8 @@ 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(), + update_kind: UpdateKind::UpdateClient, signer, }; @@ -187,7 +154,8 @@ 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(), + update_kind: UpdateKind::UpdateClient, signer, }; @@ -227,7 +195,8 @@ mod tests { let latest_header_height = block.height(); let msg = MsgUpdateClient { client_id, - header: block.into(), + client_message: block.into(), + update_kind: UpdateKind::UpdateClient, signer, }; @@ -274,7 +243,8 @@ mod tests { let latest_header_height = block.height(); let msg = MsgUpdateClient { client_id, - header: block.into(), + client_message: block.into(), + update_kind: UpdateKind::UpdateClient, signer, }; @@ -294,23 +264,21 @@ 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_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 = MockContext::new( - ChainId::new("mockgaiaA".to_string(), 1), - HostType::Mock, - 5, - chain_start_height, - ) - .with_client_parametrized( - &client_id, - client_height, - Some(tm_client_type()), // The target host chain (B) is synthetic TM. - Some(client_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, + client_height, + Some(tm_client_type()), // The target host chain (B) is synthetic TM. + Some(start_height), + ); let ctx_b = MockContext::new( - ChainId::new("mockgaiaB".to_string(), 1), + ctx_b_chain_id, HostType::SyntheticTendermint, 5, client_height, @@ -319,36 +287,94 @@ 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.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(); - } + // 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) } _ => 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 = { + #[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(); + + 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, - header: block.into(), + client_message: block.into(), + update_kind: UpdateKind::UpdateClient, signer, }; - let res = validate(&ctx, msg.clone()); - assert!(res.is_ok()); + let res = validate(&ctx_a, msg.clone()); + assert!(res.is_ok(), "result: {res:?}"); - 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] @@ -386,7 +412,8 @@ mod tests { let msg = MsgUpdateClient { client_id, - header: block_ref.clone().into(), + client_message: block_ref.clone().into(), + update_kind: UpdateKind::UpdateClient, signer, }; @@ -406,7 +433,8 @@ 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(), + update_kind: UpdateKind::UpdateClient, signer, }; @@ -425,4 +453,200 @@ 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: UpdateKind::SubmitMisbehaviour, + 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: UpdateKind::SubmitMisbehaviour, + 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).into(), + update_kind: UpdateKind::SubmitMisbehaviour, + 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()) + .into(), + update_kind: UpdateKind::SubmitMisbehaviour, + 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/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/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/ics02_client/msgs/update_client.rs b/crates/ibc/src/core/ics02_client/msgs/update_client.rs index 57d470273..16695f6ca 100644 --- a/crates/ibc/src/core/ics02_client/msgs/update_client.rs +++ b/crates/ibc/src/core/ics02_client/msgs/update_client.rs @@ -3,32 +3,42 @@ 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; 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"; -/// A type of message that triggers the update of an on-chain (IBC) client with new headers. +/// `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 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). + UpdateClient, + /// this is the scenario where misbehaviour is submitted to the client + /// (e.g 2 headers with the same height in Tendermint) + SubmitMisbehaviour, +} + +/// 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 header: Any, + pub client_message: Any, + pub update_kind: UpdateKind, 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 { @@ -40,7 +50,8 @@ 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)?, + update_kind: UpdateKind::UpdateClient, signer: raw.signer.parse().map_err(ClientError::Signer)?, }) } @@ -50,7 +61,39 @@ 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(), + } + } +} + +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, + update_kind: UpdateKind::SubmitMisbehaviour, + 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(), } } @@ -58,6 +101,7 @@ impl From for RawMsgUpdateClient { #[cfg(test)] mod tests { + use super::*; use test_log::test; @@ -74,7 +118,8 @@ mod tests { pub fn new(client_id: ClientId, header: Any, signer: Signer) -> Self { MsgUpdateClient { client_id, - header, + client_message: header, + update_kind: UpdateKind::UpdateClient, signer, } } diff --git a/crates/ibc/src/core/ics26_routing/msgs.rs b/crates/ibc/src/core/ics26_routing/msgs.rs index 36c5c3384..41a25a499 100644 --- a/crates/ibc/src/core/ics26_routing/msgs.rs +++ b/crates/ibc/src/core/ics26_routing/msgs.rs @@ -1,10 +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, }; @@ -36,8 +36,11 @@ impl TryFrom for MsgEnvelope { .map_err(RouterError::MalformedMessageBytes)?; Ok(MsgEnvelope::Client(ClientMsg::CreateClient(domain_msg))) } - update_client::TYPE_URL => { - let domain_msg = update_client::MsgUpdateClient::decode_vec(&any_msg.value) + update_client::UPDATE_CLIENT_TYPE_URL => { + let domain_msg = + >::decode_vec( + &any_msg.value, + ) .map_err(RouterError::MalformedMessageBytes)?; Ok(MsgEnvelope::Client(ClientMsg::UpdateClient(domain_msg))) } @@ -46,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 diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index ad3e3f580..63c0f19ce 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -1,3 +1,5 @@ +use crate::core::ics02_client::msgs::update_client::UpdateKind; +use crate::core::ics24_host::path::{ClientConsensusStatePath, ClientStatePath}; use crate::prelude::*; use alloc::collections::btree_map::BTreeMap as HashMap; @@ -24,7 +26,7 @@ use crate::mock::misbehaviour::Misbehaviour; use crate::Height; -use crate::core::{ContextError, ValidationContext}; +use crate::core::{ExecutionContext, ValidationContext}; pub const MOCK_CLIENT_STATE_TYPE_URL: &str = "/ibc.mock.ClientState"; @@ -184,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, @@ -285,6 +238,105 @@ impl ClientState for MockClientState { ) -> Result<(), ClientError> { Ok(()) } + + fn verify_client_message( + &self, + _ctx: &dyn ValidationContext, + _client_id: &ClientId, + client_message: Any, + update_kind: &UpdateKind, + ) -> Result<(), ClientError> { + match update_kind { + UpdateKind::UpdateClient => { + 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(), + }); + } + } + UpdateKind::SubmitMisbehaviour => { + let _misbehaviour = Misbehaviour::try_from(client_message)?; + } + } + + Ok(()) + } + + /// We consider the following to be misbehaviour: + /// + UpdateHeader: no misbehaviour possible + /// + 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: &UpdateKind, + ) -> Result { + match update_kind { + UpdateKind::UpdateClient => Ok(false), + UpdateKind::SubmitMisbehaviour => { + 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, + _update_kind: &UpdateKind, + ) -> Result, ClientError> { + 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_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(vec![header_height]) + } + + fn update_state_on_misbehaviour( + &self, + ctx: &mut dyn ExecutionContext, + client_id: &ClientId, + _client_message: Any, + _update_kind: &UpdateKind, + ) -> Result<(), ClientError> { + 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(()) + } } impl From for MockClientState { diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index 321977f9b..97d5a5046 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -210,7 +210,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, diff --git a/crates/ibc/src/mock/ics18_relayer/context.rs b/crates/ibc/src/mock/ics18_relayer/context.rs index 8282f5ae2..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; + 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; @@ -86,7 +86,8 @@ 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(), + update_kind: UpdateKind::UpdateClient, signer: dest.signer(), })) } 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 {