Skip to content

Commit

Permalink
imp: Enable custom paths for light client proof verifications (#1273)
Browse files Browse the repository at this point in the history
* refactor: allow using custom path for light client proof verification

* imp: 2nd try, place serialize_path under the ClientStateCommon

* feat: define concat() for PathBytes

* fix: update cw-check cargo.lock

* chore: add docs and unclog

* misc: apply some fixes

* imp: group serialize_path and verify_(non_)membership_raw methods

* nit: reword changelog

* fix grammar

* imp: rename to flatten + revert to Path::*(*)

* fix typo

* fix: make clippy happy

* fix: move changelog to features

---------

Co-authored-by: Ranadeep Biswas <[email protected]>
  • Loading branch information
Farhad-Shabani and rnbguy authored Jul 12, 2024
1 parent 0202dec commit e36015c
Show file tree
Hide file tree
Showing 13 changed files with 236 additions and 56 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- [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))
1 change: 1 addition & 0 deletions ci/cw-check/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions ci/no-std-check/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions ibc-clients/cw-context/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
29 changes: 11 additions & 18 deletions ibc-clients/cw-context/src/types/msgs.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
//! 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 cosmwasm_std::{Binary, Checksum};
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;

Expand Down Expand Up @@ -116,11 +115,6 @@ impl TryFrom<VerifyUpgradeAndUpdateStateMsgRaw> for VerifyUpgradeAndUpdateStateM
}
}

#[cw_serde]
pub struct MerklePath {
pub key_path: Vec<String>,
}

