Merge pull request #8346 from ethereum-optimism/cl/ctb/output-bisection-test-refactor

feat(ctb): `OutputBisectionGame` test refactor + fixes

packages/contracts-bedrock/semver-lock.json

@@ -100,8 +100,8 @@
     "sourceCodeHash": "0xa995b54dce03ddf5c9c47451bd7181996b91398ad66b54ab0b8cbf582863a33e"
   },
   "src/dispute/OutputBisectionGame.sol": {
-    "initCodeHash": "0x400a99278755979b815712d1d26598463dd98ed193df8cd1736ae2ae5831d7c7",
-    "sourceCodeHash": "0x7e267ad18eb946a0242df41ba044c5ee6f0b456e74bef07605a7dd2eb5b3ed01"
+    "initCodeHash": "0x6efe83410be6fd58eb07a5297492c6a45598d1f0f84d4ec286d93beade28f40f",
+    "sourceCodeHash": "0x1cc70ebc403581213a4853e1ef1579abeb63496d0625424fa5b9ac8351b2eeca"
   },
   "src/legacy/DeployerWhitelist.sol": {
     "initCodeHash": "0x8de80fb23b26dd9d849f6328e56ea7c173cd9e9ce1f05c9beea559d1720deb3d",

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

@@ -83,8 +83,8 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
     bool internal subgameAtRootResolved;
 
     /// @notice Semantic version.
-    /// @custom:semver 0.0.13
-    string public constant version = "0.0.13";
+    /// @custom:semver 0.0.14
+    string public constant version = "0.0.14";
 
     /// @param _gameType The type ID of the game.
     /// @param _absolutePrestate The absolute prestate of the instruction trace.
@@ -147,16 +147,20 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
             // prestate.
             // If the step is an attack at a trace index > 0, the prestate exists elsewhere in
             // the game state.
-            preStateClaim = stepPos.indexAtDepth() == 0
+            // NOTE: We localize the `indexAtDepth` for the current execution trace subgame by finding
+            //       the remainder of the index at depth divided by 2 ** (MAX_GAME_DEPTH - SPLIT_DEPTH),
+            //       which is the number of leaves in each execution trace subgame. This is so that we can
+            //       determine whether or not the step position is represents the `ABSOLUTE_PRESTATE`.
+            preStateClaim = (stepPos.indexAtDepth() % (2 ** (MAX_GAME_DEPTH - SPLIT_DEPTH))) == 0
                 ? ABSOLUTE_PRESTATE
-                : findTraceAncestor(Position.wrap(Position.unwrap(parentPos) - 1), parent.parentIndex).claim;
+                : findTraceAncestor(Position.wrap(Position.unwrap(parentPos) - 1), parent.parentIndex, false).claim;
             // For all attacks, the poststate is the parent claim.
             postState = parent;
         } else {
             // If the step is a defense, the poststate exists elsewhere in the game state,
             // and the parent claim is the expected pre-state.
             preStateClaim = parent.claim;
-            postState = findTraceAncestor(Position.wrap(Position.unwrap(parentPos) + 1), parent.parentIndex);
+            postState = findTraceAncestor(Position.wrap(Position.unwrap(parentPos) + 1), parent.parentIndex, false);
         }
 
         // INVARIANT: The prestate is always invalid if the passed `_stateData` is not the
@@ -296,16 +300,16 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
         Hash uuid = computeLocalContext(starting, startingPos, disputed, disputedPos);
 
         IPreimageOracle oracle = VM.oracle();
