fix(ics20): correct balance accounting issue with test coverage #219
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Make gas limit configurable for testing lower gas limit scenario - Get ibctesting.AppCreator when create coordinator, so other applications can leverage this test suits. - Make GenerateContractCallArgs as separate function. This doens't have to be a member method of TxFactory implementation. un-necessary dependency. - SendEvmTx now supports contract creation.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Previous ibc test suite uses different bond denom ("stake) with EVM denom ("aatom"). Even though it could be different technically, but we need to make sure everything works correctly with default config (bond denom == evm denom).
Wit this commit, we can make sure IBC and ICS20 transfers with default config should work well.
Updated test util for writing test cases more easily.
zsystm
changed the title
zsystm
changed the title
Currently, Arguments.Copy in geth panics if there are type mismatch. Returning error instead makes more sense
9 tasks
vladjdk
approved these changes
Jun 19, 2025
|
|
||
| // Run Ginkgo integration tests | ||
| RegisterFailHandler(Fail) | ||
| RunSpecs(t, "Distribution Precompile Suite") |
| return len(code) > 0 | ||
| } | ||
|
|
||
| _ = Describe("Calling ICS20 precompile", func() { |
There was a problem hiding this comment.
Rename this to Calling ICS20 Precompile from callerContract.
We'll eventually need to add EOA calls to tests as well
There was a problem hiding this comment.
Applied in 496af19
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a bug in the ICS20 precompile where the balance delta reflected in the stateDB did not correctly account for the adjusted amount when the input transfer amount was set to
UnboundedSpendLimit.In such cases, the actual amount transferred is reduced to the sender’s spendable balance by the IBC transfer module internally, but the precompile logic still emitted balance change entries based on the original (unbounded) value. This could lead to inconsistencies between the Cosmos bank state and the EVM stateDB.
Fix Summary
The ics20 precompile now mirrors the same adjustment logic
so that
SetBalanceChangeEntriescorrectly reflects the actual transferred amount.Test Covergage Improvements
In addition to the fix, this PR significantly improves the test coverage of the ICS20 precompile
Ensured test environment uses a consistent configuration with the same denom for both staking and EVM (
bond_denom == evm_denom) to avoid confusion in test assumptions and better align with default settings.Test Utility Enhancements
GenerateContractCallArgsas a standalone function to remove unnecessary dependency coupling from TxFactory.SendEvmTxto support contract creation and configurable gas limits, enabling more flexible test case writing (e.g., low gas limit failure cases).Closes: #199 #221
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
mainbranchReviewers Checklist
All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.
I have...
Unreleasedsection inCHANGELOG.md