#[cw_serde]
pub struct VerifyMembershipMsgRaw {
pub proof: Binary,
Expand All @@ -134,7 +128,7 @@ pub struct VerifyMembershipMsgRaw {
pub struct VerifyMembershipMsg {
pub prefix: CommitmentPrefix,
pub proof: CommitmentProofBytes,
pub path: Path,
pub path: PathBytes,
pub value: Vec<u8>,
pub height: Height,
pub delay_block_period: u64,
Expand All @@ -146,17 +140,16 @@ impl TryFrom<VerifyMembershipMsgRaw> for VerifyMembershipMsg {

fn try_from(mut raw: VerifyMembershipMsgRaw) -> Result<Self, Self::Error> {
let proof = CommitmentProofBytes::try_from(raw.proof.to_vec())?;
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 = CommitmentPrefix::from(raw.path.key_path.remove(0).into_vec());
let path = PathBytes::flatten(raw.path.key_path);
let height = Height::try_from(raw.height)?;

Ok(Self {
proof,
path,
value: raw.value.into(),
height,
prefix: CommitmentPrefix::from(prefix),
prefix,
delay_block_period: raw.delay_block_period,
delay_time_period: raw.delay_time_period,
})
Expand All @@ -175,7 +168,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,
Expand All @@ -186,15 +179,15 @@ impl TryFrom<VerifyNonMembershipMsgRaw> for VerifyNonMembershipMsg {

fn try_from(mut raw: VerifyNonMembershipMsgRaw) -> Result<Self, Self::Error> {
let proof = CommitmentProofBytes::try_from(raw.proof.to_vec())?;
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 = CommitmentPrefix::from(raw.path.key_path.remove(0).into_vec());
let path = PathBytes::flatten(raw.path.key_path);
let height = raw.height.try_into()?;

Ok(Self {
proof,
path,
height,
prefix: CommitmentPrefix::from(prefix),
prefix,
delay_block_period: raw.delay_block_period,
delay_time_period: raw.delay_time_period,
})
Expand Down
45 changes: 31 additions & 14 deletions ibc-clients/ics07-tendermint/src/client_state/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ use ibc_core_commitment_types::commitment::{
CommitmentPrefix, CommitmentProofBytes, CommitmentRoot,
};
use ibc_core_commitment_types::error::CommitmentError;
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::{Path, PathBytes, UpgradeClientPath};
use ibc_primitives::prelude::*;
use ibc_primitives::proto::Any;
use ibc_primitives::ToVec;
Expand Down Expand Up @@ -44,22 +44,38 @@ impl ClientStateCommon for ClientState {
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(Path::UpgradeClient(
UpgradeClientPath::UpgradedClientState(last_height),
))?;

let upgrade_consensus_path_bytes = self.serialize_path(Path::UpgradeClient(
UpgradeClientPath::UpgradedClientConsensusState(last_height),
))?;

verify_upgrade_client::<HostFunctionsManager>(
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 verify_membership(
fn serialize_path(&self, path: Path) -> Result<PathBytes, ClientError> {
Ok(path.to_string().into_bytes().into())
}

fn verify_membership_raw(
&self,
prefix: &CommitmentPrefix,
proof: &CommitmentProofBytes,
root: &CommitmentRoot,
path: Path,
path: PathBytes,
value: Vec<u8>,
) -> Result<(), ClientError> {
verify_membership::<HostFunctionsManager>(
Expand All @@ -72,12 +88,12 @@ impl ClientStateCommon for ClientState {
)
}

fn verify_non_membership(
fn verify_non_membership_raw(
&self,
prefix: &CommitmentPrefix,
proof: &CommitmentProofBytes,
root: &CommitmentRoot,
path: Path,
path: PathBytes,
) -> Result<(), ClientError> {
verify_non_membership::<HostFunctionsManager>(
&self.inner().proof_specs,
Expand Down Expand Up @@ -140,12 +156,15 @@ pub fn validate_proof_height(
/// 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<H: HostFunctionsProvider>(
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`
Expand Down Expand Up @@ -178,15 +197,13 @@ pub fn verify_upgrade_client<H: HostFunctionsProvider>(

let upgrade_path_prefix = CommitmentPrefix::from(upgrade_path[0].clone().into_bytes());

let last_height = latest_height.revision_height();

// Verify the proof of the upgraded client state
verify_membership::<H>(
&client_state.proof_specs,
&upgrade_path_prefix,
&proof_upgrade_client,
root,
Path::UpgradeClient(UpgradeClientPath::UpgradedClientState(last_height)),
upgrade_client_path_bytes,
upgraded_client_state.to_vec(),
)?;

Expand All @@ -196,7 +213,7 @@ pub fn verify_upgrade_client<H: HostFunctionsProvider>(
&upgrade_path_prefix,
&proof_upgrade_consensus_state,
root,
Path::UpgradeClient(UpgradeClientPath::UpgradedClientConsensusState(last_height)),
upgrade_consensus_path_bytes,
upgraded_consensus_state.to_vec(),
)?;

Expand All @@ -213,7 +230,7 @@ pub fn verify_membership<H: HostFunctionsProvider>(
prefix: &CommitmentPrefix,
proof: &CommitmentProofBytes,
root: &CommitmentRoot,
path: Path,
path: PathBytes,
value: Vec<u8>,
) -> Result<(), ClientError> {
if prefix.is_empty() {
Expand All @@ -222,7 +239,7 @@ pub fn verify_membership<H: HostFunctionsProvider>(
));
}

let merkle_path = apply_prefix(prefix, vec![path.to_string()]);
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
Expand All @@ -240,9 +257,9 @@ pub fn verify_non_membership<H: HostFunctionsProvider>(
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.as_bytes().to_vec().into(), path]);
let merkle_proof = MerkleProof::try_from(proof).map_err(ClientError::InvalidCommitmentProof)?;

merkle_proof
Expand Down
43 changes: 37 additions & 6 deletions ibc-core/ics02-client/context/src/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{Path, PathBytes};
use ibc_primitives::prelude::*;
use ibc_primitives::proto::Any;

Expand Down Expand Up @@ -52,26 +52,57 @@ pub trait ClientStateCommon: Convertible<Any> {
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.
/// 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 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: Path) -> Result<PathBytes, ClientError>;

/// Verifies a proof of the existence of a value at a given raw path bytes.
fn verify_membership_raw(
&self,
prefix: &CommitmentPrefix,
proof: &CommitmentProofBytes,
root: &CommitmentRoot,
path: PathBytes,
value: Vec<u8>,
) -> Result<(), ClientError>;

/// 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: Path,
value: Vec<u8>,
) -> 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>;

// Verify_non_membership is a generic proof verification method which
// verifies the absence of a given commitment.
/// Verifies the absence of a given proof at a given path object.
fn verify_non_membership(
&self,
prefix: &CommitmentPrefix,
proof: &CommitmentProofBytes,
root: &CommitmentRoot,
path: Path,
) -> Result<(), ClientError>;
) -> 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
Expand Down
12 changes: 9 additions & 3 deletions ibc-core/ics23-commitment/types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -46,30 +47,35 @@ 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",
]
schema = [
"dep:schemars",
"ibc-proto/json-schema",
"ibc-primitives/schema",
"ibc-core-host-types/schema",
"serde",
"std",
]
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",
]
Loading

0 comments on commit e36015c

Please sign in to comment.