test(ctb): Add failure tests for finalizeWithdrawalTransaction

packages/contracts-bedrock/.gas-snapshot

-GasBenchMark_L1CrossDomainMessenger:test_L1MessengerSendMessage_benchmark_0() (gas: 263435)
-GasBenchMark_L1CrossDomainMessenger:test_L1MessengerSendMessage_benchmark_1() (gas: 77595)
-GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 354407)
-GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 118585)
-GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 354429)
-GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 118560)
+GasBenchMark_L1CrossDomainMessenger:test_L1MessengerSendMessage_benchmark_0() (gas: 263442)
+GasBenchMark_L1CrossDomainMessenger:test_L1MessengerSendMessage_benchmark_1() (gas: 77602)
+GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_0() (gas: 354414)
+GasBenchMark_L1StandardBridge_Deposit:test_depositERC20_benchmark_1() (gas: 118592)
+GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_0() (gas: 354436)
+GasBenchMark_L1StandardBridge_Deposit:test_depositETH_benchmark_1() (gas: 118567)
 GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (gas: 45432)
 GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 68671)
 GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 74964)
@@ -43,16 +43,16 @@ L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageSucceeds() (gas: 77841)
 L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageToSystemContract() (gas: 67957)
 L1CrossDomainMessenger_Test:test_L1MessengerRelayShouldRevertIfPaused() (gas: 60463)
 L1CrossDomainMessenger_Test:test_L1MessengerReplayMessageWithValue() (gas: 38169)
-L1CrossDomainMessenger_Test:test_L1MessengerSendMessage() (gas: 301583)
-L1CrossDomainMessenger_Test:test_L1MessengerTwiceSendMessage() (gas: 1492570)
+L1CrossDomainMessenger_Test:test_L1MessengerSendMessage() (gas: 301590)
+L1CrossDomainMessenger_Test:test_L1MessengerTwiceSendMessage() (gas: 1492584)
 L1CrossDomainMessenger_Test:test_L1MessengerUnpause() (gas: 40872)
 L1CrossDomainMessenger_Test:test_L1MessengerXDomainSenderReverts() (gas: 24316)
 L1CrossDomainMessenger_Test:test_L1MessengerxDomainMessageSenderResets() (gas: 86376)
-L1StandardBridge_Test:test_depositERC20() (gas: 579490)
-L1StandardBridge_Test:test_depositERC20To() (gas: 581697)
-L1StandardBridge_Test:test_depositETH() (gas: 373948)
-L1StandardBridge_Test:test_depositETHTo() (gas: 331084)
-L1StandardBridge_Test:test_finalizeBridgeERC20FailSendBack() (gas: 681445)
+L1StandardBridge_Test:test_depositERC20() (gas: 579497)
+L1StandardBridge_Test:test_depositERC20To() (gas: 581704)
+L1StandardBridge_Test:test_depositETH() (gas: 373955)
+L1StandardBridge_Test:test_depositETHTo() (gas: 331091)
+L1StandardBridge_Test:test_finalizeBridgeERC20FailSendBack() (gas: 681451)
 L1StandardBridge_Test:test_finalizeERC20Withdrawal() (gas: 490132)
 L1StandardBridge_Test:test_finalizeETHWithdrawal() (gas: 64273)
 L1StandardBridge_Test:test_initialize() (gas: 26334)
@@ -60,7 +60,7 @@ L1StandardBridge_Test:test_onlyEOADepositERC20() (gas: 22376)
 L1StandardBridge_Test:test_onlyEOADepositETH() (gas: 40859)
 L1StandardBridge_Test:test_onlyL2BridgeFinalizeERC20Withdrawal() (gas: 36268)
 L1StandardBridge_Test:test_onlyPortalFinalizeERC20Withdrawal() (gas: 35573)
-L1StandardBridge_Test:test_receive() (gas: 520566)
+L1StandardBridge_Test:test_receive() (gas: 520573)
 L2CrossDomainMessenger_Test:testCannot_L2MessengerPause() (gas: 10845)
 L2CrossDomainMessenger_Test:test_L1MessengerRelayMessageRevertsOnReentrancy() (gas: 171930)
 L2CrossDomainMessenger_Test:test_L2MessengerMessageVersion() (gas: 8411)
