Skip to content
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: Solana relayer (fee payer) key importer, encryption and decryption #2673

Merged
merged 17 commits into from
Aug 15, 2024

Conversation

ws4charlie
Copy link
Contributor

@ws4charlie ws4charlie commented Aug 9, 2024

Description

The goals of this PR:

  1. remove hardcoded E2E solana private key from zetaclient code and use config file localnet.yml to inject solana E2E test keys.
  2. introduce relayer key importing process that allows TSS signers to bring their own Solana keys to sign Solana outbound transactions. Relayer key is an optional private key. Solana outbound needs at least one relayer to be able to work.

The changes of this PR:

  • Utils for encryption/decryption; Utils for creating/reading relayer key files.
  • CLI command import-relayer-key to import and encrypt relayer private key
  • CLI command relayer-address to decrypt and show imported relayer address
  • E2E test adjustment using above utils and CLIs.
  • Use above utils and CLIs to replace current relayer key handling.
  • Add relayer key metrics relayer_key_balance.

Closes: #2614
Closes: #2606

How Has This Been Tested?

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Summary by CodeRabbit

  • New Features

    • Introduced new CLI commands for importing and managing relayer private keys.
    • Added functionality to handle Solana relayer accounts in various scripts and configurations.
    • Implemented AES-256-GCM encryption methods for secure data handling.
  • Bug Fixes

    • Enhanced error handling during encryption processes.
  • Refactor

    • Improved the management of key paths and streamlined the initialization process.
    • Updated the Signer methods to reflect changes in the key management approach.
  • Tests

    • Added unit tests for new encryption functionalities and existing methods to ensure reliability.
  • Documentation

    • Updated comments and documentation to reflect recent changes in functionality and key management.

Copy link

gitguardian bot commented Aug 9, 2024

⚠️ GitGuardian has uncovered 5 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
13340122 Triggered Generic High Entropy Secret 5da629e cmd/zetaclientd/import_relayer_keys.go View secret
13340122 Triggered Generic High Entropy Secret 66b7027 zetaclient/chains/solana/signer/signer_test.go View secret
13340122 Triggered Generic High Entropy Secret 66b7027 zetaclient/chains/solana/signer/signer_test.go View secret
13340122 Triggered Generic High Entropy Secret 98f041a cmd/zetaclientd/import_relayer_keys.go View secret
13392123 Triggered Generic High Entropy Secret 66b7027 zetaclient/chains/solana/signer/signer_test.go View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@ws4charlie ws4charlie marked this pull request as draft August 9, 2024 16:33
Copy link
Contributor

coderabbitai bot commented Aug 9, 2024

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Walkthrough

The recent changes significantly refactor the ZetaChain codebase to enhance the management of relayer keys, particularly for the Solana blockchain. Key functionalities related to the encryption and handling of relayer keys have been introduced, alongside improvements to configuration structures and command-line utilities. This refactoring streamlines key management processes while improving code maintainability and readability, reflecting a strategic shift in the application's architecture towards more robust and secure operations.

Changes

Files Change Summary
cmd/zetaclientd/... Updated EncryptTSSFile to use a new utility for AES-256-GCM encryption, improving readability and error handling. Introduced commands for importing and managing relayer keys.
cmd/zetaclientd/init.go Renamed SolanaKey to RelayerKeyPath, removing Solana key management functionalities and focusing on relayer key management.
cmd/zetae2e/config/localnet.yml Added support for relayer accounts configuration, including private keys and addresses for Solana.
contrib/localnet/orchestrator/... Introduced a multi-stage Docker build for Solana support, enhancing the orchestrator’s interaction with Solana networks.
contrib/localnet/scripts/... Updated shell scripts to fund Solana accounts and manage relayer keys, improving usability and reducing console output clutter.
pkg/crypto/... Introduced new AES-256-GCM encryption and decryption functionalities with base64 encoding, along with associated tests to ensure reliability.
pkg/os/... Added utility functions for expanding home directory paths and checking file existence.
rpc/namespaces/ethereum/debug/... Replaced custom home directory expansion with zetaos.ExpandHomeDir, streamlining file path handling in multiple debug APIs.
zetaclient/chains/solana/... Refactored the Signer struct to focus on relayer keys instead of fee payers, reflecting changes in transaction signing processes.
zetaclient/config/types.go Added RelayerKeyPath to the configuration struct and removed Solana private key loading functionality, streamlining configuration management.
zetaclient/keys/relayer_keys.go Introduced functionality for managing relayer keys, including methods for resolving addresses and loading keys from files.
zetaclient/orchestrator/bootstrap.go Updated key loading processes to use the new relayer key management methods, enhancing how the application interacts with Solana.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant Config
    participant RelayerKey
    participant Signer

    User->>CLI: Import relayer key
    CLI->>Config: Set RelayerKeyPath
    CLI->>RelayerKey: Load relayer key
    RelayerKey->>Config: Retrieve key
    CLI->>Signer: Use relayer key for signing transactions
