imp(ibc-testkit): revamp mock testing framework (#1109)

* imp(testkit): Mock IBC context with `basecoin-store` types (#1068) * non empty default CommitmentPrefix * update ibc mock client types * fix tests for updated mock types * generalize MockContextConfig::into * rm clone from MockContext * add with_{client,consensus}_state * add blocks_since in utils.rs * client takes host timestamp by default * refactor relayer context test * fix few tests * add ibc-query and basecoin-store deps * add basecoin-store in mock ibc context * refactor for updated mock ibc context * imp timeout test * public git commit as dep source * fix spelling * update MockClient types * fix failing tests * rm unused utils and deps * fix cargo doc lint * use ibc host paths in mock context * rm redundant curly brackets --------- Co-authored-by: Farhad Shabani <[email protected]> * imp(ibc-testkit): generalize `Host` for `MockContext` (#1107) * fix semantic conflict * happy clippy * fix next_consensus_state impl * update prev_consensus_state impl * use self.block_time over const * use constant timestamp over dynamic now * add new host impls * changes to ibc-testkit * rm deprecated methods * refactor tests * public fields for tm block params * forged client header update test * add todo comment * minor refactor * refactor tests * builder type for light client state * minor refactor * refactor tests * code opt * rename host structs * renamings * refactor tendermint host * mv year_2023 to utils * refactor the new tests * test malicious adjacent header update * fix tests * add changelog * rm unused chain_revision_number * add doc string for year_2023 * rename to query_latest_block * feat(ibc-testkit): revamp `MockContext` with chain capabilities (#1135) * imp: impl ctxs on MockIbcStore * fix: set update_meta whenever build light client * imp: generate_client_state should take latest_height * imp: remove unnecessary latest_client_states method * refactor: migrate host relevant fields/methods * fix: get back validate_self_client * imp: bring context types up under src * rm default for CommitmentPrefix * rm default height and timestamp impl for TestBlock * default MockStore * AnyClient and AnyConsensus state in MockIbcStore * at least one consensus state when bootstrapping light client * revision number in MockIbcStore * advance height in MockIbcStore * sync host and ibc store advance in conext * add host params to build host * update host trait * convenient type generics for host associated types * update MockHost * return existing block header with correct timestamp * update TendermintHost * update MockGenericContext impl * update mock context building * add implied trait bounds * refactor method * rm redundant imports and impl * call advance_block on context * update MockContext tests * rm ClientStateCommon * use HostParams to build a host * rm using max_history_size * update few tests * ignore failing tests * clippy::use_self * clippy::flat_map_option * clippy::cloned_instead_of_copied * clippy::redundant_clone * clippy::redundant_type_annotations * clippy::as_underscore * disable slow testcase * prune old host consensus state * prune host consensus history in fixture * enable ignored test * return client_id in fixture * fix and enable failing test * avoid Arc and Mutex * fix doc build * refactor host trait and impls * into over Self::from * fix msrv * explicit relative or global path for pub use * clippy::map_identity * clippy::inconsistent_struct_constructor * clippy::std_instead_of_core * use commitment_root for block generation * rename consensus_states to host_consensus_states * use basecoin proof specs * update tests * imp: add history() under TestHost * chore: add docstring for some of methods under TestHost * update TestHost trait * update MockHost and TendermintHost * add MockGenericContext::generate_genesis_block * update MockContextConfig * update tests * update remaining tests * rm comments and rename test * rm HostParams --------- Co-authored-by: Farhad Shabani <[email protected]> * imp(ibc-testkit): Tendermint proof verifications via integration test (#1146) * test tmclient proof verification * use ClientStateConfig directly * rm MockClientConfig * use basecoin store proofspec as default * update tests * use merkle storage in MockContext * fix ProofSpecs size * refactor MockIbcStore to perform begin_block and end_block * simpler proof verification test * use ValidationContext::commitment_prefix() * nits * refactor host related trait method * tendermint host client integration test with proof verification * rm raw test * use typed relayer * add todo for channel integration test * core over std * be semantically correct * add comment for TypedRelayer * integration test for all pairs * fix semantic bug * renames * add channel management * channel creation in RelayerContext * add channel creation in integration test * add test for channel close * query client_id from connection and channel * ibc_store_mut * utils functions for packet relay * add packet relay integration test * add comments * optimize integration utils functions * serde feature for integration tests * rm redundant chain_id * sync clock only on a * add comment * imp: place router under MockGenericContext * nit: add docstring for router * nits * rm redundant lint filters * imp: ditch RelayerContext * nit: simplify build_client_update_datagram * refactor integration tests * add doc strings for TypedRelayerOps * doc strings for RelayerContext * mv client_update_ping_pong to tests dir * rename main_store to multi_store * update TestHost trait * update mock and tendermint hosts * update relayer functions * nits * renames and comments * add comments for return values in relayer ops * imp: simplify into_header --------- Co-authored-by: Farhad Shabani <[email protected]> * rm prune_block_till * apply suggestions from code review Co-authored-by: Sean Chen <[email protected]> Signed-off-by: Rano | Ranadeep <[email protected]> * rename context structs * add MockContext and TendermintContext * update with MockContext and TendermintContext * MockContextConfig to TestContextConfig * update comments with new namings * add util methods for StoreGenericTestContext * fix failing client recovery tests * increase default trusting period of MockClientState * fallible ProofSpecs conversion * update with new ClientStateConfig * use ExtClientValidationContext * fix links in doc comments * fix import order * fix semantic conflicts * Convertible over ConsensusStateConverter * rm ConsensusStateConverter * ibc-query as workspace deps * update Convertible * fix failing test * update changelog entry * add comments * rm absolute import * use expect over comment for safety * infallible try_into * update method names * add unused variable names * rm redundant TendermintBlock wrapper type * add comments * update docstrings * rm should never fail comment * update adr * fix compilation * fix failing test * docs: ADR-009 to revamp testing framework (#1157) * draft adr 009 * update adr-009 * apply suggestions from pr review Co-authored-by: Sean Chen <[email protected]> Signed-off-by: Rano | Ranadeep <[email protected]> * use numbers list for sub-proposal titles Co-authored-by: Sean Chen <[email protected]> Signed-off-by: Rano | Ranadeep <[email protected]> * links to validation and execute context Co-authored-by: Sean Chen <[email protected]> Signed-off-by: Rano | Ranadeep <[email protected]> * apply suggestion from pr review * add links to impls * update comment * markdown format * update adr * use TestContext * update comment * apply suggestions from code review Co-authored-by: Sean Chen <[email protected]> Signed-off-by: Rano | Ranadeep <[email protected]> * markdown format * update method names * fix cargo doc error --------- Signed-off-by: Rano | Ranadeep <[email protected]> Co-authored-by: Sean Chen <[email protected]> --------- Signed-off-by: Rano | Ranadeep <[email protected]> Signed-off-by: Rano | Ranadeep <[email protected]> Co-authored-by: Farhad Shabani <[email protected]> Co-authored-by: Sean Chen <[email protected]>
cosmos · May 3, 2024 · 534a465 · 534a465
1 parent 87ec770
commit 534a465
Show file tree

Hide file tree

Showing 56 changed files with 5,717 additions and 2,840 deletions.
diff --git a/.changelog/unreleased/improvements/1044-introduce-host-trait.md b/.changelog/unreleased/improvements/1044-introduce-host-trait.md
@@ -0,0 +1,3 @@
+- [ibc-testkit] Replace `HostBlock` and `HostType` enums with a `Host` trait to
+  eliminate manual delegations by utilizing monomorphization.
+  ([\#1044](https://github.com/cosmos/ibc-rs/issues/1044))
diff --git a/Cargo.toml b/Cargo.toml
@@ -80,6 +80,7 @@ ibc-core-channel    = { version = "0.52.0", path = "./ibc-core/ics04-channel", d
 ibc-core-host       = { version = "0.52.0", path = "./ibc-core/ics24-host", default-features = false }
 ibc-core-handler    = { version = "0.52.0", path = "./ibc-core/ics25-handler", default-features = false }
 ibc-core-router     = { version = "0.52.0", path = "./ibc-core/ics26-routing", default-features = false }
+ibc-query           = { version = "0.52.0", path = "./ibc-query", default-features = false }
 
 ibc-client-cw            = { version = "0.52.0", path = "./ibc-clients/cw-context", default-features = false }
 ibc-client-tendermint    = { version = "0.52.0", path = "./ibc-clients/ics07-tendermint", default-features = false }

diff --git a/docs/architecture/adr-005-handlers-redesign.md b/docs/architecture/adr-005-handlers-redesign.md
@@ -421,7 +421,6 @@ trait ExecutionContext {
 
     /// Called upon client creation.
     /// Increases the counter which keeps track of how many clients have been created.
-    /// Should never fail.
     fn increase_client_counter(&mut self);
 
     /// Called upon successful client update.
@@ -452,7 +451,6 @@ trait ExecutionContext {
 
     /// Called upon connection identifier creation (Init or Try process).
     /// Increases the counter which keeps track of how many connections have been created.
-    /// Should never fail.
     fn increase_connection_counter(&mut self);
 
     fn store_packet_commitment(
@@ -514,7 +512,6 @@ trait ExecutionContext {
 
     /// Called upon channel identifier creation (Init or Try message processing).
     /// Increases the counter which keeps track of how many channels have been created.
-    /// Should never fail.
     fn increase_channel_counter(&mut self);
 
     /// Ibc events

diff --git a/docs/architecture/adr-009-revamp-testkit.md b/docs/architecture/adr-009-revamp-testkit.md
@@ -0,0 +1,349 @@
+# ADR 009: Revamp IBC integration test framework
+
+## Changelog
+
+- 04-03-2024: Initial draft
+
+## Context
+
+The current framework in the IBC testkit uses
+[existing types and injects state and/or dependencies
+manually](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/tests/core/ics02_client/update_client.rs#L574-L578).
+Sometimes, it uses
+[semantically incorrect data as mock data](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/src/testapp/ibc/core/types.rs#L320).
+Because of this, tests with customizable steps and fixed fixtures became ad-hoc
+and unmaintainable.
+
+To overcome this, we need to improve our test framework such that it allows:
+
+- testing different implementations and traits.
+- composition of succinct tests with useful `util` methods.
+- validating code paths that were not easily testable before, i.e. Merkle proof
+  generation.
+- simulating a more realistic IBC workflow, using real IBC and relayer
+  interfaces.
+
+## Decision
+
+The main goal of this proposal is to create a test framework that is modular and
+closer to a real blockchain environment. This should also make the existing
+tests more succinct and readable. Instead of bootstrapping the mock data that
+tests use, we should use valid steps to generate it - so that we know the exact
+steps to reach a state to reproduce in a real environment.
+
+To achieve this, we have broken down the proposal into sub-proposals:
+
+### 1. Adopt a Merkle store for the test framework
+
+The current framework uses `HashMap` and `HashSet` to store data. This works for
+many test scenarios, but it fails to test proof-sensitive scenarios. Because of
+this, we don't have any connection, channel handshake, or packet relay tests
+that cover the Tendermint light client.
+
+We generalize
+[`MockContext`](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/src/testapp/ibc/core/types.rs#L103)
+to use a Merkle store which is used for IBC Context's store. For concrete or
+default implementations, we can use the IAVL Merkle store implementation from
+`informalsystems/basecoin-rs`.
+
+### 2. Modularize the host environment
+
+Currently, we are using `Mock` and `SyntheticTendermint` variants of the
+[`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 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.
+
+This `TestHost` trait should be responsible for generating blocks, headers,
+client, and consensus states specific to that host environment.
+
+```rs
+/// TestHost is a trait that defines the interface for a host blockchain.
+pub trait TestHost: Default + Debug + Sized {
+    /// The type of block produced by the host.
+    type Block: TestBlock;
+
+    /// The type of client state produced by the host.
+    type ClientState: Into<AnyClientState> + Debug;
+
+    /// The type of block parameters to produce a block.
+    type BlockParams: Debug + Default;
+
+    /// The type of light client parameters to produce a light client state.
+    type LightClientParams: Debug + Default;
+
+    /// The history of blocks produced by the host chain.
+    fn history(&self) -> &Vec<Self::Block>;
+
+    /// Commit a block with commitment root to the blockchain, by extending the history of blocks.
+    fn commit_block(
+        &mut self,
+        commitment_root: Vec<u8>,
+        block_time: Duration,
+        params: &Self::BlockParams,
+    );
+
+    /// Generate a block at the given height and timestamp, using the provided parameters.
+    fn generate_block(
+        &self,
+        commitment_root: Vec<u8>,
+        height: u64,
+        timestamp: Timestamp,
+        params: &Self::BlockParams,
+    ) -> Self::Block;
+
+    /// Generate a client state using the block at the given height and the provided parameters.
+    fn generate_client_state(
+        &self,
+        latest_height: &Height,
+        params: &Self::LightClientParams,
+    ) -> Self::ClientState;
+}
+
+/// TestBlock is a trait that defines the interface for a block produced by a host blockchain.
+pub trait TestBlock: Clone + Debug {
+    /// The type of header that can be extracted from the block.
+    type Header: TestHeader;
+
+    /// The height of the block.
+    fn height(&self) -> Height;
+
+    /// The timestamp of the block.
+    fn timestamp(&self) -> Timestamp;
+
+    /// Extract the header from the block.
+    fn into_header(self) -> Self::Header;
+}
+
+/// TestHeader is a trait that defines the interface for a header
+/// submitted by relayer from the host blockchain.
+pub trait TestHeader: Clone + Debug + Into<Any> {
+    /// The type of consensus state can be extracted from the header.
+    type ConsensusState: ConsensusState + Into<AnyConsensusState> + From<Self> + Clone + Debug;
+
+    /// The height of the block, as recorded in the header.
+    fn height(&self) -> Height;
+
+    /// The timestamp of the block, as recorded in the header.
+    fn timestamp(&self) -> Timestamp;
+
+    /// Extract the consensus state from the header.
+    fn into_consensus_state(self) -> Self::ConsensusState;
+}
+```
+
+### 3. Decoupling IbcContext and Host environment
+
+Currently, `MockContext` implements the top-level
+[validation](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-core/ics25-handler/src/entrypoint.rs#L45)
+and
+[execution](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-core/ics25-handler/src/entrypoint.rs#L112)
+contexts of `ibc-rs`, as opposed to the more granular contexts of each of the
+individual handlers. It contains other host-specific data e.g. `host_chain_id`,
+`block_time` - that are not directly relevant to the IBC context. If we think of
+`MockContext` as a real blockchain context, the `MockContext` represents the
+top-level runtime; it contains `MockIbcStore`, which is a more appropriate
+candidate to implement the validation and execution contexts for than the
+`MockContext` itself.
+
+With this, the `MockContext` contains two decoupled parts - the host and the IBC
+module.
+
+### 4. Chain-like interface for `MockContext`
+
+With the above changes, we can refactor the `MockContext` to have
+blockchain-like interfaces.
+
+The `MockContext` should have `end_block`, `produce_block`, and `begin_block` to
+mimic real blockchain environments, such as the Cosmos-SDK.
+
+```rs
+impl<S, H> MockContext<S, H>
+where
+    S: ProvableStore + Debug,
+    H: TestHost,
+{
+    // Access ibc module store
+    pub fn ibc_store_mut(&mut self) -> &mut MockIbcStore<S>;
+
+    // Advance the first block height.
+    pub fn advance_genesis_height(&mut self, genesis_time: Timestamp);
+    // Routine procedures at the beginning of a block
+    // Just after committing last block state.
+    pub fn begin_block(&mut self);
+    // Advance the current block height.
+    pub fn advance_block_height(&mut self, block_time: Duration);
+    // Routine procedures at the end of a block
+    // Just before committing current block state.
+    pub fn end_block(&mut self);
+}
+```
+
+### 5. ICS23 compatible proof generation
+
+With the new proof generation capabilities, we can now test the Tendermint light
+clients. But we need our proofs to be ICS23 compatible. ICS23 expects the IBC
+store root to be committed at a commitment prefix at a top-level store in the
+host environment.
+
+For this, we add an extra store in `MockContext` where the `MockIbcStore`
+commits its storage root at its
+[commitment prefix](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/src/testapp/ibc/core/core_ctx.rs#L127-L129)
+key.
+
+So the `MockContext` is finalized as:
+
+```rs
+pub struct MockContext<S, H>
+where
+    S: ProvableStore + Debug,
+    H: TestHost
+{
+    pub main_store: S,
+    pub host: H,
+    pub ibc_store: MockIbcStore<S>,
+}
+```
+
+Now the `MockIbcStore` can generate proofs that contain the proofs in its store
+and commitment prefix. But it has to know the proofs of its commitment prefix of
+the previous heights.
+
+So we add an extra store in `MockIbcStore` to store the proofs from previous
+heights. This is similar to storing `HostConsensusState`s of previous heights.
+
+```rs
+#[derive(Debug)]
+pub struct MockIbcStore<S>
+where
+    S: ProvableStore + Debug,
+{
+    ...
+    /// Map of host consensus states.
+    pub host_consensus_states: Arc<Mutex<BTreeMap<u64, AnyConsensusState>>>,
+    /// Map of proofs of ibc commitment prefix.
+    pub ibc_commiment_proofs: Arc<Mutex<BTreeMap<u64, CommitmentProof>>>,
+}
+```
+
+The storing of the IBC store root at the IBC commitment prefix happens in the
+`end_block` procedure. `produce_block` commits the main store, produces a block
+with its latest root, and pushes the block to the blockchain. The storing of
+proofs and host consensus states happens in the `begin_block` of the
+`MockContext`.
+
+### 6. Integration Tests via `RelayerContext`
+
+With all the above changes, we can now write an integration test that tests the
+IBC workflow - client creation, connection handshake, channel handshake, and
+packet relaying for any host environment that implements `TestHost`.
+
+This can be done by reading the
+[IBC events](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/src/testapp/ibc/core/types.rs#L95)
+from `MockIbcStore` and creating and sending the IBC messages via
+[`MockContext::deliver`](https://github.com/cosmos/ibc-rs/blob/v0.51.0/ibc-testkit/src/testapp/ibc/core/types.rs#L696).
+
+### Miscellaneous
+
+To achieve blockchain-like interfaces, we removed `max_history_size` and
+`host_chain_id` from `MockContext`.
+
+- `max_history_size`: We generate all the blocks till a block height. This gives
+  us reproducibility. If we need to prune some older block data, we use a
+  dedicated `prune_block_till` to prune older blocks. This makes our tests more
+  descriptive about the assumption of the test scenarios.
+- `host_chain_id`: The IBC runtime does not depend on `host_chain_id` directly.
+  The `TestHost` trait implementation is responsible for generating the blocks
+  with the necessary data.
+
+Also to minimize verbosity while writing tests (as Rust doesn't support default
+arguments to function parameters), we want to use some parameter builders. For
+that, we can use the [`TypedBuilder`](https://crates.io/crates/typed-builder)
+crate.
+
+## Status
+
+Proposed
+
+## Consequences
+
+This ADR pays the technical debt of the existing testing framework.
+
+### Positive
+
+Future tests will be more readable and maintainable. The test framework becomes
+modular and leverages Rust's trait system. `ibc-rs` users may benefit from this
+framework, which allows them to test their host implementations of `ibc-rs`
+components.
+
+### Negative
+
+This requires a significant refactoring of the existing tests. Since this may
+take some time, the parallel development on the `main` branch may conflict with
+this work.
+
+## References
+
+This work is being tracked at
+[cosmos/ibc-rs#1109](https://github.com/cosmos/ibc-rs/pull/1109).
+
+The following provides the concrete implementations of the proposed changes:
+
+#### MockIbcStore
+
+The modified `MockIbcStore` with Merkle store lives at
+[`testapp/ibc/core/types.rs`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/testapp/ibc/core/types.rs#L43-L96).
+
+#### TestHost
+
+The Rust trait lives at
+[`hosts/mod.rs`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/hosts/mod.rs#L27).
+The `Mock` and `Tendermint` host implementations live in
+[`hosts/mock.rs`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/hosts/mock.rs#L30)
+and
+[`hosts/tendermint.rs`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/hosts/tendermint.rs#L42)
+respectively.
+
+#### Renaming `MockContext` to `StoreGenericTestContext`
+
+There was confusion about what is a _Test_ component and what is a _Mock_
+component. We have `MockContext` with `MockClient` and `TendermintClient`.
+
+To avoid this confusion, we renamed `MockContext` to `StoreGenericTestContext`.
+This means that going forward all our general frameworks and traits should have
+`Test` in their name. But a dummy concrete implementation of these traits may
+have `Mock` in their name.
+
+#### StoreGenericTestContext
+
+[`StoreGenericTestContext`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/context.rs#L34-L52)
+is actually what is described as `MockContext` in the ADR. For convenience, we
+defined `TestContext` to have a concrete store implementation -
+[`MockStore`](https://github.com/cosmos/ibc-rs/blob/feat/refactor-testkit/ibc-testkit/src/context.rs#L55-L56).
+
+```rs
+// A mock store type using basecoin-storage implementations.
+pub type MockStore = RevertibleStore<GrowingStore<InMemoryStore>>;
+
+pub type TestContext<H> = StoreGenericTestContext<MockStore, H>;
+```
+
+With this, we can now define `MockContext` which uses `MockStore` and `MockHost`
+and `TendermintContext` which uses `MockStore` and `TendermintHost`.
+
+```rs
+pub type MockContext = TestContext<MockHost>;
+pub type TendermintContext = TestContext<TendermintHost>;
+```