fix(ibc-core): copy substitute consensus state during client recovery (…

…#1197) * Pass substitute consensus state into client recovery methods * Fix update_on_recovery call in ibc-clients/cw-context * refactor encode_vec to into_any * consume new_client_state later * update ibc_derive * update mock client state * cargo format * Change `use` statement formatting * add store_update_meta at MockClientState::initialise * consistent naming * Remove `into_any` trait methods * rm ConsensusState::into_any from ibc-derive * test consensus state recovery * link on doc strings --------- Co-authored-by: Ranadeep Biswas <[email protected]>
cosmos · Apr 26, 2024 · 6deec18 · 6deec18
1 parent 7e50323
commit 6deec18
Show file tree

Hide file tree

Showing 13 changed files with 82 additions and 31 deletions.
diff --git a/ibc-clients/cw-context/src/handlers.rs b/ibc-clients/cw-context/src/handlers.rs
@@ -126,6 +126,12 @@ impl<'a, C: ClientType<'a>> Context<'a, C> {
             SudoMsg::MigrateClientStore(_) => {
                 self.set_substitute_prefix();
                 let substitute_client_state = self.client_state(&client_id)?;
+                let substitute_consensus_state =
+                    self.consensus_state(&ClientConsensusStatePath::new(
+                        client_id.clone(),
+                        substitute_client_state.latest_height().revision_number(),
+                        substitute_client_state.latest_height().revision_height(),
+                    ))?;
 
                 self.set_subject_prefix();
                 client_state.check_substitute(self, substitute_client_state.clone().into())?;
@@ -134,6 +140,7 @@ impl<'a, C: ClientType<'a>> Context<'a, C> {
                     self,
                     &self.client_id(),
                     substitute_client_state.into(),
+                    substitute_consensus_state.into(),
                 )?;
 
                 ContractResult::success()

diff --git a/ibc-clients/ics07-tendermint/src/client_state/execution.rs b/ibc-clients/ics07-tendermint/src/client_state/execution.rs
@@ -65,6 +65,7 @@ where
         ctx: &mut E,
         subject_client_id: &ClientId,
         substitute_client_state: Any,
+        substitute_consensus_state: Any,
     ) -> Result<(), ClientError> {
         let subject_client_state = self.inner().clone();
 
@@ -73,6 +74,7 @@ where
             ctx,
             subject_client_id,
             substitute_client_state,
+            substitute_consensus_state,
         )
     }
 }
@@ -373,10 +375,12 @@ pub fn update_on_recovery<E>(
     ctx: &mut E,
     subject_client_id: &ClientId,
     substitute_client_state: Any,
+    substitute_consensus_state: Any,
 ) -> Result<(), ClientError>
 where
     E: ExtClientExecutionContext,
     E::ClientStateRef: From<ClientStateType>,
+    E::ConsensusStateRef: Convertible<ConsensusStateType, ClientError>,
 {
     let substitute_client_state = ClientState::try_from(substitute_client_state)?
         .inner()
@@ -397,6 +401,17 @@ where
     let host_timestamp = E::host_timestamp(ctx)?;
     let host_height = E::host_height(ctx)?;
 
+    let tm_consensus_state = ConsensusStateType::try_from(substitute_consensus_state)?;
+
+    ctx.store_consensus_state(
+        ClientConsensusStatePath::new(
+            subject_client_id.clone(),
+            new_client_state.latest_height.revision_number(),
+            new_client_state.latest_height.revision_height(),
+        ),
+        tm_consensus_state.into(),
+    )?;
+
     ctx.store_client_state(
         ClientStatePath::new(subject_client_id.clone()),
         new_client_state.into(),

diff --git a/ibc-clients/ics07-tendermint/src/consensus_state.rs b/ibc-clients/ics07-tendermint/src/consensus_state.rs
@@ -89,8 +89,4 @@ impl ConsensusStateTrait for ConsensusState {
     fn timestamp(&self) -> Timestamp {
         self.0.timestamp.into()
     }
-
-    fn encode_vec(self) -> Vec<u8> {
-        <Self as Protobuf<Any>>::encode_vec(self)
-    }
 }
diff --git a/ibc-core/ics02-client/context/src/client_state.rs b/ibc-core/ics02-client/context/src/client_state.rs
@@ -10,8 +10,8 @@ use ibc_core_host_types::path::Path;
 use ibc_primitives::prelude::*;
 use ibc_primitives::proto::Any;
 
-/// Convenient trait to decode a client state from an `Any` type and obtain a
-/// handle to the local instance of `ClientState`.
+/// Convenient trait to decode a client state from an [`Any`] type and obtain a
+/// handle to the local instance of [`ClientState`].
 pub trait ClientStateDecoder: Into<Any> + TryFrom<Any, Error = ClientError> {}
 
 impl<T> ClientStateDecoder for T where T: Into<Any> + TryFrom<Any, Error = ClientError> {}
@@ -202,6 +202,7 @@ where
         ctx: &mut E,
         subject_client_id: &ClientId,
         substitute_client_state: Any,
+        substitute_consensus_state: Any,
     ) -> Result<(), ClientError>;
 }
 

