Cleanup getters in client, connection and channel messages (#1860)

* remove getters from client and connection messages

* remove getters in channel messages

* fix clippy errors

* clippy fixes

modules/src/core/ics02_client/handler/create_client.rs

@@ -36,8 +36,8 @@ pub fn process(
 
     // Construct this client's identifier
     let id_counter = ctx.client_counter()?;
-    let client_id = ClientId::new(msg.client_state().client_type(), id_counter).map_err(|e| {
-        Error::client_identifier_constructor(msg.client_state().client_type(), id_counter, e)
+    let client_id = ClientId::new(msg.client_state.client_type(), id_counter).map_err(|e| {
+        Error::client_identifier_constructor(msg.client_state.client_type(), id_counter, e)
     })?;
 
     output.log(format!(
@@ -47,9 +47,9 @@ pub fn process(
 
     let result = ClientResult::Create(Result {
         client_id: client_id.clone(),
-        client_type: msg.client_state().client_type(),
-        client_state: msg.client_state(),
-        consensus_state: msg.consensus_state(),
+        client_type: msg.client_state.client_type(),
+        client_state: msg.client_state.clone(),
+        consensus_state: msg.consensus_state,
         processed_time: ctx.host_timestamp(),
         processed_height: ctx.host_height(),
     });
@@ -120,8 +120,8 @@ mod tests {
                     ClientResult::Create(create_result) => {
                         assert_eq!(create_result.client_type, ClientType::Mock);
                         assert_eq!(create_result.client_id, expected_client_id);
-                        assert_eq!(create_result.client_state, msg.client_state());
-                        assert_eq!(create_result.consensus_state, msg.consensus_state());
+                        assert_eq!(create_result.client_state, msg.client_state);
+                        assert_eq!(create_result.consensus_state, msg.consensus_state);
                     }
                     _ => {
                         panic!("unexpected result type: expected ClientResult::CreateResult!");
@@ -208,10 +208,10 @@ mod tests {
                     );
                     match result {
                         ClientResult::Create(create_res) => {
-                            assert_eq!(create_res.client_type, msg.client_state().client_type());
+                            assert_eq!(create_res.client_type, msg.client_state.client_type());
                             assert_eq!(create_res.client_id, expected_client_id);
-                            assert_eq!(create_res.client_state, msg.client_state());
-                            assert_eq!(create_res.consensus_state, msg.consensus_state());
+                            assert_eq!(create_res.client_state, msg.client_state);
+                            assert_eq!(create_res.consensus_state, msg.consensus_state);
                         }
                         _ => {
                             panic!("expected result of type ClientResult::CreateResult");
@@ -273,8 +273,8 @@ mod tests {
                     ClientResult::Create(create_res) => {
                         assert_eq!(create_res.client_type, ClientType::Tendermint);
                         assert_eq!(create_res.client_id, expected_client_id);
-                        assert_eq!(create_res.client_state, msg.client_state());
-                        assert_eq!(create_res.consensus_state, msg.consensus_state());
+                        assert_eq!(create_res.client_state, msg.client_state);
+                        assert_eq!(create_res.consensus_state, msg.consensus_state);
                     }
                     _ => {
                         panic!("expected result of type ClientResult::CreateResult");

modules/src/core/ics02_client/msgs/create_client.rs

@@ -40,14 +40,6 @@ impl MsgCreateAnyClient {
             signer,
         })
     }
-
-    pub fn client_state(&self) -> AnyClientState {
-        self.client_state.clone()
-    }
-
-    pub fn consensus_state(&self) -> AnyConsensusState {
-        self.consensus_state.clone()
-    }
 }
 
 impl Msg for MsgCreateAnyClient {

modules/src/core/ics03_connection/handler/conn_open_ack.rs

@@ -23,59 +23,59 @@ pub(crate) fn process(
     check_client_consensus_height(ctx, msg.consensus_height())?;
 
     // Validate the connection end.
-    let mut conn_end = ctx.connection_end(msg.connection_id())?;
+    let mut conn_end = ctx.connection_end(&msg.connection_id)?;
     // A connection end must be Init or TryOpen; otherwise we return an error.
     let state_is_consistent = conn_end.state_matches(&State::Init)
-        && conn_end.versions().contains(msg.version())
+        && conn_end.versions().contains(&msg.version)
         || conn_end.state_matches(&State::TryOpen)
-            && conn_end.versions().get(0).eq(&Some(msg.version()));
+            && conn_end.versions().get(0).eq(&Some(&msg.version));
 
     if !state_is_consistent {
         // Old connection end is in incorrect state, propagate the error.
-        return Err(Error::connection_mismatch(msg.connection_id().clone()));
+        return Err(Error::connection_mismatch(msg.connection_id));
     }
 
     // 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()),
+        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_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.
-        Some(msg.counterparty_connection_id().clone()), // This chain's connection id as known on counterparty.
-        ctx.commitment_prefix(),                        // Local commitment prefix.
+        Some(msg.counterparty_connection_id.clone()), // This chain's connection id as known on counterparty.
+        ctx.commitment_prefix(),                      // Local commitment prefix.
     );
 
     // Proof verification.
     let expected_conn = ConnectionEnd::new(
         State::TryOpen,
         conn_end.counterparty().client_id().clone(),
         counterparty,
-        vec![msg.version().clone()],
+        vec![msg.version.clone()],
         conn_end.delay_period(),
     );
 
     // 2. Pass the details to the verification function.
     verify_proofs(
         ctx,
-        msg.client_state(),
-        msg.proofs().height(),
+        msg.client_state.clone(),
+        msg.proofs.height(),
         &conn_end,
         &expected_conn,
-        msg.proofs(),
+        &msg.proofs,
     )?;
 
     output.log("success: connection verification passed");
 
     let result = ConnectionResult {
-        connection_id: msg.connection_id().clone(),
+        connection_id: msg.connection_id,
         connection_id_state: ConnectionIdState::Reused,
         connection_end: conn_end,
     };
@@ -144,10 +144,10 @@ mod tests {
             client_id.clone(),
             Counterparty::new(
                 client_id.clone(),
-                Some(msg_ack.counterparty_connection_id().clone()),
+                Some(msg_ack.counterparty_connection_id.clone()),
                 CommitmentPrefix::try_from(b"ibc".to_vec()).unwrap(),
             ),
-            vec![msg_ack.version().clone()],
+            vec![msg_ack.version.clone()],
             ZERO_DURATION,
         );
 

modules/src/core/ics03_connection/handler/conn_open_confirm.rs

@@ -18,11 +18,11 @@ pub(crate) fn process(
     let mut output = HandlerOutput::builder();
 
     // Validate the connection end.
-    let mut conn_end = ctx.connection_end(msg.connection_id())?;
+    let mut conn_end = ctx.connection_end(&msg.connection_id)?;
     // A connection end must be in TryOpen state; otherwise return error.
     if !conn_end.state_matches(&State::TryOpen) {
         // Old connection end is in incorrect state, propagate the error.
-        return Err(Error::connection_mismatch(msg.connection_id().clone()));
+        return Err(Error::connection_mismatch(msg.connection_id));
     }
 
     // Verify proofs. Assemble the connection end as we expect to find it on the counterparty.
@@ -32,7 +32,7 @@ pub(crate) fn process(
         Counterparty::new(
             // The counterparty is the local chain.
             conn_end.client_id().clone(), // The local client identifier.
-            Some(msg.connection_id().clone()), // Local connection id.
+            Some(msg.connection_id.clone()), // Local connection id.
             ctx.commitment_prefix(),      // Local commitment prefix.
         ),
         conn_end.versions(),
@@ -43,10 +43,10 @@ pub(crate) fn process(
     verify_proofs(
         ctx,
         None,
-        msg.proofs().height(),
+        msg.proofs.height(),
         &conn_end,
         &expected_conn,
-        msg.proofs(),
+        &msg.proofs,
     )?;
 
     output.log("success: connection verification passed");
@@ -55,7 +55,7 @@ pub(crate) fn process(
     conn_end.set_state(State::Open);
 
     let result = ConnectionResult {
-        connection_id: msg.connection_id().clone(),
+        connection_id: msg.connection_id,
         connection_id_state: ConnectionIdState::Reused,
         connection_end: conn_end,
     };
@@ -103,7 +103,7 @@ mod tests {
             MsgConnectionOpenConfirm::try_from(get_dummy_raw_msg_conn_open_confirm()).unwrap();
         let counterparty = Counterparty::new(
             client_id.clone(),
-            Some(msg_confirm.connection_id().clone()),
+            Some(msg_confirm.connection_id.clone()),
             CommitmentPrefix::try_from(b"ibc".to_vec()).unwrap(),
         );
 
@@ -132,19 +132,16 @@ mod tests {
                 ctx: context
                     .clone()
                     .with_client(&client_id, Height::new(0, 10))
-                    .with_connection(
-                        msg_confirm.connection_id().clone(),
-                        incorrect_conn_end_state,
-                    ),
+                    .with_connection(msg_confirm.connection_id.clone(), incorrect_conn_end_state),
                 msg: ConnectionMsg::ConnectionOpenConfirm(msg_confirm.clone()),
                 want_pass: false,
             },
             Test {
                 name: "Processing successful".to_string(),
                 ctx: context
                     .with_client(&client_id, Height::new(0, 10))
-                    .with_connection(msg_confirm.connection_id().clone(), correct_conn_end),
-                msg: ConnectionMsg::ConnectionOpenConfirm(msg_confirm.clone()),
+                    .with_connection(msg_confirm.connection_id.clone(), correct_conn_end),
+                msg: ConnectionMsg::ConnectionOpenConfirm(msg_confirm),
                 want_pass: true,
             },
         ]

modules/src/core/ics03_connection/handler/conn_open_init.rs

@@ -18,12 +18,12 @@ pub(crate) fn process(
     let mut output = HandlerOutput::builder();
 
     // An IBC client running on the local (host) chain should exist.
-    ctx.client_state(msg.client_id())?;
+    ctx.client_state(&msg.client_id)?;
 
     let new_connection_end = ConnectionEnd::new(
         State::Init,
-        msg.client_id().clone(),
-        msg.counterparty().clone(),
+        msg.client_id.clone(),
+        msg.counterparty.clone(),
         ctx.get_compatible_versions(),
         msg.delay_period,
     );
@@ -89,7 +89,7 @@ mod tests {
             },
             Test {
                 name: "Good parameters".to_string(),
-                ctx: context.with_client(msg_conn_init.client_id(), Height::new(0, 10)),
+                ctx: context.with_client(&msg_conn_init.client_id, Height::new(0, 10)),
                 msg: ConnectionMsg::ConnectionOpenInit(msg_conn_init.clone()),
                 want_pass: true,
             },

modules/src/core/ics03_connection/handler/conn_open_try.rs

@@ -24,15 +24,15 @@ pub(crate) fn process(
     check_client_consensus_height(ctx, msg.consensus_height())?;
 
     // Unwrap the old connection end (if any) and its identifier.
-    let (mut new_connection_end, conn_id) = match msg.previous_connection_id() {
+    let (mut new_connection_end, conn_id) = match &msg.previous_connection_id {
         // A connection with this id should already exist. Search & validate.
         Some(prev_id) => {
             let old_connection_end = ctx.connection_end(prev_id)?;
 
             // Validate that existing connection end matches with the one we're trying to establish.
             if old_connection_end.state_matches(&State::Init)
-                && old_connection_end.counterparty_matches(&msg.counterparty())
-                && old_connection_end.client_id_matches(msg.client_id())
+                && old_connection_end.counterparty_matches(&msg.counterparty)
+                && old_connection_end.client_id_matches(&msg.client_id)
                 && old_connection_end.delay_period() == msg.delay_period
             {
                 // A ConnectionEnd already exists and all validation passed.
@@ -51,9 +51,9 @@ pub(crate) fn process(
             // Build a new connection end as well as an identifier.
             let conn_end = ConnectionEnd::new(
                 State::Init,
-                msg.client_id().clone(),
-                msg.counterparty(),
-                msg.counterparty_versions(),
+                msg.client_id.clone(),
+                msg.counterparty.clone(),
+                msg.counterparty_versions.clone(),
                 msg.delay_period,
             );
             let id_counter = ctx.connection_counter()?;
@@ -71,28 +71,30 @@ pub(crate) fn process(
     // 1. Setup: build the ConnectionEnd as we expect to find it on the other party.
     let expected_conn = ConnectionEnd::new(
         State::Init,
-        msg.counterparty().client_id().clone(),
-        Counterparty::new(msg.client_id().clone(), None, ctx.commitment_prefix()),
-        msg.counterparty_versions(),
+        msg.counterparty.client_id().clone(),
+        Counterparty::new(msg.client_id.clone(), None, ctx.commitment_prefix()),
+        msg.counterparty_versions.clone(),
         msg.delay_period,
     );
 
     // 2. Pass the details to the verification function.
     verify_proofs(
         ctx,
-        msg.client_state(),
-        msg.proofs().height(),
+        msg.client_state.clone(),
+        msg.proofs.height(),
         &new_connection_end,
         &expected_conn,
-        msg.proofs(),
+        &msg.proofs,
     )?;
 
     // Transition the connection end to the new state & pick a version.
     new_connection_end.set_state(State::TryOpen);
 
     // Pick the version.
-    new_connection_end
-        .set_version(ctx.pick_version(ctx.get_compatible_versions(), msg.counterparty_versions())?);
+    new_connection_end.set_version(ctx.pick_version(
+        ctx.get_compatible_versions(),
+        msg.counterparty_versions.clone(),
+    )?);
 
     assert_eq!(new_connection_end.versions().len(), 1);
 
@@ -205,19 +207,19 @@ mod tests {
             },
             Test {
                 name: "Processing fails because the client misses the consensus state targeted by the proof".to_string(),
-                ctx: context.clone().with_client(msg_proof_height_missing.client_id(), Height::new(0, client_consensus_state_height)),
+                ctx: context.clone().with_client(&msg_proof_height_missing.client_id, Height::new(0, client_consensus_state_height)),
                 msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_proof_height_missing)),
                 want_pass: false,
             },
             Test {
                 name: "Good parameters but has previous_connection_id".to_string(),
-                ctx: context.clone().with_client(msg_conn_try.client_id(), Height::new(0, client_consensus_state_height)),
+                ctx: context.clone().with_client(&msg_conn_try.client_id, Height::new(0, client_consensus_state_height)),
                 msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_conn_try.clone())),
                 want_pass: false,
             },
             Test {
                 name: "Good parameters".to_string(),
-                ctx: context.with_client(msg_conn_try.client_id(), Height::new(0, client_consensus_state_height)),
+                ctx: context.with_client(&msg_conn_try.client_id, Height::new(0, client_consensus_state_height)),
                 msg: ConnectionMsg::ConnectionOpenTry(Box::new(msg_conn_try.with_previous_connection_id(None))),
                 want_pass: true,
             },

modules/src/core/ics03_connection/msgs/conn_open_ack.rs

@@ -27,31 +27,6 @@ pub struct MsgConnectionOpenAck {
 }
 
 impl MsgConnectionOpenAck {
-    /// Getter for accessing the connection identifier of this message.
-    pub fn connection_id(&self) -> &ConnectionId {
-        &self.connection_id
-    }
-
-    /// Getter for accessing the counterparty's connection identifier from this message.
-    pub fn counterparty_connection_id(&self) -> &ConnectionId {
-        &self.counterparty_connection_id
-    }
-
-    /// Getter for accessing the client state.
-    pub fn client_state(&self) -> Option<AnyClientState> {
-        self.client_state.clone()
-    }
-
-    /// Getter for accessing (borrow) the proofs in this message.
-    pub fn proofs(&self) -> &Proofs {
-        &self.proofs
-    }
-
-    /// Getter for the version field.
-    pub fn version(&self) -> &Version {
-        &self.version
-    }
-
     /// Getter for accessing the `consensus_height` field from this message. Returns the special
     /// value `Height(0)` if this field is not set.
     pub fn consensus_height(&self) -> Height {