Merge pull request #4866 from ethereum-optimism/clabby/ctb/loosen-reprove-reqs

fix(ctb): Loosen re-prove requirements in the `OptimismPortal`

.changeset/mean-dingos-shake.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Loosens the requirements for re-proving a withdrawal transaction in the `OptimismPortal`

packages/contracts-bedrock/.gas-snapshot

@@ -251,7 +251,7 @@ OptimismPortalUpgradeable_Test:test_initialize_cannotInitProxy_reverts() (gas: 1
 OptimismPortalUpgradeable_Test:test_params_initValuesOnProxy_succeeds() (gas: 16011)
 OptimismPortalUpgradeable_Test:test_upgradeToAndCall_upgrading_succeeds() (gas: 180481)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputRootChanges_reverts() (gas: 198758)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 201077)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifOutputTimestampIsNotFinalized_reverts() (gas: 201032)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalNotProven_reverts() (gas: 39650)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_ifWithdrawalProofNotOldEnough_reverts() (gas: 196122)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onInsufficientGas_reverts() (gas: 196954)
@@ -259,13 +259,14 @@ OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onRece
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReentrancy_reverts() (gas: 235400)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_onReplay_reverts() (gas: 236906)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_provenWithdrawalHash_succeeds() (gas: 228581)
-OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 8797746687696163638)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails_fails() (gas: 8797746687696163639)
 OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_timestampLessThanL2OracleStart_reverts() (gas: 192846)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidOutputRootProof_reverts() (gas: 83498)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onInvalidWithdrawalProof_reverts() (gas: 135223)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_onSelfCall_reverts() (gas: 50752)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRoot_succeeds() (gas: 275173)
-OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProve_reverts() (gas: 188130)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRootAndOutputIndex_succeeds() (gas: 344623)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProveChangedOutputRoot_succeeds() (gas: 277341)
+OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_replayProve_reverts() (gas: 190317)
 OptimismPortal_FinalizeWithdrawal_Test:test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() (gas: 178317)
 OptimismPortal_Test:test_constructor_succeeds() (gas: 17277)
 OptimismPortal_Test:test_depositTransaction_contractCreation_reverts() (gas: 14273)

packages/contracts-bedrock/contracts/L1/OptimismPortal.sol

@@ -193,8 +193,8 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
         // output index has been updated.
         require(
             provenWithdrawal.timestamp == 0 ||
-                (_l2OutputIndex == provenWithdrawal.l2OutputIndex &&
-                    outputRoot != provenWithdrawal.outputRoot),
+                L2_ORACLE.getL2Output(provenWithdrawal.l2OutputIndex).outputRoot !=
+                provenWithdrawal.outputRoot,
             "OptimismPortal: withdrawal hash has already been proven"
         );
 

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

@@ -412,7 +412,7 @@ contract OptimismPortal_FinalizeWithdrawal_Test is Portal_Initializer {
         bytes32 slot;
         assembly {
             mstore(0x00, sload(_withdrawalHash.slot))
-            mstore(0x20, 52) // 52 is the slot of the `provenWithdrawals` mapping in OptimismPortal
+            mstore(0x20, 52) // 52 is the slot of the `provenWithdrawals` mapping in the OptimismPortal
             slot := keccak256(0x00, 0x40)
         }
 
@@ -439,6 +439,65 @@ contract OptimismPortal_FinalizeWithdrawal_Test is Portal_Initializer {
         assertEq(timestamp, block.timestamp);
     }
 
+    // Test: proveWithdrawalTransaction succeeds if the passed transaction's withdrawalHash has
+    // already been proven AND the output root + output index + l2BlockNumber changes.
+    function test_proveWithdrawalTransaction_replayProveChangedOutputRootAndOutputIndex_succeeds()
+        external
+    {
+        vm.expectEmit(true, true, true, true);
+        emit WithdrawalProven(_withdrawalHash, alice, bob);
+        op.proveWithdrawalTransaction(
+            _defaultTx,
+            _proposedOutputIndex,
+            _outputRootProof,
+            _withdrawalProof
+        );
+
+        // Compute the storage slot of the outputRoot corresponding to the `withdrawalHash`
+        // inside of the `provenWithdrawal`s mapping.
+        bytes32 slot;
+        assembly {
+            mstore(0x00, sload(_withdrawalHash.slot))
+            mstore(0x20, 52) // 52 is the slot of the `provenWithdrawals` mapping in OptimismPortal
+            slot := keccak256(0x00, 0x40)
+        }
+
+        // Store a dummy output root within the `provenWithdrawals` mapping without touching the
+        // l2BlockNumber or timestamp.
+        vm.store(address(op), slot, bytes32(0));
+
+        // Fetch the output proposal at `_proposedOutputIndex` from the L2OutputOracle
+        Types.OutputProposal memory proposal = op.L2_ORACLE().getL2Output(_proposedOutputIndex);
+
+        // Propose the same output root again, creating the same output at a different index + l2BlockNumber.
+        vm.startPrank(op.L2_ORACLE().PROPOSER());
+        op.L2_ORACLE().proposeL2Output(
+            proposal.outputRoot,
+            op.L2_ORACLE().nextBlockNumber(),
+            blockhash(block.number),
+            block.number
+        );
+        vm.stopPrank();
+
+        // Warp ahead 1 second
+        vm.warp(block.timestamp + 1);
+
+        // Even though we have already proven this withdrawalHash, we should be allowed to re-submit
+        // our proof with a changed outputRoot + a different output index
+        vm.expectEmit(true, true, true, true);
+        emit WithdrawalProven(_withdrawalHash, alice, bob);
+        op.proveWithdrawalTransaction(
+            _defaultTx,
+            _proposedOutputIndex + 1,
+            _outputRootProof,
+            _withdrawalProof
+        );
+
+        // Ensure that the withdrawal was updated within the mapping
+        (, uint128 timestamp, ) = op.provenWithdrawals(_withdrawalHash);
+        assertEq(timestamp, block.timestamp);
+    }
+
     // Test: proveWithdrawalTransaction succeeds and emits the WithdrawalProven event.
     function test_proveWithdrawalTransaction_validWithdrawalProof_succeeds() external {
         vm.expectEmit(true, true, true, true);