Merge branch 'develop' into jm/vc-settings

packages/contracts-bedrock/.gas-snapshot

@@ -32,14 +32,20 @@ DisputeGameFactory_Test:test_setImplementation_notOwner_reverts() (gas: 16099)
 DisputeGameFactory_Test:test_setImplementation_succeeds() (gas: 44302)
 DisputeGameFactory_Test:test_transferOwnership_notOwner_reverts() (gas: 15974)
 DisputeGameFactory_Test:test_transferOwnership_succeeds() (gas: 18694)
-FaultDisputeGame_Test:test_extraData_succeeds() (gas: 17456)
-FaultDisputeGame_Test:test_gameData_succeeds() (gas: 17882)
-FaultDisputeGame_Test:test_gameStart_succeeds() (gas: 10315)
-FaultDisputeGame_Test:test_gameType_succeeds() (gas: 8237)
-FaultDisputeGame_Test:test_initialRootClaimData_succeeds() (gas: 17602)
-FaultDisputeGame_Test:test_rootClaim_succeeds() (gas: 8213)
-FaultDisputeGame_Test:test_simpleAttack_succeeds() (gas: 105058)
-FaultDisputeGame_Test:test_version_succeeds() (gas: 9758)
+FaultDisputeGame_Test:test_clockTimeExceeded_reverts() (gas: 26413)
+FaultDisputeGame_Test:test_defendRoot_reverts() (gas: 13258)
+FaultDisputeGame_Test:test_duplicateClaim_reverts() (gas: 103259)
+FaultDisputeGame_Test:test_extraData_succeeds() (gas: 17478)
+FaultDisputeGame_Test:test_gameData_succeeds() (gas: 17859)
+FaultDisputeGame_Test:test_gameDepthExceeded_reverts() (gas: 5903426)
+FaultDisputeGame_Test:test_gameStart_succeeds() (gas: 10337)
+FaultDisputeGame_Test:test_gameType_succeeds() (gas: 8194)
+FaultDisputeGame_Test:test_initialRootClaimData_succeeds() (gas: 17580)
+FaultDisputeGame_Test:test_moveAgainstNonexistentParent_reverts() (gas: 24587)
+FaultDisputeGame_Test:test_move_gameNotInProgress_reverts() (gas: 10945)
+FaultDisputeGame_Test:test_rootClaim_succeeds() (gas: 8191)
+FaultDisputeGame_Test:test_simpleAttack_succeeds() (gas: 107391)
+FaultDisputeGame_Test:test_version_succeeds() (gas: 9780)
 FeeVault_Test:test_constructor_succeeds() (gas: 18185)
 GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 352135)
 GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_1() (gas: 2950342)

packages/contracts-bedrock/contracts/dispute/FaultDisputeGame.sol

@@ -120,11 +120,6 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone {
             revert GameNotInProgress();
         }
 
-        // The zero hash is not a valid claim.
-        if (Claim.unwrap(_pivot) == bytes32(0)) {
-            revert InvalidClaim();
-        }
-
         // The only move that can be made against a root claim is an attack. This is because the
         // root claim commits to the entire state; Therefore, the only valid defense is to do
         // nothing if it is agreed with.
@@ -132,14 +127,9 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone {
             revert CannotDefendRootClaim();
         }
 
-        // Get the parent
+        // Get the parent. If it does not exist, the call will revert with OOB.
         ClaimData memory parent = claimData[_challengeIndex];
 
-        // The parent must exist.
-        if (Claim.unwrap(parent.claim) == bytes32(0)) {
-            revert ParentDoesNotExist();
-        }
-
         // Set the parent claim as countered.
         claimData[_challengeIndex].countered = true;
 

packages/contracts-bedrock/contracts/libraries/DisputeErrors.sol

@@ -50,11 +50,6 @@ error InvalidClaim();
  */
 error GameNotInProgress();
 
-/**
- * @notice Thrown when a move is attempted to be made against a claim that does not exist.
- */
-error ParentDoesNotExist();
-
 /**
  * @notice Thrown when a move is attempted to be made after the clock has timed out.
  */

packages/contracts-bedrock/contracts/test/FaultDisputeGame.t.sol

@@ -6,6 +6,7 @@ import { DisputeGameFactory } from "../dispute/DisputeGameFactory.sol";
 import { FaultDisputeGame } from "../dispute/FaultDisputeGame.sol";
 
 import "../libraries/DisputeTypes.sol";
+import "../libraries/DisputeErrors.sol";
 import { LibClock } from "../dispute/lib/LibClock.sol";
 import { LibPosition } from "../dispute/lib/LibPosition.sol";
 
