feat: implementation of Solana restricted address #2795
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 changes encompass the addition of support for restricted addresses in Solana, modifications to compliance configuration handling, and enhancements to end-to-end testing for various blockchain interactions. New test cases were introduced for restricted deposits and withdrawals, while existing tests were updated to utilize new sample configurations. Additionally, compliance checks were integrated into transaction processing methods, and logging messages were streamlined for clarity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant E2ERunner
participant Compliance
participant Solana
User->>E2ERunner: Initiate Deposit to Restricted Address
E2ERunner->>Compliance: Check if address is restricted
Compliance-->>E2ERunner: Address is restricted
E2ERunner->>Solana: Execute Deposit Transaction
Solana-->>E2ERunner: Confirm Deposit
E2ERunner-->>User: Deposit Successful
sequenceDiagram
participant User
participant E2ERunner
participant Compliance
participant Solana
User->>E2ERunner: Initiate Withdrawal from Restricted Address
E2ERunner->>Compliance: Check if address is restricted
Compliance-->>E2ERunner: Address is restricted
E2ERunner->>Solana: Execute Withdrawal Transaction
Solana-->>E2ERunner: Confirm Withdrawal
E2ERunner-->>User: Withdrawal Successful
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2795 +/- ##
===========================================
- Coverage 66.94% 66.90% -0.04%
===========================================
Files 370 370
Lines 20965 20977 +12
===========================================
Hits 14035 14035
- Misses 6290 6302 +12
Partials 640 640
|
![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 (19)
- changelog.md (1 hunks)
- cmd/zetaclientd/init.go (2 hunks)
- cmd/zetae2e/local/local.go (1 hunks)
- e2e/e2etests/e2etests.go (4 hunks)
- e2e/e2etests/helpers.go (2 hunks)
- e2e/e2etests/test_bitcoin_withdraw_restricted_address.go (2 hunks)
- e2e/e2etests/test_erc20_deposit_restricted_address.go (2 hunks)
- e2e/e2etests/test_eth_withdraw_restricted_address.go (2 hunks)
- e2e/e2etests/test_solana_deposit_restricted_address.go (1 hunks)
- e2e/e2etests/test_solana_withdraw_restricted_address.go (1 hunks)
- e2e/e2etests/test_zeta_deposit_restricted_address.go (2 hunks)
- e2e/runner/poste2e_checker.go (1 hunks)
- e2e/runner/setup_zeta.go (2 hunks)
- e2e/runner/solana.go (2 hunks)
- testutil/sample/zetaclient.go (2 hunks)
- zetaclient/chains/solana/observer/outbound.go (2 hunks)
- zetaclient/chains/solana/signer/signer.go (2 hunks)
- zetaclient/chains/solana/signer/withdraw.go (2 hunks)
- zetaclient/testutils/testdata.go (2 hunks)
Files skipped from review due to trivial changes (1)
- e2e/runner/setup_zeta.go
Additional context used
Path-based instructions (17)
e2e/e2etests/test_zeta_deposit_restricted_address.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_erc20_deposit_restricted_address.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_solana_deposit_restricted_address.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_bitcoin_withdraw_restricted_address.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_solana_withdraw_restricted_address.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.testutil/sample/zetaclient.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/test_eth_withdraw_restricted_address.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/runner/poste2e_checker.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/signer/withdraw.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.cmd/zetaclientd/init.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/runner/solana.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/helpers.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/signer/signer.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/testutils/testdata.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/observer/outbound.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.cmd/zetae2e/local/local.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/e2etests/e2etests.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
GitHub Check: codecov/patch
zetaclient/chains/solana/signer/withdraw.go
[warning] 30-31: zetaclient/chains/solana/signer/withdraw.go#L30-L31
Added lines #L30 - L31 were not covered by testszetaclient/chains/solana/signer/signer.go
[warning] 131-142: zetaclient/chains/solana/signer/signer.go#L131-L142
Added lines #L131 - L142 were not covered by tests
[warning] 146-146: zetaclient/chains/solana/signer/signer.go#L146
Added line #L146 was not covered by testszetaclient/chains/solana/observer/outbound.go
[warning] 164-164: zetaclient/chains/solana/observer/outbound.go#L164
Added line #L164 was not covered by tests
[warning] 166-166: zetaclient/chains/solana/observer/outbound.go#L166
Added line #L166 was not covered by tests
Additional comments not posted (39)
e2e/e2etests/test_zeta_deposit_restricted_address.go (2)
8-8: LGTM!The change in the import path is approved.
18-18: LGTM!The change in the reference to the restricted address constant is approved.
e2e/e2etests/test_erc20_deposit_restricted_address.go (2)
8-8: LGTM!The change in the import path is approved.
18-18: LGTM!The change in the reference to the restricted address constant is approved.
e2e/e2etests/test_solana_deposit_restricted_address.go (5)
1-1: LGTM!The package declaration is appropriate.
3-8: LGTM!The import statements are appropriate.
10-12: LGTM!The function declaration and initial checks are appropriate.
13-17: LGTM!The parsing of the restricted address and deposit amount is appropriate.
19-20: LGTM!The execution of the deposit transaction is appropriate.
e2e/e2etests/test_bitcoin_withdraw_restricted_address.go (2)
10-10: LGTM!The import path change aligns with the new structure of the test utilities.
27-27: LGTM!The usage of
sample.RestrictedBtcAddressTestaligns with the new structure of the test utilities.e2e/e2etests/test_solana_withdraw_restricted_address.go (3)
1-1: LGTM!The package declaration is correct and aligns with the file's purpose.
3-12: LGTM!The import statements are correct and necessary for the file's functionality.
14-35: LGTM!The function
TestSolanaWithdrawRestrictedis well-structured and correctly implements the test for restricted address withdrawal.testutil/sample/zetaclient.go (2)
9-14: LGTM!The new constants for restricted addresses are correctly defined and enhance the testability of the code.
35-44: LGTM!The new function
ComplianceConfigis well-structured and correctly returns a sample compliance configuration.e2e/e2etests/test_eth_withdraw_restricted_address.go (2)
11-11: LGTM!The import statement change to use
sampleinstead oftestutilsis clear and likely improves maintainability.
34-34: LGTM!The function now references
sample.RestrictedEVMAddressTestinstead oftestutils.RestrictedEVMAddressTest, which is consistent with the updated import statement.e2e/runner/poste2e_checker.go (3)
16-25: LGTM!The
EnsureNoTrackersfunction is correctly implemented and ensures that no trackers are left at the end of the test.
27-47: LGTM!The
EnsureZeroBalanceOnRestrictedAddressfunction is correctly implemented and ensures that the balance of the restricted address is zero for various tokens.
49-61: LGTM!The
ensureZRC20ZeroBalancefunction is correctly implemented and ensures that the balance of the ZRC20 token is zero on a given address.zetaclient/chains/solana/signer/withdraw.go (3)
21-21: LGTM!The addition of the
cancelTxparameter to theSignMsgWithdrawfunction enhances functionality by allowing for conditional behavior in the withdrawal process.
29-32: LGTM!The logic to set the withdrawal amount to zero if
cancelTxis true is correctly implemented.Tools
GitHub Check: codecov/patch
[warning] 30-31: zetaclient/chains/solana/signer/withdraw.go#L30-L31
Added lines #L30 - L31 were not covered by tests
21-21: Verify the impact of the change.The
attachWithdrawAccountsfunction no longer appendsgatewayIDtoaccountSlice. Ensure that this change does not affect the functionality.Run the following script to verify the impact of the change:
cmd/zetaclientd/init.go (1)
113-113: LGTM! But verify the impact on application behavior.The change to use
sample.ComplianceConfig()instead oftestutils.ComplianceConfigTest()is approved. However, ensure that this change does not negatively impact the application's behavior in production or testing environments.Run the following script to verify the impact of the change:
e2e/runner/solana.go (1)
Line range hint
148-173: LGTM!The update to the function signature of
WithdrawSOLZRC20to return a pointer to aCrossChainTxobject is approved. This change enhances the method's functionality by allowing the caller to receive and utilize the transaction context information.e2e/e2etests/helpers.go (1)
99-123: LGTM!The addition of the
verifySolanaWithdrawalAmountFromCCTXfunction is approved. This function enhances the functionality of the test suite by enabling verification of withdrawal amounts specifically for Solana transactions.zetaclient/chains/solana/signer/signer.go (3)
19-19: LGTM!The import statement for the
compliancepackage is correct and necessary for the new functionality.
130-143: LGTM! But conditionally skip transaction processing based oncancelTx.The compliance check is correctly integrated into the method. However, the
cancelTxvariable should be used to conditionally skip the transaction processing to avoid unnecessary processing.if cancelTx { compliance.PrintComplianceLog( logger, signer.Logger().Compliance, true, chainID, cctx.Index, cctx.InboundParams.Sender, params.Receiver, "SOL", ) + return }Tools
GitHub Check: codecov/patch
[warning] 131-142: zetaclient/chains/solana/signer/signer.go#L131-L142
Added lines #L131 - L142 were not covered by tests
146-146: LGTM!The
SignMsgWithdrawmethod call is correctly updated to include thecancelTxparameter.Tools
GitHub Check: codecov/patch
[warning] 146-146: zetaclient/chains/solana/signer/signer.go#L146
Added line #L146 was not covered by testszetaclient/testutils/testdata.go (1)
22-25: LGTM!The reformatting of test data path constants improves readability and consistency.
zetaclient/chains/solana/observer/outbound.go (1)
163-167: LGTM! But log the compliance status for better traceability.The compliance check is correctly integrated into the function. However, ensure that the compliance status is logged for better traceability.
if compliance.IsCctxRestricted(cctx) { + logger.Info().Msgf("VoteOutboundIfConfirmed: cctx %s is restricted", cctx.Index) // use cctx's amount to bypass the amount check in zetacore outboundAmount = cctx.GetCurrentOutboundParam().Amount.BigInt() }Tools
GitHub Check: codecov/patch
[warning] 164-164: zetaclient/chains/solana/observer/outbound.go#L164
Added line #L164 was not covered by tests
[warning] 166-166: zetaclient/chains/solana/observer/outbound.go#L166
Added line #L166 was not covered by testscmd/zetae2e/local/local.go (2)
363-363: LGTM!The addition of
e2etests.TestSolanaDepositRestrictedNameensures that deposits to restricted addresses are tested.
364-364: LGTM!The addition of
e2etests.TestSolanaWithdrawRestrictedNameensures that withdrawals to restricted addresses are tested.e2e/e2etests/e2etests.go (4)
62-62: LGTM!The addition of
TestSolanaDepositRestrictedNameconstant is necessary for the new test case.
63-63: LGTM!The addition of
TestSolanaWithdrawRestrictedNameconstant is necessary for the new test case.
378-378: Verify the impact of the default value change.The default value for the "amount in lamport" for
TestSolanaDeposithas been modified from1200000to12000000. Ensure that this change does not negatively impact the test's behavior and outcomes.
407-423: LGTM!The addition of
TestSolanaDepositRestrictedandTestSolanaWithdrawRestrictedtest cases ensures that deposits and withdrawals to restricted addresses are tested.changelog.md (1)
18-18: LGTM!The changelog entry is correctly formatted and provides a clear description of the new feature.
|
Changes LGTM, it should be good to merge after resolving the conflicts. |
…t-solana-restricted-address
Description
Closes: #2604
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores