Improve docs / :broom:

packages/contracts-bedrock/.gas-snapshot

@@ -32,12 +32,12 @@ DisputeGameFactory_SetImplementation_Test:test_setImplementation_notOwner_revert
 DisputeGameFactory_SetImplementation_Test:test_setImplementation_succeeds() (gas: 44243)
 DisputeGameFactory_TransferOwnership_Test:test_transferOwnership_notOwner_reverts() (gas: 15950)
 DisputeGameFactory_TransferOwnership_Test:test_transferOwnership_succeeds() (gas: 18642)
-FaultDisputeGame_ResolvesCorrectly_CorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 502174)
-FaultDisputeGame_ResolvesCorrectly_CorrectRoot3:test_resolvesCorrectly_succeeds() (gas: 504053)
-FaultDisputeGame_ResolvesCorrectly_CorrectRoot:test_resolvesCorrectly_succeeds() (gas: 491517)
-FaultDisputeGame_ResolvesCorrectly_IncorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 500937)
-FaultDisputeGame_ResolvesCorrectly_IncorrectRoot3:test_resolvesCorrectly_succeeds() (gas: 502816)
-FaultDisputeGame_ResolvesCorrectly_IncorrectRoot:test_resolvesCorrectly_succeeds() (gas: 490280)
+FaultDisputeGame_ResolvesCorrectly_CorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 502169)
+FaultDisputeGame_ResolvesCorrectly_CorrectRoot3:test_resolvesCorrectly_succeeds() (gas: 504048)
+FaultDisputeGame_ResolvesCorrectly_CorrectRoot:test_resolvesCorrectly_succeeds() (gas: 491512)
+FaultDisputeGame_ResolvesCorrectly_IncorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 500932)
+FaultDisputeGame_ResolvesCorrectly_IncorrectRoot3:test_resolvesCorrectly_succeeds() (gas: 502811)
+FaultDisputeGame_ResolvesCorrectly_IncorrectRoot:test_resolvesCorrectly_succeeds() (gas: 490275)
 FaultDisputeGame_Test:test_extraData_succeeds() (gas: 17426)
 FaultDisputeGame_Test:test_gameData_succeeds() (gas: 17917)
 FaultDisputeGame_Test:test_gameStart_succeeds() (gas: 10315)
@@ -51,12 +51,12 @@ FaultDisputeGame_Test:test_move_gameDepthExceeded_reverts() (gas: 408100)
 FaultDisputeGame_Test:test_move_gameNotInProgress_reverts() (gas: 10968)
 FaultDisputeGame_Test:test_move_nonExistentParent_reverts() (gas: 24655)
 FaultDisputeGame_Test:test_move_simpleAttack_succeeds() (gas: 107344)
-FaultDisputeGame_Test:test_resolve_challengeContested_succeeds() (gas: 224789)
+FaultDisputeGame_Test:test_resolve_challengeContested_succeeds() (gas: 224784)
 FaultDisputeGame_Test:test_resolve_notInProgress_reverts() (gas: 9657)
-FaultDisputeGame_Test:test_resolve_rootContested_succeeds() (gas: 109754)
+FaultDisputeGame_Test:test_resolve_rootContested_succeeds() (gas: 109749)
 FaultDisputeGame_Test:test_resolve_rootUncontestedClockNotExpired_succeeds() (gas: 21422)
-FaultDisputeGame_Test:test_resolve_rootUncontested_succeeds() (gas: 27256)
-FaultDisputeGame_Test:test_resolve_teamDeathmatch_succeeds() (gas: 395447)
+FaultDisputeGame_Test:test_resolve_rootUncontested_succeeds() (gas: 27251)
+FaultDisputeGame_Test:test_resolve_teamDeathmatch_succeeds() (gas: 395442)
 FaultDisputeGame_Test:test_rootClaim_succeeds() (gas: 8181)
 FeeVault_Test:test_constructor_succeeds() (gas: 18185)
 GasBenchMark_L1CrossDomainMessenger:test_sendMessage_benchmark_0() (gas: 352113)

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

@@ -66,19 +66,9 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
     }
 
     ////////////////////////////////////////////////////////////////
-    //                       External Logic                       //
+    //                  `IFaultDisputeGame` impl                  //
     ////////////////////////////////////////////////////////////////
 
-    /// @inheritdoc IFaultDisputeGame
-    function attack(uint256 _parentIndex, Claim _pivot) external payable {
-        move(_parentIndex, _pivot, true);
-    }
-
-    /// @inheritdoc IFaultDisputeGame
-    function defend(uint256 _parentIndex, Claim _pivot) external payable {
-        move(_parentIndex, _pivot, false);
-    }
-
     /// @inheritdoc IFaultDisputeGame
     function step(
         uint256 _stateIndex,
@@ -88,9 +78,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
         bytes calldata _proof
     ) external {
         // INVARIANT: Steps cannot be made unless the game is currently in progress.
-        if (status != GameStatus.IN_PROGRESS) {
-            revert GameNotInProgress();
-        }
+        if (status != GameStatus.IN_PROGRESS) revert GameNotInProgress();
 
         // Get the parent. If it does not exist, the call will revert with OOB.
         ClaimData storage parent = claimData[_claimIndex];
@@ -101,9 +89,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
         Position stepPos = parentPos.move(_isAttack);
 
         // INVARIANT: A step cannot be made unless the move position is 1 below the `MAX_GAME_DEPTH`
-        if (stepPos.depth() != MAX_GAME_DEPTH + 1) {
-            revert InvalidParent();
-        }
+        if (stepPos.depth() != MAX_GAME_DEPTH + 1) revert InvalidParent();
 
         // Determine the expected pre & post states of the step.
         Claim preStateClaim;
@@ -145,43 +131,32 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
             }
         }
 
-        // INVARIANT: A VM step can never counter a parent claim unless it produces an unexpected
-        //            poststate. "Unexpected" is defined as "not equal to the claim at
-        //            `_parentIndex` if the step is an attack, or the claim at `_stateIndex` if
-        //            the step is a defense."
-        if (VM.step(_stateData, _proof) == Claim.unwrap(postStateClaim)) {
-            revert ValidStep();
-        }
+        // INVARIANT: A VM step can never counter a parent claim unless it produces a poststate
+        //            that is not equal to the claim at `_parentIndex` if the step is an attack,
+        //            or the claim at `_stateIndex` if the step is a defense.
+        if (VM.step(_stateData, _proof) == Claim.unwrap(postStateClaim)) revert ValidStep();
 
         // Set the parent claim as countered. We do not need to append a new claim to the game;
         // instead, we can just set the existing parent as countered.
         parent.countered = true;
     }
 
-    ////////////////////////////////////////////////////////////////
-    //                       Internal Logic                       //
-    ////////////////////////////////////////////////////////////////
-
     /// @notice Internal move function, used by both `attack` and `defend`.
     /// @param _challengeIndex The index of the claim being moved against.
-    /// @param _pivot The claim at the next logical position in the game.
+    /// @param _claim The claim at the next logical position in the game.
     /// @param _isAttack Whether or not the move is an attack or defense.
     function move(
         uint256 _challengeIndex,
-        Claim _pivot,
+        Claim _claim,
         bool _isAttack
     ) public payable {
         // INVARIANT: Moves cannot be made unless the game is currently in progress.
-        if (status != GameStatus.IN_PROGRESS) {
-            revert GameNotInProgress();
-        }
+        if (status != GameStatus.IN_PROGRESS) revert GameNotInProgress();
 
         // INVARIANT: A defense can never be made against the root claim. 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.
-        if (_challengeIndex == 0 && !_isAttack) {
-            revert CannotDefendRootClaim();
-        }
+        if (_challengeIndex == 0 && !_isAttack) revert CannotDefendRootClaim();
 
         // Get the parent. If it does not exist, the call will revert with OOB.
         ClaimData memory parent = claimData[_challengeIndex];
@@ -198,9 +173,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
         //            claim at this depth is to perform a single instruction step on-chain via
         //            the `step` function to prove that the state transition produces an unexpected
         //            post-state.
-        if (nextPosition.depth() > MAX_GAME_DEPTH) {
-            revert GameDepthExceeded();
-        }
+        if (nextPosition.depth() > MAX_GAME_DEPTH) revert GameDepthExceeded();
 
         // Fetch the grandparent clock, if it exists.
         // The grandparent clock should always exist unless the parent is the root claim.
@@ -234,17 +207,15 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
 
         // INVARIANT: A claim may only exist at a given position once. Multiple claims may exist
         //            at the same position, however they must have different values.
-        ClaimHash claimHash = _pivot.hashClaimPos(nextPosition);
-        if (claims[claimHash]) {
-            revert ClaimAlreadyExists();
-        }
+        ClaimHash claimHash = _claim.hashClaimPos(nextPosition);
+        if (claims[claimHash]) revert ClaimAlreadyExists();
         claims[claimHash] = true;
 
         // Create the new claim.
         claimData.push(
             ClaimData({
                 parentIndex: uint32(_challengeIndex),
-                claim: _pivot,
+                claim: _claim,
                 position: nextPosition,
                 clock: nextClock,
                 countered: false
@@ -252,17 +223,22 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
         );
 
         // Emit the appropriate event for the attack or defense.
-        emit Move(_challengeIndex, _pivot, msg.sender);
+        emit Move(_challengeIndex, _claim, msg.sender);
     }
 
     /// @inheritdoc IFaultDisputeGame
-    function l2BlockNumber() public pure returns (uint256 l2BlockNumber_) {
-        l2BlockNumber_ = _getArgUint256(0x20);
+    function attack(uint256 _parentIndex, Claim _claim) external payable {
+        move(_parentIndex, _claim, true);
     }
 
-    /// @notice Returns the length of the `claimData` array.
-    function claimDataLen() external view returns (uint256 len_) {
-        len_ = claimData.length;
+    /// @inheritdoc IFaultDisputeGame
+    function defend(uint256 _parentIndex, Claim _claim) external payable {
+        move(_parentIndex, _claim, false);
+    }
+
+    /// @inheritdoc IFaultDisputeGame
+    function l2BlockNumber() public pure returns (uint256 l2BlockNumber_) {
+        l2BlockNumber_ = _getArgUint256(0x20);
     }
 
     ////////////////////////////////////////////////////////////////
@@ -282,9 +258,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
     /// @inheritdoc IDisputeGame
     function resolve() external returns (GameStatus status_) {
         // INVARIANT: Resolution cannot occur unless the game is currently in progress.
-        if (status != GameStatus.IN_PROGRESS) {
-            revert GameNotInProgress();
-        }
+        if (status != GameStatus.IN_PROGRESS) revert GameNotInProgress();
 
         // Search for the left-most dangling non-bottom node
         // The most recent claim is always a dangling, non-bottom node so we start with that
@@ -302,9 +276,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
 
             // INVARIANT: A claim can never be considered as the leftMostIndex or leftMostTraceIndex
             //            if it has been countered.
-            if (claim.countered) {
-                continue;
-            }
+            if (claim.countered) continue;
 
             // If the claim is a dangling node, we can check if it is the left-most
             // dangling node we've come across so far. If it is, we can update the
@@ -349,8 +321,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
         }
 
         // Update the game status
-        status = status_;
-        emit Resolved(status_);
+        emit Resolved(status = status_);
     }
 
     /// @inheritdoc IDisputeGame
@@ -381,6 +352,10 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
         extraData_ = extraData();
     }
 
+    ////////////////////////////////////////////////////////////////
+    //                       MISC EXTERNAL                        //
+    ////////////////////////////////////////////////////////////////
+
     /// @inheritdoc IInitializable
     function initialize() external {
         // Set the game start
@@ -399,4 +374,9 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone, Semver {
             })
         );
     }