@@ -69,8 +69,8 @@ L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageFirstStuckSecondSucceeds
 L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageSucceeds() (gas: 57372)
 L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageToSystemContract() (gas: 36193)
 L2CrossDomainMessenger_Test:test_L2MessengerRelayShouldRevertIfPaused() (gas: 41619)
-L2CrossDomainMessenger_Test:test_L2MessengerSendMessage() (gas: 121522)
-L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 135934)
+L2CrossDomainMessenger_Test:test_L2MessengerSendMessage() (gas: 121684)
+L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 136258)
 L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10609)
 L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 54887)
 L2OutputOracleTest:testCannot_ProposeWithUnmatchedBlockhash() (gas: 26829)
@@ -96,15 +96,15 @@ L2OutputOracleUpgradeable_Test:test_cannotInitImpl() (gas: 19428)
 L2OutputOracleUpgradeable_Test:test_cannotInitProxy() (gas: 24427)
 L2OutputOracleUpgradeable_Test:test_initValuesOnProxy() (gas: 39086)
 L2OutputOracleUpgradeable_Test:test_upgrading() (gas: 230843)
-L2StandardBridge_Test:test_ERC20BridgeFailed_whenLocalTokenIsBridge() (gas: 134196)
+L2StandardBridge_Test:test_ERC20BridgeFailed_whenLocalTokenIsBridge() (gas: 134358)
 L2StandardBridge_Test:test_cannotWithdrawEthWithoutSendingIt() (gas: 21619)
-L2StandardBridge_Test:test_finalizeBridgeERC20FailSendBack() (gas: 499320)
+L2StandardBridge_Test:test_finalizeBridgeERC20FailSendBack() (gas: 499449)
 L2StandardBridge_Test:test_finalizeDeposit() (gas: 93125)
-L2StandardBridge_Test:test_finalizeDeposit_failsToCompleteOutboundTransfer() (gas: 141211)
+L2StandardBridge_Test:test_finalizeDeposit_failsToCompleteOutboundTransfer() (gas: 141373)
 L2StandardBridge_Test:test_initialize() (gas: 14823)
-L2StandardBridge_Test:test_receive() (gas: 137760)
-L2StandardBridge_Test:test_withdraw() (gas: 353308)
-L2StandardBridge_Test:test_withdrawTo() (gas: 354064)
+L2StandardBridge_Test:test_receive() (gas: 137921)
+L2StandardBridge_Test:test_withdraw() (gas: 353438)
+L2StandardBridge_Test:test_withdrawTo() (gas: 354193)
 L2StandardBridge_Test:test_withdraw_onlyEOA() (gas: 251674)
 L2ToL1MessagePasserTest:test_burn() (gas: 112246)
 L2ToL1MessagePasserTest:test_initiateWithdrawal_fromContract() (gas: 68198)
@@ -135,20 +135,27 @@ OptimismPortalUpgradeable_Test:test_cannotInitImpl() (gas: 10686)
 OptimismPortalUpgradeable_Test:test_cannotInitProxy() (gas: 15662)
 OptimismPortalUpgradeable_Test:test_initValuesOnProxy() (gas: 15967)
 OptimismPortalUpgradeable_Test:test_upgrading() (gas: 230843)
-OptimismPortal_Test:test_OptimismPortalConstructor() (gas: 17319)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnInsufficientGas() (gas: 160749)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnInvalidOutputRootProof() (gas: 81285)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnRecentWithdrawal() (gas: 53001)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnReentrancy() (gas: 205586)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnReplay() (gas: 278919)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOnSelfCall() (gas: 50517)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_revertsOninvalidWithdrawalProof() (gas: 148562)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_succeeds() (gas: 186991)
+OptimismPortal_FinalizeWithdrawal_Test:test_finalizeWithdrawalTransaction_targetFails() (gas: 289662)
+OptimismPortal_Test:test_OptimismPortalConstructor() (gas: 17341)
 OptimismPortal_Test:test_OptimismPortalContractCreationReverts() (gas: 14199)
 OptimismPortal_Test:test_OptimismPortalReceiveEth() (gas: 127534)
-OptimismPortal_Test:test_cannotVerifyRecentWithdrawal() (gas: 25613)
 OptimismPortal_Test:test_depositTransaction_NoValueContract() (gas: 76668)
