Skip to content

Commit

Permalink
feat: implement client recovery feature (#1127)
Browse files Browse the repository at this point in the history
* Add `MsgRecoverClient` domain type

* Working on client recovery validation handler logic

* Implement recover_client::validate function

* Add ClientStateValidation::check_substitute function

* Working on `recover_client::execute` function

* Stub out `update_on_recovery` function

* Implement standalone `update_on_recovery` function

* Change `update_on_recovery` function param

* Remove `ClientState::initialise` call from `execute`

* Implement `check_substitute` in ics07

* Update `update_on_recovery` function

* Add `MsgRecoverClient` domain type

* Working on client recovery validation handler logic

* Implement recover_client::validate function

* Add ClientStateValidation::check_substitute function

* Working on `recover_client::execute` function

* Stub out `update_on_recovery` function

* Implement standalone `update_on_recovery` function

* Change `update_on_recovery` function param

* Remove `ClientState::initialise` call from `execute`

* Implement `check_substitute` in ics07

* Update `update_on_recovery` function

* Resolve errors upon rebasing

* Add additional bounds on `check_substitute` and `update_on_recovery`

* Implement TryFrom<AnyClientState> for ClientStateType

* Attempt to implement check_substitute and update_on_recovery in ibc-derive

* fix: introduce fully-qualified path to get ibc-derive work

* Change substitute client state function params to Any

* Update change_substitute and update_on_recovery call sites

* Add additional bounds on other validation and execution context methods

* Implement `update_on_recovery` for MockClientState

* Silence unused variables warning

* Add missing MockClientState fields

* Fix some typos in doc comments

* Remove unnecessary trait bound

* Change `upgrade_client_proposal_handler` -> `execute_upgrade_client_proposal`

* Add upgrade proposal type url

* Remove commented-out trait bounds

* Convert Any to ClientState instead of ClientStateType

* Improve error message wording

* Improve error message wording

* Remove unnused function paramter

* Replace `self` with `&self`

* Convert substitute client state to MockClientState instead of Tm client state

* Update ClientNotInactive error variant message

* Improve `check_substitute` implementation

* Make `Status` type `Copy`

* Remove some imports and streamline mock client state initialization

* rm redundant methods

* Remove unnecessary TryFrom impl

* tests: add unit tests for client recovery (#1151)

* Initial scaffolding for testing client recovery

* Add MsgRecoverClient variant to ClientMsg

* Stub out `test_recover_client_ok`

* Update recover client test fixture

* Perform client updates on subject and substitute clients

* Create mock headers with timestamps

* Extend substitute client state's trusting period

* Remove eprintln statements

* Add some more tests

* Refactor to not require call to `sleep`

* Add docstring for `MockClientState::new`

* Formatting

* Remove recover_client validate and execute calls from dispatch

* Remove recover_client validate and execute calls from dispatch

* Call recover_client::validate and execute correctly

* Change MockClientState default trusting period to 10 seconds

* Add changelog entry

---------

Co-authored-by: Farhad Shabani <[email protected]>
Co-authored-by: Ranadeep Biswas <[email protected]>
  • Loading branch information
3 people authored Apr 3, 2024
1 parent b0ea4ea commit 2b9de34
Show file tree
Hide file tree
Showing 19 changed files with 632 additions and 24 deletions.
4 changes: 4 additions & 0 deletions .changelog/unreleased/features/738-client-recovery.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- [ibc-client] Implement [client recovery][client-recovery] feature.
([\#738](https://github.com/cosmos/ibc-rs/issues/738))

client-recovery: https://github.com/cosmos/ibc-go/blob/main/docs/architecture/adr-026-ibc-client-recovery-mechanisms.md
75 changes: 74 additions & 1 deletion ibc-clients/ics07-tendermint/src/client_state/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,22 @@ where
upgraded_consensus_state,
)
}

fn update_on_recovery(
&self,
ctx: &mut E,
subject_client_id: &ClientId,
substitute_client_state: Any,
) -> Result<(), ClientError> {
let subject_client_state = self.inner().clone();

update_on_recovery(
subject_client_state,
ctx,
subject_client_id,
substitute_client_state,
)
}
}

/// Seed the host store with initial client and consensus states.
Expand Down Expand Up @@ -208,7 +224,8 @@ where
Ok(())
}

