Add a docstring and rename the "validate_self_client" argument for be…

…tter (#436) * Add docstring and rename the validate_self_client argument * Rename to host_client_state_on_counterparty * Fix mock arg * Rename ValidateSelfClientContext trait * Reword validate_self_client docstring Co-authored-by: Philippe Laferrière <[email protected]> Signed-off-by: Farhad Shabani <[email protected]> * Apply comments * Rename validate_self_tendermint_client arg --------- Signed-off-by: Farhad Shabani <[email protected]> Co-authored-by: Philippe Laferrière <[email protected]>
cosmos · Feb 17, 2023 · 38864f1 · 38864f1
1 parent 777b1fe
commit 38864f1
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 28 deletions.
diff --git a/.changelog/unreleased/improvements/434-doc-validate-self-client.md b/.changelog/unreleased/improvements/434-doc-validate-self-client.md
@@ -0,0 +1,3 @@
+- Add a docstring and rename the "validate_self_client" argument for improved
+  code documentation and readability
+  ([#434](https://github.com/cosmos/ibc-rs/issues/434))
diff --git a/crates/ibc/src/clients/ics07_tendermint/mod.rs b/crates/ibc/src/clients/ics07_tendermint/mod.rs
@@ -9,7 +9,6 @@ pub mod client_state;
 pub mod consensus_state;
 pub mod error;
 pub mod header;
-pub mod host_helpers;
 pub mod misbehaviour;
 
 pub(crate) const TENDERMINT_CLIENT_TYPE: &str = "07-tendermint";

diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs
@@ -292,8 +292,18 @@ pub trait ValidationContext: Router {
     /// Returns the ConnectionEnd for the given identifier `conn_id`.
     fn connection_end(&self, conn_id: &ConnectionId) -> Result<ConnectionEnd, ContextError>;
 
-    /// Validates the `ClientState` of the client on the counterparty chain.
-    fn validate_self_client(&self, counterparty_client_state: Any) -> Result<(), ConnectionError>;
+    /// Validates the `ClientState` of the client (a client referring to host) stored on the counterparty chain against the host's internal state.
+    ///
+    /// For more information on the specific requirements for validating the
+    /// client state of a host chain, please refer to the [ICS24 host
+    /// requirements](https://github.com/cosmos/ibc/tree/main/spec/core/ics-024-host-requirements#client-state-validation)
+    ///
+    /// Additionally, implementations specific to individual chains can be found
+    /// in the [hosts](crate::hosts) module.
+    fn validate_self_client(
+        &self,
+        client_state_of_host_on_counterparty: Any,
+    ) -> Result<(), ConnectionError>;
 
     /// Returns the prefix that the local chain uses in the KV store.
     fn commitment_prefix(&self) -> CommitmentPrefix;

diff --git a/crates/ibc/src/core/ics03_connection/context.rs b/crates/ibc/src/core/ics03_connection/context.rs
@@ -74,8 +74,19 @@ pub trait ConnectionReader {
     /// `ConnectionKeeper::increase_connection_counter`.
     fn connection_counter(&self) -> Result<u64, ConnectionError>;
 
-    /// Validates the `ClientState` of the client on the counterparty chain.
-    fn validate_self_client(&self, counterparty_client_state: Any) -> Result<(), ConnectionError>;
+    /// Validates the client state of the host chain that is stored on the
+    /// counterparty chain.
+    ///
+    /// For more information on the specific requirements for validating the
+    /// client state of a host chain, please refer to the [ICS24 host
+    /// requirements](https://github.com/cosmos/ibc/tree/main/spec/core/ics-024-host-requirements#client-state-validation)
+    ///
+    /// Additionally, implementations specific to individual chains can be found
+    /// in the [hosts](crate::hosts) module.
+    fn validate_self_client(
+        &self,
+        client_state_of_host_on_counterparty: Any,
+    ) -> Result<(), ConnectionError>;
 }
 
 /// A context supplying all the necessary write-only dependencies (i.e., storage writing facility)

diff --git a/crates/ibc/src/hosts/mod.rs b/crates/ibc/src/hosts/mod.rs
@@ -0,0 +1 @@
+pub mod tendermint;
diff --git a/.../clients/ics07_tendermint/host_helpers.rs → crates/ibc/src/hosts/tendermint.rs b/.../clients/ics07_tendermint/host_helpers.rs → crates/ibc/src/hosts/tendermint.rs
@@ -16,61 +16,64 @@ use tendermint::trust_threshold::TrustThresholdFraction as TendermintTrustThresh
 /// Provides an implementation of `ValidationContext::validate_self_client` for
 /// Tendermint-based hosts.
 pub trait ValidateSelfClientContext {
-    fn validate_self_client(&self, counterparty_client_state: Any) -> Result<(), ConnectionError> {
-        let counterparty_client_state = TmClientState::try_from(counterparty_client_state)
+    fn validate_self_tendermint_client(
+        &self,
+        client_state_of_host_on_counterparty: Any,
+    ) -> Result<(), ConnectionError> {
+        let tm_client_state = TmClientState::try_from(client_state_of_host_on_counterparty)
             .map_err(|_| ConnectionError::InvalidClientState {
                 reason: "client must be a tendermint client".to_string(),
             })?;
 
-        if counterparty_client_state.is_frozen() {
+        if tm_client_state.is_frozen() {
             return Err(ConnectionError::InvalidClientState {
                 reason: "client is frozen".to_string(),
             });
         }
 
         let self_chain_id = self.chain_id();
-        if self_chain_id != &counterparty_client_state.chain_id {
+        if self_chain_id != &tm_client_state.chain_id {
             return Err(ConnectionError::InvalidClientState {
                 reason: format!(
                     "invalid chain-id. expected: {}, got: {}",
-                    self_chain_id, counterparty_client_state.chain_id
+                    self_chain_id, tm_client_state.chain_id
                 ),
             });
         }
 
         let self_revision_number = self_chain_id.version();
-        if self_revision_number != counterparty_client_state.latest_height().revision_number() {
+        if self_revision_number != tm_client_state.latest_height().revision_number() {
             return Err(ConnectionError::InvalidClientState {
                 reason: format!(
                     "client is not in the same revision as the chain. expected: {}, got: {}",
                     self_revision_number,
-                    counterparty_client_state.latest_height().revision_number()
+                    tm_client_state.latest_height().revision_number()
                 ),
             });
         }
 
-        if counterparty_client_state.latest_height() >= self.host_current_height() {
+        if tm_client_state.latest_height() >= self.host_current_height() {
             return Err(ConnectionError::InvalidClientState {
                 reason: format!(
                     "client has latest height {} greater than or equal to chain height {}",
-                    counterparty_client_state.latest_height(),
+                    tm_client_state.latest_height(),
                     self.host_current_height()
                 ),
             });
         }
 
-        if self.proof_specs() != &counterparty_client_state.proof_specs {
+        if self.proof_specs() != &tm_client_state.proof_specs {
             return Err(ConnectionError::InvalidClientState {
                 reason: format!(
                     "client has invalid proof specs. expected: {:?}, got: {:?}",
                     self.proof_specs(),
-                    counterparty_client_state.proof_specs
+                    tm_client_state.proof_specs
                 ),
             });
         }
 
         let _ = {
-            let trust_level = counterparty_client_state.trust_level;
+            let trust_level = tm_client_state.trust_level;
 
             TendermintTrustThresholdFraction::new(
                 trust_level.numerator(),
@@ -81,32 +84,32 @@ pub trait ValidateSelfClientContext {
             })?
         };
 
-        if self.unbonding_period() != counterparty_client_state.unbonding_period {
+        if self.unbonding_period() != tm_client_state.unbonding_period {
             return Err(ConnectionError::InvalidClientState {
                 reason: format!(
                     "invalid unbonding period. expected: {:?}, got: {:?}",
                     self.unbonding_period(),
-                    counterparty_client_state.unbonding_period,
+                    tm_client_state.unbonding_period,
                 ),
             });
         }
 
-        if counterparty_client_state.unbonding_period < counterparty_client_state.trusting_period {
+        if tm_client_state.unbonding_period < tm_client_state.trusting_period {
             return Err(ConnectionError::InvalidClientState{ reason: format!(
                 "unbonding period must be greater than trusting period. unbonding period ({:?}) < trusting period ({:?})",
-                counterparty_client_state.unbonding_period,
-                counterparty_client_state.trusting_period
+                tm_client_state.unbonding_period,
+                tm_client_state.trusting_period
             )});
         }
 
-        if !counterparty_client_state.upgrade_path.is_empty()
-            && self.upgrade_path() != counterparty_client_state.upgrade_path
+        if !tm_client_state.upgrade_path.is_empty()
+            && self.upgrade_path() != tm_client_state.upgrade_path
         {
             return Err(ConnectionError::InvalidClientState {
                 reason: format!(
                     "invalid upgrade path. expected: {:?}, got: {:?}",
                     self.upgrade_path(),
-                    counterparty_client_state.upgrade_path
+                    tm_client_state.upgrade_path
                 ),
             });
         }
@@ -126,6 +129,6 @@ pub trait ValidateSelfClientContext {
     /// Returns the host unbonding period
     fn unbonding_period(&self) -> Duration;
 
-    /// Returns the host uprade path. May be empty.
+    /// Returns the host upgrade path. May be empty.
     fn upgrade_path(&self) -> &[String];
 }
diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs
@@ -53,6 +53,7 @@ pub mod dynamic_typing;
 mod erased;
 pub mod events;
 pub mod handler;
+pub mod hosts;
 pub mod signer;
 pub mod timestamp;
 pub mod tx_msg;

diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs
@@ -1159,7 +1159,10 @@ impl ConnectionReader for MockContext {
         Ok(self.ibc_store.lock().connection_ids_counter)
     }
 
-    fn validate_self_client(&self, _counterparty_client_state: Any) -> Result<(), ConnectionError> {
+    fn validate_self_client(
+        &self,
+        _client_state_of_host_on_counterparty: Any,
+    ) -> Result<(), ConnectionError> {
         Ok(())
     }
 }
@@ -1659,7 +1662,10 @@ impl ValidationContext for MockContext {
         .map_err(ContextError::ConnectionError)
     }
 
-    fn validate_self_client(&self, _counterparty_client_state: Any) -> Result<(), ConnectionError> {
+    fn validate_self_client(
+        &self,
+        _client_state_of_host_on_counterparty: Any,
+    ) -> Result<(), ConnectionError> {
         Ok(())
     }