Add check for defense of execution trace subgame root

packages/contracts-bedrock/semver-lock.json

@@ -100,8 +100,8 @@
     "sourceCodeHash": "0xa995b54dce03ddf5c9c47451bd7181996b91398ad66b54ab0b8cbf582863a33e"
   },
   "src/dispute/OutputBisectionGame.sol": {
-    "initCodeHash": "0x816f7b9687837949a9d8546bad3df64ce8d30057f3aa742a75c4bdb8f6c9625a",
-    "sourceCodeHash": "0xaa892c96f7b8c31386df1cb5ad6ceed759497f8444d4d1ec8530d6135fd15f09"
+    "initCodeHash": "0x959d79d64f526fe67a476f876370814fb583bd1674b692f1025632e4f67a8c71",
+    "sourceCodeHash": "0xfdce387743a43e48f6aaa5855de0088d9bbb003d0ce62de465cf151320979a7a"
   },
   "src/legacy/DeployerWhitelist.sol": {
     "initCodeHash": "0x8de80fb23b26dd9d849f6328e56ea7c173cd9e9ce1f05c9beea559d1720deb3d",

packages/contracts-bedrock/src/dispute/OutputBisectionGame.sol

@@ -201,11 +201,6 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
         // INVARIANT: Moves cannot be made unless the game is currently in progress.
         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();
-
         // Get the parent. If it does not exist, the call will revert with OOB.
         ClaimData memory parent = claimData[_challengeIndex];
 
@@ -213,16 +208,24 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
         // known, we can compute the next position by moving left or right depending on whether
         // or not the move is an attack or defense.
         Position nextPosition = parent.position.move(_isAttack);
+        uint256 nextPositionDepth = nextPosition.depth();
+
+        // INVARIANT: A defense can never be made against the root claim of either the output root game or any
+        //            of the execution trace bisection subgames. 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 || nextPositionDepth == SPLIT_DEPTH + 2) && !_isAttack) {
+            revert CannotDefendRootClaim();
+        }
 
         // INVARIANT: A move can never surpass the `MAX_GAME_DEPTH`. The only option to counter a
         //            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 (nextPositionDepth > MAX_GAME_DEPTH) revert GameDepthExceeded();
 
         // When the next position surpasses the split depth (i.e., it is the root claim of an execution
         // trace bisection sub-game), we need to perform some extra verification steps.
-        if (nextPosition.depth() == SPLIT_DEPTH + 1) verifyExecBisectionRoot(_claim);
+        if (nextPositionDepth == SPLIT_DEPTH + 1) verifyExecBisectionRoot(_claim);
 
         // Fetch the grandparent clock, if it exists.
         // The grandparent clock should always exist unless the parent is the root claim.