/// Commit the new client state and consensus state to the store.
/// Commit the new client state and consensus state to the store upon a
/// successful client upgrade.
///
/// Note that this function is typically implemented as part of the
/// [`ClientStateExecution`] trait, but has been made a standalone function
Expand Down Expand Up @@ -344,3 +361,59 @@ where

Ok(())
}

/// Update the `client_state`'s ID, trusting period, latest height, processed height,
/// and processed time metadata values to those values provided by a verified substitute
/// client state in response to a successful client recovery.
///
/// Note that unlike the `update_on_upgrade` function, `update_on_recovery` assumes
/// that the client being updated has already been re-initialised such that its original
/// client and consensus states have been overwritten to their new states.
///
/// This function is typically implemented as part of the [`ClientStateExecution`]
/// trait, but has been made standalone in order to enable greater flexibility
/// of the ClientState APIs.
pub fn update_on_recovery<E>(
subject_client_state: ClientStateType,
ctx: &mut E,
subject_client_id: &ClientId,
substitute_client_state: Any,
) -> Result<(), ClientError>
where
E: TmExecutionContext,
E::ClientStateRef: From<ClientStateType>,
E::ConsensusStateRef: ConsensusStateConverter,
{
let substitute_client_state = ClientState::try_from(substitute_client_state)?
.inner()
.clone();

let chain_id = substitute_client_state.chain_id;
let trusting_period = substitute_client_state.trusting_period;
let latest_height = substitute_client_state.latest_height;

let new_client_state = ClientStateType {
chain_id,
trusting_period,
latest_height,
frozen_height: None,
..subject_client_state
};

let host_timestamp = E::host_timestamp(ctx)?;
let host_height = E::host_height(ctx)?;

ctx.store_client_state(
ClientStatePath::new(subject_client_id.clone()),
new_client_state.into(),
)?;

ctx.store_update_meta(
subject_client_id.clone(),
latest_height,
host_timestamp,
host_height,
)?;

Ok(())
}
55 changes: 55 additions & 0 deletions ibc-clients/ics07-tendermint/src/client_state/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ where
fn status(&self, ctx: &V, client_id: &ClientId) -> Result<Status, ClientError> {
status(self.inner(), ctx, client_id)
}

fn check_substitute(&self, _ctx: &V, substitute_client_state: Any) -> Result<(), ClientError> {
check_substitute::<V>(self.inner(), substitute_client_state)
}
}

/// Verify the client message as part of the client state validation process.
Expand Down Expand Up @@ -202,3 +206,54 @@ where

Ok(Status::Active)
}

/// Check that the subject and substitute client states match as part of
/// the client recovery validation step.
///
/// The subject and substitute client states match if all their respective
/// client state parameters match except for frozen height, latest height,
/// trusting period, and chain ID.
pub fn check_substitute<V>(
subject_client_state: &ClientStateType,
substitute_client_state: Any,
) -> Result<(), ClientError>
where
V: TmValidationContext,
V::ConsensusStateRef: ConsensusStateConverter,
{
let ClientStateType {
latest_height: _,
frozen_height: _,
trusting_period: _,
chain_id: _,
allow_update: _,
trust_level: subject_trust_level,
unbonding_period: subject_unbonding_period,
max_clock_drift: subject_max_clock_drift,
proof_specs: subject_proof_specs,
upgrade_path: subject_upgrade_path,
} = subject_client_state.clone();

let substitute_client_state = ClientStateType::try_from(substitute_client_state)?;

let ClientStateType {
latest_height: _,
frozen_height: _,
trusting_period: _,
chain_id: _,
allow_update: _,
trust_level: substitute_trust_level,
unbonding_period: substitute_unbonding_period,
max_clock_drift: substitute_max_clock_drift,
proof_specs: substitute_proof_specs,
upgrade_path: substitute_upgrade_path,
} = substitute_client_state;

(subject_trust_level == substitute_trust_level
&& subject_unbonding_period == substitute_unbonding_period
&& subject_max_clock_drift == substitute_max_clock_drift
&& subject_proof_specs == substitute_proof_specs
&& subject_upgrade_path == substitute_upgrade_path)
.then_some(())
.ok_or(ClientError::ClientRecoveryStateMismatch)
}
25 changes: 24 additions & 1 deletion ibc-core/ics02-client/context/src/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,20 @@ where

