fix: check Solana account rent exempt in zetacore to avoid outbound failure #2545
Conversation
ws4charlie
requested review from
brewmaster012,
kingpinXD,
lumtis,
skosito,
swift1337 and
fbac
as code owners
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughWalkthroughThe recent changes enhance the cryptocurrency withdrawal functionality by introducing new constants for withdrawal thresholds, adding a method for ZRC20 token withdrawals, and improving the validation logic for both Bitcoin and Solana withdrawals. These updates aim to ensure compliance with protocol specifications and improve the robustness of transaction handling across different blockchain networks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ZRC20Withdrawal
participant Keeper
User->>ZRC20Withdrawal: Request withdrawal
ZRC20Withdrawal->>Keeper: Validate withdrawal data
Keeper->>Keeper: Check blockchain type
alt Bitcoin
Keeper->>Keeper: Validate against BTCWithdrawalDustAmount
else Solana
Keeper->>Keeper: Validate against SolanaWalletRentExempt
end
Keeper->>User: Confirm withdrawal
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2545 +/- ##
===========================================
+ Coverage 46.68% 46.70% +0.02%
===========================================
Files 464 464
Lines 30781 30793 +12
===========================================
+ Hits 14369 14381 +12
Misses 15557 15557
Partials 855 855
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
x/crosschain/keeper/evm_hooks.go (1)
279-279: Improve readability by clarifying the purpose ofadditionalChains.Consider adding a comment to explain the purpose of
additionalChainsfor better readability.+ // additionalChains contains the list of additional chains supported by the system additionalChains := k.GetAuthorityKeeper().GetAdditionalChainList(ctx)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- pkg/constant/constant.go (1 hunks)
- testutil/sample/crosschain.go (3 hunks)
- x/crosschain/keeper/evm_hooks.go (2 hunks)
- x/crosschain/keeper/evm_hooks_test.go (2 hunks)
Additional context used
Path-based instructions (4)
pkg/constant/constant.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/evm_hooks.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.testutil/sample/crosschain.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/crosschain/keeper/evm_hooks_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 (4)
pkg/constant/constant.go (2)
15-15: LGTM!The
BTCWithdrawalDustAmountconstant is well-documented and the value is justified.
21-21: LGTM!The
SolanaWalletRentExemptconstant is well-documented and the value is justified.x/crosschain/keeper/evm_hooks_test.go (2)
208-218: Verify the correctness of the invalid Solana address test case.The test case correctly checks for an invalid Solana address. Ensure that the constant
constant.SolanaWalletRentExemptis defined and imported correctly.Verification successful
The test case for validating an invalid Solana address is correctly implemented.
The constant
constant.SolanaWalletRentExemptis properly defined and imported, ensuring the test case functions as expected.
pkg/constant/constant.go: Definition ofSolanaWalletRentExemptx/crosschain/keeper/evm_hooks_test.go: Usage in the test caseScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and import of `constant.SolanaWalletRentExempt`. # Test: Search for the constant definition. Expect: The constant should be defined in the codebase. rg --type go 'SolanaWalletRentExempt'Length of output: 699
220-237: Verify the correctness of the Solana withdrawal amount test case.The test case correctly checks for amounts below the minimum required for Solana withdrawals. Ensure that the constant
constant.SolanaWalletRentExemptis defined and imported correctly.Verification successful
The constant
SolanaWalletRentExemptis correctly defined and imported.
- The constant
SolanaWalletRentExemptis defined inpkg/constant/constant.goand is used in the test case as expected.- The test case correctly checks for amounts below the minimum required for Solana withdrawals.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and import of `constant.SolanaWalletRentExempt`. # Test: Search for the constant definition. Expect: The constant should be defined in the codebase. rg --type go 'SolanaWalletRentExempt'Length of output: 699
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
Description
cctxAmount < rentExemptand the funds is to sent to an account that holds0balance, the transaction will fail and this will block outbound.ValidateZrc20WithdrawEvent, similar to Bitcoin.How Has This Been Tested?
Summary by CodeRabbit