feat: migrate interop-b tests to Anvil multichain harness#2108
feat: migrate interop-b tests to Anvil multichain harness#2108valera-grinenko-ai wants to merge 28 commits intomatter-labs:draft-v31from
Conversation
b4dda4c to
95aadd0
Compare
230437a to
7fd660c
Compare
| */ | ||
| // prettier-ignore | ||
| const DUMMY_INTEROP_RECIPIENT_BYTECODE = | ||
| "0x6080604052348015600e575f5ffd5b5061037d8061001c5f395ff3fe608060405260043610610036575f3560e01c80630adfba60146100415780632432ef261461004b578063ea3d508a146100b8575f5ffd5b3661003d57005b5f5ffd5b6100496100d0565b005b610083610059366004610208565b7f2432ef260000000000000000000000000000000000000000000000000000000095945050505050565b6040517fffffffff00000000000000000000000000000000000000000000000000000000909116815260200160405180910390f35b3480156100c3575f5ffd5b505f546100839060e01b81565b60408051808201825260028082527f307800000000000000000000000000000000000000000000000000000000000060208084018290528451808601865292835282015291517f2432ef260000000000000000000000000000000000000000000000000000000081523092632432ef2692610150925f92906004016102cd565b6020604051808303815f875af115801561016c573d5f5f3e3d5ffd5b505050506040513d601f19601f820116820180604052508101906101909190610301565b5f80547fffffffffffffffffffffffffffffffffffffffffffffffffffffffff000000001660e09290921c919091179055565b5f5f83601f8401126101d3575f5ffd5b50813567ffffffffffffffff8111156101ea575f5ffd5b602083019150836020828501011115610201575f5ffd5b9250929050565b5f5f5f5f5f6060868803121561021c575f5ffd5b85359450602086013567ffffffffffffffff811115610239575f5ffd5b610245888289016101c3565b909550935050604086013567ffffffffffffffff811115610264575f5ffd5b610270888289016101c3565b969995985093965092949392505050565b5f81518084528060208401602086015e5f6020828601015260207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f83011685010191505092915050565b838152606060208201525f6102e56060830185610281565b82810360408401526102f78185610281565b9695505050505050565b5f60208284031215610311575f5ffd5b81517fffffffff0000000000000000000000000000000000000000000000000000000081168114610340575f5ffd5b939250505056fea2646970667358221220dc87d3522f1b835dee05f2e51e504d2d5f91c8dc2b412aeac6ed91cb2f8d01fa64736f6c634300081c0033"; |
There was a problem hiding this comment.
Fixed in fb6bf6e — now loads bytecode from forge artifacts via getCreationBytecode("DummyInteropRecipient") instead of embedding hex.
There was a problem hiding this comment.
Had to reintroduce in 3d1f6df, the way our CI works it's not possible to load bytecode via getCreationBytecode in CI. Do you have any ideas on how to proceed without having to embed the bytecode?
There was a problem hiding this comment.
We just need to deploy the contract, it shouldn't be a problem? I think we do need a way to deploy these easily for anvil integration tests, we will 100% need it again in the future so I don't think hardcoding the bytecode is a good solution.
There was a problem hiding this comment.
Agreed — done in 195ab51. The solution is generic: store full forge artifacts (with bytecode) in zkstack-out/ for contracts that need runtime deployment. The artifact loader now handles both formats, so any future test contract can use the same pattern: add to ARTIFACTS registry + copy-to-zkstack-out script.
There was a problem hiding this comment.
Follow-up here as well: the generic artifact-loading approach was correct, but the implementation was not complete because regenerating zkstack-out still stripped the bytecode back out. Fixed in f955f52 by teaching copy-to-zkstack-out.ts to keep the full forge artifact for DummyInteropRecipient.sol, so the committed/regen path and the loader now agree.
There was a problem hiding this comment.
zkstack-out is for zkstack files, I don't think we should be adding more clutter there. Can we not just get the contract bytecode from the normal build directory? Anvil multichain tests could run in CI after the compile step so it shouldn't be a problem
There was a problem hiding this comment.
Applied in 2f61058. I removed the special full-artifact handling from zkstack-out, switched loadCreationBytecodeFromOut() to source deploy bytecode only from forge out/, and changed the Anvil interop CI job to compile DummyInteropRecipient.sol before running the tests. zkstack-out/DummyInteropRecipient.json is ABI-only again, so zkstack-out stays ABI-oriented while the tests get deployment bytecode from the normal build directory.
There was a problem hiding this comment.
Anvil multichain tests could run in CI after the compile step so it shouldn't be a problem
I don't want to couple these workflows. As a workaround we can compile only the contracts which are to be deployed before the anvil run, I think it's a reasonable compromise, I don't think we'll have to deploy many custom contracts
There was a problem hiding this comment.
I don't want to couple these workflows.
Why not? Don't normal unit tests already do it? I guess the compromise can work temporarily but imo it is better if we fix this now
| * Must be called before chain registration, since the Forge registration script validates | ||
| * that the base token address is a deployed contract. | ||
| */ | ||
| private async deployCustomBaseTokens(l1RpcUrl: string, chainConfigs: AnvilChainConfig[]): Promise<void> { |
There was a problem hiding this comment.
How are these base tokens being used? Ideally we should have era <> era, era <> custom, custom <> era interop cases being covered
There was a problem hiding this comment.
Good question. Chain 14 (custom ERC20 base token) was added specifically to enable cross-base-token interop testing. Here's the full picture:
Before this commit, the custom base token was used in one direction only:
- era → custom (spec 08): sends ETH from an ETH-based chain to chain 14, where it arrives as a bridged ERC20
After 26f5047, we now also cover the reverse:
- custom → era (spec 08, new test): sends chain 14's custom base token to an ETH-based chain, where it arrives as a bridged ERC20
Full cross-base-token coverage is now:
| Direction | Where | What's tested |
|---|---|---|
| era ↔ era | specs 07, 08, 09 | ETH chains 12/13, bundles + messages + unbundle |
| era → custom | spec 08 | ETH chain sends base token to chain 14 |
| custom → era | spec 08 | Chain 14 sends custom base token to ETH chain |
custom ↔ custom isn't possible with the current topology (only one custom-base-token chain). Adding a second would require new chain config, new v29/v31 states — out of scope for this PR.
There was a problem hiding this comment.
@0xValera could you check this is what's being done? It's not clear from the code. Or alternatively @valera-grinenko-ai point out where this is being done
There was a problem hiding this comment.
Custom base tokens are used in spec 08's cross-base-token tests:
- era → custom (
can send and receive base token to a chain with a different base token): Sends ETH from an ETH-based chain to chain 14 (custom ERC20 base token). ETH arrives as a bridged ERC20. - custom → era (
can send custom base token from a custom-base-token chain to an ETH chain): Sends chain 14's custom base token to an ETH chain. The custom token arrives as a bridged ERC20.
The deployCustomBaseTokens function deploys a TestnetERC20Token on L1 for chain 14 during state generation (setup-and-dump-state.ts). The deployed address is stored in state.customBaseTokens[14] and used by spec 08 to compute the asset ID for cross-chain transfers.
There was a problem hiding this comment.
Does that answer your question? I'm assuming you were confused by era<->custom interactions
7034f52 to
26f5047
Compare
Proactive improvements (26f5047)Comprehensive self-review pass covering code organization, test quality, naming, and infrastructure. Module organization
ERC-7930/7786 encoding
Test structure & assertions
Naming consistency
New test coverage
Code cleanup
Infrastructure hardening
|
| // Check callStatus for all 3 calls after round 1 | ||
| const call0StatusR1 = await getCallStatus(destProvider, bundleHash, 0); | ||
| const call1StatusR1 = await getCallStatus(destProvider, bundleHash, 1); | ||
| const call2StatusR1 = await getCallStatus(destProvider, bundleHash, 2); | ||
| expect(call0StatusR1, "Call 0 should be Unprocessed after round 1").to.equal(CallStatus.Unprocessed); | ||
| expect(call1StatusR1, "Call 1 should be Cancelled after round 1").to.equal(CallStatus.Cancelled); | ||
| expect(call2StatusR1, "Call 2 should be Executed after round 1").to.equal(CallStatus.Executed); | ||
|
|
There was a problem hiding this comment.
surely there's a prettier way to do this
There was a problem hiding this comment.
Could you clarify what you'd like changed here? The two-round unbundle flow with separate balance captures and expectBalanceDelta assertions is already using the shared helpers. Is there a specific pattern you have in mind?
There was a problem hiding this comment.
Just a nit, but I think ideally we specify a desired outcome eg expectedCallStatus = [Unprocessed, Cancelled, Executed] and then iterate through the array, would look cleaner and can be extended easily. Could even define a little helper as this is repeated across tests.
There was a problem hiding this comment.
we do not seem to be testing the fixed fee case which was being covered before anywhere
There was a problem hiding this comment.
The useFixedFee attribute requires a ZK token fee mechanism that isn't configured in the Anvil test environment (the protocol fee defaults to 0, and the fixed fee path requires a separate ZK token setup). The original zksync-era tests also only used useFixedFee=false. Adding useFixedFee=true coverage would require setting up the ZK token fee infrastructure in the harness — worth tracking as a follow-up once that infrastructure is available.
There was a problem hiding this comment.
Ideally having it in this PR would be nice, but if everything else will check out we will merge this and create a follow up Linear task
There was a problem hiding this comment.
Actually was added in latest commits
…ness Port all 27 interop-b integration tests from zksync-era into the era-contracts Anvil multichain testing framework (specs 07, 08, 09). Bundles (spec 07): single/multi/mixed direct and indirect call bundles, source-chain balance deductions, destination-chain balance deltas. Messages (spec 08): sendMessage API for base token and ERC20 transfers, with send-side and receive-side balance verification. Unbundle (spec 09): progressive unbundling, bundle/call status tracking, 7 negative-path revert checks, cross-chain verify+unbundle meta-bundle. DummyInteropRecipient contracts are deployed at test runtime using embedded creation bytecode. No changes to pre-generated chain states. Token balance migration tests will follow in a separate PR. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…interop tests Add a 5th L2 chain (chain 14) with a custom ERC20 base token to enable the 2 previously-skipped interop message tests (matter-labs#13, matter-labs#17 from source): - "sends base token to a chain with a different base token" (ETH goes through AssetRouter indirect call path) - "receives base token from sending chain as bridged ERC20" (ETH arrives as a wrapped ERC20 on the custom-base-token chain) Infrastructure changes: - AnvilChainConfig gains optional baseToken field - DeploymentRunner deploys TestnetERC20Token on L1 for custom-base-token chains - Gateway setup uses per-chain baseTokenAssetId for TBM (not hardcoded ETH) - Interop chain registrar validates registration without assuming ETH base token All 27 interop-b tests now fully covered — 0 skipped. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Regenerated v0.29.0 pre-generated chain states from the kl/generate-v29-state branch with chain 14 added. Chain 14 uses a custom ERC20 base token, matching its v0.31.0 configuration. This ensures the v29→v31 upgrade test covers all 5 L2 chains including the custom-base-token chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The v0.29.0 states were generated from kl/generate-v29-state which has v29 AdminFacet code (2-arg upgradeChainFromVersion, selector 0xfc57565f), but l1-deployment.toml set latest_protocol_version to 0x1f00000000 (v31). This caused DefaultChainUpgrade to use the 3-arg selector (0x3b6d7534) which isn't registered in the v29 AdminFacet, reverting with "F". Fix: set latest_protocol_version to 0x1d00000000 (v29) on the generation branch and regenerate all chain states including chain 14. Now the upgrade script correctly uses the 2-arg selector path for v29→v31 upgrades. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…m artifacts - Add trailing newline to chain-14.toml - Replace magic number 3 with callStarters.length in unbundle spec - Derive ERC-7786 attribute selectors from IERC7786Attributes ABI instead of hardcoding hex values; unify with token-transfer.ts which had a third approach (inline keccak256) - Load DummyInteropRecipient bytecode from forge artifacts via getCreationBytecode() instead of embedding 800 chars of hex - Add IERC7786Attributes and DummyInteropRecipient to ARTIFACTS registry Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Module organization: - Split 488-line interop-bundle-helper.ts into 3 focused modules: erc7930.ts (address encoding), interop-helpers.ts (RPC wrappers), balance-helpers.ts (assertions and balance utilities) - Deduplicate event extraction in token-transfer.ts — now uses sendInteropBundle()/executeBundle() instead of hand-rolled copies ERC-7930/7786 encoding: - Rewrite ERC-7930 encoding with string concatenation for clarity, matching temp-sdk.ts style from zksync-era - Use IERC7786Attributes.encodeFunctionData() for attribute encoding instead of manual selector+abiCoder; remove selector constants - Document why callStarters[].to uses chain-less encoding Test structure: - Make all tests self-contained (send + execute + verify per it() block) - Use exact balance assertions with gas cost from tx receipt - Extract assertion helpers: expectNativeSpend, expectBalanceDelta, expectRevert (with optional reason matching) - Standardize naming: interopFee, sourceTokenAddress, sourceAssetId - Rename helpers for accuracy: approveTokenForNtv, BalanceSnapshot, captureBalance Test coverage: - Add custom→era cross-base-token interop message test - Add replay protection test (executeBundle twice reverts) - Add executionAddress enforcement test (wrong sender reverts) - Add zero-call bundle test (documents protocol accepts empty bundles) - Add excess msg.value rejection test Code cleanup: - Remove dead code: FAILING_CALL_CONTRACT, extractBundleHash - Remove duplicated encodeEvmChain/encodeEvmAddress from token-transfer - Unify SendBundleResult/SendMessageResult into InteropSendResult - Fix cross-base-token tests to use before/after deltas not absolutes - Make destTokenAddress local per-test instead of shared mutable state Infrastructure hardening: - Fix Anvil process leak on failure in setup-and-dump-state.ts - Add missing port 4054 to cleanup.sh fallback list - Set FOUNDRY_PROFILE=anvil-interop in upgrade test - Replace deprecated fs.rmdirSync with fs.rmSync - Scope prettier to chain-states/ directory only - Improve storage slot override TODO tracking - Document chain-14.toml base_token_addr derivation - Fix baseToken JSDoc in AnvilChainConfig type Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The interop helpers load the IERC7786Attributes interface at module init time to derive ERC-7786 attribute selectors via encodeFunctionData. CI integration tests run with --no-compile and rely on zkstack-out/ for ABIs. Without this file, the import fails with ENOENT. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The CI integration-test job runs without forge build (no out/ directory). DummyInteropRecipient bytecode must be embedded since there is no compiled artifact available at runtime. This was the original design before the review refactored it to use getCreationBytecode(). Also remove DummyInteropRecipient from the ARTIFACTS registry and the unused getCreationBytecode import. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The check-zkstack-out CI job regenerates zkstack-out/ from scratch and compares against committed files. IERC7786Attributes was added to zkstack-out/ manually but not registered in the copy script, causing a mismatch. Add it to REQUIRED_CONTRACTS so it's generated consistently. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Set executionAddress = ANVIL_DEFAULT_ACCOUNT_ADDR and unbundlerAddress = ANVIL_ACCOUNT2_ADDR in sendAndPrepareBundle. This properly tests role separation — the executor and unbundler are now different accounts. Update all unbundleBundle calls to use ANVIL_ACCOUNT2_PRIVATE_KEY as the correct unbundler signer. The "wrong unbundler" test now uses the default account (the executor, not the unbundler). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add randomBigNumber(min, max) helper and replace fixed constants with per-test random amounts in all three interop specs. Each test now generates its own amount within a safe range, so tests don't pass only for specific hardcoded values. Ranges are kept small (gwei-scale for native, reasonable for ERC20) to avoid balance exhaustion while still being large enough to detect. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DummyInteropRecipient bytecode: - Store the full forge artifact (with bytecode) in zkstack-out/ so CI integration tests can load it without forge build - Update artifact loader to handle both formats: raw ABI arrays (existing zkstack-out convention) and full forge artifacts (with bytecode field) - Add DummyInteropRecipient to ARTIFACTS registry and copy-to-zkstack-out - Remove embedded hex bytecode from interop-helpers.ts expectRevert: - Now checks both the JS error message and on-chain revert data (error.error.data / error.data) for the expected reason string - Failure messages include excerpts of both message and data for debugging Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rebase onto latest draft-v31 (62de149) which regenerated v0.31.0 states due to the protocol ops tool PR (matter-labs#2097). Regenerated v0.31.0 states from scratch with chain 14 included. Chain 14's base_token_addr changed from 0xA4899D3... to 0x86A2EE8... due to upstream deploy nonce changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
195ab51 to
e0adb62
Compare
Summary
Infrastructure: Chain 14 with custom base token
Added chain 14 (GW-settled, port 4054) with a custom ERC20 base token to test cross-base-token interop paths.
Key infrastructure changes:
Test architecture
Spec breakdown
Spec 07 — Bundles (9 tests): Single/multi/mixed direct and indirect call bundles with exact balance verification. Edge cases: replay protection, executionAddress enforcement, zero-call bundle, excess msg.value.
Spec 08 — Messages (6 tests): `sendMessage` API for base token and ERC20 transfers including both cross-base-token directions (era→custom, custom→era).
Spec 09 — Unbundle (8 tests): Progressive unbundling with fresh bundle per test, bundle/call status tracking, revert checks via `expectRevert`, cross-chain verify+unbundle meta-bundle.
Test plan
🤖 Generated with Claude Code