From 188e3db4d77392a36ec88dc861ad4fd0438b73fa Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Tue, 9 Jul 2024 22:36:41 -0700 Subject: [PATCH 01/13] refactor: allow using custom path for light client proof verification --- ibc-clients/cw-context/src/handlers.rs | 11 +- ibc-clients/cw-context/src/types/msgs.rs | 44 +++--- .../src/client_state/common.rs | 112 ++------------- .../src/client_state/validation.rs | 131 +++++++++++++++++- .../ics02-client/context/src/client_state.rs | 45 +++--- ibc-core/ics02-client/context/src/context.rs | 6 +- .../src/handler/upgrade_client.rs | 18 +-- .../src/handler/conn_open_ack.rs | 15 +- .../src/handler/conn_open_confirm.rs | 6 +- .../src/handler/conn_open_try.rs | 16 ++- .../src/handler/acknowledgement.rs | 6 +- .../src/handler/chan_close_confirm.rs | 6 +- .../src/handler/chan_open_ack.rs | 6 +- .../src/handler/chan_open_confirm.rs | 6 +- .../src/handler/chan_open_try.rs | 6 +- .../ics04-channel/src/handler/recv_packet.rs | 7 +- ibc-core/ics04-channel/src/handler/timeout.rs | 10 +- .../src/handler/timeout_on_close.rs | 14 +- ibc-core/ics23-commitment/types/Cargo.toml | 12 +- ibc-core/ics23-commitment/types/src/merkle.rs | 38 +++-- ibc-core/ics24-host/types/src/path.rs | 24 ++++ .../traits/client_state_common.rs | 25 +--- .../traits/client_state_validation.rs | 23 +++ ibc-derive/src/utils.rs | 4 +- .../testapp/ibc/clients/mock/client_state.rs | 45 +++--- .../tests/core/ics02_client/create_client.rs | 8 +- 26 files changed, 380 insertions(+), 264 deletions(-) diff --git a/ibc-clients/cw-context/src/handlers.rs b/ibc-clients/cw-context/src/handlers.rs index 4b4798d1e..3ad414231 100644 --- a/ibc-clients/cw-context/src/handlers.rs +++ b/ibc-clients/cw-context/src/handlers.rs @@ -103,20 +103,13 @@ where SudoMsg::VerifyUpgradeAndUpdateState(msg) => { let msg = VerifyUpgradeAndUpdateStateMsg::try_from(msg)?; - let client_cons_state_path = ClientConsensusStatePath::new( - client_id.clone(), - client_state.latest_height().revision_number(), - client_state.latest_height().revision_height(), - ); - - let consensus_state = self.consensus_state(&client_cons_state_path)?; - client_state.verify_upgrade_client( + self, + client_id.clone(), msg.upgrade_client_state.clone(), msg.upgrade_consensus_state.clone(), msg.proof_upgrade_client, msg.proof_upgrade_consensus_state, - consensus_state.root(), )?; client_state.update_state_on_upgrade( diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index 506d65695..d6afeb3f3 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -1,14 +1,13 @@ //! Defines the messages sent to the CosmWasm contract by the 08-wasm proxy //! light client. -use std::str::FromStr; - use cosmwasm_schema::{cw_serde, QueryResponses}; use ibc_client_wasm_types::serializer::Base64; use ibc_client_wasm_types::Bytes; use ibc_core::client::types::proto::v1::Height as RawHeight; use ibc_core::client::types::Height; use ibc_core::commitment_types::commitment::{CommitmentPrefix, CommitmentProofBytes}; -use ibc_core::host::types::path::Path; +use ibc_core::commitment_types::merkle::MerklePath; +use ibc_core::host::types::path::PathBytes; use ibc_core::primitives::proto::Any; use prost::Message; @@ -133,11 +132,6 @@ impl TryFrom for VerifyUpgradeAndUpdateStateM } } -#[cw_serde] -pub struct MerklePath { - pub key_path: Vec, -} - #[cw_serde] pub struct VerifyMembershipMsgRaw { #[schemars(with = "String")] @@ -155,7 +149,7 @@ pub struct VerifyMembershipMsgRaw { pub struct VerifyMembershipMsg { pub prefix: CommitmentPrefix, pub proof: CommitmentProofBytes, - pub path: Path, + pub path: PathBytes, pub value: Vec, pub height: Height, pub delay_block_period: u64, @@ -167,17 +161,22 @@ impl TryFrom for VerifyMembershipMsg { fn try_from(mut raw: VerifyMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof)?; - let prefix = raw.path.key_path.remove(0).into_bytes(); - let path_str = raw.path.key_path.join(""); - let path = Path::from_str(&path_str)?; + let prefix = raw.path.key_path.remove(0); let height = Height::try_from(raw.height)?; + let path_bytes: Vec = raw + .path + .key_path + .into_iter() + .flat_map(|p| p.into_vec()) + .collect(); + Ok(Self { proof, - path, + path: path_bytes.into(), value: raw.value, height, - prefix: CommitmentPrefix::try_from(prefix)?, + prefix: CommitmentPrefix::try_from(prefix.into_vec())?, delay_block_period: raw.delay_block_period, delay_time_period: raw.delay_time_period, }) @@ -198,7 +197,7 @@ pub struct VerifyNonMembershipMsgRaw { pub struct VerifyNonMembershipMsg { pub prefix: CommitmentPrefix, pub proof: CommitmentProofBytes, - pub path: Path, + pub path: PathBytes, pub height: Height, pub delay_block_period: u64, pub delay_time_period: u64, @@ -209,15 +208,20 @@ impl TryFrom for VerifyNonMembershipMsg { fn try_from(mut raw: VerifyNonMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof)?; - let prefix = raw.path.key_path.remove(0).into_bytes(); - let path_str = raw.path.key_path.join(""); - let path = Path::from_str(&path_str)?; + let prefix = raw.path.key_path.remove(0); let height = raw.height.try_into()?; + + let path_bytes: Vec = raw + .path + .key_path + .into_iter() + .flat_map(|p| p.into_vec()) + .collect(); Ok(Self { proof, - path, + path: path_bytes.into(), height, - prefix: CommitmentPrefix::try_from(prefix)?, + prefix: CommitmentPrefix::try_from(prefix.into_vec())?, delay_block_period: raw.delay_block_period, delay_time_period: raw.delay_time_period, }) diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs index b7a7daf5b..aee889214 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/common.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/common.rs @@ -1,19 +1,18 @@ use ibc_client_tendermint_types::{client_type as tm_client_type, ClientState as ClientStateType}; use ibc_core_client::context::client_state::ClientStateCommon; use ibc_core_client::context::consensus_state::ConsensusState; -use ibc_core_client::types::error::{ClientError, UpgradeClientError}; +use ibc_core_client::types::error::ClientError; use ibc_core_client::types::Height; use ibc_core_commitment_types::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, }; -use ibc_core_commitment_types::merkle::{apply_prefix, MerkleProof}; +use ibc_core_commitment_types::merkle::{MerklePath, MerkleProof}; use ibc_core_commitment_types::proto::ics23::{HostFunctionsManager, HostFunctionsProvider}; use ibc_core_commitment_types::specs::ProofSpecs; use ibc_core_host::types::identifiers::ClientType; -use ibc_core_host::types::path::{Path, UpgradeClientPath}; +use ibc_core_host::types::path::PathBytes; use ibc_primitives::prelude::*; use ibc_primitives::proto::Any; -use ibc_primitives::ToVec; use super::ClientState; use crate::consensus_state::ConsensusState as TmConsensusState; @@ -35,30 +34,12 @@ impl ClientStateCommon for ClientState { validate_proof_height(self.inner(), proof_height) } - fn verify_upgrade_client( - &self, - upgraded_client_state: Any, - upgraded_consensus_state: Any, - proof_upgrade_client: CommitmentProofBytes, - proof_upgrade_consensus_state: CommitmentProofBytes, - root: &CommitmentRoot, - ) -> Result<(), ClientError> { - verify_upgrade_client::( - self.inner(), - upgraded_client_state, - upgraded_consensus_state, - proof_upgrade_client, - proof_upgrade_consensus_state, - root, - ) - } - fn verify_membership( &self, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, - path: Path, + path: PathBytes, value: Vec, ) -> Result<(), ClientError> { verify_membership::( @@ -76,7 +57,7 @@ impl ClientStateCommon for ClientState { prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, - path: Path, + path: PathBytes, ) -> Result<(), ClientError> { verify_non_membership::( &self.inner().proof_specs, @@ -128,81 +109,6 @@ pub fn validate_proof_height( Ok(()) } -/// 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. -/// -/// You can learn more about how to upgrade IBC-connected SDK chains in -/// [this](https://ibc.cosmos.network/main/ibc/upgrades/quick-guide.html) -/// guide. -/// -/// Note that this function is typically implemented as part of the -/// [`ClientStateCommon`] trait, but has been made a standalone function -/// in order to make the ClientState APIs more flexible. -pub fn verify_upgrade_client( - client_state: &ClientStateType, - upgraded_client_state: Any, - upgraded_consensus_state: Any, - proof_upgrade_client: CommitmentProofBytes, - proof_upgrade_consensus_state: CommitmentProofBytes, - root: &CommitmentRoot, -) -> Result<(), ClientError> { - // Make sure that the client type is of Tendermint type `ClientState` - let upgraded_tm_client_state = ClientState::try_from(upgraded_client_state.clone())?; - - // Make sure that the consensus type is of Tendermint type `ConsensusState` - TmConsensusState::try_from(upgraded_consensus_state.clone())?; - - let latest_height = client_state.latest_height; - let upgraded_tm_client_state_height = upgraded_tm_client_state.latest_height(); - - // Make sure the latest height of the current client is not greater then - // the upgrade height This condition checks both the revision number and - // the height - if latest_height >= upgraded_tm_client_state_height { - Err(UpgradeClientError::LowUpgradeHeight { - upgraded_height: latest_height, - client_height: upgraded_tm_client_state_height, - })? - } - - // Check to see if the upgrade path is set - let mut upgrade_path = client_state.upgrade_path.clone(); - - if upgrade_path.pop().is_none() { - return Err(ClientError::ClientSpecific { - description: "cannot upgrade client as no upgrade path has been set".to_string(), - }); - }; - - let upgrade_path_prefix = CommitmentPrefix::try_from(upgrade_path[0].clone().into_bytes()) - .map_err(ClientError::InvalidCommitmentProof)?; - - let last_height = latest_height.revision_height(); - - // Verify the proof of the upgraded client state - verify_membership::( - &client_state.proof_specs, - &upgrade_path_prefix, - &proof_upgrade_client, - root, - Path::UpgradeClient(UpgradeClientPath::UpgradedClientState(last_height)), - upgraded_client_state.to_vec(), - )?; - - // Verify the proof of the upgraded consensus state - verify_membership::( - &client_state.proof_specs, - &upgrade_path_prefix, - &proof_upgrade_consensus_state, - root, - Path::UpgradeClient(UpgradeClientPath::UpgradedClientConsensusState(last_height)), - upgraded_consensus_state.to_vec(), - )?; - - Ok(()) -} - /// Verify membership of the given value against the client's merkle proof. /// /// Note that this function is typically implemented as part of the @@ -213,10 +119,10 @@ pub fn verify_membership( prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, - path: Path, + path: PathBytes, value: Vec, ) -> Result<(), ClientError> { - let merkle_path = apply_prefix(prefix, vec![path.to_string()]); + let merkle_path = MerklePath::new(vec![prefix.clone().into_vec().into(), path]); let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?; merkle_proof @@ -234,9 +140,9 @@ pub fn verify_non_membership( prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, - path: Path, + path: PathBytes, ) -> Result<(), ClientError> { - let merkle_path = apply_prefix(prefix, vec![path.to_string()]); + let merkle_path = MerklePath::new(vec![prefix.clone().into_vec().into(), path]); let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?; merkle_proof diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index 116b84dd8..49576eeca 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -2,21 +2,29 @@ use ibc_client_tendermint_types::{ ClientState as ClientStateType, ConsensusState as ConsensusStateType, Header as TmHeader, Misbehaviour as TmMisbehaviour, TENDERMINT_HEADER_TYPE_URL, TENDERMINT_MISBEHAVIOUR_TYPE_URL, }; -use ibc_core_client::context::client_state::ClientStateValidation; -use ibc_core_client::context::{Convertible, ExtClientValidationContext}; -use ibc_core_client::types::error::ClientError; +use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation}; +use ibc_core_client::context::consensus_state::ConsensusState; +use ibc_core_client::context::{ClientValidationContext, Convertible, ExtClientValidationContext}; +use ibc_core_client::types::error::{ClientError, UpgradeClientError}; use ibc_core_client::types::Status; +use ibc_core_commitment_types::commitment::{CommitmentPrefix, CommitmentProofBytes}; +use ibc_core_commitment_types::proto::ics23::{HostFunctionsManager, HostFunctionsProvider}; use ibc_core_host::types::identifiers::ClientId; -use ibc_core_host::types::path::ClientConsensusStatePath; +use ibc_core_host::types::path::{ClientConsensusStatePath, UpgradeClientPath}; use ibc_primitives::prelude::*; use ibc_primitives::proto::Any; +use ibc_primitives::ToVec; use tendermint::crypto::default::Sha256; use tendermint::crypto::Sha256 as Sha256Trait; use tendermint::merkle::MerkleHash; use tendermint_light_client_verifier::{ProdVerifier, Verifier}; -use super::{check_for_misbehaviour_on_misbehavior, check_for_misbehaviour_on_update, ClientState}; +use super::{ + check_for_misbehaviour_on_misbehavior, check_for_misbehaviour_on_update, verify_membership, + ClientState, +}; use crate::client_state::{verify_header, verify_misbehaviour}; +use crate::consensus_state::ConsensusState as TmConsensusState; impl ClientStateValidation for ClientState where @@ -71,6 +79,26 @@ where status(self.inner(), ctx, client_id) } + fn verify_upgrade_client( + &self, + ctx: &V, + client_id: ClientId, + upgraded_client_state: Any, + upgraded_consensus_state: Any, + proof_upgrade_client: CommitmentProofBytes, + proof_upgrade_consensus_state: CommitmentProofBytes, + ) -> Result<(), ClientError> { + verify_upgrade_client::( + self.inner(), + ctx, + client_id, + upgraded_client_state, + upgraded_consensus_state, + proof_upgrade_client, + proof_upgrade_consensus_state, + ) + } + fn check_substitute(&self, _ctx: &V, substitute_client_state: Any) -> Result<(), ClientError> { check_substitute::(self.inner(), substitute_client_state) } @@ -229,6 +257,99 @@ where Ok(Status::Active) } +/// 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. +/// +/// You can learn more about how to upgrade IBC-connected SDK chains in +/// [this](https://ibc.cosmos.network/main/ibc/upgrades/quick-guide.html) +/// guide. +/// +/// Note that this function is typically implemented as part of the +/// [`ClientStateCommon`] trait, but has been made a standalone function +/// in order to make the ClientState APIs more flexible. +pub fn verify_upgrade_client( + client_state: &ClientStateType, + ctx: &V, + client_id: ClientId, + upgraded_client_state: Any, + upgraded_consensus_state: Any, + proof_upgrade_client: CommitmentProofBytes, + proof_upgrade_consensus_state: CommitmentProofBytes, +) -> Result<(), ClientError> +where + V: ClientValidationContext, + H: HostFunctionsProvider, +{ + // Read the latest consensus state from the host chain store. + let old_client_cons_state_path = ClientConsensusStatePath::new( + client_id.clone(), + client_state.latest_height.revision_number(), + client_state.latest_height.revision_height(), + ); + let old_consensus_state = ctx + .consensus_state(&old_client_cons_state_path) + .map_err(|_| ClientError::ConsensusStateNotFound { + client_id, + height: client_state.latest_height, + })?; + + // Make sure that the client type is of Tendermint type `ClientState` + let upgraded_tm_client_state = ClientState::try_from(upgraded_client_state.clone())?; + + // Make sure that the consensus type is of Tendermint type `ConsensusState` + TmConsensusState::try_from(upgraded_consensus_state.clone())?; + + let latest_height = client_state.latest_height; + let upgraded_tm_client_state_height = upgraded_tm_client_state.latest_height(); + + // Make sure the latest height of the current client is not greater then + // the upgrade height This condition checks both the revision number and + // the height + if latest_height >= upgraded_tm_client_state_height { + Err(UpgradeClientError::LowUpgradeHeight { + upgraded_height: latest_height, + client_height: upgraded_tm_client_state_height, + })? + } + + // Check to see if the upgrade path is set + let mut upgrade_path = client_state.upgrade_path.clone(); + + if upgrade_path.pop().is_none() { + return Err(ClientError::ClientSpecific { + description: "cannot upgrade client as no upgrade path has been set".to_string(), + }); + }; + + let upgrade_path_prefix = CommitmentPrefix::try_from(upgrade_path[0].clone().into_bytes()) + .map_err(ClientError::InvalidCommitmentProof)?; + + let last_height = latest_height.revision_height(); + + // Verify the proof of the upgraded client state + verify_membership::( + &client_state.proof_specs, + &upgrade_path_prefix, + &proof_upgrade_client, + old_consensus_state.root(), + ctx.serialize_path(UpgradeClientPath::UpgradedClientState(last_height))?, + upgraded_client_state.to_vec(), + )?; + + // Verify the proof of the upgraded consensus state + verify_membership::( + &client_state.proof_specs, + &upgrade_path_prefix, + &proof_upgrade_consensus_state, + old_consensus_state.root(), + ctx.serialize_path(UpgradeClientPath::UpgradedClientConsensusState(last_height))?, + upgraded_consensus_state.to_vec(), + )?; + + Ok(()) +} + /// Check that the subject and substitute client states match as part of /// the client recovery validation step. /// diff --git a/ibc-core/ics02-client/context/src/client_state.rs b/ibc-core/ics02-client/context/src/client_state.rs index 974d9b43b..8a53c9d8b 100644 --- a/ibc-core/ics02-client/context/src/client_state.rs +++ b/ibc-core/ics02-client/context/src/client_state.rs @@ -6,7 +6,7 @@ use ibc_core_commitment_types::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, }; use ibc_core_host_types::identifiers::{ClientId, ClientType}; -use ibc_core_host_types::path::Path; +use ibc_core_host_types::path::PathBytes; use ibc_primitives::prelude::*; use ibc_primitives::proto::Any; @@ -33,25 +33,6 @@ pub trait ClientStateCommon: Convertible { /// Validate that the client is at a sufficient height fn validate_proof_height(&self, proof_height: Height) -> Result<(), ClientError>; - /// Verify the upgraded client and consensus states and validate proofs - /// against the given root. - /// - /// NOTE: proof heights are not included, as upgrade to a new revision is - /// expected to pass only on the last height committed by the current - /// revision. Clients are responsible for ensuring that the planned last - /// height of the current revision is somehow encoded in the proof - /// verification process. This is to ensure that no premature upgrades - /// occur, since upgrade plans committed to by the counterparty may be - /// cancelled or modified before the last planned height. - fn verify_upgrade_client( - &self, - upgraded_client_state: Any, - upgraded_consensus_state: Any, - proof_upgrade_client: CommitmentProofBytes, - proof_upgrade_consensus_state: CommitmentProofBytes, - root: &CommitmentRoot, - ) -> Result<(), ClientError>; - // Verify_membership is a generic proof verification method which verifies a // proof of the existence of a value at a given Path. fn verify_membership( @@ -59,7 +40,7 @@ pub trait ClientStateCommon: Convertible { prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, - path: Path, + path: PathBytes, value: Vec, ) -> Result<(), ClientError>; @@ -70,7 +51,7 @@ pub trait ClientStateCommon: Convertible { prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, - path: Path, + path: PathBytes, ) -> Result<(), ClientError>; } @@ -122,6 +103,26 @@ where /// Returns the status of the client. Only Active clients are allowed to process packets. fn status(&self, ctx: &V, client_id: &ClientId) -> Result; + /// Verify the upgraded client and consensus states and validate proofs + /// against the given root. + /// + /// NOTE: proof heights are not included, as upgrade to a new revision is + /// expected to pass only on the last height committed by the current + /// revision. Clients are responsible for ensuring that the planned last + /// height of the current revision is somehow encoded in the proof + /// verification process. This is to ensure that no premature upgrades + /// occur, since upgrade plans committed to by the counterparty may be + /// cancelled or modified before the last planned height. + fn verify_upgrade_client( + &self, + ctx: &V, + client_id: ClientId, + upgraded_client_state: Any, + upgraded_consensus_state: Any, + proof_upgrade_client: CommitmentProofBytes, + proof_upgrade_consensus_state: CommitmentProofBytes, + ) -> Result<(), ClientError>; + /// Verifies whether the calling (subject) client state matches the substitute /// client state for the purposes of client recovery. /// diff --git a/ibc-core/ics02-client/context/src/context.rs b/ibc-core/ics02-client/context/src/context.rs index 972ec547e..8c44e7eec 100644 --- a/ibc-core/ics02-client/context/src/context.rs +++ b/ibc-core/ics02-client/context/src/context.rs @@ -1,7 +1,7 @@ use ibc_core_client_types::Height; use ibc_core_handler_types::error::ContextError; use ibc_core_host_types::identifiers::ClientId; -use ibc_core_host_types::path::{ClientConsensusStatePath, ClientStatePath}; +use ibc_core_host_types::path::{ClientConsensusStatePath, ClientStatePath, Path, PathBytes}; use ibc_primitives::prelude::*; use ibc_primitives::Timestamp; @@ -39,6 +39,10 @@ pub trait ClientValidationContext: Sized { client_id: &ClientId, height: &Height, ) -> Result<(Timestamp, Height), ContextError>; + + fn serialize_path(&self, path: impl Into) -> Result { + Ok(path.into().to_string().into_bytes().into()) + } } /// Defines the methods that all client `ExecutionContext`s (precisely the diff --git a/ibc-core/ics02-client/src/handler/upgrade_client.rs b/ibc-core/ics02-client/src/handler/upgrade_client.rs index a072988a2..7c45edff4 100644 --- a/ibc-core/ics02-client/src/handler/upgrade_client.rs +++ b/ibc-core/ics02-client/src/handler/upgrade_client.rs @@ -1,12 +1,10 @@ //! Protocol logic specific to processing ICS2 messages of type `MsgUpgradeAnyClient`. //! use ibc_core_client_context::prelude::*; -use ibc_core_client_types::error::ClientError; use ibc_core_client_types::events::UpgradeClient; use ibc_core_client_types::msgs::MsgUpgradeClient; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_core_host::types::path::ClientConsensusStatePath; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_primitives::prelude::*; @@ -30,26 +28,14 @@ where .status(client_val_ctx, &client_id)? .verify_is_active()?; - // Read the latest consensus state from the host chain store. - let old_client_cons_state_path = ClientConsensusStatePath::new( - client_id.clone(), - old_client_state.latest_height().revision_number(), - old_client_state.latest_height().revision_height(), - ); - let old_consensus_state = client_val_ctx - .consensus_state(&old_client_cons_state_path) - .map_err(|_| ClientError::ConsensusStateNotFound { - client_id, - height: old_client_state.latest_height(), - })?; - // Validate the upgraded client state and consensus state and verify proofs against the root old_client_state.verify_upgrade_client( + client_val_ctx, + client_id, msg.upgraded_client_state.clone(), msg.upgraded_consensus_state, msg.proof_upgrade_client, msg.proof_upgrade_consensus_state, - old_consensus_state.root(), )?; Ok(()) diff --git a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs index 443d32877..eb626eaf1 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs @@ -84,6 +84,9 @@ where let prefix_on_b = vars.conn_end_on_a.counterparty().prefix(); { + let path_bytes = + client_val_ctx_a.serialize_path(ConnectionPath::new(&msg.conn_id_on_b))?; + let expected_conn_end_on_b = ConnectionEnd::new( State::TryOpen, vars.client_id_on_b().clone(), @@ -101,18 +104,21 @@ where prefix_on_b, &msg.proof_conn_end_on_b, consensus_state_of_b_on_a.root(), - Path::Connection(ConnectionPath::new(&msg.conn_id_on_b)), + path_bytes, expected_conn_end_on_b.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; } + let path_bytes = + client_val_ctx_a.serialize_path(ClientStatePath::new(vars.client_id_on_b().clone()))?; + client_state_of_b_on_a .verify_membership( prefix_on_b, &msg.proof_client_state_of_a_on_b, consensus_state_of_b_on_a.root(), - Path::ClientState(ClientStatePath::new(vars.client_id_on_b().clone())), + path_bytes, msg.client_state_of_a_on_b.to_vec(), ) .map_err(|e| ConnectionError::ClientStateVerificationFailure { @@ -132,12 +138,15 @@ where msg.consensus_height_of_a_on_b.revision_height(), ); + let path_bytes = client_val_ctx_a + .serialize_path(Path::ClientConsensusState(client_cons_state_path_on_b))?; + client_state_of_b_on_a .verify_membership( prefix_on_b, &msg.proof_consensus_state_of_a_on_b, consensus_state_of_b_on_a.root(), - Path::ClientConsensusState(client_cons_state_path_on_b), + path_bytes, stored_consensus_state_of_a_on_b.to_vec(), ) .map_err(|e| ConnectionError::ConsensusStateVerificationFailure { diff --git a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs index f9ef4eb61..8e5428013 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs @@ -8,7 +8,7 @@ use ibc_core_connection_types::{ConnectionEnd, Counterparty, State}; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::identifiers::{ClientId, ConnectionId}; -use ibc_core_host::types::path::{ClientConsensusStatePath, ConnectionPath, Path}; +use ibc_core_host::types::path::{ClientConsensusStatePath, ConnectionPath}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_primitives::prelude::*; use ibc_primitives::proto::Protobuf; @@ -61,6 +61,8 @@ where let prefix_on_a = conn_end_on_b.counterparty().prefix(); let prefix_on_b = ctx_b.commitment_prefix(); + let path_bytes = client_val_ctx_b.serialize_path(ConnectionPath::new(conn_id_on_a))?; + let expected_conn_end_on_a = ConnectionEnd::new( State::Open, client_id_on_a.clone(), @@ -78,7 +80,7 @@ where prefix_on_a, &msg.proof_conn_end_on_a, consensus_state_of_a_on_b.root(), - Path::Connection(ConnectionPath::new(conn_id_on_a)), + path_bytes, expected_conn_end_on_a.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; diff --git a/ibc-core/ics03-connection/src/handler/conn_open_try.rs b/ibc-core/ics03-connection/src/handler/conn_open_try.rs index 4b59913e7..2ca2fe705 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_try.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_try.rs @@ -9,7 +9,7 @@ use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::identifiers::{ClientId, ConnectionId}; use ibc_core_host::types::path::{ - ClientConnectionPath, ClientConsensusStatePath, ClientStatePath, ConnectionPath, Path, + ClientConnectionPath, ClientConsensusStatePath, ClientStatePath, ConnectionPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_primitives::prelude::*; @@ -84,6 +84,9 @@ where let prefix_on_b = ctx_b.commitment_prefix(); { + let path_bytes = + client_val_ctx_b.serialize_path(ConnectionPath::new(&vars.conn_id_on_a))?; + let expected_conn_end_on_a = ConnectionEnd::new( State::Init, client_id_on_a.clone(), @@ -97,18 +100,21 @@ where prefix_on_a, &msg.proof_conn_end_on_a, consensus_state_of_a_on_b.root(), - Path::Connection(ConnectionPath::new(&vars.conn_id_on_a)), + path_bytes, expected_conn_end_on_a.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; } + let path_bytes = + client_val_ctx_b.serialize_path(ClientStatePath::new(client_id_on_a.clone()))?; + client_state_of_a_on_b .verify_membership( prefix_on_a, &msg.proof_client_state_of_b_on_a, consensus_state_of_a_on_b.root(), - Path::ClientState(ClientStatePath::new(client_id_on_a.clone())), + path_bytes, msg.client_state_of_b_on_a.to_vec(), ) .map_err(|e| ConnectionError::ClientStateVerificationFailure { @@ -128,12 +134,14 @@ where msg.consensus_height_of_b_on_a.revision_height(), ); + let path_bytes = client_val_ctx_b.serialize_path(client_cons_state_path_on_a)?; + client_state_of_a_on_b .verify_membership( prefix_on_a, &msg.proof_consensus_state_of_b_on_a, consensus_state_of_a_on_b.root(), - Path::ClientConsensusState(client_cons_state_path_on_a), + path_bytes, stored_consensus_state_of_b_on_a.to_vec(), ) .map_err(|e| ConnectionError::ConsensusStateVerificationFailure { diff --git a/ibc-core/ics04-channel/src/handler/acknowledgement.rs b/ibc-core/ics04-channel/src/handler/acknowledgement.rs index c58edc74a..d3d6994ab 100644 --- a/ibc-core/ics04-channel/src/handler/acknowledgement.rs +++ b/ibc-core/ics04-channel/src/handler/acknowledgement.rs @@ -9,7 +9,7 @@ use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::path::{ - AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, SeqAckPath, + AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, SeqAckPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; @@ -188,6 +188,8 @@ where let ack_path_on_b = AckPath::new(&packet.port_id_on_b, &packet.chan_id_on_b, packet.seq_on_a); + let path_bytes = client_val_ctx_a.serialize_path(ack_path_on_b)?; + verify_conn_delay_passed(ctx_a, msg.proof_height_on_b, &conn_end_on_a)?; // Verify the proof for the packet against the chain store. @@ -196,7 +198,7 @@ where conn_end_on_a.counterparty().prefix(), &msg.proof_acked_on_b, consensus_state_of_b_on_a.root(), - Path::Ack(ack_path_on_b), + path_bytes, ack_commitment.into_vec(), ) .map_err(|e| ChannelError::PacketVerificationFailed { diff --git a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs index 5385d18c3..fda78eab2 100644 --- a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs @@ -8,7 +8,7 @@ use ibc_core_client::context::prelude::*; use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath, Path}; +use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; @@ -149,6 +149,8 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, chan_id_on_a); + let path_bytes = client_val_ctx_b.serialize_path(chan_end_path_on_a)?; + // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. client_state_of_a_on_b @@ -156,7 +158,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - Path::ChannelEnd(chan_end_path_on_a), + path_bytes, expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs index 7e5b6b240..2855d95cf 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs @@ -7,7 +7,7 @@ use ibc_core_client::context::prelude::*; use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath, Path}; +use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; @@ -142,6 +142,8 @@ where )?; let chan_end_path_on_b = ChannelEndPath::new(port_id_on_b, &msg.chan_id_on_b); + let path_bytes = client_val_ctx_a.serialize_path(chan_end_path_on_b)?; + // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. client_state_of_b_on_a @@ -149,7 +151,7 @@ where prefix_on_b, &msg.proof_chan_end_on_b, consensus_state_of_b_on_a.root(), - Path::ChannelEnd(chan_end_path_on_b), + path_bytes, expected_chan_end_on_b.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs index dfbc777c1..7e96d96b9 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs @@ -8,7 +8,7 @@ use ibc_core_client::context::prelude::*; use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath, Path}; +use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; @@ -149,6 +149,8 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, chan_id_on_a); + let path_bytes = client_val_ctx_b.serialize_path(chan_end_path_on_a)?; + // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked in msg. client_state_of_a_on_b @@ -156,7 +158,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - Path::ChannelEnd(chan_end_path_on_a), + path_bytes, expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_try.rs b/ibc-core/ics04-channel/src/handler/chan_open_try.rs index 08c82deac..d985b3e84 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_try.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_try.rs @@ -10,7 +10,7 @@ use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::identifiers::ChannelId; use ibc_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, Path, SeqAckPath, SeqRecvPath, SeqSendPath, + ChannelEndPath, ClientConsensusStatePath, SeqAckPath, SeqRecvPath, SeqSendPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; @@ -168,6 +168,8 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(&port_id_on_a, &chan_id_on_a); + let path_bytes = client_val_ctx_b.serialize_path(chan_end_path_on_a)?; + // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. client_state_of_a_on_b @@ -175,7 +177,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - Path::ChannelEnd(chan_end_path_on_a), + path_bytes, expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs index 8c0cd25a1..10522db89 100644 --- a/ibc-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-core/ics04-channel/src/handler/recv_packet.rs @@ -10,8 +10,7 @@ use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::path::{ - AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, - SeqRecvPath, + AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, ReceiptPath, SeqRecvPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; @@ -207,6 +206,8 @@ where msg.packet.seq_on_a, ); + let path_bytes = client_val_ctx_b.serialize_path(commitment_path_on_a)?; + verify_conn_delay_passed(ctx_b, msg.proof_height_on_a, &conn_end_on_b)?; // Verify the proof for the packet against the chain store. @@ -215,7 +216,7 @@ where conn_end_on_b.counterparty().prefix(), &msg.proof_commitment_on_a, consensus_state_of_a_on_b.root(), - Path::Commitment(commitment_path_on_a), + path_bytes, expected_commitment_on_a.into_vec(), ) .map_err(|e| ChannelError::PacketVerificationFailed { diff --git a/ibc-core/ics04-channel/src/handler/timeout.rs b/ibc-core/ics04-channel/src/handler/timeout.rs index ef478ed4e..3f0d12856 100644 --- a/ibc-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-core/ics04-channel/src/handler/timeout.rs @@ -8,7 +8,7 @@ use ibc_core_connection::delay::verify_conn_delay_passed; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, SeqRecvPath, + ChannelEndPath, ClientConsensusStatePath, CommitmentPath, ReceiptPath, SeqRecvPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; @@ -221,11 +221,13 @@ where let seq_recv_path_on_b = SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b); + let path_bytes = client_val_ctx_a.serialize_path(seq_recv_path_on_b)?; + client_state_of_b_on_a.verify_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - Path::SeqRecv(seq_recv_path_on_b), + path_bytes, msg.packet.seq_on_a.to_vec(), ) } @@ -236,11 +238,13 @@ where msg.packet.seq_on_a, ); + let path_bytes = client_val_ctx_a.serialize_path(receipt_path_on_b)?; + client_state_of_b_on_a.verify_non_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - Path::Receipt(receipt_path_on_b), + path_bytes, ) } Order::None => { diff --git a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs index 8366a5d26..fefb7117f 100644 --- a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs +++ b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs @@ -6,7 +6,7 @@ use ibc_core_client::context::prelude::*; use ibc_core_connection::delay::verify_conn_delay_passed; use ibc_core_handler_types::error::ContextError; use ibc_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, SeqRecvPath, + ChannelEndPath, ClientConsensusStatePath, CommitmentPath, ReceiptPath, SeqRecvPath, }; use ibc_core_host::ValidationContext; use ibc_primitives::prelude::*; @@ -104,6 +104,8 @@ where let chan_end_path_on_b = ChannelEndPath(port_id_on_b, chan_id_on_b.clone()); + let path_bytes = client_val_ctx_a.serialize_path(chan_end_path_on_b)?; + // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. client_state_of_b_on_a @@ -111,7 +113,7 @@ where prefix_on_b, &msg.proof_close_on_b, consensus_state_of_b_on_a.root(), - Path::ChannelEnd(chan_end_path_on_b), + path_bytes, expected_chan_end_on_b.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed) @@ -131,11 +133,13 @@ where let seq_recv_path_on_b = SeqRecvPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); + let path_bytes = client_val_ctx_a.serialize_path(seq_recv_path_on_b)?; + client_state_of_b_on_a.verify_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - Path::SeqRecv(seq_recv_path_on_b), + path_bytes, packet.seq_on_a.to_vec(), ) } @@ -146,11 +150,13 @@ where msg.packet.seq_on_a, ); + let path_bytes = client_val_ctx_a.serialize_path(receipt_path_on_b)?; + client_state_of_b_on_a.verify_non_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - Path::Receipt(receipt_path_on_b), + path_bytes, ) } Order::None => { diff --git a/ibc-core/ics23-commitment/types/Cargo.toml b/ibc-core/ics23-commitment/types/Cargo.toml index 27ce743c6..c1ce47b36 100644 --- a/ibc-core/ics23-commitment/types/Cargo.toml +++ b/ibc-core/ics23-commitment/types/Cargo.toml @@ -28,9 +28,10 @@ serde = { workspace = true, optional = true } subtle-encoding = { workspace = true } # ibc dependencies -ibc-proto = { workspace = true } -ibc-primitives = { workspace = true } -ics23 = { version = "0.11", default-features = false, features = [ "host-functions" ] } +ibc-proto = { workspace = true } +ibc-primitives = { workspace = true } +ics23 = { version = "0.11", default-features = false, features = [ "host-functions" ] } +ibc-core-host-types = { workspace = true } # parity dependencies parity-scale-codec = { workspace = true, optional = true } @@ -46,12 +47,14 @@ std = [ "serde/std", "subtle-encoding/std", "ibc-primitives/std", + "ibc-core-host-types/std", "ibc-proto/std", "ics23/std", ] serde = [ "dep:serde", "ibc-primitives/serde", + "ibc-core-host-types/serde", "ibc-proto/serde", "ics23/serde", ] @@ -59,6 +62,7 @@ schema = [ "dep:schemars", "ibc-proto/json-schema", "ibc-primitives/schema", + "ibc-core-host-types/schema", "serde", "std", ] @@ -66,10 +70,12 @@ borsh = [ "dep:borsh", "ibc-proto/borsh", "ibc-primitives/borsh", + "ibc-core-host-types/borsh", ] parity-scale-codec = [ "dep:parity-scale-codec", "dep:scale-info", "ibc-primitives/parity-scale-codec", + "ibc-core-host-types/parity-scale-codec", "ibc-proto/parity-scale-codec", ] diff --git a/ibc-core/ics23-commitment/types/src/merkle.rs b/ibc-core/ics23-commitment/types/src/merkle.rs index cb36590ca..1580cef18 100644 --- a/ibc-core/ics23-commitment/types/src/merkle.rs +++ b/ibc-core/ics23-commitment/types/src/merkle.rs @@ -1,22 +1,44 @@ //! Merkle proof utilities +use ibc_core_host_types::path::PathBytes; use ibc_primitives::prelude::*; use ibc_primitives::proto::Protobuf; -use ibc_proto::ibc::core::commitment::v1::{MerklePath, MerkleProof as RawMerkleProof, MerkleRoot}; +use ibc_proto::ibc::core::commitment::v1::{ + MerklePath as RawMerklePath, MerkleProof as RawMerkleProof, MerkleRoot, +}; use ibc_proto::ics23::commitment_proof::Proof; use ibc_proto::ics23::{ calculate_existence_root, verify_membership, verify_non_membership, CommitmentProof, HostFunctionsProvider, NonExistenceProof, }; -use crate::commitment::{CommitmentPrefix, CommitmentRoot}; +use crate::commitment::CommitmentRoot; use crate::error::CommitmentError; use crate::specs::ProofSpecs; -pub fn apply_prefix(prefix: &CommitmentPrefix, mut path: Vec) -> MerklePath { - let mut key_path: Vec = vec![format!("{prefix:?}")]; - key_path.append(&mut path); - MerklePath { key_path } +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Clone, Debug, PartialEq)] +pub struct MerklePath { + pub key_path: Vec, +} + +impl MerklePath { + pub fn new(key_path: Vec) -> Self { + Self { key_path } + } +} + +impl From for MerklePath { + fn from(path: RawMerklePath) -> Self { + Self { + key_path: path + .key_path + .into_iter() + .map(|p| p.into_bytes().into()) + .collect(), + } + } } impl From for MerkleRoot { @@ -99,7 +121,7 @@ impl MerkleProof { subroot = calculate_existence_root::(existence_proof) .map_err(|_| CommitmentError::InvalidMerkleProof)?; - if !verify_membership::(proof, spec, &subroot, key.as_bytes(), &value) { + if !verify_membership::(proof, spec, &subroot, key.as_ref(), &value) { return Err(CommitmentError::VerificationFailure); } value.clone_from(&subroot); @@ -154,7 +176,7 @@ impl MerkleProof { Some(Proof::Nonexist(non_existence_proof)) => { let subroot = calculate_non_existence_root::(non_existence_proof)?; - if !verify_non_membership::(proof, spec, &subroot, key.as_bytes()) { + if !verify_non_membership::(proof, spec, &subroot, key.as_ref()) { return Err(CommitmentError::VerificationFailure); } diff --git a/ibc-core/ics24-host/types/src/path.rs b/ibc-core/ics24-host/types/src/path.rs index 43e99347e..7265b9546 100644 --- a/ibc-core/ics24-host/types/src/path.rs +++ b/ibc-core/ics24-host/types/src/path.rs @@ -42,6 +42,30 @@ pub const UPGRADED_CLIENT_STATE: &str = "upgradedClient"; /// - The key identifying the upgraded consensus state pub const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState"; +/// Represents a general-purpose path structure using the byte representation of +/// a path. This struct abstracts over different path types and can handle bytes +/// may obtained from various serialization formats (e.g., Protobuf, borsh). +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, From)] +pub struct PathBytes(Vec); + +impl PathBytes { + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn into_vec(self) -> Vec { + self.0 + } +} + +impl AsRef<[u8]> for PathBytes { + fn as_ref(&self) -> &[u8] { + &self.0 + } +} + /// The Path enum abstracts out the different sub-paths. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, From, Display)] pub enum Path { diff --git a/ibc-derive/src/client_state/traits/client_state_common.rs b/ibc-derive/src/client_state/traits/client_state_common.rs index bac320ff8..3284eb220 100644 --- a/ibc-derive/src/client_state/traits/client_state_common.rs +++ b/ibc-derive/src/client_state/traits/client_state_common.rs @@ -35,12 +35,6 @@ pub(crate) fn impl_ClientStateCommon( quote! {validate_proof_height(cs, proof_height)}, imports, ); - let verify_upgrade_client_impl = delegate_call_in_match( - client_state_enum_name, - enum_variants.iter(), - quote! {verify_upgrade_client(cs, upgraded_client_state, upgraded_consensus_state, proof_upgrade_client, proof_upgrade_consensus_state, root)}, - imports, - ); let verify_membership_impl = delegate_call_in_match( client_state_enum_name, enum_variants.iter(), @@ -64,7 +58,7 @@ pub(crate) fn impl_ClientStateCommon( let ClientType = imports.client_type(); let ClientError = imports.client_error(); let Height = imports.height(); - let Path = imports.path(); + let PathBytes = imports.path_bytes(); quote! { impl #ClientStateCommon for #HostClientState { @@ -91,25 +85,12 @@ pub(crate) fn impl_ClientStateCommon( } } - fn verify_upgrade_client( - &self, - upgraded_client_state: #Any, - upgraded_consensus_state: #Any, - proof_upgrade_client: #CommitmentProofBytes, - proof_upgrade_consensus_state: #CommitmentProofBytes, - root: &#CommitmentRoot, - ) -> core::result::Result<(), #ClientError> { - match self { - #(#verify_upgrade_client_impl),* - } - } - fn verify_membership( &self, prefix: &#CommitmentPrefix, proof: &#CommitmentProofBytes, root: &#CommitmentRoot, - path: #Path, + path: #PathBytes, value: Vec, ) -> core::result::Result<(), #ClientError> { match self { @@ -122,7 +103,7 @@ pub(crate) fn impl_ClientStateCommon( prefix: &#CommitmentPrefix, proof: &#CommitmentProofBytes, root: &#CommitmentRoot, - path: #Path, + path: #PathBytes, ) -> core::result::Result<(), #ClientError> { match self { #(#verify_non_membership_impl),* diff --git a/ibc-derive/src/client_state/traits/client_state_validation.rs b/ibc-derive/src/client_state/traits/client_state_validation.rs index 263411b15..9c28307aa 100644 --- a/ibc-derive/src/client_state/traits/client_state_validation.rs +++ b/ibc-derive/src/client_state/traits/client_state_validation.rs @@ -37,6 +37,14 @@ pub(crate) fn impl_ClientStateValidation( imports, ); + let verify_upgrade_client_impl = delegate_call_in_match( + client_state_enum_name, + enum_variants.iter(), + opts, + quote! {verify_upgrade_client(cs, ctx, client_id, upgraded_client_state, upgraded_consensus_state, proof_upgrade_client, proof_upgrade_consensus_state)}, + imports, + ); + let check_substitute_impl = delegate_call_in_match( client_state_enum_name, enum_variants.iter(), @@ -51,6 +59,7 @@ pub(crate) fn impl_ClientStateValidation( let ClientError = imports.client_error(); let ClientStateValidation = imports.client_state_validation(); let Status = imports.status(); + let CommitmentProofBytes = imports.commitment_proof_bytes(); // The types we need for the generated code. let HostClientState = client_state_enum_name; @@ -97,6 +106,20 @@ pub(crate) fn impl_ClientStateValidation( } } + fn verify_upgrade_client( + &self, + ctx: &#V, + client_id: #ClientId, + upgraded_client_state: #Any, + upgraded_consensus_state: #Any, + proof_upgrade_client: #CommitmentProofBytes, + proof_upgrade_consensus_state: #CommitmentProofBytes, + ) -> core::result::Result<(), #ClientError> { + match self { + #(#verify_upgrade_client_impl),* + } + } + fn check_substitute( &self, ctx: &#V, diff --git a/ibc-derive/src/utils.rs b/ibc-derive/src/utils.rs index ab3986951..82cf91335 100644 --- a/ibc-derive/src/utils.rs +++ b/ibc-derive/src/utils.rs @@ -45,9 +45,9 @@ impl Imports { quote! {#Prefix::commitment_types::commitment::CommitmentProofBytes} } - pub fn path(&self) -> TokenStream { + pub fn path_bytes(&self) -> TokenStream { let Prefix = self.prefix(); - quote! {#Prefix::host::types::path::Path} + quote! {#Prefix::host::types::path::PathBytes} } pub fn consensus_state(&self) -> TokenStream { diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs index 4352ac22f..afef24949 100644 --- a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs +++ b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs @@ -9,7 +9,7 @@ use ibc::core::commitment_types::commitment::{ }; use ibc::core::handler::types::error::ContextError; use ibc::core::host::types::identifiers::{ClientId, ClientType}; -use ibc::core::host::types::path::{ClientConsensusStatePath, ClientStatePath, Path}; +use ibc::core::host::types::path::{ClientConsensusStatePath, ClientStatePath, PathBytes}; use ibc::core::primitives::prelude::*; use ibc::core::primitives::Timestamp; use ibc::primitives::proto::{Any, Protobuf}; @@ -184,31 +184,12 @@ impl ClientStateCommon for MockClientState { Ok(()) } - fn verify_upgrade_client( - &self, - upgraded_client_state: Any, - upgraded_consensus_state: Any, - _proof_upgrade_client: CommitmentProofBytes, - _proof_upgrade_consensus_state: CommitmentProofBytes, - _root: &CommitmentRoot, - ) -> Result<(), ClientError> { - let upgraded_mock_client_state = Self::try_from(upgraded_client_state)?; - MockConsensusState::try_from(upgraded_consensus_state)?; - if self.latest_height() >= upgraded_mock_client_state.latest_height() { - return Err(UpgradeClientError::LowUpgradeHeight { - upgraded_height: self.latest_height(), - client_height: upgraded_mock_client_state.latest_height(), - })?; - } - Ok(()) - } - fn verify_membership( &self, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _path: Path, + _path: PathBytes, _value: Vec, ) -> Result<(), ClientError> { Ok(()) @@ -219,7 +200,7 @@ impl ClientStateCommon for MockClientState { _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _path: Path, + _path: PathBytes, ) -> Result<(), ClientError> { Ok(()) } @@ -306,6 +287,26 @@ where Ok(Status::Active) } + fn verify_upgrade_client( + &self, + _ctx: &V, + _client_id: ClientId, + upgraded_client_state: Any, + _upgraded_consensus_state: Any, + _proof_upgrade_client: CommitmentProofBytes, + _proof_upgrade_consensus_state: CommitmentProofBytes, + ) -> Result<(), ClientError> { + let upgraded_mock_client_state = Self::try_from(upgraded_client_state)?; + // MockConsensusState::try_from(upgraded_consensus_state)?; + if self.latest_height() >= upgraded_mock_client_state.latest_height() { + return Err(UpgradeClientError::LowUpgradeHeight { + upgraded_height: self.latest_height(), + client_height: upgraded_mock_client_state.latest_height(), + })?; + } + Ok(()) + } + fn check_substitute(&self, _ctx: &V, _substitute_client_state: Any) -> Result<(), ClientError> { Ok(()) } diff --git a/tests-integration/tests/core/ics02_client/create_client.rs b/tests-integration/tests/core/ics02_client/create_client.rs index 4e5dd9e77..ec284f27d 100644 --- a/tests-integration/tests/core/ics02_client/create_client.rs +++ b/tests-integration/tests/core/ics02_client/create_client.rs @@ -195,7 +195,9 @@ fn test_tm_create_client_proof_verification_ok() { &ctx_tm.ibc_store().commitment_prefix(), &proof, &root, - next_client_seq_path.clone().into(), + client_validation_ctx_mk + .serialize_path(next_client_seq_path.clone()) + .expect("path"), serde_json::to_vec(&next_client_seq_value).expect("valid json serialization"), ) .expect("successful proof verification"); @@ -207,7 +209,9 @@ fn test_tm_create_client_proof_verification_ok() { &ctx_tm.ibc_store().commitment_prefix(), &proof, &root, - next_client_seq_path.into(), + client_validation_ctx_mk + .serialize_path(next_client_seq_path.clone()) + .expect("path"), serde_json::to_vec(&(next_client_seq_value + 1)).expect("valid json serialization"), ) .expect_err("proof verification fails"), From 1f68e84d297293eeea6a170df1d91880a5b9e3de Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 10 Jul 2024 07:34:04 -0700 Subject: [PATCH 02/13] imp: 2nd try, place serialize_path under the ClientStateCommon --- ibc-clients/cw-context/src/handlers.rs | 11 +- .../src/client_state/common.rs | 113 ++++++++++++++- .../src/client_state/validation.rs | 131 +----------------- .../ics02-client/context/src/client_state.rs | 44 +++--- ibc-core/ics02-client/context/src/context.rs | 6 +- .../src/handler/upgrade_client.rs | 18 ++- .../src/handler/conn_open_ack.rs | 12 +- .../src/handler/conn_open_confirm.rs | 5 +- .../src/handler/conn_open_try.rs | 10 +- .../src/handler/acknowledgement.rs | 4 +- .../src/handler/chan_close_confirm.rs | 2 +- .../src/handler/chan_open_ack.rs | 2 +- .../src/handler/chan_open_confirm.rs | 2 +- .../src/handler/chan_open_try.rs | 2 +- .../ics04-channel/src/handler/recv_packet.rs | 4 +- ibc-core/ics04-channel/src/handler/timeout.rs | 4 +- .../src/handler/timeout_on_close.rs | 6 +- .../traits/client_state_common.rs | 32 +++++ .../traits/client_state_validation.rs | 23 --- ibc-derive/src/utils.rs | 5 + .../testapp/ibc/clients/mock/client_state.rs | 45 +++--- .../tests/core/ics02_client/create_client.rs | 4 +- 22 files changed, 255 insertions(+), 230 deletions(-) diff --git a/ibc-clients/cw-context/src/handlers.rs b/ibc-clients/cw-context/src/handlers.rs index 3ad414231..4b4798d1e 100644 --- a/ibc-clients/cw-context/src/handlers.rs +++ b/ibc-clients/cw-context/src/handlers.rs @@ -103,13 +103,20 @@ where SudoMsg::VerifyUpgradeAndUpdateState(msg) => { let msg = VerifyUpgradeAndUpdateStateMsg::try_from(msg)?; - client_state.verify_upgrade_client( - self, + let client_cons_state_path = ClientConsensusStatePath::new( client_id.clone(), + client_state.latest_height().revision_number(), + client_state.latest_height().revision_height(), + ); + + let consensus_state = self.consensus_state(&client_cons_state_path)?; + + client_state.verify_upgrade_client( msg.upgrade_client_state.clone(), msg.upgrade_consensus_state.clone(), msg.proof_upgrade_client, msg.proof_upgrade_consensus_state, + consensus_state.root(), )?; client_state.update_state_on_upgrade( diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs index aee889214..bfadd70c4 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/common.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/common.rs @@ -1,7 +1,7 @@ use ibc_client_tendermint_types::{client_type as tm_client_type, ClientState as ClientStateType}; use ibc_core_client::context::client_state::ClientStateCommon; use ibc_core_client::context::consensus_state::ConsensusState; -use ibc_core_client::types::error::ClientError; +use ibc_core_client::types::error::{ClientError, UpgradeClientError}; use ibc_core_client::types::Height; use ibc_core_commitment_types::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, @@ -10,9 +10,10 @@ use ibc_core_commitment_types::merkle::{MerklePath, MerkleProof}; use ibc_core_commitment_types::proto::ics23::{HostFunctionsManager, HostFunctionsProvider}; use ibc_core_commitment_types::specs::ProofSpecs; use ibc_core_host::types::identifiers::ClientType; -use ibc_core_host::types::path::PathBytes; +use ibc_core_host::types::path::{Path, PathBytes, UpgradeClientPath}; use ibc_primitives::prelude::*; use ibc_primitives::proto::Any; +use ibc_primitives::ToVec; use super::ClientState; use crate::consensus_state::ConsensusState as TmConsensusState; @@ -34,6 +35,38 @@ impl ClientStateCommon for ClientState { validate_proof_height(self.inner(), proof_height) } + fn verify_upgrade_client( + &self, + upgraded_client_state: Any, + upgraded_consensus_state: Any, + proof_upgrade_client: CommitmentProofBytes, + proof_upgrade_consensus_state: CommitmentProofBytes, + root: &CommitmentRoot, + ) -> Result<(), ClientError> { + let last_height = self.latest_height().revision_height(); + + let upgrade_client_path_bytes = + self.serialize_path(UpgradeClientPath::UpgradedClientState(last_height))?; + + let upgrade_consensus_path_bytes = + self.serialize_path(UpgradeClientPath::UpgradedClientConsensusState(last_height))?; + + verify_upgrade_client::( + self.inner(), + upgraded_client_state, + upgraded_consensus_state, + proof_upgrade_client, + proof_upgrade_consensus_state, + upgrade_client_path_bytes, + upgrade_consensus_path_bytes, + root, + ) + } + + fn serialize_path(&self, path: impl Into) -> Result { + Ok(path.into().to_string().into_bytes().into()) + } + fn verify_membership( &self, prefix: &CommitmentPrefix, @@ -109,6 +142,82 @@ pub fn validate_proof_height( Ok(()) } +/// 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. +/// +/// You can learn more about how to upgrade IBC-connected SDK chains in +/// [this](https://ibc.cosmos.network/main/ibc/upgrades/quick-guide.html) +/// guide. +/// +/// Note that this function is typically implemented as part of the +/// [`ClientStateCommon`] trait, but has been made a standalone function +/// in order to make the ClientState APIs more flexible. +#[allow(clippy::too_many_arguments)] +pub fn verify_upgrade_client( + client_state: &ClientStateType, + upgraded_client_state: Any, + upgraded_consensus_state: Any, + proof_upgrade_client: CommitmentProofBytes, + proof_upgrade_consensus_state: CommitmentProofBytes, + upgrade_client_path_bytes: PathBytes, + upgrade_consensus_path_bytes: PathBytes, + root: &CommitmentRoot, +) -> Result<(), ClientError> { + // Make sure that the client type is of Tendermint type `ClientState` + let upgraded_tm_client_state = ClientState::try_from(upgraded_client_state.clone())?; + + // Make sure that the consensus type is of Tendermint type `ConsensusState` + TmConsensusState::try_from(upgraded_consensus_state.clone())?; + + let latest_height = client_state.latest_height; + let upgraded_tm_client_state_height = upgraded_tm_client_state.latest_height(); + + // Make sure the latest height of the current client is not greater then + // the upgrade height This condition checks both the revision number and + // the height + if latest_height >= upgraded_tm_client_state_height { + Err(UpgradeClientError::LowUpgradeHeight { + upgraded_height: latest_height, + client_height: upgraded_tm_client_state_height, + })? + } + + // Check to see if the upgrade path is set + let mut upgrade_path = client_state.upgrade_path.clone(); + + if upgrade_path.pop().is_none() { + return Err(ClientError::ClientSpecific { + description: "cannot upgrade client as no upgrade path has been set".to_string(), + }); + }; + + let upgrade_path_prefix = CommitmentPrefix::try_from(upgrade_path[0].clone().into_bytes()) + .map_err(ClientError::InvalidCommitmentProof)?; + + // Verify the proof of the upgraded client state + verify_membership::( + &client_state.proof_specs, + &upgrade_path_prefix, + &proof_upgrade_client, + root, + upgrade_client_path_bytes, + upgraded_client_state.to_vec(), + )?; + + // Verify the proof of the upgraded consensus state + verify_membership::( + &client_state.proof_specs, + &upgrade_path_prefix, + &proof_upgrade_consensus_state, + root, + upgrade_consensus_path_bytes, + upgraded_consensus_state.to_vec(), + )?; + + Ok(()) +} + /// Verify membership of the given value against the client's merkle proof. /// /// Note that this function is typically implemented as part of the diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index 49576eeca..116b84dd8 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -2,29 +2,21 @@ use ibc_client_tendermint_types::{ ClientState as ClientStateType, ConsensusState as ConsensusStateType, Header as TmHeader, Misbehaviour as TmMisbehaviour, TENDERMINT_HEADER_TYPE_URL, TENDERMINT_MISBEHAVIOUR_TYPE_URL, }; -use ibc_core_client::context::client_state::{ClientStateCommon, ClientStateValidation}; -use ibc_core_client::context::consensus_state::ConsensusState; -use ibc_core_client::context::{ClientValidationContext, Convertible, ExtClientValidationContext}; -use ibc_core_client::types::error::{ClientError, UpgradeClientError}; +use ibc_core_client::context::client_state::ClientStateValidation; +use ibc_core_client::context::{Convertible, ExtClientValidationContext}; +use ibc_core_client::types::error::ClientError; use ibc_core_client::types::Status; -use ibc_core_commitment_types::commitment::{CommitmentPrefix, CommitmentProofBytes}; -use ibc_core_commitment_types::proto::ics23::{HostFunctionsManager, HostFunctionsProvider}; use ibc_core_host::types::identifiers::ClientId; -use ibc_core_host::types::path::{ClientConsensusStatePath, UpgradeClientPath}; +use ibc_core_host::types::path::ClientConsensusStatePath; use ibc_primitives::prelude::*; use ibc_primitives::proto::Any; -use ibc_primitives::ToVec; use tendermint::crypto::default::Sha256; use tendermint::crypto::Sha256 as Sha256Trait; use tendermint::merkle::MerkleHash; use tendermint_light_client_verifier::{ProdVerifier, Verifier}; -use super::{ - check_for_misbehaviour_on_misbehavior, check_for_misbehaviour_on_update, verify_membership, - ClientState, -}; +use super::{check_for_misbehaviour_on_misbehavior, check_for_misbehaviour_on_update, ClientState}; use crate::client_state::{verify_header, verify_misbehaviour}; -use crate::consensus_state::ConsensusState as TmConsensusState; impl ClientStateValidation for ClientState where @@ -79,26 +71,6 @@ where status(self.inner(), ctx, client_id) } - fn verify_upgrade_client( - &self, - ctx: &V, - client_id: ClientId, - upgraded_client_state: Any, - upgraded_consensus_state: Any, - proof_upgrade_client: CommitmentProofBytes, - proof_upgrade_consensus_state: CommitmentProofBytes, - ) -> Result<(), ClientError> { - verify_upgrade_client::( - self.inner(), - ctx, - client_id, - upgraded_client_state, - upgraded_consensus_state, - proof_upgrade_client, - proof_upgrade_consensus_state, - ) - } - fn check_substitute(&self, _ctx: &V, substitute_client_state: Any) -> Result<(), ClientError> { check_substitute::(self.inner(), substitute_client_state) } @@ -257,99 +229,6 @@ where Ok(Status::Active) } -/// 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. -/// -/// You can learn more about how to upgrade IBC-connected SDK chains in -/// [this](https://ibc.cosmos.network/main/ibc/upgrades/quick-guide.html) -/// guide. -/// -/// Note that this function is typically implemented as part of the -/// [`ClientStateCommon`] trait, but has been made a standalone function -/// in order to make the ClientState APIs more flexible. -pub fn verify_upgrade_client( - client_state: &ClientStateType, - ctx: &V, - client_id: ClientId, - upgraded_client_state: Any, - upgraded_consensus_state: Any, - proof_upgrade_client: CommitmentProofBytes, - proof_upgrade_consensus_state: CommitmentProofBytes, -) -> Result<(), ClientError> -where - V: ClientValidationContext, - H: HostFunctionsProvider, -{ - // Read the latest consensus state from the host chain store. - let old_client_cons_state_path = ClientConsensusStatePath::new( - client_id.clone(), - client_state.latest_height.revision_number(), - client_state.latest_height.revision_height(), - ); - let old_consensus_state = ctx - .consensus_state(&old_client_cons_state_path) - .map_err(|_| ClientError::ConsensusStateNotFound { - client_id, - height: client_state.latest_height, - })?; - - // Make sure that the client type is of Tendermint type `ClientState` - let upgraded_tm_client_state = ClientState::try_from(upgraded_client_state.clone())?; - - // Make sure that the consensus type is of Tendermint type `ConsensusState` - TmConsensusState::try_from(upgraded_consensus_state.clone())?; - - let latest_height = client_state.latest_height; - let upgraded_tm_client_state_height = upgraded_tm_client_state.latest_height(); - - // Make sure the latest height of the current client is not greater then - // the upgrade height This condition checks both the revision number and - // the height - if latest_height >= upgraded_tm_client_state_height { - Err(UpgradeClientError::LowUpgradeHeight { - upgraded_height: latest_height, - client_height: upgraded_tm_client_state_height, - })? - } - - // Check to see if the upgrade path is set - let mut upgrade_path = client_state.upgrade_path.clone(); - - if upgrade_path.pop().is_none() { - return Err(ClientError::ClientSpecific { - description: "cannot upgrade client as no upgrade path has been set".to_string(), - }); - }; - - let upgrade_path_prefix = CommitmentPrefix::try_from(upgrade_path[0].clone().into_bytes()) - .map_err(ClientError::InvalidCommitmentProof)?; - - let last_height = latest_height.revision_height(); - - // Verify the proof of the upgraded client state - verify_membership::( - &client_state.proof_specs, - &upgrade_path_prefix, - &proof_upgrade_client, - old_consensus_state.root(), - ctx.serialize_path(UpgradeClientPath::UpgradedClientState(last_height))?, - upgraded_client_state.to_vec(), - )?; - - // Verify the proof of the upgraded consensus state - verify_membership::( - &client_state.proof_specs, - &upgrade_path_prefix, - &proof_upgrade_consensus_state, - old_consensus_state.root(), - ctx.serialize_path(UpgradeClientPath::UpgradedClientConsensusState(last_height))?, - upgraded_consensus_state.to_vec(), - )?; - - Ok(()) -} - /// Check that the subject and substitute client states match as part of /// the client recovery validation step. /// diff --git a/ibc-core/ics02-client/context/src/client_state.rs b/ibc-core/ics02-client/context/src/client_state.rs index 8a53c9d8b..71ea91e37 100644 --- a/ibc-core/ics02-client/context/src/client_state.rs +++ b/ibc-core/ics02-client/context/src/client_state.rs @@ -6,7 +6,7 @@ use ibc_core_commitment_types::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, }; use ibc_core_host_types::identifiers::{ClientId, ClientType}; -use ibc_core_host_types::path::PathBytes; +use ibc_core_host_types::path::{Path, PathBytes}; use ibc_primitives::prelude::*; use ibc_primitives::proto::Any; @@ -33,6 +33,28 @@ pub trait ClientStateCommon: Convertible { /// Validate that the client is at a sufficient height fn validate_proof_height(&self, proof_height: Height) -> Result<(), ClientError>; + /// Verify the upgraded client and consensus states and validate proofs + /// against the given root. + /// + /// NOTE: proof heights are not included, as upgrade to a new revision is + /// expected to pass only on the last height committed by the current + /// revision. Clients are responsible for ensuring that the planned last + /// height of the current revision is somehow encoded in the proof + /// verification process. This is to ensure that no premature upgrades + /// occur, since upgrade plans committed to by the counterparty may be + /// cancelled or modified before the last planned height. + fn verify_upgrade_client( + &self, + upgraded_client_state: Any, + upgraded_consensus_state: Any, + proof_upgrade_client: CommitmentProofBytes, + proof_upgrade_consensus_state: CommitmentProofBytes, + root: &CommitmentRoot, + ) -> Result<(), ClientError>; + + /// Determines how a path should be serialized into a `PathBytes` object. + fn serialize_path(&self, path: impl Into) -> Result; + // Verify_membership is a generic proof verification method which verifies a // proof of the existence of a value at a given Path. fn verify_membership( @@ -103,26 +125,6 @@ where /// Returns the status of the client. Only Active clients are allowed to process packets. fn status(&self, ctx: &V, client_id: &ClientId) -> Result; - /// Verify the upgraded client and consensus states and validate proofs - /// against the given root. - /// - /// NOTE: proof heights are not included, as upgrade to a new revision is - /// expected to pass only on the last height committed by the current - /// revision. Clients are responsible for ensuring that the planned last - /// height of the current revision is somehow encoded in the proof - /// verification process. This is to ensure that no premature upgrades - /// occur, since upgrade plans committed to by the counterparty may be - /// cancelled or modified before the last planned height. - fn verify_upgrade_client( - &self, - ctx: &V, - client_id: ClientId, - upgraded_client_state: Any, - upgraded_consensus_state: Any, - proof_upgrade_client: CommitmentProofBytes, - proof_upgrade_consensus_state: CommitmentProofBytes, - ) -> Result<(), ClientError>; - /// Verifies whether the calling (subject) client state matches the substitute /// client state for the purposes of client recovery. /// diff --git a/ibc-core/ics02-client/context/src/context.rs b/ibc-core/ics02-client/context/src/context.rs index 8c44e7eec..972ec547e 100644 --- a/ibc-core/ics02-client/context/src/context.rs +++ b/ibc-core/ics02-client/context/src/context.rs @@ -1,7 +1,7 @@ use ibc_core_client_types::Height; use ibc_core_handler_types::error::ContextError; use ibc_core_host_types::identifiers::ClientId; -use ibc_core_host_types::path::{ClientConsensusStatePath, ClientStatePath, Path, PathBytes}; +use ibc_core_host_types::path::{ClientConsensusStatePath, ClientStatePath}; use ibc_primitives::prelude::*; use ibc_primitives::Timestamp; @@ -39,10 +39,6 @@ pub trait ClientValidationContext: Sized { client_id: &ClientId, height: &Height, ) -> Result<(Timestamp, Height), ContextError>; - - fn serialize_path(&self, path: impl Into) -> Result { - Ok(path.into().to_string().into_bytes().into()) - } } /// Defines the methods that all client `ExecutionContext`s (precisely the diff --git a/ibc-core/ics02-client/src/handler/upgrade_client.rs b/ibc-core/ics02-client/src/handler/upgrade_client.rs index 7c45edff4..a072988a2 100644 --- a/ibc-core/ics02-client/src/handler/upgrade_client.rs +++ b/ibc-core/ics02-client/src/handler/upgrade_client.rs @@ -1,10 +1,12 @@ //! Protocol logic specific to processing ICS2 messages of type `MsgUpgradeAnyClient`. //! use ibc_core_client_context::prelude::*; +use ibc_core_client_types::error::ClientError; use ibc_core_client_types::events::UpgradeClient; use ibc_core_client_types::msgs::MsgUpgradeClient; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; +use ibc_core_host::types::path::ClientConsensusStatePath; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_primitives::prelude::*; @@ -28,14 +30,26 @@ where .status(client_val_ctx, &client_id)? .verify_is_active()?; + // Read the latest consensus state from the host chain store. + let old_client_cons_state_path = ClientConsensusStatePath::new( + client_id.clone(), + old_client_state.latest_height().revision_number(), + old_client_state.latest_height().revision_height(), + ); + let old_consensus_state = client_val_ctx + .consensus_state(&old_client_cons_state_path) + .map_err(|_| ClientError::ConsensusStateNotFound { + client_id, + height: old_client_state.latest_height(), + })?; + // Validate the upgraded client state and consensus state and verify proofs against the root old_client_state.verify_upgrade_client( - client_val_ctx, - client_id, msg.upgraded_client_state.clone(), msg.upgraded_consensus_state, msg.proof_upgrade_client, msg.proof_upgrade_consensus_state, + old_consensus_state.root(), )?; Ok(()) diff --git a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs index eb626eaf1..f6f4d31f0 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs @@ -84,9 +84,6 @@ where let prefix_on_b = vars.conn_end_on_a.counterparty().prefix(); { - let path_bytes = - client_val_ctx_a.serialize_path(ConnectionPath::new(&msg.conn_id_on_b))?; - let expected_conn_end_on_b = ConnectionEnd::new( State::TryOpen, vars.client_id_on_b().clone(), @@ -99,6 +96,9 @@ where vars.conn_end_on_a.delay_period(), )?; + let path_bytes = + client_state_of_b_on_a.serialize_path(ConnectionPath::new(&msg.conn_id_on_b))?; + client_state_of_b_on_a .verify_membership( prefix_on_b, @@ -110,8 +110,8 @@ where .map_err(ConnectionError::VerifyConnectionState)?; } - let path_bytes = - client_val_ctx_a.serialize_path(ClientStatePath::new(vars.client_id_on_b().clone()))?; + let path_bytes = client_state_of_b_on_a + .serialize_path(ClientStatePath::new(vars.client_id_on_b().clone()))?; client_state_of_b_on_a .verify_membership( @@ -138,7 +138,7 @@ where msg.consensus_height_of_a_on_b.revision_height(), ); - let path_bytes = client_val_ctx_a + let path_bytes = client_state_of_b_on_a .serialize_path(Path::ClientConsensusState(client_cons_state_path_on_b))?; client_state_of_b_on_a diff --git a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs index 8e5428013..741c207dc 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs @@ -61,8 +61,6 @@ where let prefix_on_a = conn_end_on_b.counterparty().prefix(); let prefix_on_b = ctx_b.commitment_prefix(); - let path_bytes = client_val_ctx_b.serialize_path(ConnectionPath::new(conn_id_on_a))?; - let expected_conn_end_on_a = ConnectionEnd::new( State::Open, client_id_on_a.clone(), @@ -75,6 +73,9 @@ where conn_end_on_b.delay_period(), )?; + let path_bytes = + client_state_of_a_on_b.serialize_path(ConnectionPath::new(conn_id_on_a))?; + client_state_of_a_on_b .verify_membership( prefix_on_a, diff --git a/ibc-core/ics03-connection/src/handler/conn_open_try.rs b/ibc-core/ics03-connection/src/handler/conn_open_try.rs index 2ca2fe705..a4909d607 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_try.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_try.rs @@ -84,9 +84,6 @@ where let prefix_on_b = ctx_b.commitment_prefix(); { - let path_bytes = - client_val_ctx_b.serialize_path(ConnectionPath::new(&vars.conn_id_on_a))?; - let expected_conn_end_on_a = ConnectionEnd::new( State::Init, client_id_on_a.clone(), @@ -95,6 +92,9 @@ where msg.delay_period, )?; + let path_bytes = + client_state_of_a_on_b.serialize_path(ConnectionPath::new(&vars.conn_id_on_a))?; + client_state_of_a_on_b .verify_membership( prefix_on_a, @@ -107,7 +107,7 @@ where } let path_bytes = - client_val_ctx_b.serialize_path(ClientStatePath::new(client_id_on_a.clone()))?; + client_state_of_a_on_b.serialize_path(ClientStatePath::new(client_id_on_a.clone()))?; client_state_of_a_on_b .verify_membership( @@ -134,7 +134,7 @@ where msg.consensus_height_of_b_on_a.revision_height(), ); - let path_bytes = client_val_ctx_b.serialize_path(client_cons_state_path_on_a)?; + let path_bytes = client_state_of_a_on_b.serialize_path(client_cons_state_path_on_a)?; client_state_of_a_on_b .verify_membership( diff --git a/ibc-core/ics04-channel/src/handler/acknowledgement.rs b/ibc-core/ics04-channel/src/handler/acknowledgement.rs index d3d6994ab..a35461494 100644 --- a/ibc-core/ics04-channel/src/handler/acknowledgement.rs +++ b/ibc-core/ics04-channel/src/handler/acknowledgement.rs @@ -188,10 +188,10 @@ where let ack_path_on_b = AckPath::new(&packet.port_id_on_b, &packet.chan_id_on_b, packet.seq_on_a); - let path_bytes = client_val_ctx_a.serialize_path(ack_path_on_b)?; - verify_conn_delay_passed(ctx_a, msg.proof_height_on_b, &conn_end_on_a)?; + let path_bytes = client_state_of_b_on_a.serialize_path(ack_path_on_b)?; + // Verify the proof for the packet against the chain store. client_state_of_b_on_a .verify_membership( diff --git a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs index fda78eab2..2339e7a7a 100644 --- a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs @@ -149,7 +149,7 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, chan_id_on_a); - let path_bytes = client_val_ctx_b.serialize_path(chan_end_path_on_a)?; + let path_bytes = client_state_of_a_on_b.serialize_path(chan_end_path_on_a)?; // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. diff --git a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs index 2855d95cf..38c43c057 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs @@ -142,7 +142,7 @@ where )?; let chan_end_path_on_b = ChannelEndPath::new(port_id_on_b, &msg.chan_id_on_b); - let path_bytes = client_val_ctx_a.serialize_path(chan_end_path_on_b)?; + let path_bytes = client_state_of_b_on_a.serialize_path(chan_end_path_on_b)?; // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. diff --git a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs index 7e96d96b9..0cc71d9f9 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs @@ -149,7 +149,7 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, chan_id_on_a); - let path_bytes = client_val_ctx_b.serialize_path(chan_end_path_on_a)?; + let path_bytes = client_state_of_a_on_b.serialize_path(chan_end_path_on_a)?; // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked in msg. diff --git a/ibc-core/ics04-channel/src/handler/chan_open_try.rs b/ibc-core/ics04-channel/src/handler/chan_open_try.rs index d985b3e84..a147b95d9 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_try.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_try.rs @@ -168,7 +168,7 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(&port_id_on_a, &chan_id_on_a); - let path_bytes = client_val_ctx_b.serialize_path(chan_end_path_on_a)?; + let path_bytes = client_state_of_a_on_b.serialize_path(chan_end_path_on_a)?; // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs index 10522db89..635eaaf9e 100644 --- a/ibc-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-core/ics04-channel/src/handler/recv_packet.rs @@ -206,10 +206,10 @@ where msg.packet.seq_on_a, ); - let path_bytes = client_val_ctx_b.serialize_path(commitment_path_on_a)?; - verify_conn_delay_passed(ctx_b, msg.proof_height_on_a, &conn_end_on_b)?; + let path_bytes = client_state_of_a_on_b.serialize_path(commitment_path_on_a)?; + // Verify the proof for the packet against the chain store. client_state_of_a_on_b .verify_membership( diff --git a/ibc-core/ics04-channel/src/handler/timeout.rs b/ibc-core/ics04-channel/src/handler/timeout.rs index 3f0d12856..def90b0b3 100644 --- a/ibc-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-core/ics04-channel/src/handler/timeout.rs @@ -221,7 +221,7 @@ where let seq_recv_path_on_b = SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b); - let path_bytes = client_val_ctx_a.serialize_path(seq_recv_path_on_b)?; + let path_bytes = client_state_of_b_on_a.serialize_path(seq_recv_path_on_b)?; client_state_of_b_on_a.verify_membership( conn_end_on_a.counterparty().prefix(), @@ -238,7 +238,7 @@ where msg.packet.seq_on_a, ); - let path_bytes = client_val_ctx_a.serialize_path(receipt_path_on_b)?; + let path_bytes = client_state_of_b_on_a.serialize_path(receipt_path_on_b)?; client_state_of_b_on_a.verify_non_membership( conn_end_on_a.counterparty().prefix(), diff --git a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs index fefb7117f..df2530fb9 100644 --- a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs +++ b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs @@ -104,7 +104,7 @@ where let chan_end_path_on_b = ChannelEndPath(port_id_on_b, chan_id_on_b.clone()); - let path_bytes = client_val_ctx_a.serialize_path(chan_end_path_on_b)?; + let path_bytes = client_state_of_b_on_a.serialize_path(chan_end_path_on_b)?; // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. @@ -133,7 +133,7 @@ where let seq_recv_path_on_b = SeqRecvPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); - let path_bytes = client_val_ctx_a.serialize_path(seq_recv_path_on_b)?; + let path_bytes = client_state_of_b_on_a.serialize_path(seq_recv_path_on_b)?; client_state_of_b_on_a.verify_membership( conn_end_on_a.counterparty().prefix(), @@ -150,7 +150,7 @@ where msg.packet.seq_on_a, ); - let path_bytes = client_val_ctx_a.serialize_path(receipt_path_on_b)?; + let path_bytes = client_state_of_b_on_a.serialize_path(receipt_path_on_b)?; client_state_of_b_on_a.verify_non_membership( conn_end_on_a.counterparty().prefix(), diff --git a/ibc-derive/src/client_state/traits/client_state_common.rs b/ibc-derive/src/client_state/traits/client_state_common.rs index 3284eb220..34f593405 100644 --- a/ibc-derive/src/client_state/traits/client_state_common.rs +++ b/ibc-derive/src/client_state/traits/client_state_common.rs @@ -35,6 +35,18 @@ pub(crate) fn impl_ClientStateCommon( quote! {validate_proof_height(cs, proof_height)}, imports, ); + let verify_upgrade_client_impl = delegate_call_in_match( + client_state_enum_name, + enum_variants.iter(), + quote! {verify_upgrade_client(cs, upgraded_client_state, upgraded_consensus_state, proof_upgrade_client, proof_upgrade_consensus_state, root)}, + imports, + ); + let serialize_path_impl = delegate_call_in_match( + client_state_enum_name, + enum_variants.iter(), + quote! {serialize_path(cs, path)}, + imports, + ); let verify_membership_impl = delegate_call_in_match( client_state_enum_name, enum_variants.iter(), @@ -58,6 +70,7 @@ pub(crate) fn impl_ClientStateCommon( let ClientType = imports.client_type(); let ClientError = imports.client_error(); let Height = imports.height(); + let Path = imports.path(); let PathBytes = imports.path_bytes(); quote! { @@ -85,6 +98,25 @@ pub(crate) fn impl_ClientStateCommon( } } + fn verify_upgrade_client( + &self, + upgraded_client_state: #Any, + upgraded_consensus_state: #Any, + proof_upgrade_client: #CommitmentProofBytes, + proof_upgrade_consensus_state: #CommitmentProofBytes, + root: &#CommitmentRoot, + ) -> core::result::Result<(), #ClientError> { + match self { + #(#verify_upgrade_client_impl),* + } + } + + fn serialize_path(&self, path: impl Into<#Path>) -> core::result::Result<#PathBytes, #ClientError> { + match self { + #(#serialize_path_impl),* + } + } + fn verify_membership( &self, prefix: &#CommitmentPrefix, diff --git a/ibc-derive/src/client_state/traits/client_state_validation.rs b/ibc-derive/src/client_state/traits/client_state_validation.rs index 9c28307aa..263411b15 100644 --- a/ibc-derive/src/client_state/traits/client_state_validation.rs +++ b/ibc-derive/src/client_state/traits/client_state_validation.rs @@ -37,14 +37,6 @@ pub(crate) fn impl_ClientStateValidation( imports, ); - let verify_upgrade_client_impl = delegate_call_in_match( - client_state_enum_name, - enum_variants.iter(), - opts, - quote! {verify_upgrade_client(cs, ctx, client_id, upgraded_client_state, upgraded_consensus_state, proof_upgrade_client, proof_upgrade_consensus_state)}, - imports, - ); - let check_substitute_impl = delegate_call_in_match( client_state_enum_name, enum_variants.iter(), @@ -59,7 +51,6 @@ pub(crate) fn impl_ClientStateValidation( let ClientError = imports.client_error(); let ClientStateValidation = imports.client_state_validation(); let Status = imports.status(); - let CommitmentProofBytes = imports.commitment_proof_bytes(); // The types we need for the generated code. let HostClientState = client_state_enum_name; @@ -106,20 +97,6 @@ pub(crate) fn impl_ClientStateValidation( } } - fn verify_upgrade_client( - &self, - ctx: &#V, - client_id: #ClientId, - upgraded_client_state: #Any, - upgraded_consensus_state: #Any, - proof_upgrade_client: #CommitmentProofBytes, - proof_upgrade_consensus_state: #CommitmentProofBytes, - ) -> core::result::Result<(), #ClientError> { - match self { - #(#verify_upgrade_client_impl),* - } - } - fn check_substitute( &self, ctx: &#V, diff --git a/ibc-derive/src/utils.rs b/ibc-derive/src/utils.rs index 82cf91335..e028b677f 100644 --- a/ibc-derive/src/utils.rs +++ b/ibc-derive/src/utils.rs @@ -45,6 +45,11 @@ impl Imports { quote! {#Prefix::commitment_types::commitment::CommitmentProofBytes} } + pub fn path(&self) -> TokenStream { + let Prefix = self.prefix(); + quote! {#Prefix::host::types::path::Path} + } + pub fn path_bytes(&self) -> TokenStream { let Prefix = self.prefix(); quote! {#Prefix::host::types::path::PathBytes} diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs index afef24949..6319b4b64 100644 --- a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs +++ b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs @@ -9,7 +9,7 @@ use ibc::core::commitment_types::commitment::{ }; use ibc::core::handler::types::error::ContextError; use ibc::core::host::types::identifiers::{ClientId, ClientType}; -use ibc::core::host::types::path::{ClientConsensusStatePath, ClientStatePath, PathBytes}; +use ibc::core::host::types::path::{ClientConsensusStatePath, ClientStatePath, Path, PathBytes}; use ibc::core::primitives::prelude::*; use ibc::core::primitives::Timestamp; use ibc::primitives::proto::{Any, Protobuf}; @@ -184,6 +184,29 @@ impl ClientStateCommon for MockClientState { Ok(()) } + fn serialize_path(&self, path: impl Into) -> Result { + Ok(path.into().to_string().into_bytes().into()) + } + + fn verify_upgrade_client( + &self, + upgraded_client_state: Any, + upgraded_consensus_state: Any, + _proof_upgrade_client: CommitmentProofBytes, + _proof_upgrade_consensus_state: CommitmentProofBytes, + _root: &CommitmentRoot, + ) -> Result<(), ClientError> { + let upgraded_mock_client_state = Self::try_from(upgraded_client_state)?; + MockConsensusState::try_from(upgraded_consensus_state)?; + if self.latest_height() >= upgraded_mock_client_state.latest_height() { + return Err(UpgradeClientError::LowUpgradeHeight { + upgraded_height: self.latest_height(), + client_height: upgraded_mock_client_state.latest_height(), + })?; + } + Ok(()) + } + fn verify_membership( &self, _prefix: &CommitmentPrefix, @@ -287,26 +310,6 @@ where Ok(Status::Active) } - fn verify_upgrade_client( - &self, - _ctx: &V, - _client_id: ClientId, - upgraded_client_state: Any, - _upgraded_consensus_state: Any, - _proof_upgrade_client: CommitmentProofBytes, - _proof_upgrade_consensus_state: CommitmentProofBytes, - ) -> Result<(), ClientError> { - let upgraded_mock_client_state = Self::try_from(upgraded_client_state)?; - // MockConsensusState::try_from(upgraded_consensus_state)?; - if self.latest_height() >= upgraded_mock_client_state.latest_height() { - return Err(UpgradeClientError::LowUpgradeHeight { - upgraded_height: self.latest_height(), - client_height: upgraded_mock_client_state.latest_height(), - })?; - } - Ok(()) - } - fn check_substitute(&self, _ctx: &V, _substitute_client_state: Any) -> Result<(), ClientError> { Ok(()) } diff --git a/tests-integration/tests/core/ics02_client/create_client.rs b/tests-integration/tests/core/ics02_client/create_client.rs index ec284f27d..170baf42f 100644 --- a/tests-integration/tests/core/ics02_client/create_client.rs +++ b/tests-integration/tests/core/ics02_client/create_client.rs @@ -195,7 +195,7 @@ fn test_tm_create_client_proof_verification_ok() { &ctx_tm.ibc_store().commitment_prefix(), &proof, &root, - client_validation_ctx_mk + tm_client_state .serialize_path(next_client_seq_path.clone()) .expect("path"), serde_json::to_vec(&next_client_seq_value).expect("valid json serialization"), @@ -209,7 +209,7 @@ fn test_tm_create_client_proof_verification_ok() { &ctx_tm.ibc_store().commitment_prefix(), &proof, &root, - client_validation_ctx_mk + tm_client_state .serialize_path(next_client_seq_path.clone()) .expect("path"), serde_json::to_vec(&(next_client_seq_value + 1)).expect("valid json serialization"), From 99529095d0e7b0c01cf53ce9d32f609cd6358237 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 10 Jul 2024 09:25:30 -0700 Subject: [PATCH 03/13] feat: define concat() for PathBytes --- ibc-clients/cw-context/src/types/msgs.rs | 19 ++++--------------- ibc-core/ics24-host/types/src/path.rs | 11 ++++++++++- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index d6afeb3f3..12a92cdbe 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -162,18 +162,12 @@ impl TryFrom for VerifyMembershipMsg { fn try_from(mut raw: VerifyMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof)?; let prefix = raw.path.key_path.remove(0); + let path = PathBytes::concat(raw.path.key_path); let height = Height::try_from(raw.height)?; - let path_bytes: Vec = raw - .path - .key_path - .into_iter() - .flat_map(|p| p.into_vec()) - .collect(); - Ok(Self { proof, - path: path_bytes.into(), + path, value: raw.value, height, prefix: CommitmentPrefix::try_from(prefix.into_vec())?, @@ -209,17 +203,12 @@ impl TryFrom for VerifyNonMembershipMsg { fn try_from(mut raw: VerifyNonMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof)?; let prefix = raw.path.key_path.remove(0); + let path = PathBytes::concat(raw.path.key_path); let height = raw.height.try_into()?; - let path_bytes: Vec = raw - .path - .key_path - .into_iter() - .flat_map(|p| p.into_vec()) - .collect(); Ok(Self { proof, - path: path_bytes.into(), + path, height, prefix: CommitmentPrefix::try_from(prefix.into_vec())?, delay_block_period: raw.delay_block_period, diff --git a/ibc-core/ics24-host/types/src/path.rs b/ibc-core/ics24-host/types/src/path.rs index 7265b9546..f6af11c4f 100644 --- a/ibc-core/ics24-host/types/src/path.rs +++ b/ibc-core/ics24-host/types/src/path.rs @@ -44,7 +44,7 @@ pub const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState"; /// Represents a general-purpose path structure using the byte representation of /// a path. This struct abstracts over different path types and can handle bytes -/// may obtained from various serialization formats (e.g., Protobuf, borsh). +/// may obtained from various serialization formats (e.g., Protobuf, Borsh). #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, From)] @@ -58,6 +58,15 @@ impl PathBytes { pub fn into_vec(self) -> Vec { self.0 } + + /// Concatenates a list of path bytes into a single path. + pub fn concat(paths: Vec) -> Self { + let mut bytes = Vec::new(); + paths.iter().for_each(|path| { + bytes.extend_from_slice(&path.0); + }); + Self(bytes) + } } impl AsRef<[u8]> for PathBytes { From 0683d8c95ddc3c0daf63ba4378f631ccb704e775 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 10 Jul 2024 09:27:03 -0700 Subject: [PATCH 04/13] fix: update cw-check cargo.lock --- ci/cw-check/Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/cw-check/Cargo.lock b/ci/cw-check/Cargo.lock index f1fe980b9..f5a96697b 100644 --- a/ci/cw-check/Cargo.lock +++ b/ci/cw-check/Cargo.lock @@ -739,6 +739,7 @@ version = "0.53.0" dependencies = [ "derive_more", "displaydoc", + "ibc-core-host-types", "ibc-primitives", "ibc-proto", "ics23", From ab2f16ff31441588c535237f628d36d90a503ee7 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 10 Jul 2024 10:00:11 -0700 Subject: [PATCH 05/13] chore: add docs and unclog --- ... 1229-auto-derive-ser-commitment-prefix copy.md} | 0 ...5-enable-custom-paths-for-proof-verificaitons.md | 4 ++++ .../ics07-tendermint/src/client_state/common.rs | 4 ++-- ibc-core/ics02-client/context/src/client_state.rs | 8 +++++++- ibc-core/ics23-commitment/types/src/merkle.rs | 13 +++++++++++++ 5 files changed, 26 insertions(+), 3 deletions(-) rename .changelog/unreleased/breaking-changes/{1229-auto-derive-ser-commitment-prefix.md => 1229-auto-derive-ser-commitment-prefix copy.md} (100%) create mode 100644 .changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md diff --git a/.changelog/unreleased/breaking-changes/1229-auto-derive-ser-commitment-prefix.md b/.changelog/unreleased/breaking-changes/1229-auto-derive-ser-commitment-prefix copy.md similarity index 100% rename from .changelog/unreleased/breaking-changes/1229-auto-derive-ser-commitment-prefix.md rename to .changelog/unreleased/breaking-changes/1229-auto-derive-ser-commitment-prefix copy.md diff --git a/.changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md b/.changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md new file mode 100644 index 000000000..66edab29c --- /dev/null +++ b/.changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md @@ -0,0 +1,4 @@ +- [ibc-core-client] Enable `verify_(non_)membership()` methods to accept custom + paths as bytes by defining a new `serializer_path()` API allowing light client + developers to introduce the path serialization behavior of their system. + ([\#1255](https://github.com/cosmos/ibc-rs/issues/1255)) diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs index bfadd70c4..fa5d1958d 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/common.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/common.rs @@ -231,7 +231,7 @@ pub fn verify_membership( path: PathBytes, value: Vec, ) -> Result<(), ClientError> { - let merkle_path = MerklePath::new(vec![prefix.clone().into_vec().into(), path]); + let merkle_path = MerklePath::new(vec![prefix.as_bytes().to_vec().into(), path]); let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?; merkle_proof @@ -251,7 +251,7 @@ pub fn verify_non_membership( root: &CommitmentRoot, path: PathBytes, ) -> Result<(), ClientError> { - let merkle_path = MerklePath::new(vec![prefix.clone().into_vec().into(), path]); + let merkle_path = MerklePath::new(vec![prefix.as_bytes().to_vec().into(), path]); let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?; merkle_proof diff --git a/ibc-core/ics02-client/context/src/client_state.rs b/ibc-core/ics02-client/context/src/client_state.rs index 71ea91e37..ae77b5b74 100644 --- a/ibc-core/ics02-client/context/src/client_state.rs +++ b/ibc-core/ics02-client/context/src/client_state.rs @@ -52,7 +52,13 @@ pub trait ClientStateCommon: Convertible { root: &CommitmentRoot, ) -> Result<(), ClientError>; - /// Determines how a path should be serialized into a `PathBytes` object. + /// Serializes a given path into a `PathBytes` object. + /// + /// This method provides essential information for IBC modules, enabling + /// them to understand how path serialization is performed on the + /// counterparty chain (where this light client represents it) before + /// passing the path bytes to either `verify_membership()` or + /// `verify_non_membership()`. fn serialize_path(&self, path: impl Into) -> Result; // Verify_membership is a generic proof verification method which verifies a diff --git a/ibc-core/ics23-commitment/types/src/merkle.rs b/ibc-core/ics23-commitment/types/src/merkle.rs index 1580cef18..7b65243a8 100644 --- a/ibc-core/ics23-commitment/types/src/merkle.rs +++ b/ibc-core/ics23-commitment/types/src/merkle.rs @@ -16,6 +16,14 @@ use crate::commitment::CommitmentRoot; use crate::error::CommitmentError; use crate::specs::ProofSpecs; +/// A wrapper type representing a Merkle path, consisting of a sequence of path +/// bytes. +/// +/// This struct by definition is compatible with Cosmos SDK chains, but it is +/// also applicable to other blockchain implementations that follow similar path +/// structures. Note that while Cosmos SDK chains and some non-Cosmos chains +/// adhere to this definition, it may not be universally applicable to all +/// non-Cosmos chains. #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[derive(Clone, Debug, PartialEq)] @@ -24,6 +32,7 @@ pub struct MerklePath { } impl MerklePath { + /// Constructs a new `MerklePath` from a given `Vec`. pub fn new(key_path: Vec) -> Self { Self { key_path } } @@ -41,6 +50,10 @@ impl From for MerklePath { } } +// The conversion from `MerklePath`` to `RawMerklePath` is not provided as we +// cannot assume how the key paths of `Vec` type should be serialized +// to the `Vec`. + impl From for MerkleRoot { fn from(root: CommitmentRoot) -> Self { Self { From 1f9a55fa0a89cf102fd0a450aeb6bb208908ee05 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Wed, 10 Jul 2024 10:17:20 -0700 Subject: [PATCH 06/13] misc: apply some fixes --- ... copy.md => 1229-auto-derive-ser-commitment-prefix.md} | 0 ci/no-std-check/Cargo.lock | 1 + ibc-clients/cw-context/src/types/msgs.rs | 8 ++++---- 3 files changed, 5 insertions(+), 4 deletions(-) rename .changelog/unreleased/breaking-changes/{1229-auto-derive-ser-commitment-prefix copy.md => 1229-auto-derive-ser-commitment-prefix.md} (100%) diff --git a/.changelog/unreleased/breaking-changes/1229-auto-derive-ser-commitment-prefix copy.md b/.changelog/unreleased/breaking-changes/1229-auto-derive-ser-commitment-prefix.md similarity index 100% rename from .changelog/unreleased/breaking-changes/1229-auto-derive-ser-commitment-prefix copy.md rename to .changelog/unreleased/breaking-changes/1229-auto-derive-ser-commitment-prefix.md diff --git a/ci/no-std-check/Cargo.lock b/ci/no-std-check/Cargo.lock index da50c551d..1590997ae 100644 --- a/ci/no-std-check/Cargo.lock +++ b/ci/no-std-check/Cargo.lock @@ -1357,6 +1357,7 @@ version = "0.53.0" dependencies = [ "derive_more", "displaydoc", + "ibc-core-host-types", "ibc-primitives", "ibc-proto", "ics23", diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index 12a92cdbe..e1b4c3150 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -161,7 +161,7 @@ impl TryFrom for VerifyMembershipMsg { fn try_from(mut raw: VerifyMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof)?; - let prefix = raw.path.key_path.remove(0); + let prefix = CommitmentPrefix::try_from(raw.path.key_path.remove(0).into_vec())?; let path = PathBytes::concat(raw.path.key_path); let height = Height::try_from(raw.height)?; @@ -170,7 +170,7 @@ impl TryFrom for VerifyMembershipMsg { path, value: raw.value, height, - prefix: CommitmentPrefix::try_from(prefix.into_vec())?, + prefix, delay_block_period: raw.delay_block_period, delay_time_period: raw.delay_time_period, }) @@ -202,7 +202,7 @@ impl TryFrom for VerifyNonMembershipMsg { fn try_from(mut raw: VerifyNonMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof)?; - let prefix = raw.path.key_path.remove(0); + let prefix = CommitmentPrefix::try_from(raw.path.key_path.remove(0).into_vec())?; let path = PathBytes::concat(raw.path.key_path); let height = raw.height.try_into()?; @@ -210,7 +210,7 @@ impl TryFrom for VerifyNonMembershipMsg { proof, path, height, - prefix: CommitmentPrefix::try_from(prefix.into_vec())?, + prefix, delay_block_period: raw.delay_block_period, delay_time_period: raw.delay_time_period, }) From d44de2f38ccb453ffea1a5fa5276186dbddc44ab Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 11 Jul 2024 17:53:31 -0700 Subject: [PATCH 07/13] imp: group serialize_path and verify_(non_)membership_raw methods --- ibc-clients/cw-context/src/handlers.rs | 4 +- .../src/client_state/common.rs | 4 +- .../ics02-client/context/src/client_state.rs | 44 ++++++++++++++----- .../src/handler/conn_open_ack.rs | 17 ++----- .../src/handler/conn_open_confirm.rs | 5 +-- .../src/handler/conn_open_try.rs | 14 ++---- .../src/handler/acknowledgement.rs | 4 +- .../src/handler/chan_close_confirm.rs | 4 +- .../src/handler/chan_open_ack.rs | 4 +- .../src/handler/chan_open_confirm.rs | 4 +- .../src/handler/chan_open_try.rs | 4 +- .../ics04-channel/src/handler/recv_packet.rs | 4 +- ibc-core/ics04-channel/src/handler/timeout.rs | 8 +--- .../src/handler/timeout_on_close.rs | 12 ++--- .../traits/client_state_common.rs | 41 ++++++++++++++++- .../testapp/ibc/clients/mock/client_state.rs | 4 +- .../tests/core/ics02_client/create_client.rs | 8 +--- 17 files changed, 99 insertions(+), 86 deletions(-) diff --git a/ibc-clients/cw-context/src/handlers.rs b/ibc-clients/cw-context/src/handlers.rs index 4b4798d1e..eb92e0928 100644 --- a/ibc-clients/cw-context/src/handlers.rs +++ b/ibc-clients/cw-context/src/handlers.rs @@ -70,7 +70,7 @@ where let consensus_state = self.consensus_state(&client_cons_state_path)?; - client_state.verify_membership( + client_state.verify_membership_raw( &msg.prefix, &msg.proof, consensus_state.root(), @@ -91,7 +91,7 @@ where let consensus_state = self.consensus_state(&client_cons_state_path)?; - client_state.verify_non_membership( + client_state.verify_non_membership_raw( &msg.prefix, &msg.proof, consensus_state.root(), diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs index fa5d1958d..f84195fc7 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/common.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/common.rs @@ -67,7 +67,7 @@ impl ClientStateCommon for ClientState { Ok(path.into().to_string().into_bytes().into()) } - fn verify_membership( + fn verify_membership_raw( &self, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, @@ -85,7 +85,7 @@ impl ClientStateCommon for ClientState { ) } - fn verify_non_membership( + fn verify_non_membership_raw( &self, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, diff --git a/ibc-core/ics02-client/context/src/client_state.rs b/ibc-core/ics02-client/context/src/client_state.rs index ae77b5b74..33cf8b93a 100644 --- a/ibc-core/ics02-client/context/src/client_state.rs +++ b/ibc-core/ics02-client/context/src/client_state.rs @@ -52,18 +52,16 @@ pub trait ClientStateCommon: Convertible { root: &CommitmentRoot, ) -> Result<(), ClientError>; - /// Serializes a given path into a `PathBytes` object. + /// Serializes a given path object into a raw path bytes. /// /// This method provides essential information for IBC modules, enabling - /// them to understand how path serialization is performed on the - /// counterparty chain (where this light client represents it) before - /// passing the path bytes to either `verify_membership()` or - /// `verify_non_membership()`. + /// them to understand how path serialization is performed on the chain this + /// light client represents it before passing the path bytes to either + /// `verify_membership_raw()` or `verify_non_membership_raw()`. fn serialize_path(&self, path: impl Into) -> Result; - // Verify_membership is a generic proof verification method which verifies a - // proof of the existence of a value at a given Path. - fn verify_membership( + /// Verifies a proof of the existence of a value at a given raw path bytes. + fn verify_membership_raw( &self, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, @@ -72,15 +70,39 @@ pub trait ClientStateCommon: Convertible { value: Vec, ) -> Result<(), ClientError>; - // Verify_non_membership is a generic proof verification method which - // verifies the absence of a given commitment. - fn verify_non_membership( + /// Verifies a proof of the existence of a value at a given path object. + fn verify_membership( + &self, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + path: impl Into, + value: Vec, + ) -> Result<(), ClientError> { + let path_bytes = self.serialize_path(path)?; + self.verify_membership_raw(prefix, proof, root, path_bytes, value) + } + + /// Verifies the absence of a given proof at a given raw path bytes. + fn verify_non_membership_raw( &self, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, path: PathBytes, ) -> Result<(), ClientError>; + + /// Verifies the absence of a given proof at a given path object. + fn verify_non_membership( + &self, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + path: impl Into, + ) -> Result<(), ClientError> { + let path_bytes = self.serialize_path(path)?; + self.verify_non_membership_raw(prefix, proof, root, path_bytes) + } } /// `ClientState` methods which require access to the client's validation diff --git a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs index f6f4d31f0..582aa3ad3 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs @@ -9,7 +9,7 @@ use ibc_core_connection_types::{ConnectionEnd, Counterparty, State}; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::identifiers::ClientId; -use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath, ConnectionPath, Path}; +use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath, ConnectionPath}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_primitives::prelude::*; use ibc_primitives::proto::{Any, Protobuf}; @@ -96,29 +96,23 @@ where vars.conn_end_on_a.delay_period(), )?; - let path_bytes = - client_state_of_b_on_a.serialize_path(ConnectionPath::new(&msg.conn_id_on_b))?; - client_state_of_b_on_a .verify_membership( prefix_on_b, &msg.proof_conn_end_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + ConnectionPath::new(&msg.conn_id_on_b), expected_conn_end_on_b.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; } - let path_bytes = client_state_of_b_on_a - .serialize_path(ClientStatePath::new(vars.client_id_on_b().clone()))?; - client_state_of_b_on_a .verify_membership( prefix_on_b, &msg.proof_client_state_of_a_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + ClientStatePath::new(vars.client_id_on_b().clone()), msg.client_state_of_a_on_b.to_vec(), ) .map_err(|e| ConnectionError::ClientStateVerificationFailure { @@ -138,15 +132,12 @@ where msg.consensus_height_of_a_on_b.revision_height(), ); - let path_bytes = client_state_of_b_on_a - .serialize_path(Path::ClientConsensusState(client_cons_state_path_on_b))?; - client_state_of_b_on_a .verify_membership( prefix_on_b, &msg.proof_consensus_state_of_a_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + client_cons_state_path_on_b, stored_consensus_state_of_a_on_b.to_vec(), ) .map_err(|e| ConnectionError::ConsensusStateVerificationFailure { diff --git a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs index 741c207dc..81630ea4d 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs @@ -73,15 +73,12 @@ where conn_end_on_b.delay_period(), )?; - let path_bytes = - client_state_of_a_on_b.serialize_path(ConnectionPath::new(conn_id_on_a))?; - client_state_of_a_on_b .verify_membership( prefix_on_a, &msg.proof_conn_end_on_a, consensus_state_of_a_on_b.root(), - path_bytes, + ConnectionPath::new(conn_id_on_a), expected_conn_end_on_a.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; diff --git a/ibc-core/ics03-connection/src/handler/conn_open_try.rs b/ibc-core/ics03-connection/src/handler/conn_open_try.rs index a4909d607..16205b9ab 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_try.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_try.rs @@ -92,29 +92,23 @@ where msg.delay_period, )?; - let path_bytes = - client_state_of_a_on_b.serialize_path(ConnectionPath::new(&vars.conn_id_on_a))?; - client_state_of_a_on_b .verify_membership( prefix_on_a, &msg.proof_conn_end_on_a, consensus_state_of_a_on_b.root(), - path_bytes, + ConnectionPath::new(&vars.conn_id_on_a), expected_conn_end_on_a.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; } - let path_bytes = - client_state_of_a_on_b.serialize_path(ClientStatePath::new(client_id_on_a.clone()))?; - client_state_of_a_on_b .verify_membership( prefix_on_a, &msg.proof_client_state_of_b_on_a, consensus_state_of_a_on_b.root(), - path_bytes, + ClientStatePath::new(client_id_on_a.clone()), msg.client_state_of_b_on_a.to_vec(), ) .map_err(|e| ConnectionError::ClientStateVerificationFailure { @@ -134,14 +128,12 @@ where msg.consensus_height_of_b_on_a.revision_height(), ); - let path_bytes = client_state_of_a_on_b.serialize_path(client_cons_state_path_on_a)?; - client_state_of_a_on_b .verify_membership( prefix_on_a, &msg.proof_consensus_state_of_b_on_a, consensus_state_of_a_on_b.root(), - path_bytes, + client_cons_state_path_on_a, stored_consensus_state_of_b_on_a.to_vec(), ) .map_err(|e| ConnectionError::ConsensusStateVerificationFailure { diff --git a/ibc-core/ics04-channel/src/handler/acknowledgement.rs b/ibc-core/ics04-channel/src/handler/acknowledgement.rs index a35461494..9d6ab2838 100644 --- a/ibc-core/ics04-channel/src/handler/acknowledgement.rs +++ b/ibc-core/ics04-channel/src/handler/acknowledgement.rs @@ -190,15 +190,13 @@ where verify_conn_delay_passed(ctx_a, msg.proof_height_on_b, &conn_end_on_a)?; - let path_bytes = client_state_of_b_on_a.serialize_path(ack_path_on_b)?; - // Verify the proof for the packet against the chain store. client_state_of_b_on_a .verify_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_acked_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + ack_path_on_b, ack_commitment.into_vec(), ) .map_err(|e| ChannelError::PacketVerificationFailed { diff --git a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs index 2339e7a7a..4c5e5b55b 100644 --- a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs @@ -149,8 +149,6 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, chan_id_on_a); - let path_bytes = client_state_of_a_on_b.serialize_path(chan_end_path_on_a)?; - // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. client_state_of_a_on_b @@ -158,7 +156,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - path_bytes, + chan_end_path_on_a, expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs index 38c43c057..9857279b7 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs @@ -142,8 +142,6 @@ where )?; let chan_end_path_on_b = ChannelEndPath::new(port_id_on_b, &msg.chan_id_on_b); - let path_bytes = client_state_of_b_on_a.serialize_path(chan_end_path_on_b)?; - // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. client_state_of_b_on_a @@ -151,7 +149,7 @@ where prefix_on_b, &msg.proof_chan_end_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + chan_end_path_on_b, expected_chan_end_on_b.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs index 0cc71d9f9..25dba15a4 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs @@ -149,8 +149,6 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, chan_id_on_a); - let path_bytes = client_state_of_a_on_b.serialize_path(chan_end_path_on_a)?; - // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked in msg. client_state_of_a_on_b @@ -158,7 +156,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - path_bytes, + chan_end_path_on_a, expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_try.rs b/ibc-core/ics04-channel/src/handler/chan_open_try.rs index a147b95d9..eefcf1c2b 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_try.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_try.rs @@ -168,8 +168,6 @@ where )?; let chan_end_path_on_a = ChannelEndPath::new(&port_id_on_a, &chan_id_on_a); - let path_bytes = client_state_of_a_on_b.serialize_path(chan_end_path_on_a)?; - // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. client_state_of_a_on_b @@ -177,7 +175,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - path_bytes, + chan_end_path_on_a, expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs index 635eaaf9e..5d5e4ffac 100644 --- a/ibc-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-core/ics04-channel/src/handler/recv_packet.rs @@ -208,15 +208,13 @@ where verify_conn_delay_passed(ctx_b, msg.proof_height_on_a, &conn_end_on_b)?; - let path_bytes = client_state_of_a_on_b.serialize_path(commitment_path_on_a)?; - // Verify the proof for the packet against the chain store. client_state_of_a_on_b .verify_membership( conn_end_on_b.counterparty().prefix(), &msg.proof_commitment_on_a, consensus_state_of_a_on_b.root(), - path_bytes, + commitment_path_on_a, expected_commitment_on_a.into_vec(), ) .map_err(|e| ChannelError::PacketVerificationFailed { diff --git a/ibc-core/ics04-channel/src/handler/timeout.rs b/ibc-core/ics04-channel/src/handler/timeout.rs index def90b0b3..471b8d43e 100644 --- a/ibc-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-core/ics04-channel/src/handler/timeout.rs @@ -221,13 +221,11 @@ where let seq_recv_path_on_b = SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b); - let path_bytes = client_state_of_b_on_a.serialize_path(seq_recv_path_on_b)?; - client_state_of_b_on_a.verify_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + seq_recv_path_on_b, msg.packet.seq_on_a.to_vec(), ) } @@ -238,13 +236,11 @@ where msg.packet.seq_on_a, ); - let path_bytes = client_state_of_b_on_a.serialize_path(receipt_path_on_b)?; - client_state_of_b_on_a.verify_non_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + receipt_path_on_b, ) } Order::None => { diff --git a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs index df2530fb9..d30d9c060 100644 --- a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs +++ b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs @@ -104,8 +104,6 @@ where let chan_end_path_on_b = ChannelEndPath(port_id_on_b, chan_id_on_b.clone()); - let path_bytes = client_state_of_b_on_a.serialize_path(chan_end_path_on_b)?; - // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. client_state_of_b_on_a @@ -113,7 +111,7 @@ where prefix_on_b, &msg.proof_close_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + chan_end_path_on_b, expected_chan_end_on_b.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed) @@ -133,13 +131,11 @@ where let seq_recv_path_on_b = SeqRecvPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); - let path_bytes = client_state_of_b_on_a.serialize_path(seq_recv_path_on_b)?; - client_state_of_b_on_a.verify_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + seq_recv_path_on_b, packet.seq_on_a.to_vec(), ) } @@ -150,13 +146,11 @@ where msg.packet.seq_on_a, ); - let path_bytes = client_state_of_b_on_a.serialize_path(receipt_path_on_b)?; - client_state_of_b_on_a.verify_non_membership( conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - path_bytes, + receipt_path_on_b, ) } Order::None => { diff --git a/ibc-derive/src/client_state/traits/client_state_common.rs b/ibc-derive/src/client_state/traits/client_state_common.rs index 34f593405..12680800d 100644 --- a/ibc-derive/src/client_state/traits/client_state_common.rs +++ b/ibc-derive/src/client_state/traits/client_state_common.rs @@ -47,12 +47,24 @@ pub(crate) fn impl_ClientStateCommon( quote! {serialize_path(cs, path)}, imports, ); + let verify_membership_raw_impl = delegate_call_in_match( + client_state_enum_name, + enum_variants.iter(), + quote! {verify_membership_raw(cs, prefix, proof, root, path, value)}, + imports, + ); let verify_membership_impl = delegate_call_in_match( client_state_enum_name, enum_variants.iter(), quote! {verify_membership(cs, prefix, proof, root, path, value)}, imports, ); + let verify_non_membership_raw_impl = delegate_call_in_match( + client_state_enum_name, + enum_variants.iter(), + quote! {verify_non_membership_raw(cs, prefix, proof, root, path)}, + imports, + ); let verify_non_membership_impl = delegate_call_in_match( client_state_enum_name, enum_variants.iter(), @@ -117,25 +129,50 @@ pub(crate) fn impl_ClientStateCommon( } } - fn verify_membership( + fn verify_membership_raw( &self, prefix: &#CommitmentPrefix, proof: &#CommitmentProofBytes, root: &#CommitmentRoot, path: #PathBytes, value: Vec, + ) -> core::result::Result<(), #ClientError> { + match self { + #(#verify_membership_raw_impl),* + } + } + + fn verify_membership( + &self, + prefix: &#CommitmentPrefix, + proof: &#CommitmentProofBytes, + root: &#CommitmentRoot, + path: impl Into<#Path>, + value: Vec, ) -> core::result::Result<(), #ClientError> { match self { #(#verify_membership_impl),* } } - fn verify_non_membership( + fn verify_non_membership_raw( &self, prefix: &#CommitmentPrefix, proof: &#CommitmentProofBytes, root: &#CommitmentRoot, path: #PathBytes, + ) -> core::result::Result<(), #ClientError> { + match self { + #(#verify_non_membership_raw_impl),* + } + } + + fn verify_non_membership( + &self, + prefix: &#CommitmentPrefix, + proof: &#CommitmentProofBytes, + root: &#CommitmentRoot, + path: impl Into<#Path>, ) -> core::result::Result<(), #ClientError> { match self { #(#verify_non_membership_impl),* diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs index 6319b4b64..14d88772e 100644 --- a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs +++ b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs @@ -207,7 +207,7 @@ impl ClientStateCommon for MockClientState { Ok(()) } - fn verify_membership( + fn verify_membership_raw( &self, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, @@ -218,7 +218,7 @@ impl ClientStateCommon for MockClientState { Ok(()) } - fn verify_non_membership( + fn verify_non_membership_raw( &self, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, diff --git a/tests-integration/tests/core/ics02_client/create_client.rs b/tests-integration/tests/core/ics02_client/create_client.rs index 170baf42f..fcdf9e8ea 100644 --- a/tests-integration/tests/core/ics02_client/create_client.rs +++ b/tests-integration/tests/core/ics02_client/create_client.rs @@ -195,9 +195,7 @@ fn test_tm_create_client_proof_verification_ok() { &ctx_tm.ibc_store().commitment_prefix(), &proof, &root, - tm_client_state - .serialize_path(next_client_seq_path.clone()) - .expect("path"), + next_client_seq_path.clone(), serde_json::to_vec(&next_client_seq_value).expect("valid json serialization"), ) .expect("successful proof verification"); @@ -209,9 +207,7 @@ fn test_tm_create_client_proof_verification_ok() { &ctx_tm.ibc_store().commitment_prefix(), &proof, &root, - tm_client_state - .serialize_path(next_client_seq_path.clone()) - .expect("path"), + next_client_seq_path, serde_json::to_vec(&(next_client_seq_value + 1)).expect("valid json serialization"), ) .expect_err("proof verification fails"), From 90ccb29f8cd177c1788f31596df520088ecce7df Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Thu, 11 Jul 2024 18:27:37 -0700 Subject: [PATCH 08/13] nit: reword changelog --- .../1255-enable-custom-paths-for-proof-verificaitons.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md b/.changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md index 66edab29c..3caf85de4 100644 --- a/.changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md +++ b/.changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md @@ -1,4 +1,4 @@ -- [ibc-core-client] Enable `verify_(non_)membership()` methods to accept custom - paths as bytes by defining a new `serializer_path()` API allowing light client +- [ibc-core-client] Enable proof verification methods to accept custom paths as + bytes by defining a new `serializer_path()` API allowing light client developers to introduce the path serialization behavior of their system. ([\#1255](https://github.com/cosmos/ibc-rs/issues/1255)) From 6e42616d71689360535680d07adde8f2dc36bc75 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Fri, 12 Jul 2024 16:19:36 +0200 Subject: [PATCH 09/13] fix grammar --- ibc-core/ics24-host/types/src/path.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ibc-core/ics24-host/types/src/path.rs b/ibc-core/ics24-host/types/src/path.rs index f6af11c4f..b8da20d89 100644 --- a/ibc-core/ics24-host/types/src/path.rs +++ b/ibc-core/ics24-host/types/src/path.rs @@ -44,7 +44,7 @@ pub const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState"; /// Represents a general-purpose path structure using the byte representation of /// a path. This struct abstracts over different path types and can handle bytes -/// may obtained from various serialization formats (e.g., Protobuf, Borsh). +/// obtained from various serialization formats (e.g., Protobuf, Borsh). #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, From)] From e5dfbfb797312f46a65b019551deebf026634987 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 12 Jul 2024 07:21:59 -0700 Subject: [PATCH 10/13] imp: rename to flatten + revert to Path::*(*) --- ibc-clients/cw-context/src/types/msgs.rs | 4 ++-- .../ics07-tendermint/src/client_state/common.rs | 14 ++++++++------ ibc-core/ics02-client/context/src/client_state.rs | 6 +++--- .../ics03-connection/src/handler/conn_open_ack.rs | 8 ++++---- .../src/handler/conn_open_confirm.rs | 4 ++-- .../ics03-connection/src/handler/conn_open_try.rs | 8 ++++---- .../ics04-channel/src/handler/acknowledgement.rs | 4 ++-- .../src/handler/chan_close_confirm.rs | 4 ++-- .../ics04-channel/src/handler/chan_open_ack.rs | 4 ++-- .../ics04-channel/src/handler/chan_open_confirm.rs | 4 ++-- .../ics04-channel/src/handler/chan_open_try.rs | 4 ++-- ibc-core/ics04-channel/src/handler/recv_packet.rs | 5 +++-- ibc-core/ics04-channel/src/handler/timeout.rs | 6 +++--- .../ics04-channel/src/handler/timeout_on_close.rs | 8 ++++---- ibc-core/ics24-host/types/src/path.rs | 4 ++-- 15 files changed, 45 insertions(+), 42 deletions(-) diff --git a/ibc-clients/cw-context/src/types/msgs.rs b/ibc-clients/cw-context/src/types/msgs.rs index da92f67bc..1a7da1a4c 100644 --- a/ibc-clients/cw-context/src/types/msgs.rs +++ b/ibc-clients/cw-context/src/types/msgs.rs @@ -141,7 +141,7 @@ impl TryFrom for VerifyMembershipMsg { fn try_from(mut raw: VerifyMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof.to_vec())?; let prefix = CommitmentPrefix::from(raw.path.key_path.remove(0).into_vec()); - let path = PathBytes::concat(raw.path.key_path); + let path = PathBytes::flatten(raw.path.key_path); let height = Height::try_from(raw.height)?; Ok(Self { @@ -180,7 +180,7 @@ impl TryFrom for VerifyNonMembershipMsg { fn try_from(mut raw: VerifyNonMembershipMsgRaw) -> Result { let proof = CommitmentProofBytes::try_from(raw.proof.to_vec())?; let prefix = CommitmentPrefix::from(raw.path.key_path.remove(0).into_vec()); - let path = PathBytes::concat(raw.path.key_path); + let path = PathBytes::flatten(raw.path.key_path); let height = raw.height.try_into()?; Ok(Self { diff --git a/ibc-clients/ics07-tendermint/src/client_state/common.rs b/ibc-clients/ics07-tendermint/src/client_state/common.rs index be719baf7..c4726871e 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/common.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/common.rs @@ -46,11 +46,13 @@ impl ClientStateCommon for ClientState { ) -> Result<(), ClientError> { let last_height = self.latest_height().revision_height(); - let upgrade_client_path_bytes = - self.serialize_path(UpgradeClientPath::UpgradedClientState(last_height))?; + let upgrade_client_path_bytes = self.serialize_path(Path::UpgradeClient( + UpgradeClientPath::UpgradedClientState(last_height), + ))?; - let upgrade_consensus_path_bytes = - self.serialize_path(UpgradeClientPath::UpgradedClientConsensusState(last_height))?; + let upgrade_consensus_path_bytes = self.serialize_path(Path::UpgradeClient( + UpgradeClientPath::UpgradedClientConsensusState(last_height), + ))?; verify_upgrade_client::( self.inner(), @@ -64,8 +66,8 @@ impl ClientStateCommon for ClientState { ) } - fn serialize_path(&self, path: impl Into) -> Result { - Ok(path.into().to_string().into_bytes().into()) + fn serialize_path(&self, path: Path) -> Result { + Ok(path.to_string().into_bytes().into()) } fn verify_membership_raw( diff --git a/ibc-core/ics02-client/context/src/client_state.rs b/ibc-core/ics02-client/context/src/client_state.rs index 33cf8b93a..3464e37cb 100644 --- a/ibc-core/ics02-client/context/src/client_state.rs +++ b/ibc-core/ics02-client/context/src/client_state.rs @@ -58,7 +58,7 @@ pub trait ClientStateCommon: Convertible { /// them to understand how path serialization is performed on the chain this /// light client represents it before passing the path bytes to either /// `verify_membership_raw()` or `verify_non_membership_raw()`. - fn serialize_path(&self, path: impl Into) -> Result; + fn serialize_path(&self, path: Path) -> Result; /// Verifies a proof of the existence of a value at a given raw path bytes. fn verify_membership_raw( @@ -76,7 +76,7 @@ pub trait ClientStateCommon: Convertible { prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, - path: impl Into, + path: Path, value: Vec, ) -> Result<(), ClientError> { let path_bytes = self.serialize_path(path)?; @@ -98,7 +98,7 @@ pub trait ClientStateCommon: Convertible { prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, - path: impl Into, + path: Path, ) -> Result<(), ClientError> { let path_bytes = self.serialize_path(path)?; self.verify_non_membership_raw(prefix, proof, root, path_bytes) diff --git a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs index 582aa3ad3..443d32877 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs @@ -9,7 +9,7 @@ use ibc_core_connection_types::{ConnectionEnd, Counterparty, State}; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::identifiers::ClientId; -use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath, ConnectionPath}; +use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath, ConnectionPath, Path}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_primitives::prelude::*; use ibc_primitives::proto::{Any, Protobuf}; @@ -101,7 +101,7 @@ where prefix_on_b, &msg.proof_conn_end_on_b, consensus_state_of_b_on_a.root(), - ConnectionPath::new(&msg.conn_id_on_b), + Path::Connection(ConnectionPath::new(&msg.conn_id_on_b)), expected_conn_end_on_b.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; @@ -112,7 +112,7 @@ where prefix_on_b, &msg.proof_client_state_of_a_on_b, consensus_state_of_b_on_a.root(), - ClientStatePath::new(vars.client_id_on_b().clone()), + Path::ClientState(ClientStatePath::new(vars.client_id_on_b().clone())), msg.client_state_of_a_on_b.to_vec(), ) .map_err(|e| ConnectionError::ClientStateVerificationFailure { @@ -137,7 +137,7 @@ where prefix_on_b, &msg.proof_consensus_state_of_a_on_b, consensus_state_of_b_on_a.root(), - client_cons_state_path_on_b, + Path::ClientConsensusState(client_cons_state_path_on_b), stored_consensus_state_of_a_on_b.to_vec(), ) .map_err(|e| ConnectionError::ConsensusStateVerificationFailure { diff --git a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs index 81630ea4d..f9ef4eb61 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_confirm.rs @@ -8,7 +8,7 @@ use ibc_core_connection_types::{ConnectionEnd, Counterparty, State}; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::identifiers::{ClientId, ConnectionId}; -use ibc_core_host::types::path::{ClientConsensusStatePath, ConnectionPath}; +use ibc_core_host::types::path::{ClientConsensusStatePath, ConnectionPath, Path}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_primitives::prelude::*; use ibc_primitives::proto::Protobuf; @@ -78,7 +78,7 @@ where prefix_on_a, &msg.proof_conn_end_on_a, consensus_state_of_a_on_b.root(), - ConnectionPath::new(conn_id_on_a), + Path::Connection(ConnectionPath::new(conn_id_on_a)), expected_conn_end_on_a.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; diff --git a/ibc-core/ics03-connection/src/handler/conn_open_try.rs b/ibc-core/ics03-connection/src/handler/conn_open_try.rs index 16205b9ab..4b59913e7 100644 --- a/ibc-core/ics03-connection/src/handler/conn_open_try.rs +++ b/ibc-core/ics03-connection/src/handler/conn_open_try.rs @@ -9,7 +9,7 @@ use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::identifiers::{ClientId, ConnectionId}; use ibc_core_host::types::path::{ - ClientConnectionPath, ClientConsensusStatePath, ClientStatePath, ConnectionPath, + ClientConnectionPath, ClientConsensusStatePath, ClientStatePath, ConnectionPath, Path, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_primitives::prelude::*; @@ -97,7 +97,7 @@ where prefix_on_a, &msg.proof_conn_end_on_a, consensus_state_of_a_on_b.root(), - ConnectionPath::new(&vars.conn_id_on_a), + Path::Connection(ConnectionPath::new(&vars.conn_id_on_a)), expected_conn_end_on_a.encode_vec(), ) .map_err(ConnectionError::VerifyConnectionState)?; @@ -108,7 +108,7 @@ where prefix_on_a, &msg.proof_client_state_of_b_on_a, consensus_state_of_a_on_b.root(), - ClientStatePath::new(client_id_on_a.clone()), + Path::ClientState(ClientStatePath::new(client_id_on_a.clone())), msg.client_state_of_b_on_a.to_vec(), ) .map_err(|e| ConnectionError::ClientStateVerificationFailure { @@ -133,7 +133,7 @@ where prefix_on_a, &msg.proof_consensus_state_of_b_on_a, consensus_state_of_a_on_b.root(), - client_cons_state_path_on_a, + Path::ClientConsensusState(client_cons_state_path_on_a), stored_consensus_state_of_b_on_a.to_vec(), ) .map_err(|e| ConnectionError::ConsensusStateVerificationFailure { diff --git a/ibc-core/ics04-channel/src/handler/acknowledgement.rs b/ibc-core/ics04-channel/src/handler/acknowledgement.rs index 9d6ab2838..c58edc74a 100644 --- a/ibc-core/ics04-channel/src/handler/acknowledgement.rs +++ b/ibc-core/ics04-channel/src/handler/acknowledgement.rs @@ -9,7 +9,7 @@ use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::path::{ - AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, SeqAckPath, + AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, SeqAckPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; @@ -196,7 +196,7 @@ where conn_end_on_a.counterparty().prefix(), &msg.proof_acked_on_b, consensus_state_of_b_on_a.root(), - ack_path_on_b, + Path::Ack(ack_path_on_b), ack_commitment.into_vec(), ) .map_err(|e| ChannelError::PacketVerificationFailed { diff --git a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs index 4c5e5b55b..5385d18c3 100644 --- a/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_close_confirm.rs @@ -8,7 +8,7 @@ use ibc_core_client::context::prelude::*; use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath}; +use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath, Path}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; @@ -156,7 +156,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - chan_end_path_on_a, + Path::ChannelEnd(chan_end_path_on_a), expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs index 9857279b7..7e5b6b240 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_ack.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_ack.rs @@ -7,7 +7,7 @@ use ibc_core_client::context::prelude::*; use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath}; +use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath, Path}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; @@ -149,7 +149,7 @@ where prefix_on_b, &msg.proof_chan_end_on_b, consensus_state_of_b_on_a.root(), - chan_end_path_on_b, + Path::ChannelEnd(chan_end_path_on_b), expected_chan_end_on_b.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs index 25dba15a4..dfbc777c1 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_confirm.rs @@ -8,7 +8,7 @@ use ibc_core_client::context::prelude::*; use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath}; +use ibc_core_host::types::path::{ChannelEndPath, ClientConsensusStatePath, Path}; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; use ibc_primitives::prelude::*; @@ -156,7 +156,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - chan_end_path_on_a, + Path::ChannelEnd(chan_end_path_on_a), expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/chan_open_try.rs b/ibc-core/ics04-channel/src/handler/chan_open_try.rs index eefcf1c2b..08c82deac 100644 --- a/ibc-core/ics04-channel/src/handler/chan_open_try.rs +++ b/ibc-core/ics04-channel/src/handler/chan_open_try.rs @@ -10,7 +10,7 @@ use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::identifiers::ChannelId; use ibc_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, SeqAckPath, SeqRecvPath, SeqSendPath, + ChannelEndPath, ClientConsensusStatePath, Path, SeqAckPath, SeqRecvPath, SeqSendPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; @@ -175,7 +175,7 @@ where prefix_on_a, &msg.proof_chan_end_on_a, consensus_state_of_a_on_b.root(), - chan_end_path_on_a, + Path::ChannelEnd(chan_end_path_on_a), expected_chan_end_on_a.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed)?; diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs index 5d5e4ffac..8c0cd25a1 100644 --- a/ibc-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-core/ics04-channel/src/handler/recv_packet.rs @@ -10,7 +10,8 @@ use ibc_core_connection::types::State as ConnectionState; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::path::{ - AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, ReceiptPath, SeqRecvPath, + AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, + SeqRecvPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; @@ -214,7 +215,7 @@ where conn_end_on_b.counterparty().prefix(), &msg.proof_commitment_on_a, consensus_state_of_a_on_b.root(), - commitment_path_on_a, + Path::Commitment(commitment_path_on_a), expected_commitment_on_a.into_vec(), ) .map_err(|e| ChannelError::PacketVerificationFailed { diff --git a/ibc-core/ics04-channel/src/handler/timeout.rs b/ibc-core/ics04-channel/src/handler/timeout.rs index 471b8d43e..ef478ed4e 100644 --- a/ibc-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-core/ics04-channel/src/handler/timeout.rs @@ -8,7 +8,7 @@ use ibc_core_connection::delay::verify_conn_delay_passed; use ibc_core_handler_types::error::ContextError; use ibc_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, CommitmentPath, ReceiptPath, SeqRecvPath, + ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, SeqRecvPath, }; use ibc_core_host::{ExecutionContext, ValidationContext}; use ibc_core_router::module::Module; @@ -225,7 +225,7 @@ where conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - seq_recv_path_on_b, + Path::SeqRecv(seq_recv_path_on_b), msg.packet.seq_on_a.to_vec(), ) } @@ -240,7 +240,7 @@ where conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - receipt_path_on_b, + Path::Receipt(receipt_path_on_b), ) } Order::None => { diff --git a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs index d30d9c060..8366a5d26 100644 --- a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs +++ b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs @@ -6,7 +6,7 @@ use ibc_core_client::context::prelude::*; use ibc_core_connection::delay::verify_conn_delay_passed; use ibc_core_handler_types::error::ContextError; use ibc_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, CommitmentPath, ReceiptPath, SeqRecvPath, + ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, SeqRecvPath, }; use ibc_core_host::ValidationContext; use ibc_primitives::prelude::*; @@ -111,7 +111,7 @@ where prefix_on_b, &msg.proof_close_on_b, consensus_state_of_b_on_a.root(), - chan_end_path_on_b, + Path::ChannelEnd(chan_end_path_on_b), expected_chan_end_on_b.encode_vec(), ) .map_err(ChannelError::VerifyChannelFailed) @@ -135,7 +135,7 @@ where conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - seq_recv_path_on_b, + Path::SeqRecv(seq_recv_path_on_b), packet.seq_on_a.to_vec(), ) } @@ -150,7 +150,7 @@ where conn_end_on_a.counterparty().prefix(), &msg.proof_unreceived_on_b, consensus_state_of_b_on_a.root(), - receipt_path_on_b, + Path::Receipt(receipt_path_on_b), ) } Order::None => { diff --git a/ibc-core/ics24-host/types/src/path.rs b/ibc-core/ics24-host/types/src/path.rs index b8da20d89..c9e384989 100644 --- a/ibc-core/ics24-host/types/src/path.rs +++ b/ibc-core/ics24-host/types/src/path.rs @@ -59,8 +59,8 @@ impl PathBytes { self.0 } - /// Concatenates a list of path bytes into a single path. - pub fn concat(paths: Vec) -> Self { + /// Flattens a list of path bytes into a single path. + pub fn flatten(paths: Vec) -> Self { let mut bytes = Vec::new(); paths.iter().for_each(|path| { bytes.extend_from_slice(&path.0); From 3f7f90a6df6ffaa219532265c90c6f65881b63d5 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Fri, 12 Jul 2024 16:23:15 +0200 Subject: [PATCH 11/13] fix typo --- ibc-core/ics23-commitment/types/src/merkle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ibc-core/ics23-commitment/types/src/merkle.rs b/ibc-core/ics23-commitment/types/src/merkle.rs index 7b65243a8..d597ef1c8 100644 --- a/ibc-core/ics23-commitment/types/src/merkle.rs +++ b/ibc-core/ics23-commitment/types/src/merkle.rs @@ -50,7 +50,7 @@ impl From for MerklePath { } } -// The conversion from `MerklePath`` to `RawMerklePath` is not provided as we +// The conversion from `MerklePath` to `RawMerklePath` is not provided, as we // cannot assume how the key paths of `Vec` type should be serialized // to the `Vec`. From 8f7f0aa1adf10fd232ea0316bdc3a8bbb8ee48b6 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 12 Jul 2024 07:30:32 -0700 Subject: [PATCH 12/13] fix: make clippy happy --- ibc-derive/src/client_state/traits/client_state_common.rs | 6 +++--- ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs | 4 ++-- tests-integration/tests/core/ics02_client/create_client.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ibc-derive/src/client_state/traits/client_state_common.rs b/ibc-derive/src/client_state/traits/client_state_common.rs index 12680800d..0184d9a23 100644 --- a/ibc-derive/src/client_state/traits/client_state_common.rs +++ b/ibc-derive/src/client_state/traits/client_state_common.rs @@ -123,7 +123,7 @@ pub(crate) fn impl_ClientStateCommon( } } - fn serialize_path(&self, path: impl Into<#Path>) -> core::result::Result<#PathBytes, #ClientError> { + fn serialize_path(&self, path: #Path) -> core::result::Result<#PathBytes, #ClientError> { match self { #(#serialize_path_impl),* } @@ -147,7 +147,7 @@ pub(crate) fn impl_ClientStateCommon( prefix: &#CommitmentPrefix, proof: &#CommitmentProofBytes, root: &#CommitmentRoot, - path: impl Into<#Path>, + path: #Path, value: Vec, ) -> core::result::Result<(), #ClientError> { match self { @@ -172,7 +172,7 @@ pub(crate) fn impl_ClientStateCommon( prefix: &#CommitmentPrefix, proof: &#CommitmentProofBytes, root: &#CommitmentRoot, - path: impl Into<#Path>, + path: #Path, ) -> core::result::Result<(), #ClientError> { match self { #(#verify_non_membership_impl),* diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs index 14d88772e..ddcb0c684 100644 --- a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs +++ b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs @@ -184,8 +184,8 @@ impl ClientStateCommon for MockClientState { Ok(()) } - fn serialize_path(&self, path: impl Into) -> Result { - Ok(path.into().to_string().into_bytes().into()) + fn serialize_path(&self, path: Path) -> Result { + Ok(path.to_string().into_bytes().into()) } fn verify_upgrade_client( diff --git a/tests-integration/tests/core/ics02_client/create_client.rs b/tests-integration/tests/core/ics02_client/create_client.rs index fcdf9e8ea..4e5dd9e77 100644 --- a/tests-integration/tests/core/ics02_client/create_client.rs +++ b/tests-integration/tests/core/ics02_client/create_client.rs @@ -195,7 +195,7 @@ fn test_tm_create_client_proof_verification_ok() { &ctx_tm.ibc_store().commitment_prefix(), &proof, &root, - next_client_seq_path.clone(), + next_client_seq_path.clone().into(), serde_json::to_vec(&next_client_seq_value).expect("valid json serialization"), ) .expect("successful proof verification"); @@ -207,7 +207,7 @@ fn test_tm_create_client_proof_verification_ok() { &ctx_tm.ibc_store().commitment_prefix(), &proof, &root, - next_client_seq_path, + next_client_seq_path.into(), serde_json::to_vec(&(next_client_seq_value + 1)).expect("valid json serialization"), ) .expect_err("proof verification fails"), From 426ca1516117797f5c8f34297490ca838cab8917 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 12 Jul 2024 07:41:11 -0700 Subject: [PATCH 13/13] fix: move changelog to features --- .../1255-enable-custom-paths-for-proof-verificaitons.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/unreleased/{breaking-changes => features}/1255-enable-custom-paths-for-proof-verificaitons.md (100%) diff --git a/.changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md b/.changelog/unreleased/features/1255-enable-custom-paths-for-proof-verificaitons.md similarity index 100% rename from .changelog/unreleased/breaking-changes/1255-enable-custom-paths-for-proof-verificaitons.md rename to .changelog/unreleased/features/1255-enable-custom-paths-for-proof-verificaitons.md