Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imp: Enable custom paths for light client proof verifications #1273

Merged
merged 15 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@

let consensus_state = self.consensus_state(&client_cons_state_path)?;

client_state.verify_membership(
client_state.verify_membership_raw(

Check warning on line 73 in ibc-clients/cw-context/src/handlers.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/cw-context/src/handlers.rs#L73

Added line #L73 was not covered by tests
&msg.prefix,
&msg.proof,
consensus_state.root(),
Expand All @@ -91,7 +91,7 @@

let consensus_state = self.consensus_state(&client_cons_state_path)?;

client_state.verify_non_membership(
client_state.verify_non_membership_raw(

Check warning on line 94 in ibc-clients/cw-context/src/handlers.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/cw-context/src/handlers.rs#L94

Added line #L94 was not covered by tests
&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 @@
}
}

#[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 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 @@

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::concat(raw.path.key_path);

Check warning on line 144 in ibc-clients/cw-context/src/types/msgs.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/cw-context/src/types/msgs.rs#L143-L144

Added lines #L143 - L144 were not covered by tests
let height = Height::try_from(raw.height)?;

Ok(Self {
proof,
path,
value: raw.value.into(),
height,
prefix: CommitmentPrefix::from(prefix),
prefix,

Check warning on line 152 in ibc-clients/cw-context/src/types/msgs.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/cw-context/src/types/msgs.rs#L152

Added line #L152 was not covered by tests
delay_block_period: raw.delay_block_period,
delay_time_period: raw.delay_time_period,
})
Expand All @@ -175,7 +168,7 @@
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 @@

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::concat(raw.path.key_path);

Check warning on line 183 in ibc-clients/cw-context/src/types/msgs.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/cw-context/src/types/msgs.rs#L182-L183

Added lines #L182 - L183 were not covered by tests
let height = raw.height.try_into()?;

Ok(Self {
proof,
path,
height,
prefix: CommitmentPrefix::from(prefix),
prefix,

Check warning on line 190 in ibc-clients/cw-context/src/types/msgs.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/cw-context/src/types/msgs.rs#L190

Added line #L190 was not covered by tests
delay_block_period: raw.delay_block_period,
delay_time_period: raw.delay_time_period,
})
Expand Down
43 changes: 29 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 @@
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,36 @@
proof_upgrade_consensus_state: CommitmentProofBytes,
root: &CommitmentRoot,
) -> Result<(), ClientError> {
let last_height = self.latest_height().revision_height();

Check warning on line 47 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L47

Added line #L47 was not covered by tests

let upgrade_client_path_bytes =
self.serialize_path(UpgradeClientPath::UpgradedClientState(last_height))?;

Check warning on line 50 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L49-L50

Added lines #L49 - L50 were not covered by tests

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

Check warning on line 53 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L52-L53

Added lines #L52 - L53 were not covered by tests

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,

Check warning on line 62 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L61-L62

Added lines #L61 - L62 were not covered by tests
root,
)
}

fn verify_membership(
fn serialize_path(&self, path: impl Into<Path>) -> Result<PathBytes, ClientError> {
Ok(path.into().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 +86,12 @@
)
}

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 +154,15 @@
/// 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,

Check warning on line 165 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L164-L165

Added lines #L164 - L165 were not covered by tests
root: &CommitmentRoot,
) -> Result<(), ClientError> {
// Make sure that the client type is of Tendermint type `ClientState`
Expand Down Expand Up @@ -178,15 +195,13 @@

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,

Check warning on line 204 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L204

Added line #L204 was not covered by tests
upgraded_client_state.to_vec(),
)?;

Expand All @@ -196,7 +211,7 @@
&upgrade_path_prefix,
&proof_upgrade_consensus_state,
root,
Path::UpgradeClient(UpgradeClientPath::UpgradedClientConsensusState(last_height)),
upgrade_consensus_path_bytes,

Check warning on line 214 in ibc-clients/ics07-tendermint/src/client_state/common.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state/common.rs#L214

Added line #L214 was not covered by tests
upgraded_consensus_state.to_vec(),
)?;

Expand All @@ -213,7 +228,7 @@
prefix: &CommitmentPrefix,
proof: &CommitmentProofBytes,
root: &CommitmentRoot,
path: Path,
path: PathBytes,
value: Vec<u8>,
) -> Result<(), ClientError> {
if prefix.is_empty() {
Expand All @@ -222,7 +237,7 @@
));
}

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 +255,9 @@
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
47 changes: 39 additions & 8 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: impl Into<Path>) -> Result<PathBytes, ClientError>;
rnbguy marked this conversation as resolved.
Show resolved Hide resolved

/// 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,
path: impl Into<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>;
path: impl Into<Path>,
) -> 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
8 changes: 4 additions & 4 deletions ibc-core/ics03-connection/src/handler/conn_open_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -101,7 +101,7 @@ 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)),
ConnectionPath::new(&msg.conn_id_on_b),
expected_conn_end_on_b.encode_vec(),
)
.map_err(ConnectionError::VerifyConnectionState)?;
Expand All @@ -112,7 +112,7 @@ where
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())),
Farhad-Shabani marked this conversation as resolved.
Show resolved Hide resolved
ClientStatePath::new(vars.client_id_on_b().clone()),
msg.client_state_of_a_on_b.to_vec(),
)
.map_err(|e| ConnectionError::ClientStateVerificationFailure {
Expand All @@ -137,7 +137,7 @@ where
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),
client_cons_state_path_on_b,
stored_consensus_state_of_a_on_b.to_vec(),
)
.map_err(|e| ConnectionError::ConsensusStateVerificationFailure {
Expand Down
4 changes: 2 additions & 2 deletions ibc-core/ics03-connection/src/handler/conn_open_confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -78,7 +78,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)),
ConnectionPath::new(conn_id_on_a),
expected_conn_end_on_a.encode_vec(),
)
.map_err(ConnectionError::VerifyConnectionState)?;
Expand Down
Loading
Loading