Skip to content

Commit

Permalink
chore: add into_inner() for ICS07 ConsensusState (#1156)
Browse files Browse the repository at this point in the history
* chore: add into_inner() for TmConsensusState + remove redundant cloning in check_substitute

* fix: markdown-lint-check

* chore: add unclog

* nit

* fix: revert adr007 playground
  • Loading branch information
Farhad-Shabani authored Apr 5, 2024
1 parent d4c3655 commit 1f581fa
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- [ibc-client-tendermint] Add `into_inner()` method to ICS07 `ConsensusState`
([\#1156](https://github.com/cosmos/ibc-rs/pull/1156))
14 changes: 7 additions & 7 deletions docs/architecture/adr-007-light-client-contexts.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<ClientValidationContext>`, and
The `ClientState` functionality is split into 3 traits:
+ `ClientStateCommon`,
+ `ClientStateValidation<ClientValidationContext>`, and
+ `ClientStateExecution<ClientExecutionContext>`

Then, `ClientState` is defined as
Expand Down Expand Up @@ -68,7 +68,7 @@ where
&self,
ctx: &ClientValidationContext,
// ...
) -> Result<(), ClientError> {
) -> Result<(), ClientError> {
// `get_resource_Y()` accessible through `ctx`
}

Expand All @@ -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(
Expand Down Expand Up @@ -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.


Expand Down
12 changes: 6 additions & 6 deletions ibc-clients/ics07-tendermint/src/client_state/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand All @@ -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)
}
4 changes: 4 additions & 0 deletions ibc-clients/ics07-tendermint/src/consensus_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ impl ConsensusState {
&self.0
}

pub fn into_inner(self) -> ConsensusStateType {
self.0
}

pub fn timestamp(&self) -> Time {
self.0.timestamp
}
Expand Down

0 comments on commit 1f581fa

Please sign in to comment.