From 1f581fa8cb03f66bab72e181ceca03ed148fb8f1 Mon Sep 17 00:00:00 2001 From: Farhad Shabani Date: Fri, 5 Apr 2024 11:12:13 -0700 Subject: [PATCH] chore: add `into_inner()` for ICS07 `ConsensusState` (#1156) * chore: add into_inner() for TmConsensusState + remove redundant cloning in check_substitute * fix: markdown-lint-check * chore: add unclog * nit * fix: revert adr007 playground --- ...156-add-into-inner-for-ics07-consensus-state.md | 2 ++ docs/architecture/adr-007-light-client-contexts.md | 14 +++++++------- .../src/client_state/validation.rs | 12 ++++++------ .../ics07-tendermint/src/consensus_state.rs | 4 ++++ 4 files changed, 19 insertions(+), 13 deletions(-) create mode 100644 .changelog/unreleased/improvements/1156-add-into-inner-for-ics07-consensus-state.md diff --git a/.changelog/unreleased/improvements/1156-add-into-inner-for-ics07-consensus-state.md b/.changelog/unreleased/improvements/1156-add-into-inner-for-ics07-consensus-state.md new file mode 100644 index 000000000..08c42925b --- /dev/null +++ b/.changelog/unreleased/improvements/1156-add-into-inner-for-ics07-consensus-state.md @@ -0,0 +1,2 @@ +- [ibc-client-tendermint] Add `into_inner()` method to ICS07 `ConsensusState` + ([\#1156](https://github.com/cosmos/ibc-rs/pull/1156)) diff --git a/docs/architecture/adr-007-light-client-contexts.md b/docs/architecture/adr-007-light-client-contexts.md index 834a74bfb..b0caabdd5 100644 --- a/docs/architecture/adr-007-light-client-contexts.md +++ b/docs/architecture/adr-007-light-client-contexts.md @@ -13,7 +13,7 @@ This ADR is meant to address the main limitation of our current light client API + By giving the light clients access to `ValidationContext` and `ExecutionContext`, we're effectively giving them the same capabilities as the core handlers. + Although our current model is that all code is trusted (including light clients we didn't write), restraining the capabilities we give to light clients at the very least eliminates a class of bugs (e.g. calling the wrong method), and serves as documentation for exactly which methods the light client needs. -This ADR is all about fixing this issue; namely, to enable light clients to define their own `ValidationContext` and `ExecutionContext` traits for the host to implement. +This ADR is all about fixing this issue; namely, to enable light clients to define their own `ValidationContext` and `ExecutionContext` traits for the host to implement. [ADR 4]: ../architecture/adr-004-light-client-crates-extraction.md [later improved]: https://github.com/cosmos/ibc-rs/pull/584 @@ -26,9 +26,9 @@ This ADR is all about fixing this issue; namely, to enable light clients to defi ### Changes to `ClientState` -The `ClientState` functionality is split into 3 traits: -+ `ClientStateCommon`, -+ `ClientStateValidation`, and +The `ClientState` functionality is split into 3 traits: ++ `ClientStateCommon`, ++ `ClientStateValidation`, and + `ClientStateExecution` Then, `ClientState` is defined as @@ -68,7 +68,7 @@ where &self, ctx: &ClientValidationContext, // ... - ) -> Result<(), ClientError> { + ) -> Result<(), ClientError> { // `get_resource_Y()` accessible through `ctx` } @@ -93,7 +93,7 @@ where `ClientExecutionContext` is defined as (simplified) ```rust pub trait ClientExecutionContext: Sized { - // ... a few associated types + // ... a few associated types /// Called upon successful client creation and update fn store_client_state( @@ -235,7 +235,7 @@ We ended up having to write our own custom derive macros because existing crates ### Negative + Increased complexity. -+ Harder to document. ++ Harder to document. + Specifically, we do not write any trait bounds on the `Client{Validation, Execution}Context` generic parameters. The effective trait bounds are spread across all light client implementations that a given host uses. diff --git a/ibc-clients/ics07-tendermint/src/client_state/validation.rs b/ibc-clients/ics07-tendermint/src/client_state/validation.rs index 2510a8ade..d3d53f018 100644 --- a/ibc-clients/ics07-tendermint/src/client_state/validation.rs +++ b/ibc-clients/ics07-tendermint/src/client_state/validation.rs @@ -232,7 +232,7 @@ where max_clock_drift: subject_max_clock_drift, proof_specs: subject_proof_specs, upgrade_path: subject_upgrade_path, - } = subject_client_state.clone(); + } = subject_client_state; let substitute_client_state = ClientStateType::try_from(substitute_client_state)?; @@ -249,11 +249,11 @@ where 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) + (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) } diff --git a/ibc-clients/ics07-tendermint/src/consensus_state.rs b/ibc-clients/ics07-tendermint/src/consensus_state.rs index 242c82be4..f2473d63a 100644 --- a/ibc-clients/ics07-tendermint/src/consensus_state.rs +++ b/ibc-clients/ics07-tendermint/src/consensus_state.rs @@ -30,6 +30,10 @@ impl ConsensusState { &self.0 } + pub fn into_inner(self) -> ConsensusStateType { + self.0 + } + pub fn timestamp(&self) -> Time { self.0.timestamp }