feat(ctb): In-order restriction in addLeavesLPP (#9134)

* Large preimage proposal - restriction on submitting data in-order

* Challenger updates

* op-challenger: Update tx data decoding for new ABI

---------

Co-authored-by: Adrian Sutton <[email protected]>

op-challenger/game/fault/contracts/oracle.go

@@ -94,8 +94,8 @@ func (c *PreimageOracleContract) InitLargePreimage(uuid *big.Int, partOffset uin
 	return call.ToTxCandidate()
 }
 
-func (c *PreimageOracleContract) AddLeaves(uuid *big.Int, input []byte, commitments []common.Hash, finalize bool) (txmgr.TxCandidate, error) {
-	call := c.contract.Call(methodAddLeavesLPP, uuid, input, commitments, finalize)
+func (c *PreimageOracleContract) AddLeaves(uuid *big.Int, startingBlockIndex *big.Int, input []byte, commitments []common.Hash, finalize bool) (txmgr.TxCandidate, error) {
+	call := c.contract.Call(methodAddLeavesLPP, uuid, startingBlockIndex, input, commitments, finalize)
 	return call.ToTxCandidate()
 }
 
@@ -124,7 +124,7 @@ func (c *PreimageOracleContract) Squeeze(
 // abiEncodeStateMatrix encodes the state matrix for the contract ABI
 func abiEncodeStateMatrix(stateMatrix *matrix.StateMatrix) bindings.LibKeccakStateMatrix {
 	packedState := stateMatrix.PackState()
-	var stateSlice = new([25]uint64)
+	stateSlice := new([25]uint64)
 	// SAFETY: a maximum of 25 * 8 bytes will be read from packedState and written to stateSlice
 	for i := 0; i < min(len(packedState), 25*8); i += 8 {
 		stateSlice[i/8] = new(big.Int).SetBytes(packedState[i : i+8]).Uint64()
@@ -205,9 +205,10 @@ func (c *PreimageOracleContract) DecodeInputData(data []byte) (*big.Int, keccakT
 		return nil, keccakTypes.InputData{}, fmt.Errorf("%w: %v", ErrInvalidAddLeavesCall, method)
 	}
 	uuid := args.GetBigInt(0)
-	input := args.GetBytes(1)
-	stateCommitments := args.GetBytes32Slice(2)
-	finalize := args.GetBool(3)
+	// Arg 1 is the starting block index which we don't current use
+	input := args.GetBytes(2)
+	stateCommitments := args.GetBytes32Slice(3)
+	finalize := args.GetBool(4)
 
 	commitments := make([]common.Hash, 0, len(stateCommitments))
 	for _, c := range stateCommitments {

op-challenger/game/fault/preimages/large.go

@@ -139,11 +139,13 @@ func (p *LargePreimageUploader) initLargePreimage(ctx context.Context, uuid *big
 func (p *LargePreimageUploader) addLargePreimageData(ctx context.Context, uuid *big.Int, chunks []keccakTypes.InputData) error {
 	queue := txmgr.NewQueue[int](ctx, p.txMgr, 10)
 	receiptChs := make([]chan txmgr.TxReceipt[int], len(chunks))
+	blocksProcessed := int64(0)
 	for i, chunk := range chunks {
-		tx, err := p.contract.AddLeaves(uuid, chunk.Input, chunk.Commitments, chunk.Finalize)
+		tx, err := p.contract.AddLeaves(uuid, big.NewInt(blocksProcessed), chunk.Input, chunk.Commitments, chunk.Finalize)
 		if err != nil {
 			return fmt.Errorf("failed to create pre-image oracle tx: %w", err)
 		}
+		blocksProcessed += int64(len(chunk.Input) / keccakTypes.BlockSize)
 		receiptChs[i] = make(chan txmgr.TxReceipt[int], 1)
 		queue.Send(i, tx, receiptChs[i])
 	}

op-challenger/game/fault/preimages/large_test.go

@@ -193,14 +193,16 @@ func (s *mockPreimageOracleContract) InitLargePreimage(_ *big.Int, _ uint32, _ u
 	}
 	return txmgr.TxCandidate{}, nil
 }
-func (s *mockPreimageOracleContract) AddLeaves(_ *big.Int, input []byte, _ []common.Hash, _ bool) (txmgr.TxCandidate, error) {
+
+func (s *mockPreimageOracleContract) AddLeaves(_ *big.Int, _ *big.Int, input []byte, _ []common.Hash, _ bool) (txmgr.TxCandidate, error) {
 	s.addCalls++
 	s.addData = append(s.addData, input...)
 	if s.addFails {
 		return txmgr.TxCandidate{}, mockAddLeavesError
 	}
 	return txmgr.TxCandidate{}, nil
 }
+
 func (s *mockPreimageOracleContract) Squeeze(_ common.Address, _ *big.Int, _ *matrix.StateMatrix, _ keccakTypes.Leaf, _ contracts.MerkleProof, _ keccakTypes.Leaf, _ contracts.MerkleProof) (txmgr.TxCandidate, error) {
 	return txmgr.TxCandidate{}, nil
 }

op-challenger/game/fault/preimages/types.go

@@ -25,7 +25,7 @@ type PreimageUploader interface {
 // PreimageOracleContract is the interface for interacting with the PreimageOracle contract.
 type PreimageOracleContract interface {
 	InitLargePreimage(uuid *big.Int, partOffset uint32, claimedSize uint32) (txmgr.TxCandidate, error)
-	AddLeaves(uuid *big.Int, input []byte, commitments []common.Hash, finalize bool) (txmgr.TxCandidate, error)
+	AddLeaves(uuid *big.Int, startingBlockIndex *big.Int, input []byte, commitments []common.Hash, finalize bool) (txmgr.TxCandidate, error)
 	Squeeze(claimant common.Address, uuid *big.Int, stateMatrix *matrix.StateMatrix, preState keccakTypes.Leaf, preStateProof contracts.MerkleProof, postState keccakTypes.Leaf, postStateProof contracts.MerkleProof) (txmgr.TxCandidate, error)
 	GetProposalMetadata(ctx context.Context, block batching.Block, idents ...keccakTypes.LargePreimageIdent) ([]keccakTypes.LargePreimageMetaData, error)
 }

packages/contracts-bedrock/scripts/fpac/SubmitLPP.sol

@@ -46,6 +46,7 @@ contract SubmitLPP is Script, StdAssertions {
             vm.broadcast();
             oracle.addLeavesLPP({
                 _uuid: TEST_UUID,
+                _inputStartBlock: i / 136,
                 _input: chunk,
                 _stateCommitments: finalize ? mockStateCommitmentsLast : mockStateCommitments,
                 _finalize: finalize

packages/contracts-bedrock/slither-report.json

@@ -1096,10 +1096,10 @@
     "impact": "Medium",
     "confidence": "Medium",
     "check": "uninitialized-local",
-    "description": "PreimageOracle.challengeFirstLPP(address,uint256,PreimageOracle.Leaf,bytes32[]).stateMatrix (src/cannon/PreimageOracle.sol#459) is a local variable never initialized\n",
+    "description": "PreimageOracle.challengeFirstLPP(address,uint256,PreimageOracle.Leaf,bytes32[]).stateMatrix (src/cannon/PreimageOracle.sol#444) is a local variable never initialized\n",
     "type": "variable",
     "name": "stateMatrix",
-    "start": 20988,
+    "start": 20524,
     "length": 40,
     "filename_relative": "src/cannon/PreimageOracle.sol"
   },

packages/contracts-bedrock/snapshots/abi/PreimageOracle.json

@@ -48,6 +48,11 @@
         "name": "_uuid",
         "type": "uint256"
       },
+      {
+        "internalType": "uint256",
+        "name": "_inputStartBlock",
+        "type": "uint256"
+      },
       {
         "internalType": "bytes",
         "name": "_input",
@@ -771,5 +776,10 @@
     "inputs": [],
     "name": "TreeSizeOverflow",
     "type": "error"
+  },
+  {
+    "inputs": [],
+    "name": "WrongStartingBlock",
+    "type": "error"
   }
 ]

packages/contracts-bedrock/src/cannon/PreimageOracle.sol

@@ -278,15 +278,13 @@ contract PreimageOracle is IPreimageOracle {
     /// @notice Adds a contiguous list of keccak state matrices to the merkle tree.
     function addLeavesLPP(
         uint256 _uuid,
+        uint256 _inputStartBlock,
         bytes calldata _input,
         bytes32[] calldata _stateCommitments,
         bool _finalize
     )
         external
     {
-        // The caller of `addLeavesLPP` must be an EOA.
-        if (msg.sender != tx.origin) revert NotEOA();
-
         // If we're finalizing, pad the input for the submitter. If not, copy the input into memory verbatim.
         bytes memory input;
         if (_finalize) {
@@ -298,44 +296,26 @@ contract PreimageOracle is IPreimageOracle {
         // Pull storage variables onto the stack / into memory for operations.
         bytes32[KECCAK_TREE_DEPTH] memory branch = proposalBranches[msg.sender][_uuid];
         LPPMetaData metaData = proposalMetadata[msg.sender][_uuid];
+        uint256 blocksProcessed = metaData.blocksProcessed();
+
+        // The caller of `addLeavesLPP` must be an EOA.
+        if (msg.sender != tx.origin) revert NotEOA();
 
         // Revert if the proposal has not been initialized. 0-size preimages are *not* allowed.
         if (metaData.claimedSize() == 0) revert NotInitialized();
 
         // Revert if the proposal has already been finalized. No leaves can be added after this point.
         if (metaData.timestamp() != 0) revert AlreadyFinalized();
 
-        // Check if the part offset is present in the input data being posted. If it is, assign the part to the mapping.
-        uint256 offset = metaData.partOffset();
-        uint256 currentSize = metaData.bytesProcessed();
-        if (offset < 8 && currentSize == 0) {
-            uint32 claimedSize = metaData.claimedSize();
-            bytes32 preimagePart;
-            assembly {
-                mstore(0x00, shl(192, claimedSize))
-                mstore(0x08, calldataload(_input.offset))
-                preimagePart := mload(offset)
-            }
-            proposalParts[msg.sender][_uuid] = preimagePart;
-        } else if (offset >= 8 && (offset = offset - 8) >= currentSize && offset < currentSize + _input.length) {
-            uint256 relativeOffset = offset - currentSize;
-
-            // Revert if the full preimage part is not available in the data we're absorbing. The submitter must
-            // supply data that contains the full preimage part so that no partial preimage parts are stored in the
-            // oracle. Partial parts are *only* allowed at the tail end of the preimage, where no more data is available
-            // to be absorbed.
-            if (relativeOffset + 32 >= _input.length && !_finalize) revert PartOffsetOOB();
+        // Revert if the starting block is not the next block to be added. This is to aid submitters in ensuring that
+        // they don't corrupt an in-progress proposal by submitting input out of order.
+        if (blocksProcessed != _inputStartBlock) revert WrongStartingBlock();
 
-            // If the preimage part is in the data we're about to absorb, persist the part to the caller's large
-            // preimaage metadata.
-            bytes32 preimagePart;
-            assembly {
-                preimagePart := calldataload(add(_input.offset, relativeOffset))
-            }
-            proposalParts[msg.sender][_uuid] = preimagePart;
-        }
+        // Attempt to extract the preimage part from the input data, if the part offset is present in the current
+        // chunk of input. This function has side effects, and will persist the preimage part to the caller's large
+        // preimage proposal storage if the part offset is present in the input data.
+        _extractPreimagePart(_input, _uuid, _finalize, metaData);
 
-        uint256 blocksProcessed = metaData.blocksProcessed();
         assembly {
             let inputLen := mload(input)
             let inputPtr := add(input, 0x20)
@@ -389,15 +369,20 @@ contract PreimageOracle is IPreimageOracle {
         // Do not allow for posting preimages larger than the merkle tree can support.
         if (blocksProcessed > MAX_LEAF_COUNT) revert TreeSizeOverflow();
 
-        // Perist the branch to storage.
+        // Update the proposal metadata to include the number of blocks processed and total bytes processed.
+        metaData = metaData.setBlocksProcessed(uint32(blocksProcessed)).setBytesProcessed(
+            uint32(_input.length + metaData.bytesProcessed())
+        );
+        // If the proposal is being finalized, set the timestamp to the current block timestamp. This begins the
+        // challenge period, which must be waited out before the proposal can be finalized.
+        if (_finalize) metaData = metaData.setTimestamp(uint64(block.timestamp));
+
+        // Perist the latest branch to storage.
         proposalBranches[msg.sender][_uuid] = branch;
-        // Track the block number that these leaves were added at.
+        // Persist the block number that these leaves were added in. This assists off-chain observers in reconstructing
+        // the proposal merkle tree by querying block bodies.
         proposalBlocks[msg.sender][_uuid].push(uint64(block.number));
-
-        // Update the proposal metadata.
-        metaData =
-            metaData.setBlocksProcessed(uint32(blocksProcessed)).setBytesProcessed(uint32(_input.length + currentSize));
-        if (_finalize) metaData = metaData.setTimestamp(uint64(block.timestamp));
+        // Persist the updated metadata to storage.
         proposalMetadata[msg.sender][_uuid] = metaData;
     }
 
@@ -534,6 +519,52 @@ contract PreimageOracle is IPreimageOracle {
         }
     }
 
+    /// @notice Attempts to persist the preimage part to the caller's large preimage proposal storage, if the preimage
+    ///         part is present in the input data being posted.
+    /// @param _input The portion of the preimage being posted.
+    /// @param _uuid The UUID of the large preimage proposal.
+    /// @param _finalize Whether or not the proposal is being finalized in the current call.
+    /// @param _metaData The metadata of the large preimage proposal.
+    function _extractPreimagePart(
+        bytes calldata _input,
+        uint256 _uuid,
+        bool _finalize,
+        LPPMetaData _metaData
+    )
+        internal
+    {
+        uint256 offset = _metaData.partOffset();
+        uint256 claimedSize = _metaData.claimedSize();
+        uint256 currentSize = _metaData.bytesProcessed();
+
+        // Check if the part offset is present in the input data being posted. If it is, assign the part to the mapping.
+        if (offset < 8 && currentSize == 0) {
+            bytes32 preimagePart;
+            assembly {
+                mstore(0x00, shl(192, claimedSize))
+                mstore(0x08, calldataload(_input.offset))
+                preimagePart := mload(offset)
+            }
+            proposalParts[msg.sender][_uuid] = preimagePart;
+        } else if (offset >= 8 && (offset = offset - 8) >= currentSize && offset < currentSize + _input.length) {
+            uint256 relativeOffset = offset - currentSize;
+
+            // Revert if the full preimage part is not available in the data we're absorbing. The submitter must
+            // supply data that contains the full preimage part so that no partial preimage parts are stored in the
+            // oracle. Partial parts are *only* allowed at the tail end of the preimage, where no more data is available
+            // to be absorbed.
+            if (relativeOffset + 32 >= _input.length && !_finalize) revert PartOffsetOOB();
+
+            // If the preimage part is in the data we're about to absorb, persist the part to the caller's large
+            // preimaage metadata.
+            bytes32 preimagePart;
+            assembly {
+                preimagePart := calldataload(add(_input.offset, relativeOffset))
+            }
+            proposalParts[msg.sender][_uuid] = preimagePart;
+        }
+    }
+
     /// Check if leaf` at `index` verifies against the Merkle `root` and `branch`.
     /// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#is_valid_merkle_branch
     function _verify(

packages/contracts-bedrock/src/cannon/libraries/CannonErrors.sol

@@ -13,6 +13,9 @@ error InvalidPreimage();
 /// @notice Thrown when a leaf with an invalid input size is added.
 error InvalidInputSize();
 
+/// @notice Thrown when data is submitted out of order in a large preimage proposal.
+error WrongStartingBlock();
+
 /// @notice Thrown when the pre and post states passed aren't contiguous.
 error StatesNotContiguous();