feat: support Solana wallet address in zetacore for SOL withdrawals #2518
Conversation
ws4charlie
requested review from
brewmaster012,
kingpinXD,
lumtis,
skosito,
swift1337 and
fbac
as code owners
WalkthroughThis update enhances the Zetacore by adding support for Solana wallet addresses, improving interoperability with the Solana blockchain. Key changes include new decoding functions, expanded test coverage, and refined handling of blockchain consensus types. These updates strengthen the system's capabilities and ensure robust address validation across various blockchain platforms. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Chain
participant AddressDecoder
User->>Chain: Request address encoding
Chain->>AddressDecoder: Decode address
AddressDecoder-->>Chain: Return decoded public key
Chain-->>User: Return encoded address or error
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 (
|
![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 (5)
- changelog.md (1 hunks)
- pkg/chains/address.go (2 hunks)
- pkg/chains/address_test.go (1 hunks)
- pkg/chains/chain.go (3 hunks)
- pkg/chains/chain_test.go (3 hunks)
Additional context used
Path-based instructions (4)
pkg/chains/address.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.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.pkg/chains/address_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 (10)
pkg/chains/address.go (1)
91-105: LGTM! Ensure robust error handling.The function
DecodeSolanaWalletAddressis well-implemented with proper error handling for invalid Base58 encoding and checks for the Ed25519 curve.However, ensure that all potential edge cases are covered in the tests.
pkg/chains/chain.go (2)
Line range hint
55-83:
LGTM! Verify the correctness of the new case.The new case for
Consensus_solana_consensusin theEncodeAddressmethod is well-implemented and improves readability and maintainability.However, ensure that the new case is thoroughly tested to verify its correctness.
122-122: LGTM! Verify the correctness of the updated logic.The update to use
ChainListByNetworkin theIsBitcoinChainfunction aligns the logic with the new network-based approach and improves the structure and clarity of the code.However, ensure that the updated logic is thoroughly tested to verify its correctness.
pkg/chains/chain_test.go (5)
150-176: LGTM! Verify the correctness and coverage of the new test cases for Solana addresses.The new test cases for Solana addresses in the
EncodeAddressmethod are well-implemented and improve the clarity and specificity of the tests.However, ensure that the new test cases cover all potential edge cases and verify the correctness of the implementation.
298-303: LGTM! Verify the correctness and coverage of the new test cases for Solana addresses.The new test cases for Solana addresses in the
DecodeAddressFromChainIDmethod are well-implemented and improve the clarity and specificity of the tests.However, ensure that the new test cases cover all potential edge cases and verify the correctness of the implementation.
Line range hint
250-263:
LGTM! Verify the correctness and coverage of the new test cases for Ethereum networks.The new test cases for Ethereum networks in the
IsEVMChainmethod are well-implemented and improve the clarity and specificity of the tests.However, ensure that the new test cases cover all potential edge cases and verify the correctness of the implementation.
Line range hint
270-283:
LGTM! Verify the correctness and coverage of the new test cases for Bitcoin networks.The new test cases for Bitcoin networks in the
IsBitcoinChainmethod are well-implemented and improve the clarity and specificity of the tests.However, ensure that the new test cases cover all potential edge cases and verify the correctness of the implementation.
Line range hint
290-303:
LGTM! Verify the correctness and coverage of the new test cases for ZetaChain networks.The new test cases for ZetaChain networks in the
IsZetaChainmethod are well-implemented and improve the clarity and specificity of the tests.However, ensure that the new test cases cover all potential edge cases and verify the correctness of the implementation.
pkg/chains/address_test.go (1)
101-151: LGTM! EnsureDecodeSolanaWalletAddressfunction is implemented correctly.The new test function
Test_DecodeSolanaWalletAddressis well-structured and covers various scenarios for decoding Solana wallet addresses.However, ensure that the
DecodeSolanaWalletAddressfunction is correctly implemented and handles all edge cases.Verification successful
The
DecodeSolanaWalletAddressfunction is implemented correctly.The function decodes a Base58 encoded address and checks if it is on the ed25519 curve, handling all the edge cases mentioned in the test cases. The implementation aligns with the test scenarios provided in the
Test_DecodeSolanaWalletAddressfunction.
pkg/chains/address.go:DecodeSolanaWalletAddressScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of `DecodeSolanaWalletAddress` function. # Test: Search for the function implementation. Expect: Correct handling of all edge cases. ast-grep --lang go --pattern $'func DecodeSolanaWalletAddress($_) ($_, error) { $$$ }'Length of output: 89
Script:
#!/bin/bash # Description: Search for the function name `DecodeSolanaWalletAddress` in the codebase to locate its implementation. # Search for the function definition rg "func DecodeSolanaWalletAddress" -A 10Length of output: 838
Script:
#!/bin/bash # Description: Retrieve the complete implementation of `DecodeSolanaWalletAddress` function. # Search for the complete function implementation rg "func DecodeSolanaWalletAddress" -A 20Length of output: 1285
changelog.md (1)
35-35: Changelog entry is accurate and complete.The new entry for pull request 2518 accurately describes the addition of support for Solana addresses in the Zetacore.
|
@ws4charlie looks like there is a failure in unit tests |
…a address dedoding
Co-authored-by: Dmitry S <[email protected]>
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/address.go (2 hunks)
- pkg/chains/address_test.go (1 hunks)
- pkg/chains/chain.go (3 hunks)
Files skipped from review as they are similar to previous changes (3)
- pkg/chains/address.go
- pkg/chains/address_test.go
- pkg/chains/chain.go
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2518 +/- ##
===========================================
+ Coverage 46.50% 46.53% +0.02%
===========================================
Files 456 456
Lines 30282 30296 +14
===========================================
+ Hits 14083 14098 +15
+ Misses 15372 15371 -1
Partials 827 827
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go (1 hunks)
Additional context used
Path-based instructions (1)
x/crosschain/keeper/cctx_orchestrator_validate_inbound_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 (15)
x/crosschain/keeper/cctx_orchestrator_validate_inbound_test.go (15)
Line range hint
12-56:
LGTM!The test case "successfully validate inbound" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
58-86:
LGTM!The test case "fail if tss not found" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
88-118:
LGTM!The test case "fail if InitiateOutbound fails" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
120-151:
LGTM!The test case "does not set cctx if SetCctxAndNonceToCctxAndInboundHashToCctx fails" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
153-183:
LGTM!The test case "fail if inbound is disabled" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
185-216:
LGTM!The test case "fails when CheckIfTSSMigrationTransfer fails" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
218-239:
LGTM!The test case "Do not return error if sender is not a TSS address for evm chain" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
241-262:
LGTM!The test case "Do not return error if sender is not a TSS address for btc chain" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
264-285:
LGTM!The test case "fails when chain is not supported" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
287-308:
LGTM!The test case "skips check when an older tss address is invalid for bitcoin chain" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
310-331:
LGTM!The test case "skips check when an older tss address is invalid for evm chain" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
333-359:
LGTM!The test case "fails when sender is a TSS address for evm chain for evm chain" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
361-391:
LGTM!The test case "fails when sender is a TSS address for btc chain for btc chain" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Line range hint
393-414:
LGTM!The test case "fails if bitcoin network params not found for BTC chain" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes. The addition of the
Networkfield to theChainstruct instantiation is appropriate and enhances the test case.Also applies to: 626-633
Line range hint
635-655:
LGTM!The test case "fails if gateway is not observer" is well-structured and correctly sets up mock data, configures mocks, and verifies the expected outcomes.
Description
zetacoredHow Has This Been Tested?
Summary by CodeRabbit