Review changes

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: 491297)
-FaultDisputeGame_ResolvesCorrectly_CorrectRoot:test_resolvesCorrectly_succeeds() (gas: 484572)
-FaultDisputeGame_ResolvesCorrectly_IncorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 488105)
-FaultDisputeGame_ResolvesCorrectly_IncorrectRoot:test_resolvesCorrectly_succeeds() (gas: 481380)
+FaultDisputeGame_ResolvesCorrectly_CorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 497412)
+FaultDisputeGame_ResolvesCorrectly_CorrectRoot:test_resolvesCorrectly_succeeds() (gas: 490452)
+FaultDisputeGame_ResolvesCorrectly_IncorrectRoot2:test_resolvesCorrectly_succeeds() (gas: 494220)
+FaultDisputeGame_ResolvesCorrectly_IncorrectRoot:test_resolvesCorrectly_succeeds() (gas: 487260)
 FaultDisputeGame_Test:test_clockTimeExceeded_reverts() (gas: 26444)
-FaultDisputeGame_Test:test_defendRoot_reverts() (gas: 13258)
+FaultDisputeGame_Test:test_defendRoot_reverts() (gas: 13281)
 FaultDisputeGame_Test:test_duplicateClaim_reverts() (gas: 103343)
 FaultDisputeGame_Test:test_extraData_succeeds() (gas: 17478)
 FaultDisputeGame_Test:test_gameData_succeeds() (gas: 17881)
@@ -45,13 +45,13 @@ FaultDisputeGame_Test:test_gameDepthExceeded_reverts() (gas: 407234)
 FaultDisputeGame_Test:test_gameStart_succeeds() (gas: 10359)
 FaultDisputeGame_Test:test_gameType_succeeds() (gas: 8216)
 FaultDisputeGame_Test:test_initialRootClaimData_succeeds() (gas: 17669)
-FaultDisputeGame_Test:test_moveAgainstNonexistentParent_reverts() (gas: 24610)
+FaultDisputeGame_Test:test_moveAgainstNonexistentParent_reverts() (gas: 24633)
 FaultDisputeGame_Test:test_move_gameNotInProgress_reverts() (gas: 10901)
-FaultDisputeGame_Test:test_resolve_challengeContested() (gas: 221594)
+FaultDisputeGame_Test:test_resolve_challengeContested() (gas: 221617)
 FaultDisputeGame_Test:test_resolve_notInProgress_reverts() (gas: 9657)
 FaultDisputeGame_Test:test_resolve_rootContested() (gas: 106539)
 FaultDisputeGame_Test:test_resolve_rootUncontested() (gas: 20173)
-FaultDisputeGame_Test:test_resolve_teamDeathmatch() (gas: 392370)
+FaultDisputeGame_Test:test_resolve_teamDeathmatch() (gas: 392416)
 FaultDisputeGame_Test:test_rootClaim_succeeds() (gas: 8225)
 FaultDisputeGame_Test:test_simpleAttack_succeeds() (gas: 107438)
 FaultDisputeGame_Test:test_version_succeeds() (gas: 9802)

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

@@ -29,11 +29,6 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone {
      */
     string internal constant VERSION = "0.0.2";
 
-    /**
-     * @notice The max depth of the game.
-     */
-    uint256 internal constant MAX_GAME_DEPTH = 4;
-
     /**
      * @notice The duration of the game.
      * @dev TODO: Account for resolution buffer. (?)
@@ -47,10 +42,15 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone {
 
     /**
      * @notice The absolute prestate of the instruction trace. This is a constant that is defined
-     *         by the program that is being used to execute the trace, in this case, Cannon.
+     *         by the program that is being used to execute the trace.
      */
     Claim public immutable ABSOLUTE_PRESTATE;
 
+    /**
+     * @notice The max depth of the game.
+     */
+    uint256 public immutable MAX_GAME_DEPTH;
+
     /**
      * @notice The starting timestamp of the game
      */
@@ -79,8 +79,9 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone {
     /**
      * @param _absolutePrestate The absolute prestate of the instruction trace.
      */
-    constructor(Claim _absolutePrestate) {
+    constructor(Claim _absolutePrestate, uint256 _maxGameDepth) {
         ABSOLUTE_PRESTATE = _absolutePrestate;
+        MAX_GAME_DEPTH = _maxGameDepth;
     }
 
     ////////////////////////////////////////////////////////////////
@@ -106,7 +107,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone {
      */
     function step(
         uint256 _stateIndex,
-        uint256 _parentIndex,
+        uint256 _claimIndex,
         bool _isAttack,
         bytes calldata,
         bytes calldata
@@ -117,7 +118,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone {
         }
 
         // Get the parent. If it does not exist, the call will revert with OOB.
-        ClaimData storage parent = claimData[_parentIndex];
+        ClaimData storage parent = claimData[_claimIndex];
 
         // Pull the parent position out of storage.
         Position parentPos = parent.position;
@@ -136,7 +137,7 @@ contract FaultDisputeGame is IFaultDisputeGame, Clone {
             // If the step position's index at depth is 0, the prestate is the absolute prestate
             // and the post state is the parent claim.
             preStateClaim = ABSOLUTE_PRESTATE;
-            postStateClaim = claimData[_parentIndex].claim;
+            postStateClaim = claimData[_claimIndex].claim;
         } else {
             Position preStatePos;
             if (_isAttack) {

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

@@ -52,14 +52,14 @@ interface IFaultDisputeGame is IDisputeGame {
      *      processor contract should be generic enough such that we can use different
      *      fault proof VMs (MIPS, RiscV5, etc.)
      * @param _stateIndex The index of the pre/post state of the step within `claimData`.
-     * @param _parentIndex The index of the parent claim 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 _proof Proof to access memory leaf nodes in the VM.
      */
     function step(
         uint256 _stateIndex,
-        uint256 _parentIndex,
+        uint256 _claimIndex,
         bool _isAttack,
         bytes calldata _stateData,
         bytes calldata _proof

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

@@ -40,7 +40,7 @@ contract FaultDisputeGame_Init is DisputeGameFactory_Init {
     function init(Claim rootClaim, Claim absolutePrestate) public {
         super.setUp();
         // Deploy an implementation of the fault game
-        gameImpl = new FaultDisputeGame(absolutePrestate);
+        gameImpl = new FaultDisputeGame(absolutePrestate, 4);
         // Register the game implementation with the factory.
         factory.setImplementation(GAME_TYPE, gameImpl);
         // Create a new game.
@@ -353,14 +353,13 @@ contract FaultDisputeGame_Test is FaultDisputeGame_Init {
  *      `play` function can be overridden to change this behavior.
  */
 contract GamePlayer {
-    uint256 internal constant MAX_DEPTH = 4;
-
     bool public failedToStep;
-
     FaultDisputeGame public gameProxy;
+
     GamePlayer internal counterParty;
     Vm internal vm;
     bytes internal trace;
+    uint256 internal maxDepth;
 
     /**
      * @notice Initializes the player
@@ -373,11 +372,11 @@ contract GamePlayer {
         gameProxy = _gameProxy;
         counterParty = _counterParty;
         vm = _vm;
+        maxDepth = _gameProxy.MAX_GAME_DEPTH();
     }
 
     /**
      * @notice Perform the next move in the game.
-     * @dev This is very janky atm.
      */
     function play(uint256 _parentIndex) public virtual {
         // Grab the claim data at the parent index.
@@ -429,7 +428,7 @@ contract GamePlayer {
         }
 
         // If we are past the maximum depth, break the recursion and step.
-        if (movePos.depth() > MAX_DEPTH) {
+        if (movePos.depth() > maxDepth) {
             // Perform a step.
             uint256 stateIndex;
             // First, we need to find the pre/post state index depending on whether we
@@ -444,7 +443,7 @@ contract GamePlayer {
                     // Walk up until the valid position that commits to the prestate's
                     // trace index is found.
                     while (
-                        Position.unwrap(statePos.parent().rightIndex(MAX_DEPTH)) ==
+                        Position.unwrap(statePos.parent().rightIndex(maxDepth)) ==
                         Position.unwrap(statePos)
                     ) {
                         statePos = statePos.parent().parent().parent();
@@ -473,7 +472,7 @@ contract GamePlayer {
             }
         } else {
             // Find the trace index that our next claim must commit to.
-            uint256 traceIndex = movePos.rightIndex(MAX_DEPTH).indexAtDepth();
+            uint256 traceIndex = movePos.rightIndex(maxDepth).indexAtDepth();
             // Grab the claim that we need to make from the helper.
             Claim ourClaim = claimAt(traceIndex);
 
@@ -511,7 +510,7 @@ contract GamePlayer {
      * @notice Returns the player's claim that commits to a given gindex.
      */
     function claimAt(Position _position) internal view returns (Claim claim_) {
-        return claimAt(_position.rightIndex(MAX_DEPTH).indexAtDepth());
+        return claimAt(_position.rightIndex(maxDepth).indexAtDepth());
     }
 
     /**