Loading
sequenceDiagram
    participant User
    participant CLI
    participant AES
    participant File

    User->>CLI: Encrypt TSS file
    CLI->>AES: Encrypt using AES-256-GCM
    AES->>File: Save encrypted data
    File->>CLI: Confirm save
Loading

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Outside diff range, codebase verification and nitpick comments (10)
pkg/os/path.go (1)

10-24: LGTM! Consider enhancing the documentation for clarity.

The ExpandHomeDir function is well-implemented. Consider expanding the documentation to clarify the behavior for paths with a leading tilde.

- // ExpandHomeDir expands a leading tilde in the path to the home directory of the current user.
+ // ExpandHomeDir expands a leading tilde (~) in the path to the home directory of the current user.
pkg/crypto/aes256_gcm.go (1)

32-54: Enhance Error Logging in DecryptAES256GCMBase64 Function

The DecryptAES256GCMBase64 function currently wraps errors but does not log them. To improve debugging and error tracking, consider adding logging for any errors that occur during decryption. This will help in identifying issues without exposing sensitive information.

  • Add logging for errors in the DecryptAES256GCMBase64 function to ensure proper tracking of decryption failures.
Analysis chain

Check for proper error handling and logging.

The DecryptAES256GCMBase64 function should ensure that any errors, especially those related to decryption failures, are logged appropriately for debugging purposes without exposing sensitive information.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if error logging is implemented properly in the codebase.

# Test: Search for logging practices in the codebase.
rg --type go 'log\.(Error|Warn|Info|Debug|Fatal)'

Length of output: 9186

zetaclient/chains/solana/signer/withdraw.go (1)

Line range hint 92-101:
Clarify the signing logic for maintainability.

The logic for signing the transaction with the relayer key is appropriate. Consider adding a brief comment to explain why the relayer key is used here, which would improve code maintainability and readability.

+  // Use the relayer key to sign the transaction as per the updated security model.
zetaclient/config/types.go (1)

82-82: Consider using snake_case for JSON fields.

The JSON field RelayerKeyPath should follow the snake_case convention as noted in the TODO comment above.

- RelayerKeyPath string `json:"RelayerKeyPath"`
+ RelayerKeyPath string `json:"relayer_key_path"`
cmd/zetaclientd/import_relayer_keys.go (6)

55-59: Handle potential errors when expanding home directories.

The error from zetaos.ExpandHomeDir is logged, but the application exits immediately after logging. Consider providing more context in the log message or handling the error more gracefully.

- log.Fatal().Err(err).Msg("failed to resolve default relayer key path")
+ log.Fatal().Err(err).Msgf("failed to resolve default relayer key path: %s", defaultRelayerKeyPath)

79-84: Validate input arguments early.

The validation of the privateKey and password could be done earlier, potentially before any other operations, to fail fast and avoid unnecessary computations.

Consider moving this validation to the beginning of the ImportRelayerKey function.


93-97: Ensure directory creation errors are properly handled.

The os.MkdirAll function is used to create directories with specific permissions. Ensure that any errors are logged with sufficient context to aid in debugging.

- return errors.Wrapf(err, "failed to create relayer key path: %s", keyPath)
+ log.Error().Err(err).Msgf("failed to create relayer key path: %s", keyPath)

100-105: Prevent potential file overwrite issues.

The check for existing files is good practice. Ensure that users are adequately informed about the implications of overwriting existing files.

Consider adding a user prompt or a flag to allow overwriting if necessary.


120-123: Ensure secure file permissions.

The file is created with owner read/write permissions, which is good. Ensure that this is documented and that users are aware of the security implications.

Consider adding a comment or documentation about the chosen file permissions.


161-177: Improve error handling in path resolution.

The resolveRelayerKeyPath function handles errors but could provide more context in its error messages to aid debugging.

- return "", "", errors.Wrap(err, "failed to resolve relayer key path")
+ return "", "", errors.Wrapf(err, "failed to resolve relayer key path: %s", relayerKeyPath)
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f78ff55 and 5da629e.

Files selected for processing (23)
  • cmd/zetaclientd/encrypt_tss.go (3 hunks)
  • cmd/zetaclientd/import_relayer_keys.go (1 hunks)
  • cmd/zetaclientd/init.go (4 hunks)
  • cmd/zetae2e/config/localnet.yml (1 hunks)
  • contrib/localnet/orchestrator/Dockerfile.fastbuild (1 hunks)
  • contrib/localnet/orchestrator/start-zetae2e.sh (1 hunks)
  • contrib/localnet/scripts/start-zetaclientd.sh (3 hunks)
  • e2e/config/config.go (2 hunks)
  • pkg/chains/chain.go (1 hunks)
  • pkg/chains/chain_test.go (1 hunks)
  • pkg/crypto/aes256_gcm.go (1 hunks)
  • pkg/crypto/aes256_gcm_test.go (1 hunks)
  • pkg/os/path.go (1 hunks)
  • pkg/os/path_test.go (1 hunks)
  • rpc/namespaces/ethereum/debug/api.go (2 hunks)
  • rpc/namespaces/ethereum/debug/trace.go (2 hunks)
  • rpc/namespaces/ethereum/debug/utils.go (2 hunks)
  • server/start.go (2 hunks)
  • zetaclient/chains/solana/signer/signer.go (5 hunks)
  • zetaclient/chains/solana/signer/withdraw.go (3 hunks)
  • zetaclient/config/types.go (3 hunks)
  • zetaclient/keys/relayer_keys.go (1 hunks)
  • zetaclient/orchestrator/bootstrap.go (2 hunks)
Additional context used
Path-based instructions (21)
pkg/os/path.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

cmd/zetaclientd/encrypt_tss.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

rpc/namespaces/ethereum/debug/utils.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

pkg/os/path_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

rpc/namespaces/ethereum/debug/trace.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

pkg/crypto/aes256_gcm.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

zetaclient/keys/relayer_keys.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.

contrib/localnet/scripts/start-zetaclientd.sh (1)

Pattern **/*.sh: Review the shell scripts, point out issues relative to security, performance, and maintainability.

zetaclient/config/types.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

pkg/crypto/aes256_gcm_test.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/zetaclientd/import_relayer_keys.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

pkg/chains/chain.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

contrib/localnet/orchestrator/start-zetae2e.sh (1)

Pattern **/*.sh: Review the shell scripts, point out issues relative to security, performance, and maintainability.

zetaclient/orchestrator/bootstrap.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

rpc/namespaces/ethereum/debug/api.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

pkg/chains/chain_test.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

e2e/config/config.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

server/start.go (1)

Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

Gitleaks
cmd/zetae2e/config/localnet.yml

60-60: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


63-63: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

cmd/zetaclientd/import_relayer_keys.go

21-21: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

Additional comments not posted (30)
contrib/localnet/orchestrator/Dockerfile.fastbuild (1)

3-3: LGTM! Verify the compatibility of the new Solana base image.

The addition of the Solana base image and the Solana CLI tool enhances the orchestrator's capabilities. Ensure that the new base image is compatible with the existing setup.

Also applies to: 11-11

pkg/os/path.go (1)

26-33: LGTM! The FileExists function is efficient and clear.

The implementation correctly checks for file existence and handles errors appropriately.

cmd/zetaclientd/encrypt_tss.go (1)

Line range hint 25-43:
LGTM! Verify the integration with the new encryption utility.

The refactoring of EncryptTSSFile enhances readability and maintainability. Ensure that the crypto.EncryptAES256GCM utility function is correctly integrated and tested.

Verification successful

Integration Verified: EncryptAES256GCM is Correctly Integrated with EncryptTSSFile

The EncryptAES256GCM function is properly implemented and integrated within the EncryptTSSFile function. The function uses AES encryption in GCM mode, ensuring secure encryption of the provided data. No further issues were found regarding this integration.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of `crypto.EncryptAES256GCM` with the `EncryptTSSFile` function.

# Test: Check for the implementation of `EncryptAES256GCM` in the `crypto` package.
ast-grep --lang go --pattern $'func EncryptAES256GCM($_, $_) ($_, error) {
  $$$
}' 

Length of output: 1244

rpc/namespaces/ethereum/debug/utils.go (1)

39-39: Verify the behavior of zetaos.ExpandHomeDir.

