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

test: adding fee middleware tests for ics27 interchain accounts #1433

Merged
merged 17 commits into from
Jun 8, 2022

Conversation

damiannolan
Copy link
Member

Description

  • Adding fee middleware integration for interchain accounts

needs: #1432
closes: #1328


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@damiannolan
Copy link
Member Author

damiannolan commented May 25, 2022

Note this PR includes all the additions from #1432 as they're necessary to put the tests in place.
There is really one file of interest for this PR - apps/29-fee/ica_test.go which adds a test similar to transfer_test.go.
The test is a little verbose but essentially follows the flow:

  • Create fee enabled interchain accounts channel
  • Assert channel ends are both fee enabled
  • Escrow a packet fee, assert the pre escrow balance against the post escrow balance
  • Fund the interchain account wallet on host chain and build an ICA packet with a basic staking delegation msg
  • Relay the packet and assert the account balances after packet fee distribution

@codecov-commenter
Copy link

codecov-commenter commented May 25, 2022

Codecov Report

Merging #1433 (ee6b065) into main (042d818) will decrease coverage by 0.06%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1433      +/-   ##
==========================================
- Coverage   80.29%   80.23%   -0.07%     
==========================================
  Files         166      166              
  Lines       12143    12136       -7     
==========================================
- Hits         9750     9737      -13     
- Misses       1933     1941       +8     
+ Partials      460      458       -2     
Impacted Files Coverage Δ
...ules/apps/27-interchain-accounts/types/metadata.go 83.50% <0.00%> (-9.60%) ⬇️
...7-interchain-accounts/controller/keeper/account.go 77.77% <100.00%> (+5.05%) ⬆️
...7-interchain-accounts/controller/ibc_middleware.go 83.33% <0.00%> (-3.71%) ⬇️

DefaultPortID, _ = icatypes.NewControllerPortID(DefaultOwnerAddress)

// DefaultICAVersion defines a resuable interchainaccounts version string for testing purposes
DefaultICAVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&icatypes.Metadata{
Copy link
Member

Choose a reason for hiding this comment

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

Would it be useful to create a helper function in types that creates the default ICA version given the connection IDs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done ✅

Copy link
Contributor

@crodriguezvega crodriguezvega left a comment

Choose a reason for hiding this comment

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

It's a great test! 👍

modules/apps/29-fee/ica_test.go Show resolved Hide resolved
modules/apps/29-fee/ica_test.go Outdated Show resolved Hide resolved
@damiannolan damiannolan enabled auto-merge (squash) June 8, 2022 10:28
@damiannolan damiannolan merged commit 7999001 into main Jun 8, 2022
@damiannolan damiannolan deleted the damian/1328-ica-fee-tests branch June 8, 2022 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add fee middleware tests for interchain accounts
4 participants