feat!: IBC v2 #95
Merged
feat!: IBC v2 #95
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
This was
linked to
issues
Apr 24, 2025
* bump up ibc-go from v8 to v10 - bumped up ibc-go from v8 to v10 - removed unused ibc test codes because bumping unused testing codes are wasting time. we should use ibc testing package instead. * add ibc v1 transfer test added ibc v1 test cases to make sure ExampleChain works with ibc v1. disabled basefee param as default for ExampleChain to make test easier. * add ibc v2 components (module, middleware) - added ibc v2 components - copied basic v2 test cases from ibc-go v10 to make sure v2 components of ExampleChain works well. * add nil checks and convert erc20 keeper to interface in IBCMiddleware - added nil checks for app and keeper in NewIBCMiddleware to prevent nil pointer dereference. - changed erc20 keeper from struct to interface type to enable proper nil checking. * copy and modify from ibc-go testing To test certain key scenarios involving EVM messages (e.g., deploying an ERC20 contract), we needed a block header context with a proposer address. This required: - A custom `TestChain` to handle these messages. - A custom `SignAndDeliver` function to support the transaction signing and delivery process. - A custom `Coordinator` to integrate this tailored `TestChain`. Since `TestChain` and `SignAndDeliver` are directly or indirectly tied to most components in the testing package, and ibc-go cannot use a `TestChain` struct defined in our separate package, we had to copy and adapt nearly all related files to ensure compatibility and functionality. * fix: ci issues * replace deprecated functions * revert disable base fee Disabling the base fee to simplify testing is not ideal for a reference chain intended for developers building EVM-compatible chains. Ethereum relies on a fee market mechanism, and as a reference implementation, this chain should enable it by default to align with expected behavior. * update TestGetReceivedCoin Updated TestGetReceivedCoin to use the newly introduced interface instead of hardcoded strings. This improves maintainability by making the test logic more aligned with the actual send/receive flow. It also enhances readability and helps developers better understand how denoms are constructed and handled in real IBC transfers. * bump up ibc-go from v10.1.0 to v10.1.1
* add basic test cases for ibc middleware v1, v2 more test cases will be added. * add test cases for ibc middleware v2 * add test cases for ibc middleware v1 and post state check * add OnAcknowledgementPacket tc for v1 ibc middleware * add OnTimeoutPacket tc for v1 ibc middleware * chore: unify variable names * use internal testing pkg and add TestOnRecvPacketNativeErc20 tc * add v1 tcs for handling erc20 native coin OnTimeoutPacket, OnAcknowledgementPacket * refactor v1 middleware test codes * apply gci * fix ci: receiver name should be same * fix ci: unify receiver name and also update comments and variable name * fix ci: run gofumpt and remove tc copy * test: update TestOnRecvPacket make sure whether it is registered as dynamic precompiled contract or not * chore: test suite name convention
* replace erc20 native coin's prefix from: erc20/ to: erc20 * change prefix to erc20: * don't allow legacy format ValidateErc20Denom is not used anywhere except test code, but what if it is used from somewhere else in the future? We shouldn't treat legacy format as valid.
* test: WIP debugging ibc e2e test * fix: ics20 precompile receiver addr to bech32, not bech32 * test: add erc20 case for ics20 v1 e2e test (cherry picked from commit 2737b63160d9cefcad52cf86048638a861a478a5) * test: add ibc v2 relayer logic on testing package, add v2 ics20 precompile test cases (cherry picked from commit 886fdb7581cdcd0dcce15592aae0d8206e78a783) * chore: fix lint Useless assignment to local variable * fix!: denom trace to denom for ibc v8 -> v10 breaking changes * fix!: tmp cherry-pick to remove erc20/ Prefix in Native ERC20 Coin Denoms #92 320236a * test: add native erc20 case on ics20 precompile e2e test (cherry picked from commit d8db271) * chore: fix lint (cherry picked from commit 0d998cefc0c5bb57c396c01544b3ebe610f8a510) * fix: update comments on ics20.sol * refactor: Improve variable name clarity, apply suggestions * Revert "fix!: tmp cherry-pick to remove erc20/ Prefix in Native ERC20 Coin Denoms #92" This reverts commit e5afb56. * chore: update omitted contracts json by make contracts-all
* add OnRecvPacketNativeERC20 test case test scenario where evm chain receives erc20 native coin through IBC * add OnTimeoutPacketNativeERC20 test case * fix ci and comments * fix test case
vladjdk
requested changes
Apr 26, 2025
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
Closes: #41
Summary
This PR upgrades IBC from v8 to v10 and includes essential modifications to ensure that the existing ICS20 precompile and ERC20 middleware work properly with IBC v2 components such as
packet,channel, and denom rule. To validate this, corresponding E2E test cases have been added.Changes
This PR consolidates several sub-PRs, each addressing a required sub-task for full IBC v2 support. These changes were all individually reviewed within the team due to their significance. However, merging them independently into the
mainbranch could result in broken functionality for IBC v2. Therefore, they were first merged into thefeat/ibc-v10branch, and this PR is created to apply them atomically into themainbranch, ensuring proper integration of the full IBC v2 feature set.Below is a list of the major changes, along with their corresponding sub-issues and applied PRs:
v10erc20/prefix witherc20:in native ERC20 coin denomsICS20 precompileandx/ibcmodule function correctly for both IBC v1 and v2 supportx/erc20module’s IBC middleware, callback logic to cover IBC v2 packetChanges in IBC v2 (ibc-go v10) That Affect ics20 precompile and erc20 middleware
Packet Structure Changes
portandchannelare no longer included.clientis passed in v2.Denom Trace → Denom
Starting from IBC v9, the
DenomTraceendpoint has been renamed and restructured toDenom.Breaking Changes in the ICS20 Precompile Implementation
Packet Version-Based Validation Logic
portandchannelvalues provided in the transfer arguments to determine the packet version.channelvalue is treated as aclient ID.Denom Trace → Denom
DenomTracetoDenomwas adopted.Breaking Changes in the erc20 module
Replace erc20/ Prefix to erc20: in Native ERC20 Coin Denoms
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