+
+    /// @notice Returns the length of the `claimData` array.
+    function claimDataLen() external view returns (uint256 len_) {
+        len_ = claimData.length;
+    }
 }

packages/contracts-bedrock/contracts/dispute/interfaces/IFaultDisputeGame.sol

@@ -20,19 +20,19 @@ interface IFaultDisputeGame is IDisputeGame {
 
     /// @notice Emitted when a new claim is added to the DAG by `claimant`
     /// @param parentIndex The index within the `claimData` array of the parent claim
-    /// @param pivot The claim being added
+    /// @param claim The claim being added
     /// @param claimant The address of the claimant
-    event Move(uint256 indexed parentIndex, Claim indexed pivot, address indexed claimant);
+    event Move(uint256 indexed parentIndex, Claim indexed claim, address indexed claimant);
 
     /// @notice Attack a disagreed upon `Claim`.
     /// @param _parentIndex Index of the `Claim` to attack in `claimData`.
-    /// @param _pivot The `Claim` at the relative attack position.
-    function attack(uint256 _parentIndex, Claim _pivot) external payable;
+    /// @param _claim The `Claim` at the relative attack position.
+    function attack(uint256 _parentIndex, Claim _claim) external payable;
 
     /// @notice Defend an agreed upon `Claim`.
     /// @param _parentIndex Index of the claim to defend in `claimData`.
-    /// @param _pivot The `Claim` at the relative defense position.
-    function defend(uint256 _parentIndex, Claim _pivot) external payable;
+    /// @param _claim The `Claim` at the relative defense position.
+    function defend(uint256 _parentIndex, Claim _claim) external payable;
 
     /// @notice Perform the final step via an on-chain fault proof processor
     /// @dev This function should point to a fault proof processor in order to execute
@@ -42,7 +42,10 @@ interface IFaultDisputeGame is IDisputeGame {
     /// @param _stateIndex The index of the pre/post state of the step within `claimData`.
     /// @param _claimIndex The index of the challenged claim within `claimData`.
     /// @param _isAttack Whether or not the step is an attack or a defense.
-    /// @param _stateData The stateData of the step is the preimage of the claim @ `prestateIndex`
+    /// @param _stateData The stateData of the step is the preimage of the claim at the given
+    ///        prestate, which is at `_stateIndex` if the move is an attack and `_claimIndex` if
+    ///        the move is a defense. If the step is an attack on the first instruction, it is
+    ///        the absolute prestate of the fault proof VM.
     /// @param _proof Proof to access memory leaf nodes in the VM.
     function step(
         uint256 _stateIndex,