From 5733899d190306225b935da9d29a92ac6e769a99 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Fri, 10 Jan 2025 19:37:34 -0600 Subject: [PATCH 01/28] feat: incident response improvements First half of the original incident response improvements PR. Co-authored-by: wildmolasses --- packages/contracts-bedrock/foundry.toml | 2 +- .../dispute/IAnchorStateRegistry.sol | 17 +- .../interfaces/dispute/IDelayedWETH.sol | 2 +- .../interfaces/dispute/IDisputeGame.sol | 2 + .../interfaces/dispute/IFaultDisputeGame.sol | 12 +- .../dispute/IPermissionedDisputeGame.sol | 12 +- .../snapshots/abi/AnchorStateRegistry.json | 110 ++++- .../snapshots/abi/DelayedWETH.json | 32 +- .../snapshots/abi/FaultDisputeGame.json | 110 +++++ .../abi/PermissionedDisputeGame.json | 110 +++++ .../snapshots/semver-lock.json | 22 +- .../storageLayout/FaultDisputeGame.json | 28 ++ .../PermissionedDisputeGame.json | 28 ++ .../src/L1/OptimismPortal2.sol | 22 +- .../src/dispute/AnchorStateRegistry.sol | 117 +++-- .../src/dispute/DelayedWETH.sol | 20 +- .../src/dispute/FaultDisputeGame.sol | 128 ++++- .../src/dispute/lib/Errors.sol | 12 + .../src/dispute/lib/Types.sol | 11 + .../test/dispute/AnchorStateRegistry.t.sol | 462 ++++-------------- .../test/dispute/DelayedWETH.t.sol | 34 +- .../test/dispute/FaultDisputeGame.t.sol | 181 ++++++- .../test/invariants/FaultDisputeGame.t.sol | 10 + .../test/safe/DeputyGuardianModule.t.sol | 5 + .../test/universal/Specs.t.sol | 1 + 25 files changed, 981 insertions(+), 509 deletions(-) diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index 940eeaa3c40bf..09af861272e80 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -11,7 +11,7 @@ build_info_path = 'artifacts/build-info' snapshots = 'notarealpath' # workaround for foundry#9477 optimizer = true -optimizer_runs = 999999 +optimizer_runs = 5000 extra_output = ['devdoc', 'userdoc', 'metadata', 'storageLayout'] bytecode_hash = 'none' diff --git a/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol b/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol index 9fdbfa7b42202..bf4f88e552f5b 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol @@ -10,7 +10,6 @@ import { GameType, Hash, OutputRoot } from "src/dispute/lib/Types.sol"; interface IAnchorStateRegistry { error AnchorStateRegistry_Unauthorized(); - error AnchorStateRegistry_ImproperAnchorGame(); error AnchorStateRegistry_InvalidAnchorGame(); event AnchorNotUpdated(IFaultDisputeGame indexed game); @@ -21,16 +20,26 @@ interface IAnchorStateRegistry { function anchors(GameType) external view returns (Hash, uint256); function getAnchorRoot() external view returns (Hash, uint256); function disputeGameFactory() external view returns (IDisputeGameFactory); - function initialize(ISuperchainConfig _superchainConfig, IDisputeGameFactory _disputeGameFactory, IOptimismPortal2 _portal, OutputRoot memory _startingAnchorRoot) external; + function initialize( + ISuperchainConfig _superchainConfig, + IDisputeGameFactory _disputeGameFactory, + IOptimismPortal2 _portal, + OutputRoot memory _startingAnchorRoot + ) + external; function isGameRegistered(IDisputeGame _game) external view returns (bool); function isGameBlacklisted(IDisputeGame _game) external view returns (bool); function isGameRespected(IDisputeGame _game) external view returns (bool); function isGameRetired(IDisputeGame _game) external view returns (bool); + function isGameResolved(IDisputeGame _game) external view returns (bool); + function isGameBeyondAirgap(IDisputeGame _game) external view returns (bool); function isGameProper(IDisputeGame _game) external view returns (bool); + function isGameFinalized(IDisputeGame _game) external view returns (bool); + function isGameClaimValid(IDisputeGame _game) external view returns (bool); function portal() external view returns (IOptimismPortal2); - function setAnchorState(IFaultDisputeGame _game) external; + function respectedGameType() external view returns (GameType); + function setAnchorState(IDisputeGame _game) external; function superchainConfig() external view returns (ISuperchainConfig); - function tryUpdateAnchorState() external; function version() external view returns (string memory); function __constructor__() external; diff --git a/packages/contracts-bedrock/interfaces/dispute/IDelayedWETH.sol b/packages/contracts-bedrock/interfaces/dispute/IDelayedWETH.sol index 63ffa49919b63..7917b2cf6c510 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IDelayedWETH.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IDelayedWETH.sol @@ -11,13 +11,13 @@ interface IDelayedWETH { event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); event Initialized(uint8 version); - event Unwrap(address indexed src, uint256 wad); fallback() external payable; receive() external payable; function config() external view returns (ISuperchainConfig); function delay() external view returns (uint256); + function hold(address _guy) external; function hold(address _guy, uint256 _wad) external; function initialize(address _owner, ISuperchainConfig _config) external; function owner() external view returns (address); diff --git a/packages/contracts-bedrock/interfaces/dispute/IDisputeGame.sol b/packages/contracts-bedrock/interfaces/dispute/IDisputeGame.sol index 2f79cc79d3276..37653759875ed 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IDisputeGame.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IDisputeGame.sol @@ -14,7 +14,9 @@ interface IDisputeGame is IInitializable { function gameCreator() external pure returns (address creator_); function rootClaim() external pure returns (Claim rootClaim_); function l1Head() external pure returns (Hash l1Head_); + function l2BlockNumber() external pure returns (uint256 l2BlockNumber_); function extraData() external pure returns (bytes memory extraData_); function resolve() external returns (GameStatus status_); function gameData() external view returns (GameType gameType_, Claim rootClaim_, bytes memory extraData_); + function wasRespectedGameTypeWhenCreated() external view returns (bool); } diff --git a/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol b/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol index 038bb55998f87..4d8a9f7d8f984 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol @@ -6,7 +6,7 @@ import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol"; import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.sol"; import { IBigStepper } from "interfaces/dispute/IBigStepper.sol"; import { Types } from "src/libraries/Types.sol"; -import { GameType, Claim, Position, Clock, Hash, Duration } from "src/dispute/lib/Types.sol"; +import { GameType, Claim, Position, Clock, Hash, Duration, BondDistributionMode } from "src/dispute/lib/Types.sol"; interface IFaultDisputeGame is IDisputeGame { struct ClaimData { @@ -74,13 +74,19 @@ interface IFaultDisputeGame is IDisputeGame { error UnexpectedRootClaim(Claim rootClaim); error UnexpectedString(); error ValidStep(); + error InvalidBondDistributionMode(); + error GameNotFinalized(string reason); + error GameNotResolved(); + error ReservedGameType(); event Move(uint256 indexed parentIndex, Claim indexed claim, address indexed claimant); + event GameClosed(BondDistributionMode bondDistributionMode); function absolutePrestate() external view returns (Claim absolutePrestate_); function addLocalData(uint256 _ident, uint256 _execLeafIdx, uint256 _partOffset) external; function anchorStateRegistry() external view returns (IAnchorStateRegistry registry_); function attack(Claim _disputed, uint256 _parentIndex, Claim _claim) external payable; + function bondDistributionMode() external view returns (BondDistributionMode); function challengeRootL2Block(Types.OutputRootProof memory _outputRootProof, bytes memory _headerRLP) external; function claimCredit(address _recipient) external; function claimData(uint256) @@ -98,11 +104,13 @@ interface IFaultDisputeGame is IDisputeGame { function claimDataLen() external view returns (uint256 len_); function claims(Hash) external view returns (bool); function clockExtension() external view returns (Duration clockExtension_); + function closeGame() external; function credit(address) external view returns (uint256); function defend(Claim _disputed, uint256 _parentIndex, Claim _claim) external payable; function getChallengerDuration(uint256 _claimIndex) external view returns (Duration duration_); function getNumToResolve(uint256 _claimIndex) external view returns (uint256 numRemainingChildren_); function getRequiredBond(Position _position) external view returns (uint256 requiredBond_); + function hasUnlockedCredit(address) external view returns (bool); function l2BlockNumber() external pure returns (uint256 l2BlockNumber_); function l2BlockNumberChallenged() external view returns (bool); function l2BlockNumberChallenger() external view returns (address); @@ -110,6 +118,7 @@ interface IFaultDisputeGame is IDisputeGame { function maxClockDuration() external view returns (Duration maxClockDuration_); function maxGameDepth() external view returns (uint256 maxGameDepth_); function move(Claim _disputed, uint256 _challengeIndex, Claim _claim, bool _isAttack) external payable; + function refundModeCredit(address) external view returns (uint256); function resolutionCheckpoints(uint256) external view @@ -124,6 +133,7 @@ interface IFaultDisputeGame is IDisputeGame { function subgames(uint256, uint256) external view returns (uint256); function version() external view returns (string memory); function vm() external view returns (IBigStepper vm_); + function wasRespectedGameTypeWhenCreated() external view returns (bool); function weth() external view returns (IDelayedWETH weth_); function __constructor__(GameConstructorParams memory _params) external; diff --git a/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol b/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol index c9d26d70a6ca3..8aa4e8307765a 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; import { Types } from "src/libraries/Types.sol"; -import { Claim, Position, Clock, Hash, Duration } from "src/dispute/lib/Types.sol"; +import { Claim, Position, Clock, Hash, Duration, BondDistributionMode } from "src/dispute/lib/Types.sol"; import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.sol"; import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol"; @@ -63,13 +63,19 @@ interface IPermissionedDisputeGame is IDisputeGame { error UnexpectedRootClaim(Claim rootClaim); error UnexpectedString(); error ValidStep(); + error InvalidBondDistributionMode(); + error GameNotFinalized(string reason); + error GameNotResolved(); + error ReservedGameType(); event Move(uint256 indexed parentIndex, Claim indexed claim, address indexed claimant); + event GameClosed(BondDistributionMode bondDistributionMode); function absolutePrestate() external view returns (Claim absolutePrestate_); function addLocalData(uint256 _ident, uint256 _execLeafIdx, uint256 _partOffset) external; function anchorStateRegistry() external view returns (IAnchorStateRegistry registry_); function attack(Claim _disputed, uint256 _parentIndex, Claim _claim) external payable; + function bondDistributionMode() external view returns (BondDistributionMode); function challengeRootL2Block(Types.OutputRootProof memory _outputRootProof, bytes memory _headerRLP) external; function claimCredit(address _recipient) external; function claimData(uint256) @@ -87,11 +93,13 @@ interface IPermissionedDisputeGame is IDisputeGame { function claimDataLen() external view returns (uint256 len_); function claims(Hash) external view returns (bool); function clockExtension() external view returns (Duration clockExtension_); + function closeGame() external; function credit(address) external view returns (uint256); function defend(Claim _disputed, uint256 _parentIndex, Claim _claim) external payable; function getChallengerDuration(uint256 _claimIndex) external view returns (Duration duration_); function getNumToResolve(uint256 _claimIndex) external view returns (uint256 numRemainingChildren_); function getRequiredBond(Position _position) external view returns (uint256 requiredBond_); + function hasUnlockedCredit(address) external view returns (bool); function l2BlockNumber() external pure returns (uint256 l2BlockNumber_); function l2BlockNumberChallenged() external view returns (bool); function l2BlockNumberChallenger() external view returns (address); @@ -99,6 +107,7 @@ interface IPermissionedDisputeGame is IDisputeGame { function maxClockDuration() external view returns (Duration maxClockDuration_); function maxGameDepth() external view returns (uint256 maxGameDepth_); function move(Claim _disputed, uint256 _challengeIndex, Claim _claim, bool _isAttack) external payable; + function refundModeCredit(address) external view returns (uint256); function resolutionCheckpoints(uint256) external view @@ -113,6 +122,7 @@ interface IPermissionedDisputeGame is IDisputeGame { function subgames(uint256, uint256) external view returns (uint256); function version() external view returns (string memory); function vm() external view returns (IBigStepper vm_); + function wasRespectedGameTypeWhenCreated() external view returns (bool); function weth() external view returns (IDelayedWETH weth_); error BadAuth(); diff --git a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json index c28e085cc7809..d0f829fd66fc2 100644 --- a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json +++ b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json @@ -112,6 +112,25 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "contract IDisputeGame", + "name": "_game", + "type": "address" + } + ], + "name": "isGameBeyondAirgap", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -131,6 +150,54 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "contract IDisputeGame", + "name": "_game", + "type": "address" + } + ], + "name": "isGameClaimValid", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + }, + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "contract IDisputeGame", + "name": "_game", + "type": "address" + } + ], + "name": "isGameFinalized", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + }, + { + "internalType": "string", + "name": "", + "type": "string" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -169,6 +236,25 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "contract IDisputeGame", + "name": "_game", + "type": "address" + } + ], + "name": "isGameResolved", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -220,10 +306,23 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "respectedGameType", + "outputs": [ + { + "internalType": "GameType", + "name": "", + "type": "uint32" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { - "internalType": "contract IFaultDisputeGame", + "internalType": "contract IDisputeGame", "name": "_game", "type": "address" } @@ -246,13 +345,6 @@ "stateMutability": "view", "type": "function" }, - { - "inputs": [], - "name": "tryUpdateAnchorState", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - }, { "inputs": [], "name": "version", @@ -320,4 +412,4 @@ "name": "AnchorStateRegistry_Unauthorized", "type": "error" } -] \ No newline at end of file +] diff --git a/packages/contracts-bedrock/snapshots/abi/DelayedWETH.json b/packages/contracts-bedrock/snapshots/abi/DelayedWETH.json index a6f0dd660468f..654794f0a8453 100644 --- a/packages/contracts-bedrock/snapshots/abi/DelayedWETH.json +++ b/packages/contracts-bedrock/snapshots/abi/DelayedWETH.json @@ -149,6 +149,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "_guy", + "type": "address" + } + ], + "name": "hold", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { @@ -497,25 +510,6 @@ "name": "Transfer", "type": "event" }, - { - "anonymous": false, - "inputs": [ - { - "indexed": true, - "internalType": "address", - "name": "src", - "type": "address" - }, - { - "indexed": false, - "internalType": "uint256", - "name": "wad", - "type": "uint256" - } - ], - "name": "Unwrap", - "type": "event" - }, { "anonymous": false, "inputs": [ diff --git a/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json b/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json index a2f02cce13bd8..2d6ac86ea18a9 100644 --- a/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json @@ -134,6 +134,19 @@ "stateMutability": "payable", "type": "function" }, + { + "inputs": [], + "name": "bondDistributionMode", + "outputs": [ + { + "internalType": "enum BondDistributionMode", + "name": "", + "type": "uint8" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -281,6 +294,13 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "closeGame", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [], "name": "createdAt", @@ -455,6 +475,25 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "hasUnlockedCredit", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "initialize", @@ -581,6 +620,25 @@ "stateMutability": "payable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "refundModeCredit", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -839,6 +897,19 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "wasRespectedGameTypeWhenCreated", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "weth", @@ -852,6 +923,19 @@ "stateMutability": "view", "type": "function" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "enum BondDistributionMode", + "name": "bondDistributionMode", + "type": "uint8" + } + ], + "name": "GameClosed", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -960,16 +1044,37 @@ "name": "GameDepthExceeded", "type": "error" }, + { + "inputs": [ + { + "internalType": "string", + "name": "reason", + "type": "string" + } + ], + "name": "GameNotFinalized", + "type": "error" + }, { "inputs": [], "name": "GameNotInProgress", "type": "error" }, + { + "inputs": [], + "name": "GameNotResolved", + "type": "error" + }, { "inputs": [], "name": "IncorrectBondAmount", "type": "error" }, + { + "inputs": [], + "name": "InvalidBondDistributionMode", + "type": "error" + }, { "inputs": [], "name": "InvalidChallengePeriod", @@ -1045,6 +1150,11 @@ "name": "OutOfOrderResolution", "type": "error" }, + { + "inputs": [], + "name": "ReservedGameType", + "type": "error" + }, { "inputs": [], "name": "UnexpectedList", diff --git a/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json b/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json index eebc4adf16ea5..8da34d0ce2fb7 100644 --- a/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json @@ -144,6 +144,19 @@ "stateMutability": "payable", "type": "function" }, + { + "inputs": [], + "name": "bondDistributionMode", + "outputs": [ + { + "internalType": "enum BondDistributionMode", + "name": "", + "type": "uint8" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -304,6 +317,13 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "closeGame", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [], "name": "createdAt", @@ -478,6 +498,25 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "hasUnlockedCredit", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "initialize", @@ -617,6 +656,25 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "refundModeCredit", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -875,6 +933,19 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [], + "name": "wasRespectedGameTypeWhenCreated", + "outputs": [ + { + "internalType": "bool", + "name": "", + "type": "bool" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "weth", @@ -888,6 +959,19 @@ "stateMutability": "view", "type": "function" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "enum BondDistributionMode", + "name": "bondDistributionMode", + "type": "uint8" + } + ], + "name": "GameClosed", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -1001,16 +1085,37 @@ "name": "GameDepthExceeded", "type": "error" }, + { + "inputs": [ + { + "internalType": "string", + "name": "reason", + "type": "string" + } + ], + "name": "GameNotFinalized", + "type": "error" + }, { "inputs": [], "name": "GameNotInProgress", "type": "error" }, + { + "inputs": [], + "name": "GameNotResolved", + "type": "error" + }, { "inputs": [], "name": "IncorrectBondAmount", "type": "error" }, + { + "inputs": [], + "name": "InvalidBondDistributionMode", + "type": "error" + }, { "inputs": [], "name": "InvalidChallengePeriod", @@ -1086,6 +1191,11 @@ "name": "OutOfOrderResolution", "type": "error" }, + { + "inputs": [], + "name": "ReservedGameType", + "type": "error" + }, { "inputs": [], "name": "UnexpectedList", diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index eda6a227616a4..f277374154109 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -20,12 +20,12 @@ "sourceCodeHash": "0x2d21506cc51ebe0b60bcf89883aff5e9b1269567ce44ee779de3d3940e23fb65" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x2121a97875875150106a54a71c6c4c03afe90b3364e416be047f55fdeab57204", - "sourceCodeHash": "0x96e3de3ef0025a6def702eeb481acd2d2d88971fd418be657472f51a98029773" + "initCodeHash": "0x52ddbe70f60a6f4e5c929fed417c92d99d82a85c05de8e1f0c8dfd0d634ad648", + "sourceCodeHash": "0xc4a66e8adacd58b961938f473b22614a520fa0bdae70381b8f56210bc34f1f32" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x09ffe45f91bf59315b9fd4a2941b819ed8b1bb0d8643a630c6193bd67acea0ed", - "sourceCodeHash": "0xbb6acc3e88af9594ffcb8a2f30860511b76e09024330e70052316668fe55fd1f" + "initCodeHash": "0x6b05d7130c019d3c46dca39dfdae3797861088a9fb7b5baa84fd4e5cb87c5883", + "sourceCodeHash": "0xae2fbe02c0f8685692babeed0252ae8a624dc6d3bfb082fc3807d7b84869004b" }, "src/L1/ProtocolVersions.sol": { "initCodeHash": "0x0000ec89712d8b4609873f1ba76afffd4205bf9110818995c90134dbec12e91e", @@ -152,20 +152,20 @@ "sourceCodeHash": "0xb7b0a06cd971c4647247dc19ce997d0c64a73e87c81d30731da9cf9efa1b952a" }, "src/dispute/AnchorStateRegistry.sol": { - "initCodeHash": "0xfbeeac40d86d13e71c7add66eef6357576a93b6a175c9cff6ec6ef587fe3acc4", - "sourceCodeHash": "0xbb2e08da74d470fc30dd35dc39834e19f676a45974aa2403eb97e84bc5bed0a8" + "initCodeHash": "0x6e94c2f4129209d2b36d6a54adaaf99b91b3df322c8c75043d29f6b0a9143738", + "sourceCodeHash": "0xb81181263522e939195a70b6e281fb038de2312c9682f9123723c6ae1babf559" }, "src/dispute/DelayedWETH.sol": { - "initCodeHash": "0x759d7f9c52b7c13ce4502f39dae3a75d130c6278240cde0b60ae84616aa2bd48", - "sourceCodeHash": "0x4406c78e0557bedb88b4ee5977acb1ef13e7bd92b7dbf79f56f8bad95c53e229" + "initCodeHash": "0xb1f04c9ee86984a157b92a18754c84104e9d4df7a3838633301ca7f557d0220a", + "sourceCodeHash": "0x0162302b9c71f184d45bee34ecfb1dfbf427f38fc5652709ab7ffef1ac816d82" }, "src/dispute/DisputeGameFactory.sol": { "initCodeHash": "0xa728192115c5fdb08c633a0899043318289b1d413d7afeed06356008b2a5a7fa", "sourceCodeHash": "0x155c0334f63616ed245aadf9a94f419ef7d5e2237b3b32172484fd19890a61dc" }, "src/dispute/FaultDisputeGame.sol": { - "initCodeHash": "0x423e8488731c0b0f87b435174f412c09fbf0b17eb0b8c9a03efa37d779ec0cae", - "sourceCodeHash": "0xe53b970922b309ada1c59f94d5935ffca669e909c797f17ba8a3d309c487e7e8" + "initCodeHash": "0x734c4bce3822851f8925d3a8668630e4c503ee5a042e1234e448f4e9ea86582c", + "sourceCodeHash": "0x8888b471b484c3f0662dca39317aa75fbb380deea4e12f6a8ccd5dde1202e5c7" }, "src/legacy/DeployerWhitelist.sol": { "initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29", @@ -219,4 +219,4 @@ "initCodeHash": "0x2bfce526f82622288333d53ca3f43a0a94306ba1bab99241daa845f8f4b18bd4", "sourceCodeHash": "0xf49d7b0187912a6bb67926a3222ae51121e9239495213c975b3b4b217ee57a1b" } -} \ No newline at end of file +} diff --git a/packages/contracts-bedrock/snapshots/storageLayout/FaultDisputeGame.json b/packages/contracts-bedrock/snapshots/storageLayout/FaultDisputeGame.json index ec02f23d2e6d2..ea8c9de176223 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/FaultDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/FaultDisputeGame.json @@ -89,5 +89,33 @@ "offset": 0, "slot": "8", "type": "struct OutputRoot" + }, + { + "bytes": "1", + "label": "wasRespectedGameTypeWhenCreated", + "offset": 0, + "slot": "10", + "type": "bool" + }, + { + "bytes": "32", + "label": "refundModeCredit", + "offset": 0, + "slot": "11", + "type": "mapping(address => uint256)" + }, + { + "bytes": "32", + "label": "hasUnlockedCredit", + "offset": 0, + "slot": "12", + "type": "mapping(address => bool)" + }, + { + "bytes": "1", + "label": "bondDistributionMode", + "offset": 0, + "slot": "13", + "type": "enum BondDistributionMode" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/storageLayout/PermissionedDisputeGame.json b/packages/contracts-bedrock/snapshots/storageLayout/PermissionedDisputeGame.json index ec02f23d2e6d2..ea8c9de176223 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/PermissionedDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/PermissionedDisputeGame.json @@ -89,5 +89,33 @@ "offset": 0, "slot": "8", "type": "struct OutputRoot" + }, + { + "bytes": "1", + "label": "wasRespectedGameTypeWhenCreated", + "offset": 0, + "slot": "10", + "type": "bool" + }, + { + "bytes": "32", + "label": "refundModeCredit", + "offset": 0, + "slot": "11", + "type": "mapping(address => uint256)" + }, + { + "bytes": "32", + "label": "hasUnlockedCredit", + "offset": 0, + "slot": "12", + "type": "mapping(address => bool)" + }, + { + "bytes": "1", + "label": "bondDistributionMode", + "offset": 0, + "slot": "13", + "type": "enum BondDistributionMode" } ] \ No newline at end of file diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 1456e19cc18b8..0d27297d7f40e 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -476,9 +476,16 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { /// @param _gameType The game type to consult for output proposals. function setRespectedGameType(GameType _gameType) external { if (msg.sender != guardian()) revert Unauthorized(); - respectedGameType = _gameType; - respectedGameTypeUpdatedAt = uint64(block.timestamp); - emit RespectedGameTypeSet(_gameType, Timestamp.wrap(respectedGameTypeUpdatedAt)); + // respectedGameTypeUpdatedAt is now no longer set by default. We want to avoid modifying + // this function's signature as that would result in changes to the DeputyGuardianModule. + // We use type(uint32).max as a temporary solution to allow us to update the + // respectedGameTypeUpdatedAt timestamp without modifying this function's signature. + if (_gameType.raw() == type(uint32).max) { + respectedGameTypeUpdatedAt = uint64(block.timestamp); + } else { + respectedGameType = _gameType; + emit RespectedGameTypeSet(_gameType, Timestamp.wrap(respectedGameTypeUpdatedAt)); + } } /// @notice Checks if a withdrawal can be finalized. This function will revert if the withdrawal cannot be @@ -518,10 +525,11 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { // from finalizing withdrawals proven against non-finalized output roots. if (disputeGameProxy.status() != GameStatus.DEFENDER_WINS) revert ProposalNotValidated(); - // The game type of the dispute game must be the respected game type. This was also checked in - // `proveWithdrawalTransaction`, but we check it again in case the respected game type has changed since - // the withdrawal was proven. - if (disputeGameProxy.gameType().raw() != respectedGameType.raw()) revert InvalidGameType(); + // The game type of the dispute game must have been the respected game type at creation + // time. We check that the game type is the respected game type at proving time, but it's + // possible that the respected game type has since changed. Users can still use this game + // to finalize a withdrawal as long as it has not been otherwise invalidated. + if (!disputeGameProxy.wasRespectedGameTypeWhenCreated()) revert InvalidGameType(); // The game must have been created after `respectedGameTypeUpdatedAt`. This is to prevent users from creating // invalid disputes against a deployed game type while the off-chain challenge agents are not watching. diff --git a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol index a1a3d5f3f7c62..d646b66849ace 100644 --- a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol +++ b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol @@ -52,9 +52,6 @@ contract AnchorStateRegistry is Initializable, ISemver { /// @notice Thrown when an unauthorized caller attempts to set the anchor state. error AnchorStateRegistry_Unauthorized(); - /// @notice Thrown when an improper anchor game is provided. - error AnchorStateRegistry_ImproperAnchorGame(); - /// @notice Thrown when an invalid anchor game is provided. error AnchorStateRegistry_InvalidAnchorGame(); @@ -83,6 +80,12 @@ contract AnchorStateRegistry is Initializable, ISemver { startingAnchorRoot = _startingAnchorRoot; } + /// @notice Returns the respected game type. + /// @return The respected game type. + function respectedGameType() public view returns (GameType) { + return portal.respectedGameType(); + } + /// @custom:legacy /// @notice Returns the anchor root. Note that this is a legacy deprecated function and will /// be removed in a future release. Use getAnchorRoot() instead. Anchor roots are no @@ -123,7 +126,7 @@ contract AnchorStateRegistry is Initializable, ISemver { /// @param _game The game to check. /// @return Whether the game is of a respected game type. function isGameRespected(IDisputeGame _game) public view returns (bool) { - return _game.gameType().raw() == portal.respectedGameType().raw(); + return _game.wasRespectedGameTypeWhenCreated(); } /// @notice Determines whether a game is blacklisted. @@ -142,6 +145,21 @@ contract AnchorStateRegistry is Initializable, ISemver { return _game.createdAt().raw() < portal.respectedGameTypeUpdatedAt(); } + /// @notice Returns whether a game is resolved. + /// @param _game The game to check. + /// @return Whether the game is resolved. + function isGameResolved(IDisputeGame _game) public view returns (bool) { + return _game.resolvedAt().raw() != 0 + && (_game.status() == GameStatus.DEFENDER_WINS || _game.status() == GameStatus.CHALLENGER_WINS); + } + + /// @notice Returns whether a game is beyond the airgap period. + /// @param _game The game to check. + /// @return Whether the game is beyond the airgap period. + function isGameBeyondAirgap(IDisputeGame _game) public view returns (bool) { + return block.timestamp - _game.resolvedAt().raw() > portal.disputeGameFinalityDelaySeconds(); + } + /// @notice **READ THIS FUNCTION DOCUMENTATION CAREFULLY.** /// Determines whether a game resolved properly and the game was not subject to any /// invalidation conditions. The root claim of a proper game IS NOT guaranteed to be @@ -180,60 +198,71 @@ contract AnchorStateRegistry is Initializable, ISemver { return true; } - /// @notice Allows FaultDisputeGame contracts to attempt to become the new anchor game. A game - /// can only become the new anchor game if it is not invalid (it is a Proper Game), it - /// resolved in favor of the root claim, and it is newer than the current anchor game. - function tryUpdateAnchorState() external { - // Grab the game. - IFaultDisputeGame game = IFaultDisputeGame(msg.sender); - - // Check if the game is a proper game. - if (!isGameProper(game)) { - emit AnchorNotUpdated(game); - return; - } - - // Must be a game that resolved in favor of the state. - if (game.status() != GameStatus.DEFENDER_WINS) { - emit AnchorNotUpdated(game); - return; + /// @notice Returns whether a game is finalized. + /// @param _game The game to check. + /// @return Whether the game is finalized. + function isGameFinalized(IDisputeGame _game) public view returns (bool) { + // Game must be resolved. + if (!isGameResolved(_game)) { + return false; } - // Must be newer than the current anchor game. - (, uint256 anchorL2BlockNumber) = getAnchorRoot(); - if (game.l2BlockNumber() <= anchorL2BlockNumber) { - emit AnchorNotUpdated(game); - return; + // Game must be beyond the airgap period. + if (!isGameBeyondAirgap(_game)) { + return false; } - // Update the anchor game. - anchorGame = game; - emit AnchorUpdated(game); + return true; } - /// @notice Sets the anchor state given the game. Can only be triggered by the Guardian - /// address. Unlike tryUpdateAnchorState(), this function does not check if the - /// provided is newer than the existing anchor game. This allows the Guardian to - /// recover from situations in which the current anchor game is invalid. - /// @param _game The game to set the anchor state for. - function setAnchorState(IFaultDisputeGame _game) external { - // Function can only be triggered by the guardian. - if (msg.sender != superchainConfig.guardian()) { - revert AnchorStateRegistry_Unauthorized(); + /// @notice Returns whether a game's root claim is valid. + /// @param _game The game to check. + /// @return Whether the game's root claim is valid. + function isGameClaimValid(IDisputeGame _game) public view returns (bool) { + // Game must be a proper game. + bool properGame = isGameProper(_game); + if (!properGame) { + return false; } - // Check if the game is a proper game. - if (!isGameProper(_game)) { - revert AnchorStateRegistry_ImproperAnchorGame(); + // Game must be finalized. + bool finalized = isGameFinalized(_game); + if (!finalized) { + return false; } - // The game must have resolved in favor of the root claim. + // Game must be resolved in favor of the defender. if (_game.status() != GameStatus.DEFENDER_WINS) { + return false; + } + + return true; + } + + /// @notice Updates the anchor game. + /// @param _game New candidate anchor game. + function setAnchorState(IDisputeGame _game) public { + // Convert game to FaultDisputeGame. + // We can't use FaultDisputeGame in the interface because this function is called from the + // FaultDisputeGame contract which can't import IFaultDisputeGame by convention. We should + // likely introduce a new interface (e.g., StateDisputeGame) that can act as a more useful + // version of IDisputeGame in the future. + IFaultDisputeGame game = IFaultDisputeGame(address(_game)); + + // Check if the candidate game is valid. + bool valid = isGameClaimValid(game); + if (!valid) { + revert AnchorStateRegistry_InvalidAnchorGame(); + } + + // Must be newer than the current anchor game. + (, uint256 anchorL2BlockNumber) = getAnchorRoot(); + if (game.l2BlockNumber() <= anchorL2BlockNumber) { revert AnchorStateRegistry_InvalidAnchorGame(); } // Update the anchor game. - anchorGame = _game; - emit AnchorUpdated(_game); + anchorGame = game; + emit AnchorUpdated(game); } } diff --git a/packages/contracts-bedrock/src/dispute/DelayedWETH.sol b/packages/contracts-bedrock/src/dispute/DelayedWETH.sol index b6fc0720ae76a..89cf70f89b72a 100644 --- a/packages/contracts-bedrock/src/dispute/DelayedWETH.sol +++ b/packages/contracts-bedrock/src/dispute/DelayedWETH.sol @@ -26,14 +26,9 @@ contract DelayedWETH is OwnableUpgradeable, WETH98, ISemver { uint256 timestamp; } - /// @notice Emitted when an unwrap is started. - /// @param src The address that started the unwrap. - /// @param wad The amount of WETH that was unwrapped. - event Unwrap(address indexed src, uint256 wad); - /// @notice Semantic version. - /// @custom:semver 1.2.0-beta.5 - string public constant version = "1.2.0-beta.5"; + /// @custom:semver 1.3.0-beta.1 + string public constant version = "1.3.0-beta.1"; /// @notice Returns a withdrawal request for the given address. mapping(address => mapping(address => WithdrawalRequest)) public withdrawals; @@ -107,12 +102,19 @@ contract DelayedWETH is OwnableUpgradeable, WETH98, ISemver { require(success, "DelayedWETH: recover failed"); } - /// @notice Allows the owner to recover from error cases by pulling ETH from a specific owner. + /// @notice Allows the owner to recover from error cases by pulling all WETH from a specific owner. + /// @param _guy The address to recover the WETH from. + function hold(address _guy) external { + hold(_guy, balanceOf(_guy)); + } + + /// @notice Allows the owner to recover from error cases by pulling a specific amount of WETH from a specific owner. /// @param _guy The address to recover the WETH from. /// @param _wad The amount of WETH to recover. - function hold(address _guy, uint256 _wad) external { + function hold(address _guy, uint256 _wad) public { require(msg.sender == owner(), "DelayedWETH: not owner"); _allowance[_guy][msg.sender] = _wad; emit Approval(_guy, msg.sender, _wad); + transferFrom(_guy, msg.sender, _wad); } } diff --git a/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol b/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol index 02a2cee3ca834..f9ae25ac273f1 100644 --- a/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol +++ b/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol @@ -11,6 +11,7 @@ import { RLPReader } from "src/libraries/rlp/RLPReader.sol"; import { GameStatus, GameType, + BondDistributionMode, Claim, Clock, Duration, @@ -51,7 +52,11 @@ import { BondTransferFailed, NoCreditToClaim, InvalidOutputRootProof, - ClaimAboveSplit + ClaimAboveSplit, + GameNotFinalized, + InvalidBondDistributionMode, + GameNotResolved, + ReservedGameType } from "src/dispute/lib/Errors.sol"; // Interfaces @@ -59,6 +64,7 @@ import { ISemver } from "interfaces/universal/ISemver.sol"; import { IDelayedWETH } from "interfaces/dispute/IDelayedWETH.sol"; import { IBigStepper, IPreimageOracle } from "interfaces/dispute/IBigStepper.sol"; import { IAnchorStateRegistry } from "interfaces/dispute/IAnchorStateRegistry.sol"; +import { IDisputeGame } from "interfaces/dispute/IDisputeGame.sol"; /// @title FaultDisputeGame /// @notice An implementation of the `IFaultDisputeGame` interface. @@ -115,6 +121,9 @@ contract FaultDisputeGame is Clone, ISemver { /// @param claimant The address of the claimant event Move(uint256 indexed parentIndex, Claim indexed claim, address indexed claimant); + /// @notice Emitted when the game is closed. + event GameClosed(BondDistributionMode bondDistributionMode); + //////////////////////////////////////////////////////////////// // State Vars // //////////////////////////////////////////////////////////////// @@ -161,8 +170,8 @@ contract FaultDisputeGame is Clone, ISemver { uint256 internal constant HEADER_BLOCK_NUMBER_INDEX = 8; /// @notice Semantic version. - /// @custom:semver 1.3.1-beta.9 - string public constant version = "1.3.1-beta.9"; + /// @custom:semver 1.4.0-beta.1 + string public constant version = "1.4.0-beta.1"; /// @notice The starting timestamp of the game Timestamp public createdAt; @@ -204,6 +213,18 @@ contract FaultDisputeGame is Clone, ISemver { /// @notice The latest finalized output root, serving as the anchor for output bisection. OutputRoot public startingOutputRoot; + /// @notice A boolean for whether or not the game type was respected when the game was created. + bool public wasRespectedGameTypeWhenCreated; + + /// @notice A mapping of each claimant's refund mode credit. + mapping(address => uint256) public refundModeCredit; + + /// @notice A mapping of whether a claimant has unlocked their credit. + mapping(address => bool) public hasUnlockedCredit; + + /// @notice The bond distribution mode of the game. + BondDistributionMode public bondDistributionMode; + /// @param _params Parameters for creating a new FaultDisputeGame. constructor(GameConstructorParams memory _params) { // The max game depth may not be greater than `LibPosition.MAX_POSITION_BITLEN - 1`. @@ -239,6 +260,10 @@ contract FaultDisputeGame is Clone, ISemver { // The maximum clock extension may not be greater than the maximum clock duration. if (uint64(maxClockExtension) > _params.maxClockDuration.raw()) revert InvalidClockExtension(); + // Block type(uint32).max from being used as a game type so that it can be used in the + // OptimismPortal respected game type trick. + if (_params.gameType.raw() == type(uint32).max) revert ReservedGameType(); + // Set up initial game state. GAME_TYPE = _params.gameType; ABSOLUTE_PRESTATE = _params.absolutePrestate; @@ -320,10 +345,15 @@ contract FaultDisputeGame is Clone, ISemver { initialized = true; // Deposit the bond. + refundModeCredit[gameCreator()] += msg.value; WETH.deposit{ value: msg.value }(); // Set the game's starting timestamp createdAt = Timestamp.wrap(uint64(block.timestamp)); + + // Set whether the game type was respected when the game was created. + wasRespectedGameTypeWhenCreated = + GameType.unwrap(ANCHOR_STATE_REGISTRY.respectedGameType()) == GameType.unwrap(GAME_TYPE); } //////////////////////////////////////////////////////////////// @@ -531,6 +561,7 @@ contract FaultDisputeGame is Clone, ISemver { subgames[_challengeIndex].push(claimData.length - 1); // Deposit the bond. + refundModeCredit[msg.sender] += msg.value; WETH.deposit{ value: msg.value }(); // Emit the appropriate event for the attack or defense. @@ -695,9 +726,6 @@ contract FaultDisputeGame is Clone, ISemver { // Update the status and emit the resolved event, note that we're performing an assignment here. emit Resolved(status = status_); - - // Try to update the anchor state, this should not revert. - ANCHOR_STATE_REGISTRY.tryUpdateAnchorState(); } /// @notice Resolves the subgame rooted at the given claim index. `_numToResolve` specifies how many children of @@ -913,16 +941,43 @@ contract FaultDisputeGame is Clone, ISemver { requiredBond_ = assumedBaseFee * requiredGas; } - /// @notice Claim the credit belonging to the recipient address. + /// @notice Claim the credit belonging to the recipient address. Reverts if the game isn't + /// finalized, if the recipient has no credit to claim, or if the bond transfer + /// fails. If the game is finalized but no bond has been paid out yet, this method + /// will determine the bond distribution mode and also try to update anchor game. /// @param _recipient The owner and recipient of the credit. function claimCredit(address _recipient) external { - // Remove the credit from the recipient prior to performing the external call. - uint256 recipientCredit = credit[_recipient]; - credit[_recipient] = 0; + // Close out the game and determine the bond distribution mode if not already set. + // We call this as part of claim credit to reduce the number of additional calls that a + // Challenger needs to make to this contract. + closeGame(); + + // Fetch the recipient's credit balance based on the bond distribution mode. + uint256 recipientCredit; + if (bondDistributionMode == BondDistributionMode.REFUND) { + recipientCredit = refundModeCredit[_recipient]; + } else if (bondDistributionMode == BondDistributionMode.NORMAL) { + recipientCredit = credit[_recipient]; + } else { + // We shouldn't get here, but sanity check just in case. + revert InvalidBondDistributionMode(); + } + + // If the game is in refund mode, and the recipient has not unlocked their refund mode + // credit, we unlock it and return early. + if (!hasUnlockedCredit[_recipient]) { + hasUnlockedCredit[_recipient] = true; + WETH.unlock(_recipient, recipientCredit); + return; + } // Revert if the recipient has no credit to claim. if (recipientCredit == 0) revert NoCreditToClaim(); + // Set the recipient's credit balances to 0. + refundModeCredit[_recipient] = 0; + credit[_recipient] = 0; + // Try to withdraw the WETH amount so it can be used here. WETH.withdraw(_recipient, recipientCredit); @@ -931,6 +986,50 @@ contract FaultDisputeGame is Clone, ISemver { if (!success) revert BondTransferFailed(); } + /// @notice Closes out the game, determines the bond distribution mode, attempts to register + /// the game as the anchor game, and emits an event. + function closeGame() public { + // If the bond distribution mode has already been determined, we can return early. + if (bondDistributionMode == BondDistributionMode.REFUND || bondDistributionMode == BondDistributionMode.NORMAL) + { + // We can't revert or we'd break claimCredit(). + return; + } else if (bondDistributionMode != BondDistributionMode.UNDECIDED) { + // We shouldn't get here, but sanity check just in case. + revert InvalidBondDistributionMode(); + } + + // Make sure that the game is resolved. + // AnchorStateRegistry should be checking this but we're being defensive here. + if (resolvedAt.raw() == 0) { + revert GameNotResolved(); + } + + // Game must be finalized according to the AnchorStateRegistry. + bool finalized = ANCHOR_STATE_REGISTRY.isGameFinalized(IDisputeGame(address(this))); + if (!finalized) { + revert GameNotFinalized(); + } + + // Try to update the anchor game first. Won't always succeed because delays can lead + // to situations in which this game might not be eligible to be a new anchor game. + try ANCHOR_STATE_REGISTRY.setAnchorState(IDisputeGame(address(this))) { } catch { } + + // Check if the game is a proper game, which will determine the bond distribution mode. + bool properGame = ANCHOR_STATE_REGISTRY.isGameProper(IDisputeGame(address(this))); + + // If the game is a proper game, the bonds should be distributed normally. Otherwise, go + // into refund mode and distribute bonds back to their original depositors. + if (properGame) { + bondDistributionMode = BondDistributionMode.NORMAL; + } else { + bondDistributionMode = BondDistributionMode.REFUND; + } + + // Emit an event to signal that the game has been closed. + emit GameClosed(bondDistributionMode); + } + /// @notice Returns the amount of time elapsed on the potential challenger to `_claimIndex`'s chess clock. Maxes /// out at `MAX_CLOCK_DURATION`. /// @param _claimIndex The index of the subgame root claim. @@ -1018,14 +1117,7 @@ contract FaultDisputeGame is Clone, ISemver { /// @param _recipient The recipient of the bond. /// @param _bonded The claim to pay out the bond of. function _distributeBond(address _recipient, ClaimData storage _bonded) internal { - // Set all bits in the bond value to indicate that the bond has been paid out. - uint256 bond = _bonded.bond; - - // Increase the recipient's credit. - credit[_recipient] += bond; - - // Unlock the bond. - WETH.unlock(_recipient, bond); + credit[_recipient] += _bonded.bond; } /// @notice Verifies the integrity of an execution bisection subgame's root claim. Reverts if the claim diff --git a/packages/contracts-bedrock/src/dispute/lib/Errors.sol b/packages/contracts-bedrock/src/dispute/lib/Errors.sol index f662945404087..6ef1d282ba670 100644 --- a/packages/contracts-bedrock/src/dispute/lib/Errors.sol +++ b/packages/contracts-bedrock/src/dispute/lib/Errors.sol @@ -121,6 +121,18 @@ error BlockNumberMatches(); /// @notice Thrown when the L2 block number claim has already been challenged. error L2BlockNumberChallenged(); +/// @notice Thrown when the game is not yet finalized. +error GameNotFinalized(); + +/// @notice Thrown when an invalid bond distribution mode is supplied. +error InvalidBondDistributionMode(); + +/// @notice Thrown when the game is not yet resolved. +error GameNotResolved(); + +/// @notice Thrown when a reserved game type is used. +error ReservedGameType(); + //////////////////////////////////////////////////////////////// // `PermissionedDisputeGame` Errors // //////////////////////////////////////////////////////////////// diff --git a/packages/contracts-bedrock/src/dispute/lib/Types.sol b/packages/contracts-bedrock/src/dispute/lib/Types.sol index 74106888fb059..45a11b91d376b 100644 --- a/packages/contracts-bedrock/src/dispute/lib/Types.sol +++ b/packages/contracts-bedrock/src/dispute/lib/Types.sol @@ -26,6 +26,17 @@ enum GameStatus { DEFENDER_WINS } +/// @notice The game's bond distribution type. Games are expected to start in the `UNDECIDED` +/// state, and then choose either `NORMAL` or `REFUND`. +enum BondDistributionMode { + // Bond distribution strategy has not been chosen. + UNDECIDED, + // Bonds should be distributed as normal. + NORMAL, + // Bonds should be refunded to claimants. + REFUND +} + /// @notice Represents an L2 output root and the L2 block number at which it was generated. /// @custom:field root The output root. /// @custom:field l2BlockNumber The L2 block number at which the output root was generated. diff --git a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol index a624a89025425..72fe2be04a205 100644 --- a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol +++ b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol @@ -143,12 +143,8 @@ contract AnchorStateRegistry_IsGameBlacklisted_Test is AnchorStateRegistry_Init contract AnchorStateRegistry_IsGameRespected_Test is AnchorStateRegistry_Init { /// @notice Tests that isGameRespected will return true if the game is of the respected game type. function test_isGameRespected_isRespected_succeeds() public { - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); assertTrue(anchorStateRegistry.isGameRespected(gameProxy)); } @@ -160,10 +156,11 @@ contract AnchorStateRegistry_IsGameRespected_Test is AnchorStateRegistry_Init { _gameType = GameType.wrap(_gameType.raw() + 1); } - // Make our game type NOT the respected game type. + // Mock that the game was not respected. vm.mockCall( - address(optimismPortal2), abi.encodeCall(optimismPortal2.respectedGameType, ()), abi.encode(_gameType) + address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(false) ); + assertFalse(anchorStateRegistry.isGameRespected(gameProxy)); } } @@ -203,12 +200,8 @@ contract AnchorStateRegistry_IsGameRetired_Test is AnchorStateRegistry_Init { contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { /// @notice Tests that isGameProper will return true if the game meets all conditions. function test_isGameProper_meetsAllConditions_succeeds() public { - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); assertTrue(anchorStateRegistry.isGameProper(gameProxy)); } @@ -233,9 +226,9 @@ contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { _gameType = GameType.wrap(_gameType.raw() + 1); } - // Make our game type NOT the respected game type. + // Mock that the game was not respected. vm.mockCall( - address(optimismPortal2), abi.encodeCall(optimismPortal2.respectedGameType, ()), abi.encode(_gameType) + address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(false) ); assertFalse(anchorStateRegistry.isGameProper(gameProxy)); @@ -269,12 +262,12 @@ contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { } } -contract AnchorStateRegistry_TryUpdateAnchorState_Test is AnchorStateRegistry_Init { - /// @notice Tests that tryUpdateAnchorState will succeed if the game is valid, the game block +contract AnchorStateRegistry_SetAnchorState_Test is AnchorStateRegistry_Init { + /// @notice Tests that setAnchorState will succeed if the game is valid, the game block /// number is greater than the current anchor root block number, and the game is the /// currently respected game type. /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_tryUpdateAnchorState_validNewerState_succeeds(uint256 _l2BlockNumber) public { + function testFuzz_setAnchorState_validNewerState_succeeds(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); @@ -287,18 +280,18 @@ contract AnchorStateRegistry_TryUpdateAnchorState_Test is AnchorStateRegistry_In // Mock the DEFENDER_WINS state. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); + + // Mock the resolvedAt timestamp and fast forward to beyond the delay. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); // Update the anchor state. vm.prank(address(gameProxy)); vm.expectEmit(address(anchorStateRegistry)); emit AnchorUpdated(gameProxy); - anchorStateRegistry.tryUpdateAnchorState(); + anchorStateRegistry.setAnchorState(gameProxy); // Confirm that the anchor state is now the same as the game state. (root, l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); @@ -309,11 +302,13 @@ contract AnchorStateRegistry_TryUpdateAnchorState_Test is AnchorStateRegistry_In IFaultDisputeGame anchorGame = anchorStateRegistry.anchorGame(); assertEq(address(anchorGame), address(gameProxy)); } +} - /// @notice Tests that tryUpdateAnchorState will not update the anchor state if the game block +contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init { + /// @notice Tests that setAnchorState will revert if the game is valid and the game block /// number is less than or equal to the current anchor root block number. /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_tryUpdateAnchorState_validOlderStateNoUpdate_succeeds(uint256 _l2BlockNumber) public { + function testFuzz_setAnchorState_validOlderState_fails(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); @@ -326,132 +321,17 @@ contract AnchorStateRegistry_TryUpdateAnchorState_Test is AnchorStateRegistry_In // Mock the DEFENDER_WINS state. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); - - // Try to update the anchor state. - vm.prank(address(gameProxy)); - vm.expectEmit(address(anchorStateRegistry)); - emit AnchorNotUpdated(gameProxy); - anchorStateRegistry.tryUpdateAnchorState(); - - // Confirm that the anchor state has not updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - assertEq(updatedL2BlockNumber, l2BlockNumber); - assertEq(updatedRoot.raw(), root.raw()); - } - - /// @notice Tests that tryUpdateAnchorState will not update the anchor state if the game is not - /// registered. - /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_tryUpdateAnchorState_notFactoryRegisteredGameNoUpdate_succeeds(uint256 _l2BlockNumber) public { - // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - - // Bound the new block number. - _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber, type(uint256).max); - - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - - // Mock the DEFENDER_WINS state. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); - - // Mock the DisputeGameFactory to make it seem that the game was not registered. - vm.mockCall( - address(disputeGameFactory), - abi.encodeCall( - disputeGameFactory.games, (gameProxy.gameType(), gameProxy.rootClaim(), gameProxy.extraData()) - ), - abi.encode(address(0), 0) - ); - - // Try to update the anchor state. - vm.prank(address(gameProxy)); - vm.expectEmit(address(anchorStateRegistry)); - emit AnchorNotUpdated(gameProxy); - anchorStateRegistry.tryUpdateAnchorState(); - - // Confirm that the anchor state has not updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - assertEq(updatedL2BlockNumber, l2BlockNumber); - assertEq(updatedRoot.raw(), root.raw()); - } - - /// @notice Tests that tryUpdateAnchorState will not update the anchor state if the game status - /// is CHALLENGER_WINS. - /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_tryUpdateAnchorState_challengerWinsNoUpdate_succeeds(uint256 _l2BlockNumber) public { - // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - - // Bound the new block number. - _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber, type(uint256).max); - - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - - // Mock the CHALLENGER_WINS state. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.CHALLENGER_WINS)); - - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); - - // Try to update the anchor state. - vm.prank(address(gameProxy)); - vm.expectEmit(address(anchorStateRegistry)); - emit AnchorNotUpdated(gameProxy); - anchorStateRegistry.tryUpdateAnchorState(); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); - // Confirm that the anchor state has not updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - assertEq(updatedL2BlockNumber, l2BlockNumber); - assertEq(updatedRoot.raw(), root.raw()); - } - - /// @notice Tests that tryUpdateAnchorState will not update the anchor state if the game status - /// is IN_PROGRESS. - /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_tryUpdateAnchorState_inProgressNoUpdate_succeeds(uint256 _l2BlockNumber) public { - // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - - // Bound the new block number. - _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber, type(uint256).max); - - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - - // Mock the CHALLENGER_WINS state. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.IN_PROGRESS)); - - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock the resolvedAt timestamp and fast forward to beyond the delay. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); // Try to update the anchor state. vm.prank(address(gameProxy)); - vm.expectEmit(address(anchorStateRegistry)); - emit AnchorNotUpdated(gameProxy); - anchorStateRegistry.tryUpdateAnchorState(); + vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_InvalidAnchorGame.selector); + anchorStateRegistry.setAnchorState(gameProxy); // Confirm that the anchor state has not updated. (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.getAnchorRoot(); @@ -459,157 +339,6 @@ contract AnchorStateRegistry_TryUpdateAnchorState_Test is AnchorStateRegistry_In assertEq(updatedRoot.raw(), root.raw()); } - /// @notice Tests that tryUpdateAnchorState will not update the anchor state if the game type - /// is not the respected game type. - /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_tryUpdateAnchorState_notRespectedGameTypeNoUpdate_succeeds(uint256 _l2BlockNumber) public { - // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); - - // Bound the new block number. - _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber, type(uint256).max); - - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - - // Mock the DEFENDER_WINS state. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - - // Mock the respectedGameType call so that it does NOT match our game type. - vm.mockCall(address(optimismPortal2), abi.encodeCall(optimismPortal2.respectedGameType, ()), abi.encode(999)); - - // Try to update the anchor state. - vm.prank(address(gameProxy)); - vm.expectEmit(address(anchorStateRegistry)); - emit AnchorNotUpdated(gameProxy); - anchorStateRegistry.tryUpdateAnchorState(); - - // Confirm that the anchor state has not updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); - assertEq(updatedL2BlockNumber, l2BlockNumber); - assertEq(updatedRoot.raw(), root.raw()); - } - - /// @notice Tests that tryUpdateAnchorState will not update the anchor state if the game is - /// blacklisted. - /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_tryUpdateAnchorState_blacklistedGameNoUpdate_succeeds(uint256 _l2BlockNumber) public { - // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - - // Bound the new block number. - _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber + 1, type(uint256).max); - - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - - // Mock the DEFENDER_WINS state. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); - - // Mock the disputeGameBlacklist call to return true. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.disputeGameBlacklist, (gameProxy)), - abi.encode(true) - ); - - // Update the anchor state. - vm.prank(address(gameProxy)); - vm.expectEmit(address(anchorStateRegistry)); - emit AnchorNotUpdated(gameProxy); - anchorStateRegistry.tryUpdateAnchorState(); - - // Confirm that the anchor state has not updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); - assertEq(updatedL2BlockNumber, l2BlockNumber); - assertEq(updatedRoot.raw(), root.raw()); - } - - /// @notice Tests that tryUpdateAnchorState will not update the anchor state if the game is - /// retired. - /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_tryUpdateAnchorState_retiredGameNoUpdate_succeeds(uint256 _l2BlockNumber) public { - // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - - // Bound the new block number. - _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber + 1, type(uint256).max); - - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - - // Mock the DEFENDER_WINS state. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); - - // Mock the respectedGameTypeUpdatedAt call to be later than the game's creation time. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameTypeUpdatedAt, ()), - abi.encode(gameProxy.createdAt().raw() + 1) - ); - - // Update the anchor state. - vm.prank(address(gameProxy)); - vm.expectEmit(address(anchorStateRegistry)); - emit AnchorNotUpdated(gameProxy); - anchorStateRegistry.tryUpdateAnchorState(); - - // Confirm that the anchor state has not updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); - assertEq(updatedL2BlockNumber, l2BlockNumber); - assertEq(updatedRoot.raw(), root.raw()); - } -} - -contract AnchorStateRegistry_SetAnchorState_Test is AnchorStateRegistry_Init { - /// @notice Tests that setAnchorState will succeed with a game with any L2 block number as long - /// as the game is valid and is the currently respected game type. - /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_setAnchorState_anyL2BlockNumber_succeeds(uint256 _l2BlockNumber) public { - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - - // Mock the DEFENDER_WINS state. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); - - // Set the anchor state. - vm.prank(superchainConfig.guardian()); - vm.expectEmit(address(anchorStateRegistry)); - emit AnchorUpdated(gameProxy); - anchorStateRegistry.setAnchorState(gameProxy); - - // Confirm that the anchor state has updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); - assertEq(updatedL2BlockNumber, gameProxy.l2BlockNumber()); - assertEq(updatedRoot.raw(), gameProxy.rootClaim().raw()); - - // Confirm that the anchor game is now set. - IFaultDisputeGame anchorGame = anchorStateRegistry.anchorGame(); - assertEq(address(anchorGame), address(gameProxy)); - } -} - contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init { /// @notice Tests that setAnchorState will revert if the sender is not the guardian. /// @param _sender The address of the sender. @@ -657,18 +386,17 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init // Grab block number of the existing anchor root. (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); + // Bound the new block number. + _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber, type(uint256).max); + // Mock the l2BlockNumber call. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); // Mock the DEFENDER_WINS state. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); // Mock the DisputeGameFactory to make it seem that the game was not registered. vm.mockCall( @@ -681,37 +409,40 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init // Try to update the anchor state. vm.prank(superchainConfig.guardian()); - vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_ImproperAnchorGame.selector); + vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_InvalidAnchorGame.selector); anchorStateRegistry.setAnchorState(gameProxy); // Confirm that the anchor state has not updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); + (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.getAnchorRoot(); assertEq(updatedL2BlockNumber, l2BlockNumber); assertEq(updatedRoot.raw(), root.raw()); } - /// @notice Tests that setAnchorState will revert if the game is valid and the game status is - /// CHALLENGER_WINS. + /// @notice Tests that setAnchorState will revert if the game is valid and the game status + /// is CHALLENGER_WINS. /// @param _l2BlockNumber The L2 block number to use for the game. function testFuzz_setAnchorState_challengerWins_fails(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); + // Bound the new block number. + _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber, type(uint256).max); + // Mock the l2BlockNumber call. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); // Mock the CHALLENGER_WINS state. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.CHALLENGER_WINS)); - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); + + // Mock the resolvedAt timestamp and fast forward to beyond the delay. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); // Try to update the anchor state. - vm.prank(superchainConfig.guardian()); + vm.prank(address(gameProxy)); vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_InvalidAnchorGame.selector); anchorStateRegistry.setAnchorState(gameProxy); @@ -721,8 +452,8 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init assertEq(updatedRoot.raw(), root.raw()); } - /// @notice Tests that setAnchorState will revert if the game is valid and the game status is - /// IN_PROGRESS. + /// @notice Tests that setAnchorState will revert if the game is valid and the game status + /// is IN_PROGRESS. /// @param _l2BlockNumber The L2 block number to use for the game. function testFuzz_setAnchorState_inProgress_fails(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. @@ -734,18 +465,18 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init // Mock the l2BlockNumber call. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - // Mock the IN_PROGRESS state. + // Mock the CHALLENGER_WINS state. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.IN_PROGRESS)); - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); + + // Mock the resolvedAt timestamp and fast forward to beyond the delay. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); // Try to update the anchor state. - vm.prank(superchainConfig.guardian()); + vm.prank(address(gameProxy)); vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_InvalidAnchorGame.selector); anchorStateRegistry.setAnchorState(gameProxy); @@ -755,25 +486,34 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init assertEq(updatedRoot.raw(), root.raw()); } - /// @notice Tests that setAnchorState will revert if the game is valid and the game type is not - /// the respected game type. + /// @notice Tests that setAnchorState will revert if the game is valid and the game type + /// is not the respected game type. /// @param _l2BlockNumber The L2 block number to use for the game. function testFuzz_setAnchorState_notRespectedGameType_fails(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); + // Bound the new block number. + _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber, type(uint256).max); + // Mock the l2BlockNumber call. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); // Mock the DEFENDER_WINS state. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - // Mock the respectedGameType call so that it does NOT match our game type. - vm.mockCall(address(optimismPortal2), abi.encodeCall(optimismPortal2.respectedGameType, ()), abi.encode(999)); + // Mock the resolvedAt timestamp and fast forward to beyond the delay. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); + + // Mock that the game was not respected. + vm.mockCall( + address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(false) + ); // Try to update the anchor state. - vm.prank(superchainConfig.guardian()); - vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_ImproperAnchorGame.selector); + vm.prank(address(gameProxy)); + vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_InvalidAnchorGame.selector); anchorStateRegistry.setAnchorState(gameProxy); // Confirm that the anchor state has not updated. @@ -782,24 +522,25 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init assertEq(updatedRoot.raw(), root.raw()); } - /// @notice Tests that setAnchorState will revert if the game is valid and the game is blacklisted. + /// @notice Tests that setAnchorState will revert if the game is valid and the game is + /// blacklisted. /// @param _l2BlockNumber The L2 block number to use for the game. - function test_setAnchorState_blacklistedGame_fails(uint256 _l2BlockNumber) public { + function testFuzz_setAnchorState_blacklistedGame_fails(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); + (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); + // Bound the new block number. + _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber + 1, type(uint256).max); // Mock the DEFENDER_WINS state. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); + + // Mock the resolvedAt timestamp and fast forward to beyond the delay. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); // Mock the disputeGameBlacklist call to return true. vm.mockCall( @@ -808,9 +549,9 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init abi.encode(true) ); - // Set the anchor state. - vm.prank(superchainConfig.guardian()); - vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_ImproperAnchorGame.selector); + // Update the anchor state. + vm.prank(address(gameProxy)); + vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_InvalidAnchorGame.selector); anchorStateRegistry.setAnchorState(gameProxy); // Confirm that the anchor state has not updated. @@ -819,24 +560,21 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init assertEq(updatedRoot.raw(), root.raw()); } - /// @notice Tests that setAnchorState will revert if the game is valid and the game is retired. + /// @notice Tests that setAnchorState will revert if the game is valid and the game is + /// retired. /// @param _l2BlockNumber The L2 block number to use for the game. - function test_setAnchorState_retiredGame_fails(uint256 _l2BlockNumber) public { + function testFuzz_setAnchorState_retiredGame_fails(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); + (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); + // Bound the new block number. + _l2BlockNumber = bound(_l2BlockNumber, l2BlockNumber + 1, type(uint256).max); // Mock the DEFENDER_WINS state. vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); // Mock the respectedGameTypeUpdatedAt call to be later than the game's creation time. vm.mockCall( @@ -845,9 +583,9 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init abi.encode(gameProxy.createdAt().raw() + 1) ); - // Set the anchor state. - vm.prank(superchainConfig.guardian()); - vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_ImproperAnchorGame.selector); + // Update the anchor state. + vm.prank(address(gameProxy)); + vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_InvalidAnchorGame.selector); anchorStateRegistry.setAnchorState(gameProxy); // Confirm that the anchor state has not updated. diff --git a/packages/contracts-bedrock/test/dispute/DelayedWETH.t.sol b/packages/contracts-bedrock/test/dispute/DelayedWETH.t.sol index 06e8d7506f0d2..5ca42c28fd038 100644 --- a/packages/contracts-bedrock/test/dispute/DelayedWETH.t.sol +++ b/packages/contracts-bedrock/test/dispute/DelayedWETH.t.sol @@ -352,26 +352,48 @@ contract DelayedWETH_Recover_Test is DelayedWETH_Init { contract DelayedWETH_Hold_Test is DelayedWETH_Init { /// @dev Tests that holding WETH succeeds. - function test_hold_succeeds() public { + function test_hold_byOwner_succeeds() public { uint256 amount = 1 ether; // Pretend to be alice and deposit some WETH. vm.prank(alice); delayedWeth.deposit{ value: amount }(); + // Get our balance before. + uint256 initialBalance = delayedWeth.balanceOf(address(this)); + // Hold some WETH. vm.expectEmit(true, true, true, false); emit Approval(alice, address(this), amount); delayedWeth.hold(alice, amount); - // Verify the allowance. - assertEq(delayedWeth.allowance(alice, address(this)), amount); + // Get our balance after. + uint256 finalBalance = delayedWeth.balanceOf(address(this)); + + // Verify the transfer. + assertEq(finalBalance, initialBalance + amount); + } + + function test_hold_withoutAmount_succeeds() public { + uint256 amount = 1 ether; + + // Pretend to be alice and deposit some WETH. + vm.prank(alice); + delayedWeth.deposit{ value: amount }(); + + // Get our balance before. + uint256 initialBalance = delayedWeth.balanceOf(address(this)); + + // Hold some WETH. + vm.expectEmit(true, true, true, false); + emit Approval(alice, address(this), amount); + delayedWeth.hold(alice); // without amount parameter - // We can transfer. - delayedWeth.transferFrom(alice, address(this), amount); + // Get our balance after. + uint256 finalBalance = delayedWeth.balanceOf(address(this)); // Verify the transfer. - assertEq(delayedWeth.balanceOf(address(this)), amount); + assertEq(finalBalance, initialBalance + amount); } /// @dev Tests that holding WETH by non-owner fails. diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol index 6d61ea87d9620..58f4842ceede2 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol @@ -42,6 +42,7 @@ contract FaultDisputeGame_Init is DisputeGameFactory_Init { bytes internal extraData; event Move(uint256 indexed parentIndex, Claim indexed pivot, address indexed claimant); + event GameClosed(BondDistributionMode bondDistributionMode); event ReceiveETH(uint256 amount); @@ -967,6 +968,17 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { // Ensure the challenge was successful. assertEq(uint8(fdg.status()), uint8(GameStatus.CHALLENGER_WINS)); + // Wait for finalization delay. + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + fdg.closeGame(); + + // Claim credit once to trigger unlock period. + fdg.claimCredit(address(this)); + fdg.claimCredit(address(0xb0b)); + fdg.claimCredit(address(0xace)); + // Wait for the withdrawal delay. vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); @@ -1415,19 +1427,11 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { vm.warp(block.timestamp + 3 days + 12 hours); - assertEq(address(this).balance, 0); gameProxy.resolveClaim(2, 0); gameProxy.resolveClaim(1, 0); - // Wait for the withdrawal delay. - vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); - - gameProxy.claimCredit(address(this)); - assertEq(address(this).balance, firstBond + secondBond); - vm.expectRevert(ClaimAlreadyResolved.selector); gameProxy.resolveClaim(1, 0); - assertEq(address(this).balance, firstBond + secondBond); } /// @dev Static unit test asserting that resolve reverts when attempting to resolve a subgame at max depth @@ -1447,16 +1451,8 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { vm.warp(block.timestamp + 3 days + 12 hours); - // Resolve to claim bond - uint256 balanceBefore = address(this).balance; gameProxy.resolveClaim(8, 0); - // Wait for the withdrawal delay. - vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); - - gameProxy.claimCredit(address(this)); - assertEq(address(this).balance, balanceBefore + _getRequiredBond(7)); - vm.expectRevert(ClaimAlreadyResolved.selector); gameProxy.resolveClaim(8, 0); } @@ -1534,9 +1530,19 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { } gameProxy.resolve(); + // Wait for finalization delay + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + + // Claim credit once to trigger unlock period. + gameProxy.claimCredit(address(this)); + // Wait for the withdrawal delay. vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); + // Claim credit again to get the bond back. gameProxy.claimCredit(address(this)); // Ensure that bonds were paid out correctly. @@ -1622,11 +1628,24 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { (bool success,) = address(gameProxy).call(abi.encodeCall(gameProxy.resolveClaim, (i - 1, 0))); assertTrue(success); } + + // Resolve the game. gameProxy.resolve(); + // Wait for finalization delay + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + + // Claim credit once to trigger unlock period. + gameProxy.claimCredit(address(this)); + gameProxy.claimCredit(bob); + // Wait for the withdrawal delay. vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); + // Claim credit again to get the bond back. gameProxy.claimCredit(address(this)); // Bob's claim should revert since it's value is 0 @@ -1684,9 +1703,22 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { } gameProxy.resolve(); + // Wait for finalization delay + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + + // Claim credit once to trigger unlock period. + gameProxy.claimCredit(address(this)); + gameProxy.claimCredit(alice); + gameProxy.claimCredit(bob); + gameProxy.claimCredit(charlie); + // Wait for the withdrawal delay. vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); + // All of these claims should work. gameProxy.claimCredit(address(this)); gameProxy.claimCredit(alice); gameProxy.claimCredit(bob); @@ -1720,6 +1752,12 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { gameProxy.resolveClaim(0, 0); assertEq(uint8(gameProxy.resolve()), uint8(GameStatus.DEFENDER_WINS)); + // Wait for finalization delay. + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + // Confirm that the anchor state is now the same as the game state. (root, l2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); assertEq(l2BlockNumber, gameProxy.l2BlockNumber()); @@ -1742,6 +1780,12 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { gameProxy.resolveClaim(0, 0); assertEq(uint8(gameProxy.resolve()), uint8(GameStatus.DEFENDER_WINS)); + // Wait for finalization delay. + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + // Confirm that the anchor state is the same as the initial anchor state. (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); assertEq(updatedL2BlockNumber, l2BlockNumber); @@ -1763,6 +1807,12 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { gameProxy.resolveClaim(0, 0); assertEq(uint8(gameProxy.resolve()), uint8(GameStatus.CHALLENGER_WINS)); + // Wait for finalization delay. + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + // Confirm that the anchor state is the same as the initial anchor state. (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); assertEq(updatedL2BlockNumber, l2BlockNumber); @@ -1808,6 +1858,21 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { // Ensure that the game registered the `reenter` contract's credit. assertEq(gameProxy.credit(address(reenter)), reenterBond); + // Resolve the root claim. + gameProxy.resolveClaim(0, 0); + + // Resolve the game. + gameProxy.resolve(); + + // Wait for finalization delay. + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + + // Claim credit once to trigger unlock period. + gameProxy.claimCredit(address(reenter)); + // Wait for the withdrawal delay. vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); @@ -2115,6 +2180,90 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { assertEq(uint8(gameProxy.resolve()), uint8(GameStatus.DEFENDER_WINS)); } + /// @dev Tests that closeGame reverts if the game is not resolved + function test_closeGame_gameNotResolved_reverts() public { + vm.expectRevert(GameNotResolved.selector); + gameProxy.closeGame(); + } + + /// @dev Tests that closeGame reverts if the game is not finalized + function test_closeGame_gameNotFinalized_reverts() public { + // Resolve the game + vm.warp(block.timestamp + 3 days + 12 hours); + gameProxy.resolveClaim(0, 0); + gameProxy.resolve(); + + // Don't wait the finalization delay + vm.expectRevert(abi.encodeWithSelector(GameNotFinalized.selector, "game not beyond airgap period")); + gameProxy.closeGame(); + } + + /// @dev Tests that closeGame succeeds for a proper game (normal distribution) + function test_closeGame_properGame_succeeds() public { + // Resolve the game + vm.warp(block.timestamp + 3 days + 12 hours); + gameProxy.resolveClaim(0, 0); + gameProxy.resolve(); + + // Wait for finalization delay + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game and verify normal distribution mode + vm.expectEmit(true, true, true, true); + emit GameClosed(BondDistributionMode.NORMAL); + gameProxy.closeGame(); + assertEq(uint8(gameProxy.bondDistributionMode()), uint8(BondDistributionMode.NORMAL)); + + // Check that the anchor state was set correctly. + assertEq(address(gameProxy.anchorStateRegistry().anchorGame()), address(gameProxy)); + } + + /// @dev Tests that closeGame succeeds for an improper game (refund mode) + function test_closeGame_improperGame_succeeds() public { + // Resolve the game + vm.warp(block.timestamp + 3 days + 12 hours); + gameProxy.resolveClaim(0, 0); + gameProxy.resolve(); + + // Wait for finalization delay + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Mock the anchor registry to return improper game + vm.mockCall( + address(anchorStateRegistry), + abi.encodeCall(anchorStateRegistry.isGameProper, (IDisputeGame(address(gameProxy)))), + abi.encode(false, "") + ); + + // Close the game and verify refund mode + vm.expectEmit(true, true, true, true); + emit GameClosed(BondDistributionMode.REFUND); + gameProxy.closeGame(); + assertEq(uint8(gameProxy.bondDistributionMode()), uint8(BondDistributionMode.REFUND)); + } + + /// @dev Tests that multiple calls to closeGame succeed after initial distribution mode is set + function test_closeGame_multipleCallsAfterSet_succeeds() public { + // Resolve and close the game first + vm.warp(block.timestamp + 3 days + 12 hours); + gameProxy.resolveClaim(0, 0); + gameProxy.resolve(); + + // Wait for finalization delay + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // First close sets the mode + gameProxy.closeGame(); + assertEq(uint8(gameProxy.bondDistributionMode()), uint8(BondDistributionMode.NORMAL)); + + // Subsequent closes should succeed without changing the mode + gameProxy.closeGame(); + assertEq(uint8(gameProxy.bondDistributionMode()), uint8(BondDistributionMode.NORMAL)); + + gameProxy.closeGame(); + assertEq(uint8(gameProxy.bondDistributionMode()), uint8(BondDistributionMode.NORMAL)); + } + /// @dev Helper to generate a mock RLP encoded header (with only a real block number) & an output root proof. function _generateOutputRootProof( bytes32 _storageRoot, diff --git a/packages/contracts-bedrock/test/invariants/FaultDisputeGame.t.sol b/packages/contracts-bedrock/test/invariants/FaultDisputeGame.t.sol index 705e21bd4cfc9..41143be5a7a6f 100644 --- a/packages/contracts-bedrock/test/invariants/FaultDisputeGame.t.sol +++ b/packages/contracts-bedrock/test/invariants/FaultDisputeGame.t.sol @@ -45,6 +45,16 @@ contract FaultDisputeGame_Solvency_Invariant is FaultDisputeGame_Init { } gameProxy.resolve(); + // Wait for finalization delay + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + + // Claim credit once to trigger unlock period. + gameProxy.claimCredit(address(this)); + gameProxy.claimCredit(address(actor)); + // Wait for the withdrawal delay. vm.warp(block.timestamp + 7 days + 1 seconds); diff --git a/packages/contracts-bedrock/test/safe/DeputyGuardianModule.t.sol b/packages/contracts-bedrock/test/safe/DeputyGuardianModule.t.sol index b112547cd5624..593173acffedb 100644 --- a/packages/contracts-bedrock/test/safe/DeputyGuardianModule.t.sol +++ b/packages/contracts-bedrock/test/safe/DeputyGuardianModule.t.sol @@ -240,6 +240,11 @@ contract DeputyGuardianModule_setRespectedGameType_Test is DeputyGuardianModule_ /// @dev Tests that `setRespectedGameType` successfully updates the respected game type when called by the deputy /// guardian. function testFuzz_setRespectedGameType_succeeds(GameType _gameType) external { + // Game type(uint32).max is reserved for setting the respectedGameTypeUpdatedAt timestamp. + // TODO(kelvin): Remove this once we've removed the hack. + uint32 boundedGameType = uint32(bound(_gameType.raw(), 0, type(uint32).max - 1)); + _gameType = GameType.wrap(boundedGameType); + vm.expectEmit(address(safeInstance.safe)); emit ExecutionFromModuleSuccess(address(deputyGuardianModule)); diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 2f1c8c9b65051..9ff0f2df46e3e 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -730,6 +730,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "DelayedWETH", _sel: _getSel("delay()") }); _addSpec({ _name: "DelayedWETH", _sel: _getSel("deposit()") }); _addSpec({ _name: "DelayedWETH", _sel: _getSel("hold(address,uint256)"), _auth: Role.DELAYEDWETHOWNER }); + _addSpec({ _name: "DelayedWETH", _sel: _getSel("hold(address)"), _auth: Role.DELAYEDWETHOWNER }); _addSpec({ _name: "DelayedWETH", _sel: _getSel("initialize(address,address)") }); _addSpec({ _name: "DelayedWETH", _sel: _getSel("name()") }); _addSpec({ _name: "DelayedWETH", _sel: _getSel("owner()") }); From e3df61d80c134d14bb6f63041658cdd0398d1cec Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Mon, 13 Jan 2025 11:51:14 -0500 Subject: [PATCH 02/28] fix tests and add specs --- .../dispute/IAnchorStateRegistry.sol | 9 +-- .../dispute/IPermissionedDisputeGame.sol | 1 + .../test/L1/OptimismPortal2.t.sol | 59 ++++++++----------- .../test/dispute/AnchorStateRegistry.t.sol | 1 + .../test/universal/Specs.t.sol | 30 +++++++--- 5 files changed, 52 insertions(+), 48 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol b/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol index bf4f88e552f5b..aace7be528ecc 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol @@ -27,13 +27,14 @@ interface IAnchorStateRegistry { OutputRoot memory _startingAnchorRoot ) external; - function isGameRegistered(IDisputeGame _game) external view returns (bool); + + function isGameBeyondAirgap(IDisputeGame _game) external view returns (bool); function isGameBlacklisted(IDisputeGame _game) external view returns (bool); + function isGameProper(IDisputeGame _game) external view returns (bool); + function isGameRegistered(IDisputeGame _game) external view returns (bool); + function isGameResolved(IDisputeGame _game) external view returns (bool); function isGameRespected(IDisputeGame _game) external view returns (bool); function isGameRetired(IDisputeGame _game) external view returns (bool); - function isGameResolved(IDisputeGame _game) external view returns (bool); - function isGameBeyondAirgap(IDisputeGame _game) external view returns (bool); - function isGameProper(IDisputeGame _game) external view returns (bool); function isGameFinalized(IDisputeGame _game) external view returns (bool); function isGameClaimValid(IDisputeGame _game) external view returns (bool); function portal() external view returns (IOptimismPortal2); diff --git a/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol b/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol index 8aa4e8307765a..6b509a9c56e28 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol @@ -100,6 +100,7 @@ interface IPermissionedDisputeGame is IDisputeGame { function getNumToResolve(uint256 _claimIndex) external view returns (uint256 numRemainingChildren_); function getRequiredBond(Position _position) external view returns (uint256 requiredBond_); function hasUnlockedCredit(address) external view returns (bool); + function initialize() external payable; function l2BlockNumber() external pure returns (uint256 l2BlockNumber_); function l2BlockNumberChallenged() external view returns (bool); function l2BlockNumberChallenger() external view returns (address); diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index fef3faaddf94c..2148ceb077458 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -455,15 +455,31 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { } /// @dev Tests that the guardian role can set the respected game type to anything they want. - function testFuzz_setRespectedGameType_guardian_succeeds(GameType _ty) external { + function testFuzz_setRespectedGameType_guardianCanSetRespectedGameType_succeeds(GameType _ty) external { + vm.assume(_ty.raw() != type(uint32).max); + uint64 respectedGameTypeUpdatedAt = optimismPortal2.respectedGameTypeUpdatedAt(); vm.expectEmit(address(optimismPortal2)); - emit RespectedGameTypeSet(_ty, Timestamp.wrap(uint64(block.timestamp))); + emit RespectedGameTypeSet(_ty, Timestamp.wrap(respectedGameTypeUpdatedAt)); vm.prank(optimismPortal2.guardian()); optimismPortal2.setRespectedGameType(_ty); assertEq(optimismPortal2.respectedGameType().raw(), _ty.raw()); } + /// @dev Tests that the guardian can set the `respectedGameTypeUpdatedAt` timestamp to current timestamp. + function testFuzz_setRespectedGameType_guardianCanSetRespectedGameTypeUpdatedAt_succeeds(uint64 _elapsed) + external + { + _elapsed = uint64(bound(_elapsed, 0, type(uint64).max - uint64(block.timestamp))); + GameType _ty = GameType.wrap(type(uint32).max); + uint64 _timestamp = uint64(block.timestamp) + _elapsed; + vm.warp(_timestamp); + // TODO: event? + vm.prank(optimismPortal2.guardian()); + optimismPortal2.setRespectedGameType(_ty); + assertEq(optimismPortal2.respectedGameTypeUpdatedAt(), _timestamp); + } + /// @dev Tests that `proveWithdrawalTransaction` reverts when paused. function test_proveWithdrawalTransaction_paused_reverts() external { vm.prank(optimismPortal2.guardian()); @@ -1293,35 +1309,6 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { assertTrue(optimismPortal2.finalizedWithdrawals(_withdrawalHash)); } - /// @dev Tests that `finalizeWithdrawalTransaction` reverts if the respected game type has changed since the - /// withdrawal was proven. - function test_finalizeWithdrawalTransaction_respectedTypeChangedSinceProving_reverts() external { - vm.expectEmit(true, true, true, true); - emit WithdrawalProven(_withdrawalHash, alice, bob); - vm.expectEmit(true, true, true, true); - emit WithdrawalProvenExtension1(_withdrawalHash, address(this)); - optimismPortal2.proveWithdrawalTransaction({ - _tx: _defaultTx, - _disputeGameIndex: _proposedGameIndex, - _outputRootProof: _outputRootProof, - _withdrawalProof: _withdrawalProof - }); - - // Warp past the finalization period. - vm.warp(block.timestamp + optimismPortal2.proofMaturityDelaySeconds() + 1); - - // Resolve the dispute game. - game.resolveClaim(0, 0); - game.resolve(); - - // Change the respected game type in the portal. - vm.prank(optimismPortal2.guardian()); - optimismPortal2.setRespectedGameType(GameType.wrap(0xFF)); - - vm.expectRevert(InvalidGameType.selector); - optimismPortal2.finalizeWithdrawalTransaction(_defaultTx); - } - /// @dev Tests that `finalizeWithdrawalTransaction` reverts if the respected game type was updated after the /// dispute game was created. function test_finalizeWithdrawalTransaction_gameOlderThanRespectedGameTypeUpdate_reverts() external { @@ -1343,12 +1330,12 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { game.resolveClaim(0, 0); game.resolve(); - // Change the respected game type in the portal. - vm.prank(optimismPortal2.guardian()); - optimismPortal2.setRespectedGameType(GameType.wrap(0xFF)); + // Warp past the dispute game finality delay. + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); - // Mock the game's type so that we pass the correct game type check. - vm.mockCall(address(game), abi.encodeCall(game.gameType, ()), abi.encode(GameType.wrap(0xFF))); + // Set respectedGameTypeUpdatedAt. + vm.prank(optimismPortal2.guardian()); + optimismPortal2.setRespectedGameType(GameType.wrap(type(uint32).max)); vm.expectRevert("OptimismPortal: dispute game created before respected game type was updated"); optimismPortal2.finalizeWithdrawalTransaction(_defaultTx); diff --git a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol index 72fe2be04a205..ae10c0c9c71f4 100644 --- a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol +++ b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol @@ -338,6 +338,7 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init assertEq(updatedL2BlockNumber, l2BlockNumber); assertEq(updatedRoot.raw(), root.raw()); } +} contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init { /// @notice Tests that setAnchorState will revert if the sender is not the guardian. diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 9ff0f2df46e3e..2014f40a757b9 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -23,7 +23,7 @@ import { IProtocolVersions } from "interfaces/L1/IProtocolVersions.sol"; /// @dev Specifies common security properties of entrypoints to L1 contracts, including authorization and /// pausability. /// When adding new functions to the L1 system, the `setUp` function must be updated to document the security -/// properties of the new function. The `Spec` struct reppresents this documentation. However, this contract does +/// properties of the new function. The `Spec` struct represents this documentation. However, this contract does /// not actually test to verify these properties, only that a spec is defined. contract Specification_Test is CommonTest { enum Role { @@ -553,21 +553,25 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "MintManager", _sel: _getSel("upgrade(address)"), _auth: Role.MINTMANAGEROWNER }); // AnchorStateRegistry + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("anchorGame()") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("anchors(uint32)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("getAnchorRoot()") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("disputeGameFactory()") }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("portal()") }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("anchorGame()") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("initialize(address,address,address,(bytes32,uint256))") }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("tryUpdateAnchorState()") }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("setAnchorState(address)"), _auth: Role.GUARDIAN }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("version()") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameBeyondAirgap(address)") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameBlacklisted(address)") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameClaimValid(address)") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameFinalized(address)") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameProper(address)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameRegistered(address)") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameResolved(address)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameRespected(address)") }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameBlacklisted(address)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameRetired(address)") }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameProper(address)") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("portal()") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("respectedGameType()") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("setAnchorState(address)"), _auth: Role.GUARDIAN }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("superchainConfig()") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("version()") }); // PermissionedDisputeGame _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("absolutePrestate()") }); @@ -578,6 +582,7 @@ contract Specification_Test is CommonTest { _sel: _getSel("attack(bytes32,uint256,bytes32)"), _auth: Role.CHALLENGER }); + _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("bondDistributionMode()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("challengeRootL2Block((bytes32,bytes32,bytes32,bytes32),bytes)"), @@ -589,6 +594,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("claimDataLen()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("claims(bytes32)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("clockExtension()") }); + _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("closeGame()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("createdAt()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("credit(address)") }); _addSpec({ @@ -603,6 +609,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("getChallengerDuration(uint256)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("getNumToResolve(uint256)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("getRequiredBond(uint128)") }); + _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("hasUnlockedCredit(address)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("initialize()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("l1Head()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("l2BlockNumber()") }); @@ -617,6 +624,7 @@ contract Specification_Test is CommonTest { _auth: Role.CHALLENGER }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("proposer()") }); + _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("refundModeCredit(address)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("resolutionCheckpoints(uint256)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("resolve()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("resolveClaim(uint256,uint256)") }); @@ -636,6 +644,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("subgames(uint256,uint256)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("version()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("vm()") }); + _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("wasRespectedGameTypeWhenCreated()") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("weth()") }); // FaultDisputeGame @@ -643,6 +652,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("addLocalData(uint256,uint256,uint256)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("anchorStateRegistry()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("attack(bytes32,uint256,bytes32)") }); + _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("bondDistributionMode()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("challengeRootL2Block((bytes32,bytes32,bytes32,bytes32),bytes)") @@ -652,6 +662,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("claimDataLen()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("claims(bytes32)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("clockExtension()") }); + _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("closeGame()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("createdAt()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("credit(address)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("defend(bytes32,uint256,bytes32)") }); @@ -661,6 +672,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("gameType()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("getChallengerDuration(uint256)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("getRequiredBond(uint128)") }); + _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("hasUnlockedCredit(address)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("initialize()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("l1Head()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("l2BlockNumber()") }); @@ -673,6 +685,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolutionCheckpoints(uint256)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolve()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("getNumToResolve(uint256)") }); + _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("refundModeCredit(address)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolveClaim(uint256,uint256)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolvedAt()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolvedSubgames(uint256)") }); @@ -686,6 +699,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("subgames(uint256,uint256)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("version()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("vm()") }); + _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("wasRespectedGameTypeWhenCreated()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("weth()") }); // DisputeGameFactory From d8e329247bab50e98fbc9db499cf108e9c036d87 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Tue, 14 Jan 2025 12:32:56 -0600 Subject: [PATCH 03/28] misc fixes --- mise.toml | 6 +-- packages/contracts-bedrock/foundry.toml | 10 ++++- .../interfaces/dispute/IFaultDisputeGame.sol | 2 +- .../dispute/IPermissionedDisputeGame.sol | 2 +- .../snapshots/abi/AnchorStateRegistry.json | 17 +------- .../snapshots/abi/FaultDisputeGame.json | 8 +--- .../abi/PermissionedDisputeGame.json | 8 +--- .../snapshots/semver-lock.json | 18 ++++---- .../test/dispute/AnchorStateRegistry.t.sol | 42 ------------------- 9 files changed, 26 insertions(+), 87 deletions(-) diff --git a/mise.toml b/mise.toml index f706fecce93a9..8941be102269e 100644 --- a/mise.toml +++ b/mise.toml @@ -31,9 +31,9 @@ just = "1.37.0" # Foundry dependencies # Foundry is a special case because it supplies multiple binaries at the same # GitHub release, so we need to use the aliasing trick to get mise to not error -forge = "nightly-59f354c179f4e7f6d7292acb3d068815c79286d1" -cast = "nightly-59f354c179f4e7f6d7292acb3d068815c79286d1" -anvil = "nightly-59f354c179f4e7f6d7292acb3d068815c79286d1" +forge = "nightly-017c59d6806ce11f1dc131f8607178efad79d84a" +cast = "nightly-017c59d6806ce11f1dc131f8607178efad79d84a" +anvil = "nightly-017c59d6806ce11f1dc131f8607178efad79d84a" # Fake dependencies # Put things here if you need to track versions of tools or projects that can't diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index 09af861272e80..6bacfbfa9e9ed 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -11,7 +11,15 @@ build_info_path = 'artifacts/build-info' snapshots = 'notarealpath' # workaround for foundry#9477 optimizer = true -optimizer_runs = 5000 +optimizer_runs = 999999 + +additional_compiler_profiles = [ + { name = "dispute", optimizer_runs = 5000 }, +] +compilation_restrictions = [ + { paths = "src/dispute/FaultDisputeGame.sol", optimizer_runs = 5000 }, + { paths = "src/dispute/PermissionedDisputeGame.sol", optimizer_runs = 5000 }, +] extra_output = ['devdoc', 'userdoc', 'metadata', 'storageLayout'] bytecode_hash = 'none' diff --git a/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol b/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol index 4d8a9f7d8f984..9df883f849b93 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol @@ -75,7 +75,7 @@ interface IFaultDisputeGame is IDisputeGame { error UnexpectedString(); error ValidStep(); error InvalidBondDistributionMode(); - error GameNotFinalized(string reason); + error GameNotFinalized(); error GameNotResolved(); error ReservedGameType(); diff --git a/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol b/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol index 6b509a9c56e28..6c9ce3d404475 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol @@ -64,7 +64,7 @@ interface IPermissionedDisputeGame is IDisputeGame { error UnexpectedString(); error ValidStep(); error InvalidBondDistributionMode(); - error GameNotFinalized(string reason); + error GameNotFinalized(); error GameNotResolved(); error ReservedGameType(); diff --git a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json index d0f829fd66fc2..8b66e7af20873 100644 --- a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json +++ b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json @@ -164,11 +164,6 @@ "internalType": "bool", "name": "", "type": "bool" - }, - { - "internalType": "string", - "name": "", - "type": "string" } ], "stateMutability": "view", @@ -188,11 +183,6 @@ "internalType": "bool", "name": "", "type": "bool" - }, - { - "internalType": "string", - "name": "", - "type": "string" } ], "stateMutability": "view", @@ -397,11 +387,6 @@ "name": "Initialized", "type": "event" }, - { - "inputs": [], - "name": "AnchorStateRegistry_ImproperAnchorGame", - "type": "error" - }, { "inputs": [], "name": "AnchorStateRegistry_InvalidAnchorGame", @@ -412,4 +397,4 @@ "name": "AnchorStateRegistry_Unauthorized", "type": "error" } -] +] \ No newline at end of file diff --git a/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json b/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json index 2d6ac86ea18a9..9f42a6762bb71 100644 --- a/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json @@ -1045,13 +1045,7 @@ "type": "error" }, { - "inputs": [ - { - "internalType": "string", - "name": "reason", - "type": "string" - } - ], + "inputs": [], "name": "GameNotFinalized", "type": "error" }, diff --git a/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json b/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json index 8da34d0ce2fb7..cec052e0b0d20 100644 --- a/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json @@ -1086,13 +1086,7 @@ "type": "error" }, { - "inputs": [ - { - "internalType": "string", - "name": "reason", - "type": "string" - } - ], + "inputs": [], "name": "GameNotFinalized", "type": "error" }, diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index f277374154109..329b86857345a 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -20,12 +20,12 @@ "sourceCodeHash": "0x2d21506cc51ebe0b60bcf89883aff5e9b1269567ce44ee779de3d3940e23fb65" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x52ddbe70f60a6f4e5c929fed417c92d99d82a85c05de8e1f0c8dfd0d634ad648", - "sourceCodeHash": "0xc4a66e8adacd58b961938f473b22614a520fa0bdae70381b8f56210bc34f1f32" + "initCodeHash": "0x32eceffc74182357cb79f7218851e02a76200b1e57db7629c2b97b9401d71a25", + "sourceCodeHash": "0x95f68f577529c158070e3a23fd2da1c63581113471e1303716e387a07f3070d9" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x6b05d7130c019d3c46dca39dfdae3797861088a9fb7b5baa84fd4e5cb87c5883", - "sourceCodeHash": "0xae2fbe02c0f8685692babeed0252ae8a624dc6d3bfb082fc3807d7b84869004b" + "initCodeHash": "0x271c17198970e361cb43aff3ae788d4f726d8d1a188eeea7d0a5f0c477380a76", + "sourceCodeHash": "0xbb6acc3e88af9594ffcb8a2f30860511b76e09024330e70052316668fe55fd1f" }, "src/L1/ProtocolVersions.sol": { "initCodeHash": "0x0000ec89712d8b4609873f1ba76afffd4205bf9110818995c90134dbec12e91e", @@ -152,8 +152,8 @@ "sourceCodeHash": "0xb7b0a06cd971c4647247dc19ce997d0c64a73e87c81d30731da9cf9efa1b952a" }, "src/dispute/AnchorStateRegistry.sol": { - "initCodeHash": "0x6e94c2f4129209d2b36d6a54adaaf99b91b3df322c8c75043d29f6b0a9143738", - "sourceCodeHash": "0xb81181263522e939195a70b6e281fb038de2312c9682f9123723c6ae1babf559" + "initCodeHash": "0x751e32c8d44c5ef9b1acae2d94e07f6e91f7f59663c24e12d21298638458b272", + "sourceCodeHash": "0xb09dccd5bf662c42ab10afd19a2b51114df9fd3bc6e20d4d86e4ba7529036bbc" }, "src/dispute/DelayedWETH.sol": { "initCodeHash": "0xb1f04c9ee86984a157b92a18754c84104e9d4df7a3838633301ca7f557d0220a", @@ -164,8 +164,8 @@ "sourceCodeHash": "0x155c0334f63616ed245aadf9a94f419ef7d5e2237b3b32172484fd19890a61dc" }, "src/dispute/FaultDisputeGame.sol": { - "initCodeHash": "0x734c4bce3822851f8925d3a8668630e4c503ee5a042e1234e448f4e9ea86582c", - "sourceCodeHash": "0x8888b471b484c3f0662dca39317aa75fbb380deea4e12f6a8ccd5dde1202e5c7" + "initCodeHash": "0xd59ffa7f487db9b0761b1fa3994c59e0d78e4546c76379a14a31dec1080b3f20", + "sourceCodeHash": "0xd59fd93e7332bad50714ec1abe06cebd6378d32cc4ad18434f09156ac3c97982" }, "src/legacy/DeployerWhitelist.sol": { "initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29", @@ -219,4 +219,4 @@ "initCodeHash": "0x2bfce526f82622288333d53ca3f43a0a94306ba1bab99241daa845f8f4b18bd4", "sourceCodeHash": "0xf49d7b0187912a6bb67926a3222ae51121e9239495213c975b3b4b217ee57a1b" } -} +} \ No newline at end of file diff --git a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol index ae10c0c9c71f4..4b4ab8fd84d3c 100644 --- a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol +++ b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol @@ -338,48 +338,6 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init assertEq(updatedL2BlockNumber, l2BlockNumber); assertEq(updatedRoot.raw(), root.raw()); } -} - -contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init { - /// @notice Tests that setAnchorState will revert if the sender is not the guardian. - /// @param _sender The address of the sender. - /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_setAnchorState_notGuardian_fails(address _sender, uint256 _l2BlockNumber) public { - // Grab block number of the existing anchor root. - (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); - - // Mock the l2BlockNumber call. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.l2BlockNumber, ()), abi.encode(_l2BlockNumber)); - - // Mock the DEFENDER_WINS state. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); - - // Make our game type the respected game type. - vm.mockCall( - address(optimismPortal2), - abi.encodeCall(optimismPortal2.respectedGameType, ()), - abi.encode(gameProxy.gameType()) - ); - - // Mock the DisputeGameFactory to make it seem that the game was not registered. - vm.mockCall( - address(disputeGameFactory), - abi.encodeCall( - disputeGameFactory.games, (gameProxy.gameType(), gameProxy.rootClaim(), gameProxy.extraData()) - ), - abi.encode(address(0), 0) - ); - - // Try to update the anchor state. - vm.prank(_sender); - vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_Unauthorized.selector); - anchorStateRegistry.setAnchorState(gameProxy); - - // Confirm that the anchor state has not updated. - (Hash updatedRoot, uint256 updatedL2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); - assertEq(updatedL2BlockNumber, l2BlockNumber); - assertEq(updatedRoot.raw(), root.raw()); - } /// @notice Tests that setAnchorState will revert if the game is not registered. /// @param _l2BlockNumber The L2 block number to use for the game. From 7e520b76f77e0a41694666f3f588470357d4c5a3 Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 14:30:45 -0500 Subject: [PATCH 04/28] more fixes --- packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol | 2 +- packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol b/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol index f9ae25ac273f1..8b50d40352a34 100644 --- a/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol +++ b/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol @@ -295,7 +295,7 @@ contract FaultDisputeGame is Clone, ISemver { if (initialized) revert AlreadyInitialized(); // Grab the latest anchor root. - (Hash root, uint256 rootBlockNumber) = ANCHOR_STATE_REGISTRY.anchors(GAME_TYPE); + (Hash root, uint256 rootBlockNumber) = ANCHOR_STATE_REGISTRY.getAnchorRoot(); // Should only happen if this is a new game type that hasn't been set up yet. if (root.raw() == bytes32(0)) revert AnchorRootNotFound(); diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol index 58f4842ceede2..5f09995c71376 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol @@ -2194,7 +2194,7 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { gameProxy.resolve(); // Don't wait the finalization delay - vm.expectRevert(abi.encodeWithSelector(GameNotFinalized.selector, "game not beyond airgap period")); + vm.expectRevert(GameNotFinalized.selector); gameProxy.closeGame(); } From 2e0181d308156fd51900731c935c08fdee886822 Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 15:28:04 -0500 Subject: [PATCH 05/28] emit event on setRespectedGameTypeUpdatedAt, and test wasRespectedGameType as withdrawal finality condition --- .../src/L1/OptimismPortal2.sol | 1 + .../test/L1/OptimismPortal2.t.sol | 96 ++++++++++++++++++- 2 files changed, 92 insertions(+), 5 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 0d27297d7f40e..9d88419faa9fe 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -482,6 +482,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { // respectedGameTypeUpdatedAt timestamp without modifying this function's signature. if (_gameType.raw() == type(uint32).max) { respectedGameTypeUpdatedAt = uint64(block.timestamp); + emit RespectedGameTypeSet(respectedGameType, Timestamp.wrap(respectedGameTypeUpdatedAt)); } else { respectedGameType = _gameType; emit RespectedGameTypeSet(_gameType, Timestamp.wrap(respectedGameTypeUpdatedAt)); diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 2148ceb077458..fb558b43ea555 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -462,8 +462,9 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { emit RespectedGameTypeSet(_ty, Timestamp.wrap(respectedGameTypeUpdatedAt)); vm.prank(optimismPortal2.guardian()); optimismPortal2.setRespectedGameType(_ty); - + // GameType changes, but the timestamp doesn't. assertEq(optimismPortal2.respectedGameType().raw(), _ty.raw()); + assertEq(optimismPortal2.respectedGameTypeUpdatedAt(), respectedGameTypeUpdatedAt); } /// @dev Tests that the guardian can set the `respectedGameTypeUpdatedAt` timestamp to current timestamp. @@ -472,12 +473,15 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { { _elapsed = uint64(bound(_elapsed, 0, type(uint64).max - uint64(block.timestamp))); GameType _ty = GameType.wrap(type(uint32).max); - uint64 _timestamp = uint64(block.timestamp) + _elapsed; - vm.warp(_timestamp); - // TODO: event? + uint64 _newRespectedGameTypeUpdatedAt = uint64(block.timestamp) + _elapsed; + GameType _existingGameType = optimismPortal2.respectedGameType(); + vm.warp(_newRespectedGameTypeUpdatedAt); + emit RespectedGameTypeSet(_existingGameType, Timestamp.wrap(_newRespectedGameTypeUpdatedAt)); vm.prank(optimismPortal2.guardian()); optimismPortal2.setRespectedGameType(_ty); - assertEq(optimismPortal2.respectedGameTypeUpdatedAt(), _timestamp); + // GameType doesn't change, but the timestamp does. + assertEq(optimismPortal2.respectedGameType().raw(), _existingGameType.raw()); + assertEq(optimismPortal2.respectedGameTypeUpdatedAt(), _newRespectedGameTypeUpdatedAt); } /// @dev Tests that `proveWithdrawalTransaction` reverts when paused. @@ -1252,6 +1256,88 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { assertTrue(optimismPortal2.finalizedWithdrawals(withdrawalHash)); } + /// @dev Tests that `finalizeWithdrawalTransaction` succeeds even if the respected game type is changed. + function test_finalizeWithdrawalTransaction_wasRespectedGameType_succeeds( + address _sender, + address _target, + uint256 _value, + uint256 _gasLimit, + bytes memory _data, + GameType _newGameType + ) + external + { + vm.assume( + _target != address(optimismPortal2) // Cannot call the optimism portal or a contract + && _target.code.length == 0 // No accounts with code + && _target != CONSOLE // The console has no code but behaves like a contract + && uint160(_target) > 9 // No precompiles (or zero address) + ); + + // Bound to prevent changes in respectedGameTypeUpdatedAt + _newGameType = GameType.wrap(uint32(bound(_newGameType.raw(), 0, type(uint32).max - 1))); + + // Total ETH supply is currently about 120M ETH. + uint256 value = bound(_value, 0, 200_000_000 ether); + vm.deal(address(optimismPortal2), value); + + uint256 gasLimit = bound(_gasLimit, 0, 50_000_000); + uint256 nonce = l2ToL1MessagePasser.messageNonce(); + + // Get a withdrawal transaction and mock proof from the differential testing script. + Types.WithdrawalTransaction memory _tx = Types.WithdrawalTransaction({ + nonce: nonce, + sender: _sender, + target: _target, + value: value, + gasLimit: gasLimit, + data: _data + }); + ( + bytes32 stateRoot, + bytes32 storageRoot, + bytes32 outputRoot, + bytes32 withdrawalHash, + bytes[] memory withdrawalProof + ) = ffi.getProveWithdrawalTransactionInputs(_tx); + + // Create the output root proof + Types.OutputRootProof memory proof = Types.OutputRootProof({ + version: bytes32(uint256(0)), + stateRoot: stateRoot, + messagePasserStorageRoot: storageRoot, + latestBlockhash: bytes32(uint256(0)) + }); + + // Ensure the values returned from ffi are correct + assertEq(outputRoot, Hashing.hashOutputRootProof(proof)); + assertEq(withdrawalHash, Hashing.hashWithdrawal(_tx)); + + // Setup the dispute game to return the output root + vm.mockCall(address(game), abi.encodeCall(game.rootClaim, ()), abi.encode(outputRoot)); + + // Prove the withdrawal transaction + optimismPortal2.proveWithdrawalTransaction(_tx, _proposedGameIndex, proof, withdrawalProof); + (IDisputeGame _game,) = optimismPortal2.provenWithdrawals(withdrawalHash, address(this)); + assertTrue(_game.rootClaim().raw() != bytes32(0)); + + // Resolve the dispute game + game.resolveClaim(0, 0); + game.resolve(); + + // Warp past the finalization period + vm.warp(block.timestamp + optimismPortal2.proofMaturityDelaySeconds() + 1); + + // Change the respectedGameType + vm.prank(optimismPortal2.guardian()); + optimismPortal2.setRespectedGameType(_newGameType); + + // Withdrawal transaction still finalizable + vm.expectCallMinGas(_tx.target, _tx.value, uint64(_tx.gasLimit), _tx.data); + optimismPortal2.finalizeWithdrawalTransaction(_tx); + assertTrue(optimismPortal2.finalizedWithdrawals(withdrawalHash)); + } + /// @dev Tests that `finalizeWithdrawalTransaction` reverts if the withdrawal's dispute game has been blacklisted. function test_finalizeWithdrawalTransaction_blacklisted_reverts() external { vm.expectEmit(true, true, true, true); From fecbee65408c7db9c7a75e1c339d8aab58625f27 Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 15:47:28 -0500 Subject: [PATCH 06/28] withdrawal when gameWasNotRespectedGameType reverts --- .../test/L1/OptimismPortal2.t.sol | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index fb558b43ea555..9070a57d0c938 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -1427,6 +1427,36 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { optimismPortal2.finalizeWithdrawalTransaction(_defaultTx); } + /// @dev Tests that `finalizeWithdrawalTransaction` reverts if the game was not the respected game type when it was + /// created. `proveWithdrawalTransaction` should already prevent this, but we remove that assumption here. + function test_finalizeWithdrawalTransaction_gameWasNotRespectedGameType_reverts() external { + vm.expectEmit(address(optimismPortal2)); + emit WithdrawalProven(_withdrawalHash, alice, bob); + vm.expectEmit(address(optimismPortal2)); + emit WithdrawalProvenExtension1(_withdrawalHash, address(this)); + optimismPortal2.proveWithdrawalTransaction({ + _tx: _defaultTx, + _disputeGameIndex: _proposedGameIndex, + _outputRootProof: _outputRootProof, + _withdrawalProof: _withdrawalProof + }); + + // Warp past the finalization period. + vm.warp(block.timestamp + optimismPortal2.proofMaturityDelaySeconds() + 1); + + // Resolve the dispute game. + game.resolveClaim(0, 0); + game.resolve(); + + // Warp past the dispute game finality delay. + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); + + vm.mockCall(address(game), abi.encodeCall(game.wasRespectedGameTypeWhenCreated, ()), abi.encode(false)); + + vm.expectRevert(InvalidGameType.selector); + optimismPortal2.finalizeWithdrawalTransaction(_defaultTx); + } + /// @dev Tests an e2e prove -> finalize path, checking the edges of each delay for correctness. function test_finalizeWithdrawalTransaction_delayEdges_succeeds() external { // Prove the withdrawal transaction. From 4844149617e1f43c3d50bc1cec28eaa43358579d Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 16:37:29 -0500 Subject: [PATCH 07/28] anchor game blacklisted and getAnchorGame tests --- .../dispute/IAnchorStateRegistry.sol | 1 + .../src/dispute/AnchorStateRegistry.sol | 7 +++ .../test/dispute/AnchorStateRegistry.t.sol | 59 +++++++++++++++---- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol b/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol index aace7be528ecc..1fb45d2541d5f 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol @@ -11,6 +11,7 @@ import { GameType, Hash, OutputRoot } from "src/dispute/lib/Types.sol"; interface IAnchorStateRegistry { error AnchorStateRegistry_Unauthorized(); error AnchorStateRegistry_InvalidAnchorGame(); + error AnchorStateRegistry_AnchorGameBlacklisted(); event AnchorNotUpdated(IFaultDisputeGame indexed game); event AnchorUpdated(IFaultDisputeGame indexed game); diff --git a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol index d646b66849ace..357438fd4c229 100644 --- a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol +++ b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol @@ -55,6 +55,9 @@ contract AnchorStateRegistry is Initializable, ISemver { /// @notice Thrown when an invalid anchor game is provided. error AnchorStateRegistry_InvalidAnchorGame(); + /// @notice Thrown when an anchor game is blacklisted. + error AnchorStateRegistry_AnchorGameBlacklisted(); + /// @notice Constructor to disable initializers. constructor() { _disableInitializers(); @@ -103,6 +106,10 @@ contract AnchorStateRegistry is Initializable, ISemver { return (startingAnchorRoot.root, startingAnchorRoot.l2BlockNumber); } + if (isGameBlacklisted(anchorGame)) { + revert AnchorStateRegistry_AnchorGameBlacklisted(); + } + // Otherwise, return the anchor root. return (Hash.wrap(anchorGame.rootClaim().raw()), anchorGame.l2BlockNumber()); } diff --git a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol index 4b4ab8fd84d3c..a26cd707e573c 100644 --- a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol +++ b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol @@ -77,6 +77,48 @@ contract AnchorStateRegistry_GetAnchorRoot_Test is AnchorStateRegistry_Init { assertEq(root.raw(), 0xDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF); assertEq(l2BlockNumber, 0); } + + /// @notice Tests that getAnchorRoot will return the correct anchor root if an anchor game exists. + function test_getAnchorRoot_anchorGameExists_succeeds() public { + // Mock the game to be resolved. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); + + // Mock the game to be the defender wins. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); + + // Set the anchor game to the game proxy. + anchorStateRegistry.setAnchorState(gameProxy); + + // We should get the anchor root back. + (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); + assertEq(root.raw(), gameProxy.rootClaim().raw()); + assertEq(l2BlockNumber, gameProxy.l2BlockNumber()); + } +} + +contract AnchorStateRegistry_GetAnchorRoot_TestFail is AnchorStateRegistry_Init { + /// @notice Tests that getAnchorRoot will revert if the anchor game is blacklisted. + function test_getAnchorRoot_blacklistedGame_fails() public { + // Mock the game to be resolved. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); + + // Mock the game to be the defender wins. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); + + // Set the anchor game to the game proxy. + anchorStateRegistry.setAnchorState(gameProxy); + + // Mock the disputeGameBlacklist call to return true. + vm.mockCall( + address(optimismPortal2), + abi.encodeCall(optimismPortal2.disputeGameBlacklist, (gameProxy)), + abi.encode(true) + ); + vm.expectRevert(IAnchorStateRegistry.AnchorStateRegistry_AnchorGameBlacklisted.selector); + anchorStateRegistry.getAnchorRoot(); + } } contract AnchorStateRegistry_Anchors_Test is AnchorStateRegistry_Init { @@ -150,17 +192,11 @@ contract AnchorStateRegistry_IsGameRespected_Test is AnchorStateRegistry_Init { /// @notice Tests that isGameRespected will return false if the game is not of the respected game /// type. - /// @param _gameType The game type to use for the test. - function testFuzz_isGameRespected_isNotRespected_succeeds(GameType _gameType) public { - if (_gameType.raw() == gameProxy.gameType().raw()) { - _gameType = GameType.wrap(_gameType.raw() + 1); - } - + function test_isGameRespected_isNotRespected_succeeds() public { // Mock that the game was not respected. vm.mockCall( address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(false) ); - assertFalse(anchorStateRegistry.isGameRespected(gameProxy)); } } @@ -308,7 +344,7 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init /// @notice Tests that setAnchorState will revert if the game is valid and the game block /// number is less than or equal to the current anchor root block number. /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_setAnchorState_validOlderState_fails(uint256 _l2BlockNumber) public { + function testFuzz_setAnchorState_olderValidGameClaim_fails(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.getAnchorRoot(); @@ -445,10 +481,9 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init assertEq(updatedRoot.raw(), root.raw()); } - /// @notice Tests that setAnchorState will revert if the game is valid and the game type - /// is not the respected game type. + /// @notice Tests that setAnchorState will revert if the game is not respected. /// @param _l2BlockNumber The L2 block number to use for the game. - function testFuzz_setAnchorState_notRespectedGameType_fails(uint256 _l2BlockNumber) public { + function testFuzz_setAnchorState_isNotRespectedGame_fails(uint256 _l2BlockNumber) public { // Grab block number of the existing anchor root. (Hash root, uint256 l2BlockNumber) = anchorStateRegistry.anchors(gameProxy.gameType()); @@ -465,7 +500,7 @@ contract AnchorStateRegistry_SetAnchorState_TestFail is AnchorStateRegistry_Init vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(block.timestamp)); vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); - // Mock that the game was not respected. + // Mock that the game was not respected when created. vm.mockCall( address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(false) ); From 53f51da0cba9bc5b8c548f0ea88efd030a77717f Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 16:38:53 -0500 Subject: [PATCH 08/28] isGameAirgapped --- .../interfaces/dispute/IAnchorStateRegistry.sol | 2 +- .../contracts-bedrock/snapshots/abi/AnchorStateRegistry.json | 4 ++-- .../contracts-bedrock/src/dispute/AnchorStateRegistry.sol | 4 ++-- packages/contracts-bedrock/test/universal/Specs.t.sol | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol b/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol index 1fb45d2541d5f..c52f424a2c2f6 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IAnchorStateRegistry.sol @@ -29,7 +29,7 @@ interface IAnchorStateRegistry { ) external; - function isGameBeyondAirgap(IDisputeGame _game) external view returns (bool); + function isGameAirgapped(IDisputeGame _game) external view returns (bool); function isGameBlacklisted(IDisputeGame _game) external view returns (bool); function isGameProper(IDisputeGame _game) external view returns (bool); function isGameRegistered(IDisputeGame _game) external view returns (bool); diff --git a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json index 8b66e7af20873..1b867e6e8718b 100644 --- a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json +++ b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json @@ -120,7 +120,7 @@ "type": "address" } ], - "name": "isGameBeyondAirgap", + "name": "isGameAirgapped", "outputs": [ { "internalType": "bool", @@ -397,4 +397,4 @@ "name": "AnchorStateRegistry_Unauthorized", "type": "error" } -] \ No newline at end of file +] diff --git a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol index 357438fd4c229..c60522799ad00 100644 --- a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol +++ b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol @@ -163,7 +163,7 @@ contract AnchorStateRegistry is Initializable, ISemver { /// @notice Returns whether a game is beyond the airgap period. /// @param _game The game to check. /// @return Whether the game is beyond the airgap period. - function isGameBeyondAirgap(IDisputeGame _game) public view returns (bool) { + function isGameAirgapped(IDisputeGame _game) public view returns (bool) { return block.timestamp - _game.resolvedAt().raw() > portal.disputeGameFinalityDelaySeconds(); } @@ -215,7 +215,7 @@ contract AnchorStateRegistry is Initializable, ISemver { } // Game must be beyond the airgap period. - if (!isGameBeyondAirgap(_game)) { + if (!isGameAirgapped(_game)) { return false; } diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 2014f40a757b9..4af85fc86d9da 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -558,7 +558,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("getAnchorRoot()") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("disputeGameFactory()") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("initialize(address,address,address,(bytes32,uint256))") }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameBeyondAirgap(address)") }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameAirgapped(address)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameBlacklisted(address)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameClaimValid(address)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameFinalized(address)") }); From f2fcdb2dfa87517156504c2ac426db82bac98261 Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 16:40:16 -0500 Subject: [PATCH 09/28] tiny specs change --- packages/contracts-bedrock/test/universal/Specs.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 4af85fc86d9da..b7534857e38d3 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -569,7 +569,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("isGameRetired(address)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("portal()") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("respectedGameType()") }); - _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("setAnchorState(address)"), _auth: Role.GUARDIAN }); + _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("setAnchorState(address)") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("superchainConfig()") }); _addSpec({ _name: "AnchorStateRegistry", _sel: _getSel("version()") }); From 18a0b648f56cb099d1a3c98e9ad1b5bb19542d71 Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 16:40:54 -0500 Subject: [PATCH 10/28] add snapshots --- .../snapshots/abi/AnchorStateRegistry.json | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json index 1b867e6e8718b..e84074cf6391f 100644 --- a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json +++ b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json @@ -120,7 +120,7 @@ "type": "address" } ], - "name": "isGameAirgapped", + "name": "isGameBeyondAirgap", "outputs": [ { "internalType": "bool", @@ -387,6 +387,11 @@ "name": "Initialized", "type": "event" }, + { + "inputs": [], + "name": "AnchorStateRegistry_AnchorGameBlacklisted", + "type": "error" + }, { "inputs": [], "name": "AnchorStateRegistry_InvalidAnchorGame", @@ -397,4 +402,4 @@ "name": "AnchorStateRegistry_Unauthorized", "type": "error" } -] +] \ No newline at end of file From 643dd5ec9a8835ed09286e0a3e0c94e0feb2fc9a Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 17:23:51 -0500 Subject: [PATCH 11/28] fix specs test and ASR snapshot --- .../snapshots/abi/AnchorStateRegistry.json | 2 +- packages/contracts-bedrock/test/universal/Specs.t.sol | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json index e84074cf6391f..316cc5fcd2f58 100644 --- a/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json +++ b/packages/contracts-bedrock/snapshots/abi/AnchorStateRegistry.json @@ -120,7 +120,7 @@ "type": "address" } ], - "name": "isGameBeyondAirgap", + "name": "isGameAirgapped", "outputs": [ { "internalType": "bool", diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index b7534857e38d3..5e4632ba9699d 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -948,14 +948,14 @@ contract Specification_Test is CommonTest { /// @notice Ensures that the DeputyGuardian is authorized to take all Guardian actions. function test_deputyGuardianAuth_works() public view { - // Additional 2 roles for the DeputyPauseModule. - assertEq(specsByRole[Role.GUARDIAN].length, 5); - assertEq(specsByRole[Role.DEPUTYGUARDIAN].length, specsByRole[Role.GUARDIAN].length + 2); + // Additional 2 roles for the DeputyPauseModule + // Additional role for `setAnchorState` which is in DGM but no longer role-restricted. + assertEq(specsByRole[Role.GUARDIAN].length, 4); + assertEq(specsByRole[Role.DEPUTYGUARDIAN].length, specsByRole[Role.GUARDIAN].length + 3); mapping(bytes4 => Spec) storage dgmFuncSpecs = specs["DeputyGuardianModule"]; mapping(bytes4 => Spec) storage superchainConfigFuncSpecs = specs["SuperchainConfig"]; mapping(bytes4 => Spec) storage portal2FuncSpecs = specs["OptimismPortal2"]; - mapping(bytes4 => Spec) storage anchorRegFuncSpecs = specs["AnchorStateRegistry"]; // Ensure that for each of the DeputyGuardianModule's methods there is a corresponding method on another // system contract authed to the Guardian role. @@ -972,6 +972,5 @@ contract Specification_Test is CommonTest { _assertRolesEq(portal2FuncSpecs[_getSel("setRespectedGameType(uint32)")].auth, Role.GUARDIAN); _assertRolesEq(dgmFuncSpecs[_getSel("setAnchorState(address,address)")].auth, Role.DEPUTYGUARDIAN); - _assertRolesEq(anchorRegFuncSpecs[_getSel("setAnchorState(address)")].auth, Role.GUARDIAN); } } From 198d8b0965f78fb15059d362f381274280bd638e Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 17:39:26 -0500 Subject: [PATCH 12/28] update semver --- packages/contracts-bedrock/src/L1/OptimismPortal2.sol | 4 ++-- .../contracts-bedrock/src/dispute/AnchorStateRegistry.sol | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 9d88419faa9fe..9f6d9b3481f99 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -176,9 +176,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { } /// @notice Semantic version. - /// @custom:semver 3.11.0-beta.11 + /// @custom:semver 3.12.0-beta.1 function version() public pure virtual returns (string memory) { - return "3.11.0-beta.11"; + return "3.12.0-beta.1"; } /// @notice Constructs the OptimismPortal contract. diff --git a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol index c60522799ad00..ad0d466f36783 100644 --- a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol +++ b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol @@ -23,8 +23,8 @@ import { IOptimismPortal2 } from "interfaces/L1/IOptimismPortal2.sol"; /// be initialized with a more recent starting state which reduces the amount of required offchain computation. contract AnchorStateRegistry is Initializable, ISemver { /// @notice Semantic version. - /// @custom:semver 2.1.0-beta.1 - string public constant version = "2.1.0-beta.1"; + /// @custom:semver 2.2.0-beta.1 + string public constant version = "2.2.0-beta.1"; /// @notice Address of the SuperchainConfig contract. ISuperchainConfig public superchainConfig; From 5a3330f01e3a1c02f3d6796521979d54d2e9f1db Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 17:45:48 -0500 Subject: [PATCH 13/28] no compilation rrestrictions when optimizer is off --- packages/contracts-bedrock/foundry.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/contracts-bedrock/foundry.toml b/packages/contracts-bedrock/foundry.toml index 6bacfbfa9e9ed..0c534d1720720 100644 --- a/packages/contracts-bedrock/foundry.toml +++ b/packages/contracts-bedrock/foundry.toml @@ -93,6 +93,7 @@ depth = 32 [profile.cicoverage] optimizer = false +compilation_restrictions = [] [profile.cicoverage.fuzz] runs = 1 @@ -120,6 +121,8 @@ timeout = 300 [profile.lite] optimizer = false +compilation_restrictions = [] + ################################################################ # PROFILE: KONTROL # From cfd2c57462611b315dcb0d3d792311646e2100cf Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 17:54:45 -0500 Subject: [PATCH 14/28] interop portal semver --- packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol index eb691e6f1bf8e..44b6f4b1ecf96 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortalInterop.sol @@ -28,9 +28,9 @@ contract OptimismPortalInterop is OptimismPortal2 { OptimismPortal2(_proofMaturityDelaySeconds, _disputeGameFinalityDelaySeconds) { } - /// @custom:semver +interop-beta.8 + /// @custom:semver +interop-beta.9 function version() public pure override returns (string memory) { - return string.concat(super.version(), "+interop-beta.8"); + return string.concat(super.version(), "+interop-beta.9"); } /// @notice Sets static configuration options for the L2 system. From 05b6c9dbf8aacfbe66820f01bfb0b9526b9eec04 Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Tue, 14 Jan 2025 18:18:10 -0500 Subject: [PATCH 15/28] justfile ignore, semver --- packages/contracts-bedrock/justfile | 2 +- .../contracts-bedrock/snapshots/semver-lock.json | 16 ++++++++-------- .../src/dispute/AnchorStateRegistry.sol | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/contracts-bedrock/justfile b/packages/contracts-bedrock/justfile index 4f087ee5f122a..ef4c451255331 100644 --- a/packages/contracts-bedrock/justfile +++ b/packages/contracts-bedrock/justfile @@ -73,7 +73,7 @@ test-upgrade *ARGS: build-go-ffi #!/bin/bash echo "Running upgrade tests at block $pinnedBlockNumber" export FORK_BLOCK_NUMBER=$pinnedBlockNumber - export NO_MATCH_CONTRACTS="OptimismPortal2WithMockERC20_Test|OptimismPortal2_FinalizeWithdrawal_Test|'AnchorStateRegistry_*'|FaultDisputeGame_Test|FaultDispute_1v1_Actors_Test" + export NO_MATCH_CONTRACTS="OptimismPortal2WithMockERC20_Test|OptimismPortal2_FinalizeWithdrawal_Test|'AnchorStateRegistry_*'|FaultDisputeGame_Test|PermissionedDisputeGame_Test|FaultDispute_1v1_Actors_Test|DelayedWETH_Hold_Test" export NO_MATCH_PATHS="test/dispute/AnchorStateRegistry.t.sol" FORK_RPC_URL=$ETH_RPC_URL \ FORK_TEST=true \ diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 329b86857345a..83e84bbba9ba6 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -20,12 +20,12 @@ "sourceCodeHash": "0x2d21506cc51ebe0b60bcf89883aff5e9b1269567ce44ee779de3d3940e23fb65" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x32eceffc74182357cb79f7218851e02a76200b1e57db7629c2b97b9401d71a25", - "sourceCodeHash": "0x95f68f577529c158070e3a23fd2da1c63581113471e1303716e387a07f3070d9" + "initCodeHash": "0x5121ab634e470633d965d709bb65c8ea97f8cecf84c42aa4541b5eb9a92d6cc9", + "sourceCodeHash": "0x889a4a53f38fa885281be58bad88c25bafec0a9fd7fb2ddfa16dcda1cecaf7d5" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x271c17198970e361cb43aff3ae788d4f726d8d1a188eeea7d0a5f0c477380a76", - "sourceCodeHash": "0xbb6acc3e88af9594ffcb8a2f30860511b76e09024330e70052316668fe55fd1f" + "initCodeHash": "0x137910751db2e7a4edc76728b8433b47911b82fe1514465f2cbc5b35daf2e53a", + "sourceCodeHash": "0xc04a7f9c14a13ec3587f5cc351c8e9f27fbbe9f1291a1aba07de29edbeef418a" }, "src/L1/ProtocolVersions.sol": { "initCodeHash": "0x0000ec89712d8b4609873f1ba76afffd4205bf9110818995c90134dbec12e91e", @@ -152,8 +152,8 @@ "sourceCodeHash": "0xb7b0a06cd971c4647247dc19ce997d0c64a73e87c81d30731da9cf9efa1b952a" }, "src/dispute/AnchorStateRegistry.sol": { - "initCodeHash": "0x751e32c8d44c5ef9b1acae2d94e07f6e91f7f59663c24e12d21298638458b272", - "sourceCodeHash": "0xb09dccd5bf662c42ab10afd19a2b51114df9fd3bc6e20d4d86e4ba7529036bbc" + "initCodeHash": "0x8d04e8c4185388170beabec3de6380628d5f41869e96655322685e33c3541dd9", + "sourceCodeHash": "0x27100c19acf49ffa999baf8f5e5dd1e8f8455e65a51c27d2e949220d0a07ac3b" }, "src/dispute/DelayedWETH.sol": { "initCodeHash": "0xb1f04c9ee86984a157b92a18754c84104e9d4df7a3838633301ca7f557d0220a", @@ -164,8 +164,8 @@ "sourceCodeHash": "0x155c0334f63616ed245aadf9a94f419ef7d5e2237b3b32172484fd19890a61dc" }, "src/dispute/FaultDisputeGame.sol": { - "initCodeHash": "0xd59ffa7f487db9b0761b1fa3994c59e0d78e4546c76379a14a31dec1080b3f20", - "sourceCodeHash": "0xd59fd93e7332bad50714ec1abe06cebd6378d32cc4ad18434f09156ac3c97982" + "initCodeHash": "0x73ab839d9f77e541e0044f70fce1bd706d86f8c8ed310d96948a822b41191885", + "sourceCodeHash": "0x5abda30a900304e680d289be12798eda70677b54d7242499283954f98860a231" }, "src/legacy/DeployerWhitelist.sol": { "initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29", diff --git a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol index ad0d466f36783..b32d360093484 100644 --- a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol +++ b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol @@ -55,7 +55,7 @@ contract AnchorStateRegistry is Initializable, ISemver { /// @notice Thrown when an invalid anchor game is provided. error AnchorStateRegistry_InvalidAnchorGame(); - /// @notice Thrown when an anchor game is blacklisted. + /// @notice Thrown when the anchor root is requested, but the anchor game is blacklisted. error AnchorStateRegistry_AnchorGameBlacklisted(); /// @notice Constructor to disable initializers. From c5b8c57c97d6c449854a1c5fad19c79972653253 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Tue, 14 Jan 2025 21:02:46 -0600 Subject: [PATCH 16/28] minor tweaks --- op-chain-ops/interopgen/recipe.go | 2 +- op-deployer/pkg/deployer/standard/standard.go | 2 +- op-e2e/faultproofs/output_alphabet_test.go | 5 +++++ packages/contracts-bedrock/snapshots/semver-lock.json | 6 +++--- packages/contracts-bedrock/src/L1/OptimismPortal2.sol | 3 +-- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/op-chain-ops/interopgen/recipe.go b/op-chain-ops/interopgen/recipe.go index e70c69e9f481a..30a7c70ac4cb2 100644 --- a/op-chain-ops/interopgen/recipe.go +++ b/op-chain-ops/interopgen/recipe.go @@ -71,7 +71,7 @@ func (r *InteropDevRecipe) Build(addrs devkeys.Addresses) (*WorldConfig, error) Implementations: OPCMImplementationsConfig{ L1ContractsRelease: "dev", FaultProof: SuperFaultProofConfig{ - WithdrawalDelaySeconds: big.NewInt(604800), + WithdrawalDelaySeconds: big.NewInt(302400), MinProposalSizeBytes: big.NewInt(10000), ChallengePeriodSeconds: big.NewInt(120), ProofMaturityDelaySeconds: big.NewInt(12), diff --git a/op-deployer/pkg/deployer/standard/standard.go b/op-deployer/pkg/deployer/standard/standard.go index 50664b959b2a2..dd977afe23910 100644 --- a/op-deployer/pkg/deployer/standard/standard.go +++ b/op-deployer/pkg/deployer/standard/standard.go @@ -15,7 +15,7 @@ const ( GasLimit uint64 = 60_000_000 BasefeeScalar uint32 = 1368 BlobBaseFeeScalar uint32 = 801949 - WithdrawalDelaySeconds uint64 = 604800 + WithdrawalDelaySeconds uint64 = 302400 MinProposalSizeBytes uint64 = 126000 ChallengePeriodSeconds uint64 = 86400 ProofMaturityDelaySeconds uint64 = 604800 diff --git a/op-e2e/faultproofs/output_alphabet_test.go b/op-e2e/faultproofs/output_alphabet_test.go index 258066305ef3e..8497fb9aae994 100644 --- a/op-e2e/faultproofs/output_alphabet_test.go +++ b/op-e2e/faultproofs/output_alphabet_test.go @@ -127,6 +127,11 @@ func TestOutputAlphabetGame_ReclaimBond(t *testing.T) { sys.TimeTravelClock.AdvanceTime(game.CreditUnlockDuration(ctx)) require.NoError(t, wait.ForNextBlock(ctx, l1Client)) + // Advance the time past the finalization delay + // Finalization delay is the same as the credit unlock delay + sys.TimeTravelClock.AdvanceTime(game.CreditUnlockDuration(ctx)) + require.NoError(t, wait.ForNextBlock(ctx, l1Client)) + // Wait for alice to have no available credit // aka, wait for the challenger to claim its credit game.WaitForNoAvailableCredit(ctx, alice) diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 83e84bbba9ba6..7e5791f88230c 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -20,11 +20,11 @@ "sourceCodeHash": "0x2d21506cc51ebe0b60bcf89883aff5e9b1269567ce44ee779de3d3940e23fb65" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x5121ab634e470633d965d709bb65c8ea97f8cecf84c42aa4541b5eb9a92d6cc9", - "sourceCodeHash": "0x889a4a53f38fa885281be58bad88c25bafec0a9fd7fb2ddfa16dcda1cecaf7d5" + "initCodeHash": "0x94c880ae1974be774ec956ed2d9f8d71c19ba500d47abe28856ea8a019dc9c3f", + "sourceCodeHash": "0x99ab9155cd18ee2bbab440431923f944a89535670546bfaa01e637eadb0342b8" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x137910751db2e7a4edc76728b8433b47911b82fe1514465f2cbc5b35daf2e53a", + "initCodeHash": "0x529e192525bc0f28b4402439af1defcf5407016b44a3eda2da5a6260b31779e2", "sourceCodeHash": "0xc04a7f9c14a13ec3587f5cc351c8e9f27fbbe9f1291a1aba07de29edbeef418a" }, "src/L1/ProtocolVersions.sol": { diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 9f6d9b3481f99..f7fc1adc41767 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -482,11 +482,10 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { // respectedGameTypeUpdatedAt timestamp without modifying this function's signature. if (_gameType.raw() == type(uint32).max) { respectedGameTypeUpdatedAt = uint64(block.timestamp); - emit RespectedGameTypeSet(respectedGameType, Timestamp.wrap(respectedGameTypeUpdatedAt)); } else { respectedGameType = _gameType; - emit RespectedGameTypeSet(_gameType, Timestamp.wrap(respectedGameTypeUpdatedAt)); } + emit RespectedGameTypeSet(respectedGameType, Timestamp.wrap(respectedGameTypeUpdatedAt)); } /// @notice Checks if a withdrawal can be finalized. This function will revert if the withdrawal cannot be From d1a13795474eb28ba4f49dab0f3b28ad3e6688dc Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Tue, 14 Jan 2025 21:23:11 -0600 Subject: [PATCH 17/28] expanded test coverage --- op-e2e/faultproofs/output_alphabet_test.go | 3 +- .../test/dispute/AnchorStateRegistry.t.sol | 202 ++++++++++++++++++ .../test/dispute/FaultDisputeGame.t.sol | 123 +++++++++++ 3 files changed, 327 insertions(+), 1 deletion(-) diff --git a/op-e2e/faultproofs/output_alphabet_test.go b/op-e2e/faultproofs/output_alphabet_test.go index 8497fb9aae994..dcafe8133275a 100644 --- a/op-e2e/faultproofs/output_alphabet_test.go +++ b/op-e2e/faultproofs/output_alphabet_test.go @@ -129,7 +129,8 @@ func TestOutputAlphabetGame_ReclaimBond(t *testing.T) { // Advance the time past the finalization delay // Finalization delay is the same as the credit unlock delay - sys.TimeTravelClock.AdvanceTime(game.CreditUnlockDuration(ctx)) + // But just warp way into the future to be safe + sys.TimeTravelClock.AdvanceTime(game.CreditUnlockDuration(ctx) * 2) require.NoError(t, wait.ForNextBlock(ctx, l1Client)) // Wait for alice to have no available credit diff --git a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol index a26cd707e573c..32526d8d69b8c 100644 --- a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol +++ b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol @@ -257,6 +257,7 @@ contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { } /// @notice Tests that isGameProper will return false if the game is not the respected game type. + /// @param _gameType The game type to use for the test. function testFuzz_isGameProper_isNotRespected_succeeds(GameType _gameType) public { if (_gameType.raw() == gameProxy.gameType().raw()) { _gameType = GameType.wrap(_gameType.raw() + 1); @@ -283,6 +284,7 @@ contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { } /// @notice Tests that isGameProper will return false if the game is retired. + /// @param _retirementTimestamp The retirement timestamp to use for the test. function testFuzz_isGameProper_isRetired_succeeds(uint64 _retirementTimestamp) public { // Make sure retirement timestamp is later than the game's creation time. _retirementTimestamp = uint64(bound(_retirementTimestamp, gameProxy.createdAt().raw() + 1, type(uint64).max)); @@ -298,6 +300,206 @@ contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { } } +contract AnchorStateRegistry_IsGameResolved_Test is AnchorStateRegistry_Init { + /// @notice Tests that isGameResolved will return true if the game is resolved. + /// @param _resolvedAtTimestamp The resolvedAt timestamp to use for the test. + function testFuzz_isGameResolved_challengerWins_succeeds(uint256 _resolvedAtTimestamp) public { + // Bound resolvedAt to be less than or equal to current timestamp. + _resolvedAtTimestamp = bound(_resolvedAtTimestamp, 1, block.timestamp); + + // Mock the resolvedAt timestamp. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(_resolvedAtTimestamp)); + + // Mock the status to be CHALLENGER_WINS. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.CHALLENGER_WINS)); + + // Game should be resolved. + assertTrue(anchorStateRegistry.isGameResolved(gameProxy)); + } + + /// @notice Tests that isGameResolved will return true if the game is resolved. + /// @param _resolvedAtTimestamp The resolvedAt timestamp to use for the test. + function testFuzz_isGameResolved_defenderWins_succeeds(uint256 _resolvedAtTimestamp) public { + // Bound resolvedAt to be less than or equal to current timestamp. + _resolvedAtTimestamp = bound(_resolvedAtTimestamp, 1, block.timestamp); + + // Mock the resolvedAt timestamp. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(_resolvedAtTimestamp)); + + // Mock the status to be DEFENDER_WINS. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); + + // Game should be resolved. + assertTrue(anchorStateRegistry.isGameResolved(gameProxy)); + } + + /// @notice Tests that isGameResolved will return false if the game is in progress and not resolved. + /// @param _resolvedAtTimestamp The resolvedAt timestamp to use for the test. + function testFuzz_isGameResolved_inProgressNotResolved_succeeds(uint256 _resolvedAtTimestamp) public { + // Bound resolvedAt to be less than or equal to current timestamp. + _resolvedAtTimestamp = bound(_resolvedAtTimestamp, 1, block.timestamp); + + // Mock the resolvedAt timestamp. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(_resolvedAtTimestamp)); + + // Mock the status to be IN_PROGRESS. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.IN_PROGRESS)); + + // Game should not be resolved. + assertFalse(anchorStateRegistry.isGameResolved(gameProxy)); + } +} + +contract AnchorStateRegistry_IsGameAirgapped_TestFail is AnchorStateRegistry_Init { + /// @notice Tests that isGameAirgapped will return true if the game is airgapped. + /// @param _resolvedAtTimestamp The resolvedAt timestamp to use for the test. + function testFuzz_isGameAirgapped_isAirgapped_succeeds(uint256 _resolvedAtTimestamp) public { + // Warp forward by disputeGameFinalityDelaySeconds. + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds()); + + // Bound resolvedAt to be at least disputeGameFinalityDelaySeconds in the past. + _resolvedAtTimestamp = + bound(_resolvedAtTimestamp, 0, block.timestamp - optimismPortal2.disputeGameFinalityDelaySeconds() - 1); + + // Mock the resolvedAt timestamp. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(_resolvedAtTimestamp)); + + // Game should be airgapped. + assertTrue(anchorStateRegistry.isGameAirgapped(gameProxy)); + } + + /// @notice Tests that isGameAirgapped will return false if the game is not airgapped. + /// @param _resolvedAtTimestamp The resolvedAt timestamp to use for the test. + function testFuzz_isGameAirgapped_isNotAirgapped_succeeds(uint256 _resolvedAtTimestamp) public { + // Warp forward by disputeGameFinalityDelaySeconds. + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds()); + + // Bound resolvedAt to be less than disputeGameFinalityDelaySeconds in the past. + _resolvedAtTimestamp = bound( + _resolvedAtTimestamp, block.timestamp - optimismPortal2.disputeGameFinalityDelaySeconds(), block.timestamp + ); + + // Mock the resolvedAt timestamp. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(_resolvedAtTimestamp)); + + // Game should not be airgapped. + assertFalse(anchorStateRegistry.isGameAirgapped(gameProxy)); + } +} + +contract AnchorStateRegistry_IsGameClaimValid_Test is AnchorStateRegistry_Init { + /// @notice Tests that isGameClaimValid will return true if the game claim is valid. + /// @param _resolvedAtTimestamp The resolvedAt timestamp to use for the test. + function testFuzz_isGameClaimValid_claimIsValid_succeeds(uint256 _resolvedAtTimestamp) public { + // Warp forward by disputeGameFinalityDelaySeconds. + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds()); + + // Bound resolvedAt to be at least disputeGameFinalityDelaySeconds in the past. + _resolvedAtTimestamp = + bound(_resolvedAtTimestamp, 1, block.timestamp - optimismPortal2.disputeGameFinalityDelaySeconds() - 1); + + // Mock that the game was respected. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); + + // Mock the resolvedAt timestamp. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(_resolvedAtTimestamp)); + + // Mock the status to be DEFENDER_WINS. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.DEFENDER_WINS)); + + // Claim should be valid. + assertTrue(anchorStateRegistry.isGameClaimValid(gameProxy)); + } + + /// @notice Tests that isGameClaimValid will return false if the game is not registered. + function testFuzz_isGameClaimValid_notRegistered_succeeds() public { + // Mock the DisputeGameFactory to make it seem that the game was not registered. + vm.mockCall( + address(disputeGameFactory), + abi.encodeCall( + disputeGameFactory.games, (gameProxy.gameType(), gameProxy.rootClaim(), gameProxy.extraData()) + ), + abi.encode(address(0), 0) + ); + + // Claim should not be valid. + assertFalse(anchorStateRegistry.isGameClaimValid(gameProxy)); + } + + /// @notice Tests that isGameClaimValid will return false if the game is not respected. + /// @param _gameType The game type to use for the test. + function testFuzz_isGameClaimValid_isNotRespected_succeeds(GameType _gameType) public { + if (_gameType.raw() == gameProxy.gameType().raw()) { + _gameType = GameType.wrap(_gameType.raw() + 1); + } + + // Mock that the game was not respected. + vm.mockCall( + address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(false) + ); + + // Claim should not be valid. + assertFalse(anchorStateRegistry.isGameClaimValid(gameProxy)); + } + + /// @notice Tests that isGameClaimValid will return false if the game is blacklisted. + function testFuzz_isGameClaimValid_isBlacklisted_succeeds() public { + // Mock the disputeGameBlacklist call to return true. + vm.mockCall( + address(optimismPortal2), + abi.encodeCall(optimismPortal2.disputeGameBlacklist, (gameProxy)), + abi.encode(true) + ); + + // Claim should not be valid. + assertFalse(anchorStateRegistry.isGameClaimValid(gameProxy)); + } + + /// @notice Tests that isGameClaimValid will return false if the game is retired. + /// @param _resolvedAtTimestamp The resolvedAt timestamp to use for the test. + function testFuzz_isGameClaimValid_isRetired_succeeds(uint256 _resolvedAtTimestamp) public { + // Make sure retirement timestamp is later than the game's creation time. + _resolvedAtTimestamp = uint64(bound(_resolvedAtTimestamp, gameProxy.createdAt().raw() + 1, type(uint64).max)); + + // Mock the respectedGameTypeUpdatedAt call to be later than the game's creation time. + vm.mockCall( + address(optimismPortal2), + abi.encodeCall(optimismPortal2.respectedGameTypeUpdatedAt, ()), + abi.encode(_resolvedAtTimestamp) + ); + + // Claim should not be valid. + assertFalse(anchorStateRegistry.isGameClaimValid(gameProxy)); + } + + /// @notice Tests that isGameClaimValid will return false if the game is not resolved. + function testFuzz_isGameClaimValid_notResolved_succeeds() public { + // Mock the status to be IN_PROGRESS. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.status, ()), abi.encode(GameStatus.IN_PROGRESS)); + + // Claim should not be valid. + assertFalse(anchorStateRegistry.isGameClaimValid(gameProxy)); + } + + /// @notice Tests that isGameClaimValid will return false if the game is not airgapped. + /// @param _resolvedAtTimestamp The resolvedAt timestamp to use for the test. + function testFuzz_isGameClaimValid_notAirgapped_succeeds(uint256 _resolvedAtTimestamp) public { + // Warp forward by disputeGameFinalityDelaySeconds. + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds()); + + // Bound resolvedAt to be less than disputeGameFinalityDelaySeconds in the past. + _resolvedAtTimestamp = bound( + _resolvedAtTimestamp, block.timestamp - optimismPortal2.disputeGameFinalityDelaySeconds(), block.timestamp + ); + + // Mock the resolvedAt timestamp. + vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.resolvedAt, ()), abi.encode(_resolvedAtTimestamp)); + + // Claim should not be valid. + assertFalse(anchorStateRegistry.isGameClaimValid(gameProxy)); + } +} + contract AnchorStateRegistry_SetAnchorState_Test is AnchorStateRegistry_Init { /// @notice Tests that setAnchorState will succeed if the game is valid, the game block /// number is greater than the current anchor root block number, and the game is the diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol index 5f09995c71376..788783b99d3c5 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol @@ -351,6 +351,44 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { }); } + /// @dev Tests that the constructor of the `FaultDisputeGame` reverts when the `_gameType` + /// parameter is set to the reserved `type(uint32).max` game type. + function test_constructor_reservedGameType_reverts() public { + AlphabetVM alphabetVM = new AlphabetVM( + absolutePrestate, + IPreimageOracle( + DeployUtils.create1({ + _name: "PreimageOracle", + _args: DeployUtils.encodeConstructor(abi.encodeCall(IPreimageOracle.__constructor__, (0, 0))) + }) + ) + ); + + vm.expectRevert(ReservedGameType.selector); + DeployUtils.create1({ + _name: "FaultDisputeGame", + _args: DeployUtils.encodeConstructor( + abi.encodeCall( + IFaultDisputeGame.__constructor__, + ( + IFaultDisputeGame.GameConstructorParams({ + gameType: GameType.wrap(type(uint32).max), + absolutePrestate: absolutePrestate, + maxGameDepth: 16, + splitDepth: 8, + clockExtension: Duration.wrap(3 hours), + maxClockDuration: Duration.wrap(3.5 days), + vm: alphabetVM, + weth: IDelayedWETH(payable(address(0))), + anchorStateRegistry: IAnchorStateRegistry(address(0)), + l2ChainId: 10 + }) + ) + ) + ) + }); + } + /// @dev Tests that the game's root claim is set correctly. function test_rootClaim_succeeds() public view { assertEq(gameProxy.rootClaim().raw(), ROOT_CLAIM.raw()); @@ -460,6 +498,20 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { assertEq(gameProxy.l1Head().raw(), blockhash(block.number - 1)); } + /// @dev Tests that the game cannot be initialized when the anchor root is not found. + function test_initialize_anchorRootNotFound_reverts() public { + // Mock the AnchorStateRegistry to return a zero root. + vm.mockCall( + address(anchorStateRegistry), + abi.encodeCall(IAnchorStateRegistry.getAnchorRoot, ()), + abi.encode(Hash.wrap(bytes32(0)), 0) + ); + + // Creation should fail. + vm.expectRevert(AnchorRootNotFound.selector); + gameProxy = IFaultDisputeGame(payable(address(disputeGameFactory.create(GAME_TYPE, _dummyClaim(), hex"")))); + } + /// @dev Tests that the game cannot be initialized twice. function test_initialize_onlyOnce_succeeds() public { vm.expectRevert(AlreadyInitialized.selector); @@ -1819,6 +1871,77 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { assertEq(updatedRoot.raw(), root.raw()); } + function test_claimCredit_refundMode_succeeds() public { + // Set up actors. + address alice = address(0xa11ce); + address bob = address(0xb0b); + + // Give the game proxy 1 extra ether, unregistered. + vm.deal(address(gameProxy), 1 ether); + + // Perform a bonded move. + Claim claim = _dummyClaim(); + + // Bond the first claim. + uint256 firstBond = _getRequiredBond(0); + vm.deal(alice, firstBond); + (,,,, Claim disputed,,) = gameProxy.claimData(0); + vm.prank(alice); + gameProxy.attack{ value: firstBond }(disputed, 0, claim); + + // Bond the second claim. + uint256 secondBond = _getRequiredBond(1); + vm.deal(bob, secondBond); + (,,,, disputed,,) = gameProxy.claimData(1); + vm.prank(bob); + gameProxy.attack{ value: secondBond }(disputed, 1, claim); + + // Warp past the finalization period + vm.warp(block.timestamp + 3 days + 12 hours); + + // Resolve the game. + // Second claim wins, so bob should get alice's credit. + gameProxy.resolveClaim(2, 0); + gameProxy.resolveClaim(1, 0); + gameProxy.resolveClaim(0, 0); + gameProxy.resolve(); + + // Wait for finalization delay. + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Mock that the game proxy is not proper, trigger refund mode. + vm.mockCall( + address(anchorStateRegistry), + abi.encodeCall(anchorStateRegistry.isGameProper, (gameProxy)), + abi.encode(false) + ); + + // Close the game. + gameProxy.closeGame(); + + // Assert bond distribution mode is refund mode. + assertTrue(gameProxy.bondDistributionMode() == BondDistributionMode.REFUND); + + // Claim credit once to trigger unlock period. + gameProxy.claimCredit(alice); + gameProxy.claimCredit(bob); + + // Wait for the withdrawal delay. + vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); + + // Grab balances before claim. + uint256 aliceBalanceBefore = alice.balance; + uint256 bobBalanceBefore = bob.balance; + + // Claim credit again to get the bond back. + gameProxy.claimCredit(alice); + gameProxy.claimCredit(bob); + + // Should have original balance again. + assertEq(alice.balance, aliceBalanceBefore + firstBond); + assertEq(bob.balance, bobBalanceBefore + secondBond); + } + /// @dev Static unit test asserting that credit may not be drained past allowance through reentrancy. function test_claimCredit_claimAlreadyResolved_reverts() public { ClaimCreditReenter reenter = new ClaimCreditReenter(gameProxy, vm); From 87a3a95421aa5cea88e28a3de510d6aa31ee2974 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 15 Jan 2025 09:02:48 -0600 Subject: [PATCH 18/28] various logical tweaks --- .../snapshots/semver-lock.json | 10 ++++---- .../src/L1/OptimismPortal2.sol | 12 ++++++++++ .../src/dispute/AnchorStateRegistry.sol | 24 +++++++++---------- .../test/dispute/AnchorStateRegistry.t.sol | 9 ++++--- 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 7e5791f88230c..dd9b212b41f09 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -20,11 +20,11 @@ "sourceCodeHash": "0x2d21506cc51ebe0b60bcf89883aff5e9b1269567ce44ee779de3d3940e23fb65" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x94c880ae1974be774ec956ed2d9f8d71c19ba500d47abe28856ea8a019dc9c3f", - "sourceCodeHash": "0x99ab9155cd18ee2bbab440431923f944a89535670546bfaa01e637eadb0342b8" + "initCodeHash": "0xb906cca6c1d2ef7d783f94d502bbf8704f39488bcbe4b1633f21d5a25f39d750", + "sourceCodeHash": "0x2fa2648a82059eb6fc052254678d3bd2bf7952be83ab53c545c9302df3b9d1a0" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x529e192525bc0f28b4402439af1defcf5407016b44a3eda2da5a6260b31779e2", + "initCodeHash": "0x92e46899517ecb927a6fea35995ebf6cc534443351d1f5806a7d34c5696c0648", "sourceCodeHash": "0xc04a7f9c14a13ec3587f5cc351c8e9f27fbbe9f1291a1aba07de29edbeef418a" }, "src/L1/ProtocolVersions.sol": { @@ -152,8 +152,8 @@ "sourceCodeHash": "0xb7b0a06cd971c4647247dc19ce997d0c64a73e87c81d30731da9cf9efa1b952a" }, "src/dispute/AnchorStateRegistry.sol": { - "initCodeHash": "0x8d04e8c4185388170beabec3de6380628d5f41869e96655322685e33c3541dd9", - "sourceCodeHash": "0x27100c19acf49ffa999baf8f5e5dd1e8f8455e65a51c27d2e949220d0a07ac3b" + "initCodeHash": "0xf7b9422d4c0b175aed8fad9ac4e03054625a667cff837a9a13612ec3bb2ee936", + "sourceCodeHash": "0xe355003779215d2c9d8c55e84c3e8b984a2c284da6b222de0874722bc71fe598" }, "src/dispute/DelayedWETH.sol": { "initCodeHash": "0xb1f04c9ee86984a157b92a18754c84104e9d4df7a3838633301ca7f557d0220a", diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index f7fc1adc41767..5edb0d44715c3 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -308,6 +308,18 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { // The game type of the dispute game must be the respected game type. if (gameType.raw() != respectedGameType.raw()) revert InvalidGameType(); + // The game type of the DisputeGame must have been the respected game type at creation. + if (!gameProxy.wasRespectedGameTypeWhenCreated()) revert InvalidGameType(); + + // Creation time must be greater than or equal to the respected game type updated time. + // Games created before this timestamp are not valid and cannot be used to finalize a + // withdrawal, so for user convenience we also prevent them from being used to prove a + // withdrawal. + require( + gameProxy.createdAt().raw() >= respectedGameTypeUpdatedAt, + "OptimismPortal: dispute game created before respected game type was updated" + ); + // Verify that the output root can be generated with the elements in the proof. if (outputRoot.raw() != Hashing.hashOutputRootProof(_outputRootProof)) revert InvalidProof(); diff --git a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol index b32d360093484..fb9bbc44b996f 100644 --- a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol +++ b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol @@ -172,13 +172,12 @@ contract AnchorStateRegistry is Initializable, ISemver { /// invalidation conditions. The root claim of a proper game IS NOT guaranteed to be /// valid. The root claim of a proper game CAN BE incorrect and still be a proper game. /// DO NOT USE THIS FUNCTION ALONE TO DETERMINE IF A ROOT CLAIM IS VALID. - /// @dev Note that it is possible for games to be created when their game type is not the - /// respected game type. We do not consider these games to be Proper Games. isGameProper() - /// can currently guarantee this because the OptimismPortal contract will always set the - /// retirement timestamp whenever the respected game type is updated such that any games - /// created before any update of the respected game type are automatically retired. If - /// this coupling is broken, then we must instead check that the game type *was* the - /// respected game type at the time of the game's creation. + /// @dev Note that isGameProper previously checked that the game type was equal to the + /// respected game type. However, it should be noted that it is possible for a game other + /// than the respected game type to resolve without being invalidated. Since isGameProper + /// exists to determine if a game has (or has not) been invalidated, we now allow any game + /// type to be considered a proper game. We enforce checks on the game type in + /// isGameClaimValid(). /// @param _game The game to check. /// @return Whether the game is a proper game. function isGameProper(IDisputeGame _game) public view returns (bool) { @@ -187,11 +186,6 @@ contract AnchorStateRegistry is Initializable, ISemver { return false; } - // Must be respected game type. - if (!isGameRespected(_game)) { - return false; - } - // Must not be blacklisted. if (isGameBlacklisted(_game)) { return false; @@ -232,6 +226,12 @@ contract AnchorStateRegistry is Initializable, ISemver { return false; } + // Must be respected. + bool respected = isGameRespected(_game); + if (!respected) { + return false; + } + // Game must be finalized. bool finalized = isGameFinalized(_game); if (!finalized) { diff --git a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol index 32526d8d69b8c..3a47b4ea4111f 100644 --- a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol +++ b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol @@ -236,9 +236,7 @@ contract AnchorStateRegistry_IsGameRetired_Test is AnchorStateRegistry_Init { contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { /// @notice Tests that isGameProper will return true if the game meets all conditions. function test_isGameProper_meetsAllConditions_succeeds() public { - // Mock that the game was respected. - vm.mockCall(address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(true)); - + // Game will meet all conditions by default. assertTrue(anchorStateRegistry.isGameProper(gameProxy)); } @@ -258,7 +256,7 @@ contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { /// @notice Tests that isGameProper will return false if the game is not the respected game type. /// @param _gameType The game type to use for the test. - function testFuzz_isGameProper_isNotRespected_succeeds(GameType _gameType) public { + function testFuzz_isGameProper_anyGameType_succeeds(GameType _gameType) public { if (_gameType.raw() == gameProxy.gameType().raw()) { _gameType = GameType.wrap(_gameType.raw() + 1); } @@ -268,7 +266,8 @@ contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { address(gameProxy), abi.encodeCall(gameProxy.wasRespectedGameTypeWhenCreated, ()), abi.encode(false) ); - assertFalse(anchorStateRegistry.isGameProper(gameProxy)); + // Still a proper game. + assertTrue(anchorStateRegistry.isGameProper(gameProxy)); } /// @notice Tests that isGameProper will return false if the game is blacklisted. From 2256be93883e9321302b569f43d9db505e37d03d Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 15 Jan 2025 09:12:58 -0600 Subject: [PATCH 19/28] test fix --- packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 9070a57d0c938..c27376c4f91ce 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -672,13 +672,15 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { _withdrawalProof: _withdrawalProof }); + // Create a new game. + IDisputeGame newGame = + disputeGameFactory.create(GameType.wrap(0), Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber + 1)); + // Update the respected game type to 0xbeef. vm.prank(optimismPortal2.guardian()); optimismPortal2.setRespectedGameType(GameType.wrap(0xbeef)); // Create a new game and mock the game type as 0xbeef in the factory. - IDisputeGame newGame = - disputeGameFactory.create(GameType.wrap(0), Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber + 1)); vm.mockCall( address(disputeGameFactory), abi.encodeCall(disputeGameFactory.gameAtIndex, (_proposedGameIndex + 1)), From c643792449ddc9b8a56017e01b3e44f7d3b1b56a Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 15 Jan 2025 09:37:30 -0600 Subject: [PATCH 20/28] clearer error --- op-e2e/faultproofs/output_alphabet_test.go | 2 +- .../interfaces/L1/IOptimismPortal2.sol | 1 + .../interfaces/L1/IOptimismPortalInterop.sol | 1 + .../contracts-bedrock/snapshots/semver-lock.json | 6 +++--- .../contracts-bedrock/src/L1/OptimismPortal2.sol | 16 +++++++++++++--- .../src/libraries/PortalErrors.sol | 2 ++ .../test/dispute/AnchorStateRegistry.t.sol | 2 +- 7 files changed, 22 insertions(+), 8 deletions(-) diff --git a/op-e2e/faultproofs/output_alphabet_test.go b/op-e2e/faultproofs/output_alphabet_test.go index dcafe8133275a..f9982a8d1f3d6 100644 --- a/op-e2e/faultproofs/output_alphabet_test.go +++ b/op-e2e/faultproofs/output_alphabet_test.go @@ -121,7 +121,7 @@ func TestOutputAlphabetGame_ReclaimBond(t *testing.T) { // The actor should have no credit available because all its bonds were paid to Alice. actorCredit := game.AvailableCredit(ctx, disputegame.TestAddress) - require.True(t, actorCredit.Cmp(big.NewInt(0)) == 0, "Expected alice available credit to be zero") + require.True(t, actorCredit.Cmp(big.NewInt(0)) == 0, "Expected actor available credit to be zero") // Advance the time past the weth unlock delay sys.TimeTravelClock.AdvanceTime(game.CreditUnlockDuration(ctx)) diff --git a/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol b/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol index e1223d7d0f75a..bbf12c3fce4dc 100644 --- a/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol +++ b/packages/contracts-bedrock/interfaces/L1/IOptimismPortal2.sol @@ -31,6 +31,7 @@ interface IOptimismPortal2 { error UnexpectedList(); error UnexpectedString(); error Unproven(); + error LegacyGame(); event DisputeGameBlacklisted(IDisputeGame indexed disputeGame); event Initialized(uint8 version); diff --git a/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol b/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol index ed5f980e1839f..91ee7f7d0d7e5 100644 --- a/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol +++ b/packages/contracts-bedrock/interfaces/L1/IOptimismPortalInterop.sol @@ -33,6 +33,7 @@ interface IOptimismPortalInterop { error UnexpectedList(); error UnexpectedString(); error Unproven(); + error LegacyGame(); event DisputeGameBlacklisted(IDisputeGame indexed disputeGame); event Initialized(uint8 version); diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index dd9b212b41f09..71788e938e9ac 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -20,11 +20,11 @@ "sourceCodeHash": "0x2d21506cc51ebe0b60bcf89883aff5e9b1269567ce44ee779de3d3940e23fb65" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0xb906cca6c1d2ef7d783f94d502bbf8704f39488bcbe4b1633f21d5a25f39d750", - "sourceCodeHash": "0x2fa2648a82059eb6fc052254678d3bd2bf7952be83ab53c545c9302df3b9d1a0" + "initCodeHash": "0x3f30480b47af0a965af7d38eff673fe1ef0e8704c1776eaef5648d242cfe376d", + "sourceCodeHash": "0x6241a2e1e5b8fae5e17e13abcccd3d013f66317a6d7611d844b10bbb878d38c0" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x92e46899517ecb927a6fea35995ebf6cc534443351d1f5806a7d34c5696c0648", + "initCodeHash": "0x9e12728cae72a1c701ddc3e09fe9e3b82aa38674ab76e6d9e74f3a384dfe25d2", "sourceCodeHash": "0xc04a7f9c14a13ec3587f5cc351c8e9f27fbbe9f1291a1aba07de29edbeef418a" }, "src/L1/ProtocolVersions.sol": { diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index 5edb0d44715c3..ff0c8ec92ab5c 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -28,7 +28,8 @@ import { Blacklisted, Unproven, ProposalNotValidated, - AlreadyFinalized + AlreadyFinalized, + LegacyGame } from "src/libraries/PortalErrors.sol"; import { GameStatus, GameType, Claim, Timestamp } from "src/dispute/lib/Types.sol"; @@ -309,7 +310,11 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { if (gameType.raw() != respectedGameType.raw()) revert InvalidGameType(); // The game type of the DisputeGame must have been the respected game type at creation. - if (!gameProxy.wasRespectedGameTypeWhenCreated()) revert InvalidGameType(); + try gameProxy.wasRespectedGameTypeWhenCreated() returns (bool wasRespected_) { + if (!wasRespected_) revert InvalidGameType(); + } catch { + revert LegacyGame(); + } // Creation time must be greater than or equal to the respected game type updated time. // Games created before this timestamp are not valid and cannot be used to finalize a @@ -541,7 +546,12 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { // time. We check that the game type is the respected game type at proving time, but it's // possible that the respected game type has since changed. Users can still use this game // to finalize a withdrawal as long as it has not been otherwise invalidated. - if (!disputeGameProxy.wasRespectedGameTypeWhenCreated()) revert InvalidGameType(); + // The game type of the DisputeGame must have been the respected game type at creation. + try disputeGameProxy.wasRespectedGameTypeWhenCreated() returns (bool wasRespected_) { + if (!wasRespected_) revert InvalidGameType(); + } catch { + revert LegacyGame(); + } // The game must have been created after `respectedGameTypeUpdatedAt`. This is to prevent users from creating // invalid disputes against a deployed game type while the off-chain challenge agents are not watching. diff --git a/packages/contracts-bedrock/src/libraries/PortalErrors.sol b/packages/contracts-bedrock/src/libraries/PortalErrors.sol index a68f50328b58a..6004066b397ee 100644 --- a/packages/contracts-bedrock/src/libraries/PortalErrors.sol +++ b/packages/contracts-bedrock/src/libraries/PortalErrors.sol @@ -38,3 +38,5 @@ error Unproven(); error ProposalNotValidated(); /// @notice Error for when a withdrawal has already been finalized. error AlreadyFinalized(); +/// @notice Error for when a game is a legacy game. +error LegacyGame(); diff --git a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol index 3a47b4ea4111f..86fe7f62b25dd 100644 --- a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol +++ b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol @@ -235,7 +235,7 @@ contract AnchorStateRegistry_IsGameRetired_Test is AnchorStateRegistry_Init { contract AnchorStateRegistry_IsGameProper_Test is AnchorStateRegistry_Init { /// @notice Tests that isGameProper will return true if the game meets all conditions. - function test_isGameProper_meetsAllConditions_succeeds() public { + function test_isGameProper_meetsAllConditions_succeeds() public view { // Game will meet all conditions by default. assertTrue(anchorStateRegistry.isGameProper(gameProxy)); } From 096d4889b83c3400857c3616ad883c3f644f16dc Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 15 Jan 2025 09:50:04 -0600 Subject: [PATCH 21/28] fix test flake in go tests --- .../game/fault/contracts/faultdisputegame.go | 10 ++++++++++ op-e2e/e2eutils/disputegame/output_game_helper.go | 12 ++++++++++++ op-e2e/faultproofs/output_alphabet_test.go | 3 +++ .../snapshots/abi/OptimismPortal2.json | 5 +++++ .../snapshots/abi/OptimismPortalInterop.json | 5 +++++ 5 files changed, 35 insertions(+) diff --git a/op-challenger/game/fault/contracts/faultdisputegame.go b/op-challenger/game/fault/contracts/faultdisputegame.go index 6bee1961018e0..789be3b438eb6 100644 --- a/op-challenger/game/fault/contracts/faultdisputegame.go +++ b/op-challenger/game/fault/contracts/faultdisputegame.go @@ -53,6 +53,7 @@ var ( methodL2BlockNumberChallenged = "l2BlockNumberChallenged" methodL2BlockNumberChallenger = "l2BlockNumberChallenger" methodChallengeRootL2Block = "challengeRootL2Block" + methodBondDistributionMode = "bondDistributionMode" ) var ( @@ -455,6 +456,14 @@ func (f *FaultDisputeGameContractLatest) GetAllClaims(ctx context.Context, block return claims, nil } +func (f *FaultDisputeGameContractLatest) BondDistributionMode(ctx context.Context) (uint8, error) { + result, err := f.multiCaller.SingleCall(ctx, rpcblock.Latest, f.contract.Call(methodBondDistributionMode)) + if err != nil { + return 0, fmt.Errorf("failed to fetch bond mode: %w", err) + } + return result.GetUint8(0), nil +} + func (f *FaultDisputeGameContractLatest) IsResolved(ctx context.Context, block rpcblock.Block, claims ...types.Claim) ([]bool, error) { defer f.metrics.StartContractRequest("IsResolved")() calls := make([]batching.Call, 0, len(claims)) @@ -639,4 +648,5 @@ type FaultDisputeGameContract interface { CallResolve(ctx context.Context) (gameTypes.GameStatus, error) ResolveTx() (txmgr.TxCandidate, error) Vm(ctx context.Context) (*VMContract, error) + BondDistributionMode(ctx context.Context) (uint8, error) } diff --git a/op-e2e/e2eutils/disputegame/output_game_helper.go b/op-e2e/e2eutils/disputegame/output_game_helper.go index 0468620537f11..4c0192623d006 100644 --- a/op-e2e/e2eutils/disputegame/output_game_helper.go +++ b/op-e2e/e2eutils/disputegame/output_game_helper.go @@ -368,6 +368,18 @@ func (g *OutputGameHelper) Status(ctx context.Context) gameTypes.GameStatus { return status } +func (g *OutputGameHelper) WaitForBondModeSet(ctx context.Context) { + g.T.Logf("Waiting for game %v to have bond mode set", g.Addr) + timedCtx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + err := wait.For(timedCtx, time.Second, func() (bool, error) { + bondMode, err := g.Game.BondDistributionMode(ctx) + g.Require.NoError(err) + return bondMode != 0, nil + }) + g.Require.NoError(err, "Failed to wait for bond mode to be set") +} + func (g *OutputGameHelper) WaitForGameStatus(ctx context.Context, expected gameTypes.GameStatus) { g.T.Logf("Waiting for game %v to have status %v", g.Addr, expected) timedCtx, cancel := context.WithTimeout(ctx, defaultTimeout) diff --git a/op-e2e/faultproofs/output_alphabet_test.go b/op-e2e/faultproofs/output_alphabet_test.go index f9982a8d1f3d6..2d2cb2ab3ddca 100644 --- a/op-e2e/faultproofs/output_alphabet_test.go +++ b/op-e2e/faultproofs/output_alphabet_test.go @@ -115,6 +115,9 @@ func TestOutputAlphabetGame_ReclaimBond(t *testing.T) { game.WaitForGameStatus(ctx, types.GameStatusChallengerWon) game.LogGameData(ctx) + // Wait for the game to have bond mode set + game.WaitForBondModeSet(ctx) + // Expect Alice's credit to be non-zero // But it can't be claimed right now since there is a delay on the weth unlock require.Truef(t, game.AvailableCredit(ctx, alice).Cmp(big.NewInt(0)) > 0, "Expected alice credit to be above zero") diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json b/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json index 00c7a4061e35a..71b8677f7dd85 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismPortal2.json @@ -812,6 +812,11 @@ "name": "LargeCalldata", "type": "error" }, + { + "inputs": [], + "name": "LegacyGame", + "type": "error" + }, { "inputs": [], "name": "NonReentrant", diff --git a/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json b/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json index 3a22e3528dd18..b99b23ad029ff 100644 --- a/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json +++ b/packages/contracts-bedrock/snapshots/abi/OptimismPortalInterop.json @@ -835,6 +835,11 @@ "name": "LargeCalldata", "type": "error" }, + { + "inputs": [], + "name": "LegacyGame", + "type": "error" + }, { "inputs": [], "name": "NonReentrant", From 5d4e6c875c9002aac268525978749494d149937c Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Wed, 15 Jan 2025 12:39:23 -0500 Subject: [PATCH 22/28] add portal tests --- .../test/L1/OptimismPortal2.t.sol | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index c27376c4f91ce..594691e1d57b9 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -585,6 +585,47 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { }); } + /// @dev Tests that `proveWithdrawalTransaction` reverts if the game was not the respected game type when created. + function test_proveWithdrawalTransaction_wasNotRespectedGameTypeWhenCreated_reverts() external { + vm.mockCall( + address(game), abi.encodeWithSelector(game.wasRespectedGameTypeWhenCreated.selector), abi.encode(false) + ); + vm.expectRevert(InvalidGameType.selector); + optimismPortal2.proveWithdrawalTransaction({ + _tx: _defaultTx, + _disputeGameIndex: _proposedGameIndex, + _outputRootProof: _outputRootProof, + _withdrawalProof: _withdrawalProof + }); + } + + /// @dev Tests that `proveWithdrawalTransaction` reverts if the game is a legacy game that does not implement + /// `wasRespectedGameTypeWhenCreated`. + function test_proveWithdrawalTransaction_legacyGame_reverts() external { + vm.mockCallRevert(address(game), abi.encodeWithSelector(game.wasRespectedGameTypeWhenCreated.selector), ""); + vm.expectRevert(LegacyGame.selector); + optimismPortal2.proveWithdrawalTransaction({ + _tx: _defaultTx, + _disputeGameIndex: _proposedGameIndex, + _outputRootProof: _outputRootProof, + _withdrawalProof: _withdrawalProof + }); + } + + /// @dev Tests that `proveWithdrawalTransaction` reverts if the game was created before the game retirement + /// timestamp. + function testFuzz_proveWithdrawalTransaction_createdBeforeRetirementTimestamp_reverts(uint64 _createdAt) external { + _createdAt = uint64(bound(_createdAt, 0, optimismPortal2.respectedGameTypeUpdatedAt() - 1)); + vm.mockCall(address(game), abi.encodeWithSelector(game.createdAt.selector), abi.encode(uint64(_createdAt))); + vm.expectRevert("OptimismPortal: dispute game created before respected game type was updated"); + optimismPortal2.proveWithdrawalTransaction({ + _tx: _defaultTx, + _disputeGameIndex: _proposedGameIndex, + _outputRootProof: _outputRootProof, + _withdrawalProof: _withdrawalProof + }); + } + /// @dev Tests that `proveWithdrawalTransaction` can be re-executed if the dispute game proven against has been /// blacklisted. function test_proveWithdrawalTransaction_replayProveBlacklisted_succeeds() external { @@ -1459,6 +1500,37 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { optimismPortal2.finalizeWithdrawalTransaction(_defaultTx); } + /// @dev Tests that `finalizeWithdrawalTransaction` reverts if the game is a legacy game that does not implement + /// `wasRespectedGameTypeWhenCreated`. `proveWithdrawalTransaction` should already prevent this, but we remove + /// that assumption here. + function test_finalizeWithdrawalTransaction_legacyGame_reverts() external { + vm.expectEmit(address(optimismPortal2)); + emit WithdrawalProven(_withdrawalHash, alice, bob); + vm.expectEmit(address(optimismPortal2)); + emit WithdrawalProvenExtension1(_withdrawalHash, address(this)); + optimismPortal2.proveWithdrawalTransaction({ + _tx: _defaultTx, + _disputeGameIndex: _proposedGameIndex, + _outputRootProof: _outputRootProof, + _withdrawalProof: _withdrawalProof + }); + + // Warp past the finalization period. + vm.warp(block.timestamp + optimismPortal2.proofMaturityDelaySeconds() + 1); + + // Resolve the dispute game. + game.resolveClaim(0, 0); + game.resolve(); + + // Warp past the dispute game finality delay. + vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); + + vm.mockCallRevert(address(game), abi.encodeWithSelector(game.wasRespectedGameTypeWhenCreated.selector), ""); + + vm.expectRevert(LegacyGame.selector); + optimismPortal2.finalizeWithdrawalTransaction(_defaultTx); + } + /// @dev Tests an e2e prove -> finalize path, checking the edges of each delay for correctness. function test_finalizeWithdrawalTransaction_delayEdges_succeeds() external { // Prove the withdrawal transaction. From 63658f13613bed1da63f342aadb93e77707d5544 Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Wed, 15 Jan 2025 13:09:44 -0500 Subject: [PATCH 23/28] portal2 tests: encodeCall --- .../contracts-bedrock/test/L1/OptimismPortal2.t.sol | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 594691e1d57b9..29c83f47d8d0a 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -587,9 +587,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { /// @dev Tests that `proveWithdrawalTransaction` reverts if the game was not the respected game type when created. function test_proveWithdrawalTransaction_wasNotRespectedGameTypeWhenCreated_reverts() external { - vm.mockCall( - address(game), abi.encodeWithSelector(game.wasRespectedGameTypeWhenCreated.selector), abi.encode(false) - ); + vm.mockCall(address(game), abi.encodeCall(game.wasRespectedGameTypeWhenCreated, ()), abi.encode(false)); vm.expectRevert(InvalidGameType.selector); optimismPortal2.proveWithdrawalTransaction({ _tx: _defaultTx, @@ -602,7 +600,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { /// @dev Tests that `proveWithdrawalTransaction` reverts if the game is a legacy game that does not implement /// `wasRespectedGameTypeWhenCreated`. function test_proveWithdrawalTransaction_legacyGame_reverts() external { - vm.mockCallRevert(address(game), abi.encodeWithSelector(game.wasRespectedGameTypeWhenCreated.selector), ""); + vm.mockCallRevert(address(game), abi.encodeCall(game.wasRespectedGameTypeWhenCreated, ()), ""); vm.expectRevert(LegacyGame.selector); optimismPortal2.proveWithdrawalTransaction({ _tx: _defaultTx, @@ -616,7 +614,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { /// timestamp. function testFuzz_proveWithdrawalTransaction_createdBeforeRetirementTimestamp_reverts(uint64 _createdAt) external { _createdAt = uint64(bound(_createdAt, 0, optimismPortal2.respectedGameTypeUpdatedAt() - 1)); - vm.mockCall(address(game), abi.encodeWithSelector(game.createdAt.selector), abi.encode(uint64(_createdAt))); + vm.mockCall(address(game), abi.encodeCall(game.createdAt, ()), abi.encode(uint64(_createdAt))); vm.expectRevert("OptimismPortal: dispute game created before respected game type was updated"); optimismPortal2.proveWithdrawalTransaction({ _tx: _defaultTx, @@ -1525,7 +1523,7 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { // Warp past the dispute game finality delay. vm.warp(block.timestamp + optimismPortal2.disputeGameFinalityDelaySeconds() + 1); - vm.mockCallRevert(address(game), abi.encodeWithSelector(game.wasRespectedGameTypeWhenCreated.selector), ""); + vm.mockCallRevert(address(game), abi.encodeCall(game.wasRespectedGameTypeWhenCreated, ()), ""); vm.expectRevert(LegacyGame.selector); optimismPortal2.finalizeWithdrawalTransaction(_defaultTx); From 285c3bc7f0dacd9038fba53f5edb404b23e6e57f Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Wed, 15 Jan 2025 13:29:45 -0500 Subject: [PATCH 24/28] FDG test: recipient can't receive value reverts --- .../test/dispute/FaultDisputeGame.t.sol | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol index 788783b99d3c5..3acb3c03e39ef 100644 --- a/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol +++ b/packages/contracts-bedrock/test/dispute/FaultDisputeGame.t.sol @@ -2014,6 +2014,62 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init { vm.stopPrank(); } + /// @dev Tests that claimCredit reverts when recipient can't receive value. + function test_claimCredit_recipientCantReceiveValue_reverts() public { + // Set up actors. + address alice = address(0xa11ce); + address bob = address(0xb0b); + + // Give the game proxy 1 extra ether, unregistered. + vm.deal(address(gameProxy), 1 ether); + + // Perform a bonded move. + Claim claim = _dummyClaim(); + + // Bond the first claim. + uint256 firstBond = _getRequiredBond(0); + vm.deal(alice, firstBond); + (,,,, Claim disputed,,) = gameProxy.claimData(0); + vm.prank(alice); + gameProxy.attack{ value: firstBond }(disputed, 0, claim); + + // Bond the second claim. + uint256 secondBond = _getRequiredBond(1); + vm.deal(bob, secondBond); + (,,,, disputed,,) = gameProxy.claimData(1); + vm.prank(bob); + gameProxy.attack{ value: secondBond }(disputed, 1, claim); + + // Warp past the finalization period + vm.warp(block.timestamp + 3 days + 12 hours); + + // Resolve the game. + // Second claim wins, so bob should get alice's credit. + gameProxy.resolveClaim(2, 0); + gameProxy.resolveClaim(1, 0); + gameProxy.resolveClaim(0, 0); + gameProxy.resolve(); + + // Wait for finalization delay. + vm.warp(block.timestamp + 3.5 days + 1 seconds); + + // Close the game. + gameProxy.closeGame(); + + // Claim credit once to trigger unlock period. + gameProxy.claimCredit(alice); + gameProxy.claimCredit(bob); + + // Wait for the withdrawal delay. + vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds); + + // make bob not be able to receive value by setting his contract code to something without `receive` + vm.etch(address(bob), address(L1Token).code); + + vm.expectRevert(BondTransferFailed.selector); + gameProxy.claimCredit(address(bob)); + } + /// @dev Tests that adding local data with an out of bounds identifier reverts. function testFuzz_addLocalData_oob_reverts(uint256 _ident) public { Claim disputed; From f0372caedb719fb2760ab725c9e8aed0ca7d7cf2 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 15 Jan 2025 14:31:34 -0600 Subject: [PATCH 25/28] various final tweaks --- op-e2e/faultproofs/output_alphabet_test.go | 12 +++---- .../interfaces/dispute/IFaultDisputeGame.sol | 3 +- .../dispute/IPermissionedDisputeGame.sol | 3 +- .../snapshots/semver-lock.json | 14 ++++---- .../src/L1/OptimismPortal2.sol | 23 ++++++++----- .../src/dispute/AnchorStateRegistry.sol | 10 ++++-- .../src/dispute/FaultDisputeGame.sol | 20 ++++++++--- .../test/L1/OptimismPortal2.t.sol | 34 +++++++++++++++---- .../test/dispute/AnchorStateRegistry.t.sol | 12 ++++--- 9 files changed, 90 insertions(+), 41 deletions(-) diff --git a/op-e2e/faultproofs/output_alphabet_test.go b/op-e2e/faultproofs/output_alphabet_test.go index 2d2cb2ab3ddca..d2459fba9b9a5 100644 --- a/op-e2e/faultproofs/output_alphabet_test.go +++ b/op-e2e/faultproofs/output_alphabet_test.go @@ -115,6 +115,12 @@ func TestOutputAlphabetGame_ReclaimBond(t *testing.T) { game.WaitForGameStatus(ctx, types.GameStatusChallengerWon) game.LogGameData(ctx) + // Advance the time past the finalization delay + // Finalization delay is the same as the credit unlock delay + // But just warp way into the future to be safe + sys.TimeTravelClock.AdvanceTime(game.CreditUnlockDuration(ctx) * 2) + require.NoError(t, wait.ForNextBlock(ctx, l1Client)) + // Wait for the game to have bond mode set game.WaitForBondModeSet(ctx) @@ -130,12 +136,6 @@ func TestOutputAlphabetGame_ReclaimBond(t *testing.T) { sys.TimeTravelClock.AdvanceTime(game.CreditUnlockDuration(ctx)) require.NoError(t, wait.ForNextBlock(ctx, l1Client)) - // Advance the time past the finalization delay - // Finalization delay is the same as the credit unlock delay - // But just warp way into the future to be safe - sys.TimeTravelClock.AdvanceTime(game.CreditUnlockDuration(ctx) * 2) - require.NoError(t, wait.ForNextBlock(ctx, l1Client)) - // Wait for alice to have no available credit // aka, wait for the challenger to claim its credit game.WaitForNoAvailableCredit(ctx, alice) diff --git a/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol b/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol index 9df883f849b93..07baf92f6db67 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IFaultDisputeGame.sol @@ -105,7 +105,7 @@ interface IFaultDisputeGame is IDisputeGame { function claims(Hash) external view returns (bool); function clockExtension() external view returns (Duration clockExtension_); function closeGame() external; - function credit(address) external view returns (uint256); + function credit(address _recipient) external view returns (uint256 credit_); function defend(Claim _disputed, uint256 _parentIndex, Claim _claim) external payable; function getChallengerDuration(uint256 _claimIndex) external view returns (Duration duration_); function getNumToResolve(uint256 _claimIndex) external view returns (uint256 numRemainingChildren_); @@ -118,6 +118,7 @@ interface IFaultDisputeGame is IDisputeGame { function maxClockDuration() external view returns (Duration maxClockDuration_); function maxGameDepth() external view returns (uint256 maxGameDepth_); function move(Claim _disputed, uint256 _challengeIndex, Claim _claim, bool _isAttack) external payable; + function normalModeCredit(address) external view returns (uint256); function refundModeCredit(address) external view returns (uint256); function resolutionCheckpoints(uint256) external diff --git a/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol b/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol index 6c9ce3d404475..f5565c20d85dc 100644 --- a/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol +++ b/packages/contracts-bedrock/interfaces/dispute/IPermissionedDisputeGame.sol @@ -94,7 +94,7 @@ interface IPermissionedDisputeGame is IDisputeGame { function claims(Hash) external view returns (bool); function clockExtension() external view returns (Duration clockExtension_); function closeGame() external; - function credit(address) external view returns (uint256); + function credit(address _recipient) external view returns (uint256 credit_); function defend(Claim _disputed, uint256 _parentIndex, Claim _claim) external payable; function getChallengerDuration(uint256 _claimIndex) external view returns (Duration duration_); function getNumToResolve(uint256 _claimIndex) external view returns (uint256 numRemainingChildren_); @@ -108,6 +108,7 @@ interface IPermissionedDisputeGame is IDisputeGame { function maxClockDuration() external view returns (Duration maxClockDuration_); function maxGameDepth() external view returns (uint256 maxGameDepth_); function move(Claim _disputed, uint256 _challengeIndex, Claim _claim, bool _isAttack) external payable; + function normalModeCredit(address) external view returns (uint256); function refundModeCredit(address) external view returns (uint256); function resolutionCheckpoints(uint256) external diff --git a/packages/contracts-bedrock/snapshots/semver-lock.json b/packages/contracts-bedrock/snapshots/semver-lock.json index 71788e938e9ac..2872bf3fe8c95 100644 --- a/packages/contracts-bedrock/snapshots/semver-lock.json +++ b/packages/contracts-bedrock/snapshots/semver-lock.json @@ -20,11 +20,11 @@ "sourceCodeHash": "0x2d21506cc51ebe0b60bcf89883aff5e9b1269567ce44ee779de3d3940e23fb65" }, "src/L1/OptimismPortal2.sol": { - "initCodeHash": "0x3f30480b47af0a965af7d38eff673fe1ef0e8704c1776eaef5648d242cfe376d", - "sourceCodeHash": "0x6241a2e1e5b8fae5e17e13abcccd3d013f66317a6d7611d844b10bbb878d38c0" + "initCodeHash": "0x969e3687d4497cc168af61e610ba0ae187e80f86aaa7b5d5bb598de19f279f08", + "sourceCodeHash": "0xf215a31954f2ef166cfb26d20e466c62fafa235a08fc42c55131dcb81998ff01" }, "src/L1/OptimismPortalInterop.sol": { - "initCodeHash": "0x9e12728cae72a1c701ddc3e09fe9e3b82aa38674ab76e6d9e74f3a384dfe25d2", + "initCodeHash": "0x057c56174304f3773654fed39abf5fab70d9446f531d07fdb225b738a680ad46", "sourceCodeHash": "0xc04a7f9c14a13ec3587f5cc351c8e9f27fbbe9f1291a1aba07de29edbeef418a" }, "src/L1/ProtocolVersions.sol": { @@ -152,8 +152,8 @@ "sourceCodeHash": "0xb7b0a06cd971c4647247dc19ce997d0c64a73e87c81d30731da9cf9efa1b952a" }, "src/dispute/AnchorStateRegistry.sol": { - "initCodeHash": "0xf7b9422d4c0b175aed8fad9ac4e03054625a667cff837a9a13612ec3bb2ee936", - "sourceCodeHash": "0xe355003779215d2c9d8c55e84c3e8b984a2c284da6b222de0874722bc71fe598" + "initCodeHash": "0xb2618d650808a7a335db7cc56d15ccaf432f50aa551c01be8bde8356893c0e0d", + "sourceCodeHash": "0x745f0e2b07b8f6492e11ca2f69b53d129177fbfd346d5ca4729d72792aff1f83" }, "src/dispute/DelayedWETH.sol": { "initCodeHash": "0xb1f04c9ee86984a157b92a18754c84104e9d4df7a3838633301ca7f557d0220a", @@ -164,8 +164,8 @@ "sourceCodeHash": "0x155c0334f63616ed245aadf9a94f419ef7d5e2237b3b32172484fd19890a61dc" }, "src/dispute/FaultDisputeGame.sol": { - "initCodeHash": "0x73ab839d9f77e541e0044f70fce1bd706d86f8c8ed310d96948a822b41191885", - "sourceCodeHash": "0x5abda30a900304e680d289be12798eda70677b54d7242499283954f98860a231" + "initCodeHash": "0x152fbb1f82488d815f56087fc464b9478f1390e3ecd67ae595344115fdd9ba91", + "sourceCodeHash": "0x9bfea41bd993bc1ef2ede9a5846a432ed5ea183868634fd77c4068b0a4a779b2" }, "src/legacy/DeployerWhitelist.sol": { "initCodeHash": "0x53099379ed48b87f027d55712dbdd1da7d7099925426eb0531da9c0012e02c29", diff --git a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol index ff0c8ec92ab5c..4f3084d6d69d2 100644 --- a/packages/contracts-bedrock/src/L1/OptimismPortal2.sol +++ b/packages/contracts-bedrock/src/L1/OptimismPortal2.sol @@ -316,12 +316,14 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { revert LegacyGame(); } - // Creation time must be greater than or equal to the respected game type updated time. - // Games created before this timestamp are not valid and cannot be used to finalize a - // withdrawal, so for user convenience we also prevent them from being used to prove a - // withdrawal. + // Game must have been created after the respected game type was updated. This check is a + // strict inequality because we want to prevent users from being able to prove or finalize + // withdrawals against games that were created in the same block that the retirement + // timestamp was set. If the retirement timestamp and game type are changed in the same + // block, such games could still be considered valid even if they used the old game type + // that we intended to invalidate. require( - gameProxy.createdAt().raw() >= respectedGameTypeUpdatedAt, + gameProxy.createdAt().raw() > respectedGameTypeUpdatedAt, "OptimismPortal: dispute game created before respected game type was updated" ); @@ -521,6 +523,7 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { // a timestamp of zero. if (provenWithdrawal.timestamp == 0) revert Unproven(); + // Grab the createdAt timestamp once. uint64 createdAt = disputeGameProxy.createdAt().raw(); // As a sanity check, we make sure that the proven withdrawal's timestamp is greater than @@ -553,10 +556,14 @@ contract OptimismPortal2 is Initializable, ResourceMetering, ISemver { revert LegacyGame(); } - // The game must have been created after `respectedGameTypeUpdatedAt`. This is to prevent users from creating - // invalid disputes against a deployed game type while the off-chain challenge agents are not watching. + // Game must have been created after the respected game type was updated. This check is a + // strict inequality because we want to prevent users from being able to prove or finalize + // withdrawals against games that were created in the same block that the retirement + // timestamp was set. If the retirement timestamp and game type are changed in the same + // block, such games could still be considered valid even if they used the old game type + // that we intended to invalidate. require( - createdAt >= respectedGameTypeUpdatedAt, + createdAt > respectedGameTypeUpdatedAt, "OptimismPortal: dispute game created before respected game type was updated" ); diff --git a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol index fb9bbc44b996f..c45eb030be4a4 100644 --- a/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol +++ b/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol @@ -147,9 +147,10 @@ contract AnchorStateRegistry is Initializable, ISemver { /// @param _game The game to check. /// @return Whether the game is retired. function isGameRetired(IDisputeGame _game) public view returns (bool) { - // Must be created at or after the respectedGameTypeUpdatedAt timestamp. Note that the - // strict inequality exactly mirrors the logic in the OptimismPortal contract. - return _game.createdAt().raw() < portal.respectedGameTypeUpdatedAt(); + // Must be created after the respectedGameTypeUpdatedAt timestamp. Note that this means all + // games created in the same block as the respectedGameTypeUpdatedAt timestamp are + // considered retired. + return _game.createdAt().raw() <= portal.respectedGameTypeUpdatedAt(); } /// @notice Returns whether a game is resolved. @@ -263,6 +264,9 @@ contract AnchorStateRegistry is Initializable, ISemver { } // Must be newer than the current anchor game. + // Note that this WILL block/brick if getAnchorRoot() ever reverts because the current + // anchor game is blacklisted. A blacklisted anchor game is *very* bad and we deliberately + // want to force the situation to be handled manually. (, uint256 anchorL2BlockNumber) = getAnchorRoot(); if (game.l2BlockNumber() <= anchorL2BlockNumber) { revert AnchorStateRegistry_InvalidAnchorGame(); diff --git a/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol b/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol index 8b50d40352a34..60df4bacee486 100644 --- a/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol +++ b/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol @@ -196,7 +196,7 @@ contract FaultDisputeGame is Clone, ISemver { ClaimData[] public claimData; /// @notice Credited balances for winning participants. - mapping(address => uint256) public credit; + mapping(address => uint256) public normalModeCredit; /// @notice A mapping to allow for constant-time lookups of existing claims. mapping(Hash => bool) public claims; @@ -957,7 +957,7 @@ contract FaultDisputeGame is Clone, ISemver { if (bondDistributionMode == BondDistributionMode.REFUND) { recipientCredit = refundModeCredit[_recipient]; } else if (bondDistributionMode == BondDistributionMode.NORMAL) { - recipientCredit = credit[_recipient]; + recipientCredit = normalModeCredit[_recipient]; } else { // We shouldn't get here, but sanity check just in case. revert InvalidBondDistributionMode(); @@ -976,7 +976,7 @@ contract FaultDisputeGame is Clone, ISemver { // Set the recipient's credit balances to 0. refundModeCredit[_recipient] = 0; - credit[_recipient] = 0; + normalModeCredit[_recipient] = 0; // Try to withdraw the WETH amount so it can be used here. WETH.withdraw(_recipient, recipientCredit); @@ -1060,6 +1060,18 @@ contract FaultDisputeGame is Clone, ISemver { len_ = claimData.length; } + /// @notice Returns the credit balance of a given recipient. + /// @param _recipient The recipient of the credit. + /// @return credit_ The credit balance of the recipient. + function credit(address _recipient) external view returns (uint256 credit_) { + if (bondDistributionMode == BondDistributionMode.REFUND) { + credit_ = refundModeCredit[_recipient]; + } else { + // Always return normal credit balance by default unless we're in refund mode. + credit_ = normalModeCredit[_recipient]; + } + } + //////////////////////////////////////////////////////////////// // IMMUTABLE GETTERS // //////////////////////////////////////////////////////////////// @@ -1117,7 +1129,7 @@ contract FaultDisputeGame is Clone, ISemver { /// @param _recipient The recipient of the bond. /// @param _bonded The claim to pay out the bond of. function _distributeBond(address _recipient, ClaimData storage _bonded) internal { - credit[_recipient] += _bonded.bond; + normalModeCredit[_recipient] += _bonded.bond; } /// @notice Verifies the integrity of an execution bisection subgame's root claim. Reverts if the claim diff --git a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol index 29c83f47d8d0a..b64eb09559dd7 100644 --- a/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/L1/OptimismPortal2.t.sol @@ -395,18 +395,23 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { /// @dev Setup the system for a ready-to-use state. function setUp() public virtual override { + // Warp forward in time to ensure that the game is created after the retirement timestamp. + vm.warp(optimismPortal2.respectedGameTypeUpdatedAt() + 1 seconds); + + // Set up the dummy game. _proposedBlockNumber = 0xFF; GameType respectedGameType = optimismPortal2.respectedGameType(); - uint256 bondAmount = disputeGameFactory.initBonds(respectedGameType); game = IFaultDisputeGame( payable( address( - disputeGameFactory.create{ value: bondAmount }( - optimismPortal2.respectedGameType(), Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber) + disputeGameFactory.create{ value: disputeGameFactory.initBonds(respectedGameType) }( + respectedGameType, Claim.wrap(_outputRoot), abi.encode(_proposedBlockNumber) ) ) ) ); + + // Grab the index of the game we just created. _proposedGameIndex = disputeGameFactory.gameCount() - 1; // Warp beyond the chess clocks and finalize the game. @@ -610,10 +615,25 @@ contract OptimismPortal2_FinalizeWithdrawal_Test is CommonTest { }); } - /// @dev Tests that `proveWithdrawalTransaction` reverts if the game was created before the game retirement - /// timestamp. - function testFuzz_proveWithdrawalTransaction_createdBeforeRetirementTimestamp_reverts(uint64 _createdAt) external { - _createdAt = uint64(bound(_createdAt, 0, optimismPortal2.respectedGameTypeUpdatedAt() - 1)); + /// @dev Tests that `proveWithdrawalTransaction` succeeds if the game was created after the + /// game retirement timestamp. + function testFuzz_proveWithdrawalTransaction_createdAfterRetirementTimestamp_succeeds(uint64 _createdAt) external { + _createdAt = uint64(bound(_createdAt, optimismPortal2.respectedGameTypeUpdatedAt() + 1, type(uint64).max)); + vm.mockCall(address(game), abi.encodeCall(game.createdAt, ()), abi.encode(uint64(_createdAt))); + optimismPortal2.proveWithdrawalTransaction({ + _tx: _defaultTx, + _disputeGameIndex: _proposedGameIndex, + _outputRootProof: _outputRootProof, + _withdrawalProof: _withdrawalProof + }); + } + + /// @dev Tests that `proveWithdrawalTransaction` reverts if the game was created before or at + /// the game retirement timestamp. + function testFuzz_proveWithdrawalTransaction_createdBeforeOrAtRetirementTimestamp_reverts(uint64 _createdAt) + external + { + _createdAt = uint64(bound(_createdAt, 0, optimismPortal2.respectedGameTypeUpdatedAt())); vm.mockCall(address(game), abi.encodeCall(game.createdAt, ()), abi.encode(uint64(_createdAt))); vm.expectRevert("OptimismPortal: dispute game created before respected game type was updated"); optimismPortal2.proveWithdrawalTransaction({ diff --git a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol index 86fe7f62b25dd..8ddf7ba5b5e77 100644 --- a/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol +++ b/packages/contracts-bedrock/test/dispute/AnchorStateRegistry.t.sol @@ -205,15 +205,17 @@ contract AnchorStateRegistry_IsGameRetired_Test is AnchorStateRegistry_Init { /// @notice Tests that isGameRetired will return true if the game is retired. /// @param _retirementTimestamp The retirement timestamp to use for the test. function testFuzz_isGameRetired_isRetired_succeeds(uint64 _retirementTimestamp) public { - // Make sure retirement timestamp is later than the game's creation time. - _retirementTimestamp = uint64(bound(_retirementTimestamp, gameProxy.createdAt().raw() + 1, type(uint64).max)); + // Make sure retirement timestamp is greater than or equal to the game's creation time. + _retirementTimestamp = uint64(bound(_retirementTimestamp, gameProxy.createdAt().raw(), type(uint64).max)); - // Mock the respectedGameTypeUpdatedAt call to be later than the game's creation time. + // Mock the respectedGameTypeUpdatedAt call. vm.mockCall( address(optimismPortal2), abi.encodeCall(optimismPortal2.respectedGameTypeUpdatedAt, ()), abi.encode(_retirementTimestamp) ); + + // Game should be retired. assertTrue(anchorStateRegistry.isGameRetired(gameProxy)); } @@ -221,7 +223,7 @@ contract AnchorStateRegistry_IsGameRetired_Test is AnchorStateRegistry_Init { /// @param _retirementTimestamp The retirement timestamp to use for the test. function testFuzz_isGameRetired_isNotRetired_succeeds(uint64 _retirementTimestamp) public { // Make sure retirement timestamp is earlier than the game's creation time. - _retirementTimestamp = uint64(bound(_retirementTimestamp, 0, gameProxy.createdAt().raw())); + _retirementTimestamp = uint64(bound(_retirementTimestamp, 0, gameProxy.createdAt().raw() - 1)); // Mock the respectedGameTypeUpdatedAt call to be earlier than the game's creation time. vm.mockCall( @@ -229,6 +231,8 @@ contract AnchorStateRegistry_IsGameRetired_Test is AnchorStateRegistry_Init { abi.encodeCall(optimismPortal2.respectedGameTypeUpdatedAt, ()), abi.encode(_retirementTimestamp) ); + + // Game should not be retired. assertFalse(anchorStateRegistry.isGameRetired(gameProxy)); } } From 517acee755b52b6b543f492677b4dcdc71f0e029 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 15 Jan 2025 14:38:06 -0600 Subject: [PATCH 26/28] regenerate snapshots --- .../snapshots/abi/FaultDisputeGame.json | 23 +++++++++++++++++-- .../abi/PermissionedDisputeGame.json | 23 +++++++++++++++++-- .../storageLayout/FaultDisputeGame.json | 2 +- .../PermissionedDisputeGame.json | 2 +- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json b/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json index 9f42a6762bb71..f79cd21708539 100644 --- a/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/abi/FaultDisputeGame.json @@ -318,7 +318,7 @@ "inputs": [ { "internalType": "address", - "name": "", + "name": "_recipient", "type": "address" } ], @@ -326,7 +326,7 @@ "outputs": [ { "internalType": "uint256", - "name": "", + "name": "credit_", "type": "uint256" } ], @@ -620,6 +620,25 @@ "stateMutability": "payable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "normalModeCredit", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { diff --git a/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json b/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json index cec052e0b0d20..53438b06b91ba 100644 --- a/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/abi/PermissionedDisputeGame.json @@ -341,7 +341,7 @@ "inputs": [ { "internalType": "address", - "name": "", + "name": "_recipient", "type": "address" } ], @@ -349,7 +349,7 @@ "outputs": [ { "internalType": "uint256", - "name": "", + "name": "credit_", "type": "uint256" } ], @@ -643,6 +643,25 @@ "stateMutability": "payable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "name": "normalModeCredit", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [], "name": "proposer", diff --git a/packages/contracts-bedrock/snapshots/storageLayout/FaultDisputeGame.json b/packages/contracts-bedrock/snapshots/storageLayout/FaultDisputeGame.json index ea8c9de176223..85a5897c0202a 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/FaultDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/FaultDisputeGame.json @@ -50,7 +50,7 @@ }, { "bytes": "32", - "label": "credit", + "label": "normalModeCredit", "offset": 0, "slot": "3", "type": "mapping(address => uint256)" diff --git a/packages/contracts-bedrock/snapshots/storageLayout/PermissionedDisputeGame.json b/packages/contracts-bedrock/snapshots/storageLayout/PermissionedDisputeGame.json index ea8c9de176223..85a5897c0202a 100644 --- a/packages/contracts-bedrock/snapshots/storageLayout/PermissionedDisputeGame.json +++ b/packages/contracts-bedrock/snapshots/storageLayout/PermissionedDisputeGame.json @@ -50,7 +50,7 @@ }, { "bytes": "32", - "label": "credit", + "label": "normalModeCredit", "offset": 0, "slot": "3", "type": "mapping(address => uint256)" From 4dcb7c49c487b6449f9776d1b61982fa876367f0 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 15 Jan 2025 14:38:56 -0600 Subject: [PATCH 27/28] fix specs tests --- packages/contracts-bedrock/test/universal/Specs.t.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/contracts-bedrock/test/universal/Specs.t.sol b/packages/contracts-bedrock/test/universal/Specs.t.sol index 5e4632ba9699d..a09d818fa7647 100644 --- a/packages/contracts-bedrock/test/universal/Specs.t.sol +++ b/packages/contracts-bedrock/test/universal/Specs.t.sol @@ -624,6 +624,7 @@ contract Specification_Test is CommonTest { _auth: Role.CHALLENGER }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("proposer()") }); + _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("normalModeCredit(address)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("refundModeCredit(address)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("resolutionCheckpoints(uint256)") }); _addSpec({ _name: "PermissionedDisputeGame", _sel: _getSel("resolve()") }); @@ -685,6 +686,7 @@ contract Specification_Test is CommonTest { _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolutionCheckpoints(uint256)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolve()") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("getNumToResolve(uint256)") }); + _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("normalModeCredit(address)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("refundModeCredit(address)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolveClaim(uint256,uint256)") }); _addSpec({ _name: "FaultDisputeGame", _sel: _getSel("resolvedAt()") }); From ea17af47ef42857790e81591fb6f35f8d1fbbd91 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Wed, 15 Jan 2025 14:50:28 -0600 Subject: [PATCH 28/28] final test fixes --- .../contracts-bedrock/test/invariants/OptimismPortal2.t.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol b/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol index 0a870bc651f3d..15ce33c252b18 100644 --- a/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol +++ b/packages/contracts-bedrock/test/invariants/OptimismPortal2.t.sol @@ -118,6 +118,9 @@ contract OptimismPortal2_Invariant_Harness is CommonTest { latestBlockhash: bytes32(uint256(0)) }); + // Warp forward in time to ensure that the game is created after the retirement timestamp. + vm.warp(optimismPortal2.respectedGameTypeUpdatedAt() + 1 seconds); + // Create a dispute game with the output root we've proposed. _proposedBlockNumber = 0xFF; IFaultDisputeGame game = IFaultDisputeGame(