-        if (_ident == 1) {
+        if (_ident == LocalPreimageKey.L1_HEAD_HASH) {
             // Load the L1 head hash
             oracle.loadLocalData(_ident, Hash.unwrap(uuid), Hash.unwrap(l1Head), 32, _partOffset);
-        } else if (_ident == 2) {
+        } else if (_ident == LocalPreimageKey.STARTING_OUTPUT_ROOT) {
             // Load the starting proposal's output root.
             oracle.loadLocalData(_ident, Hash.unwrap(uuid), Claim.unwrap(starting), 32, _partOffset);
-        } else if (_ident == 3) {
+        } else if (_ident == LocalPreimageKey.DISPUTED_OUTPUT_ROOT) {
             // Load the disputed proposal's output root
             oracle.loadLocalData(_ident, Hash.unwrap(uuid), Claim.unwrap(disputed), 32, _partOffset);
-        } else if (_ident == 4) {
+        } else if (_ident == LocalPreimageKey.STARTING_L2_BLOCK_NUMBER) {
             // Load the starting proposal's L2 block number as a big-endian uint64 in the
             // high order 8 bytes of the word.
 
@@ -316,7 +320,7 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
                 : GENESIS_BLOCK_NUMBER + startingPos.indexAtDepth() + 1;
 
             oracle.loadLocalData(_ident, Hash.unwrap(uuid), bytes32(l2Number << 0xC0), 8, _partOffset);
-        } else if (_ident == 5) {
+        } else if (_ident == LocalPreimageKey.CHAIN_ID) {
             // Load the chain ID as a big-endian uint64 in the high order 8 bytes of the word.
             oracle.loadLocalData(_ident, Hash.unwrap(uuid), bytes32(block.chainid << 0xC0), 8, _partOffset);
         } else {
@@ -479,10 +483,20 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
     /// @notice Finds the trace ancestor of a given position within the DAG.
     /// @param _pos The position to find the trace ancestor claim of.
     /// @param _start The index to start searching from.
+    /// @param _global Whether or not to search the entire dag or just within an execution trace subgame. If set to
+    ///                `true`, and `_pos` is at or above the split depth, this function will revert.
     /// @return ancestor_ The ancestor claim that commits to the same trace index as `_pos`.
-    function findTraceAncestor(Position _pos, uint256 _start) internal view returns (ClaimData storage ancestor_) {
+    function findTraceAncestor(
+        Position _pos,
+        uint256 _start,
+        bool _global
+    )
+        internal
+        view
+        returns (ClaimData storage ancestor_)
+    {
         // Grab the trace ancestor's expected position.
-        Position preStateTraceAncestor = _pos.traceAncestor();
+        Position preStateTraceAncestor = _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.
@@ -547,14 +561,16 @@ contract OutputBisectionGame is IOutputBisectionGame, Clone, ISemver {
             // starting claim nor position exists in the tree. We leave these as 0, which can be easily
             // identified due to 0 being an invalid Gindex.
             if (outputPos.indexAtDepth() > 0) {
-                ClaimData storage starting = findTraceAncestor(Position.wrap(Position.unwrap(outputPos) - 1), claimIdx);
+                ClaimData storage starting =
+                    findTraceAncestor(Position.wrap(Position.unwrap(outputPos) - 1), claimIdx, true);
                 (startingClaim_, startingPos_) = (starting.claim, starting.position);
             } else {
                 startingClaim_ = Claim.wrap(Hash.unwrap(GENESIS_OUTPUT_ROOT));
             }
             (disputedClaim_, disputedPos_) = (claim.claim, claim.position);
         } else {
-            ClaimData storage disputed = findTraceAncestor(Position.wrap(Position.unwrap(outputPos) + 1), claimIdx);
+            ClaimData storage disputed =
+                findTraceAncestor(Position.wrap(Position.unwrap(outputPos) + 1), claimIdx, true);
             (startingClaim_, startingPos_) = (claim.claim, claim.position);
             (disputedClaim_, disputedPos_) = (disputed.claim, disputed.position);
         }

packages/contracts-bedrock/src/dispute/lib/LibPosition.sol

@@ -2,6 +2,7 @@
 pragma solidity ^0.8.15;
 
 import "src/libraries/DisputeTypes.sol";
+import "src/libraries/DisputeErrors.sol";
 
 /// @title LibPosition
 /// @notice This library contains helper functions for working with the `Position` type.
@@ -136,6 +137,44 @@ library LibPosition {
         }
     }
 
+    /// @notice Gets the position of the highest ancestor of `_position` that commits to the same
+    ///         trace index, while still being below `_upperBoundExclusive`.
+    /// @param _position The position to get the highest ancestor of.
+    /// @param _upperBoundExclusive The exclusive upper depth bound, used to inform where to stop in order
+    ///                             to not escape a sub-tree.
+    /// @return ancestor_ The highest ancestor of `position` that commits to the same trace index.
+    function traceAncestorBounded(
+        Position _position,
+        uint256 _upperBoundExclusive
+    )
+        internal
+        pure
+        returns (Position ancestor_)
+    {
+        // This function only works for positions that are below the upper bound.
+        if (_position.depth() <= _upperBoundExclusive) revert ClaimAboveSplit();
+
+        // Create a field with only the lowest unset bit of `_position` set.
+        Position lsb;
+        assembly {
+            lsb := and(not(_position), add(_position, 1))
+        }
+        // Find the index of the lowest unset bit within the field.
+        uint256 msb = lsb.depth();
+        assembly {
+            let a := shr(msb, _position)
+            // Bound the ancestor to the minimum gindex, 1.
+            ancestor_ := or(a, iszero(a))
+        }
+
+        // If the ancestor is above or at the upper bound, shift it to be below the upper bound.
+        // This should be a special case that only covers positions that commit to the final leaf
+        // in a sub-tree.
+        if (ancestor_.depth() <= _upperBoundExclusive) {
+            ancestor_ = ancestor_.rightIndex(_upperBoundExclusive + 1);
+        }
+    }
+
     /// @notice Get the move position of `_position`, which is the left child of:
     ///         1. `_position + 1` if `_isAttack` is true.
     ///         1. `_position` if `_isAttack` is false.

packages/contracts-bedrock/src/libraries/DisputeTypes.sol

@@ -85,26 +85,46 @@ library GameTypes {
     /// @dev A dispute game type that performs output bisection and then uses the cannon vm.
     GameType internal constant OUTPUT_CANNON = GameType.wrap(1);
 
-    /// @dev A dispute game type that performs output bisection and then uses an alphabet vm.
-    ///      Note intended for production use.
+    /// @notice A dispute game type that performs output bisection and then uses an alphabet vm.
+    ///         Not intended for production use.
     GameType internal constant OUTPUT_ALPHABET = GameType.wrap(254);
 
-    /// @dev A dispute game type that uses an alphabet vm.
-    ///      Note intended for production use.
+    /// @notice A dispute game type that uses an alphabet vm.
+    ///         Not intended for production use.
     GameType internal constant ALPHABET = GameType.wrap(255);
 }
 
 /// @title VMStatuses
+/// @notice Named type aliases for the various valid VM status bytes.
 library VMStatuses {
-    /// @dev The VM has executed successfully and the outcome is valid.
+    /// @notice The VM has executed successfully and the outcome is valid.
     VMStatus internal constant VALID = VMStatus.wrap(0);
 
-    /// @dev The VM has executed successfully and the outcome is invalid.
+    /// @notice The VM has executed successfully and the outcome is invalid.
     VMStatus internal constant INVALID = VMStatus.wrap(1);
 
-    /// @dev The VM has paniced.
+    /// @notice The VM has paniced.
     VMStatus internal constant PANIC = VMStatus.wrap(2);
 
-    /// @dev The VM execution is still in progress.
+    /// @notice The VM execution is still in progress.
     VMStatus internal constant UNFINISHED = VMStatus.wrap(3);
 }
+
+/// @title LocalPreimageKey
+/// @notice Named type aliases for local `PreimageOracle` key identifiers.
+library LocalPreimageKey {
+    /// @notice The identifier for the L1 head hash.
+    uint256 internal constant L1_HEAD_HASH = 0x01;
+
+    /// @notice The identifier for the starting output root.
+    uint256 internal constant STARTING_OUTPUT_ROOT = 0x02;
+
+    /// @notice The identifier for the disputed output root.
+    uint256 internal constant DISPUTED_OUTPUT_ROOT = 0x03;
+
+    /// @notice The identifier for the starting L2 block number.
+    uint256 internal constant STARTING_L2_BLOCK_NUMBER = 0x04;
+
+    /// @notice The identifier for the chain ID.
+    uint256 internal constant CHAIN_ID = 0x05;
+}

packages/contracts-bedrock/test/dispute/lib/LibPosition.t.sol

@@ -11,6 +11,8 @@ contract LibPosition_Test is Test {
     /// @dev At the lowest level of the tree, this allows for 2 ** 63 leaves. In reality, the max game depth
     ///      will likely be much lower.
     uint8 internal constant MAX_DEPTH = 63;
+    /// @dev Arbitrary split depth around half way down the tree.
+    uint8 internal constant SPLIT_DEPTH = 30;
 
     function boundIndexAtDepth(uint8 _depth, uint64 _indexAtDepth) internal pure returns (uint64) {
         // Index at depth bound: [0, 2 ** _depth-1]
@@ -91,6 +93,27 @@ contract LibPosition_Test is Test {
         assertEq(Position.unwrap(ancestor), Position.unwrap(loopAncestor));
     }
 
+    /// @notice Tests that the `traceAncestorBounded` function correctly computes the position of the
+    ///         highest ancestor (below `SPLIT_DEPTH`) that commits to the same trace index.
+    function testFuzz_traceAncestorBounded_correctness_succeeds(uint8 _depth, uint64 _indexAtDepth) public {
+        _depth = uint8(bound(_depth, SPLIT_DEPTH + 1, MAX_DEPTH));
+        _indexAtDepth = boundIndexAtDepth(_depth, _indexAtDepth);
+
+        Position position = LibPosition.wrap(_depth, _indexAtDepth);
+        Position ancestor = position.traceAncestorBounded(SPLIT_DEPTH);
+        Position loopAncestor = position;
+
+        // Stop at 1 below the split depth.
+        while (
+            loopAncestor.parent().traceIndex(MAX_DEPTH) == position.traceIndex(MAX_DEPTH)
+                && loopAncestor.depth() != SPLIT_DEPTH + 1
+        ) {
+            loopAncestor = loopAncestor.parent();
+        }
+
+        assertEq(Position.unwrap(ancestor), Position.unwrap(loopAncestor));
+    }
+
     /// @notice Tests that the `rightIndex` function correctly computes the deepest, right most index relative
     ///         to a given position.
     function testFuzz_rightIndex_correctness_succeeds(uint64 _maxDepth, uint8 _depth, uint64 _indexAtDepth) public {