Simplify and modularize aggregate verifier + verifiers by roger-bai-coinbase · Pull Request #201 · base/contracts

roger-bai-coinbase · 2026-03-05T20:29:09Z

TEE proof can be challenged with a ZK proof using intermediary roots
Game can still progress after a TEE nullification or ZK nullification by providing a proof of the un-nulllified system
Game can not progress if both proof systems nullified
Bond for games that cannot progress are unlocked after 2 weeks from initialization time
Nullifying strictly increases resolution time to allow time to intervene
Challenges strictly increase resolution time to allow time to nullify the challenge
Nullification automatically disables the nullified proof system, preventing further proof verifications from any game

…andling into a single function, improving code clarity and maintainability. Update tests accordingly to reflect the new verification structure.

…onventions for clarity in Nullify tests.

…ild game in AggregateVerifier. Implement corresponding test case to verify this behavior.

…d rollup configuration. Update verification methods to use a journal hash instead of root claims. Modify MockVerifier and tests accordingly.

…tion

…date validation check to use this constant

…ifier

…ier, updating documentation and logic to clarify its purpose as the L2 sequence number.

…rnal to public, enhancing accessibility for contract interactions.

…fier to emit event upon credit transfer

…er for consistency with internal function naming conventions.

…nsolidating the return statement for the first dispute game scenario.

* Replace onchain Nitro cert verification with Automata ZK verifier, add ISemver to multiproof contracts, wire SystemConfigGlobal into standard deploy pipeline * Integrate multiproof config into standard DeployConfig, fix TEEVerifier proof format to match AggregateVerifier's l1head change * Fix fmt * Replace aws-nitro-enclave-attestation submodule with no-git dependency in Makefile * Consolidate deploy configs: migrate dev scripts to standard DeployConfig, add multiproof fields to sepolia.json and hardhat.json, remove separate nitro config files * Regenerate snapshots for updated and new multiproof contracts * Fix Initializable test: guard ETHLockbox entries for non-interop deploys, exclude AggregateVerifier (uses custom bool instead of OZ _initialized) * Add test-multiproof recipe to Justfile for CI * Address PR feedback: extract GameType local var, stricter pubKey check, iterate PCRs, add nitroEnclaveVerifier to Input, revert sepolia.json owner, remove redundant CI recipe * Resolve merge conflicts and regenerate semver-lock snapshots * Regenerate semver-lock with CI profile for correct bytecode hashes * Regenerate semver-lock with all compiler profiles including dispute * Regenerate semver-lock.json to remove stale dispute profile entries * Fix misleading TEEVerifier comment, require nitroEnclaveVerifier in deploy config, and parameterize hardcoded l2ChainID/block intervals in DeployImplementations * Reset semvar versioning in SystemConfigGlobal * Regenerate semver-lock.json for SystemConfigGlobal and TEEVerifier changes * correct SystemConfigGlobal.t.sol testInitialization() test cases to check correct version() number * use a proof threshold and allow ZK proofs after TEE nullification (#199) * use a proof threshold and allow ZK proofs after TEE nullification * pr feedback * update deployment scripts and tests * allow tee nullfiication when a zk proof exists. extend timestamp in this case to allow for zk nullification * Fix stack-too-deep in DeployImplementations and regenerate semver-lock * add multiproofProofThreshold to DeployConfig.s.sol to fix CI failures * Correct semver comment * Regenerate semver-lock following fix --------- Co-authored-by: roger-bai-coinbase <roger.bai@coinbase.com>

… systems

src/multiproof/Verifier.sol

leopoldjoy · 2026-03-05T22:55:33Z

