From 468b4fd4fa008288e979e63200b47b79b3171f18 Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 22 Nov 2021 19:23:43 +0900 Subject: [PATCH 01/26] proof verification functions --- .../clients/ics07_tendermint/client_def.rs | 214 ++++++++++++------ .../clients/ics07_tendermint/client_state.rs | 14 +- modules/src/clients/ics07_tendermint/error.rs | 7 +- modules/src/core/ics02_client/client_def.rs | 116 +++++----- modules/src/core/ics02_client/error.rs | 10 +- .../ics02_client/handler/create_client.rs | 1 + .../core/ics02_client/msgs/upgrade_client.rs | 4 +- .../core/ics03_connection/handler/verify.rs | 30 ++- .../ics04_channel/handler/acknowledgement.rs | 4 +- .../core/ics04_channel/handler/recv_packet.rs | 4 +- .../src/core/ics04_channel/handler/timeout.rs | 4 +- .../ics04_channel/handler/timeout_on_close.rs | 6 +- .../src/core/ics04_channel/handler/verify.rs | 62 +++-- modules/src/core/ics23_commitment/error.rs | 24 ++ modules/src/core/ics23_commitment/merkle.rs | 162 ++++++++++++- modules/src/core/ics23_commitment/specs.rs | 18 +- modules/src/mock/client_def.rs | 49 ++-- modules/tests/runner/mod.rs | 3 +- proto/Cargo.toml | 1 + proto/src/prost/ics23.rs | 32 +-- relayer/src/chain/cosmos.rs | 2 + relayer/src/chain/mock.rs | 1 + 22 files changed, 534 insertions(+), 234 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index b02b60e07a..fe8a12d286 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -1,9 +1,11 @@ use std::convert::TryInto; -use ibc_proto::ibc::core::commitment::v1::MerkleProof; +use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof; +use prost::Message; use tendermint::Time; use tendermint_light_client::components::verifier::{ProdVerifier, Verdict, Verifier}; use tendermint_light_client::types::{TrustedBlockState, UntrustedBlockState}; +use tendermint_proto::Protobuf; use crate::clients::ics07_tendermint::client_state::ClientState; use crate::clients::ics07_tendermint::consensus_state::ConsensusState; @@ -22,8 +24,10 @@ use crate::core::ics04_channel::packet::Sequence; use crate::core::ics23_commitment::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, }; +use crate::core::ics23_commitment::merkle::{apply_prefix, MerkleProof}; use crate::core::ics24_host::identifier::ConnectionId; use crate::core::ics24_host::identifier::{ChannelId, ClientId, PortId}; +use crate::core::ics24_host::Path; use crate::prelude::*; use crate::Height; @@ -193,116 +197,198 @@ impl ClientDef for TendermintClient { fn verify_client_consensus_state( &self, - _client_state: &Self::ClientState, - _height: Height, - _prefix: &CommitmentPrefix, - _proof: &CommitmentProofBytes, - _client_id: &ClientId, - _consensus_height: Height, - _expected_consensus_state: &AnyConsensusState, + client_state: &Self::ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + client_id: &ClientId, + consensus_height: Height, + expected_consensus_state: &AnyConsensusState, ) -> Result<(), Ics02Error> { - todo!() + let path = Path::ClientConsensusState { + client_id: client_id.clone(), + epoch: consensus_height.revision_number, + height: consensus_height.revision_height, + } + .to_string(); + let value = expected_consensus_state.encode_vec().unwrap(); + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_connection_state( &self, - _client_state: &Self::ClientState, - _height: Height, - _prefix: &CommitmentPrefix, - _proof: &CommitmentProofBytes, - _connection_id: Option<&ConnectionId>, - _expected_connection_end: &ConnectionEnd, + client_state: &Self::ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + connection_id: &ConnectionId, + expected_connection_end: &ConnectionEnd, ) -> Result<(), Ics02Error> { - todo!() + let path = Path::Connections(connection_id.clone()).to_string(); + let value = expected_connection_end.encode_vec().unwrap(); + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_channel_state( &self, - _client_state: &Self::ClientState, - _height: Height, - _prefix: &CommitmentPrefix, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _expected_channel_end: &ChannelEnd, + client_state: &Self::ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + expected_channel_end: &ChannelEnd, ) -> Result<(), Ics02Error> { - todo!() + let path = Path::ChannelEnds(port_id.clone(), channel_id.clone()).to_string(); + let value = expected_channel_end.encode_vec().unwrap(); + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_client_full_state( &self, - _client_state: &Self::ClientState, - _height: Height, - _root: &CommitmentRoot, - _prefix: &CommitmentPrefix, - _client_id: &ClientId, - _proof: &CommitmentProofBytes, - _expected_client_state: &AnyClientState, + client_state: &Self::ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + client_id: &ClientId, + expected_client_state: &AnyClientState, ) -> Result<(), Ics02Error> { - unimplemented!() + let path = Path::ClientState(client_id.clone()).to_string(); + let value = expected_client_state.encode_vec().unwrap(); + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_packet_data( &self, - _client_state: &Self::ClientState, - _height: Height, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: &Sequence, - _data: String, + client_state: &Self::ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + commitment: String, ) -> Result<(), Ics02Error> { - todo!() + let path = Path::Commitments { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + } + .to_string(); + let value = commitment.encode_to_vec(); + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_packet_acknowledgement( &self, - _client_state: &Self::ClientState, - _height: Height, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: &Sequence, - _data: Vec, + client_state: &Self::ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + ack: Vec, ) -> Result<(), Ics02Error> { - todo!() + let path = Path::Acks { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + } + .to_string(); + verify_membership(client_state, prefix, proof, root, path, ack) } fn verify_next_sequence_recv( &self, - _client_state: &Self::ClientState, - _height: Height, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: &Sequence, + client_state: &Self::ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + seq: Sequence, ) -> Result<(), Ics02Error> { - todo!() + let path = Path::SeqRecvs(port_id.clone(), channel_id.clone()).to_string(); + let value = u64::from(seq).encode_to_vec(); + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_packet_receipt_absence( &self, - _client_state: &Self::ClientState, - _height: Height, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: &Sequence, + client_state: &Self::ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Ics02Error> { - todo!() + let path = Path::Receipts { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + } + .to_string(); + verify_non_membership(client_state, prefix, proof, root, path) } fn verify_upgrade_and_update_state( &self, _client_state: &Self::ClientState, _consensus_state: &Self::ConsensusState, - _proof_upgrade_client: MerkleProof, - _proof_upgrade_consensus_state: MerkleProof, + _proof_upgrade_client: RawMerkleProof, + _proof_upgrade_consensus_state: RawMerkleProof, ) -> Result<(Self::ClientState, Self::ConsensusState), Ics02Error> { todo!() } } +fn verify_membership( + client_state: &ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + path: String, + value: Vec, +) -> Result<(), Ics02Error> { + let merkle_path = apply_prefix(prefix, vec![path]).map_err(Error::ics23_error)?; + let merkle_proof: MerkleProof = RawMerkleProof::try_from(proof.clone()) + .map_err(Ics02Error::invalid_consensus_state_proof)? + .into(); + + merkle_proof + .verify_membership( + client_state.proof_specs.clone().into(), + root.clone().into(), + merkle_path, + value, + 0, + ) + .map_err(|e| Ics02Error::tendermint(Error::ics23_error(e))) +} + +fn verify_non_membership( + client_state: &ClientState, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + path: String, +) -> Result<(), Ics02Error> { + let merkle_path = apply_prefix(prefix, vec![path]).map_err(Error::ics23_error)?; + let merkle_proof: MerkleProof = RawMerkleProof::try_from(proof.clone()) + .map_err(Ics02Error::invalid_consensus_state_proof)? + .into(); + + merkle_proof + .verify_non_membership( + client_state.proof_specs.clone().into(), + root.clone().into(), + merkle_path, + ) + .map_err(|e| Ics02Error::tendermint(Error::ics23_error(e))) +} + fn downcast_consensus_state(cs: AnyConsensusState) -> Result { downcast!( cs => AnyConsensusState::Tendermint diff --git a/modules/src/clients/ics07_tendermint/client_state.rs b/modules/src/clients/ics07_tendermint/client_state.rs index 3460b6b8d6..6cbba51823 100644 --- a/modules/src/clients/ics07_tendermint/client_state.rs +++ b/modules/src/clients/ics07_tendermint/client_state.rs @@ -8,6 +8,7 @@ use tendermint_light_client::light_client::Options; use tendermint_proto::Protobuf; use ibc_proto::ibc::lightclients::tendermint::v1::ClientState as RawClientState; +use ibc_proto::ics23::ProofSpec; use crate::clients::ics07_tendermint::error::Error; use crate::clients::ics07_tendermint::header::Header; @@ -15,7 +16,6 @@ use crate::core::ics02_client::client_state::AnyClientState; use crate::core::ics02_client::client_type::ClientType; use crate::core::ics02_client::error::Error as Ics02Error; use crate::core::ics02_client::trust_threshold::TrustThreshold; -use crate::core::ics23_commitment::specs::ProofSpecs; use crate::core::ics24_host::identifier::ChainId; use crate::timestamp::ZERO_DURATION; use crate::Height; @@ -29,7 +29,7 @@ pub struct ClientState { pub max_clock_drift: Duration, pub frozen_height: Height, pub latest_height: Height, - // pub proof_specs: ::core::vec::Vec, + pub proof_specs: Vec, pub upgrade_path: Vec, pub allow_update: AllowUpdate, } @@ -52,6 +52,7 @@ impl ClientState { max_clock_drift: Duration, latest_height: Height, frozen_height: Height, + proof_specs: Vec, upgrade_path: Vec, allow_update: AllowUpdate, ) -> Result { @@ -99,6 +100,7 @@ impl ClientState { max_clock_drift, frozen_height, latest_height, + proof_specs, upgrade_path, allow_update, }) @@ -223,6 +225,7 @@ impl TryFrom for ClientState { .frozen_height .ok_or_else(Error::missing_frozen_height)? .into(), + proof_specs: raw.proof_specs, upgrade_path: raw.upgrade_path, allow_update: AllowUpdate { after_expiry: raw.allow_update_after_expiry, @@ -242,7 +245,7 @@ impl From for RawClientState { max_clock_drift: Some(value.max_clock_drift.into()), frozen_height: Some(value.frozen_height.into()), latest_height: Some(value.latest_height.into()), - proof_specs: ProofSpecs::cosmos().into(), + proof_specs: value.proof_specs, allow_update_after_expiry: value.allow_update.after_expiry, allow_update_after_misbehaviour: value.allow_update.after_misbehaviour, upgrade_path: value.upgrade_path, @@ -257,6 +260,7 @@ mod tests { use std::println; use test_env_log::test; + use ibc_proto::ics23::ProofSpec; use tendermint_rpc::endpoint::abci_query::AbciQuery; use crate::clients::ics07_tendermint::client_state::{AllowUpdate, ClientState}; @@ -293,6 +297,7 @@ mod tests { max_clock_drift: Duration, latest_height: Height, frozen_height: Height, + proof_specs: Vec, upgrade_path: Vec, allow_update: AllowUpdate, } @@ -306,6 +311,7 @@ mod tests { max_clock_drift: Duration::new(3, 0), latest_height: Height::new(0, 10), frozen_height: Height::default(), + proof_specs: vec![], upgrade_path: vec!["".to_string()], allow_update: AllowUpdate { after_expiry: false, @@ -373,6 +379,7 @@ mod tests { p.max_clock_drift, p.latest_height, p.frozen_height, + p.proof_specs, p.upgrade_path, p.allow_update, ); @@ -414,6 +421,7 @@ pub mod test_util { u64::from(tm_header.height), ), Height::zero(), + vec![], vec!["".to_string()], AllowUpdate { after_expiry: false, diff --git a/modules/src/clients/ics07_tendermint/error.rs b/modules/src/clients/ics07_tendermint/error.rs index 91e78d37a5..a4dfa64523 100644 --- a/modules/src/clients/ics07_tendermint/error.rs +++ b/modules/src/clients/ics07_tendermint/error.rs @@ -2,6 +2,7 @@ use crate::prelude::*; use flex_error::{define_error, TraceError}; +use crate::core::ics23_commitment::error::Error as Ics23Error; use crate::core::ics24_host::error::ValidationError; use crate::Height; use tendermint::account::Id; @@ -198,7 +199,11 @@ define_error! { { detail: tendermint_light_client::predicates::errors::VerificationErrorDetail } | e | { format_args!("verification failed: {}", e.detail) - } + }, + + Ics23Error + [ Ics23Error ] + | _ | { "ics23 commitment error" }, } } diff --git a/modules/src/core/ics02_client/client_def.rs b/modules/src/core/ics02_client/client_def.rs index 6f439a2e05..31650f4bc8 100644 --- a/modules/src/core/ics02_client/client_def.rs +++ b/modules/src/core/ics02_client/client_def.rs @@ -54,9 +54,9 @@ pub trait ClientDef: Clone { fn verify_client_consensus_state( &self, client_state: &Self::ClientState, - height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, client_id: &ClientId, consensus_height: Height, expected_consensus_state: &AnyConsensusState, @@ -66,10 +66,10 @@ pub trait ClientDef: Clone { fn verify_connection_state( &self, client_state: &Self::ClientState, - height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, - connection_id: Option<&ConnectionId>, + root: &CommitmentRoot, + connection_id: &ConnectionId, expected_connection_end: &ConnectionEnd, ) -> Result<(), Error>; @@ -78,9 +78,9 @@ pub trait ClientDef: Clone { fn verify_channel_state( &self, client_state: &Self::ClientState, - height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, expected_channel_end: &ChannelEnd, @@ -90,13 +90,12 @@ pub trait ClientDef: Clone { #[allow(clippy::too_many_arguments)] fn verify_client_full_state( &self, - _client_state: &Self::ClientState, - height: Height, - root: &CommitmentRoot, + client_state: &Self::ClientState, prefix: &CommitmentPrefix, - client_id: &ClientId, proof: &CommitmentProofBytes, - client_state: &AnyClientState, + root: &CommitmentRoot, + client_id: &ClientId, + expected_client_state: &AnyClientState, ) -> Result<(), Error>; /// Verify a `proof` that a packet has been commited. @@ -104,11 +103,12 @@ pub trait ClientDef: Clone { fn verify_packet_data( &self, client_state: &Self::ClientState, - height: Height, + prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + seq: Sequence, commitment: String, ) -> Result<(), Error>; @@ -117,11 +117,12 @@ pub trait ClientDef: Clone { fn verify_packet_acknowledgement( &self, client_state: &Self::ClientState, - height: Height, + prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + seq: Sequence, ack: Vec, ) -> Result<(), Error>; @@ -130,11 +131,12 @@ pub trait ClientDef: Clone { fn verify_next_sequence_recv( &self, client_state: &Self::ClientState, - height: Height, + prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + seq: Sequence, ) -> Result<(), Error>; /// Verify a `proof` that a packet has not been received. @@ -142,11 +144,12 @@ pub trait ClientDef: Clone { fn verify_packet_receipt_absence( &self, client_state: &Self::ClientState, - height: Height, + prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + seq: Sequence, ) -> Result<(), Error>; } @@ -222,9 +225,9 @@ impl ClientDef for AnyClient { fn verify_client_consensus_state( &self, client_state: &Self::ClientState, - height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, client_id: &ClientId, consensus_height: Height, expected_consensus_state: &AnyConsensusState, @@ -238,9 +241,9 @@ impl ClientDef for AnyClient { client.verify_client_consensus_state( client_state, - height, prefix, proof, + root, client_id, consensus_height, expected_consensus_state, @@ -256,9 +259,9 @@ impl ClientDef for AnyClient { client.verify_client_consensus_state( client_state, - height, prefix, proof, + root, client_id, consensus_height, expected_consensus_state, @@ -270,10 +273,10 @@ impl ClientDef for AnyClient { fn verify_connection_state( &self, client_state: &AnyClientState, - height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, - connection_id: Option<&ConnectionId>, + root: &CommitmentRoot, + connection_id: &ConnectionId, expected_connection_end: &ConnectionEnd, ) -> Result<(), Error> { match self { @@ -283,9 +286,9 @@ impl ClientDef for AnyClient { client.verify_connection_state( client_state, - height, prefix, proof, + root, connection_id, expected_connection_end, ) @@ -298,9 +301,9 @@ impl ClientDef for AnyClient { client.verify_connection_state( client_state, - height, prefix, proof, + root, connection_id, expected_connection_end, ) @@ -311,9 +314,9 @@ impl ClientDef for AnyClient { fn verify_channel_state( &self, client_state: &AnyClientState, - height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, expected_channel_end: &ChannelEnd, @@ -325,9 +328,9 @@ impl ClientDef for AnyClient { client.verify_channel_state( client_state, - height, prefix, proof, + root, port_id, channel_id, expected_channel_end, @@ -341,9 +344,9 @@ impl ClientDef for AnyClient { client.verify_channel_state( client_state, - height, prefix, proof, + root, port_id, channel_id, expected_channel_end, @@ -355,11 +358,10 @@ impl ClientDef for AnyClient { fn verify_client_full_state( &self, client_state: &Self::ClientState, - height: Height, - root: &CommitmentRoot, prefix: &CommitmentPrefix, - client_id: &ClientId, proof: &CommitmentProofBytes, + root: &CommitmentRoot, + client_id: &ClientId, client_state_on_counterparty: &AnyClientState, ) -> Result<(), Error> { match self { @@ -371,11 +373,10 @@ impl ClientDef for AnyClient { client.verify_client_full_state( client_state, - height, - root, prefix, - client_id, proof, + root, + client_id, client_state_on_counterparty, ) } @@ -389,11 +390,10 @@ impl ClientDef for AnyClient { client.verify_client_full_state( client_state, - height, - root, prefix, - client_id, proof, + root, + client_id, client_state_on_counterparty, ) } @@ -402,11 +402,12 @@ impl ClientDef for AnyClient { fn verify_packet_data( &self, client_state: &Self::ClientState, - height: Height, + prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + seq: Sequence, commitment: String, ) -> Result<(), Error> { match self { @@ -418,8 +419,9 @@ impl ClientDef for AnyClient { client.verify_packet_data( client_state, - height, + prefix, proof, + root, port_id, channel_id, seq, @@ -436,8 +438,9 @@ impl ClientDef for AnyClient { client.verify_packet_data( client_state, - height, + prefix, proof, + root, port_id, channel_id, seq, @@ -450,11 +453,12 @@ impl ClientDef for AnyClient { fn verify_packet_acknowledgement( &self, client_state: &Self::ClientState, - height: Height, + prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + seq: Sequence, ack: Vec, ) -> Result<(), Error> { match self { @@ -466,8 +470,9 @@ impl ClientDef for AnyClient { client.verify_packet_acknowledgement( client_state, - height, + prefix, proof, + root, port_id, channel_id, seq, @@ -484,8 +489,9 @@ impl ClientDef for AnyClient { client.verify_packet_acknowledgement( client_state, - height, + prefix, proof, + root, port_id, channel_id, seq, @@ -498,11 +504,12 @@ impl ClientDef for AnyClient { fn verify_next_sequence_recv( &self, client_state: &Self::ClientState, - height: Height, + prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + seq: Sequence, ) -> Result<(), Error> { match self { Self::Tendermint(client) => { @@ -513,8 +520,9 @@ impl ClientDef for AnyClient { client.verify_next_sequence_recv( client_state, - height, + prefix, proof, + root, port_id, channel_id, seq, @@ -530,8 +538,9 @@ impl ClientDef for AnyClient { client.verify_next_sequence_recv( client_state, - height, + prefix, proof, + root, port_id, channel_id, seq, @@ -542,11 +551,12 @@ impl ClientDef for AnyClient { fn verify_packet_receipt_absence( &self, client_state: &Self::ClientState, - height: Height, + prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + seq: Sequence, ) -> Result<(), Error> { match self { Self::Tendermint(client) => { @@ -557,8 +567,9 @@ impl ClientDef for AnyClient { client.verify_packet_receipt_absence( client_state, - height, + prefix, proof, + root, port_id, channel_id, seq, @@ -574,8 +585,9 @@ impl ClientDef for AnyClient { client.verify_packet_receipt_absence( client_state, - height, + prefix, proof, + root, port_id, channel_id, seq, diff --git a/modules/src/core/ics02_client/error.rs b/modules/src/core/ics02_client/error.rs index 504f7dae2a..d3da2bbf76 100644 --- a/modules/src/core/ics02_client/error.rs +++ b/modules/src/core/ics02_client/error.rs @@ -72,7 +72,7 @@ define_error! { | _ | { "the client state was not found" }, EmptyPrefix - { source: crate::core::ics23_commitment::merkle::EmptyPrefixError } + [ Ics23Error ] | _ | { "empty prefix" }, UnknownConsensusStateType @@ -165,13 +165,13 @@ define_error! { InvalidAddress | _ | { "invalid address" }, - InvalidUpgradeClientProof + InvalidClientProof [ Ics23Error ] - | _ | { "invalid proof for the upgraded client state" }, + | _ | { "invalid proof for the client state" }, - InvalidUpgradeConsensusStateProof + InvalidConsensusStateProof [ Ics23Error ] - | _ | { "invalid proof for the upgraded consensus state" }, + | _ | { "invalid proof for the consensus state" }, Tendermint [ Ics07Error ] diff --git a/modules/src/core/ics02_client/handler/create_client.rs b/modules/src/core/ics02_client/handler/create_client.rs index a21410663b..563ab5eb32 100644 --- a/modules/src/core/ics02_client/handler/create_client.rs +++ b/modules/src/core/ics02_client/handler/create_client.rs @@ -230,6 +230,7 @@ mod tests { max_clock_drift: Duration::from_millis(3000), latest_height: Height::new(0, u64::from(tm_header.height)), frozen_height: Height::zero(), + proof_specs: vec![], allow_update: AllowUpdate { after_expiry: false, after_misbehaviour: false, diff --git a/modules/src/core/ics02_client/msgs/upgrade_client.rs b/modules/src/core/ics02_client/msgs/upgrade_client.rs index 954c4969cf..b2a2fc32ce 100644 --- a/modules/src/core/ics02_client/msgs/upgrade_client.rs +++ b/modules/src/core/ics02_client/msgs/upgrade_client.rs @@ -100,9 +100,9 @@ impl TryFrom for MsgUpgradeAnyClient { client_state: AnyClientState::try_from(raw_client_state)?, consensus_state: AnyConsensusState::try_from(raw_consensus_state)?, proof_upgrade_client: RawMerkleProof::try_from(c_bytes) - .map_err(Error::invalid_upgrade_client_proof)?, + .map_err(Error::invalid_client_proof)?, proof_upgrade_consensus_state: RawMerkleProof::try_from(cs_bytes) - .map_err(Error::invalid_upgrade_consensus_state_proof)?, + .map_err(Error::invalid_consensus_state_proof)?, signer: proto_msg.signer.into(), }) } diff --git a/modules/src/core/ics03_connection/handler/verify.rs b/modules/src/core/ics03_connection/handler/verify.rs index cc474f94e6..86ac2e4ec6 100644 --- a/modules/src/core/ics03_connection/handler/verify.rs +++ b/modules/src/core/ics03_connection/handler/verify.rs @@ -42,12 +42,7 @@ pub fn verify_proofs( // If a consensus proof is attached to the message, then verify it. if let Some(proof) = proofs.consensus_proof() { - Ok(verify_consensus_proof( - ctx, - connection_end, - proofs.height(), - &proof, - )?) + Ok(verify_consensus_proof(ctx, connection_end, &proof)?) } else { Ok(()) } @@ -72,20 +67,25 @@ pub fn verify_connection_proof( } // The client must have the consensus state for the height where this proof was created. - ctx.client_consensus_state(connection_end.client_id(), proof_height)?; + let consensus_state = ctx.client_consensus_state(connection_end.client_id(), proof_height)?; + + // A counterparty connection id of None causes `unwrap()` below and indicates an internal + // error as this is the connection id on the counterparty chain that must always be present. + let connection_id = connection_end + .counterparty() + .connection_id() + .ok_or_else(Error::invalid_counterparty)?; let client_def = AnyClient::from_client_type(client_state.client_type()); // Verify the proof for the connection state against the expected connection end. - // A counterparty connection id of None causes `unwrap()` below and indicates an internal - // error as this is the connection id on the counterparty chain that must always be present. client_def .verify_connection_state( &client_state, - proof_height, connection_end.counterparty().prefix(), proof, - connection_end.counterparty().connection_id(), + consensus_state.root(), + connection_id, expected_conn, ) .map_err(Error::verify_connection_state) @@ -119,11 +119,10 @@ pub fn verify_client_proof( client_def .verify_client_full_state( &client_state, - proof_height, - consensus_state.root(), connection_end.counterparty().prefix(), - connection_end.counterparty().client_id(), proof, + consensus_state.root(), + connection_end.counterparty().client_id(), &expected_client_state, ) .map_err(|e| { @@ -134,7 +133,6 @@ pub fn verify_client_proof( pub fn verify_consensus_proof( ctx: &dyn ConnectionReader, connection_end: &ConnectionEnd, - proof_height: Height, proof: &ConsensusProof, ) -> Result<(), Error> { // Fetch the client state (IBC client on the local chain). @@ -152,9 +150,9 @@ pub fn verify_consensus_proof( client .verify_client_consensus_state( &client_state, - proof_height, connection_end.counterparty().prefix(), proof.proof(), + expected_consensus.root(), connection_end.counterparty().client_id(), proof.height(), &expected_consensus, diff --git a/modules/src/core/ics04_channel/handler/acknowledgement.rs b/modules/src/core/ics04_channel/handler/acknowledgement.rs index 80bd3c96b3..75a0ebb417 100644 --- a/modules/src/core/ics04_channel/handler/acknowledgement.rs +++ b/modules/src/core/ics04_channel/handler/acknowledgement.rs @@ -57,8 +57,6 @@ pub fn process( )); } - let client_id = connection_end.client_id().clone(); - // Verify packet commitment let packet_commitment = ctx.get_packet_commitment(&( packet.source_port.clone(), @@ -80,7 +78,7 @@ pub fn process( ctx, packet, msg.acknowledgement().clone(), - client_id, + &connection_end, msg.proofs(), )?; diff --git a/modules/src/core/ics04_channel/handler/recv_packet.rs b/modules/src/core/ics04_channel/handler/recv_packet.rs index ecce853fc7..158eb4d9fa 100644 --- a/modules/src/core/ics04_channel/handler/recv_packet.rs +++ b/modules/src/core/ics04_channel/handler/recv_packet.rs @@ -60,8 +60,6 @@ pub fn process(ctx: &dyn ChannelReader, msg: MsgRecvPacket) -> HandlerResult HandlerResult HandlerResult HandlerResult Result<(), Error> { - let client_state = ctx.client_state(&client_id)?; + let client_id = connection_end.client_id(); + let client_state = ctx.client_state(client_id)?; // The client must not be frozen. if client_state.is_frozen() { - return Err(Error::frozen_client(client_id)); + return Err(Error::frozen_client(client_id.clone())); } - ctx.client_consensus_state(&client_id, proofs.height())?; + let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; let client_def = AnyClient::from_client_type(client_state.client_type()); @@ -74,11 +75,12 @@ pub fn verify_packet_recv_proofs( client_def .verify_packet_data( &client_state, - proofs.height(), + connection_end.counterparty().prefix(), proofs.object_proof(), + consensus_state.root(), &packet.source_port, &packet.source_channel, - &packet.sequence, + packet.sequence, commitment, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; @@ -91,27 +93,31 @@ pub fn verify_packet_acknowledgement_proofs( ctx: &dyn ChannelReader, packet: &Packet, acknowledgement: Vec, - client_id: ClientId, + connection_end: &ConnectionEnd, proofs: &Proofs, ) -> Result<(), Error> { - let client_state = ctx.client_state(&client_id)?; + let client_id = connection_end.client_id(); + let client_state = ctx.client_state(client_id)?; // The client must not be frozen. if client_state.is_frozen() { - return Err(Error::frozen_client(client_id)); + return Err(Error::frozen_client(client_id.clone())); } + let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + let client_def = AnyClient::from_client_type(client_state.client_type()); // Verify the proof for the packet against the chain store. client_def .verify_packet_acknowledgement( &client_state, - proofs.height(), + connection_end.counterparty().prefix(), proofs.object_proof(), + consensus_state.root(), &packet.source_port, &packet.source_channel, - &packet.sequence, + packet.sequence, acknowledgement, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; @@ -122,29 +128,33 @@ pub fn verify_packet_acknowledgement_proofs( /// Entry point for verifying all timeout proofs. pub fn verify_next_sequence_recv( ctx: &dyn ChannelReader, - client_id: ClientId, + connection_end: &ConnectionEnd, packet: Packet, seq: Sequence, proofs: &Proofs, ) -> Result<(), Error> { - let client_state = ctx.client_state(&client_id)?; + let client_id = connection_end.client_id(); + let client_state = ctx.client_state(client_id)?; // The client must not be frozen. if client_state.is_frozen() { - return Err(Error::frozen_client(client_id)); + return Err(Error::frozen_client(client_id.clone())); } + let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + let client_def = AnyClient::from_client_type(client_state.client_type()); // Verify the proof for the packet against the chain store. client_def .verify_next_sequence_recv( &client_state, - proofs.height(), + connection_end.counterparty().prefix(), proofs.object_proof(), + consensus_state.root(), &packet.destination_port, &packet.destination_channel, - &seq, + seq, ) .map_err(|e| Error::packet_verification_failed(seq, e))?; @@ -153,28 +163,32 @@ pub fn verify_next_sequence_recv( pub fn verify_packet_receipt_absence( ctx: &dyn ChannelReader, - client_id: ClientId, + connection_end: &ConnectionEnd, packet: Packet, proofs: &Proofs, ) -> Result<(), Error> { - let client_state = ctx.client_state(&client_id)?; + let client_id = connection_end.client_id(); + let client_state = ctx.client_state(client_id)?; // The client must not be frozen. if client_state.is_frozen() { - return Err(Error::frozen_client(client_id)); + return Err(Error::frozen_client(client_id.clone())); } + let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + let client_def = AnyClient::from_client_type(client_state.client_type()); // Verify the proof for the packet against the chain store. client_def .verify_packet_receipt_absence( &client_state, - proofs.height(), + connection_end.counterparty().prefix(), proofs.object_proof(), + consensus_state.root(), &packet.destination_port, &packet.destination_channel, - &packet.sequence, + packet.sequence, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; diff --git a/modules/src/core/ics23_commitment/error.rs b/modules/src/core/ics23_commitment/error.rs index 43d8032f88..e54b685ff2 100644 --- a/modules/src/core/ics23_commitment/error.rs +++ b/modules/src/core/ics23_commitment/error.rs @@ -11,5 +11,29 @@ define_error! { CommitmentProofDecodingFailed [ TraceError ] |_| { "failed to decode commitment proof" }, + + EmptyCommitmentPrefix + |_| { "empty commitment prefix" }, + + EmptyMerkleProof + |_| { "empty merkle proof" }, + + EmptyMerkleRoot + |_| { "empty merkle proof" }, + + EmptyVerifiedValue + |_| { "empty verified value" }, + + NumberOfSpecsMismatch + |_| { "mismatch between the number of proofs with that of specs" }, + + NumberOfKeysMismatch + |_| { "mismatch between the number of proofs with that of keys" }, + + InvalidMerkleProof + |_| { "invalid merkle proof" }, + + VerificationFailure + |_| { "proof verification failed" } } } diff --git a/modules/src/core/ics23_commitment/merkle.rs b/modules/src/core/ics23_commitment/merkle.rs index df3b6c1234..0cd0c32036 100644 --- a/modules/src/core/ics23_commitment/merkle.rs +++ b/modules/src/core/ics23_commitment/merkle.rs @@ -1,21 +1,22 @@ use crate::prelude::*; -use tendermint::merkle::proof::Proof; +use tendermint::merkle::proof::Proof as TendermintProof; use ibc_proto::ibc::core::commitment::v1::MerklePath; use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof; +use ibc_proto::ibc::core::commitment::v1::MerkleRoot; +use ics23::commitment_proof::Proof; +use ics23::{ + calculate_existence_root, verify_membership, verify_non_membership, CommitmentProof, + NonExistenceProof, +}; -use crate::core::ics23_commitment::commitment::{CommitmentPrefix, CommitmentProofBytes}; +use crate::core::ics23_commitment::commitment::{CommitmentPrefix, CommitmentRoot}; use crate::core::ics23_commitment::error::Error; +use crate::core::ics23_commitment::specs::ProofSpecs; -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct EmptyPrefixError; - -pub fn apply_prefix( - prefix: &CommitmentPrefix, - mut path: Vec, -) -> Result { +pub fn apply_prefix(prefix: &CommitmentPrefix, mut path: Vec) -> Result { if prefix.is_empty() { - return Err(EmptyPrefixError); + return Err(Error::empty_commitment_prefix()); } let mut result: Vec = vec![format!("{:?}", prefix)]; @@ -24,9 +25,146 @@ pub fn apply_prefix( Ok(MerklePath { key_path: result }) } +impl From for MerkleRoot { + fn from(root: CommitmentRoot) -> Self { + Self { + hash: root.into_vec(), + } + } +} + #[derive(Clone, Debug, PartialEq)] pub struct MerkleProof { - pub proof: Vec, + pub proofs: Vec, +} + +/// Convert to ics23::CommitmentProof +/// The encoding and decoding shouldn't fail since ics23::CommitmentProof and ibc_proto::ics23::CommitmentProof should be the same +/// Ref. https://github.com/informalsystems/ibc-rs/issues/853 +impl From for MerkleProof { + fn from(proof: RawMerkleProof) -> Self { + let proofs: Vec = proof + .proofs + .into_iter() + .map(|p| { + let mut encoded = Vec::new(); + prost::Message::encode(&p, &mut encoded).unwrap(); + prost::Message::decode(&*encoded).unwrap() + }) + .collect(); + Self { proofs } + } +} + +impl MerkleProof { + pub fn verify_membership( + &self, + specs: ProofSpecs, + root: MerkleRoot, + keys: MerklePath, + value: Vec, + start_index: usize, + ) -> Result<(), Error> { + // validate arguments + if self.proofs.is_empty() { + return Err(Error::empty_merkle_proof()); + } + if root.hash.is_empty() { + return Err(Error::empty_merkle_root()); + } + let num = self.proofs.len(); + if specs.specs.len() != num { + return Err(Error::number_of_specs_mismatch()); + } + if keys.key_path.len() != num { + return Err(Error::number_of_keys_mismatch()); + } + if value.is_empty() { + return Err(Error::empty_verified_value()); + } + + let mut subroot = value.clone(); + let mut value = value; + // keys are represented from root-to-leaf + for ((proof, spec), key) in self + .proofs + .iter() + .zip(specs.specs.iter()) + .zip(keys.key_path.iter().rev()) + .skip(start_index) + { + match &proof.proof { + Some(Proof::Exist(existence_proof)) => { + subroot = calculate_existence_root(existence_proof) + .map_err(|_| Error::invalid_merkle_proof())?; + if !verify_membership(proof, spec, &subroot, key.as_bytes(), &value) { + return Err(Error::verification_failure()); + } + value = subroot.clone(); + } + _ => return Err(Error::invalid_merkle_proof()), + } + } + + if root.hash != subroot { + return Err(Error::verification_failure()); + } + + Ok(()) + } + + pub fn verify_non_membership( + &self, + specs: ProofSpecs, + root: MerkleRoot, + keys: MerklePath, + ) -> Result<(), Error> { + // validate arguments + if self.proofs.is_empty() { + return Err(Error::empty_merkle_proof()); + } + if root.hash.is_empty() { + return Err(Error::empty_merkle_root()); + } + let num = self.proofs.len(); + if specs.specs.len() != num { + return Err(Error::number_of_specs_mismatch()); + } + if keys.key_path.len() != num { + return Err(Error::number_of_keys_mismatch()); + } + + // verify the absence of key in lowest subtree + let proof = self.proofs.get(0).ok_or_else(Error::invalid_merkle_proof)?; + let spec = specs.specs.get(0).ok_or_else(Error::invalid_merkle_proof)?; + // keys are represented from root-to-leaf + let key = keys + .key_path + .get(num - 1) + .ok_or_else(Error::invalid_merkle_proof)?; + match &proof.proof { + 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()) { + return Err(Error::verification_failure()); + } + // verify membership proofs starting from index 1 with value = subroot + self.verify_membership(specs, root, keys, subroot, 1) + } + _ => Err(Error::invalid_merkle_proof()), + } + } +} + +// TODO move to ics23 +fn calculate_non_existence_root(proof: &NonExistenceProof) -> Result, Error> { + if let Some(left) = &proof.left { + calculate_existence_root(left).map_err(|_| Error::invalid_merkle_proof()) + } else if let Some(right) = &proof.right { + calculate_existence_root(right).map_err(|_| Error::invalid_merkle_proof()) + } else { + Err(Error::invalid_merkle_proof()) + } } // Merkle Proof serialization notes: @@ -81,7 +219,7 @@ pub struct MerkleProof { // } // } -pub fn convert_tm_to_ics_merkle_proof(tm_proof: &Proof) -> Result { +pub fn convert_tm_to_ics_merkle_proof(tm_proof: &TendermintProof) -> Result { let mut proofs = Vec::new(); for op in &tm_proof.ops { diff --git a/modules/src/core/ics23_commitment/specs.rs b/modules/src/core/ics23_commitment/specs.rs index 728ea63b67..aa8a7b91b7 100644 --- a/modules/src/core/ics23_commitment/specs.rs +++ b/modules/src/core/ics23_commitment/specs.rs @@ -9,8 +9,9 @@ use ics23::ProofSpec; /// Additionally, this type also aids in the conversion from `ProofSpec` types from crate `ics23` /// into proof specifications as represented in the `ibc_proto` type; see the /// `From` trait(s) below. +#[derive(Clone, Debug, PartialEq)] pub struct ProofSpecs { - specs: Vec, + pub specs: Vec, } impl ProofSpecs { @@ -44,3 +45,18 @@ impl From for Vec { raw_specs } } + +impl From> for ProofSpecs { + fn from(proto_specs: Vec) -> Self { + let specs: Vec = proto_specs + .iter() + .map(|p| { + let mut encoded = Vec::new(); + // encode/decode conversion here should be infallible. + prost::Message::encode(p, &mut encoded).unwrap(); + prost::Message::decode(&*encoded).unwrap() + }) + .collect(); + Self { specs } + } +} diff --git a/modules/src/mock/client_def.rs b/modules/src/mock/client_def.rs index 7d48ea28bd..88a4183710 100644 --- a/modules/src/mock/client_def.rs +++ b/modules/src/mock/client_def.rs @@ -46,38 +46,33 @@ impl ClientDef for MockClient { fn verify_client_consensus_state( &self, _client_state: &Self::ClientState, - height: Height, prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, client_id: &ClientId, - _consensus_height: Height, + consensus_height: Height, _expected_consensus_state: &AnyConsensusState, ) -> Result<(), Error> { let client_prefixed_path = Path::ClientConsensusState { client_id: client_id.clone(), - epoch: height.revision_number, - height: height.revision_height, + epoch: consensus_height.revision_number, + height: consensus_height.revision_height, } .to_string(); let _path = apply_prefix(prefix, vec![client_prefixed_path]).map_err(Error::empty_prefix)?; - // TODO - add ctx to all client verification functions - // let cs = ctx.fetch_self_consensus_state(height); - // TODO - implement this - // proof.verify_membership(cs.root(), path, expected_consensus_state) - Ok(()) } fn verify_connection_state( &self, _client_state: &Self::ClientState, - _height: Height, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, - _connection_id: Option<&ConnectionId>, + _root: &CommitmentRoot, + _connection_id: &ConnectionId, _expected_connection_end: &ConnectionEnd, ) -> Result<(), Error> { Ok(()) @@ -86,9 +81,9 @@ impl ClientDef for MockClient { fn verify_channel_state( &self, _client_state: &Self::ClientState, - _height: Height, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, _expected_channel_end: &ChannelEnd, @@ -99,11 +94,10 @@ impl ClientDef for MockClient { fn verify_client_full_state( &self, _client_state: &Self::ClientState, - _height: Height, - _root: &CommitmentRoot, _prefix: &CommitmentPrefix, - _client_id: &ClientId, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, + _client_id: &ClientId, _expected_client_state: &AnyClientState, ) -> Result<(), Error> { Ok(()) @@ -112,12 +106,13 @@ impl ClientDef for MockClient { fn verify_packet_data( &self, _client_state: &Self::ClientState, - _height: Height, + _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, - _seq: &Sequence, - _data: String, + _seq: Sequence, + _commitment: String, ) -> Result<(), Error> { Ok(()) } @@ -125,12 +120,13 @@ impl ClientDef for MockClient { fn verify_packet_acknowledgement( &self, _client_state: &Self::ClientState, - _height: Height, + _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, - _seq: &Sequence, - _data: Vec, + _seq: Sequence, + _ack: Vec, ) -> Result<(), Error> { Ok(()) } @@ -138,11 +134,12 @@ impl ClientDef for MockClient { fn verify_next_sequence_recv( &self, _client_state: &Self::ClientState, - _height: Height, + _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, - _seq: &Sequence, + _seq: Sequence, ) -> Result<(), Error> { Ok(()) } @@ -150,14 +147,16 @@ impl ClientDef for MockClient { fn verify_packet_receipt_absence( &self, _client_state: &Self::ClientState, - _height: Height, + _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, - _seq: &Sequence, + _sequence: Sequence, ) -> Result<(), Error> { Ok(()) } + fn verify_upgrade_and_update_state( &self, client_state: &Self::ClientState, diff --git a/modules/tests/runner/mod.rs b/modules/tests/runner/mod.rs index 4065395059..928756566b 100644 --- a/modules/tests/runner/mod.rs +++ b/modules/tests/runner/mod.rs @@ -471,6 +471,7 @@ impl modelator::step_runner::StepRunner for IbcTestRunner { } fn next_step(&mut self, step: Step) -> Result<(), String> { + let show = step.action.clone(); let result = self.apply(step.action); let outcome_matches = match step.action_outcome { ActionOutcome::None => panic!("unexpected action outcome"), @@ -529,7 +530,7 @@ impl modelator::step_runner::StepRunner for IbcTestRunner { } if !outcome_matches { - return Err("Action outcome did not match expected".into()); + return Err(format!("Action outcome did not match expected: {:?}", show)); } if !self.check_chain_states(step.chains) { diff --git a/proto/Cargo.toml b/proto/Cargo.toml index b121bbf339..9888f89570 100644 --- a/proto/Cargo.toml +++ b/proto/Cargo.toml @@ -27,6 +27,7 @@ prost-types = "0.9" bytes = "1.1" tonic = "0.6" getrandom = { version = "0.2", features = ["js"] } +serde = "1.0.130" [dependencies.tendermint-proto] version = "=0.23.0" diff --git a/proto/src/prost/ics23.rs b/proto/src/prost/ics23.rs index d7e2332338..9e6c7a1de2 100644 --- a/proto/src/prost/ics23.rs +++ b/proto/src/prost/ics23.rs @@ -18,7 +18,7 @@ ///With LengthOp this is tricker but not impossible. Which is why the "leafPrefixEqual" field ///in the ProofSpec is valuable to prevent this mutability. And why all trees should ///length-prefix the data before hashing it. -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct ExistenceProof { #[prost(bytes = "vec", tag = "1")] pub key: ::prost::alloc::vec::Vec, @@ -33,7 +33,7 @@ pub struct ExistenceProof { ///NonExistenceProof takes a proof of two neighbors, one left of the desired key, ///one right of the desired key. If both proofs are valid AND they are neighbors, ///then there is no valid proof for the given key. -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct NonExistenceProof { /// TODO: remove this as unnecessary??? we prove a range #[prost(bytes = "vec", tag = "1")] @@ -45,14 +45,14 @@ pub struct NonExistenceProof { } /// ///CommitmentProof is either an ExistenceProof or a NonExistenceProof, or a Batch of such messages -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct CommitmentProof { #[prost(oneof = "commitment_proof::Proof", tags = "1, 2, 3, 4")] pub proof: ::core::option::Option, } /// Nested message and enum types in `CommitmentProof`. pub mod commitment_proof { - #[derive(Clone, PartialEq, ::prost::Oneof)] + #[derive(Clone, PartialEq, Eq, ::prost::Oneof)] pub enum Proof { #[prost(message, tag = "1")] Exist(super::ExistenceProof), @@ -79,7 +79,7 @@ pub mod commitment_proof { /// ///Then combine the bytes, and hash it ///output = hash(prefix || length(hkey) || hkey || length(hvalue) || hvalue) -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message, ::serde::Serialize, ::serde::Deserialize)] pub struct LeafOp { #[prost(enumeration = "HashOp", tag = "1")] pub hash: i32, @@ -110,7 +110,7 @@ pub struct LeafOp { ///Any special data, like prepending child with the length, or prepending the entire operation with ///some value to differentiate from leaf nodes, should be included in prefix and suffix. ///If either of prefix or suffix is empty, we just treat it as an empty string -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message, ::serde::Serialize, ::serde::Deserialize)] pub struct InnerOp { #[prost(enumeration = "HashOp", tag = "1")] pub hash: i32, @@ -130,7 +130,7 @@ pub struct InnerOp { ///generate a given hash (by interpretting the preimage differently). ///We need this for proper security, requires client knows a priori what ///tree format server uses. But not in code, rather a configuration object. -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message, ::serde::Serialize, ::serde::Deserialize)] pub struct ProofSpec { /// any field in the ExistenceProof must be the same as in this spec. /// except Prefix, which is just the first bytes of prefix (spec can be longer) @@ -154,7 +154,7 @@ pub struct ProofSpec { ///isLeftMost(spec: InnerSpec, op: InnerOp) ///isRightMost(spec: InnerSpec, op: InnerOp) ///isLeftNeighbor(spec: InnerSpec, left: InnerOp, right: InnerOp) -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message, ::serde::Serialize, ::serde::Deserialize)] pub struct InnerSpec { /// Child order is the ordering of the children node, must count from 0 /// iavl tree is [0, 1] (left then right) @@ -176,20 +176,20 @@ pub struct InnerSpec { } /// ///BatchProof is a group of multiple proof types than can be compressed -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct BatchProof { #[prost(message, repeated, tag = "1")] pub entries: ::prost::alloc::vec::Vec, } /// Use BatchEntry not CommitmentProof, to avoid recursion -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct BatchEntry { #[prost(oneof = "batch_entry::Proof", tags = "1, 2")] pub proof: ::core::option::Option, } /// Nested message and enum types in `BatchEntry`. pub mod batch_entry { - #[derive(Clone, PartialEq, ::prost::Oneof)] + #[derive(Clone, PartialEq, Eq, ::prost::Oneof)] pub enum Proof { #[prost(message, tag = "1")] Exist(super::ExistenceProof), @@ -199,7 +199,7 @@ pub mod batch_entry { } //***** all items here are compressed forms ****** -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct CompressedBatchProof { #[prost(message, repeated, tag = "1")] pub entries: ::prost::alloc::vec::Vec, @@ -207,14 +207,14 @@ pub struct CompressedBatchProof { pub lookup_inners: ::prost::alloc::vec::Vec, } /// Use BatchEntry not CommitmentProof, to avoid recursion -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct CompressedBatchEntry { #[prost(oneof = "compressed_batch_entry::Proof", tags = "1, 2")] pub proof: ::core::option::Option, } /// Nested message and enum types in `CompressedBatchEntry`. pub mod compressed_batch_entry { - #[derive(Clone, PartialEq, ::prost::Oneof)] + #[derive(Clone, PartialEq, Eq, ::prost::Oneof)] pub enum Proof { #[prost(message, tag = "1")] Exist(super::CompressedExistenceProof), @@ -222,7 +222,7 @@ pub mod compressed_batch_entry { Nonexist(super::CompressedNonExistenceProof), } } -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct CompressedExistenceProof { #[prost(bytes = "vec", tag = "1")] pub key: ::prost::alloc::vec::Vec, @@ -234,7 +234,7 @@ pub struct CompressedExistenceProof { #[prost(int32, repeated, tag = "4")] pub path: ::prost::alloc::vec::Vec, } -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(Clone, PartialEq, Eq, ::prost::Message)] pub struct CompressedNonExistenceProof { /// TODO: remove this as unnecessary??? we prove a range #[prost(bytes = "vec", tag = "1")] diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index bc848ed5d8..13a1238564 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -49,6 +49,7 @@ use ibc::core::ics04_channel::events as ChannelEvents; use ibc::core::ics04_channel::packet::{Packet, PacketMsgType, Sequence}; use ibc::core::ics23_commitment::commitment::CommitmentPrefix; use ibc::core::ics23_commitment::merkle::convert_tm_to_ics_merkle_proof; +use ibc::core::ics23_commitment::specs::ProofSpecs; use ibc::core::ics24_host::identifier::{ChainId, ChannelId, ClientId, ConnectionId, PortId}; use ibc::core::ics24_host::Path::ClientConsensusState as ClientConsensusPath; use ibc::core::ics24_host::Path::ClientState as ClientStatePath; @@ -1927,6 +1928,7 @@ impl ChainEndpoint for CosmosSdkChain { max_clock_drift, height, ICSHeight::zero(), + ProofSpecs::cosmos().into(), vec!["upgrade".to_string(), "upgradedIBCState".to_string()], AllowUpdate { after_expiry: true, diff --git a/relayer/src/chain/mock.rs b/relayer/src/chain/mock.rs index 3cb765d02b..b7edacfa4c 100644 --- a/relayer/src/chain/mock.rs +++ b/relayer/src/chain/mock.rs @@ -356,6 +356,7 @@ impl ChainEndpoint for MockChain { self.config.clock_drift + dst_config.clock_drift + dst_config.max_block_time, height, Height::zero(), + vec![], vec!["upgrade/upgradedClient".to_string()], AllowUpdate { after_expiry: false, From 6d54a5ac5597a3b08d2384c3849204c8fbf0de7f Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 22 Nov 2021 21:24:17 +0900 Subject: [PATCH 02/26] fix counterparty in ConnectionOpenAck --- Cargo.lock | 1 + .../ics03_connection/handler/conn_open_ack.rs | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f306243ec..200f976fd5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1341,6 +1341,7 @@ dependencies = [ "getrandom 0.2.3", "prost", "prost-types", + "serde", "tendermint-proto", "tonic", ] diff --git a/modules/src/core/ics03_connection/handler/conn_open_ack.rs b/modules/src/core/ics03_connection/handler/conn_open_ack.rs index 0060d7e56c..c17192b6e5 100644 --- a/modules/src/core/ics03_connection/handler/conn_open_ack.rs +++ b/modules/src/core/ics03_connection/handler/conn_open_ack.rs @@ -35,6 +35,17 @@ pub(crate) fn process( return Err(Error::connection_mismatch(msg.connection_id().clone())); } + // Set the connection ID of the counterparty + let prev_counterparty = conn_end.counterparty(); + let counterparty = Counterparty::new( + prev_counterparty.client_id().clone(), + Some(msg.connection_id().clone()), + prev_counterparty.prefix().clone(), + ); + conn_end.set_state(State::Open); + conn_end.set_version(msg.version().clone()); + conn_end.set_counterparty(counterparty); + // The counterparty is the local chain. let counterparty = Counterparty::new( conn_end.client_id().clone(), // The local client identifier. @@ -46,7 +57,7 @@ pub(crate) fn process( let expected_conn = ConnectionEnd::new( State::TryOpen, conn_end.counterparty().client_id().clone(), - counterparty.clone(), + counterparty, vec![msg.version().clone()], conn_end.delay_period(), ); @@ -62,10 +73,6 @@ pub(crate) fn process( output.log("success: connection verification passed"); - conn_end.set_state(State::Open); - conn_end.set_version(msg.version().clone()); - conn_end.set_counterparty(counterparty); - let result = ConnectionResult { connection_id: msg.connection_id().clone(), connection_id_state: ConnectionIdState::Reused, From 5272eae1995202819ea4161075ab9f051a017f04 Mon Sep 17 00:00:00 2001 From: yito88 Date: Tue, 7 Dec 2021 14:08:51 +0900 Subject: [PATCH 03/26] verify delay --- .../clients/ics07_tendermint/client_def.rs | 125 ++++++++++++------ .../clients/ics07_tendermint/client_state.rs | 24 +++- modules/src/clients/ics07_tendermint/error.rs | 28 ++++ modules/src/core/ics02_client/client_def.rs | 111 +++++++--------- modules/src/core/ics02_client/context.rs | 3 + .../ics02_client/handler/create_client.rs | 6 + modules/src/core/ics04_channel/context.rs | 6 + modules/src/core/ics04_channel/error.rs | 16 +++ .../src/core/ics04_channel/handler/verify.rs | 47 ++++--- modules/src/mock/client_def.rs | 28 ++-- modules/src/mock/context.rs | 26 ++++ modules/src/timestamp.rs | 1 + 12 files changed, 289 insertions(+), 132 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index fe8a12d286..f3ad83867b 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -19,6 +19,7 @@ use crate::core::ics02_client::context::ClientReader; use crate::core::ics02_client::error::Error as Ics02Error; use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics04_channel::channel::ChannelEnd; +use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::packet::Sequence; use crate::core::ics23_commitment::commitment::{ @@ -260,77 +261,88 @@ impl ClientDef for TendermintClient { fn verify_packet_data( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - sequence: Sequence, + commitment_path: &Path, commitment: String, ) -> Result<(), Ics02Error> { - let path = Path::Commitments { - port_id: port_id.clone(), - channel_id: channel_id.clone(), - sequence, - } - .to_string(); - let value = commitment.encode_to_vec(); - verify_membership(client_state, prefix, proof, root, path, value) + verify_delay_passed(ctx, client_state, connection_end)?; + + verify_membership( + client_state, + connection_end.counterparty().prefix(), + proof, + root, + commitment_path.to_string(), + commitment.encode_to_vec(), + ) } fn verify_packet_acknowledgement( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - sequence: Sequence, + ack_path: &Path, ack: Vec, ) -> Result<(), Ics02Error> { - let path = Path::Acks { - port_id: port_id.clone(), - channel_id: channel_id.clone(), - sequence, - } - .to_string(); - verify_membership(client_state, prefix, proof, root, path, ack) + verify_delay_passed(ctx, client_state, connection_end)?; + + verify_membership( + client_state, + connection_end.counterparty().prefix(), + proof, + root, + ack_path.to_string(), + ack, + ) } fn verify_next_sequence_recv( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, + seq_path: &Path, seq: Sequence, ) -> Result<(), Ics02Error> { - let path = Path::SeqRecvs(port_id.clone(), channel_id.clone()).to_string(); - let value = u64::from(seq).encode_to_vec(); - verify_membership(client_state, prefix, proof, root, path, value) + verify_delay_passed(ctx, client_state, connection_end)?; + + verify_membership( + client_state, + connection_end.counterparty().prefix(), + proof, + root, + seq_path.to_string(), + u64::from(seq).encode_to_vec(), + ) } fn verify_packet_receipt_absence( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - sequence: Sequence, + receipt_path: &Path, ) -> Result<(), Ics02Error> { - let path = Path::Receipts { - port_id: port_id.clone(), - channel_id: channel_id.clone(), - sequence, - } - .to_string(); - verify_non_membership(client_state, prefix, proof, root, path) + verify_delay_passed(ctx, client_state, connection_end)?; + + verify_non_membership( + client_state, + connection_end.counterparty().prefix(), + proof, + root, + receipt_path.to_string(), + ) } fn verify_upgrade_and_update_state( @@ -389,6 +401,39 @@ fn verify_non_membership( .map_err(|e| Ics02Error::tendermint(Error::ics23_error(e))) } +fn verify_delay_passed( + ctx: &dyn ChannelReader, + client_state: &ClientState, + connection_end: &ConnectionEnd, +) -> Result<(), Ics02Error> { + let current_timestamp = ctx.host_timestamp(); + let current_height = ctx.host_height(); + + let client_id = connection_end.client_id(); + let processed_time = ctx + .processed_time(client_id) + .map_err(|_| Error::processed_time_not_found(client_id.clone()))?; + let processed_height = ctx + .processed_height(client_id) + .map_err(|_| Error::processed_height_not_found(client_id.clone()))?; + + let delay_period_time = connection_end.delay_period(); + // TODO get delay_period_height from delay_period_time + //let delay_period_height = get_block_delay(delay_period_time); + let delay_period_height = 0; + + client_state + .verify_delay_passed( + current_timestamp, + current_height, + processed_time, + processed_height, + delay_period_time, + delay_period_height, + ) + .map_err(|e| e.into()) +} + fn downcast_consensus_state(cs: AnyConsensusState) -> Result { downcast!( cs => AnyConsensusState::Tendermint diff --git a/modules/src/clients/ics07_tendermint/client_state.rs b/modules/src/clients/ics07_tendermint/client_state.rs index 6cbba51823..82e942b6f1 100644 --- a/modules/src/clients/ics07_tendermint/client_state.rs +++ b/modules/src/clients/ics07_tendermint/client_state.rs @@ -17,7 +17,7 @@ use crate::core::ics02_client::client_type::ClientType; use crate::core::ics02_client::error::Error as Ics02Error; use crate::core::ics02_client::trust_threshold::TrustThreshold; use crate::core::ics24_host::identifier::ChainId; -use crate::timestamp::ZERO_DURATION; +use crate::timestamp::{Timestamp, ZERO_DURATION}; use crate::Height; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -163,6 +163,28 @@ impl ClientState { clock_drift: self.max_clock_drift, }) } + + /// Verify the time and height delays + pub fn verify_delay_passed( + &self, + current_time: Timestamp, + current_height: Height, + processed_time: Timestamp, + processed_height: Height, + delay_period_time: Duration, + delay_period_blocks: u64, + ) -> Result<(), Error> { + let time = (processed_time + delay_period_time).map_err(Error::timestamp_overflow)?; + if !(current_time == time || current_time.after(&time)) { + return Err(Error::not_enough_time_elapsed()); + } + + if current_height < processed_height.add(delay_period_blocks) { + return Err(Error::not_enough_blocks_elapsed()); + } + + Ok(()) + } } impl crate::core::ics02_client::client_state::ClientState for ClientState { diff --git a/modules/src/clients/ics07_tendermint/error.rs b/modules/src/clients/ics07_tendermint/error.rs index a4dfa64523..03dc85e738 100644 --- a/modules/src/clients/ics07_tendermint/error.rs +++ b/modules/src/clients/ics07_tendermint/error.rs @@ -4,6 +4,8 @@ use flex_error::{define_error, TraceError}; use crate::core::ics23_commitment::error::Error as Ics23Error; use crate::core::ics24_host::error::ValidationError; +use crate::core::ics24_host::identifier::ClientId; +use crate::timestamp::TimestampOverflowError; use crate::Height; use tendermint::account::Id; use tendermint::hash::Hash; @@ -147,6 +149,16 @@ define_error! { format_args!("given other previous updates, header timestamp should be at least {0}, but was {1}", e.min, e.actual) }, + TimestampOverflow + [ TimestampOverflowError ] + |_| { "timestamp overflowed" }, + + NotEnoughTimeElapsed + |_| { "not enough time elapsed" }, + + NotEnoughBlocksElapsed + |_| { "not enough blocks elapsed" }, + InvalidHeaderHeight { height: Height } | e | { @@ -201,6 +213,22 @@ define_error! { format_args!("verification failed: {}", e.detail) }, + ProcessedTimeNotFound + { client_id: ClientId } + | e | { + format_args!( + "Processed time for the client {0} not found", + e.client_id) + }, + + ProcessedHeightNotFound + { client_id: ClientId } + | e | { + format_args!( + "Processed height for the client {0} not found", + e.client_id) + }, + Ics23Error [ Ics23Error ] | _ | { "ics23 commitment error" }, diff --git a/modules/src/core/ics02_client/client_def.rs b/modules/src/core/ics02_client/client_def.rs index 31650f4bc8..2c888c6b04 100644 --- a/modules/src/core/ics02_client/client_def.rs +++ b/modules/src/core/ics02_client/client_def.rs @@ -9,11 +9,13 @@ use crate::core::ics02_client::error::Error; use crate::core::ics02_client::header::{AnyHeader, Header}; use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics04_channel::channel::ChannelEnd; +use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::packet::Sequence; use crate::core::ics23_commitment::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, }; use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; +use crate::core::ics24_host::Path; use crate::downcast; use crate::prelude::*; use crate::Height; @@ -102,13 +104,12 @@ pub trait ClientDef: Clone { #[allow(clippy::too_many_arguments)] fn verify_packet_data( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - seq: Sequence, + commitment_path: &Path, commitment: String, ) -> Result<(), Error>; @@ -116,13 +117,12 @@ pub trait ClientDef: Clone { #[allow(clippy::too_many_arguments)] fn verify_packet_acknowledgement( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - seq: Sequence, + ack_path: &Path, ack: Vec, ) -> Result<(), Error>; @@ -130,26 +130,24 @@ pub trait ClientDef: Clone { #[allow(clippy::too_many_arguments)] fn verify_next_sequence_recv( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, + seq_path: &Path, seq: Sequence, ) -> Result<(), Error>; /// Verify a `proof` that a packet has not been received. - #[allow(clippy::too_many_arguments)] fn verify_packet_receipt_absence( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - seq: Sequence, + receipt_path: &Path, ) -> Result<(), Error>; } @@ -401,13 +399,12 @@ impl ClientDef for AnyClient { } fn verify_packet_data( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - seq: Sequence, + commitment_path: &Path, commitment: String, ) -> Result<(), Error> { match self { @@ -418,13 +415,12 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Tendermint))?; client.verify_packet_data( + ctx, client_state, - prefix, + connection_end, proof, root, - port_id, - channel_id, - seq, + commitment_path, commitment, ) } @@ -437,13 +433,12 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Mock))?; client.verify_packet_data( + ctx, client_state, - prefix, + connection_end, proof, root, - port_id, - channel_id, - seq, + commitment_path, commitment, ) } @@ -452,13 +447,12 @@ impl ClientDef for AnyClient { fn verify_packet_acknowledgement( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - seq: Sequence, + ack_path: &Path, ack: Vec, ) -> Result<(), Error> { match self { @@ -469,13 +463,12 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Tendermint))?; client.verify_packet_acknowledgement( + ctx, client_state, - prefix, + connection_end, proof, root, - port_id, - channel_id, - seq, + ack_path, ack, ) } @@ -488,13 +481,12 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Mock))?; client.verify_packet_acknowledgement( + ctx, client_state, - prefix, + connection_end, proof, root, - port_id, - channel_id, - seq, + ack_path, ack, ) } @@ -503,12 +495,12 @@ impl ClientDef for AnyClient { fn verify_next_sequence_recv( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, + seq_path: &Path, seq: Sequence, ) -> Result<(), Error> { match self { @@ -519,12 +511,12 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Tendermint))?; client.verify_next_sequence_recv( + ctx, client_state, - prefix, + connection_end, proof, root, - port_id, - channel_id, + seq_path, seq, ) } @@ -537,12 +529,12 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Mock))?; client.verify_next_sequence_recv( + ctx, client_state, - prefix, + connection_end, proof, root, - port_id, - channel_id, + seq_path, seq, ) } @@ -550,13 +542,12 @@ impl ClientDef for AnyClient { } fn verify_packet_receipt_absence( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, - prefix: &CommitmentPrefix, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, - seq: Sequence, + receipt_path: &Path, ) -> Result<(), Error> { match self { Self::Tendermint(client) => { @@ -566,13 +557,12 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Tendermint))?; client.verify_packet_receipt_absence( + ctx, client_state, - prefix, + connection_end, proof, root, - port_id, - channel_id, - seq, + receipt_path, ) } @@ -584,13 +574,12 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Mock))?; client.verify_packet_receipt_absence( + ctx, client_state, - prefix, + connection_end, proof, root, - port_id, - channel_id, - seq, + receipt_path, ) } } diff --git a/modules/src/core/ics02_client/context.rs b/modules/src/core/ics02_client/context.rs index 1ef619414b..a4af0c287d 100644 --- a/modules/src/core/ics02_client/context.rs +++ b/modules/src/core/ics02_client/context.rs @@ -55,6 +55,9 @@ pub trait ClientReader { height: Height, ) -> Result, Error>; + /// Returns the current height of the local chain. + fn host_height(&self) -> Height; + /// Returns a natural number, counting how many clients have been created thus far. /// The value of this counter should increase only via method `ClientKeeper::increase_client_counter`. fn client_counter(&self) -> Result; diff --git a/modules/src/core/ics02_client/handler/create_client.rs b/modules/src/core/ics02_client/handler/create_client.rs index 563ab5eb32..506865c3a8 100644 --- a/modules/src/core/ics02_client/handler/create_client.rs +++ b/modules/src/core/ics02_client/handler/create_client.rs @@ -8,10 +8,12 @@ use crate::core::ics02_client::context::ClientReader; use crate::core::ics02_client::error::Error; use crate::core::ics02_client::events::Attributes; use crate::core::ics02_client::handler::ClientResult; +use crate::core::ics02_client::height::Height; use crate::core::ics02_client::msgs::create_client::MsgCreateAnyClient; use crate::core::ics24_host::identifier::ClientId; use crate::events::IbcEvent; use crate::handler::{HandlerOutput, HandlerResult}; +use crate::timestamp::Timestamp; /// The result following the successful processing of a `MsgCreateAnyClient` message. Preferably /// this data type should be used with a qualified name `create_client::Result` to avoid ambiguity. #[derive(Clone, Debug, PartialEq, Eq)] @@ -20,6 +22,8 @@ pub struct Result { pub client_type: ClientType, pub client_state: AnyClientState, pub consensus_state: AnyConsensusState, + pub processed_time: Timestamp, + pub processed_height: Height, } pub fn process( @@ -44,6 +48,8 @@ pub fn process( client_type: msg.client_state().client_type(), client_state: msg.client_state(), consensus_state: msg.consensus_state(), + processed_time: Timestamp::now(), + processed_height: ctx.host_height(), }); let event_attributes = Attributes { diff --git a/modules/src/core/ics04_channel/context.rs b/modules/src/core/ics04_channel/context.rs index ba7bd040b1..38b9c295fb 100644 --- a/modules/src/core/ics04_channel/context.rs +++ b/modules/src/core/ics04_channel/context.rs @@ -71,6 +71,12 @@ pub trait ChannelReader { /// Returns the current timestamp of the local chain. fn host_timestamp(&self) -> Timestamp; + /// Returns the time when the client state of the given identifier is stored + fn processed_time(&self, client_id: &ClientId) -> Result; + + /// Returns the height when the client state of the given identifier is stored + fn processed_height(&self, client_id: &ClientId) -> Result; + /// Returns a counter on the number of channel ids have been created thus far. /// The value of this counter should increase only via method /// `ChannelKeeper::increase_channel_counter`. diff --git a/modules/src/core/ics04_channel/error.rs b/modules/src/core/ics04_channel/error.rs index 0aa71ed37d..1c746a6f12 100644 --- a/modules/src/core/ics04_channel/error.rs +++ b/modules/src/core/ics04_channel/error.rs @@ -324,6 +324,22 @@ define_error! { e.port_channel_id.1) }, + ProcessedTimeNotFound + { client_id: ClientId } + | e | { + format_args!( + "Processed time for the client {0} not found", + e.client_id) + }, + + ProcessedHeightNotFound + { client_id: ClientId } + | e | { + format_args!( + "Processed height for the client {0} not found", + e.client_id) + }, + ImplementationSpecific | _ | { "implementation specific error" }, } diff --git a/modules/src/core/ics04_channel/handler/verify.rs b/modules/src/core/ics04_channel/handler/verify.rs index 6d0b2382e4..b162fd7b36 100644 --- a/modules/src/core/ics04_channel/handler/verify.rs +++ b/modules/src/core/ics04_channel/handler/verify.rs @@ -6,6 +6,7 @@ use crate::core::ics04_channel::channel::ChannelEnd; use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::error::Error; use crate::core::ics04_channel::packet::{Packet, Sequence}; +use crate::core::ics24_host::Path; use crate::prelude::*; use crate::proofs::Proofs; @@ -65,6 +66,11 @@ pub fn verify_packet_recv_proofs( let client_def = AnyClient::from_client_type(client_state.client_type()); + let commitment_path = Path::Commitments { + port_id: packet.source_port.clone(), + channel_id: packet.source_channel.clone(), + sequence: packet.sequence, + }; let input = format!( "{:?},{:?},{:?}", packet.timeout_timestamp, packet.timeout_height, packet.data @@ -74,13 +80,12 @@ pub fn verify_packet_recv_proofs( // Verify the proof for the packet against the chain store. client_def .verify_packet_data( + ctx, &client_state, - connection_end.counterparty().prefix(), + connection_end, proofs.object_proof(), consensus_state.root(), - &packet.source_port, - &packet.source_channel, - packet.sequence, + &commitment_path, commitment, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; @@ -108,16 +113,21 @@ pub fn verify_packet_acknowledgement_proofs( let client_def = AnyClient::from_client_type(client_state.client_type()); + let ack_path = Path::Acks { + port_id: packet.source_port.clone(), + channel_id: packet.source_channel.clone(), + sequence: packet.sequence, + }; + // Verify the proof for the packet against the chain store. client_def .verify_packet_acknowledgement( + ctx, &client_state, - connection_end.counterparty().prefix(), + connection_end, proofs.object_proof(), consensus_state.root(), - &packet.source_port, - &packet.source_channel, - packet.sequence, + &ack_path, acknowledgement, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; @@ -145,15 +155,17 @@ pub fn verify_next_sequence_recv( let client_def = AnyClient::from_client_type(client_state.client_type()); + let seq_path = Path::SeqRecvs(packet.destination_port.clone(), packet.destination_channel); + // Verify the proof for the packet against the chain store. client_def .verify_next_sequence_recv( + ctx, &client_state, - connection_end.counterparty().prefix(), + connection_end, proofs.object_proof(), consensus_state.root(), - &packet.destination_port, - &packet.destination_channel, + &seq_path, seq, ) .map_err(|e| Error::packet_verification_failed(seq, e))?; @@ -179,16 +191,21 @@ pub fn verify_packet_receipt_absence( let client_def = AnyClient::from_client_type(client_state.client_type()); + let receipt_path = Path::Receipts { + port_id: packet.destination_port.clone(), + channel_id: packet.destination_channel.clone(), + sequence: packet.sequence, + }; + // Verify the proof for the packet against the chain store. client_def .verify_packet_receipt_absence( + ctx, &client_state, - connection_end.counterparty().prefix(), + connection_end, proofs.object_proof(), consensus_state.root(), - &packet.destination_port, - &packet.destination_channel, - packet.sequence, + &receipt_path, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; diff --git a/modules/src/mock/client_def.rs b/modules/src/mock/client_def.rs index 88a4183710..1068de980e 100644 --- a/modules/src/mock/client_def.rs +++ b/modules/src/mock/client_def.rs @@ -7,6 +7,7 @@ use crate::core::ics02_client::context::ClientReader; use crate::core::ics02_client::error::Error; use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics04_channel::channel::ChannelEnd; +use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::packet::Sequence; use crate::core::ics23_commitment::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, @@ -105,13 +106,12 @@ impl ClientDef for MockClient { fn verify_packet_data( &self, + _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, - _prefix: &CommitmentPrefix, + _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: Sequence, + _commitment_path: &Path, _commitment: String, ) -> Result<(), Error> { Ok(()) @@ -119,13 +119,12 @@ impl ClientDef for MockClient { fn verify_packet_acknowledgement( &self, + _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, - _prefix: &CommitmentPrefix, + _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: Sequence, + _ack_path: &Path, _ack: Vec, ) -> Result<(), Error> { Ok(()) @@ -133,12 +132,12 @@ impl ClientDef for MockClient { fn verify_next_sequence_recv( &self, + _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, - _prefix: &CommitmentPrefix, + _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _port_id: &PortId, - _channel_id: &ChannelId, + _seq_path: &Path, _seq: Sequence, ) -> Result<(), Error> { Ok(()) @@ -146,13 +145,12 @@ impl ClientDef for MockClient { fn verify_packet_receipt_absence( &self, + _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, - _prefix: &CommitmentPrefix, + _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _port_id: &PortId, - _channel_id: &ChannelId, - _sequence: Sequence, + _receipt_path: &Path, ) -> Result<(), Error> { Ok(()) } diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index 73b9a1e5d3..1d60ba9998 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -68,6 +68,12 @@ pub struct MockContext { /// The set of all clients, indexed by their id. clients: BTreeMap, + /// Tracks the processed time for the clients + client_processed_times: BTreeMap, + + /// Tracks the processed height for the clients + client_processed_heights: BTreeMap, + /// Counter for the client identifiers, necessary for `increase_client_counter` and the /// `client_counter` methods. client_ids_counter: u64, @@ -171,6 +177,8 @@ impl MockContext { connections: Default::default(), client_ids_counter: 0, clients: Default::default(), + client_processed_times: Default::default(), + client_processed_heights: Default::default(), client_connections: Default::default(), channels: Default::default(), connection_channels: Default::default(), @@ -665,6 +673,20 @@ impl ChannelReader for MockContext { self.timestamp } + fn processed_time(&self, client_id: &ClientId) -> Result { + match self.client_processed_times.get(client_id) { + Some(time) => Ok(*time), + None => Err(Ics04Error::processed_time_not_found(client_id.clone())), + } + } + + fn processed_height(&self, client_id: &ClientId) -> Result { + match self.client_processed_heights.get(client_id) { + Some(height) => Ok(*height), + None => Err(Ics04Error::processed_height_not_found(client_id.clone())), + } + } + fn channel_counter(&self) -> Result { Ok(self.channel_ids_counter) } @@ -939,6 +961,10 @@ impl ClientReader for MockContext { Ok(None) } + fn host_height(&self) -> Height { + self.latest_height + } + fn client_counter(&self) -> Result { Ok(self.client_ids_counter) } diff --git a/modules/src/timestamp.rs b/modules/src/timestamp.rs index 1e9cd3fdd2..d0fd2ec5a9 100644 --- a/modules/src/timestamp.rs +++ b/modules/src/timestamp.rs @@ -164,6 +164,7 @@ impl Display for Timestamp { } define_error! { + #[derive(Debug, PartialEq, Eq)] TimestampOverflowError { TimestampOverflow |_| { "Timestamp overflow when modifying with duration" } From 4735aa7c140af96c761e597fe14a6ac23eb99460 Mon Sep 17 00:00:00 2001 From: yito88 Date: Tue, 7 Dec 2021 17:38:19 +0900 Subject: [PATCH 04/26] verify height --- .../clients/ics07_tendermint/client_def.rs | 98 ++++++++------ modules/src/core/ics02_client/client_def.rs | 128 +++++++++--------- modules/src/core/ics02_client/client_state.rs | 14 ++ modules/src/core/ics02_client/error.rs | 24 ++++ modules/src/core/ics03_connection/error.rs | 3 - .../core/ics03_connection/handler/verify.rs | 96 ++++++------- .../src/core/ics04_channel/handler/verify.rs | 29 ++-- modules/src/mock/client_def.rs | 45 +++--- 8 files changed, 233 insertions(+), 204 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index 2988ede510..0bcfdeb78e 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -26,11 +26,10 @@ use crate::core::ics23_commitment::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, }; use crate::core::ics23_commitment::merkle::{apply_prefix, MerkleProof}; -use crate::core::ics24_host::identifier::ConnectionId; -use crate::core::ics24_host::identifier::{ChannelId, ClientId, PortId}; +use crate::core::ics24_host::identifier::ClientId; use crate::core::ics24_host::Path; use crate::prelude::*; -use crate::Height; +use crate::proofs::Proofs; use crate::downcast; @@ -192,63 +191,88 @@ impl ClientDef for TendermintClient { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - client_id: &ClientId, - consensus_height: Height, + consensus_state_path: &Path, expected_consensus_state: &AnyConsensusState, ) -> Result<(), Ics02Error> { - let path = Path::ClientConsensusState { - client_id: client_id.clone(), - epoch: consensus_height.revision_number, - height: consensus_height.revision_height, - } - .to_string(); + let consensus_proof = proofs + .consensus_proof() + .ok_or_else(Ics02Error::null_consensus_proof)?; let value = expected_consensus_state.encode_vec().unwrap(); - verify_membership(client_state, prefix, proof, root, path, value) + verify_membership( + client_state, + prefix, + consensus_proof.proof(), + root, + consensus_state_path.to_string(), + value, + ) } fn verify_connection_state( &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - connection_id: &ConnectionId, + connection_path: &Path, expected_connection_end: &ConnectionEnd, ) -> Result<(), Ics02Error> { - let path = Path::Connections(connection_id.clone()).to_string(); let value = expected_connection_end.encode_vec().unwrap(); - verify_membership(client_state, prefix, proof, root, path, value) + verify_membership( + client_state, + prefix, + proofs.object_proof(), + root, + connection_path.to_string(), + value, + ) } fn verify_channel_state( &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, + channel_path: &Path, expected_channel_end: &ChannelEnd, ) -> Result<(), Ics02Error> { - let path = Path::ChannelEnds(port_id.clone(), channel_id.clone()).to_string(); let value = expected_channel_end.encode_vec().unwrap(); - verify_membership(client_state, prefix, proof, root, path, value) + verify_membership( + client_state, + prefix, + proofs.object_proof(), + root, + channel_path.to_string(), + value, + ) } fn verify_client_full_state( &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - client_id: &ClientId, + client_state_path: &Path, expected_client_state: &AnyClientState, ) -> Result<(), Ics02Error> { - let path = Path::ClientState(client_id.clone()).to_string(); + let proof = proofs + .client_proof() + .as_ref() + .ok_or_else(Ics02Error::null_client_proof)?; + let value = expected_client_state.encode_vec().unwrap(); - verify_membership(client_state, prefix, proof, root, path, value) + verify_membership( + client_state, + prefix, + proof, + root, + client_state_path.to_string(), + value, + ) } fn verify_packet_data( @@ -256,7 +280,7 @@ impl ClientDef for TendermintClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, commitment_path: &Path, commitment: String, @@ -266,7 +290,7 @@ impl ClientDef for TendermintClient { verify_membership( client_state, connection_end.counterparty().prefix(), - proof, + proofs.object_proof(), root, commitment_path.to_string(), commitment.encode_to_vec(), @@ -278,7 +302,7 @@ impl ClientDef for TendermintClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, ack_path: &Path, ack: Vec, @@ -288,7 +312,7 @@ impl ClientDef for TendermintClient { verify_membership( client_state, connection_end.counterparty().prefix(), - proof, + proofs.object_proof(), root, ack_path.to_string(), ack, @@ -300,7 +324,7 @@ impl ClientDef for TendermintClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, seq_path: &Path, seq: Sequence, @@ -310,7 +334,7 @@ impl ClientDef for TendermintClient { verify_membership( client_state, connection_end.counterparty().prefix(), - proof, + proofs.object_proof(), root, seq_path.to_string(), u64::from(seq).encode_to_vec(), @@ -322,7 +346,7 @@ impl ClientDef for TendermintClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, receipt_path: &Path, ) -> Result<(), Ics02Error> { @@ -331,7 +355,7 @@ impl ClientDef for TendermintClient { verify_non_membership( client_state, connection_end.counterparty().prefix(), - proof, + proofs.object_proof(), root, receipt_path.to_string(), ) @@ -385,11 +409,7 @@ fn verify_non_membership( .into(); merkle_proof - .verify_non_membership( - &client_state.proof_specs, - root.clone().into(), - merkle_path, - ) + .verify_non_membership(&client_state.proof_specs, root.clone().into(), merkle_path) .map_err(|e| Ics02Error::tendermint(Error::ics23_error(e))) } diff --git a/modules/src/core/ics02_client/client_def.rs b/modules/src/core/ics02_client/client_def.rs index 2c888c6b04..3e287d978f 100644 --- a/modules/src/core/ics02_client/client_def.rs +++ b/modules/src/core/ics02_client/client_def.rs @@ -11,14 +11,12 @@ use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics04_channel::channel::ChannelEnd; use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::packet::Sequence; -use crate::core::ics23_commitment::commitment::{ - CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, -}; -use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; +use crate::core::ics23_commitment::commitment::{CommitmentPrefix, CommitmentRoot}; +use crate::core::ics24_host::identifier::ClientId; use crate::core::ics24_host::Path; use crate::downcast; use crate::prelude::*; -use crate::Height; +use crate::proofs::Proofs; #[cfg(any(test, feature = "mocks"))] use crate::mock::client_def::MockClient; @@ -57,10 +55,9 @@ pub trait ClientDef: Clone { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - client_id: &ClientId, - consensus_height: Height, + consensus_state_path: &Path, expected_consensus_state: &AnyConsensusState, ) -> Result<(), Error>; @@ -69,9 +66,9 @@ pub trait ClientDef: Clone { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - connection_id: &ConnectionId, + connection_path: &Path, expected_connection_end: &ConnectionEnd, ) -> Result<(), Error>; @@ -81,10 +78,9 @@ pub trait ClientDef: Clone { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, + channel_path: &Path, expected_channel_end: &ChannelEnd, ) -> Result<(), Error>; @@ -94,9 +90,9 @@ pub trait ClientDef: Clone { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - client_id: &ClientId, + client_state_path: &Path, expected_client_state: &AnyClientState, ) -> Result<(), Error>; @@ -107,7 +103,7 @@ pub trait ClientDef: Clone { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, commitment_path: &Path, commitment: String, @@ -120,7 +116,7 @@ pub trait ClientDef: Clone { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, ack_path: &Path, ack: Vec, @@ -133,7 +129,7 @@ pub trait ClientDef: Clone { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, seq_path: &Path, seq: Sequence, @@ -145,7 +141,7 @@ pub trait ClientDef: Clone { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, receipt_path: &Path, ) -> Result<(), Error>; @@ -224,12 +220,13 @@ impl ClientDef for AnyClient { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - client_id: &ClientId, - consensus_height: Height, + consensus_state_path: &Path, expected_consensus_state: &AnyConsensusState, ) -> Result<(), Error> { + client_state.verify_height(proofs.height())?; + match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -240,10 +237,9 @@ impl ClientDef for AnyClient { client.verify_client_consensus_state( client_state, prefix, - proof, + proofs, root, - client_id, - consensus_height, + consensus_state_path, expected_consensus_state, ) } @@ -258,10 +254,9 @@ impl ClientDef for AnyClient { client.verify_client_consensus_state( client_state, prefix, - proof, + proofs, root, - client_id, - consensus_height, + consensus_state_path, expected_consensus_state, ) } @@ -272,11 +267,13 @@ impl ClientDef for AnyClient { &self, client_state: &AnyClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - connection_id: &ConnectionId, + connection_path: &Path, expected_connection_end: &ConnectionEnd, ) -> Result<(), Error> { + client_state.verify_height(proofs.height())?; + match self { Self::Tendermint(client) => { let client_state = downcast!(client_state => AnyClientState::Tendermint) @@ -285,9 +282,9 @@ impl ClientDef for AnyClient { client.verify_connection_state( client_state, prefix, - proof, + proofs, root, - connection_id, + connection_path, expected_connection_end, ) } @@ -300,9 +297,9 @@ impl ClientDef for AnyClient { client.verify_connection_state( client_state, prefix, - proof, + proofs, root, - connection_id, + connection_path, expected_connection_end, ) } @@ -313,12 +310,13 @@ impl ClientDef for AnyClient { &self, client_state: &AnyClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - port_id: &PortId, - channel_id: &ChannelId, + channel_path: &Path, expected_channel_end: &ChannelEnd, ) -> Result<(), Error> { + client_state.verify_height(proofs.height())?; + match self { Self::Tendermint(client) => { let client_state = downcast!(client_state => AnyClientState::Tendermint) @@ -327,10 +325,9 @@ impl ClientDef for AnyClient { client.verify_channel_state( client_state, prefix, - proof, + proofs, root, - port_id, - channel_id, + channel_path, expected_channel_end, ) } @@ -343,10 +340,9 @@ impl ClientDef for AnyClient { client.verify_channel_state( client_state, prefix, - proof, + proofs, root, - port_id, - channel_id, + channel_path, expected_channel_end, ) } @@ -357,11 +353,13 @@ impl ClientDef for AnyClient { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, - client_id: &ClientId, + client_state_path: &Path, client_state_on_counterparty: &AnyClientState, ) -> Result<(), Error> { + client_state.verify_height(proofs.height())?; + match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -372,9 +370,9 @@ impl ClientDef for AnyClient { client.verify_client_full_state( client_state, prefix, - proof, + proofs, root, - client_id, + client_state_path, client_state_on_counterparty, ) } @@ -389,9 +387,9 @@ impl ClientDef for AnyClient { client.verify_client_full_state( client_state, prefix, - proof, + proofs, root, - client_id, + client_state_path, client_state_on_counterparty, ) } @@ -402,11 +400,13 @@ impl ClientDef for AnyClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, commitment_path: &Path, commitment: String, ) -> Result<(), Error> { + client_state.verify_height(proofs.height())?; + match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -418,7 +418,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proof, + proofs, root, commitment_path, commitment, @@ -436,7 +436,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proof, + proofs, root, commitment_path, commitment, @@ -450,11 +450,13 @@ impl ClientDef for AnyClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, ack_path: &Path, ack: Vec, ) -> Result<(), Error> { + client_state.verify_height(proofs.height())?; + match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -466,7 +468,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proof, + proofs, root, ack_path, ack, @@ -484,7 +486,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proof, + proofs, root, ack_path, ack, @@ -498,11 +500,13 @@ impl ClientDef for AnyClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, seq_path: &Path, seq: Sequence, ) -> Result<(), Error> { + client_state.verify_height(proofs.height())?; + match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -514,7 +518,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proof, + proofs, root, seq_path, seq, @@ -532,7 +536,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proof, + proofs, root, seq_path, seq, @@ -545,10 +549,12 @@ impl ClientDef for AnyClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proof: &CommitmentProofBytes, + proofs: &Proofs, root: &CommitmentRoot, receipt_path: &Path, ) -> Result<(), Error> { + client_state.verify_height(proofs.height())?; + match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -560,7 +566,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proof, + proofs, root, receipt_path, ) @@ -577,7 +583,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proof, + proofs, root, receipt_path, ) diff --git a/modules/src/core/ics02_client/client_state.rs b/modules/src/core/ics02_client/client_state.rs index f53ac517bd..651261ade3 100644 --- a/modules/src/core/ics02_client/client_state.rs +++ b/modules/src/core/ics02_client/client_state.rs @@ -116,6 +116,20 @@ impl AnyClientState { AnyClientState::Mock(mock_state) => mock_state.expired(elapsed_since_latest), } } + + /// Verify that the client is at a sufficient height and unfrozen at the given height + pub fn verify_height(&self, height: Height) -> Result<(), Error> { + if self.latest_height() < height { + return Err(Error::insufficient_height(self.latest_height(), height)); + } + + match self.frozen_height() { + Some(frozen_height) if frozen_height <= height => { + Err(Error::frozen_height(frozen_height, height)) + } + _ => Ok(()), + } + } } impl Protobuf for AnyClientState {} diff --git a/modules/src/core/ics02_client/error.rs b/modules/src/core/ics02_client/error.rs index d3da2bbf76..945d7b8d91 100644 --- a/modules/src/core/ics02_client/error.rs +++ b/modules/src/core/ics02_client/error.rs @@ -173,6 +173,12 @@ define_error! { [ Ics23Error ] | _ | { "invalid proof for the consensus state" }, + NullClientProof + | _ | { "client state proof must be present" }, + + NullConsensusProof + | _ | { "consensus state proof must be present" }, + Tendermint [ Ics07Error ] | _ | { "tendermint error" }, @@ -242,6 +248,24 @@ define_error! { format_args!("header not withing trusting period: expires_at={0} now={1}", e.latest_time, e.update_time) }, + InsufficientHeight + { + latest_height: Height, + target_height: Height, + } + | e | { + format_args!("the height is insufficient: latest_height={0} target_height={1}", e.latest_height, e.target_height) + }, + + FrozenHeight + { + frozen_height: Height, + target_height: Height, + } + | e | { + format_args!("the client is frozen at the height: frozen_height={0} target_height={1}", e.frozen_height, e.target_height) + }, + TendermintHandlerError [ Ics07Error ] | _ | { format_args!("Tendermint-specific handler error") }, diff --git a/modules/src/core/ics03_connection/error.rs b/modules/src/core/ics03_connection/error.rs index 2a0d687e05..b0aed21f5c 100644 --- a/modules/src/core/ics03_connection/error.rs +++ b/modules/src/core/ics03_connection/error.rs @@ -113,9 +113,6 @@ define_error! { MissingCounterpartyPrefix | _ | { "missing counterparty prefix" }, - NullClientProof - | _ | { "client proof must be present" }, - FrozenClient { client_id: ClientId } | e | { diff --git a/modules/src/core/ics03_connection/handler/verify.rs b/modules/src/core/ics03_connection/handler/verify.rs index 86ac2e4ec6..fe89882d6f 100644 --- a/modules/src/core/ics03_connection/handler/verify.rs +++ b/modules/src/core/ics03_connection/handler/verify.rs @@ -1,13 +1,13 @@ //! ICS3 verification functions, common across all four handlers of ICS3. use crate::core::ics02_client::client_consensus::ConsensusState; -use crate::core::ics02_client::client_state::{AnyClientState, ClientState}; +use crate::core::ics02_client::client_state::AnyClientState; use crate::core::ics02_client::{client_def::AnyClient, client_def::ClientDef}; use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics03_connection::context::ConnectionReader; use crate::core::ics03_connection::error::Error; -use crate::core::ics23_commitment::commitment::CommitmentProofBytes; -use crate::proofs::{ConsensusProof, Proofs}; +use crate::core::ics24_host::Path; +use crate::proofs::Proofs; use crate::Height; /// Entry point for verifying all proofs bundled in any ICS3 message. @@ -18,34 +18,14 @@ pub fn verify_proofs( expected_conn: &ConnectionEnd, proofs: &Proofs, ) -> Result<(), Error> { - verify_connection_proof( - ctx, - connection_end, - expected_conn, - proofs.height(), - proofs.object_proof(), - )?; + verify_connection_proof(ctx, connection_end, expected_conn, proofs)?; // If the message includes a client state, then verify the proof for that state. if let Some(expected_client_state) = client_state { - verify_client_proof( - ctx, - connection_end, - expected_client_state, - proofs.height(), - proofs - .client_proof() - .as_ref() - .ok_or_else(Error::null_client_proof)?, - )?; + verify_client_proof(ctx, connection_end, expected_client_state, proofs)?; } - // If a consensus proof is attached to the message, then verify it. - if let Some(proof) = proofs.consensus_proof() { - Ok(verify_consensus_proof(ctx, connection_end, &proof)?) - } else { - Ok(()) - } + verify_consensus_proof(ctx, connection_end, proofs) } /// Verifies the authenticity and semantic correctness of a commitment `proof`. The commitment @@ -55,19 +35,14 @@ pub fn verify_connection_proof( ctx: &dyn ConnectionReader, connection_end: &ConnectionEnd, expected_conn: &ConnectionEnd, - proof_height: Height, - proof: &CommitmentProofBytes, + proofs: &Proofs, ) -> Result<(), Error> { // Fetch the client state (IBC client on the local/host chain). let client_state = ctx.client_state(connection_end.client_id())?; - // The client must not be frozen. - if client_state.is_frozen() { - return Err(Error::frozen_client(connection_end.client_id().clone())); - } - // The client must have the consensus state for the height where this proof was created. - let consensus_state = ctx.client_consensus_state(connection_end.client_id(), proof_height)?; + let consensus_state = + ctx.client_consensus_state(connection_end.client_id(), proofs.height())?; // A counterparty connection id of None causes `unwrap()` below and indicates an internal // error as this is the connection id on the counterparty chain that must always be present. @@ -75,6 +50,7 @@ pub fn verify_connection_proof( .counterparty() .connection_id() .ok_or_else(Error::invalid_counterparty)?; + let path = Path::Connections(connection_id.clone()); let client_def = AnyClient::from_client_type(client_state.client_type()); @@ -83,9 +59,9 @@ pub fn verify_connection_proof( .verify_connection_state( &client_state, connection_end.counterparty().prefix(), - proof, + proofs, consensus_state.root(), - connection_id, + &path, expected_conn, ) .map_err(Error::verify_connection_state) @@ -102,17 +78,15 @@ pub fn verify_client_proof( ctx: &dyn ConnectionReader, connection_end: &ConnectionEnd, expected_client_state: AnyClientState, - proof_height: Height, - proof: &CommitmentProofBytes, + proofs: &Proofs, ) -> Result<(), Error> { - // Fetch the local client state (IBC client running on the host chain). - let client_state = ctx.client_state(connection_end.client_id())?; + let client_id = connection_end.client_id(); + let path = Path::ClientState(client_id.clone()); - if client_state.is_frozen() { - return Err(Error::frozen_client(connection_end.client_id().clone())); - } + // Fetch the local client state (IBC client running on the host chain). + let client_state = ctx.client_state(client_id)?; - let consensus_state = ctx.client_consensus_state(connection_end.client_id(), proof_height)?; + let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; let client_def = AnyClient::from_client_type(client_state.client_type()); @@ -120,9 +94,9 @@ pub fn verify_client_proof( .verify_client_full_state( &client_state, connection_end.counterparty().prefix(), - proof, + proofs, consensus_state.root(), - connection_end.counterparty().client_id(), + &path, &expected_client_state, ) .map_err(|e| { @@ -133,17 +107,26 @@ pub fn verify_client_proof( pub fn verify_consensus_proof( ctx: &dyn ConnectionReader, connection_end: &ConnectionEnd, - proof: &ConsensusProof, + proofs: &Proofs, ) -> Result<(), Error> { - // Fetch the client state (IBC client on the local chain). - let client_state = ctx.client_state(connection_end.client_id())?; + // If a consensus proof is attached to the message, then verify it. + let consensus_height = match proofs.consensus_proof() { + Some(consensus_proof) => consensus_proof.height(), + None => return Ok(()), + }; + + let client_id = connection_end.client_id(); + let path = Path::ClientConsensusState { + client_id: client_id.clone(), + epoch: consensus_height.revision_number, + height: consensus_height.revision_height, + }; - if client_state.is_frozen() { - return Err(Error::frozen_client(connection_end.client_id().clone())); - } + // Fetch the client state (IBC client on the local chain). + let client_state = ctx.client_state(client_id)?; // Fetch the expected consensus state from the historical (local) header data. - let expected_consensus = ctx.host_consensus_state(proof.height())?; + let expected_consensus = ctx.host_consensus_state(consensus_height)?; let client = AnyClient::from_client_type(client_state.client_type()); @@ -151,13 +134,12 @@ pub fn verify_consensus_proof( .verify_client_consensus_state( &client_state, connection_end.counterparty().prefix(), - proof.proof(), + proofs, expected_consensus.root(), - connection_end.counterparty().client_id(), - proof.height(), + &path, &expected_consensus, ) - .map_err(|e| Error::consensus_state_verification_failure(proof.height(), e)) + .map_err(|e| Error::consensus_state_verification_failure(consensus_height, e)) } /// Checks that `claimed_height` is within normal bounds, i.e., fresh enough so that the chain has diff --git a/modules/src/core/ics04_channel/handler/verify.rs b/modules/src/core/ics04_channel/handler/verify.rs index b162fd7b36..7f99ef76aa 100644 --- a/modules/src/core/ics04_channel/handler/verify.rs +++ b/modules/src/core/ics04_channel/handler/verify.rs @@ -18,17 +18,15 @@ pub fn verify_channel_proofs( expected_chan: &ChannelEnd, proofs: &Proofs, ) -> Result<(), Error> { - // This is the client which will perform proof verification. - let client_id = connection_end.client_id().clone(); - - let client_state = ctx.client_state(&client_id)?; + let client_id = connection_end.client_id(); + let path = Path::ChannelEnds( + channel_end.counterparty().port_id().clone(), + channel_end.counterparty().channel_id().unwrap().clone(), + ); - // The client must not be frozen. - if client_state.is_frozen() { - return Err(Error::frozen_client(client_id)); - } + let client_state = ctx.client_state(client_id)?; - let consensus_state = ctx.client_consensus_state(&client_id, proofs.height())?; + let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; let client_def = AnyClient::from_client_type(client_state.client_type()); @@ -38,10 +36,9 @@ pub fn verify_channel_proofs( .verify_channel_state( &client_state, connection_end.counterparty().prefix(), - proofs.object_proof(), + proofs, consensus_state.root(), - channel_end.counterparty().port_id(), - channel_end.counterparty().channel_id().unwrap(), + &path, expected_chan, ) .map_err(Error::verify_channel_failed) @@ -83,7 +80,7 @@ pub fn verify_packet_recv_proofs( ctx, &client_state, connection_end, - proofs.object_proof(), + proofs, consensus_state.root(), &commitment_path, commitment, @@ -125,7 +122,7 @@ pub fn verify_packet_acknowledgement_proofs( ctx, &client_state, connection_end, - proofs.object_proof(), + proofs, consensus_state.root(), &ack_path, acknowledgement, @@ -163,7 +160,7 @@ pub fn verify_next_sequence_recv( ctx, &client_state, connection_end, - proofs.object_proof(), + proofs, consensus_state.root(), &seq_path, seq, @@ -203,7 +200,7 @@ pub fn verify_packet_receipt_absence( ctx, &client_state, connection_end, - proofs.object_proof(), + proofs, consensus_state.root(), &receipt_path, ) diff --git a/modules/src/mock/client_def.rs b/modules/src/mock/client_def.rs index 4eed21d454..36f8f16e36 100644 --- a/modules/src/mock/client_def.rs +++ b/modules/src/mock/client_def.rs @@ -9,16 +9,14 @@ use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics04_channel::channel::ChannelEnd; use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::packet::Sequence; -use crate::core::ics23_commitment::commitment::{ - CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, -}; +use crate::core::ics23_commitment::commitment::{CommitmentPrefix, CommitmentRoot}; use crate::core::ics23_commitment::merkle::apply_prefix; -use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; +use crate::core::ics24_host::identifier::ClientId; use crate::core::ics24_host::Path; use crate::mock::client_state::{MockClientState, MockConsensusState}; use crate::mock::header::MockHeader; use crate::prelude::*; -use crate::Height; +use crate::proofs::Proofs; #[derive(Clone, Debug, PartialEq, Eq)] pub struct MockClient; @@ -51,21 +49,13 @@ impl ClientDef for MockClient { &self, _client_state: &Self::ClientState, prefix: &CommitmentPrefix, - _proof: &CommitmentProofBytes, + _proofs: &Proofs, _root: &CommitmentRoot, - client_id: &ClientId, - consensus_height: Height, + consensus_state_path: &Path, _expected_consensus_state: &AnyConsensusState, ) -> Result<(), Error> { - let client_prefixed_path = Path::ClientConsensusState { - client_id: client_id.clone(), - epoch: consensus_height.revision_number, - height: consensus_height.revision_height, - } - .to_string(); - - let _path = - apply_prefix(prefix, vec![client_prefixed_path]).map_err(Error::empty_prefix)?; + let _path = apply_prefix(prefix, vec![consensus_state_path.to_string()]) + .map_err(Error::empty_prefix)?; Ok(()) } @@ -74,9 +64,9 @@ impl ClientDef for MockClient { &self, _client_state: &Self::ClientState, _prefix: &CommitmentPrefix, - _proof: &CommitmentProofBytes, + _proofs: &Proofs, _root: &CommitmentRoot, - _connection_id: &ConnectionId, + _connection_path: &Path, _expected_connection_end: &ConnectionEnd, ) -> Result<(), Error> { Ok(()) @@ -86,10 +76,9 @@ impl ClientDef for MockClient { &self, _client_state: &Self::ClientState, _prefix: &CommitmentPrefix, - _proof: &CommitmentProofBytes, + _proofs: &Proofs, _root: &CommitmentRoot, - _port_id: &PortId, - _channel_id: &ChannelId, + _channel_path: &Path, _expected_channel_end: &ChannelEnd, ) -> Result<(), Error> { Ok(()) @@ -99,9 +88,9 @@ impl ClientDef for MockClient { &self, _client_state: &Self::ClientState, _prefix: &CommitmentPrefix, - _proof: &CommitmentProofBytes, + _proofs: &Proofs, _root: &CommitmentRoot, - _client_id: &ClientId, + _client_state_path: &Path, _expected_client_state: &AnyClientState, ) -> Result<(), Error> { Ok(()) @@ -112,7 +101,7 @@ impl ClientDef for MockClient { _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _connection_end: &ConnectionEnd, - _proof: &CommitmentProofBytes, + _proofs: &Proofs, _root: &CommitmentRoot, _commitment_path: &Path, _commitment: String, @@ -125,7 +114,7 @@ impl ClientDef for MockClient { _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _connection_end: &ConnectionEnd, - _proof: &CommitmentProofBytes, + _proofs: &Proofs, _root: &CommitmentRoot, _ack_path: &Path, _ack: Vec, @@ -138,7 +127,7 @@ impl ClientDef for MockClient { _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _connection_end: &ConnectionEnd, - _proof: &CommitmentProofBytes, + _proofs: &Proofs, _root: &CommitmentRoot, _seq_path: &Path, _seq: Sequence, @@ -151,7 +140,7 @@ impl ClientDef for MockClient { _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _connection_end: &ConnectionEnd, - _proof: &CommitmentProofBytes, + _proofs: &Proofs, _root: &CommitmentRoot, _receipt_path: &Path, ) -> Result<(), Error> { From aa2336bdd9ae5cb210c61434dc12b8be58f94be7 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Thu, 9 Dec 2021 23:17:16 +0530 Subject: [PATCH 05/26] Revert "verify height" This reverts commit 4735aa7c140af96c761e597fe14a6ac23eb99460. --- .../clients/ics07_tendermint/client_def.rs | 98 ++++++-------- modules/src/core/ics02_client/client_def.rs | 128 +++++++++--------- modules/src/core/ics02_client/client_state.rs | 14 -- modules/src/core/ics02_client/error.rs | 24 ---- modules/src/core/ics03_connection/error.rs | 3 + .../core/ics03_connection/handler/verify.rs | 96 +++++++------ .../src/core/ics04_channel/handler/verify.rs | 29 ++-- modules/src/mock/client_def.rs | 45 +++--- 8 files changed, 204 insertions(+), 233 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index 0bcfdeb78e..2988ede510 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -26,10 +26,11 @@ use crate::core::ics23_commitment::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, }; use crate::core::ics23_commitment::merkle::{apply_prefix, MerkleProof}; -use crate::core::ics24_host::identifier::ClientId; +use crate::core::ics24_host::identifier::ConnectionId; +use crate::core::ics24_host::identifier::{ChannelId, ClientId, PortId}; use crate::core::ics24_host::Path; use crate::prelude::*; -use crate::proofs::Proofs; +use crate::Height; use crate::downcast; @@ -191,88 +192,63 @@ impl ClientDef for TendermintClient { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - consensus_state_path: &Path, + client_id: &ClientId, + consensus_height: Height, expected_consensus_state: &AnyConsensusState, ) -> Result<(), Ics02Error> { - let consensus_proof = proofs - .consensus_proof() - .ok_or_else(Ics02Error::null_consensus_proof)?; + let path = Path::ClientConsensusState { + client_id: client_id.clone(), + epoch: consensus_height.revision_number, + height: consensus_height.revision_height, + } + .to_string(); let value = expected_consensus_state.encode_vec().unwrap(); - verify_membership( - client_state, - prefix, - consensus_proof.proof(), - root, - consensus_state_path.to_string(), - value, - ) + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_connection_state( &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - connection_path: &Path, + connection_id: &ConnectionId, expected_connection_end: &ConnectionEnd, ) -> Result<(), Ics02Error> { + let path = Path::Connections(connection_id.clone()).to_string(); let value = expected_connection_end.encode_vec().unwrap(); - verify_membership( - client_state, - prefix, - proofs.object_proof(), - root, - connection_path.to_string(), - value, - ) + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_channel_state( &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - channel_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, expected_channel_end: &ChannelEnd, ) -> Result<(), Ics02Error> { + let path = Path::ChannelEnds(port_id.clone(), channel_id.clone()).to_string(); let value = expected_channel_end.encode_vec().unwrap(); - verify_membership( - client_state, - prefix, - proofs.object_proof(), - root, - channel_path.to_string(), - value, - ) + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_client_full_state( &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - client_state_path: &Path, + client_id: &ClientId, expected_client_state: &AnyClientState, ) -> Result<(), Ics02Error> { - let proof = proofs - .client_proof() - .as_ref() - .ok_or_else(Ics02Error::null_client_proof)?; - + let path = Path::ClientState(client_id.clone()).to_string(); let value = expected_client_state.encode_vec().unwrap(); - verify_membership( - client_state, - prefix, - proof, - root, - client_state_path.to_string(), - value, - ) + verify_membership(client_state, prefix, proof, root, path, value) } fn verify_packet_data( @@ -280,7 +256,7 @@ impl ClientDef for TendermintClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, commitment_path: &Path, commitment: String, @@ -290,7 +266,7 @@ impl ClientDef for TendermintClient { verify_membership( client_state, connection_end.counterparty().prefix(), - proofs.object_proof(), + proof, root, commitment_path.to_string(), commitment.encode_to_vec(), @@ -302,7 +278,7 @@ impl ClientDef for TendermintClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, ack_path: &Path, ack: Vec, @@ -312,7 +288,7 @@ impl ClientDef for TendermintClient { verify_membership( client_state, connection_end.counterparty().prefix(), - proofs.object_proof(), + proof, root, ack_path.to_string(), ack, @@ -324,7 +300,7 @@ impl ClientDef for TendermintClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, seq_path: &Path, seq: Sequence, @@ -334,7 +310,7 @@ impl ClientDef for TendermintClient { verify_membership( client_state, connection_end.counterparty().prefix(), - proofs.object_proof(), + proof, root, seq_path.to_string(), u64::from(seq).encode_to_vec(), @@ -346,7 +322,7 @@ impl ClientDef for TendermintClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, receipt_path: &Path, ) -> Result<(), Ics02Error> { @@ -355,7 +331,7 @@ impl ClientDef for TendermintClient { verify_non_membership( client_state, connection_end.counterparty().prefix(), - proofs.object_proof(), + proof, root, receipt_path.to_string(), ) @@ -409,7 +385,11 @@ fn verify_non_membership( .into(); merkle_proof - .verify_non_membership(&client_state.proof_specs, root.clone().into(), merkle_path) + .verify_non_membership( + &client_state.proof_specs, + root.clone().into(), + merkle_path, + ) .map_err(|e| Ics02Error::tendermint(Error::ics23_error(e))) } diff --git a/modules/src/core/ics02_client/client_def.rs b/modules/src/core/ics02_client/client_def.rs index 3e287d978f..2c888c6b04 100644 --- a/modules/src/core/ics02_client/client_def.rs +++ b/modules/src/core/ics02_client/client_def.rs @@ -11,12 +11,14 @@ use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics04_channel::channel::ChannelEnd; use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::packet::Sequence; -use crate::core::ics23_commitment::commitment::{CommitmentPrefix, CommitmentRoot}; -use crate::core::ics24_host::identifier::ClientId; +use crate::core::ics23_commitment::commitment::{ + CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, +}; +use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; use crate::core::ics24_host::Path; use crate::downcast; use crate::prelude::*; -use crate::proofs::Proofs; +use crate::Height; #[cfg(any(test, feature = "mocks"))] use crate::mock::client_def::MockClient; @@ -55,9 +57,10 @@ pub trait ClientDef: Clone { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - consensus_state_path: &Path, + client_id: &ClientId, + consensus_height: Height, expected_consensus_state: &AnyConsensusState, ) -> Result<(), Error>; @@ -66,9 +69,9 @@ pub trait ClientDef: Clone { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - connection_path: &Path, + connection_id: &ConnectionId, expected_connection_end: &ConnectionEnd, ) -> Result<(), Error>; @@ -78,9 +81,10 @@ pub trait ClientDef: Clone { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - channel_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, expected_channel_end: &ChannelEnd, ) -> Result<(), Error>; @@ -90,9 +94,9 @@ pub trait ClientDef: Clone { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - client_state_path: &Path, + client_id: &ClientId, expected_client_state: &AnyClientState, ) -> Result<(), Error>; @@ -103,7 +107,7 @@ pub trait ClientDef: Clone { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, commitment_path: &Path, commitment: String, @@ -116,7 +120,7 @@ pub trait ClientDef: Clone { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, ack_path: &Path, ack: Vec, @@ -129,7 +133,7 @@ pub trait ClientDef: Clone { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, seq_path: &Path, seq: Sequence, @@ -141,7 +145,7 @@ pub trait ClientDef: Clone { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, receipt_path: &Path, ) -> Result<(), Error>; @@ -220,13 +224,12 @@ impl ClientDef for AnyClient { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - consensus_state_path: &Path, + client_id: &ClientId, + consensus_height: Height, expected_consensus_state: &AnyConsensusState, ) -> Result<(), Error> { - client_state.verify_height(proofs.height())?; - match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -237,9 +240,10 @@ impl ClientDef for AnyClient { client.verify_client_consensus_state( client_state, prefix, - proofs, + proof, root, - consensus_state_path, + client_id, + consensus_height, expected_consensus_state, ) } @@ -254,9 +258,10 @@ impl ClientDef for AnyClient { client.verify_client_consensus_state( client_state, prefix, - proofs, + proof, root, - consensus_state_path, + client_id, + consensus_height, expected_consensus_state, ) } @@ -267,13 +272,11 @@ impl ClientDef for AnyClient { &self, client_state: &AnyClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - connection_path: &Path, + connection_id: &ConnectionId, expected_connection_end: &ConnectionEnd, ) -> Result<(), Error> { - client_state.verify_height(proofs.height())?; - match self { Self::Tendermint(client) => { let client_state = downcast!(client_state => AnyClientState::Tendermint) @@ -282,9 +285,9 @@ impl ClientDef for AnyClient { client.verify_connection_state( client_state, prefix, - proofs, + proof, root, - connection_path, + connection_id, expected_connection_end, ) } @@ -297,9 +300,9 @@ impl ClientDef for AnyClient { client.verify_connection_state( client_state, prefix, - proofs, + proof, root, - connection_path, + connection_id, expected_connection_end, ) } @@ -310,13 +313,12 @@ impl ClientDef for AnyClient { &self, client_state: &AnyClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - channel_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, expected_channel_end: &ChannelEnd, ) -> Result<(), Error> { - client_state.verify_height(proofs.height())?; - match self { Self::Tendermint(client) => { let client_state = downcast!(client_state => AnyClientState::Tendermint) @@ -325,9 +327,10 @@ impl ClientDef for AnyClient { client.verify_channel_state( client_state, prefix, - proofs, + proof, root, - channel_path, + port_id, + channel_id, expected_channel_end, ) } @@ -340,9 +343,10 @@ impl ClientDef for AnyClient { client.verify_channel_state( client_state, prefix, - proofs, + proof, root, - channel_path, + port_id, + channel_id, expected_channel_end, ) } @@ -353,13 +357,11 @@ impl ClientDef for AnyClient { &self, client_state: &Self::ClientState, prefix: &CommitmentPrefix, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, - client_state_path: &Path, + client_id: &ClientId, client_state_on_counterparty: &AnyClientState, ) -> Result<(), Error> { - client_state.verify_height(proofs.height())?; - match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -370,9 +372,9 @@ impl ClientDef for AnyClient { client.verify_client_full_state( client_state, prefix, - proofs, + proof, root, - client_state_path, + client_id, client_state_on_counterparty, ) } @@ -387,9 +389,9 @@ impl ClientDef for AnyClient { client.verify_client_full_state( client_state, prefix, - proofs, + proof, root, - client_state_path, + client_id, client_state_on_counterparty, ) } @@ -400,13 +402,11 @@ impl ClientDef for AnyClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, commitment_path: &Path, commitment: String, ) -> Result<(), Error> { - client_state.verify_height(proofs.height())?; - match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -418,7 +418,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proofs, + proof, root, commitment_path, commitment, @@ -436,7 +436,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proofs, + proof, root, commitment_path, commitment, @@ -450,13 +450,11 @@ impl ClientDef for AnyClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, ack_path: &Path, ack: Vec, ) -> Result<(), Error> { - client_state.verify_height(proofs.height())?; - match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -468,7 +466,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proofs, + proof, root, ack_path, ack, @@ -486,7 +484,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proofs, + proof, root, ack_path, ack, @@ -500,13 +498,11 @@ impl ClientDef for AnyClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, seq_path: &Path, seq: Sequence, ) -> Result<(), Error> { - client_state.verify_height(proofs.height())?; - match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -518,7 +514,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proofs, + proof, root, seq_path, seq, @@ -536,7 +532,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proofs, + proof, root, seq_path, seq, @@ -549,12 +545,10 @@ impl ClientDef for AnyClient { ctx: &dyn ChannelReader, client_state: &Self::ClientState, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &CommitmentProofBytes, root: &CommitmentRoot, receipt_path: &Path, ) -> Result<(), Error> { - client_state.verify_height(proofs.height())?; - match self { Self::Tendermint(client) => { let client_state = downcast!( @@ -566,7 +560,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proofs, + proof, root, receipt_path, ) @@ -583,7 +577,7 @@ impl ClientDef for AnyClient { ctx, client_state, connection_end, - proofs, + proof, root, receipt_path, ) diff --git a/modules/src/core/ics02_client/client_state.rs b/modules/src/core/ics02_client/client_state.rs index 651261ade3..f53ac517bd 100644 --- a/modules/src/core/ics02_client/client_state.rs +++ b/modules/src/core/ics02_client/client_state.rs @@ -116,20 +116,6 @@ impl AnyClientState { AnyClientState::Mock(mock_state) => mock_state.expired(elapsed_since_latest), } } - - /// Verify that the client is at a sufficient height and unfrozen at the given height - pub fn verify_height(&self, height: Height) -> Result<(), Error> { - if self.latest_height() < height { - return Err(Error::insufficient_height(self.latest_height(), height)); - } - - match self.frozen_height() { - Some(frozen_height) if frozen_height <= height => { - Err(Error::frozen_height(frozen_height, height)) - } - _ => Ok(()), - } - } } impl Protobuf for AnyClientState {} diff --git a/modules/src/core/ics02_client/error.rs b/modules/src/core/ics02_client/error.rs index 945d7b8d91..d3da2bbf76 100644 --- a/modules/src/core/ics02_client/error.rs +++ b/modules/src/core/ics02_client/error.rs @@ -173,12 +173,6 @@ define_error! { [ Ics23Error ] | _ | { "invalid proof for the consensus state" }, - NullClientProof - | _ | { "client state proof must be present" }, - - NullConsensusProof - | _ | { "consensus state proof must be present" }, - Tendermint [ Ics07Error ] | _ | { "tendermint error" }, @@ -248,24 +242,6 @@ define_error! { format_args!("header not withing trusting period: expires_at={0} now={1}", e.latest_time, e.update_time) }, - InsufficientHeight - { - latest_height: Height, - target_height: Height, - } - | e | { - format_args!("the height is insufficient: latest_height={0} target_height={1}", e.latest_height, e.target_height) - }, - - FrozenHeight - { - frozen_height: Height, - target_height: Height, - } - | e | { - format_args!("the client is frozen at the height: frozen_height={0} target_height={1}", e.frozen_height, e.target_height) - }, - TendermintHandlerError [ Ics07Error ] | _ | { format_args!("Tendermint-specific handler error") }, diff --git a/modules/src/core/ics03_connection/error.rs b/modules/src/core/ics03_connection/error.rs index b0aed21f5c..2a0d687e05 100644 --- a/modules/src/core/ics03_connection/error.rs +++ b/modules/src/core/ics03_connection/error.rs @@ -113,6 +113,9 @@ define_error! { MissingCounterpartyPrefix | _ | { "missing counterparty prefix" }, + NullClientProof + | _ | { "client proof must be present" }, + FrozenClient { client_id: ClientId } | e | { diff --git a/modules/src/core/ics03_connection/handler/verify.rs b/modules/src/core/ics03_connection/handler/verify.rs index fe89882d6f..86ac2e4ec6 100644 --- a/modules/src/core/ics03_connection/handler/verify.rs +++ b/modules/src/core/ics03_connection/handler/verify.rs @@ -1,13 +1,13 @@ //! ICS3 verification functions, common across all four handlers of ICS3. use crate::core::ics02_client::client_consensus::ConsensusState; -use crate::core::ics02_client::client_state::AnyClientState; +use crate::core::ics02_client::client_state::{AnyClientState, ClientState}; use crate::core::ics02_client::{client_def::AnyClient, client_def::ClientDef}; use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics03_connection::context::ConnectionReader; use crate::core::ics03_connection::error::Error; -use crate::core::ics24_host::Path; -use crate::proofs::Proofs; +use crate::core::ics23_commitment::commitment::CommitmentProofBytes; +use crate::proofs::{ConsensusProof, Proofs}; use crate::Height; /// Entry point for verifying all proofs bundled in any ICS3 message. @@ -18,14 +18,34 @@ pub fn verify_proofs( expected_conn: &ConnectionEnd, proofs: &Proofs, ) -> Result<(), Error> { - verify_connection_proof(ctx, connection_end, expected_conn, proofs)?; + verify_connection_proof( + ctx, + connection_end, + expected_conn, + proofs.height(), + proofs.object_proof(), + )?; // If the message includes a client state, then verify the proof for that state. if let Some(expected_client_state) = client_state { - verify_client_proof(ctx, connection_end, expected_client_state, proofs)?; + verify_client_proof( + ctx, + connection_end, + expected_client_state, + proofs.height(), + proofs + .client_proof() + .as_ref() + .ok_or_else(Error::null_client_proof)?, + )?; } - verify_consensus_proof(ctx, connection_end, proofs) + // If a consensus proof is attached to the message, then verify it. + if let Some(proof) = proofs.consensus_proof() { + Ok(verify_consensus_proof(ctx, connection_end, &proof)?) + } else { + Ok(()) + } } /// Verifies the authenticity and semantic correctness of a commitment `proof`. The commitment @@ -35,14 +55,19 @@ pub fn verify_connection_proof( ctx: &dyn ConnectionReader, connection_end: &ConnectionEnd, expected_conn: &ConnectionEnd, - proofs: &Proofs, + proof_height: Height, + proof: &CommitmentProofBytes, ) -> Result<(), Error> { // Fetch the client state (IBC client on the local/host chain). let client_state = ctx.client_state(connection_end.client_id())?; + // The client must not be frozen. + if client_state.is_frozen() { + return Err(Error::frozen_client(connection_end.client_id().clone())); + } + // The client must have the consensus state for the height where this proof was created. - let consensus_state = - ctx.client_consensus_state(connection_end.client_id(), proofs.height())?; + let consensus_state = ctx.client_consensus_state(connection_end.client_id(), proof_height)?; // A counterparty connection id of None causes `unwrap()` below and indicates an internal // error as this is the connection id on the counterparty chain that must always be present. @@ -50,7 +75,6 @@ pub fn verify_connection_proof( .counterparty() .connection_id() .ok_or_else(Error::invalid_counterparty)?; - let path = Path::Connections(connection_id.clone()); let client_def = AnyClient::from_client_type(client_state.client_type()); @@ -59,9 +83,9 @@ pub fn verify_connection_proof( .verify_connection_state( &client_state, connection_end.counterparty().prefix(), - proofs, + proof, consensus_state.root(), - &path, + connection_id, expected_conn, ) .map_err(Error::verify_connection_state) @@ -78,15 +102,17 @@ pub fn verify_client_proof( ctx: &dyn ConnectionReader, connection_end: &ConnectionEnd, expected_client_state: AnyClientState, - proofs: &Proofs, + proof_height: Height, + proof: &CommitmentProofBytes, ) -> Result<(), Error> { - let client_id = connection_end.client_id(); - let path = Path::ClientState(client_id.clone()); - // Fetch the local client state (IBC client running on the host chain). - let client_state = ctx.client_state(client_id)?; + let client_state = ctx.client_state(connection_end.client_id())?; - let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + if client_state.is_frozen() { + return Err(Error::frozen_client(connection_end.client_id().clone())); + } + + let consensus_state = ctx.client_consensus_state(connection_end.client_id(), proof_height)?; let client_def = AnyClient::from_client_type(client_state.client_type()); @@ -94,9 +120,9 @@ pub fn verify_client_proof( .verify_client_full_state( &client_state, connection_end.counterparty().prefix(), - proofs, + proof, consensus_state.root(), - &path, + connection_end.counterparty().client_id(), &expected_client_state, ) .map_err(|e| { @@ -107,26 +133,17 @@ pub fn verify_client_proof( pub fn verify_consensus_proof( ctx: &dyn ConnectionReader, connection_end: &ConnectionEnd, - proofs: &Proofs, + proof: &ConsensusProof, ) -> Result<(), Error> { - // If a consensus proof is attached to the message, then verify it. - let consensus_height = match proofs.consensus_proof() { - Some(consensus_proof) => consensus_proof.height(), - None => return Ok(()), - }; - - let client_id = connection_end.client_id(); - let path = Path::ClientConsensusState { - client_id: client_id.clone(), - epoch: consensus_height.revision_number, - height: consensus_height.revision_height, - }; - // Fetch the client state (IBC client on the local chain). - let client_state = ctx.client_state(client_id)?; + let client_state = ctx.client_state(connection_end.client_id())?; + + if client_state.is_frozen() { + return Err(Error::frozen_client(connection_end.client_id().clone())); + } // Fetch the expected consensus state from the historical (local) header data. - let expected_consensus = ctx.host_consensus_state(consensus_height)?; + let expected_consensus = ctx.host_consensus_state(proof.height())?; let client = AnyClient::from_client_type(client_state.client_type()); @@ -134,12 +151,13 @@ pub fn verify_consensus_proof( .verify_client_consensus_state( &client_state, connection_end.counterparty().prefix(), - proofs, + proof.proof(), expected_consensus.root(), - &path, + connection_end.counterparty().client_id(), + proof.height(), &expected_consensus, ) - .map_err(|e| Error::consensus_state_verification_failure(consensus_height, e)) + .map_err(|e| Error::consensus_state_verification_failure(proof.height(), e)) } /// Checks that `claimed_height` is within normal bounds, i.e., fresh enough so that the chain has diff --git a/modules/src/core/ics04_channel/handler/verify.rs b/modules/src/core/ics04_channel/handler/verify.rs index 7f99ef76aa..b162fd7b36 100644 --- a/modules/src/core/ics04_channel/handler/verify.rs +++ b/modules/src/core/ics04_channel/handler/verify.rs @@ -18,15 +18,17 @@ pub fn verify_channel_proofs( expected_chan: &ChannelEnd, proofs: &Proofs, ) -> Result<(), Error> { - let client_id = connection_end.client_id(); - let path = Path::ChannelEnds( - channel_end.counterparty().port_id().clone(), - channel_end.counterparty().channel_id().unwrap().clone(), - ); + // This is the client which will perform proof verification. + let client_id = connection_end.client_id().clone(); - let client_state = ctx.client_state(client_id)?; + let client_state = ctx.client_state(&client_id)?; - let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + // The client must not be frozen. + if client_state.is_frozen() { + return Err(Error::frozen_client(client_id)); + } + + let consensus_state = ctx.client_consensus_state(&client_id, proofs.height())?; let client_def = AnyClient::from_client_type(client_state.client_type()); @@ -36,9 +38,10 @@ pub fn verify_channel_proofs( .verify_channel_state( &client_state, connection_end.counterparty().prefix(), - proofs, + proofs.object_proof(), consensus_state.root(), - &path, + channel_end.counterparty().port_id(), + channel_end.counterparty().channel_id().unwrap(), expected_chan, ) .map_err(Error::verify_channel_failed) @@ -80,7 +83,7 @@ pub fn verify_packet_recv_proofs( ctx, &client_state, connection_end, - proofs, + proofs.object_proof(), consensus_state.root(), &commitment_path, commitment, @@ -122,7 +125,7 @@ pub fn verify_packet_acknowledgement_proofs( ctx, &client_state, connection_end, - proofs, + proofs.object_proof(), consensus_state.root(), &ack_path, acknowledgement, @@ -160,7 +163,7 @@ pub fn verify_next_sequence_recv( ctx, &client_state, connection_end, - proofs, + proofs.object_proof(), consensus_state.root(), &seq_path, seq, @@ -200,7 +203,7 @@ pub fn verify_packet_receipt_absence( ctx, &client_state, connection_end, - proofs, + proofs.object_proof(), consensus_state.root(), &receipt_path, ) diff --git a/modules/src/mock/client_def.rs b/modules/src/mock/client_def.rs index 36f8f16e36..4eed21d454 100644 --- a/modules/src/mock/client_def.rs +++ b/modules/src/mock/client_def.rs @@ -9,14 +9,16 @@ use crate::core::ics03_connection::connection::ConnectionEnd; use crate::core::ics04_channel::channel::ChannelEnd; use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::packet::Sequence; -use crate::core::ics23_commitment::commitment::{CommitmentPrefix, CommitmentRoot}; +use crate::core::ics23_commitment::commitment::{ + CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, +}; use crate::core::ics23_commitment::merkle::apply_prefix; -use crate::core::ics24_host::identifier::ClientId; +use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; use crate::core::ics24_host::Path; use crate::mock::client_state::{MockClientState, MockConsensusState}; use crate::mock::header::MockHeader; use crate::prelude::*; -use crate::proofs::Proofs; +use crate::Height; #[derive(Clone, Debug, PartialEq, Eq)] pub struct MockClient; @@ -49,13 +51,21 @@ impl ClientDef for MockClient { &self, _client_state: &Self::ClientState, prefix: &CommitmentPrefix, - _proofs: &Proofs, + _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - consensus_state_path: &Path, + client_id: &ClientId, + consensus_height: Height, _expected_consensus_state: &AnyConsensusState, ) -> Result<(), Error> { - let _path = apply_prefix(prefix, vec![consensus_state_path.to_string()]) - .map_err(Error::empty_prefix)?; + let client_prefixed_path = Path::ClientConsensusState { + client_id: client_id.clone(), + epoch: consensus_height.revision_number, + height: consensus_height.revision_height, + } + .to_string(); + + let _path = + apply_prefix(prefix, vec![client_prefixed_path]).map_err(Error::empty_prefix)?; Ok(()) } @@ -64,9 +74,9 @@ impl ClientDef for MockClient { &self, _client_state: &Self::ClientState, _prefix: &CommitmentPrefix, - _proofs: &Proofs, + _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _connection_path: &Path, + _connection_id: &ConnectionId, _expected_connection_end: &ConnectionEnd, ) -> Result<(), Error> { Ok(()) @@ -76,9 +86,10 @@ impl ClientDef for MockClient { &self, _client_state: &Self::ClientState, _prefix: &CommitmentPrefix, - _proofs: &Proofs, + _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _channel_path: &Path, + _port_id: &PortId, + _channel_id: &ChannelId, _expected_channel_end: &ChannelEnd, ) -> Result<(), Error> { Ok(()) @@ -88,9 +99,9 @@ impl ClientDef for MockClient { &self, _client_state: &Self::ClientState, _prefix: &CommitmentPrefix, - _proofs: &Proofs, + _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _client_state_path: &Path, + _client_id: &ClientId, _expected_client_state: &AnyClientState, ) -> Result<(), Error> { Ok(()) @@ -101,7 +112,7 @@ impl ClientDef for MockClient { _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _connection_end: &ConnectionEnd, - _proofs: &Proofs, + _proof: &CommitmentProofBytes, _root: &CommitmentRoot, _commitment_path: &Path, _commitment: String, @@ -114,7 +125,7 @@ impl ClientDef for MockClient { _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _connection_end: &ConnectionEnd, - _proofs: &Proofs, + _proof: &CommitmentProofBytes, _root: &CommitmentRoot, _ack_path: &Path, _ack: Vec, @@ -127,7 +138,7 @@ impl ClientDef for MockClient { _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _connection_end: &ConnectionEnd, - _proofs: &Proofs, + _proof: &CommitmentProofBytes, _root: &CommitmentRoot, _seq_path: &Path, _seq: Sequence, @@ -140,7 +151,7 @@ impl ClientDef for MockClient { _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _connection_end: &ConnectionEnd, - _proofs: &Proofs, + _proof: &CommitmentProofBytes, _root: &CommitmentRoot, _receipt_path: &Path, ) -> Result<(), Error> { From 33e493c1924bbe8f9d05ca6aa66db0a5c8471f0a Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 10 Dec 2021 00:57:46 +0530 Subject: [PATCH 06/26] Impl verify_height() --- .../clients/ics07_tendermint/client_def.rs | 26 ++++++++++++--- .../clients/ics07_tendermint/client_state.rs | 14 ++++++++ modules/src/clients/ics07_tendermint/error.rs | 18 +++++++++++ modules/src/core/ics02_client/client_def.rs | 32 +++++++++++++++++++ .../ics03_connection/handler/conn_open_ack.rs | 1 + .../handler/conn_open_confirm.rs | 9 +++++- .../ics03_connection/handler/conn_open_try.rs | 1 + .../core/ics03_connection/handler/verify.rs | 11 ++++++- .../ics04_channel/handler/acknowledgement.rs | 1 + .../handler/chan_close_confirm.rs | 1 + .../ics04_channel/handler/chan_open_ack.rs | 1 + .../handler/chan_open_confirm.rs | 1 + .../ics04_channel/handler/chan_open_try.rs | 1 + .../core/ics04_channel/handler/recv_packet.rs | 8 ++++- .../src/core/ics04_channel/handler/timeout.rs | 9 +++++- .../ics04_channel/handler/timeout_on_close.rs | 10 +++++- .../src/core/ics04_channel/handler/verify.rs | 11 +++++++ .../core/ics04_channel/msgs/recv_packet.rs | 4 +++ .../src/core/ics04_channel/msgs/timeout.rs | 4 +++ .../ics04_channel/msgs/timeout_on_close.rs | 4 +++ 20 files changed, 157 insertions(+), 10 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index 2988ede510..ea11a6e913 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -191,6 +191,7 @@ impl ClientDef for TendermintClient { fn verify_client_consensus_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -198,6 +199,8 @@ impl ClientDef for TendermintClient { consensus_height: Height, expected_consensus_state: &AnyConsensusState, ) -> Result<(), Ics02Error> { + client_state.verify_height(height)?; + let path = Path::ClientConsensusState { client_id: client_id.clone(), epoch: consensus_height.revision_number, @@ -211,12 +214,15 @@ impl ClientDef for TendermintClient { fn verify_connection_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, connection_id: &ConnectionId, expected_connection_end: &ConnectionEnd, ) -> Result<(), Ics02Error> { + client_state.verify_height(height)?; + let path = Path::Connections(connection_id.clone()).to_string(); let value = expected_connection_end.encode_vec().unwrap(); verify_membership(client_state, prefix, proof, root, path, value) @@ -225,6 +231,7 @@ impl ClientDef for TendermintClient { fn verify_channel_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -232,6 +239,8 @@ impl ClientDef for TendermintClient { channel_id: &ChannelId, expected_channel_end: &ChannelEnd, ) -> Result<(), Ics02Error> { + client_state.verify_height(height)?; + let path = Path::ChannelEnds(port_id.clone(), channel_id.clone()).to_string(); let value = expected_channel_end.encode_vec().unwrap(); verify_membership(client_state, prefix, proof, root, path, value) @@ -240,12 +249,15 @@ impl ClientDef for TendermintClient { fn verify_client_full_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, client_id: &ClientId, expected_client_state: &AnyClientState, ) -> Result<(), Ics02Error> { + client_state.verify_height(height)?; + let path = Path::ClientState(client_id.clone()).to_string(); let value = expected_client_state.encode_vec().unwrap(); verify_membership(client_state, prefix, proof, root, path, value) @@ -255,12 +267,14 @@ impl ClientDef for TendermintClient { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, commitment_path: &Path, commitment: String, ) -> Result<(), Ics02Error> { + client_state.verify_height(height)?; verify_delay_passed(ctx, client_state, connection_end)?; verify_membership( @@ -277,12 +291,14 @@ impl ClientDef for TendermintClient { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, ack_path: &Path, ack: Vec, ) -> Result<(), Ics02Error> { + client_state.verify_height(height)?; verify_delay_passed(ctx, client_state, connection_end)?; verify_membership( @@ -299,12 +315,14 @@ impl ClientDef for TendermintClient { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, seq_path: &Path, seq: Sequence, ) -> Result<(), Ics02Error> { + client_state.verify_height(height)?; verify_delay_passed(ctx, client_state, connection_end)?; verify_membership( @@ -321,11 +339,13 @@ impl ClientDef for TendermintClient { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, receipt_path: &Path, ) -> Result<(), Ics02Error> { + client_state.verify_height(height)?; verify_delay_passed(ctx, client_state, connection_end)?; verify_non_membership( @@ -385,11 +405,7 @@ fn verify_non_membership( .into(); merkle_proof - .verify_non_membership( - &client_state.proof_specs, - root.clone().into(), - merkle_path, - ) + .verify_non_membership(&client_state.proof_specs, root.clone().into(), merkle_path) .map_err(|e| Ics02Error::tendermint(Error::ics23_error(e))) } diff --git a/modules/src/clients/ics07_tendermint/client_state.rs b/modules/src/clients/ics07_tendermint/client_state.rs index 584b2b4ba9..f7333b90b2 100644 --- a/modules/src/clients/ics07_tendermint/client_state.rs +++ b/modules/src/clients/ics07_tendermint/client_state.rs @@ -189,6 +189,20 @@ impl ClientState { Ok(()) } + + /// Verify that the client is at a sufficient height and unfrozen at the given height + pub fn verify_height(&self, height: Height) -> Result<(), Error> { + if self.latest_height < height { + return Err(Error::insufficient_height(self.latest_height(), height)); + } + + match self.frozen_height { + Some(frozen_height) if frozen_height <= height => { + Err(Error::client_frozen(frozen_height, height)) + } + _ => Ok(()), + } + } } impl crate::core::ics02_client::client_state::ClientState for ClientState { diff --git a/modules/src/clients/ics07_tendermint/error.rs b/modules/src/clients/ics07_tendermint/error.rs index 03dc85e738..11fff4c48f 100644 --- a/modules/src/clients/ics07_tendermint/error.rs +++ b/modules/src/clients/ics07_tendermint/error.rs @@ -232,6 +232,24 @@ define_error! { Ics23Error [ Ics23Error ] | _ | { "ics23 commitment error" }, + + InsufficientHeight + { + latest_height: Height, + target_height: Height, + } + | e | { + format_args!("the height is insufficient: latest_height={0} target_height={1}", e.latest_height, e.target_height) + }, + + ClientFrozen + { + frozen_height: Height, + target_height: Height, + } + | e | { + format_args!("the client is frozen: frozen_height={0} target_height={1}", e.frozen_height, e.target_height) + }, } } diff --git a/modules/src/core/ics02_client/client_def.rs b/modules/src/core/ics02_client/client_def.rs index 2c888c6b04..a7cf02d1fa 100644 --- a/modules/src/core/ics02_client/client_def.rs +++ b/modules/src/core/ics02_client/client_def.rs @@ -56,6 +56,7 @@ pub trait ClientDef: Clone { fn verify_client_consensus_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -68,6 +69,7 @@ pub trait ClientDef: Clone { fn verify_connection_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -80,6 +82,7 @@ pub trait ClientDef: Clone { fn verify_channel_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -93,6 +96,7 @@ pub trait ClientDef: Clone { fn verify_client_full_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -106,6 +110,7 @@ pub trait ClientDef: Clone { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -119,6 +124,7 @@ pub trait ClientDef: Clone { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -132,6 +138,7 @@ pub trait ClientDef: Clone { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -144,6 +151,7 @@ pub trait ClientDef: Clone { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -223,6 +231,7 @@ impl ClientDef for AnyClient { fn verify_client_consensus_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -239,6 +248,7 @@ impl ClientDef for AnyClient { client.verify_client_consensus_state( client_state, + height, prefix, proof, root, @@ -257,6 +267,7 @@ impl ClientDef for AnyClient { client.verify_client_consensus_state( client_state, + height, prefix, proof, root, @@ -271,6 +282,7 @@ impl ClientDef for AnyClient { fn verify_connection_state( &self, client_state: &AnyClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -284,6 +296,7 @@ impl ClientDef for AnyClient { client.verify_connection_state( client_state, + height, prefix, proof, root, @@ -299,6 +312,7 @@ impl ClientDef for AnyClient { client.verify_connection_state( client_state, + height, prefix, proof, root, @@ -312,6 +326,7 @@ impl ClientDef for AnyClient { fn verify_channel_state( &self, client_state: &AnyClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -326,6 +341,7 @@ impl ClientDef for AnyClient { client.verify_channel_state( client_state, + height, prefix, proof, root, @@ -342,6 +358,7 @@ impl ClientDef for AnyClient { client.verify_channel_state( client_state, + height, prefix, proof, root, @@ -356,6 +373,7 @@ impl ClientDef for AnyClient { fn verify_client_full_state( &self, client_state: &Self::ClientState, + height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -371,6 +389,7 @@ impl ClientDef for AnyClient { client.verify_client_full_state( client_state, + height, prefix, proof, root, @@ -388,6 +407,7 @@ impl ClientDef for AnyClient { client.verify_client_full_state( client_state, + height, prefix, proof, root, @@ -401,6 +421,7 @@ impl ClientDef for AnyClient { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -417,6 +438,7 @@ impl ClientDef for AnyClient { client.verify_packet_data( ctx, client_state, + height, connection_end, proof, root, @@ -435,6 +457,7 @@ impl ClientDef for AnyClient { client.verify_packet_data( ctx, client_state, + height, connection_end, proof, root, @@ -449,6 +472,7 @@ impl ClientDef for AnyClient { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -465,6 +489,7 @@ impl ClientDef for AnyClient { client.verify_packet_acknowledgement( ctx, client_state, + height, connection_end, proof, root, @@ -483,6 +508,7 @@ impl ClientDef for AnyClient { client.verify_packet_acknowledgement( ctx, client_state, + height, connection_end, proof, root, @@ -497,6 +523,7 @@ impl ClientDef for AnyClient { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -513,6 +540,7 @@ impl ClientDef for AnyClient { client.verify_next_sequence_recv( ctx, client_state, + height, connection_end, proof, root, @@ -531,6 +559,7 @@ impl ClientDef for AnyClient { client.verify_next_sequence_recv( ctx, client_state, + height, connection_end, proof, root, @@ -544,6 +573,7 @@ impl ClientDef for AnyClient { &self, ctx: &dyn ChannelReader, client_state: &Self::ClientState, + height: Height, connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, @@ -559,6 +589,7 @@ impl ClientDef for AnyClient { client.verify_packet_receipt_absence( ctx, client_state, + height, connection_end, proof, root, @@ -576,6 +607,7 @@ impl ClientDef for AnyClient { client.verify_packet_receipt_absence( ctx, client_state, + height, connection_end, proof, root, diff --git a/modules/src/core/ics03_connection/handler/conn_open_ack.rs b/modules/src/core/ics03_connection/handler/conn_open_ack.rs index 4d4f60a45f..69038d348e 100644 --- a/modules/src/core/ics03_connection/handler/conn_open_ack.rs +++ b/modules/src/core/ics03_connection/handler/conn_open_ack.rs @@ -66,6 +66,7 @@ pub(crate) fn process( verify_proofs( ctx, msg.client_state(), + msg.proofs().height(), &conn_end, &expected_conn, msg.proofs(), diff --git a/modules/src/core/ics03_connection/handler/conn_open_confirm.rs b/modules/src/core/ics03_connection/handler/conn_open_confirm.rs index c806126302..df8af596fd 100644 --- a/modules/src/core/ics03_connection/handler/conn_open_confirm.rs +++ b/modules/src/core/ics03_connection/handler/conn_open_confirm.rs @@ -40,7 +40,14 @@ pub(crate) fn process( ); // 2. Pass the details to the verification function. - verify_proofs(ctx, None, &conn_end, &expected_conn, msg.proofs())?; + verify_proofs( + ctx, + None, + msg.proofs().height(), + &conn_end, + &expected_conn, + msg.proofs(), + )?; output.log("success: connection verification passed"); diff --git a/modules/src/core/ics03_connection/handler/conn_open_try.rs b/modules/src/core/ics03_connection/handler/conn_open_try.rs index 6e1647e4e8..1042fd9c20 100644 --- a/modules/src/core/ics03_connection/handler/conn_open_try.rs +++ b/modules/src/core/ics03_connection/handler/conn_open_try.rs @@ -81,6 +81,7 @@ pub(crate) fn process( verify_proofs( ctx, msg.client_state(), + msg.proofs().height(), &new_connection_end, &expected_conn, msg.proofs(), diff --git a/modules/src/core/ics03_connection/handler/verify.rs b/modules/src/core/ics03_connection/handler/verify.rs index 86ac2e4ec6..917fb5ac6d 100644 --- a/modules/src/core/ics03_connection/handler/verify.rs +++ b/modules/src/core/ics03_connection/handler/verify.rs @@ -14,12 +14,14 @@ use crate::Height; pub fn verify_proofs( ctx: &dyn ConnectionReader, client_state: Option, + height: Height, connection_end: &ConnectionEnd, expected_conn: &ConnectionEnd, proofs: &Proofs, ) -> Result<(), Error> { verify_connection_proof( ctx, + height, connection_end, expected_conn, proofs.height(), @@ -30,6 +32,7 @@ pub fn verify_proofs( if let Some(expected_client_state) = client_state { verify_client_proof( ctx, + height, connection_end, expected_client_state, proofs.height(), @@ -42,7 +45,7 @@ pub fn verify_proofs( // If a consensus proof is attached to the message, then verify it. if let Some(proof) = proofs.consensus_proof() { - Ok(verify_consensus_proof(ctx, connection_end, &proof)?) + Ok(verify_consensus_proof(ctx, height, connection_end, &proof)?) } else { Ok(()) } @@ -53,6 +56,7 @@ pub fn verify_proofs( /// which created this proof). This object must match the state of `expected_conn`. pub fn verify_connection_proof( ctx: &dyn ConnectionReader, + height: Height, connection_end: &ConnectionEnd, expected_conn: &ConnectionEnd, proof_height: Height, @@ -82,6 +86,7 @@ pub fn verify_connection_proof( client_def .verify_connection_state( &client_state, + height, connection_end.counterparty().prefix(), proof, consensus_state.root(), @@ -100,6 +105,7 @@ pub fn verify_connection_proof( /// `proof` is correct. pub fn verify_client_proof( ctx: &dyn ConnectionReader, + height: Height, connection_end: &ConnectionEnd, expected_client_state: AnyClientState, proof_height: Height, @@ -119,6 +125,7 @@ pub fn verify_client_proof( client_def .verify_client_full_state( &client_state, + height, connection_end.counterparty().prefix(), proof, consensus_state.root(), @@ -132,6 +139,7 @@ pub fn verify_client_proof( pub fn verify_consensus_proof( ctx: &dyn ConnectionReader, + height: Height, connection_end: &ConnectionEnd, proof: &ConsensusProof, ) -> Result<(), Error> { @@ -150,6 +158,7 @@ pub fn verify_consensus_proof( client .verify_client_consensus_state( &client_state, + height, connection_end.counterparty().prefix(), proof.proof(), expected_consensus.root(), diff --git a/modules/src/core/ics04_channel/handler/acknowledgement.rs b/modules/src/core/ics04_channel/handler/acknowledgement.rs index 6907c10010..d602668fdc 100644 --- a/modules/src/core/ics04_channel/handler/acknowledgement.rs +++ b/modules/src/core/ics04_channel/handler/acknowledgement.rs @@ -76,6 +76,7 @@ pub fn process( // Verify the acknowledgement proof verify_packet_acknowledgement_proofs( ctx, + msg.proofs.height(), packet, msg.acknowledgement().clone(), &connection_end, diff --git a/modules/src/core/ics04_channel/handler/chan_close_confirm.rs b/modules/src/core/ics04_channel/handler/chan_close_confirm.rs index 661d4bd9e1..8984e4cb30 100644 --- a/modules/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/modules/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -67,6 +67,7 @@ pub(crate) fn process( verify_channel_proofs( ctx, + msg.proofs().height(), &channel_end, &conn, &expected_channel_end, diff --git a/modules/src/core/ics04_channel/handler/chan_open_ack.rs b/modules/src/core/ics04_channel/handler/chan_open_ack.rs index 5311033c74..8efe3c809b 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_ack.rs @@ -71,6 +71,7 @@ pub(crate) fn process( //2. Verify proofs verify_channel_proofs( ctx, + msg.proofs().height(), &channel_end, &conn, &expected_channel_end, diff --git a/modules/src/core/ics04_channel/handler/chan_open_confirm.rs b/modules/src/core/ics04_channel/handler/chan_open_confirm.rs index 1c798d5afb..62d82a6708 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -70,6 +70,7 @@ pub(crate) fn process( //2. Verify proofs verify_channel_proofs( ctx, + msg.proofs().height(), &channel_end, &conn, &expected_channel_end, diff --git a/modules/src/core/ics04_channel/handler/chan_open_try.rs b/modules/src/core/ics04_channel/handler/chan_open_try.rs index f75e994b64..c70dfcf659 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_try.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_try.rs @@ -117,6 +117,7 @@ pub(crate) fn process( // 2. Actual proofs are verified now. verify_channel_proofs( ctx, + msg.proofs().height(), &new_channel_end, &conn, &expected_channel_end, diff --git a/modules/src/core/ics04_channel/handler/recv_packet.rs b/modules/src/core/ics04_channel/handler/recv_packet.rs index e698b43841..f097c4ef48 100644 --- a/modules/src/core/ics04_channel/handler/recv_packet.rs +++ b/modules/src/core/ics04_channel/handler/recv_packet.rs @@ -75,7 +75,13 @@ pub fn process(ctx: &dyn ChannelReader, msg: MsgRecvPacket) -> HandlerResult HandlerResult HandlerResult, connection_end: &ConnectionEnd, @@ -124,6 +130,7 @@ pub fn verify_packet_acknowledgement_proofs( .verify_packet_acknowledgement( ctx, &client_state, + height, connection_end, proofs.object_proof(), consensus_state.root(), @@ -138,6 +145,7 @@ pub fn verify_packet_acknowledgement_proofs( /// Entry point for verifying all timeout proofs. pub fn verify_next_sequence_recv( ctx: &dyn ChannelReader, + height: Height, connection_end: &ConnectionEnd, packet: Packet, seq: Sequence, @@ -162,6 +170,7 @@ pub fn verify_next_sequence_recv( .verify_next_sequence_recv( ctx, &client_state, + height, connection_end, proofs.object_proof(), consensus_state.root(), @@ -175,6 +184,7 @@ pub fn verify_next_sequence_recv( pub fn verify_packet_receipt_absence( ctx: &dyn ChannelReader, + height: Height, connection_end: &ConnectionEnd, packet: Packet, proofs: &Proofs, @@ -202,6 +212,7 @@ pub fn verify_packet_receipt_absence( .verify_packet_receipt_absence( ctx, &client_state, + height, connection_end, proofs.object_proof(), consensus_state.root(), diff --git a/modules/src/core/ics04_channel/msgs/recv_packet.rs b/modules/src/core/ics04_channel/msgs/recv_packet.rs index 02a80d9b70..2624314025 100644 --- a/modules/src/core/ics04_channel/msgs/recv_packet.rs +++ b/modules/src/core/ics04_channel/msgs/recv_packet.rs @@ -30,6 +30,10 @@ impl MsgRecvPacket { signer, } } + + pub fn proofs(&self) -> &Proofs { + &self.proofs + } } impl Msg for MsgRecvPacket { diff --git a/modules/src/core/ics04_channel/msgs/timeout.rs b/modules/src/core/ics04_channel/msgs/timeout.rs index 8feee057bb..4d9876fa38 100644 --- a/modules/src/core/ics04_channel/msgs/timeout.rs +++ b/modules/src/core/ics04_channel/msgs/timeout.rs @@ -37,6 +37,10 @@ impl MsgTimeout { signer, } } + + pub fn proofs(&self) -> &Proofs { + &self.proofs + } } impl Msg for MsgTimeout { diff --git a/modules/src/core/ics04_channel/msgs/timeout_on_close.rs b/modules/src/core/ics04_channel/msgs/timeout_on_close.rs index 2179ee2cd5..390a89893e 100644 --- a/modules/src/core/ics04_channel/msgs/timeout_on_close.rs +++ b/modules/src/core/ics04_channel/msgs/timeout_on_close.rs @@ -36,6 +36,10 @@ impl MsgTimeoutOnClose { signer, } } + + pub fn proofs(&self) -> &Proofs { + &self.proofs + } } impl Msg for MsgTimeoutOnClose { From be733a341ae9e704757c6a1d3797b5bb3741ee19 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Sat, 11 Dec 2021 00:28:43 +0530 Subject: [PATCH 07/26] Revert changes to ics23 types --- proto/src/prost/ics23.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/proto/src/prost/ics23.rs b/proto/src/prost/ics23.rs index 877774b2c1..92340fe10d 100644 --- a/proto/src/prost/ics23.rs +++ b/proto/src/prost/ics23.rs @@ -18,7 +18,7 @@ ///With LengthOp this is tricker but not impossible. Which is why the "leafPrefixEqual" field ///in the ProofSpec is valuable to prevent this mutability. And why all trees should ///length-prefix the data before hashing it. -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct ExistenceProof { #[prost(bytes = "vec", tag = "1")] pub key: ::prost::alloc::vec::Vec, @@ -33,7 +33,7 @@ pub struct ExistenceProof { ///NonExistenceProof takes a proof of two neighbors, one left of the desired key, ///one right of the desired key. If both proofs are valid AND they are neighbors, ///then there is no valid proof for the given key. -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct NonExistenceProof { /// TODO: remove this as unnecessary??? we prove a range #[prost(bytes = "vec", tag = "1")] @@ -45,14 +45,14 @@ pub struct NonExistenceProof { } /// ///CommitmentProof is either an ExistenceProof or a NonExistenceProof, or a Batch of such messages -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct CommitmentProof { #[prost(oneof = "commitment_proof::Proof", tags = "1, 2, 3, 4")] pub proof: ::core::option::Option, } /// Nested message and enum types in `CommitmentProof`. pub mod commitment_proof { - #[derive(Clone, PartialEq, Eq, ::prost::Oneof)] + #[derive(Clone, PartialEq, ::prost::Oneof)] pub enum Proof { #[prost(message, tag = "1")] Exist(super::ExistenceProof), @@ -176,20 +176,20 @@ pub struct InnerSpec { } /// ///BatchProof is a group of multiple proof types than can be compressed -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct BatchProof { #[prost(message, repeated, tag = "1")] pub entries: ::prost::alloc::vec::Vec, } /// Use BatchEntry not CommitmentProof, to avoid recursion -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct BatchEntry { #[prost(oneof = "batch_entry::Proof", tags = "1, 2")] pub proof: ::core::option::Option, } /// Nested message and enum types in `BatchEntry`. pub mod batch_entry { - #[derive(Clone, PartialEq, Eq, ::prost::Oneof)] + #[derive(Clone, PartialEq, ::prost::Oneof)] pub enum Proof { #[prost(message, tag = "1")] Exist(super::ExistenceProof), @@ -199,7 +199,7 @@ pub mod batch_entry { } //***** all items here are compressed forms ****** -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct CompressedBatchProof { #[prost(message, repeated, tag = "1")] pub entries: ::prost::alloc::vec::Vec, @@ -207,14 +207,14 @@ pub struct CompressedBatchProof { pub lookup_inners: ::prost::alloc::vec::Vec, } /// Use BatchEntry not CommitmentProof, to avoid recursion -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct CompressedBatchEntry { #[prost(oneof = "compressed_batch_entry::Proof", tags = "1, 2")] pub proof: ::core::option::Option, } /// Nested message and enum types in `CompressedBatchEntry`. pub mod compressed_batch_entry { - #[derive(Clone, PartialEq, Eq, ::prost::Oneof)] + #[derive(Clone, PartialEq, ::prost::Oneof)] pub enum Proof { #[prost(message, tag = "1")] Exist(super::CompressedExistenceProof), @@ -222,7 +222,7 @@ pub mod compressed_batch_entry { Nonexist(super::CompressedNonExistenceProof), } } -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct CompressedExistenceProof { #[prost(bytes = "vec", tag = "1")] pub key: ::prost::alloc::vec::Vec, @@ -234,7 +234,7 @@ pub struct CompressedExistenceProof { #[prost(int32, repeated, tag = "4")] pub path: ::prost::alloc::vec::Vec, } -#[derive(Clone, PartialEq, Eq, ::prost::Message)] +#[derive(Clone, PartialEq, ::prost::Message)] pub struct CompressedNonExistenceProof { /// TODO: remove this as unnecessary??? we prove a range #[prost(bytes = "vec", tag = "1")] From 50aa14ab7a01aec46a1ea618ab8227d48e99d3d4 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Sat, 11 Dec 2021 00:39:29 +0530 Subject: [PATCH 08/26] Update mock impl to use new ClientDef API with height --- modules/src/mock/client_def.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/modules/src/mock/client_def.rs b/modules/src/mock/client_def.rs index 4eed21d454..b45763b024 100644 --- a/modules/src/mock/client_def.rs +++ b/modules/src/mock/client_def.rs @@ -50,6 +50,7 @@ impl ClientDef for MockClient { fn verify_client_consensus_state( &self, _client_state: &Self::ClientState, + _height: Height, prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, @@ -73,6 +74,7 @@ impl ClientDef for MockClient { fn verify_connection_state( &self, _client_state: &Self::ClientState, + _height: Height, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, @@ -85,6 +87,7 @@ impl ClientDef for MockClient { fn verify_channel_state( &self, _client_state: &Self::ClientState, + _height: Height, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, @@ -98,6 +101,7 @@ impl ClientDef for MockClient { fn verify_client_full_state( &self, _client_state: &Self::ClientState, + _height: Height, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, @@ -111,6 +115,7 @@ impl ClientDef for MockClient { &self, _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, + _height: Height, _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, @@ -124,6 +129,7 @@ impl ClientDef for MockClient { &self, _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, + _height: Height, _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, @@ -137,6 +143,7 @@ impl ClientDef for MockClient { &self, _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, + _height: Height, _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, @@ -150,6 +157,7 @@ impl ClientDef for MockClient { &self, _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, + _height: Height, _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, From 3ec08f85be1a793b1a9c913d0cf4db9853b1fa29 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Sat, 11 Dec 2021 00:39:50 +0530 Subject: [PATCH 09/26] Clippy happy --- modules/src/core/ics02_client/client_def.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/modules/src/core/ics02_client/client_def.rs b/modules/src/core/ics02_client/client_def.rs index a7cf02d1fa..6015810470 100644 --- a/modules/src/core/ics02_client/client_def.rs +++ b/modules/src/core/ics02_client/client_def.rs @@ -66,6 +66,7 @@ pub trait ClientDef: Clone { ) -> Result<(), Error>; /// Verify a `proof` that a connection state matches that of the input `connection_end`. + #[allow(clippy::too_many_arguments)] fn verify_connection_state( &self, client_state: &Self::ClientState, @@ -147,6 +148,7 @@ pub trait ClientDef: Clone { ) -> Result<(), Error>; /// Verify a `proof` that a packet has not been received. + #[allow(clippy::too_many_arguments)] fn verify_packet_receipt_absence( &self, ctx: &dyn ChannelReader, From 16189f227a689e4ef47b46b47ed661006225c772 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Sat, 11 Dec 2021 23:43:06 +0530 Subject: [PATCH 10/26] Modify ClientDef API --- .../clients/ics07_tendermint/client_def.rs | 35 ++++++++-- modules/src/core/ics02_client/client_def.rs | 69 +++++++++++++------ .../src/core/ics04_channel/handler/verify.rs | 37 ++++------ modules/src/mock/client_def.rs | 13 ++-- 4 files changed, 98 insertions(+), 56 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index ea11a6e913..dcda6664d5 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -271,12 +271,19 @@ impl ClientDef for TendermintClient { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - commitment_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, commitment: String, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; verify_delay_passed(ctx, client_state, connection_end)?; + let commitment_path = Path::Commitments { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + }; verify_membership( client_state, connection_end.counterparty().prefix(), @@ -295,12 +302,19 @@ impl ClientDef for TendermintClient { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - ack_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ack: Vec, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; verify_delay_passed(ctx, client_state, connection_end)?; + let ack_path = Path::Acks { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + }; verify_membership( client_state, connection_end.counterparty().prefix(), @@ -319,19 +333,21 @@ impl ClientDef for TendermintClient { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - seq_path: &Path, - seq: Sequence, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; verify_delay_passed(ctx, client_state, connection_end)?; + let seq_path = Path::SeqRecvs(port_id.clone(), channel_id.clone()); verify_membership( client_state, connection_end.counterparty().prefix(), proof, root, seq_path.to_string(), - u64::from(seq).encode_to_vec(), + u64::from(sequence).encode_to_vec(), ) } @@ -343,11 +359,18 @@ impl ClientDef for TendermintClient { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - receipt_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; verify_delay_passed(ctx, client_state, connection_end)?; + let receipt_path = Path::Receipts { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + }; verify_non_membership( client_state, connection_end.counterparty().prefix(), diff --git a/modules/src/core/ics02_client/client_def.rs b/modules/src/core/ics02_client/client_def.rs index 6015810470..d1a4311408 100644 --- a/modules/src/core/ics02_client/client_def.rs +++ b/modules/src/core/ics02_client/client_def.rs @@ -15,7 +15,6 @@ use crate::core::ics23_commitment::commitment::{ CommitmentPrefix, CommitmentProofBytes, CommitmentRoot, }; use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; -use crate::core::ics24_host::Path; use crate::downcast; use crate::prelude::*; use crate::Height; @@ -115,7 +114,9 @@ pub trait ClientDef: Clone { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - commitment_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, commitment: String, ) -> Result<(), Error>; @@ -129,7 +130,9 @@ pub trait ClientDef: Clone { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - ack_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ack: Vec, ) -> Result<(), Error>; @@ -143,8 +146,9 @@ pub trait ClientDef: Clone { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - seq_path: &Path, - seq: Sequence, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Error>; /// Verify a `proof` that a packet has not been received. @@ -157,7 +161,9 @@ pub trait ClientDef: Clone { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - receipt_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Error>; } @@ -427,7 +433,9 @@ impl ClientDef for AnyClient { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - commitment_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, commitment: String, ) -> Result<(), Error> { match self { @@ -444,7 +452,9 @@ impl ClientDef for AnyClient { connection_end, proof, root, - commitment_path, + port_id, + channel_id, + sequence, commitment, ) } @@ -463,7 +473,9 @@ impl ClientDef for AnyClient { connection_end, proof, root, - commitment_path, + port_id, + channel_id, + sequence, commitment, ) } @@ -478,7 +490,9 @@ impl ClientDef for AnyClient { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - ack_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ack: Vec, ) -> Result<(), Error> { match self { @@ -495,7 +509,9 @@ impl ClientDef for AnyClient { connection_end, proof, root, - ack_path, + port_id, + channel_id, + sequence, ack, ) } @@ -514,7 +530,9 @@ impl ClientDef for AnyClient { connection_end, proof, root, - ack_path, + port_id, + channel_id, + sequence, ack, ) } @@ -529,8 +547,9 @@ impl ClientDef for AnyClient { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - seq_path: &Path, - seq: Sequence, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Error> { match self { Self::Tendermint(client) => { @@ -546,8 +565,9 @@ impl ClientDef for AnyClient { connection_end, proof, root, - seq_path, - seq, + port_id, + channel_id, + sequence, ) } @@ -565,8 +585,9 @@ impl ClientDef for AnyClient { connection_end, proof, root, - seq_path, - seq, + port_id, + channel_id, + sequence, ) } } @@ -579,7 +600,9 @@ impl ClientDef for AnyClient { connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, root: &CommitmentRoot, - receipt_path: &Path, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Error> { match self { Self::Tendermint(client) => { @@ -595,7 +618,9 @@ impl ClientDef for AnyClient { connection_end, proof, root, - receipt_path, + port_id, + channel_id, + sequence, ) } @@ -613,7 +638,9 @@ impl ClientDef for AnyClient { connection_end, proof, root, - receipt_path, + port_id, + channel_id, + sequence, ) } } diff --git a/modules/src/core/ics04_channel/handler/verify.rs b/modules/src/core/ics04_channel/handler/verify.rs index 745b6eff09..34a72b8f3a 100644 --- a/modules/src/core/ics04_channel/handler/verify.rs +++ b/modules/src/core/ics04_channel/handler/verify.rs @@ -6,7 +6,6 @@ use crate::core::ics04_channel::channel::ChannelEnd; use crate::core::ics04_channel::context::ChannelReader; use crate::core::ics04_channel::error::Error; use crate::core::ics04_channel::packet::{Packet, Sequence}; -use crate::core::ics24_host::Path; use crate::prelude::*; use crate::proofs::Proofs; use crate::Height; @@ -70,11 +69,6 @@ pub fn verify_packet_recv_proofs( let client_def = AnyClient::from_client_type(client_state.client_type()); - let commitment_path = Path::Commitments { - port_id: packet.source_port.clone(), - channel_id: packet.source_channel.clone(), - sequence: packet.sequence, - }; let input = format!( "{:?},{:?},{:?}", packet.timeout_timestamp, packet.timeout_height, packet.data @@ -90,7 +84,9 @@ pub fn verify_packet_recv_proofs( connection_end, proofs.object_proof(), consensus_state.root(), - &commitment_path, + &packet.source_port, + &packet.source_channel, + packet.sequence, commitment, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; @@ -119,12 +115,6 @@ pub fn verify_packet_acknowledgement_proofs( let client_def = AnyClient::from_client_type(client_state.client_type()); - let ack_path = Path::Acks { - port_id: packet.source_port.clone(), - channel_id: packet.source_channel.clone(), - sequence: packet.sequence, - }; - // Verify the proof for the packet against the chain store. client_def .verify_packet_acknowledgement( @@ -134,7 +124,9 @@ pub fn verify_packet_acknowledgement_proofs( connection_end, proofs.object_proof(), consensus_state.root(), - &ack_path, + &packet.destination_port, + &packet.destination_channel, + packet.sequence, acknowledgement, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; @@ -163,8 +155,6 @@ pub fn verify_next_sequence_recv( let client_def = AnyClient::from_client_type(client_state.client_type()); - let seq_path = Path::SeqRecvs(packet.destination_port.clone(), packet.destination_channel); - // Verify the proof for the packet against the chain store. client_def .verify_next_sequence_recv( @@ -174,8 +164,9 @@ pub fn verify_next_sequence_recv( connection_end, proofs.object_proof(), consensus_state.root(), - &seq_path, - seq, + &packet.destination_port, + &packet.destination_channel, + packet.sequence, ) .map_err(|e| Error::packet_verification_failed(seq, e))?; @@ -201,12 +192,6 @@ pub fn verify_packet_receipt_absence( let client_def = AnyClient::from_client_type(client_state.client_type()); - let receipt_path = Path::Receipts { - port_id: packet.destination_port.clone(), - channel_id: packet.destination_channel.clone(), - sequence: packet.sequence, - }; - // Verify the proof for the packet against the chain store. client_def .verify_packet_receipt_absence( @@ -216,7 +201,9 @@ pub fn verify_packet_receipt_absence( connection_end, proofs.object_proof(), consensus_state.root(), - &receipt_path, + &packet.destination_port, + &packet.destination_channel, + packet.sequence, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; diff --git a/modules/src/mock/client_def.rs b/modules/src/mock/client_def.rs index b45763b024..916ed7e3c5 100644 --- a/modules/src/mock/client_def.rs +++ b/modules/src/mock/client_def.rs @@ -119,7 +119,9 @@ impl ClientDef for MockClient { _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _commitment_path: &Path, + _port_id: &PortId, + _channel_id: &ChannelId, + _sequence: Sequence, _commitment: String, ) -> Result<(), Error> { Ok(()) @@ -133,7 +135,9 @@ impl ClientDef for MockClient { _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _ack_path: &Path, + _port_id: &PortId, + _channel_id: &ChannelId, + _sequence: Sequence, _ack: Vec, ) -> Result<(), Error> { Ok(()) @@ -147,8 +151,9 @@ impl ClientDef for MockClient { _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _seq_path: &Path, - _seq: Sequence, + _port_id: &PortId, + _channel_id: &ChannelId, + _sequence: Sequence, ) -> Result<(), Error> { Ok(()) } From a5483ea9f40f06a8e86c5bc101b41c3ba93141fa Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Sun, 12 Dec 2021 14:19:10 +0530 Subject: [PATCH 11/26] Implement max_expected_time_per_block() --- modules/src/clients/ics07_tendermint/client_def.rs | 14 +++++++++++--- modules/src/core/ics04_channel/context.rs | 4 ++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index dcda6664d5..d75e9fdd35 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -1,3 +1,4 @@ +use core::time::Duration; use std::convert::TryInto; use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof; @@ -449,9 +450,7 @@ fn verify_delay_passed( .map_err(|_| Error::processed_height_not_found(client_id.clone()))?; let delay_period_time = connection_end.delay_period(); - // TODO get delay_period_height from delay_period_time - //let delay_period_height = get_block_delay(delay_period_time); - let delay_period_height = 0; + let delay_period_height = get_block_delay(ctx, delay_period_time); client_state .verify_delay_passed( @@ -465,6 +464,15 @@ fn verify_delay_passed( .map_err(|e| e.into()) } +fn get_block_delay(ctx: &dyn ChannelReader, delay_period_time: Duration) -> u64 { + let expected_time_per_block = ctx.max_expected_time_per_block(); + if expected_time_per_block.is_zero() { + return 0; + } + + (delay_period_time.as_secs_f64() / expected_time_per_block.as_secs_f64()).ceil() as u64 +} + fn downcast_consensus_state(cs: AnyConsensusState) -> Result { downcast!( cs => AnyConsensusState::Tendermint diff --git a/modules/src/core/ics04_channel/context.rs b/modules/src/core/ics04_channel/context.rs index 4aa6b81ed9..bbb0181853 100644 --- a/modules/src/core/ics04_channel/context.rs +++ b/modules/src/core/ics04_channel/context.rs @@ -1,6 +1,7 @@ //! ICS4 (channel) context. The two traits `ChannelReader ` and `ChannelKeeper` define //! the interface that any host chain must implement to be able to process any `ChannelMsg`. //! +use core::time::Duration; use crate::core::ics02_client::client_consensus::AnyConsensusState; use crate::core::ics02_client::client_state::AnyClientState; @@ -81,6 +82,9 @@ pub trait ChannelReader { /// The value of this counter should increase only via method /// `ChannelKeeper::increase_channel_counter`. fn channel_counter(&self) -> Result; + + /// Returns the maximum expected time per block + fn max_expected_time_per_block(&self) -> Duration; } /// A context supplying all the necessary write-only dependencies (i.e., storage writing facility) From 41b80ddfa0201a0fbe3da2b34ef9e75ba2a17ef1 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Sun, 12 Dec 2021 15:20:44 +0530 Subject: [PATCH 12/26] Fix mock build --- modules/src/mock/client_def.rs | 4 +++- modules/src/mock/context.rs | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/src/mock/client_def.rs b/modules/src/mock/client_def.rs index 916ed7e3c5..c5a755e478 100644 --- a/modules/src/mock/client_def.rs +++ b/modules/src/mock/client_def.rs @@ -166,7 +166,9 @@ impl ClientDef for MockClient { _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, _root: &CommitmentRoot, - _receipt_path: &Path, + _port_id: &PortId, + _channel_id: &ChannelId, + _sequence: Sequence, ) -> Result<(), Error> { Ok(()) } diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index 26b0349dcd..44540bb9f9 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -42,6 +42,7 @@ use crate::relayer::ics18_relayer::error::Error as Ics18Error; use crate::signer::Signer; use crate::timestamp::Timestamp; use crate::Height; +use core::time::Duration; /// A context implementing the dependencies necessary for testing any IBC module. #[derive(Clone, Debug)] @@ -690,6 +691,10 @@ impl ChannelReader for MockContext { fn channel_counter(&self) -> Result { Ok(self.channel_ids_counter) } + + fn max_expected_time_per_block(&self) -> Duration { + Duration::from_secs(10) + } } impl ChannelKeeper for MockContext { From 34c029e50c8e3df2069b2600dd02dd552f2d0fe7 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 13 Dec 2021 13:16:26 +0530 Subject: [PATCH 13/26] Refactor verify_delay_passed() --- .../clients/ics07_tendermint/client_def.rs | 28 +++++++++---------- .../clients/ics07_tendermint/client_state.rs | 1 - 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index d75e9fdd35..f775a993ea 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -278,7 +278,7 @@ impl ClientDef for TendermintClient { commitment: String, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - verify_delay_passed(ctx, client_state, connection_end)?; + verify_delay_passed(ctx, connection_end)?; let commitment_path = Path::Commitments { port_id: port_id.clone(), @@ -309,7 +309,7 @@ impl ClientDef for TendermintClient { ack: Vec, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - verify_delay_passed(ctx, client_state, connection_end)?; + verify_delay_passed(ctx, connection_end)?; let ack_path = Path::Acks { port_id: port_id.clone(), @@ -339,7 +339,7 @@ impl ClientDef for TendermintClient { sequence: Sequence, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - verify_delay_passed(ctx, client_state, connection_end)?; + verify_delay_passed(ctx, connection_end)?; let seq_path = Path::SeqRecvs(port_id.clone(), channel_id.clone()); verify_membership( @@ -365,7 +365,7 @@ impl ClientDef for TendermintClient { sequence: Sequence, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - verify_delay_passed(ctx, client_state, connection_end)?; + verify_delay_passed(ctx, connection_end)?; let receipt_path = Path::Receipts { port_id: port_id.clone(), @@ -435,7 +435,6 @@ fn verify_non_membership( fn verify_delay_passed( ctx: &dyn ChannelReader, - client_state: &ClientState, connection_end: &ConnectionEnd, ) -> Result<(), Ics02Error> { let current_timestamp = ctx.host_timestamp(); @@ -452,16 +451,15 @@ fn verify_delay_passed( let delay_period_time = connection_end.delay_period(); let delay_period_height = get_block_delay(ctx, delay_period_time); - client_state - .verify_delay_passed( - current_timestamp, - current_height, - processed_time, - processed_height, - delay_period_time, - delay_period_height, - ) - .map_err(|e| e.into()) + ClientState::verify_delay_passed( + current_timestamp, + current_height, + processed_time, + processed_height, + delay_period_time, + delay_period_height, + ) + .map_err(|e| e.into()) } fn get_block_delay(ctx: &dyn ChannelReader, delay_period_time: Duration) -> u64 { diff --git a/modules/src/clients/ics07_tendermint/client_state.rs b/modules/src/clients/ics07_tendermint/client_state.rs index f7333b90b2..62f72a71b1 100644 --- a/modules/src/clients/ics07_tendermint/client_state.rs +++ b/modules/src/clients/ics07_tendermint/client_state.rs @@ -170,7 +170,6 @@ impl ClientState { /// Verify the time and height delays pub fn verify_delay_passed( - &self, current_time: Timestamp, current_height: Height, processed_time: Timestamp, From 3075c61278f9cf98d80a156f444ea83912224403 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 13 Dec 2021 16:55:41 +0530 Subject: [PATCH 14/26] Move get_block_delay() into ChannelReader trait as provided method --- modules/src/clients/ics07_tendermint/client_def.rs | 12 +----------- modules/src/core/ics04_channel/context.rs | 9 +++++++++ 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index f775a993ea..f3b11313ec 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -1,4 +1,3 @@ -use core::time::Duration; use std::convert::TryInto; use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof; @@ -449,7 +448,7 @@ fn verify_delay_passed( .map_err(|_| Error::processed_height_not_found(client_id.clone()))?; let delay_period_time = connection_end.delay_period(); - let delay_period_height = get_block_delay(ctx, delay_period_time); + let delay_period_height = ctx.block_delay(delay_period_time); ClientState::verify_delay_passed( current_timestamp, @@ -462,15 +461,6 @@ fn verify_delay_passed( .map_err(|e| e.into()) } -fn get_block_delay(ctx: &dyn ChannelReader, delay_period_time: Duration) -> u64 { - let expected_time_per_block = ctx.max_expected_time_per_block(); - if expected_time_per_block.is_zero() { - return 0; - } - - (delay_period_time.as_secs_f64() / expected_time_per_block.as_secs_f64()).ceil() as u64 -} - fn downcast_consensus_state(cs: AnyConsensusState) -> Result { downcast!( cs => AnyConsensusState::Tendermint diff --git a/modules/src/core/ics04_channel/context.rs b/modules/src/core/ics04_channel/context.rs index bbb0181853..8dece3aeb2 100644 --- a/modules/src/core/ics04_channel/context.rs +++ b/modules/src/core/ics04_channel/context.rs @@ -85,6 +85,15 @@ pub trait ChannelReader { /// Returns the maximum expected time per block fn max_expected_time_per_block(&self) -> Duration; + + fn block_delay(&self, delay_period_time: Duration) -> u64 { + let expected_time_per_block = self.max_expected_time_per_block(); + if expected_time_per_block.is_zero() { + return 0; + } + + (delay_period_time.as_secs_f64() / expected_time_per_block.as_secs_f64()).ceil() as u64 + } } /// A context supplying all the necessary write-only dependencies (i.e., storage writing facility) From 424088d222b4f72e2a9bf68c09fd195bb15de4e3 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 13 Dec 2021 16:59:56 +0530 Subject: [PATCH 15/26] Remove usages of std:: --- .../ics20_fungible_token_transfer/msgs/transfer.rs | 4 ++-- modules/src/clients/ics07_tendermint/client_def.rs | 2 +- modules/src/clients/ics07_tendermint/client_state.rs | 3 --- modules/src/clients/ics07_tendermint/consensus_state.rs | 3 --- modules/src/core/ics04_channel/handler/send_packet.rs | 4 ++-- modules/src/test.rs | 3 --- 6 files changed, 5 insertions(+), 14 deletions(-) diff --git a/modules/src/applications/ics20_fungible_token_transfer/msgs/transfer.rs b/modules/src/applications/ics20_fungible_token_transfer/msgs/transfer.rs index 2522376f95..8cb3238c42 100644 --- a/modules/src/applications/ics20_fungible_token_transfer/msgs/transfer.rs +++ b/modules/src/applications/ics20_fungible_token_transfer/msgs/transfer.rs @@ -99,8 +99,8 @@ impl From for RawMsgTransfer { #[cfg(test)] pub mod test_util { - use std::ops::Add; - use std::time::Duration; + use core::ops::Add; + use core::time::Duration; use crate::{ core::ics24_host::identifier::{ChannelId, PortId}, diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index f3b11313ec..e1a2501b36 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -1,4 +1,4 @@ -use std::convert::TryInto; +use core::convert::TryInto; use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof; use prost::Message; diff --git a/modules/src/clients/ics07_tendermint/client_state.rs b/modules/src/clients/ics07_tendermint/client_state.rs index 62f72a71b1..25a1fff7bd 100644 --- a/modules/src/clients/ics07_tendermint/client_state.rs +++ b/modules/src/clients/ics07_tendermint/client_state.rs @@ -301,7 +301,6 @@ impl From for RawClientState { mod tests { use crate::prelude::*; use core::time::Duration; - use std::println; use test_log::test; use tendermint_rpc::endpoint::abci_query::AbciQuery; @@ -318,7 +317,6 @@ mod tests { fn serialization_roundtrip_no_proof() { let json_data = include_str!("../../../tests/support/query/serialization/client_state.json"); - println!("json_data: {:?}", json_data); test_serialization_roundtrip::(json_data); } @@ -326,7 +324,6 @@ mod tests { fn serialization_roundtrip_with_proof() { let json_data = include_str!("../../../tests/support/query/serialization/client_state_proof.json"); - println!("json_data: {:?}", json_data); test_serialization_roundtrip::(json_data); } diff --git a/modules/src/clients/ics07_tendermint/consensus_state.rs b/modules/src/clients/ics07_tendermint/consensus_state.rs index d439264bc0..2b48a58f70 100644 --- a/modules/src/clients/ics07_tendermint/consensus_state.rs +++ b/modules/src/clients/ics07_tendermint/consensus_state.rs @@ -114,7 +114,6 @@ impl From
for ConsensusState { #[cfg(test)] mod tests { - use std::println; use tendermint_rpc::endpoint::abci_query::AbciQuery; use test_log::test; @@ -124,7 +123,6 @@ mod tests { fn serialization_roundtrip_no_proof() { let json_data = include_str!("../../../tests/support/query/serialization/consensus_state.json"); - println!("json_data: {:?}", json_data); test_serialization_roundtrip::(json_data); } @@ -132,7 +130,6 @@ mod tests { fn serialization_roundtrip_with_proof() { let json_data = include_str!("../../../tests/support/query/serialization/consensus_state_proof.json"); - println!("json_data: {:?}", json_data); test_serialization_roundtrip::(json_data); } } diff --git a/modules/src/core/ics04_channel/handler/send_packet.rs b/modules/src/core/ics04_channel/handler/send_packet.rs index dde7d60ede..76db241966 100644 --- a/modules/src/core/ics04_channel/handler/send_packet.rs +++ b/modules/src/core/ics04_channel/handler/send_packet.rs @@ -129,8 +129,8 @@ mod tests { use crate::mock::context::MockContext; use crate::timestamp::Timestamp; use crate::timestamp::ZERO_DURATION; - use std::ops::Add; - use std::time::Duration; + use core::ops::Add; + use core::time::Duration; #[test] fn send_packet_processing() { diff --git a/modules/src/test.rs b/modules/src/test.rs index 45ba0d0aa4..3f823b388a 100644 --- a/modules/src/test.rs +++ b/modules/src/test.rs @@ -1,6 +1,5 @@ use core::fmt::Debug; use serde::{de::DeserializeOwned, Serialize}; -use std::println; /// Test that a struct `T` can be: /// @@ -23,8 +22,6 @@ where let parsed1 = serde_json::from_str::(&serialized); assert!(parsed1.is_ok()); let parsed1 = parsed1.unwrap(); - println!("json_data0: {:?}", parsed0); - println!("json_data1: {:?}", parsed1); // TODO - fix PartialEq bound issue in AbciQuery //assert_eq!(parsed0, parsed1); From 493326940ddff95cee8c6f69ff765ed5220364a8 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 13 Dec 2021 17:34:12 +0530 Subject: [PATCH 16/26] Fix clippy errors --- modules/src/test.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/src/test.rs b/modules/src/test.rs index 3f823b388a..b6a9cb1f34 100644 --- a/modules/src/test.rs +++ b/modules/src/test.rs @@ -21,7 +21,6 @@ where let parsed1 = serde_json::from_str::(&serialized); assert!(parsed1.is_ok()); - let parsed1 = parsed1.unwrap(); // TODO - fix PartialEq bound issue in AbciQuery //assert_eq!(parsed0, parsed1); From f9b16e6aa73e6d5ff2d50920cb3871f9acb3c76f Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 13 Dec 2021 19:58:41 +0530 Subject: [PATCH 17/26] Add keeper methods for processed time and height --- .../clients/ics07_tendermint/client_def.rs | 13 +++--- modules/src/core/ics02_client/context.rs | 12 +++++ modules/src/core/ics04_channel/context.rs | 4 +- modules/src/mock/context.rs | 44 ++++++++++++++++--- 4 files changed, 59 insertions(+), 14 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index e1a2501b36..c062b9bad8 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -277,7 +277,7 @@ impl ClientDef for TendermintClient { commitment: String, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - verify_delay_passed(ctx, connection_end)?; + verify_delay_passed(ctx, height, connection_end)?; let commitment_path = Path::Commitments { port_id: port_id.clone(), @@ -308,7 +308,7 @@ impl ClientDef for TendermintClient { ack: Vec, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - verify_delay_passed(ctx, connection_end)?; + verify_delay_passed(ctx, height, connection_end)?; let ack_path = Path::Acks { port_id: port_id.clone(), @@ -338,7 +338,7 @@ impl ClientDef for TendermintClient { sequence: Sequence, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - verify_delay_passed(ctx, connection_end)?; + verify_delay_passed(ctx, height, connection_end)?; let seq_path = Path::SeqRecvs(port_id.clone(), channel_id.clone()); verify_membership( @@ -364,7 +364,7 @@ impl ClientDef for TendermintClient { sequence: Sequence, ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - verify_delay_passed(ctx, connection_end)?; + verify_delay_passed(ctx, height, connection_end)?; let receipt_path = Path::Receipts { port_id: port_id.clone(), @@ -434,6 +434,7 @@ fn verify_non_membership( fn verify_delay_passed( ctx: &dyn ChannelReader, + height: Height, connection_end: &ConnectionEnd, ) -> Result<(), Ics02Error> { let current_timestamp = ctx.host_timestamp(); @@ -441,10 +442,10 @@ fn verify_delay_passed( let client_id = connection_end.client_id(); let processed_time = ctx - .processed_time(client_id) + .processed_time(client_id, height) .map_err(|_| Error::processed_time_not_found(client_id.clone()))?; let processed_height = ctx - .processed_height(client_id) + .processed_height(client_id, height) .map_err(|_| Error::processed_height_not_found(client_id.clone()))?; let delay_period_time = connection_end.delay_period(); diff --git a/modules/src/core/ics02_client/context.rs b/modules/src/core/ics02_client/context.rs index a4af0c287d..d70db84be8 100644 --- a/modules/src/core/ics02_client/context.rs +++ b/modules/src/core/ics02_client/context.rs @@ -87,6 +87,8 @@ pub trait ClientKeeper { res.client_state.latest_height(), res.consensus_state, )?; + self.store_processed_time(res.client_id.clone(), res.client_state.latest_height())?; + self.store_processed_height(res.client_id, res.client_state.latest_height())?; Ok(()) } Upgrade(res) => { @@ -127,4 +129,14 @@ pub trait ClientKeeper { /// Increases the counter which keeps track of how many clients have been created. /// Should never fail. fn increase_client_counter(&mut self); + + /// Called upon successful client update. + /// Implementations are expected to use this to record the current (host) time as the time at + /// which this update (or header) was processed. + fn store_processed_time(&mut self, client_id: ClientId, height: Height) -> Result<(), Error>; + + /// Called upon successful client update. + /// Implementations are expected to use this to record the current (host) height as the height + /// at which this update (or header) was processed. + fn store_processed_height(&mut self, client_id: ClientId, height: Height) -> Result<(), Error>; } diff --git a/modules/src/core/ics04_channel/context.rs b/modules/src/core/ics04_channel/context.rs index 8dece3aeb2..f3c02511ec 100644 --- a/modules/src/core/ics04_channel/context.rs +++ b/modules/src/core/ics04_channel/context.rs @@ -73,10 +73,10 @@ pub trait ChannelReader { fn host_timestamp(&self) -> Timestamp; /// Returns the time when the client state of the given identifier is stored - fn processed_time(&self, client_id: &ClientId) -> Result; + fn processed_time(&self, client_id: &ClientId, height: Height) -> Result; /// Returns the height when the client state of the given identifier is stored - fn processed_height(&self, client_id: &ClientId) -> Result; + fn processed_height(&self, client_id: &ClientId, height: Height) -> Result; /// Returns a counter on the number of channel ids have been created thus far. /// The value of this counter should increase only via method diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index 44540bb9f9..c6a05b1cd4 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -70,10 +70,10 @@ pub struct MockContext { clients: BTreeMap, /// Tracks the processed time for the clients - client_processed_times: BTreeMap, + client_processed_times: BTreeMap<(ClientId, Height), Timestamp>, /// Tracks the processed height for the clients - client_processed_heights: BTreeMap, + client_processed_heights: BTreeMap<(ClientId, Height), Height>, /// Counter for the client identifiers, necessary for `increase_client_counter` and the /// `client_counter` methods. @@ -674,15 +674,25 @@ impl ChannelReader for MockContext { self.timestamp } - fn processed_time(&self, client_id: &ClientId) -> Result { - match self.client_processed_times.get(client_id) { + fn processed_time( + &self, + client_id: &ClientId, + height: Height, + ) -> Result { + match self + .client_processed_times + .get(&(client_id.clone(), height)) + { Some(time) => Ok(*time), None => Err(Ics04Error::processed_time_not_found(client_id.clone())), } } - fn processed_height(&self, client_id: &ClientId) -> Result { - match self.client_processed_heights.get(client_id) { + fn processed_height(&self, client_id: &ClientId, height: Height) -> Result { + match self + .client_processed_heights + .get(&(client_id.clone(), height)) + { Some(height) => Ok(*height), None => Err(Ics04Error::processed_height_not_found(client_id.clone())), } @@ -1027,6 +1037,28 @@ impl ClientKeeper for MockContext { fn increase_client_counter(&mut self) { self.client_ids_counter += 1 } + + fn store_processed_time( + &mut self, + client_id: ClientId, + height: Height, + ) -> Result<(), Ics02Error> { + let _ = self + .client_processed_times + .insert((client_id, height), Timestamp::now()); + Ok(()) + } + + fn store_processed_height( + &mut self, + client_id: ClientId, + height: Height, + ) -> Result<(), Ics02Error> { + let _ = self + .client_processed_heights + .insert((client_id, height), ClientReader::host_height(self)); + Ok(()) + } } impl Ics18Context for MockContext { From db66a34221b28a818a639d72731de5a3900956df Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 13 Dec 2021 23:55:54 +0530 Subject: [PATCH 18/26] Set processed time using host_timestamp() --- modules/src/mock/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index c6a05b1cd4..90de372b4e 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -1045,7 +1045,7 @@ impl ClientKeeper for MockContext { ) -> Result<(), Ics02Error> { let _ = self .client_processed_times - .insert((client_id, height), Timestamp::now()); + .insert((client_id, height), ChannelReader::host_timestamp(self)); Ok(()) } From 11c641df5a34a74d1228f7dee7643648034a9934 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 17 Dec 2021 16:56:32 +0530 Subject: [PATCH 19/26] Add new ICS02 error variant InvalidCommitmentProof --- modules/src/clients/ics07_tendermint/client_def.rs | 4 ++-- modules/src/core/ics02_client/error.rs | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index c062b9bad8..98ee8d8b69 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -401,7 +401,7 @@ fn verify_membership( ) -> Result<(), Ics02Error> { let merkle_path = apply_prefix(prefix, vec![path]).map_err(Error::ics23_error)?; let merkle_proof: MerkleProof = RawMerkleProof::try_from(proof.clone()) - .map_err(Ics02Error::invalid_consensus_state_proof)? + .map_err(Ics02Error::invalid_commitment_proof)? .into(); merkle_proof @@ -424,7 +424,7 @@ fn verify_non_membership( ) -> Result<(), Ics02Error> { let merkle_path = apply_prefix(prefix, vec![path]).map_err(Error::ics23_error)?; let merkle_proof: MerkleProof = RawMerkleProof::try_from(proof.clone()) - .map_err(Ics02Error::invalid_consensus_state_proof)? + .map_err(Ics02Error::invalid_commitment_proof)? .into(); merkle_proof diff --git a/modules/src/core/ics02_client/error.rs b/modules/src/core/ics02_client/error.rs index d3da2bbf76..2dc3873e8b 100644 --- a/modules/src/core/ics02_client/error.rs +++ b/modules/src/core/ics02_client/error.rs @@ -173,6 +173,10 @@ define_error! { [ Ics23Error ] | _ | { "invalid proof for the consensus state" }, + InvalidCommitmentProof + [ Ics23Error ] + | _ | { "invalid commitment proof bytes" }, + Tendermint [ Ics07Error ] | _ | { "tendermint error" }, From 5c1ee1d829aa598f0c649ef4534cf6de08d083ab Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 17 Dec 2021 17:14:38 +0530 Subject: [PATCH 20/26] Augment packet delay errors --- .../src/clients/ics07_tendermint/client_state.rs | 15 ++++++++++----- modules/src/clients/ics07_tendermint/error.rs | 14 +++++++++++--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_state.rs b/modules/src/clients/ics07_tendermint/client_state.rs index 25a1fff7bd..a427b7325b 100644 --- a/modules/src/clients/ics07_tendermint/client_state.rs +++ b/modules/src/clients/ics07_tendermint/client_state.rs @@ -177,13 +177,18 @@ impl ClientState { delay_period_time: Duration, delay_period_blocks: u64, ) -> Result<(), Error> { - let time = (processed_time + delay_period_time).map_err(Error::timestamp_overflow)?; - if !(current_time == time || current_time.after(&time)) { - return Err(Error::not_enough_time_elapsed()); + let earliest_time = + (processed_time + delay_period_time).map_err(Error::timestamp_overflow)?; + if !(current_time == earliest_time || current_time.after(&earliest_time)) { + return Err(Error::not_enough_time_elapsed(current_time, earliest_time)); } - if current_height < processed_height.add(delay_period_blocks) { - return Err(Error::not_enough_blocks_elapsed()); + let earliest_height = processed_height.add(delay_period_blocks); + if current_height < earliest_height { + return Err(Error::not_enough_blocks_elapsed( + current_height, + earliest_height, + )); } Ok(()) diff --git a/modules/src/clients/ics07_tendermint/error.rs b/modules/src/clients/ics07_tendermint/error.rs index 11fff4c48f..77d2f0fd0a 100644 --- a/modules/src/clients/ics07_tendermint/error.rs +++ b/modules/src/clients/ics07_tendermint/error.rs @@ -5,7 +5,7 @@ use flex_error::{define_error, TraceError}; use crate::core::ics23_commitment::error::Error as Ics23Error; use crate::core::ics24_host::error::ValidationError; use crate::core::ics24_host::identifier::ClientId; -use crate::timestamp::TimestampOverflowError; +use crate::timestamp::{Timestamp, TimestampOverflowError}; use crate::Height; use tendermint::account::Id; use tendermint::hash::Hash; @@ -154,10 +154,18 @@ define_error! { |_| { "timestamp overflowed" }, NotEnoughTimeElapsed - |_| { "not enough time elapsed" }, + { + current_time: Timestamp, + earliest_time: Timestamp, + } + |_| { "not enough time elapsed, current timestamp {0} is still less than earliest acceptable timestamp {1}" }, NotEnoughBlocksElapsed - |_| { "not enough blocks elapsed" }, + { + current_height: Height, + earliest_height: Height, + } + |_| { "not enough blocks elapsed, current height {0} is still less than earliest acceptable height {1}" }, InvalidHeaderHeight { height: Height } From ec08b33c7ee933230c6a16bd2ea6dbc718c26b6d Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 17 Dec 2021 17:25:06 +0530 Subject: [PATCH 21/26] Revert to old naming of errors for client upgrade proofs --- modules/src/core/ics02_client/error.rs | 8 ++++---- modules/src/core/ics02_client/msgs/upgrade_client.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/src/core/ics02_client/error.rs b/modules/src/core/ics02_client/error.rs index 2dc3873e8b..2bb71bc529 100644 --- a/modules/src/core/ics02_client/error.rs +++ b/modules/src/core/ics02_client/error.rs @@ -165,13 +165,13 @@ define_error! { InvalidAddress | _ | { "invalid address" }, - InvalidClientProof + InvalidUpgradeClientProof [ Ics23Error ] - | _ | { "invalid proof for the client state" }, + | _ | { "invalid proof for the upgraded client state" }, - InvalidConsensusStateProof + InvalidUpgradeConsensusStateProof [ Ics23Error ] - | _ | { "invalid proof for the consensus state" }, + | _ | { "invalid proof for the upgraded consensus state" }, InvalidCommitmentProof [ Ics23Error ] diff --git a/modules/src/core/ics02_client/msgs/upgrade_client.rs b/modules/src/core/ics02_client/msgs/upgrade_client.rs index d2de775368..79f2be07bb 100644 --- a/modules/src/core/ics02_client/msgs/upgrade_client.rs +++ b/modules/src/core/ics02_client/msgs/upgrade_client.rs @@ -100,9 +100,9 @@ impl TryFrom for MsgUpgradeAnyClient { client_state: AnyClientState::try_from(raw_client_state)?, consensus_state: AnyConsensusState::try_from(raw_consensus_state)?, proof_upgrade_client: RawMerkleProof::try_from(c_bytes) - .map_err(Error::invalid_client_proof)?, + .map_err(Error::invalid_upgrade_client_proof)?, proof_upgrade_consensus_state: RawMerkleProof::try_from(cs_bytes) - .map_err(Error::invalid_consensus_state_proof)?, + .map_err(Error::invalid_upgrade_consensus_state_proof)?, signer: proto_msg.signer.into(), }) } From 8dcb33d3db390346c9b96ec98f84fdfa63adb618 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 17 Dec 2021 17:32:22 +0530 Subject: [PATCH 22/26] Rename processed_{time,height}() --- modules/src/clients/ics07_tendermint/client_def.rs | 4 ++-- modules/src/core/ics04_channel/context.rs | 4 ++-- modules/src/mock/context.rs | 8 ++++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index 98ee8d8b69..11fad12972 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -442,10 +442,10 @@ fn verify_delay_passed( let client_id = connection_end.client_id(); let processed_time = ctx - .processed_time(client_id, height) + .client_update_time(client_id, height) .map_err(|_| Error::processed_time_not_found(client_id.clone()))?; let processed_height = ctx - .processed_height(client_id, height) + .client_update_height(client_id, height) .map_err(|_| Error::processed_height_not_found(client_id.clone()))?; let delay_period_time = connection_end.delay_period(); diff --git a/modules/src/core/ics04_channel/context.rs b/modules/src/core/ics04_channel/context.rs index f3c02511ec..50fabdf01c 100644 --- a/modules/src/core/ics04_channel/context.rs +++ b/modules/src/core/ics04_channel/context.rs @@ -73,10 +73,10 @@ pub trait ChannelReader { fn host_timestamp(&self) -> Timestamp; /// Returns the time when the client state of the given identifier is stored - fn processed_time(&self, client_id: &ClientId, height: Height) -> Result; + fn client_update_time(&self, client_id: &ClientId, height: Height) -> Result; /// Returns the height when the client state of the given identifier is stored - fn processed_height(&self, client_id: &ClientId, height: Height) -> Result; + fn client_update_height(&self, client_id: &ClientId, height: Height) -> Result; /// Returns a counter on the number of channel ids have been created thus far. /// The value of this counter should increase only via method diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index 90de372b4e..7d3a0a6880 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -674,7 +674,7 @@ impl ChannelReader for MockContext { self.timestamp } - fn processed_time( + fn client_update_time( &self, client_id: &ClientId, height: Height, @@ -688,7 +688,11 @@ impl ChannelReader for MockContext { } } - fn processed_height(&self, client_id: &ClientId, height: Height) -> Result { + fn client_update_height( + &self, + client_id: &ClientId, + height: Height, + ) -> Result { match self .client_processed_heights .get(&(client_id.clone(), height)) From 6b010bf7f8029df76c5c8583b40a7213bc7ee2ee Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Fri, 17 Dec 2021 17:34:54 +0530 Subject: [PATCH 23/26] Apply suggestions from code review Co-authored-by: Adi Seredinschi --- modules/src/clients/ics07_tendermint/client_state.rs | 4 ++-- modules/src/core/ics02_client/context.rs | 4 ++-- modules/src/core/ics04_channel/context.rs | 4 ++-- modules/src/core/ics23_commitment/error.rs | 2 +- modules/src/mock/context.rs | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_state.rs b/modules/src/clients/ics07_tendermint/client_state.rs index 25a1fff7bd..185c042062 100644 --- a/modules/src/clients/ics07_tendermint/client_state.rs +++ b/modules/src/clients/ics07_tendermint/client_state.rs @@ -177,8 +177,8 @@ impl ClientState { delay_period_time: Duration, delay_period_blocks: u64, ) -> Result<(), Error> { - let time = (processed_time + delay_period_time).map_err(Error::timestamp_overflow)?; - if !(current_time == time || current_time.after(&time)) { + let earliest_time = (processed_time + delay_period_time).map_err(Error::timestamp_overflow)?; + if !(current_time == earliest_time || current_time.after(&earliest_time)) { return Err(Error::not_enough_time_elapsed()); } diff --git a/modules/src/core/ics02_client/context.rs b/modules/src/core/ics02_client/context.rs index d70db84be8..1a67d77e3d 100644 --- a/modules/src/core/ics02_client/context.rs +++ b/modules/src/core/ics02_client/context.rs @@ -133,10 +133,10 @@ pub trait ClientKeeper { /// Called upon successful client update. /// Implementations are expected to use this to record the current (host) time as the time at /// which this update (or header) was processed. - fn store_processed_time(&mut self, client_id: ClientId, height: Height) -> Result<(), Error>; + fn store_update_time(&mut self, client_id: ClientId, height: Height) -> Result<(), Error>; /// Called upon successful client update. /// Implementations are expected to use this to record the current (host) height as the height /// at which this update (or header) was processed. - fn store_processed_height(&mut self, client_id: ClientId, height: Height) -> Result<(), Error>; + fn store_update_height(&mut self, client_id: ClientId, height: Height) -> Result<(), Error>; } diff --git a/modules/src/core/ics04_channel/context.rs b/modules/src/core/ics04_channel/context.rs index f3c02511ec..c669aa822f 100644 --- a/modules/src/core/ics04_channel/context.rs +++ b/modules/src/core/ics04_channel/context.rs @@ -72,10 +72,10 @@ pub trait ChannelReader { /// Returns the current timestamp of the local chain. fn host_timestamp(&self) -> Timestamp; - /// Returns the time when the client state of the given identifier is stored + /// Returns the time when the client state for the given [`ClientId`] was updated with a header for the given [`Height`] fn processed_time(&self, client_id: &ClientId, height: Height) -> Result; - /// Returns the height when the client state of the given identifier is stored + /// Returns the height when the client state for the given [`ClientId`] was updated with a header for the given [`Height`] fn processed_height(&self, client_id: &ClientId, height: Height) -> Result; /// Returns a counter on the number of channel ids have been created thus far. diff --git a/modules/src/core/ics23_commitment/error.rs b/modules/src/core/ics23_commitment/error.rs index e54b685ff2..260c9557b4 100644 --- a/modules/src/core/ics23_commitment/error.rs +++ b/modules/src/core/ics23_commitment/error.rs @@ -19,7 +19,7 @@ define_error! { |_| { "empty merkle proof" }, EmptyMerkleRoot - |_| { "empty merkle proof" }, + |_| { "empty merkle root" }, EmptyVerifiedValue |_| { "empty verified value" }, diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index 90de372b4e..c2d3ca78b3 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -69,7 +69,7 @@ pub struct MockContext { /// The set of all clients, indexed by their id. clients: BTreeMap, - /// Tracks the processed time for the clients + /// Tracks the processed time for clients header updates client_processed_times: BTreeMap<(ClientId, Height), Timestamp>, /// Tracks the processed height for the clients From 7381e02bd64c2c7821380b8de70e180b642d5642 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Sat, 18 Dec 2021 01:32:10 +0530 Subject: [PATCH 24/26] Record height in processed height/time errors --- .../src/clients/ics07_tendermint/client_def.rs | 4 ++-- modules/src/clients/ics07_tendermint/error.rs | 18 ++++++++++++------ modules/src/core/ics04_channel/error.rs | 18 ++++++++++++------ modules/src/mock/context.rs | 10 ++++++++-- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index 11fad12972..cfba41c18a 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -443,10 +443,10 @@ fn verify_delay_passed( let client_id = connection_end.client_id(); let processed_time = ctx .client_update_time(client_id, height) - .map_err(|_| Error::processed_time_not_found(client_id.clone()))?; + .map_err(|_| Error::processed_time_not_found(client_id.clone(), height))?; let processed_height = ctx .client_update_height(client_id, height) - .map_err(|_| Error::processed_height_not_found(client_id.clone()))?; + .map_err(|_| Error::processed_height_not_found(client_id.clone(), height))?; let delay_period_time = connection_end.delay_period(); let delay_period_height = ctx.block_delay(delay_period_time); diff --git a/modules/src/clients/ics07_tendermint/error.rs b/modules/src/clients/ics07_tendermint/error.rs index 77d2f0fd0a..a8a12009b1 100644 --- a/modules/src/clients/ics07_tendermint/error.rs +++ b/modules/src/clients/ics07_tendermint/error.rs @@ -222,19 +222,25 @@ define_error! { }, ProcessedTimeNotFound - { client_id: ClientId } + { + client_id: ClientId, + height: Height, + } | e | { format_args!( - "Processed time for the client {0} not found", - e.client_id) + "Processed time for the client {0} at height {1} not found", + e.client_id, e.height) }, ProcessedHeightNotFound - { client_id: ClientId } + { + client_id: ClientId, + height: Height, + } | e | { format_args!( - "Processed height for the client {0} not found", - e.client_id) + "Processed height for the client {0} at height {1} not found", + e.client_id, e.height) }, Ics23Error diff --git a/modules/src/core/ics04_channel/error.rs b/modules/src/core/ics04_channel/error.rs index 1c746a6f12..a31f6ed4f9 100644 --- a/modules/src/core/ics04_channel/error.rs +++ b/modules/src/core/ics04_channel/error.rs @@ -325,19 +325,25 @@ define_error! { }, ProcessedTimeNotFound - { client_id: ClientId } + { + client_id: ClientId, + height: Height, + } | e | { format_args!( - "Processed time for the client {0} not found", - e.client_id) + "Processed time for the client {0} at height {1} not found", + e.client_id, e.height) }, ProcessedHeightNotFound - { client_id: ClientId } + { + client_id: ClientId, + height: Height, + } | e | { format_args!( - "Processed height for the client {0} not found", - e.client_id) + "Processed height for the client {0} at height {1} not found", + e.client_id, e.height) }, ImplementationSpecific diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index dc09f2cedb..3830b4446e 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -684,7 +684,10 @@ impl ChannelReader for MockContext { .get(&(client_id.clone(), height)) { Some(time) => Ok(*time), - None => Err(Ics04Error::processed_time_not_found(client_id.clone())), + None => Err(Ics04Error::processed_time_not_found( + client_id.clone(), + height, + )), } } @@ -698,7 +701,10 @@ impl ChannelReader for MockContext { .get(&(client_id.clone(), height)) { Some(height) => Ok(*height), - None => Err(Ics04Error::processed_height_not_found(client_id.clone())), + None => Err(Ics04Error::processed_height_not_found( + client_id.clone(), + height, + )), } } From bb66a5f370dd735a99d8cda8e7dd03e83e9d81f5 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 22 Dec 2021 00:30:53 +0530 Subject: [PATCH 25/26] Fix clippy errors --- modules/src/core/ics04_channel/handler/send_packet.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/modules/src/core/ics04_channel/handler/send_packet.rs b/modules/src/core/ics04_channel/handler/send_packet.rs index 1a9665e965..0065aab900 100644 --- a/modules/src/core/ics04_channel/handler/send_packet.rs +++ b/modules/src/core/ics04_channel/handler/send_packet.rs @@ -111,8 +111,8 @@ pub fn send_packet(ctx: &dyn ChannelReader, packet: Packet) -> HandlerResult Date: Wed, 22 Dec 2021 00:41:30 +0530 Subject: [PATCH 26/26] Add changelog entry --- .../unreleased/features/ibc/1583-module-verification-ICS07.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/features/ibc/1583-module-verification-ICS07.md diff --git a/.changelog/unreleased/features/ibc/1583-module-verification-ICS07.md b/.changelog/unreleased/features/ibc/1583-module-verification-ICS07.md new file mode 100644 index 0000000000..5f11ff8e2f --- /dev/null +++ b/.changelog/unreleased/features/ibc/1583-module-verification-ICS07.md @@ -0,0 +1,3 @@ +- Implement proof verification for Tendermint client (ICS07). + ([#1583](https://github.com/informalsystems/ibc-rs/pull/1583)) +