imp(testkit): Mock IBC context with `basecoin-store` types #1068

rnbguy · 2024-01-30T12:13:24Z

Closes: #1045
Closes: #1052

Description

This PR adopts basecoin-store in ibc-testkit. Several refactors have been performed in the current code base to achieve this. To avoid breaking the tests, the reactors are committed first, followed by the main refactor, adopting base coin-store types.

This PR uses two unpublished crates.

The local paths of the workspace of these crates are expected to be at the same directory level as ibc-rs

$ git clone github.com/cosmos/ibc-rs
$ cd ibc-rs
$ ls ../
basecoin-rs
ibc-proto-rs
ibc-rs

PR author checklist:

Added changelog entry, using unclog.
Added tests.
Linked to GitHub issue.
Updated code comments and documentation (e.g., docs/).
Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

Reviewed Files changed in the GitHub PR explorer.
Manually tested (in case integration/unit/mock tests are absent).

rnbguy · 2024-01-30T17:23:36Z

There are few future PRs from this.

store type and MockContext::history - should be maintained by one single MockChainContext.
advance_host_chain_height should apply commit on store type so that its height advances too.
generate real blocks using tendermint_testgen::Header passing root_hash of underlying store as an argument to app_hash parameter.

After finishing these, we can start writing ics03 and ics04 tests with Tendermint client types - mainly, start testing verify_membership implementation of Tendermint light client.

there should be a few code-quality PRs

remove all dummy functions
move all *Config structs to the fixture module.

Farhad-Shabani

Fantastic job, @rnbguy! This gets us ready for a powerful ibc-testkit 🚀.
Had a cursory look. Overall sounds great. I’ll have another look once we finalize the mock proto definitions.

ibc-testkit/src/testapp/ibc/utils.rs

Cargo.toml

ibc-core/ics23-commitment/types/src/commitment.rs

ibc-testkit/src/testapp/ibc/core/types.rs

ibc-testkit/tests/core/ics04_channel/timeout.rs

ibc-testkit/src/testapp/ibc/core/types.rs

codecov · 2024-02-02T10:00:49Z

Codecov Report

Attention: 396 lines in your changes are missing coverage. Please review.

Comparison is base (4428fa9) 66.63% compared to head (297f278) 66.07%.

Files	Patch %	Lines
ibc-testkit/src/testapp/ibc/core/core_ctx.rs	41.86%	311 Missing ⚠️
ibc-testkit/src/testapp/ibc/core/client_ctx.rs	73.40%	50 Missing ⚠️
ibc-testkit/src/testapp/ibc/core/types.rs	81.42%	26 Missing ⚠️
...stkit/src/testapp/ibc/clients/mock/client_state.rs	78.26%	5 Missing ⚠️
ibc-testkit/src/testapp/ibc/clients/mock/header.rs	66.66%	3 Missing ⚠️
ibc-testkit/src/fixtures/mod.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@                   Coverage Diff                   @@
##           feat/basecoin-store    #1068      +/-   ##
=======================================================
- Coverage                66.63%   66.07%   -0.56%     
=======================================================
  Files                      204      205       +1     
  Lines                    20435    20775     +340     
=======================================================
+ Hits                     13617    13728     +111     
- Misses                    6818     7047     +229

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rnbguy · 2024-02-06T19:21:50Z

@Farhad-Shabani I updated the MockIbcStore in 78cb644 with the new ibc host paths. The tests are passing.

We may need to add some new error variants.
Interesting that ibc-rs expects to store the current {Client,Connection,Channel}Sequence instead of next. Look how they needed to be initialized with 0, instead of 1.

Farhad-Shabani · 2024-02-07T04:40:25Z

We may need to add some new error variants.

...I am becoming more certain that it's crucial to refactor our error system as soon as we can.

Interesting that ibc-rs expects to store the current {Client,Connection,Channel}Sequence instead of next. Look how they needed to be initialized with 0, instead of 1.

In the genesis, where no clients, connections, or channels exist, anything created subsequently will be assigned the number 0. Hence, 0 is the next.