/// Returns the status of the client. Only Active clients are allowed to process packets.
fn status(&self, ctx: &V, client_id: &ClientId) -> Result<Status, ClientError>;

/// Verifies whether the calling (subject) client state matches the substitute
/// client state for the purposes of client recovery.
///
/// Note that this validation function does not need to perform *all* of the
/// validation steps necessary to confirm client recovery. Some checks, such
/// as checking that the subject client state's latest height < the substitute
/// client's latest height, as well as checking that the subject client is
/// inactive and that the substitute client is active, are performed by the
/// `validate` function in the `recover_client` module at the ics02-client
/// level.
///
/// Returns `Ok` if the subject and substitute client states match, `Err` otherwise.
fn check_substitute(&self, ctx: &V, substitute_client_state: Any) -> Result<(), ClientError>;
}

/// `ClientState` methods which require access to the client's
Expand Down Expand Up @@ -172,14 +186,23 @@ where
client_message: Any,
) -> Result<(), ClientError>;

// Update the client state and consensus state in the store with the upgraded ones.
/// Update the client state and consensus state in the store with the upgraded ones.
fn update_state_on_upgrade(
&self,
ctx: &mut E,
client_id: &ClientId,
upgraded_client_state: Any,
upgraded_consensus_state: Any,
) -> Result<Height, ClientError>;

/// Update the subject client using the `substitute_client_state` in response
/// to a successful client recovery.
fn update_on_recovery(
&self,
ctx: &mut E,
subject_client_id: &ClientId,
substitute_client_state: Any,
) -> Result<(), ClientError>;
}

use crate::context::{ClientExecutionContext, ClientValidationContext};
Expand Down
1 change: 1 addition & 0 deletions ibc-core/ics02-client/src/handler/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! This module implements the processing logic for ICS2 (client abstractions and functions) msgs.
pub mod create_client;
pub mod recover_client;
pub mod update_client;
pub mod upgrade_client;
83 changes: 83 additions & 0 deletions ibc-core/ics02-client/src/handler/recover_client.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
//! Protocol logic for processing ICS02 messages of type `MsgRecoverClient`.
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::{ExecutionContext, ValidationContext};

/// Performs the validation steps associated with the client recovery process. This
/// includes validating that the parameters of the subject and substitute clients match,
/// as well as validating that the substitute client *is* active and that the subject
/// client is *not* active.
pub fn validate<Ctx>(ctx: &Ctx, msg: MsgRecoverClient) -> Result<(), ContextError>
where
Ctx: ValidationContext,
{
let signer = msg.signer;
let subject_client_id = msg.subject_client_id.clone();
let substitute_client_id = msg.substitute_client_id.clone();

ctx.validate_message_signer(&signer)?;

let client_val_ctx = ctx.get_client_validation_context();

let subject_client_state = client_val_ctx.client_state(&subject_client_id)?;
let substitute_client_state = client_val_ctx.client_state(&substitute_client_id)?;

let subject_height = subject_client_state.latest_height();
let substitute_height = substitute_client_state.latest_height();

if subject_height >= substitute_height {
return Err(ClientError::ClientRecoveryHeightMismatch {
subject_height,
substitute_height,
}
.into());
}

substitute_client_state
.status(ctx.get_client_validation_context(), &substitute_client_id)?
.verify_is_active()?;

// Verify that the subject client is inactive, i.e., that it is either frozen or expired
subject_client_state
.status(ctx.get_client_validation_context(), &subject_client_id)?
.verify_is_inactive()?;

// Check that the subject client state and substitute client states match, i.e., that
// all their respective client state parameters match except for frozen height, latest
// height, trusting period, and chain ID
subject_client_state.check_substitute(
ctx.get_client_validation_context(),
substitute_client_state.into(),
)?;

Ok(())
}