The function ExpandHomeDir from the zetaos package replaces the local implementation. Ensure that it correctly handles all edge cases related to home directory expansion.

Verification successful

Verification of zetaos.ExpandHomeDir Functionality

The ExpandHomeDir function in pkg/os/path.go correctly handles the expansion of home directory paths by checking if the path starts with ~, ~/, or ~\. It retrieves the current user's home directory and joins it with the rest of the path. The function also handles errors in user retrieval and cleans the resulting path before returning it. This implementation appears robust for the intended purpose.

  • The function handles basic cases for home directory expansion.
  • It includes error handling for user retrieval failures.

No further issues were found with this implementation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the behavior of `zetaos.ExpandHomeDir`.

# Test: Search for the function implementation. Expect: Correct handling of home directory paths.
ast-grep --lang go --pattern $'func ExpandHomeDir($_) ($_, $_) {
  $$$
}' | rg --context 5 'ExpandHomeDir'

Length of output: 387


Script:

#!/bin/bash
# Extract the full implementation of the ExpandHomeDir function to verify its behavior.
ast-grep --lang go --pattern $'func ExpandHomeDir($_) ($_, $_) {
  $$$
}' --output

Length of output: 283


Script:

#!/bin/bash
# Correctly extract the full implementation of the ExpandHomeDir function for analysis.
ast-grep --lang go --pattern $'func ExpandHomeDir($_) ($_, $_) {
  $$$
}'

Length of output: 580

pkg/os/path_test.go (2)

14-49: Comprehensive tests for home directory resolution.

The test cases for ExpandHomeDir are well-structured and cover various scenarios, ensuring robust validation of the functionality.


51-83: Effective tests for file existence checks.

The test cases for FileExists effectively verify the function's behavior for both existing and non-existing files.

rpc/namespaces/ethereum/debug/trace.go (1)

42-42: Ensure consistent behavior of zetaos.ExpandHomeDir.

The ExpandHomeDir function from the zetaos package is used here. Verify that it handles path expansion consistently across different scenarios.

Verification successful

Consistent Behavior of zetaos.ExpandHomeDir Verified

The ExpandHomeDir function consistently handles the expansion of home directory paths. It checks for common prefixes (~, ~/, ~\) and uses user.Current() to retrieve the home directory. Errors are handled gracefully, and the path is cleaned before returning. No issues found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the behavior of `zetaos.ExpandHomeDir` in the context of `StartGoTrace`.

# Test: Search for the function implementation. Expect: Consistent handling of home directory paths.
ast-grep --lang go --pattern $'func ExpandHomeDir($_) ($_, $_) {
  $$$
}' | rg --context 5 'ExpandHomeDir'

Length of output: 387


Script:

#!/bin/bash
# Description: Retrieve the full implementation of `ExpandHomeDir` to verify its behavior.
ast-grep --lang go --pattern 'func ExpandHomeDir($_) ($_, $_) {
  $$$
}'

Length of output: 580

pkg/crypto/aes256_gcm.go (1)

14-30: Ensure secure key management practices.

The EncryptAES256GCMBase64 function handles encryption keys as strings, which can be insecure if not managed properly. Ensure that keys are stored and transmitted securely, possibly using environment variables or secure vaults.

zetaclient/keys/relayer_keys.go (1)

21-24: Ensure sensitive data is protected.

The RelayerKey struct contains a private key. Ensure that this data is handled securely, especially when logged or transmitted.

Verification successful

Ensure Secure Handling of RelayerKey in ReadRelayerKeyFromFile Function

The ReadRelayerKeyFromFile function returns a RelayerKey, which contains sensitive data. While there are no direct logging or transmission concerns found, ensure that:

  • The private key is not inadvertently exposed in logs or error messages.
  • Access to the file containing the key is properly secured.
  • Consider encrypting the key at rest and in transit, if applicable.

These precautions will help protect sensitive data effectively.

  • Location to Review:
    • zetaclient/keys/relayer_keys.go:67-95
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if there are any logging practices that might expose sensitive data.

# Test: Search for any logging statements involving `RelayerKey`.
rg --type go 'log\..*\(.*RelayerKey.*\)'

Length of output: 42


Script:

#!/bin/bash
# Description: Check for any error handling or transmission of `RelayerKey`.

# Test: Search for any error handling involving `RelayerKey`.
rg --type go 'errors\..*\(.*RelayerKey.*\)'

# Test: Search for any instances where `RelayerKey` might be transmitted over networks.
rg --type go 'http\..*\(.*RelayerKey.*\)'