@roger-bai-coinbase Nit: would it be possible to add a description to the PR? I started reviewing it but ran into some difficulty understanding the objectives (particularly in light of all of the different considerations we've been discussing of late).

roger-bai-coinbase · 2026-03-05T23:01:36Z

@roger-bai-coinbase Nit: would it be possible to add a description to the PR? I started reviewing it but ran into some difficulty understanding the objectives (particularly in light of all of the different considerations we've been discussing of late).

Done

cb-heimdall · 2026-03-06T18:06:31Z

🟡 Heimdall Review Status

Requirement Status More Info

Reviews

🟡 0/1

Denominator calculation

Show calculation

1 if user is bot 0

1 if user is external 0

2 if repo is sensitive 0

From .codeflow.yml 1

Additional review requirements

Show calculation

Max	0

0

From CODEOWNERS 0

Global minimum 0

Max 1

1

1 if commit is unverified 0

Sum 1

leopoldjoy · 2026-03-09T12:42:26Z

src/multiproof/Verifier.sol

@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: UNLICENSED


Nit: I think we should likely keep the same licensing for all contracts:

Suggested change

// SPDX-License-Identifier: UNLICENSED

// SPDX-License-Identifier: MIT

leopoldjoy · 2026-03-09T12:43:33Z

src/multiproof/AggregateVerifier.sol

    }

+    /// @notice Checks if the intermediate root index is valid and that the intermediate root differs from the proposed
+    /// intermediate root. @param intermediateRootIndex The index of the intermediate root to check.


Nit:

Suggested change

/// intermediate root. @param intermediateRootIndex The index of the intermediate root to check.

/// intermediate root.

/// @param intermediateRootIndex The index of the intermediate root to check.

leopoldjoy · 2026-03-09T12:45:25Z

src/multiproof/AggregateVerifier.sol

-        AggregateVerifier challengingGame = AggregateVerifier(address(game));
+        // Can only challenge with a ZK proof.
+        ProofType proofType = ProofType(uint8(proofBytes[0]));
+        if (proofType != ProofType.ZK) revert InvalidProofType();


Nit: in the case where this conditional evaluates to true, it would save gas to place it above the _checkIntermediateRoot check on line 492.

leopoldjoy · 2026-03-09T12:50:48Z

src/multiproof/Verifier.sol

+            !ANCHOR_STATE_REGISTRY.isGameProper(IDisputeGame(msg.sender))
+                || !ANCHOR_STATE_REGISTRY.isGameRespected(IDisputeGame(msg.sender))
+        ) revert NotProperGame();
+        nullified = true;


Just to confirm, since a single game's nullification permanently disables the verifier for all games (current and future) and no reversal mechanism exists, if a TEE verifier is nullified due to one bad game, for example, no new TEE proofs can ever be verified by any game until a new verifier is deployed and reconfigured, correct?

This seems correct to me, but want to categorically confirm.

leopoldjoy · 2026-03-09T12:55:05Z

src/multiproof/AggregateVerifier.sol


-        emit Proved(gameCreator(), proofType);
+        // Set the bond recipient to the creator. It can change if challenged successfully.
+        bondRecipient = gameCreator();


So if a challenger counters successfully but the game never resolves, the challenger doesn't get the bond, right? This behaviour seems correct to me, but wanted to confirm expectations.

leopoldjoy · 2026-03-09T13:40:50Z

src/multiproof/AggregateVerifier.sol

-        // Update the expected resolution.
-        _updateExpectedResolution();
+        // We purposely increase the resolution to allow for a ZK nullification.
+        expectedResolution = Timestamp.wrap(uint64(block.timestamp + SLOW_FINALIZATION_DELAY));


To confirm, this enables TEE-only resolution following ZK nullification by reverting to the SLOW_FINALIZATION_DELAY timeout (thus potentially causing a total finalisation delay of up to 2 weeks), correct?

Again, this aligns with my expectations of the system, but want to confirm.

leopoldjoy · 2026-03-09T13:48:04Z

src/multiproof/AggregateVerifier.sol

