Merge pull request #6565 from ethereum-optimism/jm/fix-feevault

packages/contracts-bedrock/.gas-snapshot

@@ -552,11 +552,12 @@ SafeCall_Test:test_callWithMinGas_noLeakageHigh_succeeds() (gas: 1021670598)
 SafeCall_Test:test_callWithMinGas_noLeakageLow_succeeds() (gas: 1095190710)
 Semver_Test:test_behindProxy_succeeds() (gas: 506748)
 Semver_Test:test_version_succeeds() (gas: 9418)
-SequencerFeeVault_L2Withdrawal_Test:test_withdraw_toL2_succeeds() (gas: 78286)
+SequencerFeeVault_L2Withdrawal_Test:test_withdraw_toL2_succeeds() (gas: 78333)
+SequencerFeeVault_L2Withdrawal_Test:test_withdraw_toL2recipientReverts_fails() (gas: 46486)
 SequencerFeeVault_Test:test_constructor_succeeds() (gas: 5526)
 SequencerFeeVault_Test:test_minWithdrawalAmount_succeeds() (gas: 5464)
 SequencerFeeVault_Test:test_receive_succeeds() (gas: 17373)
-SequencerFeeVault_Test:test_withdraw_notEnough_reverts() (gas: 9331)
+SequencerFeeVault_Test:test_withdraw_notEnough_reverts() (gas: 9332)
 SequencerFeeVault_Test:test_withdraw_toL1_succeeds() (gas: 169242)
 SetPrevBaseFee_Test:test_setPrevBaseFee_succeeds() (gas: 11515)
 StandardBridge_Stateless_Test:test_isCorrectTokenPair_succeeds() (gas: 49936)

packages/contracts-bedrock/semver-lock.json

@@ -5,15 +5,15 @@
   "src/L1/L2OutputOracle.sol": "0x2b285a897d3285975bd47e89bd5ec7025369931384f9f02a20f48254dbfca181",
   "src/L1/OptimismPortal.sol": "0xd5abaa3d1093c41f8e81b3cd298d4a35f90d103d9bca566a47ca562635f2f943",
   "src/L1/SystemConfig.sol": "0xbd2be6c19e6e85eae73ddf3cd6304a395e2a41d86aee1c15b6b0f044bf54232e",
-  "src/L2/BaseFeeVault.sol": "0xc7bf7c43dd5362f6474fc3da8bb38d1c616ed2a3396486446482acf62cacfc2f",
+  "src/L2/BaseFeeVault.sol": "0xaa2bf66e16ed9098af4251af94c30741f6ba094e321b786eff51b59574c6ad25",
   "src/L2/GasPriceOracle.sol": "0x712134045fba966b0d0cc28019f5c2bd298d999649f387d989f744b274020a82",
   "src/L2/L1Block.sol": "0x64d2517a595a5b5af7eef1070920eb90aa595871ec55ba8b6d6aa323043ac000",
-  "src/L2/L1FeeVault.sol": "0x49f6a1a89e83467e84110232f9e3c5fb2d0fad373f12afeef6b15d135a605659",
+  "src/L2/L1FeeVault.sol": "0xc41752b9ce3d72b0f24c9baaa995e574e1a6b6166337485413a86c20f7063256",
   "src/L2/L2CrossDomainMessenger.sol": "0xd1e057fe1889e0701f447af8016e4a201febdc28f640138878435746b3c6f647",
   "src/L2/L2ERC721Bridge.sol": "0x2a0c241efb516161a12625e23d1e5aa32da815892e4fcc52f3b12d41cdff53b2",
   "src/L2/L2StandardBridge.sol": "0x8ee5257e03ae4ba8555d9f7d13374c8a388315d62c16107bb4cadd450bfeb3d3",
   "src/L2/L2ToL1MessagePasser.sol": "0x7e35c3c4f1dd3d131dd71db07676301f7c477f02b6d6bf0ec468ecf2bed8325b",
-  "src/L2/SequencerFeeVault.sol": "0x17b30ccaed8b8dbe965c892cb8aae7f594fb4a87e0edd3ca6cd8f94559b86df9",
+  "src/L2/SequencerFeeVault.sol": "0xd7266bada6ee69aa484d5d3c0590441b4475a786ba0c7c4872f2114fca96d9ee",
   "src/dispute/FaultDisputeGame.sol": "0xb36f6456d74a9ee93df97bb6d5a554dcb531d730e6e2b10dd442897875f2ca81",
   "src/legacy/DeployerWhitelist.sol": "0x47277d9c8409d517501d172db6697d55090d3d3a9e4bb2b1adea83471d793b6b",
   "src/legacy/L1BlockNumber.sol": "0x1a1690b8b5ab53cf2b5c8e85fb86028b4078ae656286ae482cabe68374334f2a",

packages/contracts-bedrock/src/L2/BaseFeeVault.sol