Length of output: 190


Script:

#!/bin/bash
# Description: Check for functions returning `RelayerKey` and ensure they handle the private key securely.

# Test: Search for functions that return `RelayerKey`.
ast-grep --lang go --pattern 'func $_($_) (RelayerKey, $_) { $$$ }'

Length of output: 1949

zetaclient/chains/solana/signer/withdraw.go (1)

Line range hint 50-68:
Ensure correct usage of the relayer key.

The change to use signer.relayerKey instead of signer.solanaFeePayerKey is correct given the context. Ensure that this change is reflected consistently across the codebase where this function is used.

Verification successful

Verified: Correct usage of signer.relayerKey in withdraw.go.

The change to use signer.relayerKey is correctly applied within the withdraw.go file, and no inconsistencies were found in other parts of the codebase. This indicates that the update is well-contained and appropriate for its intended purpose.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify usage of `signer.relayerKey` across the codebase.

# Test: Search for usage of `signer.relayerKey`. Expect: Consistent usage in the context of transaction signing.
rg --type go 'signer\.relayerKey'

Length of output: 109

cmd/zetaclientd/init.go (3)

39-39: Rename field to RelayerKeyPath.

The renaming of SolanaKey to RelayerKeyPath accurately reflects the new focus on relayer key management. Ensure that this change is consistently applied throughout the codebase.


112-112: Assign RelayerKeyPath in configuration.

The assignment of RelayerKeyPath to configData is appropriate. Ensure that this path is correctly utilized in the application logic where relayer keys are required.


73-74: Update command-line flag for relayer keys.

The new command-line flag --relayer-key-path is correctly introduced. Verify that this flag is documented in any user-facing documentation and that it is used correctly in all relevant scripts and configurations.

contrib/localnet/scripts/start-zetaclientd.sh (2)

82-82: Create relayer key file for Solana.

The call to create_relayer_key_file for Solana is appropriate. Ensure that the key file is correctly used in subsequent operations and that any errors during creation are handled gracefully.


105-105: Create relayer key file for Solana in non-zero hosts.

The logic to create the relayer key file for non-zero hosts is appropriate. Ensure that this logic is consistent with the overall initialization process and that any errors are logged and handled.

zetaclient/config/types.go (1)

162-166: Ensure consistent use of mutex for thread safety.

The GetRelayerKeyPath method correctly uses a read lock for thread safety, which is consistent with the other methods in the Config struct.

pkg/crypto/aes256_gcm_test.go (1)

1-187: Comprehensive test coverage for AES-256-GCM functions.

The test cases cover a wide range of scenarios, ensuring robustness of the encryption and decryption functions. The use of table-driven tests and require assertions is commendable.

zetaclient/chains/solana/signer/signer.go (2)

32-33: Update reflects new role of the relayer key.

The renaming of solanaFeePayerKey to relayerKey accurately reflects its new purpose, improving code clarity.


Line range hint 48-74:
Ensure correct handling of relayer key in NewSigner.

The NewSigner function correctly constructs the Solana private key from the relayerKey. Ensure that all dependent code is updated to handle keys.RelayerKey appropriately.

Verification successful

Verification of NewSigner Handling of keys.RelayerKey: Successful

The NewSigner function for the Solana signer is correctly using keys.RelayerKey as intended in the bootstrap.go file. The implementation aligns with the review comment's requirements, ensuring that the relayer key is handled appropriately. No further updates are necessary for this aspect of the code.

  • Location Confirmed: zetaclient/orchestrator/bootstrap.go uses solanasigner.NewSigner with relayerKey.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `NewSigner` are updated to use `keys.RelayerKey`.

# Test: Search for `NewSigner` usage. Expect: Only occurrences with `keys.RelayerKey`.
rg --type go -A 3 $'NewSigner'

Length of output: 5337

pkg/chains/chain.go (1)

150-154: Ensure map access safety.

The GetNetworkName function uses a map to retrieve network names. Ensure that the map is initialized and populated correctly elsewhere in the codebase.

Check that Network_name is properly initialized and populated.

Verification successful

Map Initialization and Population Verified

The Network_name map is properly initialized and populated in pkg/chains/chains.pb.go. This ensures safe access in the GetNetworkName function. No further action is required.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `Network_name` is initialized and populated correctly.

# Test: Search for the initialization and population of `Network_name`. Expect: Valid map initialization and population.
rg --type go --context 5 'Network_name\s*=\s*map\[int32\]string'

