Bounded trace ancestor

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

@@ -147,16 +147,18 @@ 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.
+            // WARN: This will not work, except for in the first execution trace bisection game!
+            //       We need to replace `0` with the correct starting index for the given sub-game.
             preStateClaim = stepPos.indexAtDepth() == 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
@@ -479,10 +481,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 +559,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

@@ -138,16 +138,21 @@ library LibPosition {
     }
 
     /// @notice Gets the position of the highest ancestor of `_position` that commits to the same
-    ///         trace index.
+    ///         trace index, while still being below `_upperBoundExclusive`.
     /// @param _position The position to get the highest ancestor of.
-    /// @param _splitDepth The split depth within the tree, used to inform where to stop in order to not escape an
-    ///                    execution trace sub-game.
+    /// @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 traceAncestorExec(Position _position, uint256 _splitDepth) internal pure returns (Position ancestor_) {
-        // This function only works for positions that are below the split depth (i.e., commit to state hashes
-        // of the fault proof program rather than output roots)
-        uint256 posDepth = _position.depth();
-        if (posDepth <= _splitDepth) revert ClaimAboveSplit();
+    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;
@@ -162,11 +167,11 @@ library LibPosition {
             ancestor_ := or(a, iszero(a))
         }
 
-        // If the ancestor is above or at the split depth, shift it to below the split depth.
-        // This should be a special case that only covers positions that commit to the final trace
-        // index in an execution trace subtree.
-        if (ancestor_.depth() <= _splitDepth) {
-            ancestor_ = ancestor_.rightIndex(_splitDepth + 1);
+        // 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);
         }
     }
 

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

@@ -93,18 +93,21 @@ contract LibPosition_Test is Test {
         assertEq(Position.unwrap(ancestor), Position.unwrap(loopAncestor));
     }
 
-    /// @notice Tests that the `traceAncestor` function correctly computes the position of the
-    ///         highest ancestor that commits to the same trace index.
-    function testFuzz_traceAncestorExec_correctness_succeeds(uint8 _depth, uint64 _indexAtDepth) public {
+    /// @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.traceAncestorExec(SPLIT_DEPTH);
+        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) {
+        while (
+            loopAncestor.parent().traceIndex(MAX_DEPTH) == position.traceIndex(MAX_DEPTH)
+                && loopAncestor.depth() != SPLIT_DEPTH + 1
+        ) {
             loopAncestor = loopAncestor.parent();
         }