diff --git a/ibc-core/ics02-client/context/src/consensus_state.rs b/ibc-core/ics02-client/context/src/consensus_state.rs
@@ -1,22 +1,25 @@
 //! Defines the trait to be implemented by all concrete consensus state types
 
+use ibc_core_client_types::error::ClientError;
 use ibc_core_commitment_types::commitment::CommitmentRoot;
 use ibc_primitives::prelude::*;
+use ibc_primitives::proto::Any;
 use ibc_primitives::Timestamp;
 
+/// Convenient trait to decode a consensus state from an [`Any`] type and obtain
+/// a handle to the local instance of [`ConsensusState`].
+pub trait ConsensusStateDecoder: Into<Any> + TryFrom<Any, Error = ClientError> {}
+
+impl<T> ConsensusStateDecoder for T where T: Into<Any> + TryFrom<Any, Error = ClientError> {}
+
 /// Defines methods that all `ConsensusState`s should provide.
 ///
 /// One can think of a "consensus state" as a pruned header, to be stored on chain. In other words,
 /// a consensus state only contains the header's information needed by IBC message handlers.
-pub trait ConsensusState: Send + Sync {
+pub trait ConsensusState: Send + Sync + ConsensusStateDecoder {
     /// Commitment root of the consensus state, which is used for key-value pair verification.
     fn root(&self) -> &CommitmentRoot;
 
     /// The timestamp of the consensus state
     fn timestamp(&self) -> Timestamp;
-
-    /// Serializes the `ConsensusState`. This is expected to be implemented as
-    /// first converting to the raw type (i.e. the protobuf definition), and then
-    /// serializing that.
-    fn encode_vec(self) -> Vec<u8>;
 }
diff --git a/ibc-core/ics02-client/src/handler/recover_client.rs b/ibc-core/ics02-client/src/handler/recover_client.rs
@@ -4,6 +4,7 @@ use ibc_core_client_context::prelude::*;
 use ibc_core_client_types::error::ClientError;
 use ibc_core_client_types::msgs::MsgRecoverClient;
 use ibc_core_handler_types::error::ContextError;
+use ibc_core_host::types::path::ClientConsensusStatePath;
 use ibc_core_host::{ExecutionContext, ValidationContext};
 
 /// Performs the validation steps associated with the client recovery process. This
@@ -72,11 +73,18 @@ where
 
     let subject_client_state = client_exec_ctx.client_state(&subject_client_id)?;
     let substitute_client_state = client_exec_ctx.client_state(&substitute_client_id)?;
+    let substitute_consensus_state =
+        client_exec_ctx.consensus_state(&ClientConsensusStatePath::new(
+            substitute_client_id.clone(),
+            substitute_client_state.latest_height().revision_number(),
+            substitute_client_state.latest_height().revision_height(),
+        ))?;
 
     subject_client_state.update_on_recovery(
         ctx.get_client_execution_context(),
         &subject_client_id,
         substitute_client_state.into(),
+        substitute_consensus_state.into(),
     )?;
 
     Ok(())

diff --git a/ibc-core/ics03-connection/src/handler/conn_open_ack.rs b/ibc-core/ics03-connection/src/handler/conn_open_ack.rs
@@ -128,7 +128,7 @@ where
                 &msg.proof_consensus_state_of_a_on_b,
                 consensus_state_of_b_on_a.root(),
                 Path::ClientConsensusState(client_cons_state_path_on_b),
-                expected_consensus_state_of_a_on_b.encode_vec(),
+                expected_consensus_state_of_a_on_b.into().to_vec(),
             )
             .map_err(|e| ConnectionError::ConsensusStateVerificationFailure {
                 height: msg.proofs_height_on_b,

diff --git a/ibc-core/ics03-connection/src/handler/conn_open_try.rs b/ibc-core/ics03-connection/src/handler/conn_open_try.rs
@@ -124,7 +124,7 @@ where
                 &msg.proof_consensus_state_of_b_on_a,
                 consensus_state_of_a_on_b.root(),
                 Path::ClientConsensusState(client_cons_state_path_on_a),
-                expected_consensus_state_of_b_on_a.encode_vec(),
+                expected_consensus_state_of_b_on_a.into().to_vec(),
             )
             .map_err(|e| ConnectionError::ConsensusStateVerificationFailure {
                 height: msg.proofs_height_on_a,

diff --git a/ibc-derive/src/client_state/traits/client_state_execution.rs b/ibc-derive/src/client_state/traits/client_state_execution.rs
@@ -47,7 +47,7 @@ pub(crate) fn impl_ClientStateExecution(
         client_state_enum_name,
         enum_variants.iter(),
         opts,
-        quote! { update_on_recovery(cs, ctx, client_id, substitute_client_state) },
+        quote! { update_on_recovery(cs, ctx, client_id, substitute_client_state, substitute_consensus_state) },
         imports,
     );
 
@@ -121,6 +121,7 @@ pub(crate) fn impl_ClientStateExecution(
                 ctx: &mut #E,
                 client_id: &#ClientId,
                 substitute_client_state: #Any,
+                substitute_consensus_state: #Any,
             ) -> core::result::Result<(), #ClientError> {
                 match self {
                     #(#update_on_recovery_impl),*

diff --git a/ibc-derive/src/consensus_state.rs b/ibc-derive/src/consensus_state.rs
@@ -20,12 +20,6 @@ pub fn consensus_state_derive_impl(ast: DeriveInput, imports: &Imports) -> Token
         quote! {timestamp(cs)},
         imports,
     );
-    let encode_vec_impl = delegate_call_in_match(
-        enum_name,
-        enum_variants.iter(),
-        quote! {encode_vec(cs)},
-        imports,
-    );
 
     let CommitmentRoot = imports.commitment_root();
     let ConsensusState = imports.consensus_state();
@@ -44,12 +38,6 @@ pub fn consensus_state_derive_impl(ast: DeriveInput, imports: &Imports) -> Token
                     #(#timestamp_impl),*
                 }
             }
-
-            fn encode_vec(self) -> Vec<u8> {
-                match self {
-                    #(#encode_vec_impl),*
-                }
-            }
         }
     }
 }

diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs b/ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs
@@ -343,6 +343,12 @@ where
             ),
             mock_consensus_state.into(),
         )?;
+        ctx.store_update_meta(
+            client_id.clone(),
+            self.latest_height(),
+            ctx.host_timestamp()?,
+            ctx.host_height()?,
+        )?;
 
         Ok(())
     }
@@ -440,6 +446,7 @@ where
         ctx: &mut E,
         subject_client_id: &ClientId,
         substitute_client_state: Any,
+        substitute_consensus_state: Any,
     ) -> Result<(), ClientError> {
         let substitute_client_state = MockClientState::try_from(substitute_client_state)?;
 
@@ -453,6 +460,17 @@ where
         let host_timestamp = ctx.host_timestamp()?;
         let host_height = ctx.host_height()?;
 
+        let mock_consensus_state = MockConsensusState::try_from(substitute_consensus_state)?;
+
+        ctx.store_consensus_state(
+            ClientConsensusStatePath::new(
+                subject_client_id.clone(),
+                new_mock_client_state.latest_height().revision_number(),
+                new_mock_client_state.latest_height().revision_height(),
+            ),
+            mock_consensus_state.into(),
+        )?;
+
         ctx.store_client_state(
             ClientStatePath::new(subject_client_id.clone()),
             new_mock_client_state.into(),

diff --git a/ibc-testkit/src/testapp/ibc/clients/mock/consensus_state.rs b/ibc-testkit/src/testapp/ibc/clients/mock/consensus_state.rs
@@ -98,8 +98,4 @@ impl ConsensusState for MockConsensusState {
     fn timestamp(&self) -> Timestamp {
         self.header.timestamp
     }
-
-    fn encode_vec(self) -> Vec<u8> {
-        <Self as Protobuf<Any>>::encode_vec(self)
-    }
 }
diff --git a/ibc-testkit/tests/core/ics02_client/recover_client.rs b/ibc-testkit/tests/core/ics02_client/recover_client.rs
@@ -7,6 +7,7 @@ use ibc::core::client::types::Height;
 use ibc::core::entrypoint::{execute, validate};
 use ibc::core::handler::types::msgs::MsgEnvelope;
 use ibc::core::host::types::identifiers::ClientId;
+use ibc::core::host::types::path::ClientConsensusStatePath;
 use ibc::core::host::ValidationContext;
 use ibc::core::primitives::{Signer, Timestamp};
 use ibc_testkit::fixtures::core::signer::dummy_account_id;
@@ -126,10 +127,27 @@ fn test_recover_client_ok() {
 
     assert!(res.is_ok(), "client recovery execution happy path");
 
+    // client state is copied.
     assert_eq!(
         ctx.client_state(&msg.subject_client_id).unwrap(),
         ctx.client_state(&msg.substitute_client_id).unwrap(),
     );
+
+    // latest consensus state is copied.
+    assert_eq!(
+        ctx.consensus_state(&ClientConsensusStatePath::new(
+            msg.subject_client_id,
+            substitute_height.revision_number(),
+            substitute_height.revision_height(),
+        ))
+        .unwrap(),
+        ctx.consensus_state(&ClientConsensusStatePath::new(
+            msg.substitute_client_id,
+            substitute_height.revision_number(),
+            substitute_height.revision_height(),
+        ))
+        .unwrap(),
+    );
 }
 
 #[rstest]