Skip to content

Commit 30a2e5d

Browse files
plaferplafer
authored
ADR 7: Light client contexts (#708)
* ADR 7 intro * ADR progress * progress * `ClientState` section * ClientState section * edits * generics attribute syntax * alternatives doc * first pass done * dependency -> context * Remove `ClientStateInitializer` * ClientStateCommon * rework intro * update with `ClientExecutionContext` * add solomachine note
1 parent cd3a266 commit 30a2e5d

File tree

2 files changed

+255
-0
lines changed

2 files changed

+255
-0
lines changed

‎docs/architecture/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,5 @@ To suggest an ADR, please make use of the [ADR template](./adr-template.md) prov
3232
| [003](./adr-003-ics20-implementation.md) | ICS20 implementation | Accepted |
3333
| [004](./adr-004-light-client-crates-extraction.md) | Light client crates extraction | Accepted |
3434
| [005](./adr-005-handlers-redesign.md) | Handlers validation and execution separation | Accepted |
35+
| [006](./adr-006-upgrade-client-implementation.md) | Chain and client upgradability | Accepted |
36+
| [007](./adr-007-light-client-contexts.md) | Light client contexts | Accepted |

‎docs/architecture/adr-007-light-client-contexts.md

Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
# ADR 007: LIGHT CLIENT CONTEXTS
2+
3+
## Context
4+
5+
This ADR is meant to address the main limitation of our current light client API, first introduced in [ADR 4] and [later improved] to adopt some of the ideas present in ibc-go's [ADR 6]. Implementing some `ClientState` methods require additional information from the host. For example, the Tendermint client's implementation of `ClientState::verify_client_message` needs [access to the host timestamp] to properly perform a message's verification. Previously, we solved this problem by [giving a reference] to a `ValidationContext` and `ExecutionContext`, since most methods are already made available by these traits. However, this solution has some limitations:
6+
7+
1. Not all methods needed by every future light client is present in `ValidationContext` or `ExecutionContext`. For example, if a light client X finds that it would need access to some resource Y, currently the only way to solve this is to submit a PR on the ibc-rs repository that adds a method `get_resource_Y()` to `ValidationContext`.
8+
+ This means that every host will need to implement `get_resource_Y()`, even if they don't use light client X.
9+
+ It clutters up `ValidationContext` and `ExecutionContext`.
10+
2. We found that some methods only needed by the Tendermint light client made their way into `ValidationContext`.
11+
+ `next_consensus_state()` and `prev_consensus_state()` are not used in the core handlers; they're only there because of the Tendermint light client.
12+
3. It gives more power to light clients than they really need
13+
+ By giving the light clients access to `ValidationContext` and `ExecutionContext`, we're effectively giving them the same capabilities as the core handlers.
14+
+ 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.
15+
16+
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.
17+
18+
[ADR 4]: ../architecture/adr-004-light-client-crates-extraction.md
19+
[later improved]: https://github.com/cosmos/ibc-rs/pull/584
20+
[ADR 6]: https://github.com/cosmos/ibc-go/blob/main/docs/architecture/adr-006-02-client-refactor.md
21+
[access to the host timestamp]: https://github.com/cosmos/ibc-rs/blob/3e2566b3102af3fb6185cdc158cff818ec605535/crates/ibc/src/clients/ics07_tendermint/client_state/update_client.rs#L70
22+
[giving a reference]: https://github.com/cosmos/ibc-rs/blob/3e2566b3102af3fb6185cdc158cff818ec605535/crates/ibc/src/core/ics02_client/client_state.rs#L72
23+
24+
## Decision
25+
26+
27+
### Changes to `ClientState`
28+
29+
The `ClientState` functionality is split into 3 traits:
30+
+ `ClientStateCommon`,
31+
+ `ClientStateValidation<ClientValidationContext>`, and
32+
+ `ClientStateExecution<ClientExecutionContext>`
33+
34+
Then, `ClientState` is defined as
35+
36+
```rust
37+
pub trait ClientState<ClientValidationContext, E: ClientExecutionContext>:
38+
ClientStateCommon
39+
+ ClientStateValidation<ClientValidationContext>
40+
+ ClientStateExecution<E>
41+
// + ...
42+
{
43+
}
44+
```
45+
46+
A blanket implementation implements `ClientState` when these 3 traits are implemented on a given type. For details as to why `ClientState` was split into 3 traits, see the section "Why are there 3 `ClientState` traits?".
47+
48+
The `ClientStateValidation` and `ClientStateExecution` traits are the most important ones, as they are the ones that enable light clients to define `Context` traits for the host to implement.
49+
50+
#### `ClientStateValidation`
51+
52+
Say the implementation of a light client needs a `get_resource_Y()` method from the host in `ClientState::verify_client_message()`. The implementor would first define a trait for the host to implement.
53+
54+
```rust
55+
trait MyClientValidationContext {
56+
fn get_resource_Y(&self) -> Y;
57+
}
58+
```
59+
60+
Then, they would implement the `ClientStateValidation<ClientValidationContext>` trait *conditioned on* `ClientValidationContext` having `MyClientValidationContext` as supertrait.
61+
62+
```rust
63+
impl<ClientValidationContext> ClientStateValidation<ClientValidationContext> for MyClientState
64+
where
65+
ClientValidationContext: MyClientValidationContext,
66+
{
67+
fn verify_client_message(
68+
&self,
69+
ctx: &ClientValidationContext,
70+
// ...
71+
) -> Result<(), ClientError> {
72+
// `get_resource_Y()` accessible through `ctx`
73+
}
74+
75+
// ...
76+
}
77+
```
78+
79+
This is the core idea of this ADR. Everything else is a consequence of wanting to make this work.
80+
81+
#### `ClientStateExecution`
82+
83+
`ClientStateExecution` is defined a little differently from `ClientStateValidation`.
84+
85+
```rust
86+
pub trait ClientStateExecution<E>
87+
where
88+
E: ClientExecutionContext,
89+
{ ... }
90+
```
91+
92+
where `ClientExecutionContext` is defined as (simplified)
93+
94+
```rust
95+
pub trait ClientExecutionContext: Sized {
96+
// ... a few associated types
97+
98+
/// Called upon successful client creation and update
99+
fn store_client_state(
100+
...
101+
) -> Result<(), ContextError>;
102+
103+
/// Called upon successful client creation and update
104+
fn store_consensus_state(
105+
...
106+
) -> Result<(), ContextError>;
107+
}
108+
```
109+
110+
Under our current architecture (inspired from ibc-go's [ADR 6]), clients have the responsibility to store the `ClientState` and `ConsensusState`. Hence, `ClientExecutionContext` defines a uniform interface that clients can use to store their `ClientState` and `ConsensusState`. It also means that the host only needs to implement these methods once, as opposed to once per client. Note that clients who don't store consensus states (e.g. solomachine) can simply leave the implementation of `store_consensus_state()` empty (or return an error, whichever is most appropriate).
111+
112+
### Changes to `ValidationContext` and `ExecutionContext`
113+
114+
The `ClientState` changes described above induce some changes on `ValidationContext` and `ExecutionContext`.
115+
116+
`ValidationContext` is now defined as:
117+
118+
```rust
119+
pub trait ValidationContext: Router {
120+
type ClientValidationContext;
121+
type ClientExecutionContext;
122+
/// Enum that can contain a `ConsensusState` object of any supported light client
123+
type AnyConsensusState: ConsensusState<EncodeError = ContextError>;
124+
/// Enum that can contain a `ClientState` object of any supported light client
125+
type AnyClientState: ClientState<
126+
Self::AnyConsensusState,
127+
Self::ClientValidationContext,
128+
Self::ClientExecutionContext,
129+
>;
130+
131+
// ...
132+
}
133+
```
134+
135+
`AnyConsensusState` and `AnyClientState` are expected to be enums that hold the consensus states and client states of all supported light clients. For example,
136+
137+
```rust
138+
enum AnyConsensusState {
139+
Tendermint(TmConsensusState),
140+
Near(NearConsensusState),
141+
// ...
142+
}
143+
144+
enum AnyClientState {
145+
Tendermint(TmClientState),
146+
Near(NearClientState),
147+
// ...
148+
}
149+
```
150+
151+
`ClientValidationContext` and `ClientExecutionContext` correspond to the same types described in the previous section. The host must ensure that these 2 types implement the Tendermint and Near "`ValidationContext` and `ExecutionContext` traits" (as discussed in the previous section). For example,
152+
153+
```rust
154+
struct MyClientValidationContext;
155+
156+
// Here, `TmClientValidationContext` is a Tendermint's `ValidationContext`, meaning that it contains all the methods
157+
// that the Tendermint client requires from the host in order to perform message validation.
158+
impl TmClientValidationContext for MyClientValidationContext {
159+
// ...
160+
}
161+
162+
impl NearClientValidationContext for MyClientValidationContext {
163+
// ...
164+
}
165+
166+
// Code for `ClientExecutionContext` is analogous
167+
```
168+
169+
### `ClientState` and `ConsensusState` convenience derive macros
170+
Notice that `ValidationContext::AnyClientState` needs to implement `ClientState`, and `ValidationContext::AnyConsensusState` needs to implement `ConsensusState`. Given that `AnyClientState` and `AnyConsensusState` are enums that wrap types that *must* implement `ClientState` or `ConsensusState` (respectively), implementing these traits is gruesome boilerplate:
171+
172+
```rust
173+
impl ClientStateCommon for AnyClientState {
174+
fn client_type(&self) -> ClientType {
175+
match self {
176+
Tendermint(cs) => cs.client_type(),
177+
Near(cs) => cs.client_type()
178+
}
179+
}
180+
181+
// ...
182+
}
183+
```
184+
185+
To relieve users of such torture, we provide derive macros that do just that:
186+
187+
```rust
188+
#[derive(ConsensusState)]
189+
enum AnyConsensusState {
190+
Tendermint(TmConsensusState),
191+
Near(NearConsensusState),
192+
// ...
193+
}
194+
195+
#[derive(ClientState)]
196+
#[generics(
197+
ClientValidationContext = MyClientValidationContext,
198+
ClientExecutionContext = MyClientExecutionContext
199+
)]
200+
enum AnyClientState {
201+
Tendermint(TmClientState),
202+
Near(NearClientState),
203+
// ...
204+
}
205+
```
206+
207+
## FAQs
208+
209+
### Why are there 3 `ClientState` traits?
210+
211+
The `ClientState` trait is defined as
212+
213+
```rust
214+
trait ClientState<ClientValidationContext, ClientExecutionContext>
215+
```
216+
217+
The problem with defining all methods directly under `ClientState` is that it would force users to use fully qualified notation to call any method.
218+
219+
This arises from the fact that no method uses both generic parameters. [This playground] provides an explanatory example. Hence, our solution is to have all methods in a trait use every generic parameter of the trait to avoid this problem.
220+
221+
[This playground]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=da65c22f1532cecc9f92a2b7cb2d1360
222+
223+
### Why did you write custom `ClientState` and `ConsensusState` derive macros? Why not use `enum_dispatch` or `enum_delegate`?
224+
We ended up having to write our own custom derive macros because existing crates that offer similar functionality had shortcomings that prevented us from using them:
225+
226+
+ `enum_dispatch`: the trait `ClientState` and the enum that implements `ClientState` need to be defined in the same crate
227+
+ `enum_delegate` (v0.2.*): was designed to remove the above restriction. However, generic traits are not supported.
228+
+ we investigated [turning the generic types] of `ClientState` into associated types. However we were hit by the other limitation of `enum_delegate`: `ClientState` cannot have any supertrait.
229+
230+
[turning the generic types]: https://github.com/cosmos/ibc-rs/issues/296#issuecomment-1540630517
231+
## Consequences
232+
233+
### Positive
234+
+ All light clients can now be implemented in their crates without ever needing to modify ibc-rs
235+
+ Removes trait object downcasting in light client implementations
236+
+ downcasting fails at runtime; these errors are now compile-time
237+
238+
### Negative
239+
+ Increased complexity.
240+
+ Harder to document.
241+
+ 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.
242+
243+
244+
### Neutral
245+
+ Our light client traits are no longer trait-object safe. Hence, for example, all uses of `Box<dyn ConsensusState>` are replaced by the analogous `ValidationContext::AnyConsensusState`.
246+
247+
## Future work
248+
249+
In the methods `ClientState::{verify_client_message, check_for_misbehaviour, update_state, update_state_on_misbehaviour}`, the `client_message` argument is still of type `ibc_proto::google::protobuf::Any` (i.e. still serialized). Ideally, we would have it be well-typed and unserialized. Since there are many ways to do this, and this was slightly tangential to this work, we left it as future work.
250+
251+
## References
252+
253+
* [Main issue](https://github.com/cosmos/ibc-rs/issues/296)

0 commit comments

Comments
 (0)