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)) + 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 88be8fb54b..60170d2c73 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 f9b2199514..0fb7eac146 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -1,8 +1,10 @@ -use std::convert::TryInto; +use core::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_light_client::components::verifier::{ProdVerifier, Verdict, Verifier}; use tendermint_light_client::types::{TrustedBlockState, UntrustedBlockState}; +use tendermint_proto::Protobuf; use time::OffsetDateTime; use crate::clients::ics07_tendermint::client_state::ClientState; @@ -17,13 +19,16 @@ 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::{ 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; @@ -185,116 +190,278 @@ 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, + height: Height, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + client_id: &ClientId, + consensus_height: Height, + expected_consensus_state: &AnyConsensusState, ) -> Result<(), Ics02Error> { - todo!() + client_state.verify_height(height)?; + + 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, + height: Height, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + connection_id: &ConnectionId, + expected_connection_end: &ConnectionEnd, ) -> Result<(), Ics02Error> { - todo!() + 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) } 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, + height: Height, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + expected_channel_end: &ChannelEnd, ) -> Result<(), Ics02Error> { - todo!() + 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) } 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, + height: Height, + prefix: &CommitmentPrefix, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + client_id: &ClientId, + expected_client_state: &AnyClientState, ) -> Result<(), Ics02Error> { - unimplemented!() + 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) } fn verify_packet_data( &self, - _client_state: &Self::ClientState, - _height: Height, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: &Sequence, - _data: String, + ctx: &dyn ChannelReader, + client_state: &Self::ClientState, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + commitment: String, ) -> Result<(), Ics02Error> { - todo!() + client_state.verify_height(height)?; + verify_delay_passed(ctx, height, 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(), + proof, + root, + commitment_path.to_string(), + commitment.encode_to_vec(), + ) } fn verify_packet_acknowledgement( &self, - _client_state: &Self::ClientState, - _height: Height, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: &Sequence, - _data: Vec, + ctx: &dyn ChannelReader, + client_state: &Self::ClientState, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + ack: Vec, ) -> Result<(), Ics02Error> { - todo!() + client_state.verify_height(height)?; + verify_delay_passed(ctx, height, 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(), + proof, + root, + ack_path.to_string(), + ack, + ) } fn verify_next_sequence_recv( &self, - _client_state: &Self::ClientState, - _height: Height, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: &Sequence, + ctx: &dyn ChannelReader, + client_state: &Self::ClientState, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Ics02Error> { - todo!() + client_state.verify_height(height)?; + verify_delay_passed(ctx, height, 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(sequence).encode_to_vec(), + ) } fn verify_packet_receipt_absence( &self, - _client_state: &Self::ClientState, - _height: Height, - _proof: &CommitmentProofBytes, - _port_id: &PortId, - _channel_id: &ChannelId, - _seq: &Sequence, + ctx: &dyn ChannelReader, + client_state: &Self::ClientState, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, ) -> Result<(), Ics02Error> { - todo!() + client_state.verify_height(height)?; + verify_delay_passed(ctx, height, 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(), + proof, + root, + receipt_path.to_string(), + ) } 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_commitment_proof)? + .into(); + + merkle_proof + .verify_membership( + &client_state.proof_specs, + 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_commitment_proof)? + .into(); + + merkle_proof + .verify_non_membership(&client_state.proof_specs, root.clone().into(), merkle_path) + .map_err(|e| Ics02Error::tendermint(Error::ics23_error(e))) +} + +fn verify_delay_passed( + ctx: &dyn ChannelReader, + height: Height, + 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 + .client_update_time(client_id, height) + .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(), height))?; + + let delay_period_time = connection_end.delay_period(); + let delay_period_height = ctx.block_delay(delay_period_time); + + ClientState::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 f25a11ea4b..0ced24fe1c 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::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::timestamp::{Timestamp, ZERO_DURATION}; use crate::Height; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -175,6 +175,46 @@ impl ClientState { clock_drift: self.max_clock_drift, }) } + + /// Verify the time and height delays + pub fn verify_delay_passed( + current_time: Timestamp, + current_height: Height, + processed_time: Timestamp, + processed_height: Height, + delay_period_time: Duration, + delay_period_blocks: u64, + ) -> Result<(), Error> { + 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)); + } + + 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(()) + } + + /// 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 { @@ -274,7 +314,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; @@ -291,7 +330,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); } @@ -299,7 +337,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 141df337a6..69b1c27b20 100644 --- a/modules/src/clients/ics07_tendermint/consensus_state.rs +++ b/modules/src/clients/ics07_tendermint/consensus_state.rs @@ -118,7 +118,6 @@ impl From
for ConsensusState { #[cfg(test)] mod tests { - use std::println; use tendermint_rpc::endpoint::abci_query::AbciQuery; use test_log::test; @@ -128,7 +127,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); } @@ -136,7 +134,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/clients/ics07_tendermint/error.rs b/modules/src/clients/ics07_tendermint/error.rs index 91e78d37a5..a8a12009b1 100644 --- a/modules/src/clients/ics07_tendermint/error.rs +++ b/modules/src/clients/ics07_tendermint/error.rs @@ -2,7 +2,10 @@ 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::core::ics24_host::identifier::ClientId; +use crate::timestamp::{Timestamp, TimestampOverflowError}; use crate::Height; use tendermint::account::Id; use tendermint::hash::Hash; @@ -146,6 +149,24 @@ 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 + { + current_time: Timestamp, + earliest_time: Timestamp, + } + |_| { "not enough time elapsed, current timestamp {0} is still less than earliest acceptable timestamp {1}" }, + + NotEnoughBlocksElapsed + { + current_height: Height, + earliest_height: Height, + } + |_| { "not enough blocks elapsed, current height {0} is still less than earliest acceptable height {1}" }, + InvalidHeaderHeight { height: Height } | e | { @@ -198,7 +219,51 @@ define_error! { { detail: tendermint_light_client::predicates::errors::VerificationErrorDetail } | e | { format_args!("verification failed: {}", e.detail) + }, + + ProcessedTimeNotFound + { + client_id: ClientId, + height: Height, + } + | e | { + format_args!( + "Processed time for the client {0} at height {1} not found", + e.client_id, e.height) + }, + + ProcessedHeightNotFound + { + client_id: ClientId, + height: Height, + } + | e | { + format_args!( + "Processed height for the client {0} at height {1} not found", + e.client_id, e.height) + }, + + 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 6f439a2e05..d1a4311408 100644 --- a/modules/src/core/ics02_client/client_def.rs +++ b/modules/src/core/ics02_client/client_def.rs @@ -9,6 +9,7 @@ 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, @@ -57,19 +58,22 @@ pub trait ClientDef: Clone { height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, client_id: &ClientId, consensus_height: Height, expected_consensus_state: &AnyConsensusState, ) -> 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, height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, - connection_id: Option<&ConnectionId>, + root: &CommitmentRoot, + connection_id: &ConnectionId, expected_connection_end: &ConnectionEnd, ) -> Result<(), Error>; @@ -81,6 +85,7 @@ pub trait ClientDef: Clone { height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, expected_channel_end: &ChannelEnd, @@ -90,25 +95,28 @@ pub trait ClientDef: Clone { #[allow(clippy::too_many_arguments)] fn verify_client_full_state( &self, - _client_state: &Self::ClientState, + client_state: &Self::ClientState, height: Height, - root: &CommitmentRoot, 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. #[allow(clippy::too_many_arguments)] fn verify_packet_data( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, height: Height, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + sequence: Sequence, commitment: String, ) -> Result<(), Error>; @@ -116,12 +124,15 @@ pub trait ClientDef: Clone { #[allow(clippy::too_many_arguments)] fn verify_packet_acknowledgement( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, height: Height, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + sequence: Sequence, ack: Vec, ) -> Result<(), Error>; @@ -129,24 +140,30 @@ pub trait ClientDef: Clone { #[allow(clippy::too_many_arguments)] fn verify_next_sequence_recv( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, height: Height, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + sequence: 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, height: Height, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + sequence: Sequence, ) -> Result<(), Error>; } @@ -225,6 +242,7 @@ impl ClientDef for AnyClient { height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, client_id: &ClientId, consensus_height: Height, expected_consensus_state: &AnyConsensusState, @@ -241,6 +259,7 @@ impl ClientDef for AnyClient { height, prefix, proof, + root, client_id, consensus_height, expected_consensus_state, @@ -259,6 +278,7 @@ impl ClientDef for AnyClient { height, prefix, proof, + root, client_id, consensus_height, expected_consensus_state, @@ -273,7 +293,8 @@ impl ClientDef for AnyClient { height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, - connection_id: Option<&ConnectionId>, + root: &CommitmentRoot, + connection_id: &ConnectionId, expected_connection_end: &ConnectionEnd, ) -> Result<(), Error> { match self { @@ -286,6 +307,7 @@ impl ClientDef for AnyClient { height, prefix, proof, + root, connection_id, expected_connection_end, ) @@ -301,6 +323,7 @@ impl ClientDef for AnyClient { height, prefix, proof, + root, connection_id, expected_connection_end, ) @@ -314,6 +337,7 @@ impl ClientDef for AnyClient { height: Height, prefix: &CommitmentPrefix, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, expected_channel_end: &ChannelEnd, @@ -328,6 +352,7 @@ impl ClientDef for AnyClient { height, prefix, proof, + root, port_id, channel_id, expected_channel_end, @@ -344,6 +369,7 @@ impl ClientDef for AnyClient { height, prefix, proof, + root, port_id, channel_id, expected_channel_end, @@ -356,10 +382,10 @@ impl ClientDef for AnyClient { &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 { @@ -372,10 +398,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, ) } @@ -390,10 +416,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, ) } @@ -401,12 +427,15 @@ impl ClientDef for AnyClient { } fn verify_packet_data( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, height: Height, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + sequence: Sequence, commitment: String, ) -> Result<(), Error> { match self { @@ -417,12 +446,15 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Tendermint))?; client.verify_packet_data( + ctx, client_state, height, + connection_end, proof, + root, port_id, channel_id, - seq, + sequence, commitment, ) } @@ -435,12 +467,15 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Mock))?; client.verify_packet_data( + ctx, client_state, height, + connection_end, proof, + root, port_id, channel_id, - seq, + sequence, commitment, ) } @@ -449,12 +484,15 @@ impl ClientDef for AnyClient { fn verify_packet_acknowledgement( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, height: Height, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + sequence: Sequence, ack: Vec, ) -> Result<(), Error> { match self { @@ -465,12 +503,15 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Tendermint))?; client.verify_packet_acknowledgement( + ctx, client_state, height, + connection_end, proof, + root, port_id, channel_id, - seq, + sequence, ack, ) } @@ -483,12 +524,15 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Mock))?; client.verify_packet_acknowledgement( + ctx, client_state, height, + connection_end, proof, + root, port_id, channel_id, - seq, + sequence, ack, ) } @@ -497,12 +541,15 @@ impl ClientDef for AnyClient { fn verify_next_sequence_recv( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, height: Height, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + sequence: Sequence, ) -> Result<(), Error> { match self { Self::Tendermint(client) => { @@ -512,12 +559,15 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Tendermint))?; client.verify_next_sequence_recv( + ctx, client_state, height, + connection_end, proof, + root, port_id, channel_id, - seq, + sequence, ) } @@ -529,24 +579,30 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Mock))?; client.verify_next_sequence_recv( + ctx, client_state, height, + connection_end, proof, + root, port_id, channel_id, - seq, + sequence, ) } } } fn verify_packet_receipt_absence( &self, + ctx: &dyn ChannelReader, client_state: &Self::ClientState, height: Height, + connection_end: &ConnectionEnd, proof: &CommitmentProofBytes, + root: &CommitmentRoot, port_id: &PortId, channel_id: &ChannelId, - seq: &Sequence, + sequence: Sequence, ) -> Result<(), Error> { match self { Self::Tendermint(client) => { @@ -556,12 +612,15 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Tendermint))?; client.verify_packet_receipt_absence( + ctx, client_state, height, + connection_end, proof, + root, port_id, channel_id, - seq, + sequence, ) } @@ -573,12 +632,15 @@ impl ClientDef for AnyClient { .ok_or_else(|| Error::client_args_type_mismatch(ClientType::Mock))?; client.verify_packet_receipt_absence( + ctx, client_state, height, + connection_end, proof, + root, port_id, channel_id, - seq, + sequence, ) } } diff --git a/modules/src/core/ics02_client/context.rs b/modules/src/core/ics02_client/context.rs index 1ef619414b..2d6b138eab 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; @@ -84,6 +87,8 @@ pub trait ClientKeeper { res.client_state.latest_height(), res.consensus_state, )?; + self.store_update_time(res.client_id.clone(), res.client_state.latest_height())?; + self.store_update_height(res.client_id, res.client_state.latest_height())?; Ok(()) } Upgrade(res) => { @@ -124,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_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_update_height(&mut self, client_id: ClientId, height: Height) -> Result<(), Error>; } diff --git a/modules/src/core/ics02_client/error.rs b/modules/src/core/ics02_client/error.rs index 504f7dae2a..2bb71bc529 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 @@ -173,6 +173,10 @@ define_error! { [ Ics23Error ] | _ | { "invalid proof for the upgraded consensus state" }, + InvalidCommitmentProof + [ Ics23Error ] + | _ | { "invalid commitment proof bytes" }, + Tendermint [ Ics07Error ] | _ | { "tendermint error" }, diff --git a/modules/src/core/ics02_client/handler/create_client.rs b/modules/src/core/ics02_client/handler/create_client.rs index bbf1c14dee..811bbf9e3c 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/ics03_connection/handler/conn_open_ack.rs b/modules/src/core/ics03_connection/handler/conn_open_ack.rs index 0447b34cc6..69038d348e 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(), ); @@ -55,6 +66,7 @@ pub(crate) fn process( verify_proofs( ctx, msg.client_state(), + msg.proofs().height(), &conn_end, &expected_conn, msg.proofs(), @@ -62,10 +74,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, 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 cc474f94e6..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,12 +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, - proofs.height(), - &proof, - )?) + Ok(verify_consensus_proof(ctx, height, connection_end, &proof)?) } else { Ok(()) } @@ -58,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, @@ -72,20 +71,26 @@ 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, + height, connection_end.counterparty().prefix(), proof, - connection_end.counterparty().connection_id(), + consensus_state.root(), + connection_id, expected_conn, ) .map_err(Error::verify_connection_state) @@ -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,11 +125,11 @@ pub fn verify_client_proof( client_def .verify_client_full_state( &client_state, - proof_height, - consensus_state.root(), + height, connection_end.counterparty().prefix(), - connection_end.counterparty().client_id(), proof, + consensus_state.root(), + connection_end.counterparty().client_id(), &expected_client_state, ) .map_err(|e| { @@ -133,8 +139,8 @@ pub fn verify_client_proof( pub fn verify_consensus_proof( ctx: &dyn ConnectionReader, + height: Height, connection_end: &ConnectionEnd, - proof_height: Height, proof: &ConsensusProof, ) -> Result<(), Error> { // Fetch the client state (IBC client on the local chain). @@ -152,9 +158,10 @@ pub fn verify_consensus_proof( client .verify_client_consensus_state( &client_state, - proof_height, + 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/context.rs b/modules/src/core/ics04_channel/context.rs index e148cd50c3..9ea3b237b8 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; @@ -71,10 +72,28 @@ pub trait ChannelReader { /// Returns the current timestamp of the local chain. fn host_timestamp(&self) -> Timestamp; + /// Returns the time when the client state for the given [`ClientId`] was updated with a header for the given [`Height`] + fn client_update_time(&self, client_id: &ClientId, height: Height) -> Result; + + /// Returns the height when the client state for the given [`ClientId`] was updated with a header for the given [`Height`] + 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 /// `ChannelKeeper::increase_channel_counter`. fn channel_counter(&self) -> Result; + + /// 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) diff --git a/modules/src/core/ics04_channel/error.rs b/modules/src/core/ics04_channel/error.rs index 93d766018f..202521a5ed 100644 --- a/modules/src/core/ics04_channel/error.rs +++ b/modules/src/core/ics04_channel/error.rs @@ -321,6 +321,28 @@ define_error! { e.port_channel_id.1) }, + ProcessedTimeNotFound + { + client_id: ClientId, + height: Height, + } + | e | { + format_args!( + "Processed time for the client {0} at height {1} not found", + e.client_id, e.height) + }, + + ProcessedHeightNotFound + { + client_id: ClientId, + height: Height, + } + | e | { + format_args!( + "Processed height for the client {0} at height {1} not found", + e.client_id, e.height) + }, + ImplementationSpecific | _ | { "implementation specific error" }, } diff --git a/modules/src/core/ics04_channel/handler/acknowledgement.rs b/modules/src/core/ics04_channel/handler/acknowledgement.rs index adcc1a7bd8..baccc0d4be 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(), @@ -78,9 +76,10 @@ pub fn process( // Verify the acknowledgement proof verify_packet_acknowledgement_proofs( ctx, + msg.proofs.height(), packet, msg.acknowledgement().clone(), - client_id, + &connection_end, msg.proofs(), )?; 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 f3b836b702..724884020b 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 b8e8d1446b..403a5a3f9d 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_ack.rs @@ -75,6 +75,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 6259f395d4..aefefabdea 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 93f674651e..1b79851d28 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_try.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_try.rs @@ -113,6 +113,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 091d5cbfd4..cbf69fb3f3 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 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()); @@ -73,12 +78,15 @@ pub fn verify_packet_recv_proofs( // Verify the proof for the packet against the chain store. client_def .verify_packet_data( + ctx, &client_state, - proofs.height(), + height, + connection_end, 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))?; @@ -89,29 +97,36 @@ pub fn verify_packet_recv_proofs( /// Entry point for verifying all proofs bundled in an ICS4 packet ack message. pub fn verify_packet_acknowledgement_proofs( ctx: &dyn ChannelReader, + height: Height, 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( + ctx, &client_state, - proofs.height(), + height, + connection_end, proofs.object_proof(), - &packet.source_port, - &packet.source_channel, - &packet.sequence, + consensus_state.root(), + &packet.destination_port, + &packet.destination_channel, + packet.sequence, acknowledgement, ) .map_err(|e| Error::packet_verification_failed(packet.sequence, e))?; @@ -122,29 +137,36 @@ 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, + height: Height, + 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( + ctx, &client_state, - proofs.height(), + height, + connection_end, proofs.object_proof(), + consensus_state.root(), &packet.destination_port, &packet.destination_channel, - &seq, + packet.sequence, ) .map_err(|e| Error::packet_verification_failed(seq, e))?; @@ -153,28 +175,35 @@ pub fn verify_next_sequence_recv( pub fn verify_packet_receipt_absence( ctx: &dyn ChannelReader, - client_id: ClientId, + height: Height, + 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( + ctx, &client_state, - proofs.height(), + height, + connection_end, 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/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 { diff --git a/modules/src/core/ics23_commitment/error.rs b/modules/src/core/ics23_commitment/error.rs index 43d8032f88..260c9557b4 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 root" }, + + 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..6eb05c80f3 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,148 @@ 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(); + let ics23_specs = Vec::::from(specs.clone()); + if ics23_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(ics23_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(); + let ics23_specs = Vec::::from(specs.clone()); + if ics23_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 = ics23_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 +221,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/mock/client_def.rs b/modules/src/mock/client_def.rs index c0f63d6d87..c5a755e478 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, @@ -49,28 +50,24 @@ impl ClientDef for MockClient { fn verify_client_consensus_state( &self, _client_state: &Self::ClientState, - height: Height, + _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(()) } @@ -80,7 +77,8 @@ impl ClientDef for MockClient { _height: Height, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, - _connection_id: Option<&ConnectionId>, + _root: &CommitmentRoot, + _connection_id: &ConnectionId, _expected_connection_end: &ConnectionEnd, ) -> Result<(), Error> { Ok(()) @@ -92,6 +90,7 @@ impl ClientDef for MockClient { _height: Height, _prefix: &CommitmentPrefix, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, _expected_channel_end: &ChannelEnd, @@ -103,10 +102,10 @@ impl ClientDef for MockClient { &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(()) @@ -114,53 +113,66 @@ impl ClientDef for MockClient { fn verify_packet_data( &self, + _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _height: Height, + _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, - _seq: &Sequence, - _data: String, + _sequence: Sequence, + _commitment: String, ) -> Result<(), Error> { Ok(()) } fn verify_packet_acknowledgement( &self, + _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _height: Height, + _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, - _seq: &Sequence, - _data: Vec, + _sequence: Sequence, + _ack: Vec, ) -> Result<(), Error> { Ok(()) } fn verify_next_sequence_recv( &self, + _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _height: Height, + _connection_end: &ConnectionEnd, _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, _port_id: &PortId, _channel_id: &ChannelId, - _seq: &Sequence, + _sequence: Sequence, ) -> Result<(), Error> { Ok(()) } fn verify_packet_receipt_absence( &self, + _ctx: &dyn ChannelReader, _client_state: &Self::ClientState, _height: Height, + _connection_end: &ConnectionEnd, _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/src/mock/context.rs b/modules/src/mock/context.rs index 69da32514b..3123b40f44 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)] @@ -68,6 +69,12 @@ pub struct MockContext { /// The set of all clients, indexed by their id. clients: BTreeMap, + /// Tracks the processed time for clients header updates + client_processed_times: BTreeMap<(ClientId, Height), Timestamp>, + + /// Tracks the processed height for the clients + client_processed_heights: BTreeMap<(ClientId, Height), Height>, + /// Counter for the client identifiers, necessary for `increase_client_counter` and the /// `client_counter` methods. client_ids_counter: u64, @@ -171,6 +178,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,9 +674,47 @@ impl ChannelReader for MockContext { self.timestamp } + fn client_update_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(), + height, + )), + } + } + + fn client_update_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(), + height, + )), + } + } + 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 { @@ -939,6 +986,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) } @@ -996,6 +1047,24 @@ impl ClientKeeper for MockContext { fn increase_client_counter(&mut self) { self.client_ids_counter += 1 } + + fn store_update_time(&mut self, client_id: ClientId, height: Height) -> Result<(), Ics02Error> { + let _ = self + .client_processed_times + .insert((client_id, height), ChannelReader::host_timestamp(self)); + Ok(()) + } + + fn store_update_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 { diff --git a/modules/src/test.rs b/modules/src/test.rs index 45ba0d0aa4..b6a9cb1f34 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: /// @@ -22,9 +21,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); diff --git a/modules/src/timestamp.rs b/modules/src/timestamp.rs index 1dbb29e719..142fc489d0 100644 --- a/modules/src/timestamp.rs +++ b/modules/src/timestamp.rs @@ -172,6 +172,7 @@ impl Display for Timestamp { } define_error! { + #[derive(Debug, PartialEq, Eq)] TimestampOverflowError { TimestampOverflow |_| { "Timestamp overflow when modifying with duration" } diff --git a/modules/tests/runner/mod.rs b/modules/tests/runner/mod.rs index b569c2a0dd..eee2539cf5 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) {