-OptimismPortal_Test:test_depositTransaction_NoValueEOA() (gas: 76992)
-OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract() (gas: 76673)
+OptimismPortal_Test:test_depositTransaction_NoValueEOA() (gas: 76969)
+OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract() (gas: 76650)
 OptimismPortal_Test:test_depositTransaction_createWithZeroValueForEOA() (gas: 76994)
 OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreation() (gas: 83694)
 OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation() (gas: 75881)
-OptimismPortal_Test:test_depositTransaction_withEthValueFromContract() (gas: 83356)
+OptimismPortal_Test:test_depositTransaction_withEthValueFromContract() (gas: 83333)
 OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA() (gas: 83993)
-OptimismPortal_Test:test_invalidWithdrawalProof() (gas: 38103)
-OptimismPortal_Test:test_isBlockFinalized() (gas: 113703)
+OptimismPortal_Test:test_isBlockFinalized() (gas: 113725)
 OptimismPortal_Test:test_simple_isBlockFinalized() (gas: 26630)
 Proxy_Test:test_clashingFunctionSignatures() (gas: 101427)
 Proxy_Test:test_implementationKey() (gas: 20942)
@@ -272,4 +279,4 @@ SequencerFeeVault_Test:test_constructor() (gas: 7678)
 SequencerFeeVault_Test:test_minWithdrawalAmount() (gas: 5440)
 SequencerFeeVault_Test:test_receive() (gas: 17338)
 SequencerFeeVault_Test:test_revertWithdraw() (gas: 9342)
-SequencerFeeVault_Test:test_withdraw() (gas: 148623)
+SequencerFeeVault_Test:test_withdraw() (gas: 148784)

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

@@ -15,6 +15,7 @@ import { L2CrossDomainMessenger } from "../L2/L2CrossDomainMessenger.sol";
 import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol";
 import { LegacyERC20ETH } from "../legacy/LegacyERC20ETH.sol";
 import { Predeploys } from "../libraries/Predeploys.sol";
+import { Types } from "../libraries/Types.sol";
 import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import { Proxy } from "../universal/Proxy.sol";
 import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
@@ -78,7 +79,6 @@ contract CommonTest is Test {
             abi.encodePacked(_mint, _value, _gasLimit, _isCreation, _data)
         );
     }