/// Executes the steps needed to recover the subject client, namely:
/// - setting the subject's status from either `frozen` or `expired` to `active`
/// - copying the substitute client's consensus state as the subject's consensus state
/// - setting the subject client's processed height and processed time values to match the substitute client's
/// - setting the subject client's latest height, trusting period, and chain ID values to match the substitute client's
pub fn execute<Ctx>(ctx: &mut Ctx, msg: MsgRecoverClient) -> Result<(), ContextError>
where
Ctx: ExecutionContext,
{
let subject_client_id = msg.subject_client_id.clone();
let substitute_client_id = msg.substitute_client_id.clone();

let client_exec_ctx = ctx.get_client_execution_context();

let subject_client_state = client_exec_ctx.client_state(&subject_client_id)?;
let substitute_client_state = client_exec_ctx.client_state(&substitute_client_id)?;

subject_client_state.update_on_recovery(
ctx.get_client_execution_context(),
&subject_client_id,
substitute_client_state.into(),
)?;

Ok(())
}
12 changes: 11 additions & 1 deletion ibc-core/ics02-client/types/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Defines the client error type
use displaydoc::Display;
// use ibc::core::ContextError;
use ibc_core_commitment_types::error::CommitmentError;
use ibc_core_host_types::error::IdentifierError;
use ibc_core_host_types::identifiers::{ClientId, ClientType};
Expand All @@ -20,10 +19,19 @@ pub enum ClientError {
ClientFrozen { description: String },
/// client is not active. Status=`{status}`
ClientNotActive { status: Status },
/// client is not frozen or expired. Status=`{status}`
ClientNotInactive { status: Status },
/// client state not found: `{client_id}`
ClientStateNotFound { client_id: ClientId },
/// client state already exists: `{client_id}`
ClientStateAlreadyExists { client_id: ClientId },
/// Substitute client height `{substitute_height}` is not greater than subject client height `{subject_height}` during client recovery
ClientRecoveryHeightMismatch {
subject_height: Height,
substitute_height: Height,
},
/// Subject and substitute client state mismatch during client recovery
ClientRecoveryStateMismatch,
/// consensus state not found at: `{client_id}` at height `{height}`
ConsensusStateNotFound { client_id: ClientId, height: Height },
/// Processed time or height for the client `{client_id}` at height `{height}` not found
Expand All @@ -50,6 +58,8 @@ pub enum ClientError {
MissingRawConsensusState,
/// invalid client id in the update client message: `{0}`
InvalidMsgUpdateClientId(IdentifierError),
/// invalid client id in recover client message: `{0}`
InvalidMsgRecoverClientId(IdentifierError),
/// invalid client identifier error: `{0}`
InvalidClientIdentifier(IdentifierError),
/// invalid raw header error: `{reason}`
Expand Down
3 changes: 3 additions & 0 deletions ibc-core/ics02-client/types/src/msgs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ use ibc_proto::google::protobuf::Any;

mod create_client;
mod misbehaviour;
mod recover_client;
mod update_client;
mod upgrade_client;

pub use create_client::*;
pub use misbehaviour::*;
pub use recover_client::*;
pub use update_client::*;
pub use upgrade_client::*;

Expand All @@ -30,6 +32,7 @@ pub enum ClientMsg {
UpdateClient(MsgUpdateClient),
Misbehaviour(MsgSubmitMisbehaviour),
UpgradeClient(MsgUpgradeClient),
RecoverClient(MsgRecoverClient),
}

pub enum MsgUpdateOrMisbehaviour {
Expand Down
Loading

0 comments on commit 2b9de34

Please sign in to comment.