Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 17 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ibc-clients/cw-context/src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand All @@ -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()
Expand Down
15 changes: 15 additions & 0 deletions ibc-clients/ics07-tendermint/src/client_state/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -73,6 +74,7 @@ where
ctx,
subject_client_id,
substitute_client_state,
substitute_consensus_state,
)
}
}
Expand Down Expand Up @@ -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()
Expand All @@ -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(),
Expand Down
4 changes: 0 additions & 4 deletions ibc-clients/ics07-tendermint/src/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
5 changes: 3 additions & 2 deletions ibc-core/ics02-client/context/src/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {}
Expand Down Expand Up @@ -202,6 +202,7 @@ where
ctx: &mut E,
subject_client_id: &ClientId,
substitute_client_state: Any,
substitute_consensus_state: Any,
) -> Result<(), ClientError>;
}

Expand Down
15 changes: 9 additions & 6 deletions ibc-core/ics02-client/context/src/consensus_state.rs
Original file line number Diff line number Diff line change
@@ -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>;
}
8 changes: 8 additions & 0 deletions ibc-core/ics02-client/src/handler/recover_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(())
Expand Down
2 changes: 1 addition & 1 deletion ibc-core/ics03-connection/src/handler/conn_open_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion ibc-core/ics03-connection/src/handler/conn_open_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion ibc-derive/src/client_state/traits/client_state_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);

Expand Down Expand Up @@ -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),*
Expand Down
12 changes: 0 additions & 12 deletions ibc-derive/src/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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),*
}
}
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,12 @@
),
mock_consensus_state.into(),
)?;
ctx.store_update_meta(
client_id.clone(),
self.latest_height(),
ctx.host_timestamp()?,
ctx.host_height()?,
)?;

Check warning on line 351 in ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs

View check run for this annotation

Codecov / codecov/patch

ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs#L351

Added line #L351 was not covered by tests
seanchen1991 marked this conversation as resolved.
Show resolved Hide resolved

Ok(())
}
Expand Down Expand Up @@ -440,6 +446,7 @@
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)?;

Expand All @@ -453,6 +460,17 @@
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(),
Expand Down
4 changes: 0 additions & 4 deletions ibc-testkit/src/testapp/ibc/clients/mock/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
18 changes: 18 additions & 0 deletions ibc-testkit/tests/core/ics02_client/recover_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]
Expand Down
Loading