-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: withdraw SOL from ZEVM to Solana #2560
Conversation
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 updates enhance the ZetaChain framework's functionality by integrating Solana support across various components, including transaction handling and testing. Key improvements involve streamlining methods for managing outbound transactions and gas price monitoring, while refining error handling and logging. The changes also introduce new constants and structures to facilitate withdrawal operations, thereby improving the robustness of cross-chain interactions. Overall, the enhancements position the framework to effectively handle multi-chain scenarios, particularly with Solana. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ZetaCore
participant SolanaRPCClient
participant Observer
User->>ZetaCore: Initiate transaction
ZetaCore->>SolanaRPCClient: Sign transaction
SolanaRPCClient->>Observer: Monitor transaction status
Observer-->>ZetaCore: Transaction finalized
ZetaCore-->>User: Confirmation of transaction
sequenceDiagram
participant User
participant ZetaCore
participant Observer
User->>ZetaCore: Check gas price
ZetaCore->>Observer: Get current gas price
Observer-->>ZetaCore: Return gas price
ZetaCore-->>User: Display gas price
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
Outside diff range, codebase verification and nitpick comments (5)
e2e/e2etests/test_solana_withdraw.go (2)
15-18
: Improve logging statement for clarity.The logging statement should clearly indicate the start of the test.
- r.Logger.Print("TestSolanaWithdraw...sol zrc20 %s", r.SOLZRC20Addr.String()) + r.Logger.Print("Starting TestSolanaWithdraw for sol zrc20 address: %s", r.SOLZRC20Addr.String())
39-41
: Remove repeated logging statement.The logging statement is repeated and can be removed for conciseness.
- r.Logger.Print("TestSolanaWithdraw...sol zrc20 %s", r.SOLZRC20Addr.String())
zetaclient/zetacore/tx.go (1)
Line range hint
15-28
:
LGTM!The function is straightforward and appropriate.
Reminder: Refactor the function.
The TODO comment indicates a potential refactor.
Do you want me to assist with refactoring this function or open a GitHub issue to track this task?
e2e/runner/solana.go (2)
116-116
: Improve logging clarity.The log message should be more descriptive and structured for better readability.
- r.Logger.Print("TestSolanaWithdraw...sol zrc20 %s to %s", r.SOLZRC20Addr.String(), to.String()) + r.Logger.Infof("Initiating SOL ZRC20 withdrawal from %s to %s", r.SOLZRC20Addr.String(), to.String())
126-127
: Clarify approval amount calculation.The approval amount calculation should be more explicit to avoid confusion.
- approveAmount := big.NewInt(1e9 * 100) // 100 SOL + approveAmount := big.NewInt(100).Mul(big.NewInt(100), big.NewInt(1e9)) // 100 SOL in lamports
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (33)
- cmd/zetaclientd/utils.go (1 hunks)
- cmd/zetae2e/local/local.go (2 hunks)
- contrib/localnet/docker-compose.yml (1 hunks)
- contrib/localnet/orchestrator/start-zetae2e.sh (1 hunks)
- contrib/localnet/scripts/start-zetacored.sh (1 hunks)
- e2e/e2etests/e2etests.go (2 hunks)
- e2e/e2etests/test_solana_deposit.go (1 hunks)
- e2e/e2etests/test_solana_withdraw.go (1 hunks)
- e2e/runner/setup_solana.go (4 hunks)
- e2e/runner/solana.go (2 hunks)
- e2e/txserver/zeta_tx_server.go (1 hunks)
- pkg/contract/solana/gateway_message.go (1 hunks)
- pkg/contract/solana/types.go (2 hunks)
- x/observer/types/chain_params.go (1 hunks)
- zetaclient/chains/bitcoin/observer/observer.go (3 hunks)
- zetaclient/chains/bitcoin/observer/outbound.go (4 hunks)
- zetaclient/chains/bitcoin/signer/signer.go (4 hunks)
- zetaclient/chains/evm/observer/outbound.go (4 hunks)
- zetaclient/chains/evm/observer/outbound_test.go (4 hunks)
- zetaclient/chains/evm/signer/outbound_data.go (1 hunks)
- zetaclient/chains/evm/signer/signer.go (10 hunks)
- zetaclient/chains/interfaces/interfaces.go (2 hunks)
- zetaclient/chains/solana/observer/gas_price.go (1 hunks)
- zetaclient/chains/solana/observer/observer.go (5 hunks)
- zetaclient/chains/solana/observer/outbound.go (1 hunks)
- zetaclient/chains/solana/signer/signer.go (1 hunks)
- zetaclient/common/constant.go (1 hunks)
- zetaclient/orchestrator/bootstrap.go (3 hunks)
- zetaclient/orchestrator/orchestrator.go (7 hunks)
- zetaclient/testutils/mocks/chain_clients.go (3 hunks)
- zetaclient/zetacore/client_vote.go (1 hunks)
- zetaclient/zetacore/tx.go (2 hunks)
- zetaclient/zetacore/tx_test.go (1 hunks)
Files skipped from review due to trivial changes (2)
- cmd/zetaclientd/utils.go
- contrib/localnet/docker-compose.yml
Additional context used
Path-based instructions (31)
zetaclient/common/constant.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.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/contract/solana/types.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/contract/solana/gateway_message.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/observer/gas_price.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/testutils/mocks/chain_clients.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.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/runner/setup_solana.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/zetacore/tx.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/chains/solana/observer/observer.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/zetacore/client_vote.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/evm/signer/outbound_data.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.contrib/localnet/orchestrator/start-zetae2e.sh (1)
Pattern
**/*.sh
: Review the shell scripts, point out issues relative to security, performance, and maintainability.zetaclient/chains/interfaces/interfaces.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.zetaclient/orchestrator/bootstrap.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/observer/types/chain_params.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/signer/signer.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.cmd/zetae2e/local/local.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/zetacore/tx_test.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.e2e/txserver/zeta_tx_server.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.contrib/localnet/scripts/start-zetacored.sh (1)
Pattern
**/*.sh
: Review the shell scripts, point out issues relative to security, performance, and maintainability.zetaclient/chains/evm/observer/outbound.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/observer/observer.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/orchestrator/orchestrator.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/evm/observer/outbound_test.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/observer/outbound.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/evm/signer/signer.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (63)
zetaclient/common/constant.go (1)
4-5
: LGTM! Verify the usage of the new constant.The addition of the
DefaultGasPriceMultiplier
constant is correct and well-documented.However, ensure that this constant is used correctly across the codebase.
Verification successful
The usage of
DefaultGasPriceMultiplier
is correctly implemented.The constant is used as a fallback in a switch-case statement, ensuring a default multiplier is applied when the consensus type does not match specific cases.
- Location of usage:
zetaclient/zetacore/tx.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new constant `DefaultGasPriceMultiplier`. # Test: Search for the usage of the constant. Expect: Occurrences of the new constant. rg --type go 'DefaultGasPriceMultiplier'Length of output: 288
Script:
#!/bin/bash # Extract lines around the usage of DefaultGasPriceMultiplier in zetaclient/zetacore/tx.go for context verification rg --type go --context 5 'DefaultGasPriceMultiplier' zetaclient/zetacore/tx.goLength of output: 459
e2e/e2etests/test_solana_deposit.go (1)
14-20
: LGTM! Consider adding a test case for invalid input.The changes to accept a dynamic deposit amount and validate the input are correct.
Consider adding a test case to handle invalid input scenarios.
pkg/contract/solana/types.go (2)
45-64
: LGTM! Verify the usage of the new structure.The addition of the
WithdrawInstructionParams
structure is correct and well-documented.Ensure that this structure is used correctly across the codebase.
Verification successful
Usage of
WithdrawInstructionParams
VerifiedThe
WithdrawInstructionParams
structure is correctly implemented and used within the codebase. The parameters are appropriately populated and serialized, with error handling in place.
- Verified usage in
zetaclient/chains/solana/signer/signer.go
within theSignWithdrawTx
function.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new structure `WithdrawInstructionParams`. # Test: Search for the usage of the new structure. Expect: Occurrences of the new structure. rg --type go 'WithdrawInstructionParams'Length of output: 461
Script:
#!/bin/bash # Extract lines around the usage of WithdrawInstructionParams in signer.go for verification. rg --type go -A 10 -B 10 'WithdrawInstructionParams' zetaclient/chains/solana/signer/signer.goLength of output: 1746
17-18
: LGTM! Verify the usage of the renamed field.The renaming of the
ChainId
field toChainID
improves clarity and follows Go naming conventions.Ensure that all references to this field are updated accordingly.
pkg/contract/solana/gateway_message.go (4)
28-36
: LGTM!The
NewMsgWithdraw
function correctly initializes aMsgWithdraw
struct.
57-61
: LGTM!The
WithSignature
function correctly attaches the signature to the message and returns the modified message.
63-68
: LGTM!The
SigRS
function correctly extracts and returns the 64-byte core part of the signature.
70-73
: LGTM!The
SigV
function correctly extracts and returns the V part of the signature.zetaclient/chains/solana/observer/gas_price.go (2)
13-49
: Consistently use context forsolClient.GetSlot
call.The context should be passed to
solClient.GetSlot
instead of creating a new one.- slot, err := ob.solClient.GetSlot(context.Background(), rpc.CommitmentConfirmed) + slot, err := ob.solClient.GetSlot(ctx, rpc.CommitmentConfirmed)Likely invalid or redundant comment.
13-49
: Add error handling for ticker creation.The function should handle the error case when
clienttypes.NewDynamicTicker
fails.- ticker, err := clienttypes.NewDynamicTicker( + ticker, err := clienttypes.NewDynamicTicker( fmt.Sprintf("Solana_WatchGasPrice_%d", ob.Chain().ChainId), ob.GetChainParams().GasPriceTicker, ) if err != nil { return errors.Wrapf(err, "NewDynamicTicker error") } ob.Logger().GasPrice.Info().Msgf("WatchGasPrice started for chain %d with interval %d", ob.Chain().ChainId, ob.GetChainParams().GasPriceTicker) defer ticker.Stop()Likely invalid or redundant comment.
zetaclient/testutils/mocks/chain_clients.go (2)
Line range hint
42-46
:
Verify the impact of removing thezerolog.Logger
parameter on logging functionality.The removal of the
zerolog.Logger
parameter simplifies the method signature but may affect logging functionality. Ensure that logging is handled appropriately elsewhere.Verification successful
Verify the impact of removing the
zerolog.Logger
parameter on logging functionality.The removal of the
zerolog.Logger
parameter from theIsOutboundProcessed
method does not impact logging functionality. The calling functions handle logging appropriately using their own logger instances.
- Logging is managed by the calling functions.
- No broken logging functionality observed.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing the zerolog.Logger parameter on logging functionality. # Test: Search for the usage of IsOutboundProcessed method. Expect: No broken logging functionality. rg --type go -A 5 $'IsOutboundProcessed'Length of output: 15296
Line range hint
17-21
:
Verify the impact of removing thezerolog.Logger
parameter on logging functionality.The removal of the
zerolog.Logger
parameter simplifies the method signature but may affect logging functionality. Ensure that logging is handled appropriately elsewhere.e2e/e2etests/test_solana_withdraw.go (5)
20-26
: LGTM!The balance retrieval and logging are appropriate.
28-37
: LGTM!The parsing and validation of the withdrawal amount are appropriate.
43-47
: LGTM!The approval process and waiting for the transaction receipt are appropriate.
49-52
: LGTM!The withdrawal process and logging are appropriate.
54-68
: LGTM!The process of waiting for the receipt, retrieving the balance, and waiting for the cross-chain transaction to be mined is appropriate.
e2e/runner/setup_solana.go (2)
16-20
: LGTM!The function is straightforward and appropriate. The logging statement is clear.
Line range hint
26-72
:
LGTM!The function is comprehensive and appropriate. The logging statements are clear and provide useful information. The addition of the PDA parsing is helpful.
zetaclient/zetacore/tx.go (3)
58-65
: LGTM!The restructuring simplifies the flow of control. The removal of the error return type streamlines the function's output.
Line range hint
68-79
:
LGTM!The function is straightforward and appropriate. Error handling is consistent.
Line range hint
82-114
:
LGTM!The function is comprehensive and appropriate. Error handling is consistent.
e2e/runner/solana.go (1)
134-135
: Ensure correct conversion of public key to bytes.Ensure that the public key is correctly converted to bytes for the withdrawal function.
- tx, err = r.SOLZRC20.Withdraw(r.ZEVMAuth, []byte(privkey.PublicKey().String()), amount) + tx, err = r.SOLZRC20.Withdraw(r.ZEVMAuth, privkey.PublicKey().Bytes(), amount)zetaclient/chains/evm/signer/outbound_data.go (1)
149-149
: Verify the removal of thelogger
parameter inIsOutboundProcessed
.The
logger
parameter has been removed from theIsOutboundProcessed
method call. Ensure that logging is handled appropriately elsewhere if needed.Verification successful
Verify the removal of the
logger
parameter inIsOutboundProcessed
.The
logger
parameter has been removed from theIsOutboundProcessed
method call. Ensure that logging is handled appropriately elsewhere if needed.
- The
IsOutboundProcessed
method inzetaclient/chains/evm/observer/outbound.go
does not include alogger
parameter.- Logging is handled within the method or the calling context, as seen in other parts of the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `IsOutboundProcessed` method implementation to ensure logging is handled appropriately. # Test: Search for the `IsOutboundProcessed` method implementation. Expect: No logger parameter in the method signature. ast-grep --lang go --pattern $'func IsOutboundProcessed(ctx context.Context, cctx *crosschaintypes.CrossChainTx) (included bool, confirmed bool, err error)'Length of output: 157
Script:
#!/bin/bash # Description: Search for the `IsOutboundProcessed` method implementation with any signature. # Test: Search for the `IsOutboundProcessed` method implementation. ast-grep --lang go --pattern $'func IsOutboundProcessed($$$) $$$'Length of output: 66
Script:
#!/bin/bash # Description: Search for the `IsOutboundProcessed` method name across the entire codebase. # Test: Search for the `IsOutboundProcessed` method definition. rg 'IsOutboundProcessed' -A 5Length of output: 15286
zetaclient/chains/interfaces/interfaces.go (2)
Line range hint
21-24
:
Verify the removal of thelogger
parameter inIsOutboundProcessed
.The
logger
parameter has been removed from theIsOutboundProcessed
method in theChainObserver
interface. Ensure that logging is handled appropriately elsewhere if needed.Verification successful
Verify the removal of the
logger
parameter inIsOutboundProcessed
.The
logger
parameter has been removed from theIsOutboundProcessed
method in theChainObserver
interface. The current implementations of this method do not include thelogger
parameter, confirming the change. Ensure that logging is handled appropriately elsewhere if needed.
- Implementations found in:
zetaclient/testutils/mocks/chain_clients.go
zetaclient/chains/bitcoin/observer/outbound.go
zetaclient/chains/solana/observer/outbound.go
zetaclient/chains/evm/observer/outbound.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `IsOutboundProcessed` method implementation in the `ChainObserver` interface to ensure logging is handled appropriately. # Test: Search for the `IsOutboundProcessed` method implementation. Expect: No logger parameter in the method signature. ast-grep --lang go --pattern $'func IsOutboundProcessed(ctx context.Context, cctx *crosschaintypes.CrossChainTx) (included bool, confirmed bool, err error)'Length of output: 157
Script:
#!/bin/bash # Description: Search for any implementation of the `IsOutboundProcessed` method in the codebase to verify its current signature and logging handling. # Test: Search for the `IsOutboundProcessed` method in the codebase. rg 'func IsOutboundProcessed' -A 5Length of output: 36
Script:
#!/bin/bash # Description: Search for method definitions within the `ChainObserver` interface and related implementations to verify the current signature and logging handling. # Test: Search for the `ChainObserver` interface definition and its methods. rg 'type ChainObserver interface' -A 20 # Test: Search for any implementation of methods within the `ChainObserver` interface. rg 'func (.*) ChainObserver' -A 10Length of output: 1590
Script:
#!/bin/bash # Description: Search for the implementations of the `IsOutboundProcessed` method across the codebase to verify logging handling. # Test: Search for the `IsOutboundProcessed` method implementations. rg 'func (.*) IsOutboundProcessed' -A 10Length of output: 4045
191-215
: Ensure consistency in method signatures forSolanaRPCClient
.The changes to the
SolanaRPCClient
interface improve readability and consistency. Verify the correctness of the new methods and altered return types.zetaclient/chains/solana/observer/outbound.go (2)
24-27
: LGTM!The
GetTxID
function correctly generates a unique transaction ID based on the chain ID, TSS address, and nonce.
188-254
: LGTM!The
PostVoteOutbound
function correctly handles the posting of votes to Zetacore for finalized outbound transactions. The logging and error handling are detailed and robust.zetaclient/orchestrator/bootstrap.go (1)
154-213
: LGTM!The
syncSignerMap
function includes enhanced error handling and correctly adds support for Solana chain configurations. The function is well-structured and robust.x/observer/types/chain_params.go (1)
324-327
: LGTM!The changes to the
GetDefaultSolanaLocalnetChainParams
function appear to be optimizations or adjustments to operational parameters.zetaclient/chains/bitcoin/signer/signer.go (5)
49-49
: LGTM! Idiomatic Go interface implementation.Changing the implementation declaration to use
(*Signer)(nil)
is a more idiomatic way to ensure that the interface is implemented by*Signer
.
65-66
: LGTM! Enhanced readability with trailing comma.Including a trailing comma in multi-line function signatures enhances readability and simplifies future additions.
331-333
: Ensure proper context management.The context retrieval using
zctx.FromContext(ctx)
has been removed. Verify that the context is managed correctly elsewhere in the code to avoid potential issues with context propagation.
339-348
: Improved logging structure.The logging statements have been reorganized, making the logging preparation more structured and the log messages clearer.
349-353
: Removed outbound flags validation.The condition checking for outbound flags has been removed. Ensure that this validation is not required for the correct functioning of the outbound process.
zetaclient/chains/solana/signer/signer.go (5)
28-28
: LGTM! Correct interface implementation.The
ChainSigner
interface is correctly implemented for theSigner
struct.
44-77
: LGTM! Robust initialization and error handling.The
NewSigner
function correctly initializes the Solana signer, including setting up the gateway ID and PDA with proper error handling.
79-111
: LGTM! Secure message signing process.The
SignMsgWithdraw
function correctly signs the message for the Solana gateway 'withdraw' transaction, ensuring security and integrity.
113-172
: LGTM! Secure transaction signing process.The
SignWithdrawTx
function correctly signs the Solana gateway 'withdraw' transaction, ensuring security and integrity.
174-380
: LGTM! Comprehensive outbound processing.The
TryProcessOutbound
function comprehensively handles the building, signing, and broadcasting of Solana transactions, with robust error handling and logging.cmd/zetae2e/local/local.go (2)
100-103
: LGTM! Enforcing Solana tests.The conditional check ensures that the
testSolana
flag is set to true, enforcing the execution of Solana tests.
321-325
: LGTM! Expanded Solana test scope.Constructing a slice of test names for Solana tests, including both deposit and withdrawal tests, correctly expands the scope of Solana tests executed.
zetaclient/zetacore/tx_test.go (2)
91-97
: Add test cases for Solana.The test cases for Solana mainnet and devnet have been added, which is a good addition to expand the test coverage.
103-105
: Simplify test logic.The test logic is simplified to directly compare the gas price multiplier without error handling, which improves the clarity and focus of the tests.
e2e/e2etests/e2etests.go (2)
58-58
: Add new constant for Solana withdrawal tests.The new constant
TestSolanaWithdrawName
is added for Solana withdrawal tests, which aligns with the existing naming conventions for test constants.
342-353
: Update test configurations for Solana-related end-to-end tests.The test configurations are updated to standardize the unit of measurement to lamports and add a new withdrawal test case, enhancing clarity and expanding test coverage.
e2e/txserver/zeta_tx_server.go (1)
397-397
: Update error message for Solana ZRC20 deployment.The error message is updated to indicate a failure related to "sol zrc20" instead of "btc zrc20," improving the clarity of the error reporting.
contrib/localnet/scripts/start-zetacored.sh (1)
250-252
: LGTM! Verify the configuration file's existence and correctness.The new section correctly retrieves the Solana tester's bech32 address and adds the genesis account. Ensure that the configuration file
/root/config.yml
exists and contains the correct entries.zetaclient/chains/evm/observer/outbound.go (4)
144-149
: LGTM! Improved logging structure.The logging structure has been improved for better readability and maintainability.
152-155
: LGTM! Enhanced error logging.The error logging has been enhanced for better readability and context.
159-163
: LGTM! Conditional logging for successful transactions.The conditional logging for successful transactions provides additional context.
181-181
: LGTM! Updated logger initialization.The logger initialization has been updated to enhance consistency and context for log messages.
zetaclient/chains/bitcoin/observer/observer.go (3)
327-327
: LGTM! Simplified error handling for ticker creation.The error handling for ticker creation has been simplified, enhancing readability and maintainability.
385-385
: LGTM! Enhanced error handling for block count retrieval.The error handling for block count retrieval has been enhanced to provide better context.
394-394
: LGTM! Enhanced error handling for gas price posting.The error handling for gas price posting has been enhanced to provide better context.
zetaclient/orchestrator/orchestrator.go (2)
224-230
: Approved: Integration of Solana chain parameters inresolveObserver
.The changes correctly add support for Solana chain parameters and update the observer accordingly.
580-626
: Approved: Addition ofScheduleCctxSolana
method.The new method
ScheduleCctxSolana
is well-structured and follows the existing pattern for scheduling outbound transactions. It includes appropriate error handling and logging.zetaclient/chains/evm/observer/outbound_test.go (1)
Line range hint
67-171
:
Approved: Refactor ofIsOutboundProcessed
test cases.The removal of the
zerolog.Nop()
parameter simplifies the function calls and the tests still cover the necessary functionality.zetaclient/chains/bitcoin/observer/outbound.go (1)
Line range hint
144-246
:
Approved: Refactor ofIsOutboundProcessed
method.The removal of the
logger
parameter and the use of the logger instance from theObserver
itself streamline the logging process and enhance encapsulation.zetaclient/chains/evm/signer/signer.go (4)
356-362
: Enhance error handling with deferred function.The addition of a deferred function to handle panics enhances the robustness of the
TryProcessOutbound
function.
364-366
: Improve logging consistency.The logger's context has been updated to use more consistent naming conventions, improving clarity and traceability in the logs.
664-664
: Simplify function by removing unnecessary parameter.The removal of the
flags
parameter simplifies theIsSenderZetaChain
function.
433-433
: Verify function usage in the codebase.Ensure that all calls to
IsSenderZetaChain
match the new signature.Verification successful
All calls to
IsSenderZetaChain
match the new signature.The function
IsSenderZetaChain
is defined and used only withinzetaclient/chains/evm/signer/signer.go
, and all occurrences match the new signature.
- Definition and usage of
IsSenderZetaChain
are consistent.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `IsSenderZetaChain` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'IsSenderZetaChain'Length of output: 1196
Script:
#!/bin/bash # Description: Verify all function definitions and calls to `IsSenderZetaChain` match the new signature. # Search for the function definition rg --type go -A 5 'func IsSenderZetaChain' # Search for all function calls rg --type go 'IsSenderZetaChain('Length of output: 643
Script:
#!/bin/bash # Description: Verify all function definitions and calls to `IsSenderZetaChain` match the new signature. # Search for the function definition rg --type go -A 5 'func IsSenderZetaChain' # Search for all function calls rg --type go 'IsSenderZetaChain\('Length of output: 712
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 26
Outside diff range, codebase verification and nitpick comments (12)
pkg/contract/solana/gateway_message.go (1)
1-8
: Optimize imports.The import statement for
encoding/binary
is separated from the others. Group all imports together for better readability.-import ( - "encoding/binary" - - "github.com/ethereum/go-ethereum/crypto" - "github.com/gagliardetto/solana-go" -) +import ( + "encoding/binary" + "github.com/ethereum/go-ethereum/crypto" + "github.com/gagliardetto/solana-go" +)zetaclient/testutils/mocks/chain_clients.go (3)
Line range hint
1-8
:
Optimize imports.The imports can be grouped together for better readability.
import ( "context" crosschaintypes "github.com/zeta-chain/zetacore/x/crosschain/types" observertypes "github.com/zeta-chain/zetacore/x/observer/types" "github.com/zeta-chain/zetacore/zetaclient/chains/interfaces" )
Line range hint
25-29
:
Improve method documentation.Add comments to document the
IsOutboundProcessed
method, explaining its purpose and parameters.// IsOutboundProcessed checks if the outbound transaction is processed. func (ob *EVMObserver) IsOutboundProcessed( _ context.Context, _ *crosschaintypes.CrossChainTx, ) (bool, bool, error) { return false, false, nil }
Line range hint
64-68
:
Improve method documentation.Add comments to document the
IsOutboundProcessed
method, explaining its purpose and parameters.// IsOutboundProcessed checks if the outbound transaction is processed. func (ob *BTCObserver) IsOutboundProcessed( _ context.Context, _ *crosschaintypes.CrossChainTx, ) (bool, bool, error) { return false, false, nil }e2e/e2etests/test_solana_withdraw.go (5)
18-18
: Improve logging format.The
r.Logger.Print
function should use a format string for better readability and consistency.- r.Logger.Print("TestSolanaWithdraw...sol zrc20 %s", r.SOLZRC20Addr.String()) + r.Logger.Printf("TestSolanaWithdraw...sol zrc20 %s", r.SOLZRC20Addr.String())
26-26
: Improve logging format.The
r.Logger.Print
function should use a format string for better readability and consistency.- r.Logger.Print(" from %s supply of %s sol zrc20: %d", r.ZEVMAuth.From, r.EVMAddress(), supply) + r.Logger.Printf(" from %s supply of %s sol zrc20: %d", r.ZEVMAuth.From, r.EVMAddress(), supply)
59-59
: Improve logging format.The
r.Logger.Print
function should use a format string for better readability and consistency.- r.Logger.Print("Receipt txhash %s status %d", receipt.TxHash, receipt.Status) + r.Logger.Printf("Receipt txhash %s status %d", receipt.TxHash, receipt.Status)
64-64
: Improve logging format.The
r.Logger.Print
function should use a format string for better readability and consistency.- r.Logger.Print(" from %s supply of %s sol zrc20 after: %d", r.ZEVMAuth.From, r.EVMAddress(), supply) + r.Logger.Printf(" from %s supply of %s sol zrc20 after: %d", r.ZEVMAuth.From, r.EVMAddress(), supply)
Line range hint
72-72
:
Use the logger for output.Instead of using
fmt.Printf
, consider using the logger for consistency.- fmt.Printf("pda parsed: %+v\n", pda) + r.Logger.Infof("pda parsed: %+v", pda)e2e/runner/setup_solana.go (3)
26-26
: Improve logging format.The
r.Logger.Print
function should use a format string for better readability and consistency.- r.Logger.Print("⚙️ deploying gateway program on Solana") + r.Logger.Printf("⚙️ deploying gateway program on Solana")
Line range hint
34-34
:
Handle errors more gracefully.Instead of using
require.NoError
directly, consider logging the error for better debugging.- require.NoError(r, err) + require.NoError(r, err, "Error getting deployer account balance")
72-72
: Use the logger for output.Instead of using
fmt.Printf
, consider using the logger for consistency.- fmt.Printf("pda parsed: %+v\n", pda) + r.Logger.Infof("pda parsed: %+v", pda)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (33)
- cmd/zetaclientd/utils.go (1 hunks)
- cmd/zetae2e/local/local.go (2 hunks)
- contrib/localnet/docker-compose.yml (1 hunks)
- contrib/localnet/orchestrator/start-zetae2e.sh (1 hunks)
- contrib/localnet/scripts/start-zetacored.sh (1 hunks)
- e2e/e2etests/e2etests.go (2 hunks)
- e2e/e2etests/test_solana_deposit.go (1 hunks)
- e2e/e2etests/test_solana_withdraw.go (1 hunks)
- e2e/runner/setup_solana.go (4 hunks)
- e2e/runner/solana.go (2 hunks)
- e2e/txserver/zeta_tx_server.go (1 hunks)
- pkg/contract/solana/gateway_message.go (1 hunks)
- pkg/contract/solana/types.go (2 hunks)
- x/observer/types/chain_params.go (1 hunks)
- zetaclient/chains/bitcoin/observer/observer.go (3 hunks)
- zetaclient/chains/bitcoin/observer/outbound.go (4 hunks)
- zetaclient/chains/bitcoin/signer/signer.go (4 hunks)
- zetaclient/chains/evm/observer/outbound.go (4 hunks)
- zetaclient/chains/evm/observer/outbound_test.go (4 hunks)
- zetaclient/chains/evm/signer/outbound_data.go (1 hunks)
- zetaclient/chains/evm/signer/signer.go (10 hunks)
- zetaclient/chains/interfaces/interfaces.go (2 hunks)
- zetaclient/chains/solana/observer/gas_price.go (1 hunks)
- zetaclient/chains/solana/observer/observer.go (5 hunks)
- zetaclient/chains/solana/observer/outbound.go (1 hunks)
- zetaclient/chains/solana/signer/signer.go (1 hunks)
- zetaclient/common/constant.go (1 hunks)
- zetaclient/orchestrator/bootstrap.go (3 hunks)
- zetaclient/orchestrator/orchestrator.go (7 hunks)
- zetaclient/testutils/mocks/chain_clients.go (3 hunks)
- zetaclient/zetacore/client_vote.go (1 hunks)
- zetaclient/zetacore/tx.go (2 hunks)
- zetaclient/zetacore/tx_test.go (1 hunks)
Files skipped from review due to trivial changes (2)
- cmd/zetaclientd/utils.go
- contrib/localnet/docker-compose.yml
Additional context used
Path-based instructions (31)
zetaclient/common/constant.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.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/contract/solana/types.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.pkg/contract/solana/gateway_message.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/solana/observer/gas_price.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/testutils/mocks/chain_clients.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.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.e2e/runner/setup_solana.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/zetacore/tx.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/chains/solana/observer/observer.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/zetacore/client_vote.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/evm/signer/outbound_data.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.contrib/localnet/orchestrator/start-zetae2e.sh (1)
Pattern
**/*.sh
: Review the shell scripts, point out issues relative to security, performance, and maintainability.zetaclient/chains/interfaces/interfaces.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.zetaclient/orchestrator/bootstrap.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.x/observer/types/chain_params.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/signer/signer.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.cmd/zetae2e/local/local.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/zetacore/tx_test.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.e2e/txserver/zeta_tx_server.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.contrib/localnet/scripts/start-zetacored.sh (1)
Pattern
**/*.sh
: Review the shell scripts, point out issues relative to security, performance, and maintainability.zetaclient/chains/evm/observer/outbound.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/observer/observer.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/orchestrator/orchestrator.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/evm/observer/outbound_test.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/bitcoin/observer/outbound.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.zetaclient/chains/evm/signer/signer.go (1)
Pattern
**/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (57)
zetaclient/common/constant.go (1)
4-5
: LGTM!The addition of
DefaultGasPriceMultiplier
enhances configurability across different blockchain implementations. The comment clearly explains its purpose.e2e/e2etests/test_solana_deposit.go (1)
14-19
: LGTM! But verify the function usage in the codebase.The changes improve the flexibility and robustness of the test function by introducing dynamic input handling and validation. Ensure that all function calls to
TestSolanaDeposit
match the new signature.pkg/contract/solana/types.go (2)
17-18
: LGTM!The renaming of
ChainId
toChainID
improves consistency and clarity.
45-64
: LGTM!The introduction of
WithdrawInstructionParams
extends functionality and is well-defined for handling withdrawal operations.pkg/contract/solana/gateway_message.go (1)
63-68
: Add error handling for signature copy.The
copy
operation should include error handling to ensure the signature is copied correctly.func (msg *MsgWithdraw) SigRS() [64]byte { var sig [64]byte if n := copy(sig[:], msg.Signature[:64]); n != 64 { // Handle error } return sig }Likely invalid or redundant comment.
e2e/e2etests/test_solana_withdraw.go (1)
16-16
: Ensure the correct number of arguments.The
require.Len
function is used to ensure that the number of arguments is exactly 1. This is a good practice to validate input.e2e/runner/setup_solana.go (1)
Line range hint
16-21
:
Simplified method looks good.The changes simplify the method by focusing on setting the
SolanaDeployerAddress
.zetaclient/zetacore/tx.go (1)
58-65
: Simplified function looks good, but verify the impact of removing error handling.The changes improve the function's clarity and reliability by ensuring it always returns a valid multiplier. However, ensure that removing error handling for unknown chains does not cause issues elsewhere in the codebase.
e2e/runner/solana.go (1)
4-14
: Imports look good!The new imports are necessary for the added functionality and are correctly included.
zetaclient/chains/solana/observer/observer.go (6)
35-37
: New fields inObserver
struct look good!The additions of
pda
andfinalizedTxResults
enhance the observer's capability to track finalized transactions effectively.
67-90
: Initialization logic inNewObserver
looks good!The new fields are correctly initialized, and error handling is appropriately managed.
131-136
: New background tasks inStart
method look good!The additions for watching outbound trackers and monitoring gas prices enhance the observer's functionality.
149-154
: Thread-safeSetTxResult
method looks good!The method ensures thread safety while setting the transaction result.
156-161
: Thread-safeGetTxResult
method looks good!The method ensures thread safety while retrieving the transaction result.
163-168
: Thread-safeIsTxFinalized
method looks good!The method ensures thread safety while checking if a transaction is finalized.
zetaclient/chains/evm/signer/outbound_data.go (1)
149-149
: Verify the logging mechanism after removing thelogger
parameter.The removal of the
logger
parameter from theIsOutboundProcessed
function call simplifies the function signature. Ensure that logging is handled appropriately elsewhere to maintain visibility into the function's operations.zetaclient/chains/interfaces/interfaces.go (2)
191-215
: LGTM! The method signature updates enhance consistency and clarity.The updates to the
SolanaRPCClient
interface use shorthand notation for error handling and add new methods for expanded functionality. These changes improve the interface's usability and maintainability.
Line range hint
35-38
:
Verify the logging mechanism after removing thelogger
parameter.The removal of the
logger
parameter from theIsOutboundProcessed
method simplifies the method signature. Ensure that logging is handled appropriately elsewhere to maintain visibility into the method's operations.Verification successful
Verify the logging mechanism after removing the
logger
parameter.The search results indicate that logging is handled in the
orchestrator.go
file when theIsOutboundProcessed
method is called. Theoc.logger.Error
andoc.logger.Info
methods are used to log errors and informational messages respectively. This confirms that logging is appropriately handled outside theIsOutboundProcessed
method, ensuring visibility into the method's operations even after the removal of thelogger
parameter.
- Files to review:
zetaclient/orchestrator/orchestrator.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logging mechanism for the `IsOutboundProcessed` method in the `ChainObserver` interface. # Test: Search for the `IsOutboundProcessed` method implementation. Expect: Logging is handled within the method. rg --type go -A 10 'func IsOutboundProcessed' zetaclient/chains/evm/observerLength of output: 78
Script:
#!/bin/bash # Description: Verify the logging mechanism for the `IsOutboundProcessed` method in the `ChainObserver` interface. # Test: Search for the `IsOutboundProcessed` method implementation across the entire codebase. Expect: Logging is handled within the method. rg --type go -A 10 'func IsOutboundProcessed'Length of output: 47
Script:
#!/bin/bash # Description: Verify the logging mechanism for the `IsOutboundProcessed` method in the `ChainObserver` interface. # Test: Search for the `ChainObserver` interface definition and its methods. Expect: Locate the `IsOutboundProcessed` method and verify its logging mechanism. rg --type go -A 10 'type ChainObserver interface'Length of output: 862
Script:
#!/bin/bash # Description: Verify the logging mechanism for the `IsOutboundProcessed` method in the `ChainObserver` interface. # Test: Search for the implementations of the `ChainObserver` interface. Expect: Locate the `IsOutboundProcessed` method implementation and verify its logging mechanism. rg --type go 'ChainObserver' --files-with-matches | xargs rg --type go -A 10 'IsOutboundProcessed'Length of output: 5787
zetaclient/chains/solana/observer/outbound.go (3)
24-27
: Verify performance implications offmt.Sprintf
in high-frequency calls.While
fmt.Sprintf
is convenient, it may introduce performance overhead in high-frequency calls. Consider using string concatenation for better performance.
219-219
: Verify slot value for correctness.The TODO comment suggests verifying if the slot is equivalent to block height. This is important for accurate reporting.
208-208
: Review default compute units consumed value.The default value of 5001 for compute units consumed should be reviewed to ensure it is appropriate. Consider making this configurable.
x/observer/types/chain_params.go (1)
324-327
: Verify implications of parameter adjustments.The adjustments to
GasPriceTicker
,InboundTicker
, andOutboundTicker
values may have significant implications on transaction processing and resource management. Verify these changes to ensure they align with the operational strategy.zetaclient/chains/bitcoin/signer/signer.go (5)
49-49
: LGTM! The interface implementation declaration is correct.The change from
&Signer{}
to(*Signer)(nil)
is a more idiomatic way to assert thatSigner
implementsChainSigner
.
65-66
: LGTM! The trailing comma improves readability.The addition of the trailing comma in the
NewSigner
function signature enhances readability and facilitates future modifications.
331-332
: Robust error handling with deferred function.The deferred function ensures that any panic is logged appropriately, enhancing the robustness of the outbound processing.
339-348
: Improved logging mechanism.The logger instance is prepared earlier in the function, and a new log message for the outbound parameters is included before the gas token check, improving clarity and maintainability.
349-356
: Streamlined coin type check.The check for valid coin types has been streamlined, simplifying the control flow.
zetaclient/chains/solana/signer/signer.go (3)
44-77
: LGTM! The function initializes the Solana signer correctly.The function correctly initializes the base signer and sets up the Solana RPC client and gateway information.
79-111
: LGTM! The function signs the Solana gateway 'withdraw' message correctly.The function correctly signs the message using TSS and prepares the withdraw message.
174-380
: LGTM! The function handles the outbound process correctly.The function correctly builds, signs, and broadcasts the Solana transaction using the TSS signer.
cmd/zetae2e/local/local.go (2)
100-103
: Ensure thetestSolana
flag is correctly set.The change ensures that Solana tests will run when the flag is not explicitly set, leading to more comprehensive testing scenarios.
321-325
: Expanded Solana testing routine.The Solana testing routine now includes multiple test cases, improving the robustness of the testing framework.
zetaclient/zetacore/tx_test.go (1)
91-97
: LGTM! But verify the function usage in the codebase.The addition of test cases for Solana-related chains and the removal of the
fail
field are appropriate changes.However, ensure that all function calls to
GasPriceMultiplier
are correctly handled in the codebase.e2e/e2etests/e2etests.go (2)
57-58
: LGTM!The addition of
TestSolanaWithdrawName
is appropriate and aligns with the new functionality.
342-353
: LGTM!The inclusion of new test cases for Solana deposit and withdrawal enhances the test coverage and ensures comprehensive testing.
e2e/txserver/zeta_tx_server.go (1)
397-397
: LGTM!The change in the error message from "btc zrc20" to "sol zrc20" is appropriate and reflects the shift in focus to Solana-related deployments.
contrib/localnet/scripts/start-zetacored.sh (1)
250-252
: Verify Solana address configuration and yq availability.Ensure that the configuration file
/root/config.yml
contains the correct Solana addresses and that theyq
command is available in the environment.zetaclient/chains/evm/observer/outbound.go (2)
144-149
: LGTM! Improved logging structure.The new logging structure is concise and focuses on essential information, enhancing readability and debugging.
181-181
: LGTM! Enhanced logger initialization.Using a more specific logger instance provides better context in the logs, improving granularity and relevance.
zetaclient/chains/bitcoin/observer/observer.go (2)
327-327
: LGTM! Improved error handling.Wrapping the error with additional context improves the clarity of error messages and aids in debugging.
385-385
: LGTM! Improved error handling.Wrapping errors with additional context improves the clarity of error messages and aids in debugging.
Also applies to: 394-394
zetaclient/orchestrator/orchestrator.go (3)
224-230
: Confirm chain parameter updates for Solana.The changes correctly update the chain parameters for Solana if they differ from the current parameters.
389-392
: Confirm scheduling of cross-chain transactions for Solana.The changes correctly include a case for scheduling cross-chain transactions for Solana, invoking
ScheduleCctxSolana
.
Line range hint
580-630
: Confirm scheduling of Solana outbound keysign transactions.The new function
ScheduleCctxSolana
follows a structure similar to existing methods for Bitcoin and EVM, ensuring consistency in handling cross-chain transactions.zetaclient/chains/evm/observer/outbound_test.go (6)
67-69
: Confirm removal of the third parameter fromIsOutboundProcessed
.The removal of the third parameter simplifies the function's interface and enhances readability.
91-93
: Confirm removal of the third parameter fromIsOutboundProcessed
.The removal of the third parameter simplifies the function's interface and enhances readability.
99-101
: Confirm removal of the third parameter fromIsOutboundProcessed
.The removal of the third parameter simplifies the function's interface and enhances readability.
113-115
: Confirm removal of the third parameter fromIsOutboundProcessed
.The removal of the third parameter simplifies the function's interface and enhances readability.
163-165
: Confirm removal of the third parameter fromIsOutboundProcessed
.The removal of the third parameter simplifies the function's interface and enhances readability.
171-173
: Confirm removal of the third parameter fromIsOutboundProcessed
.The removal of the third parameter simplifies the function's interface and enhances readability.
zetaclient/chains/bitcoin/observer/outbound.go (3)
144-146
: Confirm removal of thelogger
parameter fromIsOutboundProcessed
.The removal of the
logger
parameter and centralizing logging operations within the class enhance encapsulation and readability.
204-206
: Confirm centralized logging for outbound confirmation.The centralized logging approach improves the clarity and organization of log outputs related to outbound transactions.
240-246
: Confirm centralized logging for error handling and confirmation messages.The centralized logging approach improves the clarity and organization of log outputs related to outbound transactions.
zetaclient/chains/evm/signer/signer.go (4)
50-50
: LGTM!The change to use
(*Signer)(nil)
instead of&Signer{}
is more idiomatic and avoids creating an unnecessary instance.
356-362
: Improved error handling with deferred function.The addition of a deferred function to capture panics enhances the robustness of the
TryProcessOutbound
function.
364-366
: Improved logging for better traceability.The updated logging messages provide better clarity and consistency.
664-664
: LGTM!The simplification of the
IsSenderZetaChain
function improves readability and maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass, will need to do a second round
…ana-outbound-SOL
!!!WARNING!!! Be very careful about using Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203 Pay extra attention to the way |
…d tracker iteration logic into ProcessOutboundTrackers sub method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Issues seem to be tracked correctly
* port Panruo's outbound code and make compile pass * make SOL withdraw e2e test passing * make solana outbound tracker goroutine working * allow solana gateway address to update * integrate sub methods of SignMsgWithdraw and SignWithdrawTx * initiate solana outbound tracker reporter * implemented solana outbound tx verification * use the amount in tx result for outbound vote * post Solana priority fee to zetacore * config Solana fee payer private key * resolve 1st wave of comments in PR review * resolve 2nd wave of comments * refactor IsOutboundProcessed as VoteOutboundIfConfirmed; move outbound tracker iteration logic into ProcessOutboundTrackers sub method * resolve 3rd wave of PR feedback * added description to explain what do we do about the outbound tracker txHash * add additional error message; add additional method comment * fix gosec err * replace contex.TODO() with context.Background()
* update protocol contracts package * update config and runner * init contract deploy * add erc1967proxy in contracts * fix gateway evm deploy * zevm setuip * format * update version * update v1 import path * reorganize import v2 * fix import * update new version * add custody setup * feat(E2E): add body for smart contract V2 tests (#2609) * add runner for v2 * implement helper function * add test bodies * add makefile entry * import * update system contracts * feat: implement gas token deposit with protocol contract v2 (#2616) * add protocol contract version in cctx * read deposit from ZetaClient * deposit gas token * run deposit * add tests * add event check * add workflow for test * put error handling higher level ObserverGateway * add named interface * feat: withdraw SOL from ZEVM to Solana (#2560) * port Panruo's outbound code and make compile pass * make SOL withdraw e2e test passing * make solana outbound tracker goroutine working * allow solana gateway address to update * integrate sub methods of SignMsgWithdraw and SignWithdrawTx * initiate solana outbound tracker reporter * implemented solana outbound tx verification * use the amount in tx result for outbound vote * post Solana priority fee to zetacore * config Solana fee payer private key * resolve 1st wave of comments in PR review * resolve 2nd wave of comments * refactor IsOutboundProcessed as VoteOutboundIfConfirmed; move outbound tracker iteration logic into ProcessOutboundTrackers sub method * resolve 3rd wave of PR feedback * added description to explain what do we do about the outbound tracker txHash * add additional error message; add additional method comment * fix gosec err * replace contex.TODO() with context.Background() * feat: detect memo in btc txn from OP_RETURN and inscription (#2533) * parse inscription like witness data * more comment * remove unused code * parse inscription * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: Dmitry S <[email protected]> * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Dmitry S <[email protected]> * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: Dmitry S <[email protected]> * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * pull origin * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Dmitry S <[email protected]> * review feedbacks * update review feedbacks * add mainnet txn * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * parse inscription like witness data * more comment * remove unused code * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: Dmitry S <[email protected]> * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Dmitry S <[email protected]> * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: Dmitry S <[email protected]> * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * pull origin * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Dmitry S <[email protected]> * review feedbacks * update review feedbacks * update make generate * fix linter * remove over flow * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * Update zetaclient/chains/bitcoin/tokenizer.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * Update zetaclient/chains/bitcoin/tokenizer.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * Update zetaclient/chains/bitcoin/tokenizer.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * Update zetaclient/chains/bitcoin/tokenizer.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * update review feedback * update code commnet * update comment * more comments * Update changelog.md * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * clean up * format code --------- Co-authored-by: Dmitry S <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * refactor(zetaclient)!: improve AppContext (#2568) * Implement chain registry * Rewrite test-cases for AppContext * Drop `supplychecker` * Refactor app ctx Update worker * Refactor orchestrator * Refactor observer&signer; DROP postBlockHeaders * Fix test cases [1] * Update changelog * Allow Zeta Chain in appContext; address PR comments [1] * Fix app context update * Check for `chain.IsZeta()` * Add AppContext.FilterChains * Fix test cases [2] * Fix test cases [3] * Address PR comments [1] * Address PR comments [2] * Add tests for `slices` * Fix e2e tests [1] * Fix e2e tests [2] * Resolve conflicts, converge codebase between PRs * Add lodash; remove slices pkg * Address PR comments * Minor logging fix * Address PR comments * fix(`crosschain`): set sender for ERC20 whitelist admin CCTX inbound (#2631) * fix whitelist sender * add E2E test * fix(ci): Update golang cross compile to 1.22.4 (#2635) * Update golang cross compile to 1.22.4 * update deprecated --skip-validate --skip-release flags * Added goreleaser check (#2636) * update solidity version * feat: integrate Smart Contract V2 `depositAndCall` for ETH and ERC20 (#2639) * add testdapp v2 * update ZRC20 version * initialize deposit and call * add deposit and call * implement E2E test * move contracts * fix evm deploy ZRC20 * deposit and call local test * complete deposit and call test * add support for E2E tests * comments * fix unit tests * feat: support withdraws, calls and reverts with Gateway contract (#2666) * add v2 zevm inbound * refactor abi in signers * outbound for gas withdraw * fix withdraw * gateway execute parse * add support for withdraw and call * erc20 withdraw and withdraw and call * implement erc20 withdraw e2e tests * test withdraw and call * reading inbound for no assset call * simple call implementation * initialize revert * fix the tests * upgrade smart contracts to latest version * small fix smart contract call * revert tests skeleton * fix test * implement revert tests * add revert support * fix revert tests * make generate * add support for revert gas limit * fix tests * add liquidity test * fix liquidity test * Update proto/zetachain/zetacore/pkg/coin/coin.proto Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * Update x/crosschain/types/revert_options.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * Update x/crosschain/types/revert_options.go Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * Borja comments * Stefan comments * dmitry review * fix unit tests --------- Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> * changelogs * fix unit test * tentative fix for upgrade * fix gateway zero address * fix admin test * add back admin tests * v2 conditional matric --------- Co-authored-by: Charlie Chen <[email protected]> Co-authored-by: dev-bitSmiley <[email protected]> Co-authored-by: Dmitry S <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Francisco de Borja Aranda Castillejo <[email protected]> Co-authored-by: Christopher Fuka <[email protected]> Co-authored-by: Charlie <[email protected]>
Description
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Tests