Length of output: 520

contrib/localnet/orchestrator/start-zetae2e.sh (2)

47-92: Suppressing command output improves script cleanliness.

Redirecting output to /dev/null is a good practice for reducing unnecessary console output. Ensure that important logs are not suppressed.


94-104: Securely handle Solana account funding.

The script funds Solana relayer accounts. Ensure that sensitive information, such as private keys, is not exposed in the script or logs.

Check that no sensitive information is exposed in the configuration or logs.

zetaclient/orchestrator/bootstrap.go (1)

166-169: Change to relayer key loading approved.

The switch from loading a Solana private key to a relayer key enhances the security and flexibility of key management. Ensure that the LoadRelayerKey function is correctly implemented and used throughout the codebase.

rpc/namespaces/ethereum/debug/api.go (1)

203-203: Integration with Zeta Chain utilities approved.

The use of zetaos.ExpandHomeDir enhances compatibility with Zeta Chain's ecosystem. Ensure that this function is consistently used where applicable.

Verification successful

Consistent Usage of zetaos.ExpandHomeDir Verified

The function zetaos.ExpandHomeDir is consistently used across various files, ensuring compatibility with Zeta Chain's ecosystem. This integration appears to be implemented as intended.

  • Files with usage:
    • zetaclient/keys/relayer_keys.go
    • server/start.go
    • rpc/namespaces/ethereum/debug/trace.go
    • rpc/namespaces/ethereum/debug/utils.go
    • rpc/namespaces/ethereum/debug/api.go
    • pkg/os/path_test.go
    • cmd/zetaclientd/import_relayer_keys.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `zetaos.ExpandHomeDir` in the codebase.

# Test: Search for the function usage. Expect: Consistent usage across all relevant files.
rg --type go 'zetaos.ExpandHomeDir'

Length of output: 693

pkg/chains/chain_test.go (2)

399-405: New test TestGetNetworkName approved.

This test effectively validates the GetNetworkName function, enhancing test coverage for network identification.


411-411: Modification in TestGetChainFromChainID approved.

The change to focus on the found boolean improves the clarity and intent of the test.

e2e/config/config.go (3)

44-50: Integration of ObserverRelayerAccounts field.

The addition of the ObserverRelayerAccounts field in the Config struct is well-integrated and enhances multi-chain account management. Ensure that this field is documented in any relevant configuration documentation.


58-59: Integration of Solana fields in Account struct.

The SolanaAddress and SolanaPrivateKey fields are well-integrated into the Account struct, supporting Solana account management. Consider adding validation for these fields similar to existing EVM address validation.


81-85: Design of ObserverRelayerAccounts struct.

The ObserverRelayerAccounts struct is well-designed for managing observer relayer accounts. Ensure that its usage is consistent across the codebase to maintain clarity and organization.

server/start.go (1)

340-342: Refactoring to use zetaos.ExpandHomeDir.

The refactoring to use zetaos.ExpandHomeDir instead of ethdebug.ExpandHome aligns with the architectural direction of consolidating functionality under the zetaos package. Verify that this change maintains the intended functionality and does not introduce regressions.

cmd/zetaclientd/import_relayer_keys.go Show resolved Hide resolved
cmd/zetaclientd/import_relayer_keys.go Outdated Show resolved Hide resolved
cmd/zetaclientd/import_relayer_keys.go Outdated Show resolved Hide resolved
pkg/crypto/aes256_gcm.go Outdated Show resolved Hide resolved
pkg/crypto/aes256_gcm.go Outdated Show resolved Hide resolved
zetaclient/keys/relayer_keys.go Outdated Show resolved Hide resolved
zetaclient/keys/relayer_keys.go Outdated Show resolved Hide resolved
contrib/localnet/scripts/start-zetaclientd.sh Outdated Show resolved Hide resolved
contrib/localnet/scripts/start-zetaclientd.sh Show resolved Hide resolved
cmd/zetae2e/config/localnet.yml Outdated Show resolved Hide resolved
pkg/os/console.go Fixed Show resolved Hide resolved
Copy link
Contributor

@swift1337 swift1337 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a first review round. Can you add a description with the context & purpose of these changes?

