Skip to content

Multiproof audit fixes #227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roger-bai-coinbase wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Multiproof audit fixes #227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dd587c9
If the parent is invalid, the child is automatically invalid (Finding 1)
roger-bai-coinbase Mar 25, 2026
eb82db7
Update NitroEnclaveVerifier events (Finding 2)
roger-bai-coinbase Mar 25, 2026
488feae
Update challenge comment (Finding 3)
roger-bai-coinbase Mar 25, 2026
2007b31
Refactor duplicate code (Finding 4)
roger-bai-coinbase Mar 25, 2026
8cbc65b
semver lock
roger-bai-coinbase Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions snapshots/semver-lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,12 @@
"sourceCodeHash": "0x955bd0c9b47e43219865e4e92abf28d916c96de20cbdf2f94c8ab14d02083759"
},
"src/multiproof/AggregateVerifier.sol:AggregateVerifier": {
"initCodeHash": "0xe631e0c2e8e86711f83cb97884065ce38d1ff519e0d2dbf8704e7e4c183a56cc",
"sourceCodeHash": "0xfa0464c07c06fddc98ba20e9a362ba10ecf94496556d0f7ac88d1986f79a8a6b"
"initCodeHash": "0x73f657f5df4491ba169a48becd1b4ef6902cf6221fa509e10efff0407c460c61",
"sourceCodeHash": "0xfc7d78c3ad110de584c8e1c34cda9547e09e9368a1dd8823680f5e734156a532"
},
"src/multiproof/AggregateVerifier.sol:AggregateVerifier:dispute": {
"initCodeHash": "0x89f15344142ede7ca9d6af3c998fb3ffec84293105b28daa8427aa0608dd1d9e",
"sourceCodeHash": "0xfa0464c07c06fddc98ba20e9a362ba10ecf94496556d0f7ac88d1986f79a8a6b"
"initCodeHash": "0x01de5ce7cb737465f4e413e959c60d68184c50b31423bfe769fc129ca776d6e9",
"sourceCodeHash": "0xfc7d78c3ad110de584c8e1c34cda9547e09e9368a1dd8823680f5e734156a532"
},
"src/multiproof/tee/TEEProverRegistry.sol:TEEProverRegistry": {
"initCodeHash": "0x4c89ecad0d48b6da64ef7f489326aae63b7fbcd33f4fed949a496afd1be49009",
Expand Down
51 changes: 27 additions & 24 deletions src/multiproof/AggregateVerifier.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,25 +437,24 @@ contract AggregateVerifier is Clone, ReentrancyGuard, ISemver {
// The parent game must have resolved.
if (parentGameStatus == GameStatus.IN_PROGRESS) revert ParentGameNotResolved();

bool isChallenged = counteredByIntermediateRootIndexPlusOne > 0;

// If the parent game's claim is invalid, blacklisted, or retired, then the current game's claim is invalid.
// We don't care about what happens in this game once the parent is invalid.
if (parentGameStatus == GameStatus.CHALLENGER_WINS) {
status = GameStatus.CHALLENGER_WINS;
} else {
// Game must be completed with a valid proof.
// Game must be completed with a valid proof and enough proofs.
if (!gameOver()) revert GameNotOver();
// If the game is challenged, status is CHALLENGER_WINS.
// If the game is not challenged, status is DEFENDER_WINS.
status = isChallenged ? GameStatus.CHALLENGER_WINS : GameStatus.DEFENDER_WINS;
if (proofCount < PROOF_THRESHOLD) revert NotEnoughProofs();

// If the game is challenged, reward the challenger.
if (counteredByIntermediateRootIndexPlusOne > 0) {
status = GameStatus.CHALLENGER_WINS;
bondRecipient = proofTypeToProver[ProofType.ZK];
} else {
status = GameStatus.DEFENDER_WINS;
}
}

if (proofCount < PROOF_THRESHOLD) revert NotEnoughProofs();

// Default bond recipient is the creator. We only change if successfully challenged.
if (isChallenged) {
bondRecipient = proofTypeToProver[ProofType.ZK];
}
// Mark the game as resolved.
resolvedAt = Timestamp.wrap(uint64(block.timestamp));
emit Resolved(status);
Expand Down Expand Up @@ -516,6 +515,8 @@ contract AggregateVerifier is Clone, ReentrancyGuard, ISemver {
// This is only in case the ZK proof is nullified, which would lower the proof count.
// If the ZK is nullified, we allow the remaining TEE proof to resolve.
// The expected resolution time can no longer be increased as both proof types have been submitted.
// The exception is if the ZK proof is nullified, in which case the expected resolution will be
// increased by SLOW_FINALIZATION_DELAY from the time of nullification.
proofCount += 1;

// We purposely increase the resolution to allow for a ZK nullification.
Expand Down Expand Up @@ -759,13 +760,9 @@ contract AggregateVerifier is Clone, ReentrancyGuard, ISemver {

/// @notice Decreases the expected resolution timestamp.
function _decreaseExpectedResolution() internal {
uint64 delay;
uint64 delay = _getDelay();

if (proofCount >= 2) {
delay = FAST_FINALIZATION_DELAY;
} else if (proofCount == 1) {
delay = SLOW_FINALIZATION_DELAY;
} else {
if (delay == type(uint64).max) {
// If there are no proofs, don't allow the game to resolve.
expectedResolution = Timestamp.wrap(type(uint64).max);
return;
Expand All @@ -790,13 +787,9 @@ contract AggregateVerifier is Clone, ReentrancyGuard, ISemver {
}

function _increaseExpectedResolution() internal {
uint64 delay;
uint64 delay = _getDelay();

if (proofCount >= 2) {
delay = FAST_FINALIZATION_DELAY;
} else if (proofCount == 1) {
delay = SLOW_FINALIZATION_DELAY;
} else {
if (delay == type(uint64).max) {
// If there are no proofs, don't allow the game to resolve.
expectedResolution = Timestamp.wrap(type(uint64).max);
return;
Expand All @@ -808,6 +801,16 @@ contract AggregateVerifier is Clone, ReentrancyGuard, ISemver {
expectedResolution = Timestamp.wrap(uint64(block.timestamp) + delay);
}

function _getDelay() internal view returns (uint64) {
if (proofCount >= 2) {
return FAST_FINALIZATION_DELAY;
} else if (proofCount == 1) {
return SLOW_FINALIZATION_DELAY;
} else {
return type(uint64).max;
}
}

function _verifyProof(
bytes calldata proofBytes,
ProofType proofType,
Expand Down
6 changes: 3 additions & 3 deletions src/multiproof/tee/NitroEnclaveVerifier.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
event ZkRouteWasFrozen(ZkCoProcessorType indexed zkCoProcessor, bytes4 indexed selector);

/// @dev Emitted when the proof of attestation has been successfully verified
event AttestationSubmitted(VerificationResult result, ZkCoProcessorType zkCoProcessor, bytes output);
event AttestationSubmitted(VerificationResult result, ZkCoProcessorType indexed zkCoProcessor, bytes output);

/// @dev Emitted when a batched proof has been successfully verified; encodedBatched = abi.encode(VerifierJournal[])
event BatchAttestationSubmitted(bytes32 verifierId, ZkCoProcessorType zkCoProcessor, bytes encodedBatch);
event BatchAttestationSubmitted(bytes32 verifierId, ZkCoProcessorType indexed zkCoProcessor, bytes encodedBatch);

/// @dev Event emitted when the proof submitter address is changed
event ProofSubmitterChanged(address newProofSubmitter);
Expand Down Expand Up @@ -554,7 +554,7 @@ contract NitroEnclaveVerifier is Ownable, INitroEnclaveVerifier {
_verifyZk(zkCoprocessor, programId, output, proofBytes);
journal = abi.decode(output, (VerifierJournal));
journal = _verifyJournal(journal);
emit AttestationSubmitted(journal.result, zkCoprocessor, output);
emit AttestationSubmitted(journal.result, zkCoprocessor, abi.encode(journal));
}

/**
Expand Down
Loading