imp(api)!: refactor client relevant APIs to allow independent ICS-02 integration (#1115)

* imp: refactor client-related APIs to allow independent ICS-02 integration

* chore: add unclog

* imp: keep self methods in ICS24 context + better signature for validate_self_client

* nit: revert naming to host_consensus_state

* imp: use client prelude for context relevant imports

* nit: rename to HostClient/ConsensusState

* imp: use trait bounds instead of ClientStateMut

* imp: move away from AnyConsensusState assoc type at ICS07

* docs: add a docstring for client_update_meta

* chore: further clean-ups

* fix: revert to the initial design approach using ClientStateMut

Author: Farhad-Shabani

.changelog/unreleased/breaking-changes/1114-refactor-client-relevant-apis.md

@@ -0,0 +1,3 @@
+- [ibc] Refactor client relevant APIs for improved modularity and allow
+  standalone ICS-02 integration
+  ([\#1114](https://github.com/cosmos/ibc-rs/issues/1114))

ci/no-std-check/Cargo.lock

@@ -1,25 +1,25 @@
 use ibc_client_tendermint_types::{
     ClientState as ClientStateType, ConsensusState as ConsensusStateType, Header as TmHeader,
 };
-use ibc_core_client::context::client_state::ClientStateExecution;
-use ibc_core_client::context::ClientExecutionContext;
+use ibc_core_client::context::prelude::*;
 use ibc_core_client::types::error::ClientError;
 use ibc_core_client::types::Height;
 use ibc_core_host::types::identifiers::ClientId;
 use ibc_core_host::types::path::{ClientConsensusStatePath, ClientStatePath};
-use ibc_core_host::ExecutionContext;
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
 
 use super::ClientState;
-use crate::consensus_state::ConsensusState as TmConsensusState;
-use crate::context::{CommonContext, ExecutionContext as TmExecutionContext};
+use crate::context::{
+    ConsensusStateConverter, ExecutionContext as TmExecutionContext,
+    ValidationContext as TmValidationContext,
+};
 
 impl<E> ClientStateExecution<E> for ClientState
 where
-    E: TmExecutionContext + ExecutionContext,
-    <E as ClientExecutionContext>::AnyClientState: From<ClientStateType>,
-    <E as ClientExecutionContext>::AnyConsensusState: From<ConsensusStateType>,
+    E: TmExecutionContext,
+    E::ClientStateRef: From<ClientStateType>,
+    E::ConsensusStateRef: ConsensusStateConverter,
 {
     fn initialise(
         &self,
@@ -77,12 +77,12 @@ pub fn initialise<E>(
     consensus_state: Any,
 ) -> Result<(), ClientError>
 where
-    E: TmExecutionContext + ExecutionContext,
-    <E as ClientExecutionContext>::AnyClientState: From<ClientStateType>,
-    <E as ClientExecutionContext>::AnyConsensusState: From<ConsensusStateType>,
+    E: TmExecutionContext,
+    E::ClientStateRef: From<ClientStateType>,
+    E::ConsensusStateRef: ConsensusStateConverter,
 {
-    let host_timestamp = CommonContext::host_timestamp(ctx)?;
-    let host_height = CommonContext::host_height(ctx)?;
+    let host_timestamp = TmValidationContext::host_timestamp(ctx)?;
+    let host_height = TmValidationContext::host_height(ctx)?;
 
     let tm_consensus_state = ConsensusStateType::try_from(consensus_state)?;
 
@@ -122,9 +122,9 @@ pub fn update_state<E>(
     header: Any,
 ) -> Result<Vec<Height>, ClientError>
 where
-    E: TmExecutionContext + ExecutionContext,
-    <E as ClientExecutionContext>::AnyClientState: From<ClientStateType>,
-    <E as ClientExecutionContext>::AnyConsensusState: From<ConsensusStateType>,
+    E: TmExecutionContext,
+    E::ClientStateRef: From<ClientStateType>,
+    E::ConsensusStateRef: ConsensusStateConverter,
 {
     let header = TmHeader::try_from(header)?;
     let header_height = header.height();
@@ -138,7 +138,7 @@ where
             header_height.revision_height(),
         );
 
-        CommonContext::consensus_state(ctx, &path_at_header_height).ok()
+        ctx.consensus_state(&path_at_header_height).ok()
     };
 
     if maybe_existing_consensus_state.is_some() {
@@ -147,8 +147,8 @@ where
         //
         // Do nothing.
     } else {
-        let host_timestamp = CommonContext::host_timestamp(ctx)?;
-        let host_height = CommonContext::host_height(ctx)?;
+        let host_timestamp = TmValidationContext::host_timestamp(ctx)?;
+        let host_height = TmValidationContext::host_height(ctx)?;
 
         let new_consensus_state = ConsensusStateType::from(header.clone());
         let new_client_state = client_state.clone().with_header(header)?;
@@ -189,9 +189,9 @@ pub fn update_on_misbehaviour<E>(
     _client_message: Any,
 ) -> Result<(), ClientError>
 where
-    E: TmExecutionContext + ExecutionContext,
-    <E as ClientExecutionContext>::AnyClientState: From<ClientStateType>,
-    <E as ClientExecutionContext>::AnyConsensusState: From<ConsensusStateType>,
+    E: TmExecutionContext,
+    E::ClientStateRef: From<ClientStateType>,
+    E::ConsensusStateRef: ConsensusStateConverter,
 {
     // NOTE: frozen height is  set to `Height {revision_height: 0,
     // revision_number: 1}` and it is the same for all misbehaviour. This
@@ -221,12 +221,12 @@ pub fn update_on_upgrade<E>(
     upgraded_consensus_state: Any,
 ) -> Result<Height, ClientError>
 where
-    E: TmExecutionContext + ExecutionContext,
-    <E as ClientExecutionContext>::AnyClientState: From<ClientStateType>,
-    <E as ClientExecutionContext>::AnyConsensusState: From<ConsensusStateType>,
+    E: TmExecutionContext,
+    E::ClientStateRef: From<ClientStateType>,
+    E::ConsensusStateRef: ConsensusStateConverter,
 {
     let mut upgraded_tm_client_state = ClientState::try_from(upgraded_client_state)?;
-    let upgraded_tm_cons_state = TmConsensusState::try_from(upgraded_consensus_state)?;
+    let upgraded_tm_cons_state = ConsensusStateType::try_from(upgraded_consensus_state)?;
 
     upgraded_tm_client_state.0.zero_custom_fields();
 
@@ -263,12 +263,12 @@ where
     let new_consensus_state = ConsensusStateType::new(
         sentinel_root.into(),
         upgraded_tm_cons_state.timestamp(),
-        upgraded_tm_cons_state.next_validators_hash(),
+        upgraded_tm_cons_state.next_validators_hash,
     );
 
     let latest_height = new_client_state.latest_height;
-    let host_timestamp = CommonContext::host_timestamp(ctx)?;
-    let host_height = CommonContext::host_height(ctx)?;
+    let host_timestamp = TmValidationContext::host_timestamp(ctx)?;
+    let host_height = TmValidationContext::host_height(ctx)?;
 
     ctx.store_client_state(
         ClientStatePath::new(client_id.clone()),
@@ -301,7 +301,9 @@ pub fn prune_oldest_consensus_state<E>(
     client_id: &ClientId,
 ) -> Result<(), ClientError>
 where
-    E: ClientExecutionContext + CommonContext,
+    E: ClientExecutionContext + TmValidationContext,
+    E::ClientStateRef: From<ClientStateType>,
+    E::ConsensusStateRef: ConsensusStateConverter,
 {
     let mut heights = ctx.consensus_state_heights(client_id)?;
 
@@ -313,12 +315,8 @@ where
             height.revision_number(),
             height.revision_height(),
         );
-        let consensus_state = CommonContext::consensus_state(ctx, &client_consensus_state_path)?;
-        let tm_consensus_state = consensus_state
-            .try_into()
-            .map_err(|err| ClientError::Other {
-                description: err.to_string(),
-            })?;
+        let consensus_state = ctx.consensus_state(&client_consensus_state_path)?;
+        let tm_consensus_state = consensus_state.try_into()?;
 
         let host_timestamp =
             ctx.host_timestamp()?

ibc-clients/ics07-tendermint/src/client_state/execution.rs

@@ -11,7 +11,7 @@ use ibc_primitives::Timestamp;
 use tendermint_light_client_verifier::Verifier;
 
 use super::TmValidationContext;
-use crate::context::TmVerifier;
+use crate::context::{ConsensusStateConverter, TmVerifier};
 
 /// Determines whether or not two conflicting headers at the same height would
 /// have convinced the light client.
@@ -24,6 +24,7 @@ pub fn verify_misbehaviour<V>(
 ) -> Result<(), ClientError>
 where
     V: TmValidationContext,
+    V::ConsensusStateRef: ConsensusStateConverter,
 {
     misbehaviour.validate_basic()?;
 
@@ -36,11 +37,7 @@ where
         );
         let consensus_state = ctx.consensus_state(&consensus_state_path)?;
 
-        consensus_state
-            .try_into()
-            .map_err(|err| ClientError::Other {
-                description: err.to_string(),
-            })?
+        consensus_state.try_into()?
     };
 
     let header_2 = misbehaviour.header2();
@@ -52,26 +49,22 @@ where
         );
         let consensus_state = ctx.consensus_state(&consensus_state_path)?;
 
-        consensus_state
-            .try_into()
-            .map_err(|err| ClientError::Other {
-                description: err.to_string(),
-            })?
+        consensus_state.try_into()?
     };
 
     let current_timestamp = ctx.host_timestamp()?;
 
     verify_misbehaviour_header(
         client_state,
         header_1,
-        trusted_consensus_state_1.inner(),
+        &trusted_consensus_state_1,
         current_timestamp,
         verifier,
     )?;
     verify_misbehaviour_header(
         client_state,
         header_2,
-        trusted_consensus_state_2.inner(),
+        &trusted_consensus_state_2,
         current_timestamp,
         verifier,
     )

ibc-clients/ics07-tendermint/src/client_state/misbehaviour.rs

@@ -9,8 +9,9 @@ use ibc_primitives::prelude::*;
 use tendermint_light_client_verifier::types::{TrustedBlockState, UntrustedBlockState};
 use tendermint_light_client_verifier::Verifier;
 
-use crate::consensus_state::ConsensusState as TmConsensusState;
-use crate::context::{TmVerifier, ValidationContext as TmValidationContext};
+use crate::context::{
+    ConsensusStateConverter, TmVerifier, ValidationContext as TmValidationContext,
+};
 
 pub fn verify_header<V>(
     client_state: &ClientStateType,
@@ -21,6 +22,7 @@ pub fn verify_header<V>(
 ) -> Result<(), ClientError>
 where
     V: TmValidationContext,
+    V::ConsensusStateRef: ConsensusStateConverter,
 {
     // Checks that the header fields are valid.
     header.validate_basic()?;
@@ -38,14 +40,11 @@ where
                 header.trusted_height.revision_number(),
                 header.trusted_height.revision_height(),
             );
-            let trusted_consensus_state: TmConsensusState = ctx
+            let trusted_consensus_state = ctx
                 .consensus_state(&trusted_client_cons_state_path)?
-                .try_into()
-                .map_err(|err| ClientError::Other {
-                    description: err.to_string(),
-                })?;
+                .try_into()?;
 
-            header.check_trusted_next_validator_set(trusted_consensus_state.inner())?;
+            header.check_trusted_next_validator_set(&trusted_consensus_state)?;
 
             TrustedBlockState {
                 chain_id: &client_state.chain_id.to_string().try_into().map_err(|e| {
@@ -65,7 +64,7 @@ where
                         .to_string(),
                     })?,
                 next_validators: &header.trusted_next_validator_set,
-                next_validators_hash: trusted_consensus_state.next_validators_hash(),
+                next_validators_hash: trusted_consensus_state.next_validators_hash,
             }
         };
 
@@ -106,6 +105,7 @@ pub fn check_for_misbehaviour_update_client<V>(
 ) -> Result<bool, ClientError>
 where
     V: TmValidationContext,
+    V::ConsensusStateRef: ConsensusStateConverter,
 {
     let maybe_existing_consensus_state = {
         let path_at_header_height = ClientConsensusStatePath::new(
@@ -119,15 +119,9 @@ where
 
     match maybe_existing_consensus_state {
         Some(existing_consensus_state) => {
-            let existing_consensus_state =
-                existing_consensus_state
-                    .try_into()
-                    .map_err(|err| ClientError::Other {
-                        description: err.to_string(),
-                    })?;
+            let existing_consensus_state = existing_consensus_state.try_into()?;
 
-            let header_consensus_state =
-                TmConsensusState::from(ConsensusStateType::from(header.clone()));
+            let header_consensus_state = ConsensusStateType::from(header.clone());
 
             // There is evidence of misbehaviour if the stored consensus state
             // is different from the new one we received.
@@ -144,10 +138,7 @@ where
                 if let Some(prev_cs) = maybe_prev_cs {
                     // New header timestamp cannot occur *before* the
                     // previous consensus state's height
-                    let prev_cs: TmConsensusState =
-                        prev_cs.try_into().map_err(|err| ClientError::Other {
-                            description: err.to_string(),
-                        })?;
+                    let prev_cs = prev_cs.try_into()?;
 
                     if header.signed_header.header().time <= prev_cs.timestamp() {
                         return Ok(true);
@@ -163,9 +154,7 @@ where
                 if let Some(next_cs) = maybe_next_cs {
                     // New (untrusted) header timestamp cannot occur *after* next
                     // consensus state's height
-                    let next_cs = next_cs.try_into().map_err(|err| ClientError::Other {
-                        description: err.to_string(),
-                    })?;
+                    let next_cs = next_cs.try_into()?;
 
                     if header.signed_header.header().time >= next_cs.timestamp() {
                         return Ok(true);