feat(op-acceptance-tests): l2cm upgrade accaptance tests and fork tests on karst betanet by 0xCoati · Pull Request #20668 · ethereum-optimism/optimism

0xCoati · 2026-05-12T09:42:43Z

Description
Ability to run fork tests on karst betanet, add acceptance tests for withdrawal. L2 Fork test for checking the existence of the deterministic-deployment-proxy.

Tests
mise exec -- just test -run TestWithdrawal_Karst ./op-acceptance-tests/tests/karst/

Running L2ForkUpgrade.t.sol with a betanet

export L2_FORK_RPC_URL=<url>
export L2_BLOCK_BEFORE_FORK=<previous blockNumber before activation>
just test-l2cm-activation-test

Additional Context
Setting up a karst betanet is required for running the tests this way.

Metadata

fixes #18870

…imal upgrade with l2cm

… and add flags to support running against betanet

0xiamflux · 2026-05-12T16:39:35Z

        // Skip if not L2 fork test
        skipIfNotL2ForkTest("L2ForkUpgrade: not a fork test");

        // Skip if L2CM dev feature is not enabled


Shouldn't we also check for KARST_BETANET_L2_FORK_TEST before skipping L2 Forked tests? Setting KARST_BETANET_L2_FORK_TEST=true would still just silently skip these.

wouldn't the L2CM feature flag be true on the karst betanet?

Sorry, I meant the check above: skipIfNotL2ForkTest("L2ForkUpgrade: not a fork test");

I see. Probably better to adjust the justfile so L2_FORK_TEST=true is set too for the test-karst-betanet-l2-fork-upgrade task? There are quite a few places were we rely on l2ForkTest / isL2ForkTest.

That sounds good.

actually it's already done, because test-karst-betanet-l2-fork-upgrade starts with prepare-l2-upgrade-env which does set L2_FORK_TEST. I removed a redundant check here 1971c09

This reverts commit 329e54d.

…ng a minimal upgrade with l2cm" This reverts commit 9782ee9.

…k test due to PastUpgrades handling it already

…he deterministic deployer

0xOneTony · 2026-05-13T13:17:16Z

/ci authorize da9943e

0xOneTony · 2026-05-13T13:45:54Z

/ci authorize 331ce17

0xiamflux · 2026-05-14T15:59:04Z

/ci authorize 96f15bc

0xiamflux · 2026-05-14T17:32:22Z


    /// @notice Executes the current generated NUT bundle with any fork-specific wrappers.
    function _executeCurrentBundle() internal virtual {
        PastNUTBundles.ForkWrappers memory w = PastNUTBundles.wrappersForFork(currentFork);


I think I see this more clearly here, it should be able to skip the current bundle if it's already applied not only if karst is applied:

if (_isCurrentBundleAlreadyApplied()) return;

…rrent bundle is applied

…with before/after blocknumbers and capture events from the activation block

0xiamflux · 2026-05-18T22:36:30Z

-        return vm.envString("KARST_BETANET_L2_FORK_RPC_URL");
+    /// @notice Returns the L2 block after the fork.
+    function l2BlockAfterFork() internal view returns (uint256) {
+        return vm.envOr("L2_FORK_BLOCK_NUMBER", uint256(0));


I would think it should be an error to either attempt to use this on a non-activation test or try to use it during a activation test while it is 0. wdyt?

fixed in fff70c2

0xiamflux · 2026-05-18T22:49:08Z


-test-karst-betanet-l2-fork-upgrade-rerun *ARGS:
+test-post-karst-betanet-l2-fork-upgrade-rerun *ARGS:
  just test-karst-betanet-l2-fork-upgrade {{ARGS}} --rerun -vvvv


Shouldn't this be also updated to match test-post-karst-betanet-l2-fork-upgrade?

fixed in fff70c2

…terFork revert outside l2CMActivationTest

lumoswiz · 2026-05-19T08:09:50Z

+    /// @notice Returns the L2 block after the fork.
+    function l2BlockAfterFork() internal view returns (uint256) {
+        if (l2CMActivationTest()) {
+            return vm.envOr("L2_FORK_BLOCK_NUMBER", uint256(0));


I think this needs to be fixed before merge.

The justfile documents L2_BLOCK_AFTER_FORK, but this reads L2_FORK_BLOCK_NUMBER and defaults to 0. I assume 0 is meant to preserve the existing "use latest" behaviour from the normal L2 fork setup. But activation mode passes this value directly to createSelectFork(url, block), so following the documented command makes the post upgrade fork select explicit block 0, not latest or the intended after fork block.

Could we do one of the following:

Read a mandatory L2_BLOCK_AFTER_FORK

Keep 0 as latest and have _executeCurrentBundleOrSwitchFork() call createSelectFork(url) overload when this returns 0?

The pre/post activation flow can become:

Fork at L2_BLOCK_BEFORE_FORK

Capture pre upgrade state

Fork at block 0

Verify post upgrade state

fixed in d292e28 and e05e520

…after activation block number

lumoswiz · 2026-05-19T09:40:06Z

+                address(0),
+                topics
+            );
+            logs = _ethGetLogsToLogs(ethLogs);


Currently the L2CM activation path only checks the historical Upgraded(address) logs. That doesn't prove the activation block contained the expected NUT bundle txns.

Can we also assert that the activation block contains the expected activation bundle txns? Otherwise missing, reordered, or corrupted upgrade txns could still be missed if the expected upgrade logs are present.

0xOneTony · 2026-05-19T12:36:14Z

/ci authorize e05e520

0xOneTony · 2026-05-21T13:56:55Z

/ci authorize 33b1700

0xiamflux · 2026-05-21T14:39:14Z

+
+    /// @notice Verify betanet fork upgrade tests switch to the block after the activation block.
+    function _executeCurrentBundle() internal virtual override {
+                uint256 l2BlockAfterFork = Config.l2BlockAfterFork();


nit: let's fix this indentation

0xOneTony · 2026-05-21T14:50:17Z

/ci authorize 281e44a

0xiamflux · 2026-05-21T15:03:05Z

+# Env Vars:
+# - L2_FORK_RPC_URL must be set to a post-Karst betanet L2 RPC URL.
+# - L2_BLOCK_BEFORE_FORK must be set to the block right before the fork
+# - L2_FORK_BLOCK_NUMBER can be set in the env to pin a block (defaults to latest).


Can we just clearly document that when running in 'activation mode' this is used as the post-fork block number?

fixed in 521f991

…n an upgraded network

maurelian · 2026-05-21T20:46:45Z

+    }
+
+    /// @notice Verify betanet fork upgrade tests switch to the block after the activation block.
+    function _executeCurrentBundle() internal virtual override {


Just add a short natspec comment clarifying that this doesn't do what the name says it does, ie. it overrides the execution by going to a block after the fork.

fixed in 9f54a9c

maurelian · 2026-05-21T20:58:19Z

+        L2VerifyBetanetForkUpgrade_TestInit._executeCurrentBundle();
+    }
+
+    function test_l2ForkUpgrade_upgradeEventsEmitted_succeeds() public override(L2ForkUpgrade_Events_Test) {


This test looks very similar to the one in the original test, why does it need to be reimplemented?

there are some ordering issues which make more sense to fix by an override, but I'll move the verification logic to a shared helper.

fixed in c1181f3.

…entBundle override

0xOneTony · 2026-05-22T08:23:55Z

/ci authorize ba75e97

0xOneTony · 2026-05-25T15:09:42Z

+        if (l2CMActivationTest()) {
+            return vm.envUint("L2_BLOCK_BEFORE_FORK");
+        }


would it be better to have this its own getter?

this is the only place where it's used, as we are taking advantage of the existing test setup which forks based on this getter.

0xOneTony · 2026-05-25T15:13:39Z

+// TestWithdrawal_KarstUpgrade is the same withdrawal flow but on a network that
+// started pre-Karst and activated Karst mid-chain via a scheduled upgrade.
+func TestWithdrawal_KarstUpgrade(gt *testing.T) {
+	offset := uint64(10)


how was this offset picked? would be good to have an inline comment here for this

fixed in 599e4a9

0xOneTony · 2026-05-25T15:32:49Z

+    /// @notice Converts RPC logs from `vm.eth_getLogs` into the shape returned by `vm.getRecordedLogs`.
+    /// @param _ethLogs The RPC logs from `vm.eth_getLogs`.
+    /// @return logs_ The logs in the shape returned by `vm.getRecordedLogs`.
+    function _ethGetLogsToLogs(Vm.EthGetLogs[] memory _ethLogs) internal pure returns (Vm.Log[] memory logs_) {


this is only used in _getLogs i think we can just remove this and do it directly inside the nested loop

0xOneTony · 2026-05-25T15:37:45Z

+    }
+}
+
+/// @title L2GenesisForkUpgrade_Versions_Test


wrong title

fixed in c9ea936

0xOneTony · 2026-05-25T15:38:17Z

    }

+    /// @notice Indicates whether a test is running against a Karst betanet L2 fork test.
+    function isL2CMActivationTest() public view returns (bool) {


this is not used

fixed in c9ea936

0xOneTony · 2026-05-25T15:42:02Z

+        returns (uint256)
+    {
+        for (uint256 i = 0; i < _froms.length; i++) {
+            if (_froms[i] == _firstBundleTxn.from && _tos[i] == _firstBundleTxn.to) {


since we have the first bundle tx, should we also check the data just to be more robust?

fixed in d4711ae

0xOneTony · 2026-05-25T15:44:17Z

+    /// @notice Returns true when the current bundle has already been applied to the forked chain.
+    ///         Uses two checks: ConditionalDeployer exists (Karst ran) and this bundle's
+    ///         L2ContractsManager was deployed at its expected address.
+    function _isCurrentBundleAlreadyApplied() internal view returns (bool) {


i think this should be _isKarstBundleAlreadyApplied or we should add a TODO to change this at some point since the "current" bundle will change

That wouldn't be correct, that's just the first half of the condition, as we also check that the l2cm matches the current bundle. so it's any current bundle post karst.

0xCoati added 5 commits May 7, 2026 09:58

test(withdraw-post-karst): acceptance test for withdrawing on karst

0179e8a

test(acceptance-test-l2cm-karst): add acceptance test for doing a min…

9782ee9

…imal upgrade with l2cm

test(karst-fork-tests): skip upgrade executing when already on karst,…

a118a9c

… and add flags to support running against betanet

test(acceptance-test-l2cm-karst): make the test proper e2e

329e54d

Merge branch 'develop' into test/l2cm-upgrade-accaptance-test

0a8fe52

0xiamflux reviewed May 12, 2026

View reviewed changes

0xiamflux mentioned this pull request May 12, 2026

[L2CM Upgrades] Devnet acceptance testing #18870

Open

3 tasks

0xCoati added 5 commits May 13, 2026 10:25

fix(acceptance-test-l2cm-karst): remove redundant flag check

1971c09

Revert "test(acceptance-test-l2cm-karst): make the test proper e2e"

a12f6d0

This reverts commit 329e54d.

Revert "test(acceptance-test-l2cm-karst): add acceptance test for doi…

e2eb850

…ng a minimal upgrade with l2cm" This reverts commit 9782ee9.

fix(acceptance-test-l2cm-karst): remove karst activation check in for…

81bcb09

…k test due to PastUpgrades handling it already

test(acceptance-test-l2cm-karst): add test to check the presence of t…

da9943e

…he deterministic deployer

0xOneTony changed the title ~~l2cm upgrade accaptance tests and fork tests on karst betanet~~ feat(op-acceptance-tests): l2cm upgrade accaptance tests and fork tests on karst betanet May 13, 2026

fix(acceptance-test-l2cm-karst): fix testname lint error

331ce17

fix(acceptance-test-l2cm-karst): remove misleading comment

96f15bc

0xCoati marked this pull request as ready for review May 13, 2026 20:34

0xCoati requested review from a team and wwared May 13, 2026 20:34

0xCoati requested a review from a team as a code owner May 13, 2026 20:34

0xiamflux reviewed May 14, 2026

View reviewed changes

0xCoati added 2 commits May 15, 2026 14:51

fix(acceptance-test-l2cm-karst): skip execute and events test when cu…

7cd0a82

…rrent bundle is applied

fix(acceptance-test-l2cm-karst): rework betanet upgrade test to work …

71506e6

…with before/after blocknumbers and capture events from the activation block

0xiamflux reviewed May 18, 2026

View reviewed changes

fix(acceptance-test-l2cm-karst): fix rerun command and make l2BlockAf…

fff70c2

…terFork revert outside l2CMActivationTest

lumoswiz reviewed May 19, 2026

View reviewed changes

fix(acceptance-test-l2cm-karst): fix usage example

d292e28

fix(acceptance-test-l2cm-karst): add missing latest fallback for the …

e05e520

…after activation block number

lumoswiz reviewed May 19, 2026

View reviewed changes

0xCoati added 3 commits May 20, 2026 13:50

fix(acceptance-test-l2cm-karst): fix semgrep errors

5185926

fix(acceptance-test-l2cm-karst): all tests fixed

ba4153b

fix(acceptance-test-l2cm-karst): move tests to separate file

33b1700

fix(acceptance-test-l2cm-karst): fix linter issues

373b1ee

0xiamflux reviewed May 21, 2026

View reviewed changes

Merge branch 'develop' into test/l2cm-upgrade-accaptance-test

281e44a

0xiamflux reviewed May 21, 2026

View reviewed changes

0xCoati added 2 commits May 21, 2026 17:07

fix(acceptance-test-l2cm-karst): improve env var comment

521f991

test(acceptance-test-l2cm-karst): make a withdrawal acceptance test o…

2017222

…n an upgraded network

maurelian reviewed May 21, 2026

View reviewed changes

Comment thread op-acceptance-tests/tests/karst/withdrawal_test.go

maurelian reviewed May 21, 2026

View reviewed changes

0xCoati added 3 commits May 21, 2026 23:15

fix(acceptance-test-l2cm-karst): fix natspec comment for _executeCurr…

9f54a9c

…entBundle override

fix(acceptance-test-l2cm-karst): reuse assertion logic in events tests

c1181f3

Merge branch 'develop' into test/l2cm-upgrade-accaptance-test

ba75e97

maurelian approved these changes May 22, 2026

View reviewed changes

0xOneTony reviewed May 25, 2026

View reviewed changes

0xCoati added 4 commits May 25, 2026 18:43

fix(acceptance-test-l2cm-karst): add comment for fork offset

599e4a9

fix(acceptance-test-l2cm-karst): inline log transformation

bdd84c8

fix(acceptance-test-l2cm-karst): cleanup

c9ea936

fix(acceptance-test-l2cm-karst): include data in _findBundleStart

d4711ae

Conversation

0xCoati commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0xOneTony commented May 13, 2026

Uh oh!

0xOneTony commented May 13, 2026

Uh oh!

0xiamflux commented May 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xiamflux May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xOneTony commented May 19, 2026

Uh oh!

0xOneTony commented May 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xOneTony commented May 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xOneTony commented May 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

0xCoati commented May 12, 2026 •

edited

Loading

0xiamflux May 18, 2026 •

edited

Loading