-
Notifications
You must be signed in to change notification settings - Fork 108
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
test: fix local zetae2e run solana tests and balances #3217
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 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3217 +/- ##
===========================================
- Coverage 62.74% 62.06% -0.69%
===========================================
Files 425 428 +3
Lines 30231 30438 +207
===========================================
- Hits 18969 18890 -79
- Misses 10425 10708 +283
- Partials 837 840 +3 |
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: 4
🧹 Outside diff range and nitpick comments (2)
cmd/zetae2e/config/local.yml (1)
Line range hint
31-31
: Avoid reusing credentials across different accountsThe same Solana private key is used in both
default_account
anduser_solana
. For testing clarity and to better simulate production scenarios, consider using unique credentials for each account.🧰 Tools
🪛 Gitleaks (8.21.2)
5-5: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
e2e/runner/balances.go (1)
101-109
: Enhance error handling and use constants for commitmentThe error handling could be more specific, and the commitment level should be defined as a constant.
Consider these improvements:
+const defaultCommitment = rpc.CommitmentFinalized + if r.Account.SolanaAddress != "" && r.Account.SolanaPrivateKey != "" { solSOLBalance, err := r.SolanaClient.GetBalance( r.Ctx, solana.MustPublicKeyFromBase58(r.Account.SolanaAddress.String()), - rpc.CommitmentFinalized, + defaultCommitment, ) if err != nil { - return AccountBalances{}, err + return AccountBalances{}, fmt.Errorf("failed to get SOL balance: %w", err) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
cmd/zetae2e/config/local.yml
(3 hunks)e2e/runner/balances.go
(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
e2e/runner/balances.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🪛 Gitleaks (8.21.2)
cmd/zetae2e/config/local.yml
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (2)
cmd/zetae2e/config/local.yml (1)
111-111
: Verify contract addresses across environments
The contract addresses have been updated to support Solana integration. Please ensure these addresses are:
- Correctly deployed and verified
- Consistent with other test environments
- Compatible with the local test network configuration
Also applies to: 120-121
✅ Verification successful
The previous attempt to verify contract deployment status failed as it required a running local node. Let's instead analyze the configuration files and deployment scripts to understand the contract address usage and verification process across environments.
The previous search was too broad. Let's focus specifically on the configuration files and deployment scripts that might reference these contract addresses to verify their consistency across environments.
Contract addresses are consistently configured across local and example environments
The verification shows that:
- The
erc20_zrc20
address is identical in both local and example configurations - The new
sol_zrc20
andspl_zrc20
addresses are properly added to the local configuration - All addresses follow the correct EVM address format (40 hex characters with 0x prefix)
The contract addresses are properly configured and consistent with the test environment setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify contract addresses are properly deployed
# Check if contracts are deployed and accessible
for addr in "0x0cbe0dF132a6c6B4a2974Fa1b7Fb953CF0Cc798a" "0x48f80608B672DC30DC7e3dbBd0343c5F02C738Eb" "0x7c8dDa80bbBE1254a7aACf3219EBe1481c6E01d7"; do
curl -X POST -H "Content-Type: application/json" --data "{\"jsonrpc\":\"2.0\",\"method\":\"eth_getCode\",\"params\":[\"$addr\", \"latest\"],\"id\":1}" http://localhost:9545
done
Length of output: 2255
Script:
#!/bin/bash
# Let's check for contract addresses in configuration files across the repository
rg -g '*.{yml,yaml,json}' -i '0x[a-fA-F0-9]{40}' -B 2 -A 2
# Also check for any contract deployment or verification scripts
fd -e js -e ts -e sh "deploy|verify" --exec head -n 20 {}
Length of output: 131676
Script:
#!/bin/bash
# Let's check for contract addresses in configuration files
rg -g '*.{yml,yaml}' -B 2 -A 2 'erc20_zrc20|sol_zrc20|spl_zrc20'
# Also check for any deployment or verification scripts that might contain contract addresses
fd -e js -e ts -e sh "deploy|verify" --type f --exec grep -l "0x0cbe0dF132a6c6B4a2974Fa1b7Fb953CF0Cc798a" {} \;
Length of output: 1596
e2e/runner/balances.go (1)
10-11
: LGTM: Clean integration of Solana support
The new imports and struct fields maintain consistency with the existing codebase patterns.
Also applies to: 30-31
!!!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 |
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.
LGTM
Description
Enabling
zetae2e run
andzetae2e balances
to work with solana.Currently tested on solana tests locally like this:
How Has This Been Tested?
Summary by CodeRabbit
New Features
solana_address
andsolana_private_key
.Bug Fixes
Documentation