Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: ibc module testing suite and implementation #4

Merged
merged 11 commits into from
Sep 22, 2023

Conversation

Farhad-Shabani
Copy link
Member

@Farhad-Shabani Farhad-Shabani commented Sep 14, 2023

Background

In this PR, a comprehensive test suite has been established, encompassing the setup of mock Sovereign SDK chain, Cosmos chain, and a relayer, all organized under the test_utils module.
These components enable us to instantiate a versatile testing environment capable of accommodating various cases that can be plugged into the tests under the tests.rs module.

Design Approach

The approach taken in developing this test suite closely aligns with the methodology employed in the implementation of relayer-cosmos-mock as presented in this PR.
This enables us to harness the capabilities of the relayer-next framework in future work, where we can implement interfaces on MockRelayer and take advantage of blanket and default implementations, as well as leverage the pre-existing components available there. This eventually will facilitate the integration of the entire IBC system, particularly when dealing with complex testing scenarios.

Featured tests

Within the context of this PR, we cover the following scenarios for mock sovereign chains:

More comprehensive and thorough tests, (as suggested by @plafer here) can be deferred to a subsequent PR to maintain a manageable scope.

To be handled

  • Some fields in the Transfer struct are currently made public. However, once sov-transfer is merged into sov-ibc, these fields can be reverted to their original non-public state. Should be noted that the transfer module depends on ibc-core and is not usable by other modules independently.
  • The mock cosmos chain already depends to basecoin-rs application and store types, which should ideally be decoupled after developing a new mock module in ibc-rs.
  • Good to define a simple error type for tests

@plafer
Copy link

plafer commented Sep 20, 2023

Critical ICS-20 tests we need:

Test sending tokens with the same name

  1. On chain A, create 2 tokens that have the same token name (e.g. token_A).
  2. send a different amount of each token to chain B. Verify that the expected amount of each respective token was sent.
  3. On chain B, send both tokens back.
  4. On chain A, verify that the expected amount of each respective token was received.

Test receiving a token where its IBC denom is the same as an existing token on the chain

  1. On chain A, create a token named e.g. transfer/channel-1/tokenA (assuming the port/channel name for the ICS-20 app is transfer and channel-1) - call it "EXISTING TOKEN".
  2. On chain B, send a token named tokenA to port/channel such that it is received on chain A on port transfer, channel channel-1.
  3. Ensure that a new token is created (named transfer/channel-1/tokenA); that it is distinct from the one already named transfer/channel-1/tokenA. Call it "NEW TOKEN".
  4. Send an amount of NEW TOKEN back to chain B, and ensure that the expected amount of NEW TOKEN was sent.

We also want to ensure that the amount of EXISTING TOKEN was unchanged throughout this whole interaction.

@Farhad-Shabani Farhad-Shabani marked this pull request as ready for review September 22, 2023 15:08
Copy link

@plafer plafer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌

@plafer plafer merged commit 54db60b into plafer/ibc-integration Sep 22, 2023
@Farhad-Shabani Farhad-Shabani deleted the farhad/setup-ibc-tests branch September 22, 2023 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants