test: zk dispute game factory and registry by 0xChin · Pull Request #19872 · ethereum-optimism/optimism

0xChin · 2026-04-01T15:56:25Z

Summary

Adds ZK dispute game test coverage for DisputeGameFactory and AnchorStateRegistry, proving both contracts work correctly with the new ZK_DISPUTE_GAME type (game type 10).

Changes

`DisputeGameFactory.t.sol`

Restructuring:

Extracted ZK tests from generic factory test contracts into dedicated _ZkDisputeGame_ contracts
Moved test_create_implArgs_succeeds into DisputeGameFactory_Create_FaultDisputeGame_Test for explicit game-type separation
Created DisputeGameFactory_ZkDisputeGame_TestInit abstract base with shared helpers (_createZKGame, _assertZKGameFactoryStorage, _assertZKGameCWIA)

New test contracts:

DisputeGameFactory_Create_ZkDisputeGame_Test — create happy path, zero bond, wrong bond, no impl, duplicate UUID
DisputeGameFactory_SetImplementation_ZkDisputeGame_Test — impl+args storage, non-owner revert
DisputeGameFactory_SetInitBond_ZkDisputeGame_Test — bond set/update, non-owner revert
DisputeGameFactory_FindLatestGames_ZkDisputeGame_Test — multi-game ordering

New generic test:

testFuzz_create_overpayment_reverts — verifies factory rejects overpayment (sends more ETH than required bond)

`AnchorStateRegistry.t.sol`

New abstract base:

AnchorStateRegistry_ZkDisputeGame_TestInit — creates a real ZK game via factory, provides _mockZkGameAsValid() helper

New test contracts:

AnchorStateRegistry_SetAnchorState_ZkDisputeGame_Test — valid newer state, older game claim rejected, blacklisted, retired, challenger wins, not finalized (6 tests)
AnchorStateRegistry_IsGameClaimValid_ZkDisputeGame_Test — valid claim, blacklisted, retired, not respected (4 tests)
AnchorStateRegistry_IsGameProper_ZkDisputeGame_Test — all conditions met, blacklisted (2 tests)
AnchorStateRegistry_BlacklistDisputeGame_ZkDisputeGame_Test — blacklist succeeds (1 test)

Test plan

DEV_FEATURE__ZK_DISPUTE_GAME=true forge test --match-path test/dispute/DisputeGameFactory.t.sol
DEV_FEATURE__ZK_DISPUTE_GAME=true forge test --match-path test/dispute/AnchorStateRegistry.t.sol
just pre-pr passes

ashitakah · 2026-04-01T17:04:37Z

packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol

+    function setUp() public virtual override {
+        super.setUp();
+
+        skipIfDevFeatureDisabled(DevFeatures.ZK_DISPUTE_GAME);


Every this is testes when zk dispute game is enabled. Maybe create one to check that reverts if the flag is disabled.

With the flag disabled, the ZK game implementation wouldn't be deployed or registered in the factory so there's no zkGameProxy to test against. The ASR and factory are game-type agnostic; they don't check the feature flag themselves. The flag only gates deployment at the OPCM level, which is already covered by test_upgrade_enableZKGameWithoutDevFeature_reverts in the OPCM tests. At the ASR/factory level, "flag disabled" just means "no ZK game exists," which isn't a ZK-specific scenario worth testing here.

ashitakah · 2026-04-01T17:05:44Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+
+        bytes memory args = abi.encodePacked(
+            keccak256("absolutePrestate"), // absolutePrestate  32 bytes
+            zkVerifier, // verifier          20 bytes


could we just create some vars instead of having the numer

refactor: use vars instead of magic numbers

ashitakah · 2026-04-01T17:07:02Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+        // Verify CWIA getters — confirms gameArgs were forwarded correctly without re-encoding.
+        assertEq(GameType.unwrap(_proxy.gameType()), GameType.unwrap(GameTypes.ZK_DISPUTE_GAME));
+        assertEq(_proxy.gameCreator(), _proposer);
+        assertTrue(_proxy.l1Head().raw() != bytes32(0));


you only check that is not 0 , there is any way to check with the proper value

test: do assertions over exact values

ashitakah · 2026-04-01T17:07:10Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+        assertTrue(_proxy.l1Head().raw() != bytes32(0));
+        assertEq(_proxy.parentIndex(), type(uint32).max);
+        assertEq(_proxy.absolutePrestate(), keccak256("absolutePrestate"));
+        assertTrue(address(_proxy.verifier()) != address(0));


test: do assertions over exact values

ashitakah · 2026-04-01T17:08:39Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+    function test_create_zeroBond_reverts() public {
+        skipIfDevFeatureDisabled(DevFeatures.ZK_DISPUTE_GAME);
+
+        setupZKDisputeGame(


Could we crearte a func to create this? we have something similar in test_create_wrongBond_reverts

refactor: add zkCreateParams helper function for reused logic

ashitakah · 2026-04-01T18:12:07Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+        vm.prank(proposer);
+        vm.expectRevert(IncorrectBondAmount.selector);
+        // Sending 0.5 ether instead of the required 1 ether.
+        disputeGameFactory.create{ value: 0.5 ether }(GameTypes.ZK_DISPUTE_GAME, rootClaim_, extraData_);


Could we create something like: testFuzz_create_overpayment_revert we are not testing the overpay only the under.

There is no harm in adding more tests, but the generic testFuzz_create_overpayment_reverts in DisputeGameFactory_Create_Test already covers game type 10 (ZK).

The bond check (msg.value != initBonds[_gameType]) happens in the factory before initialize()is called, so the game implementation is irrelevant FakeClone and ZKDisputeGame hit the exact same code path. Adding a ZK-specific version wouldn't improve coverage

ashitakah · 2026-04-01T18:21:58Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+        address zkImpl = address(new ZKDisputeGame());
+        IZKVerifier zkVerifier = IZKVerifier(address(new ZKMockVerifier()));
+
+        bytes memory args = abi.encodePacked(


Also could we use the same helper func that use the contract to encode

not quite following what helper function for encoding you are talking about

ashitakah · 2026-04-01T18:25:15Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+
+        // Find the 2 most recent ZK games.
+        IDisputeGameFactory.GameSearchResult[] memory results =
+            disputeGameFactory.findLatestGames(GameTypes.ZK_DISPUTE_GAME, latestIdx, 2);


Do we have another tests with n > gameCount

we don't. good catch test: check findLatestGames works if n > gameCount

ashitakah · 2026-04-01T18:26:24Z

packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol

+contract AnchorStateRegistry_IsGameProper_ZkDisputeGame_Test is AnchorStateRegistry_ZkDisputeGame_TestInit {
+    /// @notice Tests that a ZK game meeting all conditions is proper.
+    function test_isGameProper_meetsAllConditions_succeeds() public view {
+        assertTrue(anchorStateRegistry.isGameProper(zkGameProxy));


Should we add another one for farse, with a random address or something likes this

test: check notRegistered path in ASR ZKDisputeGame test

Added test_isGameProper_notRegistered_succeeds which mocks the factory to report the ZK game as unregistered and asserts isGameProper returns false. Note that the generic tests (test_isGameProper_isNotFactoryRegistered_succeeds, testFuzz_isGameClaimValid_notRegistered_succeeds) already cover this code path since the registration check is game-type agnostic, but having the explicit ZK
variant keeps the test structure consistent so good catch ser

0xOneTony · 2026-04-02T11:16:05Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+            disputeGameFactory.setInitBond(lgt, 1 ether);
+        }
+
+        vm.deal(address(this), 2 ether);


should fuzz the value

test: add fuzz testing

0xOneTony · 2026-04-02T11:27:57Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+        for (uint8 i; i < maxGameType + 1; i++) {
+            GameType lgt = GameType.wrap(i);
+            disputeGameFactory.setImplementation(lgt, IDisputeGame(address(fakeClone)));
+            disputeGameFactory.setInitBond(lgt, 1 ether);
+        }


is this loop needed? whats the purpose of going through all game types?

this test was removed in test: add fuzz testing given testFuzz_create_wrongBond_reverts covers all bond mismatch scenarios (underpayment, overpayment, zero) with fuzzing.

but regarding the loop logic, it would set the implementation to all game types like it's done in the previously existing testFuzz_create_incorrectBondAmount_reverts to avoid the NoImplementation() error

0xOneTony · 2026-04-02T11:41:57Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+        (, uint256 anchorL2SeqNum) = anchorStateRegistry.getAnchorRoot();
+
+        // extraData: l2SequenceNumber (32 bytes) || parentIndex (4 bytes).
+        // parentIndex = uint32.max indicates this is the first game in the chain.
+        bytes memory extraData_ = abi.encodePacked(anchorL2SeqNum + 1000, type(uint32).max);
+
+        Claim rootClaim_ = changeClaimStatus(Claim.wrap(keccak256("zkRootClaim")), VMStatuses.INVALID);


couldnt you use _zkCreateParams ?

yes. implemented here defi-wonderland@38a5518708#diff-c1542157f455ff7ac4f807241035b540d9e5a9f75ec3f7224cbdc46843b66de7R890-R897

0xOneTony · 2026-04-02T12:19:50Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+
+/// @title DisputeGameFactory_Create_ZkDisputeGame_Test
+/// @notice Tests the `create` function of the `DisputeGameFactory` contract with ZKDisputeGame.
+contract DisputeGameFactory_Create_ZkDisputeGame_Test is DisputeGameFactory_ZkDisputeGame_TestInit {


missing fuzzing - same for the rest of the test contracts

done in test: add fuzz testing and test: add more fuzz tests

0xOneTony · 2026-04-02T12:23:56Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+            ZKDisputeGameParams({
+                maxChallengeDuration: Duration.wrap(3.5 days),
+                maxProveDuration: Duration.wrap(12 hours),
+                absolutePrestate: keccak256("absolutePrestate"),
+                challengerBond: 1 ether
+            })


I think that if we are gonna hardcode these values for everything then there is no point in setupZKDisputeGame to have a ZKDisputeGameParams input for this and repeat the same hardcoded values we could just define a default set of params, unless we want to have different params or fuzz any of these.
This is especially important since in _assertZKGameCWIA the expected values are always hardcoded for the specific vars.

Yes, I've defined a defaultZKParams var. Also modified the _assertZKGameCWIA allowing it to receive the game params as arg for all the fuzz testing implemented

default params: https://github.com/defi-wonderland/optimism/blob/b864e0890edef05980aa778dd236c0f2c50577be/packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol#L879-L884

0xOneTony · 2026-04-02T12:29:07Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+        vm.warp(block.timestamp + 1000);
+        vm.prank(proposer);
+        vm.expectRevert(IncorrectBondAmount.selector);
+        disputeGameFactory.create{ value: 0.5 ether }(GameTypes.ZK_DISPUTE_GAME, rootClaim_, extraData_);


we could fuzz the amount here

if you fuzz the testFuzz_create_overpayment_reverts then probably this becomes a repeated test

same as here #19872 (comment)

0xOneTony · 2026-04-02T12:34:12Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+contract DisputeGameFactory_Create_ZkDisputeGame_Test is DisputeGameFactory_ZkDisputeGame_TestInit {
+    /// @notice Tests that the factory creates a ZKDisputeGame CWIA clone correctly.
+    function test_create_succeeds() public {
+        skipIfDevFeatureDisabled(DevFeatures.ZK_DISPUTE_GAME);


The best practice in cases like this where we skip all tests since all are related to the ZK dispute game dev flag is to define this in a setUp function probably best would be in DisputeGameFactory_ZkDisputeGame_TestInit

refactor: add reused vars/logic in init contract

0xOneTony · 2026-04-02T12:56:21Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+/// @title DisputeGameFactory_ZkDisputeGame_TestInit
+/// @notice Reusable test initialization for ZKDisputeGame factory tests.
+abstract contract DisputeGameFactory_ZkDisputeGame_TestInit is DisputeGameFactory_TestInit {
+    IZKVerifier zkVerifier;


do we need this?

It's declared as a state var in TestInit because both _createZKGameWithParams (which sets it) and _assertZKGameCWIA (which reads it) live there. The alternative would be returning it from _createZKGameWithParams and passing it as an extra param to _assertZKGameCWIA, but I'd rather keep it this way

0xOneTony · 2026-04-02T12:56:54Z

packages/contracts-bedrock/test/dispute/DisputeGameFactory.t.sol

+        address zkImpl = address(new ZKDisputeGame());
+        bytes memory args = abi.encodePacked(keccak256("absolutePrestate"));
+
+        vm.prank(address(0));


test: add fuzz testing

0xOneTony · 2026-04-02T13:03:48Z

packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol

 import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.sol";
 import { IFaultDisputeGame } from "interfaces/dispute/IFaultDisputeGame.sol";
 import { IProxyAdminOwnedBase } from "interfaces/universal/IProxyAdminOwnedBase.sol";
+import { ZKDisputeGame } from "src/dispute/zk/ZKDisputeGame.sol";


Can't we use the interface instead?

yes refactor: use IDisputeGame interface, actually given that the ASR should be game-agnostic, I've directly used the IDisputeGame interface. which works

0xOneTony

requesting changes above^

0xOneTony · 2026-04-02T16:58:31Z

/ci authorize a651fe6

0xChin · 2026-04-02T20:20:15Z

/ci authorize c41aa6f

0xChin · 2026-04-03T00:58:39Z

/ci authorize e8910df

0xChin added 7 commits March 31, 2026 17:01

test: add ZK DGF tests

9ccc54d

test: restructure ZK dispute game factory tests into dedicated contracts

f868731

test: add bond overpayment scenario

3946a3f

chore: pre-pr

34ff082

test: add ZK dispute game tests for AnchorStateRegistry

661f1c6

chore: run just pr

84923be

Merge branch 'develop' into test/zk-dispute-game-factory-and-registry

c15f8c5

0xChin self-assigned this Apr 1, 2026

ashitakah requested changes Apr 1, 2026

View reviewed changes

0xChin added 5 commits April 1, 2026 16:18

refactor: use vars instead of magic numbers

a02e5d6

test: do assertions over exact values

107747f

refactor: add zkCreateParams helper function for reused logic

818ca5b

test: check findLatestGames works if n > gameCount

27d0889

test: check notRegistered path in ASR ZKDisputeGame test

dfe8893

0xOneTony approved these changes Apr 2, 2026

View reviewed changes

0xOneTony requested changes Apr 2, 2026

View reviewed changes

0xChin added 5 commits April 2, 2026 12:08

test: add fuzz testing

38a5518

test: add more fuzz tests

b864e08

refactor: use IDisputeGame interface

e1d31f8

refactor: add reused vars/logic in init contract

46e6ce0

chore: run just pr

a651fe6

0xChin added 2 commits April 2, 2026 16:37

fix: just-upgrade CI failing

4b80e5b

chore: run just pr

c41aa6f

Merge branch 'develop' into test/zk-dispute-game-factory-and-registry

e8910df

Conversation

0xChin commented Apr 1, 2026

Summary

Changes

DisputeGameFactory.t.sol

AnchorStateRegistry.t.sol

Test plan

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!

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!

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!

0xChin Apr 1, 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!

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!

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!

`DisputeGameFactory.t.sol`

`AnchorStateRegistry.t.sol`

0xChin Apr 1, 2026 •

edited

Loading