@@ -40,6 +41,8 @@ contract FaultDisputeGame_Test is Test {
      */
     FaultDisputeGame internal gameProxy;
 
+    event Move(uint256 indexed parentIndex, Claim indexed pivot, address indexed claimant);
+
     function setUp() public {
         // Deploy a new dispute game factory.
         factory = new DisputeGameFactory();
@@ -131,6 +134,97 @@ contract FaultDisputeGame_Test is Test {
         );
     }
 
+    /**
+     * @dev Tests that a move while the game status is not `IN_PROGRESS` causes the call to revert
+     *      with the `GameNotInProgress` error
+     */
+    function test_move_gameNotInProgress_reverts() public {
+        uint256 chalWins = uint256(GameStatus.CHALLENGER_WINS);
+
+        // Replace the game status in storage. It exists in slot 0 at offset 8.
+        uint256 slot = uint256(vm.load(address(gameProxy), bytes32(0)));
+        uint256 offset = (8 << 3);
+        uint256 mask = 0xFF << offset;
+        // Replace the byte in the slot value with the challenger wins status.
+        slot = (slot & ~mask) | (chalWins << offset);
+        vm.store(address(gameProxy), bytes32(uint256(0)), bytes32(slot));
+
+        // Ensure that the game status was properly updated.
+        GameStatus status = gameProxy.status();
+        assertEq(uint256(status), chalWins);
+
+        // Attempt to make a move. Should revert.
+        vm.expectRevert(GameNotInProgress.selector);
+        gameProxy.attack(0, Claim.wrap(0));
+    }
+
+    /**
+     * @dev Tests that an attempt to defend the root claim reverts with the `CannotDefendRootClaim` error.
+     */
+    function test_defendRoot_reverts() public {
+        vm.expectRevert(CannotDefendRootClaim.selector);
+        gameProxy.defend(0, Claim.wrap(bytes32(uint256(5))));
+    }
+
+    /**
+     * @dev Tests that an attempt to move against a claim that does not exist reverts with the
+     *      `ParentDoesNotExist` error.
+     */
+    function test_moveAgainstNonexistentParent_reverts() public {
+        Claim claim = Claim.wrap(bytes32(uint256(5)));
+
+        // Expect an out of bounds revert for an attack
+        vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x32));
+        gameProxy.attack(1, claim);
+
+        // Expect an out of bounds revert for an attack
+        vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x32));
+        gameProxy.defend(1, claim);
+    }
+
+    /**
+     * @dev Tests that an attempt to move at the maximum game depth reverts with the
+     *      `GameDepthExceeded` error.
+     */
+    function test_gameDepthExceeded_reverts() public {
+        Claim claim = Claim.wrap(bytes32(uint256(5)));
+
+        for (uint256 i = 0; i < 63; i++) {
+            // At the max game depth, the `_move` function should revert with
+            // the `GameDepthExceeded` error.
+            if (i == 62) {
+                vm.expectRevert(GameDepthExceeded.selector);
+            }
+            gameProxy.attack(i, claim);
+        }
+    }
+
+    /**
+     * @dev Tests that a move made after the clock time has exceeded reverts with the
+     *      `ClockTimeExceeded` error.
+     */
+    function test_clockTimeExceeded_reverts() public {
+        // Warp ahead past the clock time for the first move (3 1/2 days)
+        vm.warp(block.timestamp + 3 days + 12 hours + 1);
+        vm.expectRevert(ClockTimeExceeded.selector);
+        gameProxy.attack(0, Claim.wrap(bytes32(uint256(5))));
+    }
+
+    /**
+     * @dev Tests that an identical claim cannot be made twice. The duplicate claim attempt should
+     *      revert with the `ClaimAlreadyExists` error.
+     */
+    function test_duplicateClaim_reverts() public {
+        Claim claim = Claim.wrap(bytes32(uint256(5)));
+
+        // Make the first move. This should succeed.
+        gameProxy.attack(0, claim);
+
+        // Attempt to make the same move again.
+        vm.expectRevert(ClaimAlreadyExists.selector);
+        gameProxy.attack(0, claim);
+    }
+
     /**
      * @dev Static unit test for the correctness of an opening attack.
      */
@@ -138,8 +232,12 @@ contract FaultDisputeGame_Test is Test {
         // Warp ahead 5 seconds.
         vm.warp(block.timestamp + 5);
 
+        Claim counter = Claim.wrap(bytes32(uint256(5)));
+
         // Perform the attack.
-        gameProxy.attack(0, Claim.wrap(bytes32(uint256(5))));
+        vm.expectEmit(true, true, true, false);
+        emit Move(0, counter, address(this));
+        gameProxy.attack(0, counter);
 
         // Grab the claim data of the attack.
         (
@@ -153,7 +251,7 @@ contract FaultDisputeGame_Test is Test {
         // Assert correctness of the attack claim's data.
         assertEq(parentIndex, 0);
         assertEq(countered, false);
-        assertEq(Claim.unwrap(claim), Claim.unwrap(Claim.wrap(bytes32(uint256(5)))));
+        assertEq(Claim.unwrap(claim), Claim.unwrap(counter));
         assertEq(Position.unwrap(position), Position.unwrap(LibPosition.attack(Position.wrap(0))));
         assertEq(
             Clock.unwrap(clock),