From 968e553605acacdf799f51745373e08e02365a3b Mon Sep 17 00:00:00 2001 From: Davirain Date: Wed, 4 Jan 2023 23:33:04 +0800 Subject: [PATCH] Fix the function parameters in the Reader trait should be used with the reference modifier (#313) * the function parameters in the Reader trait used with the reference modifier * fix clippy * Create 304-reader-function-use-reference-modifier.md --- ...-reader-function-use-reference-modifier.md | 3 + .../clients/ics07_tendermint/client_state.rs | 20 ++--- crates/ibc/src/core/ics02_client/context.rs | 9 +- .../core/ics02_client/handler/misbehaviour.rs | 2 +- .../ics02_client/handler/update_client.rs | 10 +-- .../ibc/src/core/ics03_connection/context.rs | 16 ++-- .../ics03_connection/handler/conn_open_ack.rs | 4 +- .../handler/conn_open_confirm.rs | 2 +- .../ics03_connection/handler/conn_open_try.rs | 7 +- .../ibc/src/core/ics03_connection/version.rs | 6 +- crates/ibc/src/core/ics04_channel/context.rs | 64 +++++++------- .../ics04_channel/handler/acknowledgement.rs | 19 +++-- .../handler/chan_close_confirm.rs | 2 +- .../ics04_channel/handler/chan_open_ack.rs | 2 +- .../handler/chan_open_confirm.rs | 2 +- .../ics04_channel/handler/chan_open_try.rs | 2 +- .../core/ics04_channel/handler/recv_packet.rs | 2 +- .../core/ics04_channel/handler/send_packet.rs | 8 +- .../src/core/ics04_channel/handler/timeout.rs | 21 +++-- .../ics04_channel/handler/timeout_on_close.rs | 19 +++-- .../src/core/ics04_channel/handler/verify.rs | 18 ++-- .../handler/write_acknowledgement.rs | 4 +- crates/ibc/src/core/ics26_routing/handler.rs | 2 +- crates/ibc/src/mock/context.rs | 85 ++++++++++--------- crates/ibc/src/test_utils.rs | 10 +-- 25 files changed, 178 insertions(+), 161 deletions(-) create mode 100644 .changelog/unreleased/feature/304-reader-function-use-reference-modifier.md diff --git a/.changelog/unreleased/feature/304-reader-function-use-reference-modifier.md b/.changelog/unreleased/feature/304-reader-function-use-reference-modifier.md new file mode 100644 index 000000000..8a02945f1 --- /dev/null +++ b/.changelog/unreleased/feature/304-reader-function-use-reference-modifier.md @@ -0,0 +1,3 @@ +- The function parameters in the Reader trait should be used with the reference + modifier, while the functions in the Keeper trait take ownership directly. + ([#304](https://github.com/cosmos/ibc-rs/issues/304)) \ No newline at end of file diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index f6c492ee3..37a9823ed 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -416,7 +416,7 @@ impl Ics2ClientState for ClientState { fn maybe_consensus_state( ctx: &dyn ClientReader, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result>, ClientError> { match ctx.consensus_state(client_id, height) { Ok(cs) => Ok(Some(cs)), @@ -447,7 +447,7 @@ impl Ics2ClientState for ClientState { // match the untrusted header. let header_consensus_state = TmConsensusState::from(header.clone()); let existing_consensus_state = - match maybe_consensus_state(ctx, &client_id, header.height())? { + match maybe_consensus_state(ctx, &client_id, &header.height())? { Some(cs) => { let cs = downcast_tm_consensus_state(cs.as_ref())?; // If this consensus state matches, skip verification @@ -466,7 +466,7 @@ impl Ics2ClientState for ClientState { }; let trusted_consensus_state = downcast_tm_consensus_state( - ctx.consensus_state(&client_id, header.trusted_height)? + ctx.consensus_state(&client_id, &header.trusted_height)? .as_ref(), )?; @@ -523,7 +523,7 @@ impl Ics2ClientState for ClientState { // (cs-new, cs-next, cs-latest) if header.height() < client_state.latest_height() { let maybe_next_cs = ctx - .next_consensus_state(&client_id, header.height())? + .next_consensus_state(&client_id, &header.height())? .as_ref() .map(|cs| downcast_tm_consensus_state(cs.as_ref())) .transpose()?; @@ -546,7 +546,7 @@ impl Ics2ClientState for ClientState { // (cs-trusted, cs-prev, cs-new) if header.trusted_height < header.height() { let maybe_prev_cs = ctx - .prev_consensus_state(&client_id, header.height())? + .prev_consensus_state(&client_id, &header.height())? .as_ref() .map(|cs| downcast_tm_consensus_state(cs.as_ref())) .transpose()?; @@ -597,11 +597,11 @@ impl Ics2ClientState for ClientState { } let consensus_state_1 = { - let cs = ctx.consensus_state(&client_id, header_1.trusted_height)?; + let cs = ctx.consensus_state(&client_id, &header_1.trusted_height)?; downcast_tm_consensus_state(cs.as_ref()) }?; let consensus_state_2 = { - let cs = ctx.consensus_state(&client_id, header_2.trusted_height)?; + let cs = ctx.consensus_state(&client_id, &header_2.trusted_height)?; downcast_tm_consensus_state(cs.as_ref()) }?; @@ -1144,12 +1144,12 @@ fn verify_delay_passed( let client_id = connection_end.client_id(); let processed_time = - ctx.client_update_time(client_id, height) + ctx.client_update_time(client_id, &height) .map_err(|_| Error::ProcessedTimeNotFound { client_id: client_id.clone(), height, })?; - let processed_height = ctx.client_update_height(client_id, height).map_err(|_| { + let processed_height = ctx.client_update_height(client_id, &height).map_err(|_| { Error::ProcessedHeightNotFound { client_id: client_id.clone(), height, @@ -1157,7 +1157,7 @@ fn verify_delay_passed( })?; let delay_period_time = connection_end.delay_period(); - let delay_period_height = ctx.block_delay(delay_period_time); + let delay_period_height = ctx.block_delay(&delay_period_time); ClientState::verify_delay_passed( current_timestamp, diff --git a/crates/ibc/src/core/ics02_client/context.rs b/crates/ibc/src/core/ics02_client/context.rs index b0aa53d81..c9225e62d 100644 --- a/crates/ibc/src/core/ics02_client/context.rs +++ b/crates/ibc/src/core/ics02_client/context.rs @@ -33,21 +33,21 @@ pub trait ClientReader { fn consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, ClientError>; /// Search for the lowest consensus state higher than `height`. fn next_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result>, ClientError>; /// Search for the highest consensus state lower than `height`. fn prev_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result>, ClientError>; /// Returns the current height of the local chain. @@ -62,7 +62,8 @@ pub trait ClientReader { } /// Returns the `ConsensusState` of the host (local) chain at a specific height. - fn host_consensus_state(&self, height: Height) -> Result, ClientError>; + fn host_consensus_state(&self, height: &Height) + -> Result, ClientError>; /// Returns the pending `ConsensusState` of the host (local) chain. fn pending_host_consensus_state(&self) -> Result, ClientError>; diff --git a/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs b/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs index 4c7ea368d..000d7ac39 100644 --- a/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs +++ b/crates/ibc/src/core/ics02_client/handler/misbehaviour.rs @@ -306,7 +306,7 @@ mod tests { // Get chain-B's header at `misbehaviour_height` let header1: TmHeader = { - let mut block = ctx_b.host_block(misbehaviour_height).unwrap().clone(); + let mut block = ctx_b.host_block(&misbehaviour_height).unwrap().clone(); block.set_trusted_height(client_height); block.try_into_tm_block().unwrap().into() }; diff --git a/crates/ibc/src/core/ics02_client/handler/update_client.rs b/crates/ibc/src/core/ics02_client/handler/update_client.rs index 8e74e0aeb..d7aaa605f 100644 --- a/crates/ibc/src/core/ics02_client/handler/update_client.rs +++ b/crates/ibc/src/core/ics02_client/handler/update_client.rs @@ -165,7 +165,7 @@ pub fn process( // Read consensus state from the host chain store. let latest_consensus_state = - ClientReader::consensus_state(ctx, &client_id, client_state.latest_height()).map_err( + ClientReader::consensus_state(ctx, &client_id, &client_state.latest_height()).map_err( |_| ClientError::ConsensusStateNotFound { client_id: client_id.clone(), height: client_state.latest_height(), @@ -385,7 +385,7 @@ mod tests { let signer = get_dummy_account_id(); - let mut block = ctx_b.host_block(update_height).unwrap().clone(); + let mut block = ctx_b.host_block(&update_height).unwrap().clone(); block.set_trusted_height(client_height); let latest_header_height = block.height(); @@ -445,7 +445,7 @@ mod tests { let signer = get_dummy_account_id(); - let mut block = ctx_b.host_block(update_height).unwrap().clone(); + let mut block = ctx_b.host_block(&update_height).unwrap().clone(); let trusted_height = client_height.clone().sub(1).unwrap(); block.set_trusted_height(trusted_height); @@ -510,7 +510,7 @@ mod tests { let signer = get_dummy_account_id(); - let block = ctx_b.host_block(client_height).unwrap().clone(); + let block = ctx_b.host_block(&client_height).unwrap().clone(); let block = match block { HostBlock::SyntheticTendermint(mut theader) => { let cons_state = ctx.latest_consensus_states(&client_id, &client_height); @@ -588,7 +588,7 @@ mod tests { let signer = get_dummy_account_id(); - let block_ref = ctx_b.host_block(client_update_height).unwrap(); + let block_ref = ctx_b.host_block(&client_update_height).unwrap(); let msg = MsgUpdateClient { client_id, diff --git a/crates/ibc/src/core/ics03_connection/context.rs b/crates/ibc/src/core/ics03_connection/context.rs index ac31e1563..09229a72f 100644 --- a/crates/ibc/src/core/ics03_connection/context.rs +++ b/crates/ibc/src/core/ics03_connection/context.rs @@ -44,13 +44,13 @@ pub trait ConnectionReader { fn client_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, ConnectionError>; /// Returns the ConsensusState of the host (local) chain at a specific height. fn host_consensus_state( &self, - height: Height, + height: &Height, ) -> Result, ConnectionError>; /// Function required by ICS 03. Returns the list of all possible versions that the connection @@ -63,8 +63,8 @@ pub trait ConnectionReader { /// connection handshake protocol prefers. fn pick_version( &self, - supported_versions: Vec, - counterparty_candidate_versions: Vec, + supported_versions: &[Version], + counterparty_candidate_versions: &[Version], ) -> Result { pick_version(supported_versions, counterparty_candidate_versions) } @@ -82,7 +82,7 @@ pub trait ConnectionReader { /// for processing any `ConnectionMsg`. pub trait ConnectionKeeper { fn store_connection_result(&mut self, result: ConnectionResult) -> Result<(), ConnectionError> { - self.store_connection(result.connection_id.clone(), &result.connection_end)?; + self.store_connection(result.connection_id.clone(), result.connection_end.clone())?; // If we generated an identifier, increase the counter & associate this new identifier // with the client id. @@ -92,7 +92,7 @@ pub trait ConnectionKeeper { // Also associate the connection end to its client identifier. self.store_connection_to_client( result.connection_id.clone(), - result.connection_end.client_id(), + result.connection_end.client_id().clone(), )?; } @@ -103,14 +103,14 @@ pub trait ConnectionKeeper { fn store_connection( &mut self, connection_id: ConnectionId, - connection_end: &ConnectionEnd, + connection_end: ConnectionEnd, ) -> Result<(), ConnectionError>; /// Stores the given connection_id at a path associated with the client_id. fn store_connection_to_client( &mut self, connection_id: ConnectionId, - client_id: &ClientId, + client_id: ClientId, ) -> Result<(), ConnectionError>; /// Called upon connection identifier creation (Init or Try process). diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs index 10442e5f7..bee059ef5 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs @@ -244,7 +244,7 @@ pub(crate) fn process( { let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?; let consensus_state_of_b_on_a = - ctx_a.client_consensus_state(conn_end_on_a.client_id(), msg.proofs_height_on_b)?; + ctx_a.client_consensus_state(conn_end_on_a.client_id(), &msg.proofs_height_on_b)?; let prefix_on_a = ctx_a.commitment_prefix(); let prefix_on_b = conn_end_on_a.counterparty().prefix(); @@ -289,7 +289,7 @@ pub(crate) fn process( })?; let expected_consensus_state_of_a_on_b = - ctx_a.host_consensus_state(msg.consensus_height_of_a_on_b)?; + ctx_a.host_consensus_state(&msg.consensus_height_of_a_on_b)?; client_state_of_b_on_a .verify_client_consensus_state( msg.proofs_height_on_b, diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs index 8f9830445..53cc17259 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_confirm.rs @@ -203,7 +203,7 @@ pub(crate) fn process( { let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?; let consensus_state_of_a_on_b = - ctx_b.client_consensus_state(client_id_on_b, msg.proof_height_on_a)?; + ctx_b.client_consensus_state(client_id_on_b, &msg.proof_height_on_a)?; let prefix_on_a = conn_end_on_b.counterparty().prefix(); let prefix_on_b = ctx_b.commitment_prefix(); diff --git a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs index 42ff6ff78..fd68eaa31 100644 --- a/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs +++ b/crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs @@ -228,8 +228,7 @@ pub(crate) fn process( }); } - let version_on_b = - ctx_b.pick_version(ctx_b.get_compatible_versions(), msg.versions_on_a.clone())?; + let version_on_b = ctx_b.pick_version(&ctx_b.get_compatible_versions(), &msg.versions_on_a)?; let conn_end_on_b = ConnectionEnd::new( State::TryOpen, @@ -249,7 +248,7 @@ pub(crate) fn process( { let client_state_of_a_on_b = ctx_b.client_state(conn_end_on_b.client_id())?; let consensus_state_of_a_on_b = - ctx_b.client_consensus_state(&msg.client_id_on_b, msg.proofs_height_on_a)?; + ctx_b.client_consensus_state(&msg.client_id_on_b, &msg.proofs_height_on_a)?; let prefix_on_a = conn_end_on_b.counterparty().prefix(); let prefix_on_b = ctx_b.commitment_prefix(); @@ -291,7 +290,7 @@ pub(crate) fn process( })?; let expected_consensus_state_of_b_on_a = - ctx_b.host_consensus_state(msg.consensus_height_of_b_on_a)?; + ctx_b.host_consensus_state(&msg.consensus_height_of_b_on_a)?; client_state_of_a_on_b .verify_client_consensus_state( msg.proofs_height_on_a, diff --git a/crates/ibc/src/core/ics03_connection/version.rs b/crates/ibc/src/core/ics03_connection/version.rs index 368c5a8de..785246f68 100644 --- a/crates/ibc/src/core/ics03_connection/version.rs +++ b/crates/ibc/src/core/ics03_connection/version.rs @@ -97,8 +97,8 @@ pub fn get_compatible_versions() -> Vec { /// Selects a version from the intersection of locally supported and counterparty versions. pub fn pick_version( - supported_versions: Vec, - counterparty_versions: Vec, + supported_versions: &[Version], + counterparty_versions: &[Version], ) -> Result { let mut intersection: Vec = Vec::new(); for s in supported_versions.iter() { @@ -303,7 +303,7 @@ mod tests { ]; for test in tests { - let version = pick_version(test.supported, test.counterparty); + let version = pick_version(&test.supported, &test.counterparty); assert_eq!( test.want_pass, diff --git a/crates/ibc/src/core/ics04_channel/context.rs b/crates/ibc/src/core/ics04_channel/context.rs index 908dfafa6..ffe4d3db3 100644 --- a/crates/ibc/src/core/ics04_channel/context.rs +++ b/crates/ibc/src/core/ics04_channel/context.rs @@ -48,7 +48,7 @@ pub trait ChannelReader { fn client_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, ChannelError>; fn get_next_sequence_send( @@ -73,21 +73,21 @@ pub trait ChannelReader { &self, port_id: &PortId, channel_id: &ChannelId, - sequence: Sequence, + sequence: &Sequence, ) -> Result; fn get_packet_receipt( &self, port_id: &PortId, channel_id: &ChannelId, - sequence: Sequence, + sequence: &Sequence, ) -> Result; fn get_packet_acknowledgement( &self, port_id: &PortId, channel_id: &ChannelId, - sequence: Sequence, + sequence: &Sequence, ) -> Result; /// Compute the commitment for a packet. @@ -97,9 +97,9 @@ pub trait ChannelReader { /// fn packet_commitment( &self, - packet_data: Vec, - timeout_height: TimeoutHeight, - timeout_timestamp: Timestamp, + packet_data: &[u8], + timeout_height: &TimeoutHeight, + timeout_timestamp: &Timestamp, ) -> PacketCommitment { let mut hash_input = timeout_timestamp.nanoseconds().to_be_bytes().to_vec(); @@ -112,15 +112,15 @@ pub trait ChannelReader { let packet_data_hash = self.hash(packet_data); hash_input.append(&mut packet_data_hash.to_vec()); - self.hash(hash_input).into() + self.hash(&hash_input).into() } - fn ack_commitment(&self, ack: Acknowledgement) -> AcknowledgementCommitment { - self.hash(ack.into()).into() + fn ack_commitment(&self, ack: &Acknowledgement) -> AcknowledgementCommitment { + self.hash(ack.as_ref()).into() } /// A hashing function for packet commitments - fn hash(&self, value: Vec) -> Vec; + fn hash(&self, value: &[u8]) -> Vec; /// Returns the current height of the local chain. fn host_height(&self) -> Result; @@ -134,8 +134,10 @@ pub trait ChannelReader { } /// Returns the `ConsensusState` of the host (local) chain at a specific height. - fn host_consensus_state(&self, height: Height) - -> Result, ChannelError>; + fn host_consensus_state( + &self, + height: &Height, + ) -> Result, ChannelError>; /// Returns the pending `ConsensusState` of the host (local) chain. fn pending_host_consensus_state(&self) -> Result, ChannelError>; @@ -144,14 +146,14 @@ pub trait ChannelReader { fn client_update_time( &self, client_id: &ClientId, - height: Height, + 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, + height: &Height, ) -> Result; /// Returns a counter on the number of channel ids have been created thus far. @@ -164,8 +166,8 @@ pub trait ChannelReader { /// Calculates the block delay period using the connection's delay period and the maximum /// expected time per block. - fn block_delay(&self, delay_period_time: Duration) -> u64 { - calculate_block_delay(delay_period_time, self.max_expected_time_per_block()) + fn block_delay(&self, delay_period_time: &Duration) -> u64 { + calculate_block_delay(delay_period_time, &self.max_expected_time_per_block()) } } @@ -187,7 +189,7 @@ pub trait SendPacketReader { fn client_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, PacketError>; fn get_next_sequence_send( @@ -196,13 +198,13 @@ pub trait SendPacketReader { channel_id: &ChannelId, ) -> Result; - fn hash(&self, value: Vec) -> Vec; + fn hash(&self, value: &[u8]) -> Vec; fn packet_commitment( &self, - packet_data: Vec, - timeout_height: TimeoutHeight, - timeout_timestamp: Timestamp, + packet_data: &[u8], + timeout_height: &TimeoutHeight, + timeout_timestamp: &Timestamp, ) -> PacketCommitment { let mut hash_input = timeout_timestamp.nanoseconds().to_be_bytes().to_vec(); @@ -215,7 +217,7 @@ pub trait SendPacketReader { let packet_data_hash = self.hash(packet_data); hash_input.append(&mut packet_data_hash.to_vec()); - self.hash(hash_input).into() + self.hash(&hash_input).into() } } @@ -242,7 +244,7 @@ where fn client_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, PacketError> { ChannelReader::client_consensus_state(self, client_id, height).map_err(PacketError::Channel) } @@ -255,7 +257,7 @@ where ChannelReader::get_next_sequence_send(self, port_id, channel_id) } - fn hash(&self, value: Vec) -> Vec { + fn hash(&self, value: &[u8]) -> Vec { ChannelReader::hash(self, value) } } @@ -338,14 +340,14 @@ pub trait ChannelKeeper { )?; } PacketResult::Ack(res) => { - self.delete_packet_commitment(&res.port_id, &res.channel_id, res.seq)?; + self.delete_packet_commitment(&res.port_id, &res.channel_id, &res.seq)?; if let Some(s) = res.seq_number { //Ordered Channel self.store_next_sequence_ack(res.port_id, res.channel_id, s)?; } } PacketResult::Timeout(res) => { - self.delete_packet_commitment(&res.port_id, &res.channel_id, res.seq)?; + self.delete_packet_commitment(&res.port_id, &res.channel_id, &res.seq)?; if let Some(c) = res.channel { // Ordered Channel: closes channel self.store_channel(res.port_id, res.channel_id, c) @@ -368,7 +370,7 @@ pub trait ChannelKeeper { &mut self, port_id: &PortId, channel_id: &ChannelId, - seq: Sequence, + seq: &Sequence, ) -> Result<(), PacketError>; fn store_packet_receipt( @@ -391,7 +393,7 @@ pub trait ChannelKeeper { &mut self, port_id: &PortId, channel_id: &ChannelId, - sequence: Sequence, + sequence: &Sequence, ) -> Result<(), PacketError>; fn store_connection_channels( @@ -437,8 +439,8 @@ pub trait ChannelKeeper { } pub fn calculate_block_delay( - delay_period_time: Duration, - max_expected_time_per_block: Duration, + delay_period_time: &Duration, + max_expected_time_per_block: &Duration, ) -> u64 { if max_expected_time_per_block.is_zero() { return 0; diff --git a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs index fa4e4d6d8..4f135bc3a 100644 --- a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs @@ -61,14 +61,17 @@ pub fn process( } // Verify packet commitment - let packet_commitment = - ctx.get_packet_commitment(&packet.source_port, &packet.source_channel, packet.sequence)?; + let packet_commitment = ctx.get_packet_commitment( + &packet.source_port, + &packet.source_channel, + &packet.sequence, + )?; if packet_commitment != ctx.packet_commitment( - packet.data.clone(), - packet.timeout_height, - packet.timeout_timestamp, + &packet.data, + &packet.timeout_height, + &packet.timeout_timestamp, ) { return Err(PacketError::IncorrectPacketCommitment { @@ -165,9 +168,9 @@ mod tests { let packet = msg.packet.clone(); let data = context.packet_commitment( - packet.data.clone(), - packet.timeout_height, - packet.timeout_timestamp, + &packet.data, + &packet.timeout_height, + &packet.timeout_timestamp, ); let source_channel_end = ChannelEnd::new( diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs index 760eacde2..408de1acf 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -46,7 +46,7 @@ pub(crate) fn process( let client_id_on_b = conn_end_on_b.client_id().clone(); let client_state_of_a_on_b = ctx_b.client_state(&client_id_on_b)?; let consensus_state_of_a_on_b = - ctx_b.client_consensus_state(&client_id_on_b, msg.proof_height_on_a)?; + ctx_b.client_consensus_state(&client_id_on_b, &msg.proof_height_on_a)?; let prefix_on_a = conn_end_on_b.counterparty().prefix(); let port_id_on_a = &chan_end_on_b.counterparty().port_id; let chan_id_on_a = chan_end_on_b diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs index 43dde0b90..0c500a7a7 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs @@ -48,7 +48,7 @@ pub(crate) fn process( let client_id_on_a = conn_end_on_a.client_id().clone(); let client_state_of_b_on_a = ctx_a.client_state(&client_id_on_a)?; let consensus_state_of_b_on_a = - ctx_a.client_consensus_state(&client_id_on_a, msg.proof_height_on_b)?; + ctx_a.client_consensus_state(&client_id_on_a, &msg.proof_height_on_b)?; let prefix_on_b = conn_end_on_a.counterparty().prefix(); let port_id_on_b = &chan_end_on_a.counterparty().port_id; let conn_id_on_b = conn_end_on_a.counterparty().connection_id().ok_or( diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs index c2f37710c..50fc55237 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -47,7 +47,7 @@ pub(crate) fn process( let client_id_on_b = conn_end_on_b.client_id().clone(); let client_state_of_a_on_b = ctx_b.client_state(&client_id_on_b)?; let consensus_state_of_a_on_b = - ctx_b.client_consensus_state(&client_id_on_b, msg.proof_height_on_a)?; + ctx_b.client_consensus_state(&client_id_on_b, &msg.proof_height_on_a)?; let prefix_on_a = conn_end_on_b.counterparty().prefix(); let port_id_on_a = &chan_end_on_b.counterparty().port_id; let chan_id_on_a = chan_end_on_b diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index 44f5d436a..847be25af 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -48,7 +48,7 @@ pub(crate) fn process( let client_id_on_b = conn_end_on_b.client_id().clone(); let client_state_of_a_on_b = ctx_b.client_state(&client_id_on_b)?; let consensus_state_of_a_on_b = - ctx_b.client_consensus_state(&client_id_on_b, msg.proof_height_on_a)?; + ctx_b.client_consensus_state(&client_id_on_b, &msg.proof_height_on_a)?; let prefix_on_a = conn_end_on_b.counterparty().prefix(); let port_id_on_a = &msg.chan_end_on_b.counterparty().port_id; let chan_id_on_a = msg diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index 698a40480..41cee63e3 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -120,7 +120,7 @@ pub fn process( let packet_rec = ctx.get_packet_receipt( &packet.destination_port, &packet.destination_channel, - packet.sequence, + &packet.sequence, ); match packet_rec { diff --git a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs index 81cae8f80..0cd039245 100644 --- a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs @@ -67,7 +67,7 @@ pub fn send_packet( }); } - let consensus_state = ctx.client_consensus_state(&client_id, latest_height)?; + let consensus_state = ctx.client_consensus_state(&client_id, &latest_height)?; let latest_timestamp = consensus_state.timestamp(); let packet_timestamp = packet.timeout_timestamp; if let Expiry::Expired = latest_timestamp.check_expiry(&packet_timestamp) { @@ -91,9 +91,9 @@ pub fn send_packet( seq: packet.sequence, seq_number: next_seq_send.increment(), commitment: ctx.packet_commitment( - packet.data.clone(), - packet.timeout_height, - packet.timeout_timestamp, + &packet.data, + &packet.timeout_height, + &packet.timeout_timestamp, ), }; diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout.rs b/crates/ibc/src/core/ics04_channel/handler/timeout.rs index b040dad31..0794a50ff 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout.rs @@ -74,7 +74,7 @@ pub fn process( } let consensus_state = ctx - .client_consensus_state(&client_id, proof_height) + .client_consensus_state(&client_id, &proof_height) .map_err(PacketError::Channel)?; let proof_timestamp = consensus_state.timestamp(); @@ -88,13 +88,16 @@ pub fn process( } //verify packet commitment - let packet_commitment = - ctx.get_packet_commitment(&packet.source_port, &packet.source_channel, packet.sequence)?; + let packet_commitment = ctx.get_packet_commitment( + &packet.source_port, + &packet.source_channel, + &packet.sequence, + )?; let expected_commitment = ctx.packet_commitment( - packet.data.clone(), - packet.timeout_height, - packet.timeout_timestamp, + &packet.data, + &packet.timeout_height, + &packet.timeout_timestamp, ); if packet_commitment != expected_commitment { return Err(PacketError::IncorrectPacketCommitment { @@ -215,9 +218,9 @@ mod tests { msg_ok.packet.timeout_timestamp = Default::default(); let data = context.packet_commitment( - msg_ok.packet.data.clone(), - msg_ok.packet.timeout_height, - msg_ok.packet.timeout_timestamp, + &msg_ok.packet.data, + &msg_ok.packet.timeout_height, + &msg_ok.packet.timeout_timestamp, ); let source_channel_end = ChannelEnd::new( diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs index 9b3036230..4ac4197b1 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs @@ -45,13 +45,16 @@ pub fn process( .map_err(PacketError::Channel)?; //verify the packet was sent, check the store - let packet_commitment = - ctx.get_packet_commitment(&packet.source_port, &packet.source_channel, packet.sequence)?; + let packet_commitment = ctx.get_packet_commitment( + &packet.source_port, + &packet.source_channel, + &packet.sequence, + )?; let expected_commitment = ctx.packet_commitment( - packet.data.clone(), - packet.timeout_height, - packet.timeout_timestamp, + &packet.data, + &packet.timeout_height, + &packet.timeout_timestamp, ); if packet_commitment != expected_commitment { return Err(PacketError::IncorrectPacketCommitment { @@ -206,9 +209,9 @@ mod tests { let packet = msg.packet.clone(); let data = context.packet_commitment( - msg.packet.data.clone(), - msg.packet.timeout_height, - msg.packet.timeout_timestamp, + &msg.packet.data, + &msg.packet.timeout_height, + &msg.packet.timeout_timestamp, ); let source_channel_end = ChannelEnd::new( diff --git a/crates/ibc/src/core/ics04_channel/handler/verify.rs b/crates/ibc/src/core/ics04_channel/handler/verify.rs index f52b8951c..90ec14794 100644 --- a/crates/ibc/src/core/ics04_channel/handler/verify.rs +++ b/crates/ibc/src/core/ics04_channel/handler/verify.rs @@ -27,7 +27,7 @@ pub fn verify_channel_proofs( return Err(ChannelError::FrozenClient { client_id }); } - let consensus_state = ctx.client_consensus_state(&client_id, proofs.height())?; + let consensus_state = ctx.client_consensus_state(&client_id, &proofs.height())?; // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. @@ -65,12 +65,12 @@ pub fn verify_packet_recv_proofs( }); } - let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + let consensus_state = ctx.client_consensus_state(client_id, &proofs.height())?; let commitment = ctx.packet_commitment( - packet.data.clone(), - packet.timeout_height, - packet.timeout_timestamp, + &packet.data, + &packet.timeout_height, + &packet.timeout_timestamp, ); // Verify the proof for the packet against the chain store. @@ -113,9 +113,9 @@ pub fn verify_packet_acknowledgement_proofs( }); } - let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + let consensus_state = ctx.client_consensus_state(client_id, &proofs.height())?; - let ack_commitment = ctx.ack_commitment(acknowledgement); + let ack_commitment = ctx.ack_commitment(&acknowledgement); // Verify the proof for the packet against the chain store. client_state @@ -157,7 +157,7 @@ pub fn verify_next_sequence_recv( }); } - let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + let consensus_state = ctx.client_consensus_state(client_id, &proofs.height())?; // Verify the proof for the packet against the chain store. client_state @@ -196,7 +196,7 @@ pub fn verify_packet_receipt_absence( }); } - let consensus_state = ctx.client_consensus_state(client_id, proofs.height())?; + let consensus_state = ctx.client_consensus_state(client_id, &proofs.height())?; // Verify the proof for the packet against the chain store. client_state diff --git a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs index 21b87e6db..fe57bbed5 100644 --- a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs @@ -43,7 +43,7 @@ pub fn process( match ctx.get_packet_acknowledgement( &packet.destination_port, &packet.destination_channel, - packet.sequence, + &packet.sequence, ) { Ok(_) => { return Err(PacketError::AcknowledgementExists { @@ -67,7 +67,7 @@ pub fn process( port_id: packet.destination_port.clone(), channel_id: packet.destination_channel.clone(), seq: packet.sequence, - ack_commitment: ctx.ack_commitment(ack.clone()), + ack_commitment: ctx.ack_commitment(&ack), }); output.log("success: packet write acknowledgement"); diff --git a/crates/ibc/src/core/ics26_routing/handler.rs b/crates/ibc/src/core/ics26_routing/handler.rs index 6b18ce877..4f4b33f43 100644 --- a/crates/ibc/src/core/ics26_routing/handler.rs +++ b/crates/ibc/src/core/ics26_routing/handler.rs @@ -524,7 +524,7 @@ mod tests { ctx.get_packet_commitment( &msg_ack_packet.packet.source_port, &msg_ack_packet.packet.source_channel, - msg_ack_packet.packet.sequence, + &msg_ack_packet.packet.sequence, ) .is_err() })), diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index 714d70388..8c1144b15 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -467,7 +467,7 @@ impl MockContext { /// Accessor for a block of the local (host) chain from this context. /// Returns `None` if the block at the requested height does not exist. - pub fn host_block(&self, target_height: Height) -> Option<&HostBlock> { + pub fn host_block(&self, target_height: &Height) -> Option<&HostBlock> { let target = target_height.revision_height() as usize; let latest = self.latest_height().revision_height() as usize; @@ -751,7 +751,7 @@ impl ChannelReader for MockContext { fn client_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, ChannelError> { ClientReader::consensus_state(self, client_id, height) .map_err(|e| ChannelError::Connection(ConnectionError::Client(e))) @@ -824,7 +824,7 @@ impl ChannelReader for MockContext { &self, port_id: &PortId, channel_id: &ChannelId, - seq: Sequence, + seq: &Sequence, ) -> Result { match self .ibc_store @@ -833,10 +833,10 @@ impl ChannelReader for MockContext { .packet_commitment .get(port_id) .and_then(|map| map.get(channel_id)) - .and_then(|map| map.get(&seq)) + .and_then(|map| map.get(seq)) { Some(commitment) => Ok(commitment.clone()), - None => Err(PacketError::PacketCommitmentNotFound { sequence: seq }), + None => Err(PacketError::PacketCommitmentNotFound { sequence: *seq }), } } @@ -844,7 +844,7 @@ impl ChannelReader for MockContext { &self, port_id: &PortId, channel_id: &ChannelId, - seq: Sequence, + seq: &Sequence, ) -> Result { match self .ibc_store @@ -853,10 +853,10 @@ impl ChannelReader for MockContext { .packet_receipt .get(port_id) .and_then(|map| map.get(channel_id)) - .and_then(|map| map.get(&seq)) + .and_then(|map| map.get(seq)) { Some(receipt) => Ok(receipt.clone()), - None => Err(PacketError::PacketReceiptNotFound { sequence: seq }), + None => Err(PacketError::PacketReceiptNotFound { sequence: *seq }), } } @@ -864,7 +864,7 @@ impl ChannelReader for MockContext { &self, port_id: &PortId, channel_id: &ChannelId, - seq: Sequence, + seq: &Sequence, ) -> Result { match self .ibc_store @@ -873,14 +873,14 @@ impl ChannelReader for MockContext { .packet_acknowledgement .get(port_id) .and_then(|map| map.get(channel_id)) - .and_then(|map| map.get(&seq)) + .and_then(|map| map.get(seq)) { Some(ack) => Ok(ack.clone()), - None => Err(PacketError::PacketAcknowledgementNotFound { sequence: seq }), + None => Err(PacketError::PacketAcknowledgementNotFound { sequence: *seq }), } } - fn hash(&self, value: Vec) -> Vec { + fn hash(&self, value: &[u8]) -> Vec { sha2::Sha256::digest(value).to_vec() } @@ -896,7 +896,7 @@ impl ChannelReader for MockContext { fn host_consensus_state( &self, - height: Height, + height: &Height, ) -> Result, ChannelError> { ConnectionReader::host_consensus_state(self, height).map_err(ChannelError::Connection) } @@ -909,19 +909,19 @@ impl ChannelReader for MockContext { fn client_update_time( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result { match self .ibc_store .lock() .unwrap() .client_processed_times - .get(&(client_id.clone(), height)) + .get(&(client_id.clone(), *height)) { Some(time) => Ok(*time), None => Err(ChannelError::ProcessedTimeNotFound { client_id: client_id.clone(), - height, + height: *height, }), } } @@ -929,19 +929,19 @@ impl ChannelReader for MockContext { fn client_update_height( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result { match self .ibc_store .lock() .unwrap() .client_processed_heights - .get(&(client_id.clone(), height)) + .get(&(client_id.clone(), *height)) { Some(height) => Ok(*height), None => Err(ChannelError::ProcessedHeightNotFound { client_id: client_id.clone(), - height, + height: *height, }), } } @@ -998,7 +998,7 @@ impl ChannelKeeper for MockContext { &mut self, port_id: &PortId, channel_id: &ChannelId, - seq: Sequence, + seq: &Sequence, ) -> Result<(), PacketError> { self.ibc_store .lock() @@ -1006,7 +1006,7 @@ impl ChannelKeeper for MockContext { .packet_acknowledgement .get_mut(port_id) .and_then(|map| map.get_mut(channel_id)) - .and_then(|map| map.remove(&seq)); + .and_then(|map| map.remove(seq)); Ok(()) } @@ -1098,7 +1098,7 @@ impl ChannelKeeper for MockContext { &mut self, port_id: &PortId, channel_id: &ChannelId, - seq: Sequence, + seq: &Sequence, ) -> Result<(), PacketError> { self.ibc_store .lock() @@ -1106,7 +1106,7 @@ impl ChannelKeeper for MockContext { .packet_commitment .get_mut(port_id) .and_then(|map| map.get_mut(channel_id)) - .and_then(|map| map.remove(&seq)); + .and_then(|map| map.remove(seq)); Ok(()) } @@ -1168,7 +1168,7 @@ impl ConnectionReader for MockContext { fn client_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, ConnectionError> { // Forward method call to the Ics2Client-specific method. ClientReader::consensus_state(self, client_id, height).map_err(ConnectionError::Client) @@ -1176,7 +1176,7 @@ impl ConnectionReader for MockContext { fn host_consensus_state( &self, - height: Height, + height: &Height, ) -> Result, ConnectionError> { ClientReader::host_consensus_state(self, height).map_err(ConnectionError::Client) } @@ -1194,26 +1194,26 @@ impl ConnectionKeeper for MockContext { fn store_connection( &mut self, connection_id: ConnectionId, - connection_end: &ConnectionEnd, + connection_end: ConnectionEnd, ) -> Result<(), ConnectionError> { self.ibc_store .lock() .unwrap() .connections - .insert(connection_id, connection_end.clone()); + .insert(connection_id, connection_end); Ok(()) } fn store_connection_to_client( &mut self, connection_id: ConnectionId, - client_id: &ClientId, + client_id: ClientId, ) -> Result<(), ConnectionError> { self.ibc_store .lock() .unwrap() .client_connections - .insert(client_id.clone(), connection_id); + .insert(client_id, connection_id); Ok(()) } @@ -1263,19 +1263,19 @@ impl ClientReader for MockContext { fn consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, ClientError> { match self.ibc_store.lock().unwrap().clients.get(client_id) { - Some(client_record) => match client_record.consensus_states.get(&height) { + Some(client_record) => match client_record.consensus_states.get(height) { Some(consensus_state) => Ok(consensus_state.clone()), None => Err(ClientError::ConsensusStateNotFound { client_id: client_id.clone(), - height, + height: *height, }), }, None => Err(ClientError::ConsensusStateNotFound { client_id: client_id.clone(), - height, + height: *height, }), } } @@ -1284,7 +1284,7 @@ impl ClientReader for MockContext { fn next_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result>, ClientError> { let ibc_store = self.ibc_store.lock().unwrap(); let client_record = @@ -1301,7 +1301,7 @@ impl ClientReader for MockContext { // Search for next state. for h in heights { - if h > height { + if h > *height { // unwrap should never happen, as the consensus state for h must exist return Ok(Some( client_record.consensus_states.get(&h).unwrap().clone(), @@ -1315,7 +1315,7 @@ impl ClientReader for MockContext { fn prev_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result>, ClientError> { let ibc_store = self.ibc_store.lock().unwrap(); let client_record = @@ -1332,7 +1332,7 @@ impl ClientReader for MockContext { // Search for previous state. for h in heights { - if h < height { + if h < *height { // unwrap should never happen, as the consensus state for h must exist return Ok(Some( client_record.consensus_states.get(&h).unwrap().clone(), @@ -1356,10 +1356,13 @@ impl ClientReader for MockContext { .unwrap()) } - fn host_consensus_state(&self, height: Height) -> Result, ClientError> { + fn host_consensus_state( + &self, + height: &Height, + ) -> Result, ClientError> { match self.host_block(height) { Some(block_ref) => Ok(block_ref.clone().into()), - None => Err(ClientError::MissingLocalConsensusState { height }), + None => Err(ClientError::MissingLocalConsensusState { height: *height }), } } @@ -1479,7 +1482,7 @@ impl RelayerContext for MockContext { } fn query_latest_header(&self) -> Option> { - let block_ref = self.host_block(self.host_current_height().unwrap()); + let block_ref = self.host_block(&self.host_current_height().unwrap()); block_ref.cloned().map(Header::into_box) } @@ -1820,7 +1823,7 @@ mod tests { ); assert_eq!( - test.ctx.host_block(current_height).unwrap().height(), + test.ctx.host_block(¤t_height).unwrap().height(), current_height, "failed while fetching height {:?} of context {:?}", current_height, diff --git a/crates/ibc/src/test_utils.rs b/crates/ibc/src/test_utils.rs index 71c892387..17ed4a37f 100644 --- a/crates/ibc/src/test_utils.rs +++ b/crates/ibc/src/test_utils.rs @@ -257,19 +257,19 @@ impl SendPacketReader for DummyTransferModule { fn client_consensus_state( &self, client_id: &ClientId, - height: Height, + height: &Height, ) -> Result, PacketError> { match self.ibc_store.lock().unwrap().clients.get(client_id) { - Some(client_record) => match client_record.consensus_states.get(&height) { + Some(client_record) => match client_record.consensus_states.get(height) { Some(consensus_state) => Ok(consensus_state.clone()), None => Err(ClientError::ConsensusStateNotFound { client_id: client_id.clone(), - height, + height: *height, }), }, None => Err(ClientError::ConsensusStateNotFound { client_id: client_id.clone(), - height, + height: *height, }), } .map_err(|e| PacketError::Connection(ConnectionError::Client(e))) @@ -296,7 +296,7 @@ impl SendPacketReader for DummyTransferModule { } } - fn hash(&self, value: Vec) -> Vec { + fn hash(&self, value: &[u8]) -> Vec { use sha2::Digest; sha2::Sha256::digest(value).to_vec()