* 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 <farhad.shabani@gmail.com> * 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 <farhad.shabani@gmail.com> * 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 <farhad.shabani@gmail.com> * rm prune_block_till * apply suggestions from code review Co-authored-by: Sean Chen <seanchen11235@gmail.com> Signed-off-by: Rano | Ranadeep <ranadip.bswas@gmail.com> * 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 <seanchen11235@gmail.com> Signed-off-by: Rano | Ranadeep <ranadip.bswas@gmail.com> * use numbers list for sub-proposal titles Co-authored-by: Sean Chen <seanchen11235@gmail.com> Signed-off-by: Rano | Ranadeep <ranadip.bswas@gmail.com> * links to validation and execute context Co-authored-by: Sean Chen <seanchen11235@gmail.com> Signed-off-by: Rano | Ranadeep <ranadip.bswas@gmail.com> * 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 <seanchen11235@gmail.com> Signed-off-by: Rano | Ranadeep <ranadip.bswas@gmail.com> * markdown format * update method names * fix cargo doc error --------- Signed-off-by: Rano | Ranadeep <ranadip.bswas@gmail.com> Co-authored-by: Sean Chen <seanchen11235@gmail.com> --------- Signed-off-by: Rano | Ranadeep <ranadip.bswas@gmail.com> Signed-off-by: Rano | Ranadeep <ranadeep@informal.systems> Co-authored-by: Farhad Shabani <farhad.shabani@gmail.com> Co-authored-by: Sean Chen <seanchen11235@gmail.com>

rnbguy added 13 commits January 30, 2024 10:38

non empty default CommitmentPrefix

0d592b7

update ibc mock client types

9bc3187

fix tests for updated mock types

e389a9f

generalize MockContextConfig::into

8dc3299

rm clone from MockContext

e9d0722

add with_{client,consensus}_state

aa44d48

add blocks_since in utils.rs

8a8c448

client takes host timestamp by default

e213d2d

refactor relayer context test

0f80861

fix few tests

c65c053

add ibc-query and basecoin-store deps

68d8ab0

add basecoin-store in mock ibc context

bbec93a

refactor for updated mock ibc context

16f6d60

rnbguy requested a review from Farhad-Shabani January 30, 2024 12:13

Farhad-Shabani requested changes Feb 1, 2024

View reviewed changes

imp timeout test

1de553c

rnbguy force-pushed the rano/testkit/basecoin-store branch from 0de4fd5 to 1de553c Compare February 1, 2024 08:43

rnbguy added 5 commits February 2, 2024 10:41

public git commit as dep source

6061b74

fix spelling

e006151

update MockClient types

173ff7b

fix failing tests

0c9bee1

rm unused utils and deps

ebf3019

rnbguy added 3 commits February 2, 2024 11:01

fix cargo doc lint

4bb11d6

Merge branch 'main' into rano/testkit/basecoin-store

7e6913a

use ibc host paths in mock context

78cb644

rnbguy and others added 2 commits February 6, 2024 20:35

rm redundant curly brackets

283a9de

Merge branch 'main' into rano/testkit/basecoin-store

297f278

Farhad-Shabani marked this pull request as ready for review February 7, 2024 04:40

Farhad-Shabani approved these changes Feb 7, 2024

View reviewed changes

Farhad-Shabani merged commit 1c3ba50 into feat/basecoin-store Feb 7, 2024
15 checks passed

Farhad-Shabani deleted the rano/testkit/basecoin-store branch February 7, 2024 04:41

This was referenced Feb 29, 2024

MockClientState to RawMockClientState conversion ignores frozen_height #1052

Closed

Revamp mock testing artifacts in the ibc-testkit #677

Closed

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

imp(testkit): Mock IBC context with `basecoin-store` types #1068

imp(testkit): Mock IBC context with `basecoin-store` types #1068

rnbguy commented Jan 30, 2024 •

edited

Loading

rnbguy commented Jan 30, 2024 •

edited

Loading

Farhad-Shabani left a comment

codecov bot commented Feb 2, 2024 •

edited

Loading

rnbguy commented Feb 6, 2024 •

edited

Loading

Farhad-Shabani commented Feb 7, 2024

imp(testkit): Mock IBC context with basecoin-store types #1068

imp(testkit): Mock IBC context with basecoin-store types #1068

Conversation

rnbguy commented Jan 30, 2024 • edited Loading

Description

PR author checklist:

Reviewer checklist:

rnbguy commented Jan 30, 2024 • edited Loading

Farhad-Shabani left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 2, 2024 • edited Loading

Codecov Report

rnbguy commented Feb 6, 2024 • edited Loading

Farhad-Shabani commented Feb 7, 2024

imp(testkit): Mock IBC context with `basecoin-store` types #1068

imp(testkit): Mock IBC context with `basecoin-store` types #1068

rnbguy commented Jan 30, 2024 •

edited

Loading

rnbguy commented Jan 30, 2024 •

edited

Loading

codecov bot commented Feb 2, 2024 •

edited

Loading

rnbguy commented Feb 6, 2024 •

edited

Loading