@@ -9,7 +9,7 @@ import { FeeVault } from "../universal/FeeVault.sol";
 /// @title BaseFeeVault
 /// @notice The BaseFeeVault accumulates the base fee that is paid by transactions.
 contract BaseFeeVault is FeeVault, Semver {
-    /// @custom:semver 1.2.1
+    /// @custom:semver 1.3.0
     /// @notice Constructs the BaseFeeVault contract.
     /// @param _recipient           Wallet that will receive the fees.
     /// @param _minWithdrawalAmount Minimum balance for withdrawals.
@@ -18,5 +18,5 @@ contract BaseFeeVault is FeeVault, Semver {
         address _recipient,
         uint256 _minWithdrawalAmount,
         WithdrawalNetwork _withdrawalNetwork
-    ) FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork) Semver(1, 2, 1) {}
+    ) FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork) Semver(1, 3, 0) {}
 }

packages/contracts-bedrock/src/L2/L1FeeVault.sol

@@ -9,7 +9,7 @@ import { FeeVault } from "../universal/FeeVault.sol";
 /// @title L1FeeVault
 /// @notice The L1FeeVault accumulates the L1 portion of the transaction fees.
 contract L1FeeVault is FeeVault, Semver {
-    /// @custom:semver 1.2.1
+    /// @custom:semver 1.3.0
     /// @notice Constructs the L1FeeVault contract.
     /// @param _recipient           Wallet that will receive the fees.
     /// @param _minWithdrawalAmount Minimum balance for withdrawals.
@@ -18,5 +18,5 @@ contract L1FeeVault is FeeVault, Semver {
         address _recipient,
         uint256 _minWithdrawalAmount,
         WithdrawalNetwork _withdrawalNetwork
-    ) FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork) Semver(1, 2, 1) {}
+    ) FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork) Semver(1, 3, 0) {}
 }

packages/contracts-bedrock/src/L2/SequencerFeeVault.sol

@@ -10,7 +10,7 @@ import { FeeVault } from "../universal/FeeVault.sol";
 /// @notice The SequencerFeeVault is the contract that holds any fees paid to the Sequencer during
 ///         transaction processing and block production.
 contract SequencerFeeVault is FeeVault, Semver {
-    /// @custom:semver 1.2.1
+    /// @custom:semver 1.3.0
     /// @notice Constructs the SequencerFeeVault contract.
     /// @param _recipient           Wallet that will receive the fees.
     /// @param _minWithdrawalAmount Minimum balance for withdrawals.
@@ -19,7 +19,7 @@ contract SequencerFeeVault is FeeVault, Semver {
         address _recipient,
         uint256 _minWithdrawalAmount,
         WithdrawalNetwork _withdrawalNetwork
-    ) FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork) Semver(1, 2, 1) {}
+    ) FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork) Semver(1, 3, 0) {}
 
     /// @custom:legacy
     /// @notice Legacy getter for the recipient address.

packages/contracts-bedrock/src/universal/FeeVault.sol

@@ -3,7 +3,6 @@ pragma solidity 0.8.15;
 
 import { L2StandardBridge } from "../L2/L2StandardBridge.sol";
 import { Predeploys } from "../libraries/Predeploys.sol";
-import { SafeCall } from "../libraries/SafeCall.sol";
 
 /// @title FeeVault
 /// @notice The FeeVault contract contains the basic logic for the various different vault contracts
@@ -76,7 +75,8 @@ abstract contract FeeVault {
         emit Withdrawal(value, RECIPIENT, msg.sender, WITHDRAWAL_NETWORK);
 
         if (WITHDRAWAL_NETWORK == WithdrawalNetwork.L2) {
-            SafeCall.send(RECIPIENT, gasleft(), value);
+            (bool success, ) = RECIPIENT.call{ value: value }(hex"");
+            require(success, "FeeVault: failed to send ETH to L2 fee recipient");
         } else {
             L2StandardBridge(payable(Predeploys.L2_STANDARD_BRIDGE)).bridgeETHTo{ value: value }(
                 RECIPIENT,

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

@@ -2,7 +2,7 @@
 pragma solidity 0.8.15;
 
 // Testing utilities
-import { FeeVault_Initializer } from "./CommonTest.t.sol";
+import { FeeVault_Initializer, Reverter } from "./CommonTest.t.sol";
 import { StandardBridge } from "../src/universal/StandardBridge.sol";
 
 // Libraries
@@ -137,4 +137,25 @@ contract SequencerFeeVault_L2Withdrawal_Test is FeeVault_Initializer {
         assertEq(address(vault).balance, ZERO_VALUE);
         assertEq(recipient.balance, amount);
     }
+
+    /// @dev Tests that `withdraw` fails if the Recipient reverts. This also serves to simulate
+    ///     a situation where insufficient gas is provided to the RECIPIENT.
+    function test_withdraw_toL2recipientReverts_fails() external {
+        uint256 amount = vault.MIN_WITHDRAWAL_AMOUNT();
+
+        vm.deal(address(vault), amount);
+        // No ether has been withdrawn yet
+        assertEq(vault.totalProcessed(), 0);
+
+        // Ensure the RECIPIENT reverts
+        vm.etch(vault.RECIPIENT(), type(Reverter).runtimeCode);
+
+        // The entire vault's balance is withdrawn
+        vm.expectCall(recipient, address(vault).balance, bytes(""));
+        vm.expectRevert(
+            "FeeVault: failed to send ETH to L2 fee recipient"
+        );
+        vault.withdraw();
+        assertEq(vault.totalProcessed(), 0);
+    }
 }