test(e2e): solana upgrade contract e2e test
#3536
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 📝 WalkthroughWalkthroughThis pull request introduces a new Makefile target to initiate an end-to-end test for upgrading Solana contracts. It adds and updates several Docker Compose configurations, Dockerfile modifications, and shell scripts to deploy and upgrade Solana programs in a local network environment. Additionally, the changes enhance the testing framework by adding a new verification method and assertion utilities in the test packages, and update the changelog with new features and integrations related to contract upgrades. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant Docker
participant SolanaScript
participant E2ERunner
participant TestUtil
User->>Makefile: Run target "start-e2e-test-upgrade-contracts"
Makefile->>Docker: Execute docker-compose with upgrade configuration
Docker->>SolanaScript: Launch container with entrypoint "start-solana-upgrade.sh"
SolanaScript->>SolanaScript: Generate keypair, start validator, perform airdrops, deploy programs, and upgrade gateway program
Note over SolanaScript: Upgrade contracts process completes
E2ERunner->>E2ERunner: Invoke VerifySolanaContractsUpgrade()
E2ERunner->>TestUtil: Use "requireTrue" for assertion check
TestUtil-->>E2ERunner: Return assertion result
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
|
kingpinXD
changed the title
e2e : solana upgrade contract e2e testtest : solana upgrade contract e2e test
kingpinXD
changed the title
test : solana upgrade contract e2e testtest: solana upgrade contract e2e test
kingpinXD
changed the title
test: solana upgrade contract e2e teste2e): solana upgrade contract e2e test
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3536 +/- ##
========================================
Coverage 64.80% 64.80%
========================================
Files 462 462
Lines 31934 31934
========================================
Hits 20694 20694
Misses 10332 10332
Partials 908 908 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
cmd/zetae2e/local/local.go (1)
291-294: Add error context to contract upgrade verification.While the verification is correct, adding error context would help with debugging failures.
if upgradeContracts { deployerRunner.UpgradeGatewaysAndERC20Custody() - requireTrue(deployerRunner.VerifySolanaContractsUpgrade()) + requireTrue(deployerRunner.VerifySolanaContractsUpgrade(), "Solana contract upgrade verification failed") }contrib/localnet/solana/start-solana-upgrade.sh (1)
1-18: Consider extracting common setup logic.The initial setup logic is duplicated from
start-solana.sh. Consider extracting common setup into a shared script.Create a new file
setup-common.sh:#!/bin/bash setup_solana() { echo "making an id" solana-keygen new -o /root/.config/solana/id.json --no-bip39-passphrase solana config set --url localhost echo "starting solana test validator..." solana-test-validator & sleep 5 # airdrop to e2e sol account solana airdrop 1000 solana airdrop 1000 37yGiHAnLvWZUNVwu9esp74YQFqxU1qHCbABkDvRddUQ }Makefile (1)
277-281: Clarify the echo message for the new upgrade test target.
The target correctly sets the required environment variable and invokes Docker Compose with the proper profile and Compose file. However, the echo message output as"--> Starting e2e test"is generic and may be ambiguous given the number of similar targets. It would be more informative to modify it to explicitly reference the contract upgrade functionality (for example,"--> Starting e2e test for upgrading Solana gateway contracts").
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
contrib/localnet/scripts/gateway-upgrade.sois excluded by!**/*.socontrib/localnet/solana/gateway.sois excluded by!**/*.socontrib/localnet/solana/gateway_upgrade.sois excluded by!**/*.so
📒 Files selected for processing (11)
Makefile(1 hunks)changelog.md(1 hunks)cmd/zetae2e/local/local.go(2 hunks)contrib/localnet/docker-compose-upgrade-contracts.yml(1 hunks)contrib/localnet/solana/Dockerfile(1 hunks)contrib/localnet/solana/start-solana-upgrade.sh(1 hunks)contrib/localnet/solana/start-solana.sh(1 hunks)e2e/runner/setup_solana.go(1 hunks)e2e/runner/verify.go(1 hunks)pkg/contracts/solana/pda.go(1 hunks)testutil/helpers.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- contrib/localnet/docker-compose-upgrade-contracts.yml
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.go`: Review the Go code, point out issues relative to ...
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
testutil/helpers.goe2e/runner/verify.gocmd/zetae2e/local/local.goe2e/runner/setup_solana.gopkg/contracts/solana/pda.go
`**/*.sh`: Review the shell scripts, point out issues relati...
**/*.sh: Review the shell scripts, point out issues relative to security, performance, and maintainability.
contrib/localnet/solana/start-solana.shcontrib/localnet/solana/start-solana-upgrade.sh
🔇 Additional comments (6)
e2e/runner/verify.go (1)
56-57: LGTM!The variable renaming improves code clarity and maintains consistent naming conventions.
Also applies to: 60-60
cmd/zetae2e/local/local.go (2)
58-61: LGTM! Clean variable declarations.The addition of
requireTruefollows the existing pattern and improves code readability.
50-50: LGTM! Well-documented flag addition.The
flagUpgradeContractsfollows the established naming convention and its purpose is clear.contrib/localnet/solana/Dockerfile (1)
5-14: LGTM! Clean Dockerfile modifications.The changes properly handle script permissions and maintain a consistent structure with existing patterns.
contrib/localnet/solana/start-solana.sh (1)
14-18: Consider conditional program deployment.As per previous discussions, deploying programs immediately might prevent testing with old contracts. Consider making the deployment conditional based on a flag.
changelog.md (1)
32-35: New Changelog Entry for Solana Gateway Upgrade E2E Test.
The newly added entry for PR 3536 clearly documents the addition of an e2e test for upgrading the Solana gateway program. Ensure that this entry remains consistent with the descriptions in other related sections (such as the Makefile target) and consider emphasizing that this test enhances the reliability of the upgrade workflow.
Description
Related PR from protocol-contracts-solana
zeta-chain/protocol-contracts-solana#81
Refer this issue for more context : #3535
Closes : #3443
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Tests