pkg/os/console.go Outdated Show resolved Hide resolved
e2e/config/config.go Outdated Show resolved Hide resolved
zetaclient/context/app.go Outdated Show resolved Hide resolved
zetaclient/context/app.go Outdated Show resolved Hide resolved
zetaclient/context/app.go Outdated Show resolved Hide resolved
zetaclient/orchestrator/bootstrap.go Outdated Show resolved Hide resolved
@ws4charlie ws4charlie changed the title WIP(feat): Solana relayer (fee payer) key importer, encryption and decryption feat: Solana relayer (fee payer) key importer, encryption and decryption Aug 13, 2024
Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 79.71698% with 43 lines in your changes missing coverage. Please review.

Project coverage is 71.25%. Comparing base (7039dcc) to head (c9f0b42).
Report is 1 commits behind head on develop.

Files Patch % Lines
pkg/crypto/aes256_gcm.go 76.92% 6 Missing and 6 partials ⚠️
zetaclient/keys/relayer_key.go 91.17% 3 Missing and 3 partials ⚠️
zetaclient/chains/solana/signer/signer.go 81.48% 4 Missing and 1 partial ⚠️
zetaclient/orchestrator/bootstrap.go 44.44% 5 Missing ⚠️
pkg/os/console.go 78.94% 2 Missing and 2 partials ⚠️
zetaclient/config/types.go 0.00% 4 Missing ⚠️
zetaclient/context/chain.go 62.50% 3 Missing ⚠️
pkg/os/path.go 85.71% 1 Missing and 1 partial ⚠️
zetaclient/chains/solana/signer/withdraw.go 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2673      +/-   ##
===========================================
+ Coverage    71.02%   71.25%   +0.22%     
===========================================
  Files          346      351       +5     
  Lines        18807    18982     +175     
===========================================
+ Hits         13358    13525     +167     
+ Misses        4858     4853       -5     
- Partials       591      604      +13     
Files Coverage Δ
pkg/crypto/privkey.go 100.00% <100.00%> (ø)
zetaclient/context/app.go 74.80% <100.00%> (ø)
zetaclient/metrics/metrics.go 76.74% <ø> (ø)
pkg/os/path.go 85.71% <85.71%> (ø)
zetaclient/chains/solana/signer/withdraw.go 0.00% <0.00%> (ø)
zetaclient/context/chain.go 76.82% <62.50%> (-2.66%) ⬇️
pkg/os/console.go 78.94% <78.94%> (ø)
zetaclient/config/types.go 51.21% <0.00%> (+9.21%) ⬆️
zetaclient/chains/solana/signer/signer.go 30.33% <81.48%> (+30.33%) ⬆️
zetaclient/orchestrator/bootstrap.go 58.03% <44.44%> (+1.08%) ⬆️
... and 2 more

@ws4charlie
Copy link
Contributor Author

Did a first review round. Can you add a description with the context & purpose of these changes?

I updated the description of the PR. The goal is to formalize the usage of relayer key.

@ws4charlie ws4charlie marked this pull request as ready for review August 13, 2024 05:26
@gartnera gartnera added the SOLANA_TESTS Run make start-solana-test label Aug 13, 2024
pkg/chains/chain.go Outdated Show resolved Hide resolved
pkg/os/console.go Show resolved Hide resolved
zetaclient/config/types.go Show resolved Hide resolved
zetaclient/chains/solana/signer/signer.go Outdated Show resolved Hide resolved
zetaclient/testutils/mocks/solana_rpc.go Show resolved Hide resolved
Copy link

!!!WARNING!!!
nosec detected in the following files: zetaclient/keys/relayer_key.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

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
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

