Skip to content

fix: StandardValidator incorrect checks #15977

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

Merged
smartcontracts merged 1 commit into from
May 16, 2025
+27 −106
Merged

fix: StandardValidator incorrect checks #15977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 9 additions & 22 deletions packages/contracts-bedrock/src/L1/StandardValidator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,18 +496,10 @@ contract StandardValidator {
_overrides,
"PDDG"
);
address _challenger = expectedChallenger(_overrides);
_errors = internalRequire(_game.challenger() == _challenger, "PDDG-120", _errors);

(Hash anchorRoot,) = _game.anchorStateRegistry().getAnchorRoot();
bytes32 _anchorRoot = Hash.unwrap(anchorRoot);

_errors = internalRequire(_anchorRoot != bytes32(0), "PDDG-130", _errors);
if (_anchorRoot == bytes32(hex"dead")) {
_errors = internalRequire(_game.l2SequenceNumber() == 0, "PDDG-140", _errors);
} else {
_errors = internalRequire(_game.l2SequenceNumber() != 0, "PDDG-150", _errors);
}
// Challenger is specific to the PermissionedDisputeGame contract.
address _challenger = expectedChallenger(_overrides);
_errors = internalRequire(_game.challenger() == _challenger, "PDDG-130", _errors);

return _errors;
}
Expand Down Expand Up @@ -548,12 +540,6 @@ contract StandardValidator {
"PLDG"
);

(Hash anchorRoot,) = _game.anchorStateRegistry().getAnchorRoot();
bytes32 _anchorRoot = Hash.unwrap(anchorRoot);
_errors = internalRequire(_anchorRoot != bytes32(0), "PLDG-130", _errors);
_errors = internalRequire(_anchorRoot != bytes32(hex"dead"), "PLDG-140", _errors);
_errors = internalRequire(_game.l2SequenceNumber() != 0, "PLDG-150", _errors);

return _errors;
}