-
 }
 
 contract L2OutputOracle_Initializer is CommonTest {
@@ -457,24 +457,26 @@ contract Bridge_Initializer is Messenger_Initializer {
 }
 
 contract FFIInterface is Test {
-    function getFinalizeWithdrawalTransactionInputs(
-        uint256 _nonce,
-        address _sender,
-        address _target,
-        uint64 _value,
-        uint256 _gasLimit,
-        bytes memory _data
-    ) external returns (bytes32, bytes32, bytes32, bytes32, bytes memory) {
+    function getFinalizeWithdrawalTransactionInputs(Types.WithdrawalTransaction memory _tx)
+        external
+        returns (
+            bytes32,
+            bytes32,
+            bytes32,
+            bytes32,
+            bytes memory
+        )
+    {
         string[] memory cmds = new string[](9);
         cmds[0] = "node";
         cmds[1] = "dist/scripts/differential-testing.js";
         cmds[2] = "getFinalizeWithdrawalTransactionInputs";
-        cmds[3] = vm.toString(_nonce);
-        cmds[4] = vm.toString(_sender);
-        cmds[5] = vm.toString(_target);
-        cmds[6] = vm.toString(_value);
-        cmds[7] = vm.toString(_gasLimit);
-        cmds[8] = vm.toString(_data);
+        cmds[3] = vm.toString(_tx.nonce);
+        cmds[4] = vm.toString(_tx.sender);
+        cmds[5] = vm.toString(_tx.target);
+        cmds[6] = vm.toString(_tx.value);
+        cmds[7] = vm.toString(_tx.gasLimit);
+        cmds[8] = vm.toString(_tx.data);
 
         bytes memory result = vm.ffi(cmds);
         (

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

@@ -10,7 +10,6 @@ import { Hashing } from "../libraries/Hashing.sol";
 import { Proxy } from "../universal/Proxy.sol";
 
 contract OptimismPortal_Test is Portal_Initializer {
-
     function test_OptimismPortalConstructor() external {
         assertEq(op.FINALIZATION_PERIOD_SECONDS(), 7 days);
         assertEq(address(op.L2_ORACLE()), address(oracle));
@@ -215,62 +214,11 @@ contract OptimismPortal_Test is Portal_Initializer {
         assertEq(address(op).balance, NON_ZERO_VALUE);
     }
 
-    function test_cannotVerifyRecentWithdrawal() external {
-        Types.OutputRootProof memory outputRootProof = Types
-            .OutputRootProof({
-                version: bytes32(0),
-                stateRoot: bytes32(0),
-                withdrawerStorageRoot: bytes32(0),
-                latestBlockhash: bytes32(0)
-            });
-        // Setup the Oracle to return an output with a recent timestamp
-        uint256 recentTimestamp = block.timestamp - 1000;
-        vm.mockCall(
-            address(op.L2_ORACLE()),
-            abi.encodeWithSelector(L2OutputOracle.getL2Output.selector),
-            abi.encode(Types.OutputProposal(bytes32(uint256(1)), recentTimestamp))
-        );
-
-        vm.expectRevert("OptimismPortal: proposal is not yet finalized");
-        op.finalizeWithdrawalTransaction(Types.WithdrawalTransaction(0, alice, alice, 0, 0, hex""), 0, outputRootProof, hex"");
-    }
-
-    function test_invalidWithdrawalProof() external {
-        vm.mockCall(
-            address(op.L2_ORACLE()),
-            abi.encodeWithSelector(L2OutputOracle.getL2Output.selector),
-            abi.encode(Types.OutputProposal(bytes32(uint256(1)), block.timestamp))
-        );
-
-        Types.OutputRootProof memory outputRootProof = Types
-            .OutputRootProof({
-                version: bytes32(0),
-                stateRoot: bytes32(0),
-                withdrawerStorageRoot: bytes32(0),
-                latestBlockhash: bytes32(0)
-            });
-
-        vm.warp(
-            oracle.getL2Output(oracle.latestBlockNumber()).timestamp +
-                op.FINALIZATION_PERIOD_SECONDS() + 1
-        );
-
-        vm.expectRevert("OptimismPortal: invalid output root proof");
-        op.finalizeWithdrawalTransaction(Types.WithdrawalTransaction(0, alice, alice, 0, 0, hex""), 0, outputRootProof, hex"");
-    }
-
     function test_simple_isBlockFinalized() external {
         vm.mockCall(
             address(op.L2_ORACLE()),
-            abi.encodeWithSelector(
-                L2OutputOracle.getL2Output.selector
-            ),
-            abi.encode(
-                Types.OutputProposal(
-                    bytes32(uint256(1)),
-                    startingBlockNumber
-                )
-            )
+            abi.encodeWithSelector(L2OutputOracle.getL2Output.selector),
+            abi.encode(Types.OutputProposal(bytes32(uint256(1)), startingBlockNumber))
         );
 
         // warp to the finalization period
@@ -310,6 +258,270 @@ contract OptimismPortal_Test is Portal_Initializer {
         vm.expectRevert("L2OutputOracle: No output found for that block number.");
         assertEq(op.isBlockFinalized(checkpoint + 1), false);
     }
+}
+
+contract OptimismPortal_FinalizeWithdrawal_Test is Portal_Initializer {
+    // Reusable default values for a test withdrawal
+    Types.WithdrawalTransaction _defaultTx;
+
+    uint256 _proposedBlockNumber;
+    bytes32 _stateRoot;
+    bytes32 _storageRoot;
+    bytes32 _outputRoot;
+    bytes32 _withdrawalHash;
+    bytes _withdrawalProof;
+    Types.OutputRootProof internal _outputRootProof;
+
+    event WithdrawalFinalized(bytes32 indexed, bool success);
+
+    // Use a constructor to set the storage vars above, so as to minimize the number of ffi calls.
+    constructor() public {
+        super.setUp();
+        _defaultTx = Types.WithdrawalTransaction({
+            nonce: 0,
+            sender: alice,
+            target: bob,
+            value: 100,
+            gasLimit: 100_000,
+            data: hex""
+        });
+        // Get withdrawal proof data we can use for testing.
+        (_stateRoot, _storageRoot, _outputRoot, _withdrawalHash, _withdrawalProof) = ffi
+            .getFinalizeWithdrawalTransactionInputs(_defaultTx);
+
+        // Setup a dummy output root proof for reuse.
+        _outputRootProof = Types.OutputRootProof({
+            version: bytes32(uint256(0)),
+            stateRoot: _stateRoot,
+            withdrawerStorageRoot: _storageRoot,
+            latestBlockhash: bytes32(uint256(0))
+        });
+        _proposedBlockNumber = oracle.nextBlockNumber();
+    }
+
+    // Get the system into a nice ready-to-use state.
+    function setUp() public override {
+        // Configure the oracle to return the output root we've prepared.
+        vm.warp(oracle.computeL2Timestamp(_proposedBlockNumber) + 1);
+        vm.prank(oracle.proposer());
+        oracle.proposeL2Output(_outputRoot, _proposedBlockNumber, 0, 0);
+
+        // Warp beyond the finalization period for the block we've proposed.
+        vm.warp(
+            oracle.getL2Output(_proposedBlockNumber).timestamp +
+                op.FINALIZATION_PERIOD_SECONDS() +
+                1
+        );
+        // Fund the portal so that we can withdraw ETH.
+        vm.deal(address(op), 0xFFFFFFFF);
+    }
+
+    // Test: finalizeWithdrawalTransaction succeeds and emits the WithdrawalFinalized event.
+    function test_finalizeWithdrawalTransaction_succeeds() external {
+        uint256 bobBalanceBefore = address(bob).balance;
+        vm.expectEmit(true, true, true, true);
+        emit WithdrawalFinalized(_withdrawalHash, true);
+        op.finalizeWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+        assert(address(bob).balance == bobBalanceBefore + 100);
+    }
+
+    // Test: finalizeWithdrawalTransaction fails because the target reverts,
+    // and emits the WithdrawalFinalized event with success=false.
+    function test_finalizeWithdrawalTransaction_targetFails() external {
+        uint256 bobBalanceBefore = address(bob).balance;
+        vm.etch(bob, hex"fe"); // Contract with just the invalid opcode.
+
+        vm.expectEmit(true, true, true, true);
+        emit WithdrawalFinalized(_withdrawalHash, false);
+        op.finalizeWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+        assert(address(bob).balance == bobBalanceBefore);
+    }
+
+    // Test: finalizeWithdrawalTransaction cannot finalize a withdrawal with itself (the OptimismPortal) as the target.
+    function test_finalizeWithdrawalTransaction_revertsOnSelfCall() external {
+        _defaultTx.target = address(op);
+        vm.expectRevert("OptimismPortal: you cannot send messages to the portal contract");
+        op.finalizeWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+    }
+
+    // Test: finalizeWithdrawalTransaction reverts if the outputRootProof does not match the output root
+    function test_finalizeWithdrawalTransaction_revertsOnInvalidOutputRootProof() external {
+        // Modify the version to invalidate the withdrawal proof.
+        _outputRootProof.version = bytes32(uint256(1));
+        vm.expectRevert("OptimismPortal: invalid output root proof");
+        op.finalizeWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+    }
+
+    // Test: finalizeWithdrawalTransaction reverts if the finalization period has not yet passed.
+    function test_finalizeWithdrawalTransaction_revertsOnRecentWithdrawal() external {
+        // Setup the Oracle to return an output with a recent timestamp
+        uint256 recentTimestamp = block.timestamp - 1000;
+        vm.mockCall(
+            address(op.L2_ORACLE()),
+            abi.encodeWithSelector(L2OutputOracle.getL2Output.selector),
+            abi.encode(bytes32(uint256(1)), recentTimestamp)
+        );
+
+        vm.expectRevert("OptimismPortal: proposal is not yet finalized");
+        op.finalizeWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+    }
+
+    // Test: finalizeWithdrawalTransaction reverts if the withdrawal has already been finalized.
+    function test_finalizeWithdrawalTransaction_revertsOnReplay() external {
+        vm.expectEmit(true, true, true, true);
+        emit WithdrawalFinalized(_withdrawalHash, true);
+        op.finalizeWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+        vm.expectRevert("OptimismPortal: withdrawal has already been finalized");
+        op.finalizeWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+    }
+
+    // Test: finalizeWithdrawalTransaction reverts if insufficient gas is supplied.
+    function test_finalizeWithdrawalTransaction_revertsOnInsufficientGas() external {
+        // This number was identified through trial and error.
+        uint256 gasLimit = 150_000;
+        Types.WithdrawalTransaction memory insufficientGasTx = Types.WithdrawalTransaction({
+            nonce: 0,
+            sender: alice,
+            target: bob,
+            value: 100,
+            gasLimit: gasLimit,
+            data: hex""
+        });
+        (
+            bytes32 stateRoot,
+            bytes32 storageRoot,
+            bytes32 outputRoot,
+            bytes32 withdrawalHash,
+            bytes memory withdrawalProof
+        ) = ffi.getFinalizeWithdrawalTransactionInputs(insufficientGasTx);
+        Types.OutputRootProof memory outputRootProof = Types.OutputRootProof({
+            version: bytes32(0),
+            stateRoot: stateRoot,
+            withdrawerStorageRoot: storageRoot,
+            latestBlockhash: bytes32(0)
+        });
+        vm.mockCall(
+            address(op.L2_ORACLE()),
+            abi.encodeWithSelector(L2OutputOracle.getL2Output.selector),
+            abi.encode(Hashing.hashOutputRootProof(outputRootProof), _proposedBlockNumber)
+        );
+        vm.expectRevert("OptimismPortal: insufficient gas to finalize withdrawal");
+        op.finalizeWithdrawalTransaction{ gas: gasLimit }(
+            insufficientGasTx,
+            _proposedBlockNumber,
+            outputRootProof,
+            withdrawalProof
+        );
+    }
+
+    // Test: finalizeWithdrawalTransaction reverts if the proof is invalid due to non-existence of
+    // the withdrawal.
+    function test_finalizeWithdrawalTransaction_revertsOninvalidWithdrawalProof() external {
+        // modify the default test values to invalidate the proof.
+        _defaultTx.data = hex"abcd";
+        vm.expectRevert("OptimismPortal: invalid withdrawal inclusion proof");
+        op.finalizeWithdrawalTransaction(
+            _defaultTx,
+            _proposedBlockNumber,
+            _outputRootProof,
+            _withdrawalProof
+        );
+    }
+
+    // Utility function used in the subsequent test. This is necessary to assert that the
+    // reentrant call will revert.
+    function callPortalAndExpectRevert() external payable {
+        vm.expectRevert("OptimismPortal: can only trigger one withdrawal per transaction");
+        // Arguments here don't matter, as the require check is the first thing that happens.
+        op.finalizeWithdrawalTransaction(_defaultTx, 0, _outputRootProof, hex"");
+        // Assert that the withdrawal was not finalized.
+        assertFalse(op.finalizedWithdrawals(Hashing.hashWithdrawal(_defaultTx)));
+    }
+
+    // Test: finalizeWithdrawalTransaction reverts if a sub-call attempts to finalize another
+    // withdrawal.
+    function test_finalizeWithdrawalTransaction_revertsOnReentrancy() external {
+        uint256 bobBalanceBefore = address(bob).balance;
+        // Copy and modify the default test values to attempt a reentrant call by first calling to
+        // this contract's callPortalAndExpectRevert() function above.
+        Types.WithdrawalTransaction memory _testTx = _defaultTx;
+        _testTx.target = address(this);
+        _testTx.data = abi.encodeWithSelector(this.callPortalAndExpectRevert.selector);
+
+        // Get modified proof inputs.
+        (
+            bytes32 stateRoot,
+            bytes32 storageRoot,
+            bytes32 outputRoot,
+            bytes32 withdrawalHash,
+            bytes memory withdrawalProof
+        ) = ffi.getFinalizeWithdrawalTransactionInputs(_testTx);
+        Types.OutputRootProof memory outputRootProof = Types.OutputRootProof({
+            version: bytes32(0),
+            stateRoot: stateRoot,
+            withdrawerStorageRoot: storageRoot,
+            latestBlockhash: bytes32(0)
+        });
+
+        // Setup the Oracle to return the outputRoot we want as well as a finalized timestamp.
+        uint256 finalizedTimestamp = block.timestamp - op.FINALIZATION_PERIOD_SECONDS() - 1;
+        vm.mockCall(
+            address(op.L2_ORACLE()),
+            abi.encodeWithSelector(L2OutputOracle.getL2Output.selector),
+            abi.encode(Types.OutputProposal(outputRoot, finalizedTimestamp))
+        );
+
+        // Assert that this contract is called with the expected data (i.e. the function signature of
+        // callPortalAndExpectRevert).
+        vm.expectCall(address(this), _testTx.data);
+        vm.expectEmit(true, true, true, true);
+        // Assert that the withdrawal should be finalized, and that the sub-call passes (because the
+        // assertions in callPortalAndExpectRevert pass).
+        emit WithdrawalFinalized(withdrawalHash, true);
+        op.finalizeWithdrawalTransaction(
+            _testTx,
+            _proposedBlockNumber,
+            outputRootProof,
+            withdrawalProof
+        );
+        // Ensure that bob's balance was not changed by the reentrant call.
+        assert(address(bob).balance == bobBalanceBefore);
+    }
 
     function test_finalizeWithdrawalTransaction_differential(
         address _sender,
@@ -321,40 +533,32 @@ contract OptimismPortal_Test is Portal_Initializer {
         // Cannot call the optimism portal
         vm.assume(_target != address(op));
         uint256 _nonce = messagePasser.nonce();
-
+        Types.WithdrawalTransaction memory _tx = Types.WithdrawalTransaction({
+            nonce: _nonce,
+            sender: _sender,
+            target: _target,
+            value: _value,
+            gasLimit: _gasLimit,
+            data: _data
+        });
         (
             bytes32 stateRoot,
             bytes32 storageRoot,
             bytes32 outputRoot,
             bytes32 withdrawalHash,
             bytes memory withdrawalProof
-        ) = ffi.getFinalizeWithdrawalTransactionInputs(
-            _nonce,
-            _sender,
-            _target,
-            _value,
-            uint256(_gasLimit),
-            _data
-        );
+        ) = ffi.getFinalizeWithdrawalTransactionInputs(_tx);
 
-        // Ensure the values returned from ffi are correct
-        assertEq(outputRoot, Hashing.hashOutputRootProof(Types.OutputRootProof({
+        Types.OutputRootProof memory proof = Types.OutputRootProof({
             version: bytes32(uint256(0)),
             stateRoot: stateRoot,
             withdrawerStorageRoot: storageRoot,
             latestBlockhash: bytes32(uint256(0))
-         })));
-
-        assertEq(withdrawalHash, Hashing.hashWithdrawal(
-            Types.WithdrawalTransaction(
-                _nonce,
-                _sender,
-                _target,
-                _value,
-                uint64(_gasLimit),
-                _data
-            )
-        ));
+        });
+
+        // Ensure the values returned from ffi are correct
+        assertEq(outputRoot, Hashing.hashOutputRootProof(proof));
+        assertEq(withdrawalHash, Hashing.hashWithdrawal(_tx));
 
         // Mock the call to the oracle
         vm.mockCall(
@@ -365,33 +569,17 @@ contract OptimismPortal_Test is Portal_Initializer {
 
         // Start the withdrawal, it must be initiated by the _sender and the
         // correct value must be passed along
-        vm.deal(_sender, _value);
-        vm.prank(_sender);
-        messagePasser.initiateWithdrawal{ value: _value }(
-            _target,
-            uint256(_gasLimit),
-            _data
-        );
+        vm.deal(_tx.sender, _tx.value);
+        vm.prank(_tx.sender);
+        messagePasser.initiateWithdrawal{ value: _tx.value }(_tx.target, _tx.gasLimit, _tx.data);
         // Ensure that the sentMessages is correct
         assertEq(messagePasser.sentMessages(withdrawalHash), true);
 
         vm.warp(op.FINALIZATION_PERIOD_SECONDS() + 1);
-        op.finalizeWithdrawalTransaction{ value: _value }(
-            Types.WithdrawalTransaction(
-                messagePasser.nonce() - 1,
-                _sender,
-                _target,
-                _value,
-                uint64(_gasLimit),
-                _data
-            ),
+        op.finalizeWithdrawalTransaction{ value: _tx.value }(
+            _tx,
             100, // l2BlockNumber
-            Types.OutputRootProof({
-                version: bytes32(uint256(0)),
-                stateRoot: stateRoot,
-                withdrawerStorageRoot: storageRoot,
-                latestBlockhash: bytes32(uint256(0))
-            }),
+            proof,
             withdrawalProof
         );
     }