@github-actions github-actions bot added the nosec label Aug 14, 2024
pkg/os/console.go Dismissed Show resolved Hide resolved
@lumtis lumtis added the UPGRADE_TESTS Run make start-upgrade-tests label Aug 15, 2024
cmd/zetae2e/config/localnet.yml Show resolved Hide resolved
zetaclient/context/app.go Outdated Show resolved Hide resolved
@ws4charlie ws4charlie added this pull request to the merge queue Aug 15, 2024
Merged via the queue into develop with commit 2093314 Aug 15, 2024
27 of 28 checks passed
@ws4charlie ws4charlie deleted the feat-solana-fee-payer-key branch August 15, 2024 22:17
gartnera pushed a commit that referenced this pull request Aug 15, 2024
…ion (#2673)

* configure observer relayer key for Solana; remove hardcoded solana test key from zetaclient code

* implementation of relayer key importer, encryption and decryption

* integrate relayer key into E2E and Solana signer

* add relayer_key_balance metrics and unit tests

* use TrimSpace to trim password

* add changelog entry

* use relayer account array in E2E config; a few renaming; add private key validation when importing

* fix linter

* remove GetNetworkName method for simplification

* added PromptPassword method to prompt single password

* use network name as map index to store relayer key passwords

* moved relayer passwords to chain registry

* airdrop SOL token only if solana local node is available

---------

Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>
gartnera added a commit that referenced this pull request Aug 16, 2024
* feat: parse inscription like witness data (#2524)

* parse inscription like witness data

* more comment

* remove unused code

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>

* 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 <11892559+swift1337@users.noreply.github.com>

* 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 <me@fbac.dev>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* update review feedback

* update code commnet

* update comment

* more comments

* Update changelog.md

---------

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

fix version

* 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 <11892559+swift1337@users.noreply.github.com>

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>

* 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 <11892559+swift1337@users.noreply.github.com>

* 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 <11892559+swift1337@users.noreply.github.com>

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>

* Update zetaclient/chains/bitcoin/tx_script.go

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>

* 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 <11892559+swift1337@users.noreply.github.com>

* 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 <me@fbac.dev>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* Update zetaclient/chains/bitcoin/tokenizer.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* 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 <me@fbac.dev>

* Update zetaclient/chains/bitcoin/observer/inbound.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* clean up

* format code

---------

Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

* 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

tmp

* feat(zetaclient): add generic rpc metrics (#2597)

* feat(zetaclient): add generic rpc metrics

* feedback

* changelog

* fmt

* fix(zetaclient): use name in pending tx metric (#2642)

* feat(pkg): add `ticker` package (#2617)

* Add `pkg/ticker`

* Sample ticker usage in evm observer

* Change naming

* Address PR comments

* Address PR comments

* feat(zetaclient)!: Add support for EIP-1559 gas fees (#2634)

* Add Gas struct

* Add EIP-1559 fees

* Update changelog

* Add test cases for legacy vs dynamicFee txs

* Fix typo; Add E2E coverage

* Address PR comments

* Address PR comments

* Use gasFeeCap formula

* Revert "Use gasFeeCap formula"

This reverts commit 2260925.

* Address PR comments

* Fix e2e upgrade tests

* fix: adjust evm outbound tracker reporter to avoid submitting invalid hashes (#2628)

* refactor and fix evm outbound tracker reporter to avoid invalid hashes; print log when outbound tracker is full of invalid hashes

* add changelog entry

* used predefined log fields

* remove repeated fields information from log message; Devops team would configure Datadog to show the fields

* remove redundant fields in log message; unified logs

* remove pending transaction map from observer; the outbound tracker reporter will no longer report pending hash

* use bg.Work() to launch outbound tracker reporter goroutines

* bring the checking EnsureNoTrackers() back

* add more rationale to EVM outbound tracker submission

* sync observer and signers without wait on startup

* try fixing tss migration E2E failure by increase timeout

* feat: Solana relayer (fee payer) key importer, encryption and decryption (#2673)

* configure observer relayer key for Solana; remove hardcoded solana test key from zetaclient code

* implementation of relayer key importer, encryption and decryption

* integrate relayer key into E2E and Solana signer

* add relayer_key_balance metrics and unit tests

* use TrimSpace to trim password

* add changelog entry

* use relayer account array in E2E config; a few renaming; add private key validation when importing

* fix linter

* remove GetNetworkName method for simplification

* added PromptPassword method to prompt single password

* use network name as map index to store relayer key passwords

* moved relayer passwords to chain registry

* airdrop SOL token only if solana local node is available

---------

Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

* ci: Set Docker Workflow to use Go 1.22 (#2722)

* Set go 1.22.2

* Set go 1.22.2

* Set go 1.22

* Set go 1.22

* Refactor contrib/rpc and contrib/docker-scripts to use snapshots API (#2724)

Co-authored-by: Julian Rubino <julian@zetachain.com>

---------

Co-authored-by: dev-bitSmiley <153714963+bitSmiley@users.noreply.github.com>
Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>
Co-authored-by: Charlie Chen <34498985+ws4charlie@users.noreply.github.com>
Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>
Co-authored-by: Charlie <31941002+CharlieMc0@users.noreply.github.com>
Co-authored-by: Julian Rubino <julian.rubino0@gmail.com>
Co-authored-by: Julian Rubino <julian@zetachain.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking:cli nosec SOLANA_TESTS Run make start-solana-test UPGRADE_TESTS Run make start-upgrade-tests
Projects
None yet
5 participants