+    /// @dev Should only occur if challenged or nullified.
+    function _proofRefutedUpdate(ProofType proofType) internal {
+        delete proofTypeToProver[proofType];
+        proofCount -= 1;


Observation: proofCount can't underflow since the MissingProof check before nullification always prevents it. However, could it make sense to nonetheless explicitly guard against underflow in the function itself to improve robustness / resilience? Thoughts?

roger-bai-coinbase added 30 commits December 4, 2025 16:23

initial commit

5041935

Refactor tests and simplify verifier

bd550fd

Refactor proof verification methods to consolidate TEE and ZK proof h…

e1a7654

…andling into a single function, improving code clarity and maintainability. Update tests accordingly to reflect the new verification structure.

refactor tests and errors

2ea6dff

Prevent proofs from extending resolution time

70864f8

ZK proof allows game creator to immediately reclaim bond

2ef75e4

Additional tests

7e70a75

Refactor test cases to standardize warp duration and improve naming c…

ab2e400

…onventions for clarity in Nullify tests.

Add validation to ensure parent game has a proof before creating a ch…

aa2518d

…ild game in AggregateVerifier. Implement corresponding test case to verify this behavior.

Use calldata and remove unused variable

1613646

Make internal verify functions consistent

5ba6ed7

fix: can claim bond after nullifying a challenge

ab2d01b

prevent challenges if parent or game itself is invalid

8e38be6

Enhance AggregateVerifier to include hashes for TEE and ZK images, an…

3a2b31c

…d rollup configuration. Update verification methods to use a journal hash instead of root claims. Modify MockVerifier and tests accordingly.

Add ReentrancyGuard to AggregateVerifier and protect claimCredit func…

0a053f1

…tion

Add constant for initialize calldata size in AggregateVerifier and up…

308f591

…date validation check to use this constant

Add test to validate failure on invalid calldata size in AggregateVer…

9585f67

…ifier

Refactor l2BlockNumber function to l2SequenceNumber in AggregateVerif…

45c47f6

…ier, updating documentation and logic to clarify its purpose as the L2 sequence number.

Refactor visibility of state variables in AggregateVerifier from inte…

5cfe30b

…rnal to public, enhancing accessibility for contract interactions.

Moved wasRespectedGameTypeWhenCreated for storage optimization

b2dd1ff

Remove underscores from constructor variables

f0d3f54

IDIsputeGame compliance

5fcecd5

Remove implicit returns

ba44e13

Add CreditClaimed event and update claimCredit logic in AggregateVeri…

54c4192

…fier to emit event upon credit transfer

Rename getParentGameStatus to _getParentGameStatus in AggregateVerifi…

49e2eea

…er for consistency with internal function naming conventions.

Refactor return logic in getParentGameStatus to improve clarity by co…

2b3238d

…nsolidating the return statement for the first dispute game scenario.

solidity style guide

8d47e48

forge fmt

abe2710

Fix tests

c200cb7

remove unused imports

3fbec56

jjtny1 and others added 6 commits March 2, 2026 21:48

rm just test-multiproof

8e00905

Simplify AggregateVerifier and make it more modular for further proof…

cd2c74c

… systems

allow bond refund after 14 days

c9a3748

fix build and tests

1f81c50

forge fmt

b4e62b9

roger-bai-coinbase requested review from jackchuma, jjtny1 and leopoldjoy March 5, 2026 20:29

forge fmt

85e463b

jackchuma reviewed Mar 5, 2026

View reviewed changes

src/multiproof/Verifier.sol Outdated Show resolved Hide resolved

roger-bai-coinbase added 6 commits March 5, 2026 16:13

pr feedback

cd07d7c

semver lock

1f670f5

Clean up AggregateVerifier

250534c

Remove challenge after nullification

2d40f60

semver lock

51f36b2

Clean up

34fb1c0

roger-bai-coinbase added 3 commits March 5, 2026 18:17

semver lock

4009c97

fix: nullifying zk challenge has to use original intermediate root

86fa133

forge fmt

bd460e3

Base automatically changed from joby.thundil/CHAIN-3293-migrate-multiproof-contracts-to-basecontracts to main March 6, 2026 18:06

roger-bai-coinbase added 5 commits March 6, 2026 15:27

fix: use starting anchor root when there is no parent game

8c6c89c

Merge branch 'main' into simplify-and-modularize-aggregate-verifier

a81630d

merge from main

71f4f0c

forge clean and rebuild

f94c6a3

fix forge fmt

2e49e4c

leopoldjoy reviewed Mar 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify and modularize aggregate verifier + verifiers#201

Simplify and modularize aggregate verifier + verifiers#201
roger-bai-coinbase wants to merge 138 commits intomainfrom
simplify-and-modularize-aggregate-verifier

roger-bai-coinbase commented Mar 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

leopoldjoy commented Mar 5, 2026

Uh oh!

roger-bai-coinbase commented Mar 5, 2026

Uh oh!

cb-heimdall commented Mar 6, 2026 •

edited

Loading

Uh oh!

leopoldjoy Mar 9, 2026

Uh oh!

leopoldjoy Mar 9, 2026

Uh oh!

leopoldjoy Mar 9, 2026

Uh oh!

leopoldjoy Mar 9, 2026

Uh oh!

leopoldjoy Mar 9, 2026

Uh oh!

leopoldjoy Mar 9, 2026

Uh oh!

leopoldjoy Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	// SPDX-License-Identifier: UNLICENSED
	// SPDX-License-Identifier: MIT

	/// intermediate root. @param intermediateRootIndex The index of the intermediate root to check.
	/// intermediate root.
	/// @param intermediateRootIndex The index of the intermediate root to check.

Conversation

roger-bai-coinbase commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

leopoldjoy commented Mar 5, 2026

Uh oh!

roger-bai-coinbase commented Mar 5, 2026

Uh oh!

cb-heimdall commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Heimdall Review Status

Uh oh!

leopoldjoy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

leopoldjoy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

leopoldjoy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

leopoldjoy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

leopoldjoy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

leopoldjoy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

leopoldjoy Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

roger-bai-coinbase commented Mar 5, 2026 •

edited

Loading

cb-heimdall commented Mar 6, 2026 •

edited

Loading