fix(pkg/chains): check if VM is EVM for IsEVMChain check
#2801
Conversation
WalkthroughWalkthroughThe changes involve modifications to the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2801 +/- ##
===========================================
- Coverage 66.65% 66.62% -0.04%
===========================================
Files 370 370
Lines 20680 20735 +55
===========================================
+ Hits 13784 13814 +30
- Misses 6261 6283 +22
- Partials 635 638 +3
|
lumtis
requested review from
brewmaster012,
kingpinXD,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- pkg/chains/chain.go (2 hunks)
- pkg/chains/chain_test.go (2 hunks)
- x/crosschain/keeper/refund_test.go (1 hunks)
Additional context used
Path-based instructions (3)
pkg/chains/chain.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/chains/chain_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/refund_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (5)
pkg/chains/chain.go (2)
89-89: RefactorIsEVMChainmethod to check VM type.The change from checking the consensus mechanism to checking the VM type (
Vm_evm) aligns with the PR objectives to more accurately determine EVM compatibility. This is a crucial update for the functionality of systems that rely on the VM type rather than the consensus mechanism.
112-120: Enhanced error handling inIsEVMChainfunction.The update to first retrieve the chain using
GetChainFromChainIDand then check if it's an EVM chain using the updatedIsEVMChainmethod improves the robustness of this function. This ensures that the function operates on a valid chain object, enhancing error handling by returningfalseif the chain is not found.pkg/chains/chain_test.go (2)
225-225: Updated test case forZeta Mainnet.The test case update for "Zeta Mainnet" to return
trueforIsEVMChainreflects the updated logic in the main codebase. This ensures that the test is aligned with the new classification of "Zeta Mainnet" as an EVM chain.
334-334: Consistency in test cases forIsEVMChain.The addition of "Zeta Mainnet" with a return value of
truein the standaloneIsEVMChainfunction test case maintains consistency with the method test case and the updated logic in the main codebase.x/crosschain/keeper/refund_test.go (1)
219-224: Dynamic assignment ofSenderChainIdin test case.The update to dynamically assign the
SenderChainIdusinggetValidBtcChainID()enhances the flexibility and accuracy of the test case. This ensures that theSenderChainIdreflects a valid chain ID, improving the relevance of the test scenario.
Description
IsEVMChainused to check if the chain consensus is Ethereum, which is invalid, the components usingIsEVMChaindon't perform logic that relies on consensus used by Ethereum.Example: Base doesn't use Ethereum consensus as a L2 but comply with logic using isEVMChain.
This PR change the check.
All EVM chains so far had the value
ConsensusEthereumexcept Base where we made a patch in the chain info. Therefore changing this check in theory doesn't bring any change in the workflowClose: #2752
Summary by CodeRabbit
New Features
IsEVMChainfunction for better reliability.Bug Fixes
Tests
SenderChainIdin cross-chain transactions, improving accuracy.