Expand All @@ -574,21 +560,21 @@ contract StandardValidator {
view
returns (string memory)
{
bool validGameVM = address(_game.vm()) == address(mipsImpl);
IAnchorStateRegistry _asr = _game.anchorStateRegistry();
(Hash anchorRoot,) = _asr.getAnchorRoot();

_errors = internalRequire(
LibString.eq(_game.version(), permissionedDisputeGameVersion()), string.concat(_errorPrefix, "-20"), _errors
);
IAnchorStateRegistry _asr = _game.anchorStateRegistry();
_errors = internalRequire(
GameType.unwrap(_game.gameType()) == GameType.unwrap(_gameType), string.concat(_errorPrefix, "-30"), _errors
);
_errors = internalRequire(
Claim.unwrap(_game.absolutePrestate()) == _absolutePrestate, string.concat(_errorPrefix, "-40"), _errors
);
_errors = internalRequire(validGameVM, string.concat(_errorPrefix, "-50"), _errors);
_errors = internalRequire(address(_game.vm()) == mipsImpl, string.concat(_errorPrefix, "-50"), _errors);
_errors = internalRequire(_game.l2ChainId() == _l2ChainID, string.concat(_errorPrefix, "-60"), _errors);
_errors = internalRequire(_game.l2BlockNumber() == 0, string.concat(_errorPrefix, "-70"), _errors);
_errors = internalRequire(_game.l2SequenceNumber() == 0, string.concat(_errorPrefix, "-70"), _errors);
_errors = internalRequire(
Duration.unwrap(_game.clockExtension()) == 10800, string.concat(_errorPrefix, "-80"), _errors
);
Expand All @@ -597,13 +583,14 @@ contract StandardValidator {
_errors = internalRequire(
Duration.unwrap(_game.maxClockDuration()) == 302400, string.concat(_errorPrefix, "-110"), _errors
);
_errors = internalRequire(Hash.unwrap(anchorRoot) != bytes32(0), string.concat(_errorPrefix, "-120"), _errors);

_errors = assertValidDelayedWETH(_errors, _sysCfg, _game.weth(), _admin, _overrides, _errorPrefix);
_errors = assertValidAnchorStateRegistry(_errors, _sysCfg, _factory, _asr, _admin, _errorPrefix);

// Only assert valid preimage oracle if the game VM is valid, since otherwise
// the contract is likely to revert.
if (validGameVM) {
if (address(_game.vm()) == mipsImpl) {
_errors = assertValidPreimageOracle(_errors, _game.vm().oracle(), _errorPrefix);
}

Expand Down
102 changes: 18 additions & 84 deletions packages/contracts-bedrock/test/L1/StandardValidator.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,17 +202,6 @@ contract StandardValidator_TestInit is CommonTest {
vm.prank(disputeGameFactory.owner());
disputeGameFactory.setImplementation(GameTypes.CANNON, IDisputeGame(address(fdg)));
}

vm.mockCall(
address(disputeGameFactory.gameImpls(GameTypes.PERMISSIONED_CANNON)),
abi.encodeCall(IDisputeGame.l2SequenceNumber, ()),
abi.encode(1)
);
vm.mockCall(
address(disputeGameFactory.gameImpls(GameTypes.CANNON)),
abi.encodeCall(IDisputeGame.l2SequenceNumber, ()),
abi.encode(1)
);
}

/// @notice Runs the StandardValidator.validate function.
Expand Down Expand Up @@ -795,9 +784,9 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit {
}

/// @notice Tests that the validate function successfully returns the right error when the
/// PermissionedDisputeGame L2 Block Number is invalid.
function test_validate_permissionedDisputeGameInvalidL2BlockNumber_succeeds() public {
vm.mockCall(address(pdg), abi.encodeCall(IPermissionedDisputeGame.l2BlockNumber, ()), abi.encode(123));
/// PermissionedDisputeGame L2 Sequence Number is invalid.
function test_validate_permissionedDisputeGameInvalidL2SequenceNumber_succeeds() public {
vm.mockCall(address(pdg), abi.encodeCall(IDisputeGame.l2SequenceNumber, ()), abi.encode(123));
assertEq("PDDG-70", _validate(true));
}

Expand Down Expand Up @@ -834,76 +823,21 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit {
}

/// @notice Tests that the validate function successfully returns the right error when the
/// PermissionedDisputeGame challenger is invalid.
function test_validate_permissionedDisputeGameInvalidChallenger_succeeds() public {
vm.mockCall(address(pdg), abi.encodeCall(IPermissionedDisputeGame.challenger, ()), abi.encode(address(0xbad)));
assertEq("PDDG-120", _validate(true));
}

/// @notice Tests that the validate function successfully returns the right error when the anchor root is
/// bytes32(hex"dead") but the l2 sequence number is not 0.
function test_validate_0xdeadAnchorRootAndNonZeroSequenceNumber_succeeds() public {
uint256 randomSequenceNumber = 1; // this does not correspond to the sequence number of the games.
/// PermissionedDisputeGame anchor root is 0.
function test_validate_permissionedDisputeGameZeroAnchorRoot_succeeds() public {
vm.mockCall(
address(anchorStateRegistry),
abi.encodeCall(IAnchorStateRegistry.getAnchorRoot, ()),
abi.encode(bytes32(hex"dead"), randomSequenceNumber)
);
assertEq("PDDG-140,PLDG-140", _validate(true));
}

function test_validate_0xdeadAnchorRootAndZeroSequenceNumber_succeeds() public {
uint256 randomSequenceNumber = 1; // this does not correspond to the sequence number of the games.
vm.mockCall(
address(anchorStateRegistry),
abi.encodeCall(IAnchorStateRegistry.getAnchorRoot, ()),
abi.encode(bytes32(hex"dead"), randomSequenceNumber)
);

// Set the sequence number returned by the games to be 0
vm.mockCall(
address(disputeGameFactory.gameImpls(GameTypes.PERMISSIONED_CANNON)),
abi.encodeCall(IDisputeGame.l2SequenceNumber, ()),
abi.encode(0)
);
vm.mockCall(
address(disputeGameFactory.gameImpls(GameTypes.CANNON)),
abi.encodeCall(IDisputeGame.l2SequenceNumber, ()),
abi.encode(0)
abi.encode(bytes32(0), 1)
);
assertEq("PLDG-140,PLDG-150", _validate(true));
assertEq("PDDG-120,PLDG-120", _validate(true));
}

function test_validate_0x00AnchorRoot_succeeds() public {
vm.mockCall(
address(anchorStateRegistry),
abi.encodeCall(IAnchorStateRegistry.getAnchorRoot, ()),
abi.encode(bytes32(hex"00"), 1)
);
assertEq("PDDG-130,PLDG-130", _validate(true));
}

function test_validate_nonZeroOrDeadAnchorRootAndZeroSequenceNumber_succeeds() public {
uint256 randomSequenceNumber = 1; // this does not correspond to the sequence number of the games.
vm.mockCall(
address(anchorStateRegistry),
abi.encodeCall(IAnchorStateRegistry.getAnchorRoot, ()),
abi.encode(bytes32(hex"01"), randomSequenceNumber)
);

// Set the sequence number returned by the games to be 0
vm.mockCall(
address(disputeGameFactory.gameImpls(GameTypes.PERMISSIONED_CANNON)),
abi.encodeCall(IDisputeGame.l2SequenceNumber, ()),
abi.encode(0)
);
vm.mockCall(
address(disputeGameFactory.gameImpls(GameTypes.CANNON)),
abi.encodeCall(IDisputeGame.l2SequenceNumber, ()),
abi.encode(0)
);

assertEq("PDDG-150,PLDG-150", _validate(true));
/// @notice Tests that the validate function successfully returns the right error when the
/// PermissionedDisputeGame challenger is invalid.
function test_validate_permissionedDisputeGameInvalidChallenger_succeeds() public {
vm.mockCall(address(pdg), abi.encodeCall(IPermissionedDisputeGame.challenger, ()), abi.encode(address(0xbad)));
assertEq("PDDG-130", _validate(true));
}

/// @notice Tests that the validate function successfully returns the right overrides error when the
Expand All @@ -920,7 +854,7 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit {
function test_validateOverridesChallenger_permissionedDisputeGameInvalidChallenger_succeeds() public view {
IStandardValidator.ValidationOverrides memory overrides = _defaultValidationOverrides();
overrides.challenger = address(0xbad);
assertEq("OVERRIDES-CHALLENGER,PDDG-120", _validate(true, overrides));
assertEq("OVERRIDES-CHALLENGER,PDDG-130", _validate(true, overrides));
}

/// @notice Tests that the validate function successfully returns the right error when the
Expand Down Expand Up @@ -1138,9 +1072,9 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit {
}

/// @notice Tests that the validate function successfully returns the right error when the
/// FaultDisputeGame (permissionless) L2 Block Number is invalid.
function test_validate_faultDisputeGameInvalidL2BlockNumber_succeeds() public {
vm.mockCall(address(fdg), abi.encodeCall(IFaultDisputeGame.l2BlockNumber, ()), abi.encode(123));
/// FaultDisputeGame (permissionless) L2 Sequence Number is invalid.
function test_validate_faultDisputeGameInvalidL2SequenceNumber_succeeds() public {
vm.mockCall(address(fdg), abi.encodeCall(IDisputeGame.l2SequenceNumber, ()), abi.encode(123));
assertEq("PLDG-70", _validate(true));
}

Expand Down Expand Up @@ -1243,7 +1177,7 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit {
overrides.l1PAOMultisig = address(0xace);
overrides.challenger = address(0xbad);
assertEq(
"OVERRIDES-L1PAOMULTISIG,OVERRIDES-CHALLENGER,PROXYA-10,DF-30,PDDG-DWETH-30,PDDG-120,PLDG-DWETH-30",
"OVERRIDES-L1PAOMULTISIG,OVERRIDES-CHALLENGER,PROXYA-10,DF-30,PDDG-DWETH-30,PDDG-130,PLDG-DWETH-30",
_validate(true, overrides)
);
}
Expand Down Expand Up @@ -1275,7 +1209,7 @@ contract StandardValidator_validate_Test is StandardValidator_TestInit {

vm.expectRevert(
bytes(
"StandardValidator: OVERRIDES-L1PAOMULTISIG,OVERRIDES-CHALLENGER,PROXYA-10,DF-30,PDDG-DWETH-30,PDDG-120,PLDG-DWETH-30"
"StandardValidator: OVERRIDES-L1PAOMULTISIG,OVERRIDES-CHALLENGER,PROXYA-10,DF-30,PDDG-DWETH-30,PDDG-130,PLDG-DWETH-30"
)
);
_validate(false, overrides);
Expand Down