@@ -496,12 +499,12 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
         returns (ClaimData storage ancestor_)
     {
         // Grab the trace ancestor's expected position.
-        Position preStateTraceAncestor = _global ? _pos.traceAncestor() : _pos.traceAncestorBounded(SPLIT_DEPTH);
+        Position traceAncestorPos = _global ? _pos.traceAncestor() : _pos.traceAncestorBounded(SPLIT_DEPTH);
 
         // Walk up the DAG to find a claim that commits to the same trace index as `_pos`. It is
         // guaranteed that such a claim exists.
         ancestor_ = claimData[_start];
-        while (Position.unwrap(ancestor_.position) != Position.unwrap(preStateTraceAncestor)) {
+        while (Position.unwrap(ancestor_.position) != Position.unwrap(traceAncestorPos)) {
             ancestor_ = claimData[ancestor_.parentIndex];
         }
     }

packages/contracts-bedrock/test/actors/OutputBisectionActors.sol

@@ -155,9 +155,10 @@ contract HonestGameSolver is GameSolver {
         returns (Direction direction_, Position movePos_)
     {
         bool rightLevel = isRightLevel(_claimData.position);
+        bool localAgree = Claim.unwrap(claimAt(_claimData.position)) == Claim.unwrap(_claimData.claim);
         if (_claimData.parentIndex == type(uint32).max) {
             // If we agree with the parent claim and it is on a level we agree with, ignore it.
-            if (Claim.unwrap(claimAt(_claimData.position)) == Claim.unwrap(_claimData.claim) && rightLevel) {
+            if (localAgree && rightLevel) {
                 return (Direction.Noop, Position.wrap(0));
             }
 
@@ -165,10 +166,18 @@ contract HonestGameSolver is GameSolver {
             direction_ = Direction.Attack;
             movePos_ = _claimData.position.move(true);
         } else {
+            // Never attempt to defend an execution trace subgame root. Only attack if we disagree with it,
+            // otherwise do nothing.
+            // NOTE: This is not correct behavior in the context of the honest actor; The alphabet game has
+            //       a constant status byte, and is not safe from someone being dishonest in output bisection
+            //       and then posting a correct execution trace bisection root claim.
+            if (_claimData.position.depth() == SPLIT_DEPTH + 1 && localAgree) {
+                return (Direction.Noop, Position.wrap(0));
+            }
+
             // If the parent claim is not the root claim, first check if the observed claim is on a level that
             // agrees with the local view of the root claim. If it is, noop. If it is not, perform an attack or
             // defense depending on the local view of the observed claim.
-
             if (rightLevel) {
                 // Never move against a claim on the right level. Even if it's wrong, if it's uncountered, it furthers
                 // our goals.
@@ -233,7 +242,7 @@ contract HonestGameSolver is GameSolver {
         // are making an attack step or a defense step. If the relative index at depth of the
         // move position is 0, the prestate is the absolute prestate and we need to
         // do nothing.
-        if ((_movePos.indexAtDepth() % (2 ** (MAX_DEPTH - SPLIT_DEPTH))) > 0) {
+        if ((_movePos.indexAtDepth() % (2 ** (MAX_DEPTH - SPLIT_DEPTH))) != 0) {
             // Grab the trace up to the prestate's trace index.
             if (isAttack) {
                 Position leafPos = Position.wrap(Position.unwrap(_parentPos) - 1);

packages/contracts-bedrock/test/dispute/OutputBisectionGame.t.sol

@@ -654,6 +654,47 @@ contract OutputBisection_1v1_Actors_Test is OutputBisectionGame_Init {
         super.setUp();
     }
 
+    /// @notice Fuzz test for a 1v1 output bisection dispute.
+    /// @dev The alphabet game has a constant status byte, and is not safe from someone being dishonest in
+    ///      output bisection and then posting a correct execution trace bisection root claim. This test
+    ///      does not cover this case (i.e. root claim of output bisection is dishonest, root claim of
+    ///      execution trace bisection is made by the dishonest actor but is honest, honest actor cannot
+    ///      attack it without risk of losing).
+    function testFuzz_outputBisection1v1honestRoot_succeeds(uint8 _divergeOutput, uint8 _divergeStep) public {
+        uint256[] memory honestL2Outputs = new uint256[](16);
+        for (uint256 i; i < honestL2Outputs.length; i++) {
+            honestL2Outputs[i] = i + 1;
+        }
+        bytes memory honestTrace = new bytes(256);
+        for (uint256 i; i < honestTrace.length; i++) {
+            honestTrace[i] = bytes1(uint8(i));
+        }
+
+        uint256 divergeAtOutput = bound(_divergeOutput, 0, 15);
+        uint256 divergeAtStep = bound(_divergeStep, 0, 7);
+        uint256 divergeStepOffset = (divergeAtOutput << 4) + divergeAtStep;
+
+        uint256[] memory dishonestL2Outputs = new uint256[](16);
+        for (uint256 i; i < dishonestL2Outputs.length; i++) {
+            dishonestL2Outputs[i] = i >= divergeAtOutput ? 0xFF : i + 1;
+        }
+        bytes memory dishonestTrace = new bytes(256);
+        for (uint256 i; i < dishonestTrace.length; i++) {
+            dishonestTrace[i] = i >= divergeStepOffset ? bytes1(uint8(0xFF)) : bytes1(uint8(i));
+        }
+
+        // Run the actor test
+        _actorTest({
+            _rootClaim: 16,
+            _absolutePrestateData: 0,
+            _honestTrace: honestTrace,
+            _honestL2Outputs: honestL2Outputs,
+            _dishonestTrace: dishonestTrace,
+            _dishonestL2Outputs: dishonestL2Outputs,
+            _expectedStatus: GameStatus.DEFENDER_WINS
+        });
+    }
+
     /// @notice Static unit test for a 1v1 output bisection dispute.
     function test_static_1v1honestRootGenesisAbsolutePrestate_succeeds() public {
         // The honest l2 outputs are from [1, 16] in this game.