fix: ask for 3 accounts (signer, pda, system_program) when parsing Sonala inbound signer address #2787
Conversation
…relayer key path if not set
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 introduce a new feature in the Zeta Chain project related to the Solana gateway deposit process, requiring three specific accounts: a signer, a program-derived address (PDA), and a system program. Additionally, modifications were made to improve the configuration management for relayer keys, streamline deposit instruction handling, and update test cases and transaction result structures to align with the new account requirements. Changes
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 #2787 +/- ##
===========================================
- Coverage 66.79% 66.65% -0.15%
===========================================
Files 370 370
Lines 20677 20680 +3
===========================================
- Hits 13811 13784 -27
- Misses 6230 6261 +31
+ Partials 636 635 -1
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- changelog.md (1 hunks)
- cmd/zetaclientd/import_relayer_keys.go (2 hunks)
- e2e/runner/solana.go (1 hunks)
- pkg/contracts/solana/gateway.go (1 hunks)
- zetaclient/chains/solana/observer/inbound.go (4 hunks)
- zetaclient/chains/solana/observer/inbound_test.go (5 hunks)
- zetaclient/config/types.go (2 hunks)
- zetaclient/testdata/solana/chain_901_inbound_tx_result_MS3MPLN7hkbyCZFwKqXcg8fmEvQMD74fN6Ps2LSWXJoRxPW5ehaxBorK9q1JFVbqnAvu9jXm6ertj7kT7HpYw1j.json (1 hunks)
- zetaclient/testutils/constant.go (1 hunks)
Files skipped from review due to trivial changes (1)
- changelog.md
Additional context used
Path-based instructions (7)
pkg/contracts/solana/gateway.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/testutils/constant.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.cmd/zetaclientd/import_relayer_keys.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.zetaclient/config/types.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/observer/inbound_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/observer/inbound.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/config/types.go
[warning] 171-172: zetaclient/config/types.go#L171-L172
Added lines #L171 - L172 were not covered by testszetaclient/chains/solana/observer/inbound.go
[warning] 291-294: zetaclient/chains/solana/observer/inbound.go#L291-L294
Added lines #L291 - L294 were not covered by tests
Additional comments not posted (16)
pkg/contracts/solana/gateway.go (1)
17-18: LGTM! But verify the constant usage in the codebase.The constant
AccountsNumDeposithas been correctly updated to reflect the new number of accounts required for the Solana gateway deposit instruction. Ensure that all references to this constant in the codebase are compatible with the new value.The code changes are approved.
Run the following script to verify the constant usage:
Verification successful
Verification Successful: Consistent Use of
AccountsNumDepositThe constant
AccountsNumDeposithas been correctly updated and is consistently used across the codebase. The checks and comments align with the new requirement of 3 accounts.
zetaclient/chains/solana/observer/inbound.go: The usage ofAccountsNumDepositis consistent with the updated value.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `AccountsNumDeposit` in the codebase. # Test: Search for the constant usage. Expect: Only occurrences of the new value. rg --type go -A 5 $'AccountsNumDeposit'Length of output: 1044
zetaclient/testdata/solana/chain_901_inbound_tx_result_MS3MPLN7hkbyCZFwKqXcg8fmEvQMD74fN6Ps2LSWXJoRxPW5ehaxBorK9q1JFVbqnAvu9jXm6ertj7kT7HpYw1j.json (1)
1-64: LGTM! Verify the JSON usage in the codebase.The JSON structure appears to be well-formed and contains all necessary fields for representing transaction data. Ensure that the data aligns with the expected schema and is used correctly in the codebase.
The code changes are approved.
Run the following script to verify the JSON usage:
zetaclient/testutils/constant.go (1)
39-39: LGTM! Verify the variable usage in the codebase.The gateway address for Solana devnet has been correctly updated. Ensure that all tests relying on this address are updated accordingly.
The code changes are approved.
Run the following script to verify the variable usage:
Verification successful
Verification Successful: Updated Gateway Address is Correctly Used
The updated gateway address for the Solana devnet is correctly referenced in the relevant test files. Ensure that these tests pass to confirm the change's correctness.
- Files referencing the updated address:
zetaclient/chains/solana/observer/inbound_test.gozetaclient/chains/solana/signer/signer_test.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `GatewayAddresses` in the codebase. # Test: Search for the variable usage. Expect: Only occurrences of the new value. rg --type go -A 5 $'GatewayAddresses'Length of output: 2682
cmd/zetaclientd/import_relayer_keys.go (1)
15-15: LGTM!The change improves maintainability and configurability by centralizing the default path definition.
The code changes are approved.
Also applies to: 56-58
e2e/runner/solana.go (1)
Line range hint
32-36: Verify the impact of the change on the deposit instruction functionality.The removal of
solana.Meta(programID)from theaccountSlicemight impact the functionality of the deposit instruction. Ensure that this change does not cause issues in transaction processing or validation within the Solana blockchain context.Run the following script to verify the impact of the change:
zetaclient/config/types.go (1)
23-25: LGTM!The introduction of the new constant
DefaultRelayerKeyPathenhances the configuration capabilities by providing a predefined path for relayer keys.The code changes are approved.
zetaclient/chains/solana/observer/inbound_test.go (7)
29-30: LGTM!The transaction hash update aligns the test with the latest transaction data.
The code changes are approved.
54-55: LGTM!The transaction hash update aligns the test with the latest transaction data.
The code changes are approved.
64-64: LGTM!The dynamic retrieval of
GatewayAddressenhances the flexibility and maintainability of the test.The code changes are approved.
76-77: LGTM!The updates to the
AmountandMemofields ensure the test reflects the expected behavior.The code changes are approved.
159-160: LGTM!The transaction hash update aligns the test with the latest transaction data.
The code changes are approved.
172-172: LGTM!The dynamic retrieval of
GatewayAddressenhances the flexibility and maintainability of the test.The code changes are approved.
183-184: LGTM!The updates to the
AmountandMemofields ensure the test reflects the expected behavior.The code changes are approved.
zetaclient/chains/solana/observer/inbound.go (3)
288-294: Enhanced error handling and logging.The improved error handling and logging for the signer address retrieval enhance the robustness of the method against errors.
The code changes are approved.
Tools
GitHub Check: codecov/patch
[warning] 291-294: zetaclient/chains/solana/observer/inbound.go#L291-L294
Added lines #L291 - L294 were not covered by tests
329-335: Streamlined account validation logic.The reduction in the expected number of accounts and the simplification of the account validation logic streamline the processing of deposit instructions and improve maintainability.
The code changes are approved.
355-355: LGTM!The validation condition now correctly checks for the presence of the signer, PDA, and system program accounts.
The code changes are approved.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- changelog.md (1 hunks)
- cmd/zetaclientd/import_relayer_keys.go (2 hunks)
- e2e/runner/solana.go (1 hunks)
- pkg/contracts/solana/gateway.go (1 hunks)
- zetaclient/chains/solana/observer/inbound.go (4 hunks)
- zetaclient/chains/solana/observer/inbound_test.go (5 hunks)
- zetaclient/config/types.go (2 hunks)
- zetaclient/testdata/solana/chain_901_inbound_tx_result_MS3MPLN7hkbyCZFwKqXcg8fmEvQMD74fN6Ps2LSWXJoRxPW5ehaxBorK9q1JFVbqnAvu9jXm6ertj7kT7HpYw1j.json (1 hunks)
- zetaclient/testutils/constant.go (1 hunks)
Additional context used
Path-based instructions (7)
pkg/contracts/solana/gateway.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/testutils/constant.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.cmd/zetaclientd/import_relayer_keys.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.zetaclient/config/types.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/observer/inbound_test.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/observer/inbound.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/config/types.go
[warning] 171-172: zetaclient/config/types.go#L171-L172
Added lines #L171 - L172 were not covered by testszetaclient/chains/solana/observer/inbound.go
[warning] 291-294: zetaclient/chains/solana/observer/inbound.go#L291-L294
Added lines #L291 - L294 were not covered by tests
Additional comments not posted (12)
pkg/contracts/solana/gateway.go (1)
17-18: LGTM!The change in the number of accounts required for the deposit instruction is clear and the accompanying comment is updated accordingly.
The code changes are approved.
zetaclient/testdata/solana/chain_901_inbound_tx_result_MS3MPLN7hkbyCZFwKqXcg8fmEvQMD74fN6Ps2LSWXJoRxPW5ehaxBorK9q1JFVbqnAvu9jXm6ertj7kT7HpYw1j.json (1)
1-64: LGTM!The JSON structure is well-formed and the data appears to be comprehensive and accurate.
The code changes are approved.
zetaclient/testutils/constant.go (1)
39-39: LGTM!The update to the gateway address for the Solana devnet is clear and straightforward.
The code changes are approved.
cmd/zetaclientd/import_relayer_keys.go (1)
15-15: LGTM!The change to use
config.DefaultRelayerKeyPathenhances configurability and maintainability by centralizing the default path in the configuration module.The code changes are approved.
Also applies to: 56-58
e2e/runner/solana.go (1)
Line range hint
32-36: LGTM! But verify the impact on deposit instruction.The removal of
solana.Meta(programID)from theaccountSlicemay streamline account handling or address a specific issue. Ensure that this change does not negatively impact the deposit instruction's functionality.The code changes are approved.
Run the following script to verify the impact on the deposit instruction:
Verification successful
Removal of
solana.Meta(programID)is SafeThe removal of
solana.Meta(programID)from theaccountSlicein theCreateDepositInstructionmethod does not negatively impact its functionality. TheprogramIDis still correctly assigned toinst.ProgID, and the essential components of theaccountSliceare maintained. This change streamlines the account handling without affecting the transaction's integrity.
- The
accountSliceincludes necessary accounts: signer, computed PDA, and Solana System Program ID.- The
programIDis used ininst.ProgID, ensuring proper association with the program.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing `solana.Meta(programID)` from the `accountSlice`. # Test: Search for the `CreateDepositInstruction` method usage. Expect: No negative impact on functionality. rg --type go -A 5 $'CreateDepositInstruction'Length of output: 821
Script:
#!/bin/bash # Description: Examine the construction and usage of `accountSlice` in `CreateDepositInstruction`. # Search for the `accountSlice` within the `CreateDepositInstruction` method to understand its construction and usage. rg --type go -A 10 'accountSlice' e2e/runner/solana.goLength of output: 647
zetaclient/config/types.go (1)
23-25: LGTM!The introduction of
DefaultRelayerKeyPathenhances configurability by providing a predefined path for relayer keys, improving usability for users who may not specify a custom path.The code changes are approved.
zetaclient/chains/solana/observer/inbound_test.go (3)
29-30: LGTM!The transaction hash update is correctly implemented.
The code changes are approved.
54-55: LGTM!The transaction hash, dynamic gateway address, and expected values for
AmountandMemofields are correctly updated.The code changes are approved.
Also applies to: 64-64, 76-77
159-160: LGTM!The transaction hash, dynamic gateway address, and expected values for
AmountandMemofields are correctly updated.The code changes are approved.
Also applies to: 172-172, 183-184
zetaclient/chains/solana/observer/inbound.go (2)
288-294: Enhanced error handling for signer address.The error handling for obtaining the signer address has been improved by logging a detailed error message and returning
nilwithout propagating the error. This change improves the robustness of the function.The code changes are approved.
Tools
GitHub Check: codecov/patch
[warning] 291-294: zetaclient/chains/solana/observer/inbound.go#L291-L294
Added lines #L291 - L294 were not covered by tests
329-329: Updated account validation logic.The expected number of accounts for a deposit instruction has been reduced from four to three. The validation logic now only verifies the presence of the signer, PDA, and system accounts, which streamlines the function and aligns it with the updated account structure.
The code changes are approved.
Also applies to: 334-335, 355-355
changelog.md (1)
18-18: Changelog entry added.The new entry is correctly formatted and provides a clear description of the feature related to the Solana gateway deposit process.
The code changes are approved.
ws4charlie
changed the title
The title of this PR is updated. |
Description
~/.zetacored/relayer-keysif not set in zetaclient config fileHow Has This Been Tested?
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests