Fix the function parameters in the Reader trait should be used with t…

…he 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

.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))

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<Option<Box<dyn ConsensusState>>, 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,20 +1144,20 @@ 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,
         }
     })?;
 
     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,

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<Box<dyn ConsensusState>, ClientError>;
 
     /// Search for the lowest consensus state higher than `height`.
     fn next_consensus_state(
         &self,
         client_id: &ClientId,
-        height: Height,
+        height: &Height,
     ) -> Result<Option<Box<dyn ConsensusState>>, ClientError>;
 
     /// Search for the highest consensus state lower than `height`.
     fn prev_consensus_state(
         &self,
         client_id: &ClientId,
-        height: Height,
+        height: &Height,
     ) -> Result<Option<Box<dyn ConsensusState>>, 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<Box<dyn ConsensusState>, ClientError>;
+    fn host_consensus_state(&self, height: &Height)
+        -> Result<Box<dyn ConsensusState>, ClientError>;
 
     /// Returns the pending `ConsensusState` of the host (local) chain.
     fn pending_host_consensus_state(&self) -> Result<Box<dyn ConsensusState>, ClientError>;

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()
         };

crates/ibc/src/core/ics02_client/handler/update_client.rs

@@ -165,7 +165,7 @@ pub fn process<Ctx: ClientReader>(
 
     // 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,

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<Box<dyn ConsensusState>, ConnectionError>;
 
     /// Returns the ConsensusState of the host (local) chain at a specific height.
     fn host_consensus_state(
         &self,
-        height: Height,
+        height: &Height,
     ) -> Result<Box<dyn ConsensusState>, 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<Version>,
-        counterparty_candidate_versions: Vec<Version>,
+        supported_versions: &[Version],
+        counterparty_candidate_versions: &[Version],
     ) -> Result<Version, ConnectionError> {
         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).

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,

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();

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,

crates/ibc/src/core/ics03_connection/version.rs

@@ -97,8 +97,8 @@ pub fn get_compatible_versions() -> Vec<Version> {
 
 /// Selects a version from the intersection of locally supported and counterparty versions.
 pub fn pick_version(
-    supported_versions: Vec<Version>,
-    counterparty_versions: Vec<Version>,
+    supported_versions: &[Version],
+    counterparty_versions: &[Version],
 ) -> Result<Version, ConnectionError> {
     let mut intersection: Vec<Version> = 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,