test: add deployment of connector V2 to e2e test #3975
Conversation
kingpinXD
changed the title
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes introduce deployment and initialization of the Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant EVM
participant ZetaConnectorNative
participant Proxy
TestRunner->>EVM: Deploy ZetaConnectorNative implementation
EVM-->>TestRunner: Return deployment receipt & ABI
TestRunner->>EVM: Encode initializer data (gateway, ZetaETH, TSS, account)
TestRunner->>EVM: Deploy ERC1967 proxy with initializer data
EVM-->>TestRunner: Return proxy deployment receipt
TestRunner->>Proxy: Bind contract instance at proxy address
TestRunner->>TestRunner: Store ConnectorNativeAddr and ConnectorNative instance
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
go.mod (1)
260-260: Rungo mod tidyafter promotinggjsonto a direct dependency
github.com/tidwall/gjson v1.14.4moved from indirect → direct.
To keepgo.summinimal and reproducible, please run:go mod tidyand commit any resulting diff.
x/fungible/keeper/evm_gateway.go (1)
239-243: Verify thatdepositAndRevert0exists in the deployed Gateway contractThe selector changed from
depositAndRevert➜depositAndRevert0. If the on-chain Gateway you invoke does not expose the suffixed version, every revert workflow will fail at the EVM level.
- Ensure the new protocol-contracts tag truly renames the method.
- Make sure existing migrations redeploy Gateway with the new ABI before this code reaches production.
Optionally extract method names into typed constants to avoid silent typos:
- "depositAndRevert0", + gatewayMethodDepositAndRevert0,where
const gatewayMethodDepositAndRevert0 = "depositAndRevert0".changelog.md (1)
22-24: Header wording / spacing inconsistent with the rest of the changelogOther sections use
### Tests; this entry introduces### Test(singular, trailing space).
For consistency and better Markdown rendering, consider:-### Test +### Testse2e/config/config.go (1)
169-178: AddConnectorNativeAddrto default config (and docs) for completenessThe new field is missing from
DefaultConfig()and therefore from the auto-generatedcontracts.tomlproduced byWriteConfig.
Including a placeholder (empty string is fine) avoids silent""YAML drops and helps users discover the option.@@ EVM: EVM{ ERC20: "0xff3135df4F2775f4091b81f4c7B6359CfA07862a", + ConnectorNativeAddr: "", },e2e/runner/runner.go (1)
32-35: Suspicious triple-n alias ‑ likely a typo
zetaconnnectornative(3 × “n”) is easy to mis-type and inconsistent with the package namezetaconnectornative.
While legal, it reduces greppability and deviates from the aliasing style used elsewhere (zetaconnectoreth,connectorzevm, …).-import zetaconnnectornative "github.com/zeta-chain/protocol-contracts/pkg/zetaconnectornative.sol" +import zetaconnectornative "github.com/zeta-chain/protocol-contracts/pkg/zetaconnectornative.sol"Remember to update the references below (
zetaconnnectornative.ZetaConnectorNative→zetaconnectornative.ZetaConnectorNative).cmd/zetae2e/config/contracts.go (1)
170-178: Happy path looks good – consider early-return guardThe block cleanly instantiates the native connector when present.
Optionally, add a short-circuitif conf.Contracts.EVM.ConnectorNativeAddr == "" {}early in the function (similar to other setters) to keep the main path lean; not blocking.e2e/runner/setup_evm.go (2)
12-13: Alias typo hinders readability
zetaconnnectornative(three “n”) is easy to mis-type later. Prefer a deliberate alias, e.g.connectornative, to reduce cognitive load.
121-154: Verbose variable names & duplicate “n” – propose cleanupThe new block deploys the native connector but introduces highly repetitive names and a triple-
nspelling that propagates through every identifier.- zetaConnnectorNativeAddress, txZetaConnnectorNativeHash, _, err := zetaconnnectornative.DeployZetaConnectorNative( + connectorNativeImpl, txConnectorNative, _, err := connectornative.DeployZetaConnectorNative(…and downstream:
- zetaConnnectorNativeABI, err := zetaconnnectornative.ZetaConnectorNativeMetaData.GetAbi() + connectorNativeABI, err := connectornative.ZetaConnectorNativeMetaData.GetAbi()Adopting concise names (
connectorNativeImpl,connectorNativeProxy) makes the section far easier to scan without altering functionality.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
changelog.md(1 hunks)cmd/zetae2e/config/contracts.go(2 hunks)cmd/zetae2e/local/local.go(4 hunks)contrib/rpctest/main.go(1 hunks)e2e/config/config.go(1 hunks)e2e/e2etests/e2etests.go(0 hunks)e2e/runner/legacy_setup_evm.go(1 hunks)e2e/runner/runner.go(2 hunks)e2e/runner/setup_evm.go(2 hunks)go.mod(2 hunks)x/crosschain/keeper/cctx_orchestrator_validate_outbound.go(1 hunks)x/fungible/keeper/evm_gateway.go(1 hunks)
💤 Files with no reviewable changes (1)
- e2e/e2etests/e2etests.go
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
contrib/rpctest/main.gox/fungible/keeper/evm_gateway.gox/crosschain/keeper/cctx_orchestrator_validate_outbound.goe2e/runner/legacy_setup_evm.goe2e/runner/runner.goe2e/config/config.gocmd/zetae2e/config/contracts.goe2e/runner/setup_evm.gocmd/zetae2e/local/local.go
🧬 Code Graph Analysis (1)
cmd/zetae2e/config/contracts.go (1)
e2e/config/config.go (2)
Contracts(128-134)EVM(168-178)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: build-zetanode
- GitHub Check: test-sim-after-import / sim
- GitHub Check: test-sim-import-export / sim
- GitHub Check: test-sim-fullappsimulation / sim
- GitHub Check: test-sim-nondeterminism / sim
- GitHub Check: lint
- GitHub Check: gosec
- GitHub Check: build-and-test
- GitHub Check: analyze (go)
- GitHub Check: build
🔇 Additional comments (7)
go.mod (1)
53-54: Confirm contract-interface compatibility after theprotocol-contractsupgradeThe jump to
v1.0.2-athens3.0.20250603154140-a2b6c07814edlikely carries new ABIs (e.g. the recently-addeddepositAndRevert0/depositAndCall0entry points).
Please compile the full node, run the E2E suite, and double-check that:
- All keeper calls (
CallDepositAndCallZRC20,CallDepositAndRevert, …) map to existing functions in the generated Go bindings.- Event topics used elsewhere remain unchanged.
Failing to do so will surface only at runtime with cryptic “function selector not found” reverts.
contrib/rpctest/main.go (1)
20-21: Alias removal looks goodDropping the explicit
zetaethalias keeps the reference identifier unchanged (package name is alreadyzetaeth), so compilation behaviour is identical while reducing noise.e2e/runner/legacy_setup_evm.go (1)
11-12: Alias removal is fine – nothing to flag.
The import path change compiles and improves consistency with the rest of the codebase.x/crosschain/keeper/cctx_orchestrator_validate_outbound.go (1)
240-241: Good catch – revert error is now propagated.Surfacing the concrete
errinstead ofnilgreatly improves observability when debugging failed V2 reverts.cmd/zetae2e/local/local.go (2)
58-59: Flag constant looks good
The addition offlagTestV2ConnectorMigrationis consistent with the existing naming pattern.
102-103: Flag is registered correctly
The flag is wired into Cobra with a precise help string.e2e/runner/setup_evm.go (1)
170-172: Missing receipt check after uncommentingSetConnectorWhen the call above is restored, add:
ensureTxReceipt(txSetConnector, "Set connector in Gateway failed")
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3975 +/- ##
===========================================
+ Coverage 64.74% 64.82% +0.08%
===========================================
Files 474 474
Lines 34809 34809
===========================================
+ Hits 22536 22566 +30
+ Misses 11223 11192 -31
- Partials 1050 1051 +1
🚀 New features to boost your workflow:
|
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
4 participants
Description
Closes #3974
This is the intial PR for a set of PR to migrate to V2 Connector contracts
How Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores