From d86ddadbf8d1b01c2e0015428f39b23b5043f1fa Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Mon, 8 Apr 2024 11:44:51 +0200 Subject: [PATCH] apply suggestion from pr review --- docs/architecture/adr-009-revamp-testkit.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/architecture/adr-009-revamp-testkit.md b/docs/architecture/adr-009-revamp-testkit.md index fe56ce8d05..9bc542eb24 100644 --- a/docs/architecture/adr-009-revamp-testkit.md +++ b/docs/architecture/adr-009-revamp-testkit.md @@ -46,16 +46,18 @@ default implementations, we can use the IAVL Merkle store implementation from ### 2. Modularize the host environment -Currently, we are using `Mock` and `SyntheticTendermint` -[variants](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/src/hosts/block.rs#L33-L36) -as host environments. To manage these two different environments, we also +Currently, we are using `Mock` and `SyntheticTendermint` variants of +[`HostType`](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/src/hosts/block.rs#L33-L36) +enum as host environments. To manage these two different environments, we also introduced [`HostBlocks`](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/src/hosts/block.rs#L72-75) for encapsulating the possible host variants that need to be covered by `ibc-testkit`. However, this creates friction in the case when we need to add new host variants. It creates the same -problem as why we have `ibc-derive` crate for `ClientState` and -`ConsensusState`. This should be refactored by a generic `TestHost` trait that +problem that the `ibc-derive` crate was designed to solve for `ClientState` and +`ConsensusState` types, namely: dispatching methods to underlying variants of a top-level enum. +But, a concrete `MockContext` will always have a unique `HostType` and its corresponding `HostBlocks`. +So we can refactor `HostTypes` and `HockBlocks` with a generic `TestHost` trait that maintains its corresponding types e.g. `Block` types, via associated types. Finally, we generalize the `MockContext` once more to use this `TestHost` trait instead of a concrete enum variant.