contracts-bedrock: cleanup FeeVault (ethereum-optimism#12338)

* contracts-bedrock: cleanup `FeeVault` Updates the `FeeVault` to follow modern conventions used in the repo by moving to usage of interfaces rather than implementations. Also moves the `FeeVault` into the `L2` package as its only really useful on L2. This is meant to reduce the diff for the Stanard L2 Genesis by breaking up the refactor into its own small PR. * contracts: update semver-lock * semver-lock: fixup * cleanup: refactor * lint: fix * snapshots: regenerate * interface check: ignore fee vaults There is an issue with normalization of enums when they are return values
unattended-backpack · Oct 7, 2024 · b460aa2 · b460aa2
1 parent 981ee6a
commit b460aa2
Show file tree

Hide file tree

Showing 21 changed files with 151 additions and 116 deletions.
diff --git a/packages/contracts-bedrock/scripts/L2Genesis.s.sol b/packages/contracts-bedrock/scripts/L2Genesis.s.sol
@@ -20,12 +20,12 @@ import { BaseFeeVault } from "src/L2/BaseFeeVault.sol";
 import { L1FeeVault } from "src/L2/L1FeeVault.sol";
 import { OptimismSuperchainERC20Beacon } from "src/L2/OptimismSuperchainERC20Beacon.sol";
 import { OptimismMintableERC721Factory } from "src/universal/OptimismMintableERC721Factory.sol";
-import { FeeVault } from "src/universal/FeeVault.sol";
 import { GovernanceToken } from "src/governance/GovernanceToken.sol";
 
 // Libraries
 import { Predeploys } from "src/libraries/Predeploys.sol";
 import { Preinstalls } from "src/libraries/Preinstalls.sol";
+import { Types } from "src/libraries/Types.sol";
 
 // Interfaces
 import { IOptimismMintableERC20Factory } from "src/universal/interfaces/IOptimismMintableERC20Factory.sol";
@@ -344,7 +344,7 @@ contract L2Genesis is Deployer {
         SequencerFeeVault vault = new SequencerFeeVault({
             _recipient: cfg.sequencerFeeVaultRecipient(),
             _minWithdrawalAmount: cfg.sequencerFeeVaultMinimumWithdrawalAmount(),
-            _withdrawalNetwork: FeeVault.WithdrawalNetwork(cfg.sequencerFeeVaultWithdrawalNetwork())
+            _withdrawalNetwork: Types.WithdrawalNetwork(cfg.sequencerFeeVaultWithdrawalNetwork())
         });
 
         address impl = Predeploys.predeployToCodeNamespace(Predeploys.SEQUENCER_FEE_WALLET);
@@ -428,7 +428,7 @@ contract L2Genesis is Deployer {
         BaseFeeVault vault = new BaseFeeVault({
             _recipient: cfg.baseFeeVaultRecipient(),
             _minWithdrawalAmount: cfg.baseFeeVaultMinimumWithdrawalAmount(),
-            _withdrawalNetwork: FeeVault.WithdrawalNetwork(cfg.baseFeeVaultWithdrawalNetwork())
+            _withdrawalNetwork: Types.WithdrawalNetwork(cfg.baseFeeVaultWithdrawalNetwork())
         });
 
         address impl = Predeploys.predeployToCodeNamespace(Predeploys.BASE_FEE_VAULT);
@@ -445,7 +445,7 @@ contract L2Genesis is Deployer {
         L1FeeVault vault = new L1FeeVault({
             _recipient: cfg.l1FeeVaultRecipient(),
             _minWithdrawalAmount: cfg.l1FeeVaultMinimumWithdrawalAmount(),
-            _withdrawalNetwork: FeeVault.WithdrawalNetwork(cfg.l1FeeVaultWithdrawalNetwork())
+            _withdrawalNetwork: Types.WithdrawalNetwork(cfg.l1FeeVaultWithdrawalNetwork())
         });
 
         address impl = Predeploys.predeployToCodeNamespace(Predeploys.L1_FEE_VAULT);

diff --git a/packages/contracts-bedrock/scripts/checks/check-interfaces.sh b/packages/contracts-bedrock/scripts/checks/check-interfaces.sh
@@ -75,6 +75,12 @@ EXCLUDE_CONTRACTS=(
     "ICrossL2Inbox"
     "ISystemConfigInterop"
 
+    # Enums need to be normalized
+    "ISequencerFeeVault"
+    "IBaseFeeVault"
+    "IL1FeeVault"
+    "IFeeVault"
+
     # Solidity complains about receive but contract doens't have it.
     "IResolvedDelegateProxy"
 )

diff --git a/packages/contracts-bedrock/scripts/ops/FeeVaultWithdrawal.s.sol b/packages/contracts-bedrock/scripts/ops/FeeVaultWithdrawal.s.sol
@@ -5,7 +5,7 @@ import { console } from "forge-std/console.sol";
 import { Script } from "forge-std/Script.sol";
 import { IMulticall3 } from "forge-std/interfaces/IMulticall3.sol";
 import { Predeploys } from "src/libraries/Predeploys.sol";
-import { FeeVault } from "src/universal/FeeVault.sol";
+import { IFeeVault } from "src/L2/interfaces/IFeeVault.sol";
 
 /// @title FeeVaultWithdrawal
 /// @notice A script to make it very simple to withdraw from the fee vaults.
@@ -35,11 +35,11 @@ contract FeeVaultWithdrawal is Script {
                     IMulticall3.Call3({
                         target: vault,
                         allowFailure: false,
-                        callData: abi.encodeWithSelector(FeeVault.withdraw.selector)
+                        callData: abi.encodeWithSelector(IFeeVault.withdraw.selector)
                     })
                 );
 
-                address recipient = FeeVault(payable(vault)).RECIPIENT();
+                address recipient = IFeeVault(payable(vault)).RECIPIENT();
                 uint256 balance = vault.balance;
                 log(balance, recipient, vault);
             } else {
@@ -59,7 +59,7 @@ contract FeeVaultWithdrawal is Script {
     /// @notice Checks whether or not a FeeVault can be withdrawn. The balance of the account must
     ///         be larger than the `MIN_WITHDRAWAL_AMOUNT`.
     function canWithdrawal(address _vault) internal view returns (bool) {
-        uint256 minWithdrawalAmount = FeeVault(payable(_vault)).MIN_WITHDRAWAL_AMOUNT();
+        uint256 minWithdrawalAmount = IFeeVault(payable(_vault)).MIN_WITHDRAWAL_AMOUNT();
         uint256 balance = _vault.balance;
         return balance >= minWithdrawalAmount;
     }

diff --git a/packages/contracts-bedrock/semver-lock.json b/packages/contracts-bedrock/semver-lock.json
@@ -56,8 +56,8 @@
     "sourceCodeHash": "0x441d1e3e8e987f829f55996b5b6c850da8c59ad48f09cf7e0a69a1fa559d42a2"
   },
   "src/L2/BaseFeeVault.sol": {
-    "initCodeHash": "0x3bfcd57e25ad54b66c374f63e24e33a6cf107044aa8f5f69ef21202c380b5c5b",
-    "sourceCodeHash": "0x2dc2284cf7c68e743da50e4113e96ffeab435de2390aeba2eab2f1e8ca411ce9"
+    "initCodeHash": "0xbf49824cf37e201181484a8a423fcad8f504dc925921a2b28e83398197858dec",
+    "sourceCodeHash": "0x983e8e248c61e362ba6a01dd2e217a535c9bb828dc0b4421f5f27e0577f2e14c"
   },
   "src/L2/CrossL2Inbox.sol": {
     "initCodeHash": "0x66b052adce7e9194d054952d67d08b53964120067600358243ec86c85b90877b",
@@ -80,8 +80,8 @@
     "sourceCodeHash": "0x7417677643e1df1ae1782513b94c7821097b9529d3f8626c3bcb8b3a9ae0d180"
   },
   "src/L2/L1FeeVault.sol": {
-    "initCodeHash": "0x3bfcd57e25ad54b66c374f63e24e33a6cf107044aa8f5f69ef21202c380b5c5b",
-    "sourceCodeHash": "0x927cc729bf5c9f209112df597f649493f276c4c50e17a57f7da02c2be266b192"
+    "initCodeHash": "0xbf49824cf37e201181484a8a423fcad8f504dc925921a2b28e83398197858dec",
+    "sourceCodeHash": "0xc7cda130f2bb3648e04d5a480082aa1789e16456c1280954d822b05d30100b2d"
   },
   "src/L2/L2CrossDomainMessenger.sol": {
     "initCodeHash": "0xcc4527d21cceeedbb3cbf8e7028e22fe12bc1ab30365dbebd0713499451b959d",
@@ -120,8 +120,8 @@
     "sourceCodeHash": "0x155a4b22ff8e266560d1fae72e1db7fc164afd84b8a81afb74c69414e0d5438e"
   },
   "src/L2/SequencerFeeVault.sol": {
-    "initCodeHash": "0x2e6551705e493bacba8cffe22e564d5c401ae5bb02577a5424e0d32784e13e74",
-    "sourceCodeHash": "0xd56922cb04597dea469c65e5a49d4b3c50c171e603601e6f41da9517cae0b11a"
+    "initCodeHash": "0xcaadbf08057b5d47f7704257e9385a29e42a7a08c818646d109c5952d3d35218",
+    "sourceCodeHash": "0x05bbc6039e5a9ff38987e7b9b89c69e2ee8aa4b7ca20dd002ea1bbd3d70f27f3"
   },
   "src/L2/SuperchainWETH.sol": {
     "initCodeHash": "0x4ccd25f37a816205bc26f8532afa66e02f2b36ca7b7404d0fa48a4313ed16f0c",

diff --git a/packages/contracts-bedrock/snapshots/abi/BaseFeeVault.json b/packages/contracts-bedrock/snapshots/abi/BaseFeeVault.json
@@ -12,7 +12,7 @@
         "type": "uint256"
       },
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "_withdrawalNetwork",
         "type": "uint8"
       }
@@ -55,7 +55,7 @@
     "name": "WITHDRAWAL_NETWORK",
     "outputs": [
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "",
         "type": "uint8"
       }
@@ -127,7 +127,7 @@
     "name": "withdrawalNetwork",
     "outputs": [
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "network_",
         "type": "uint8"
       }
@@ -183,7 +183,7 @@
       },
       {
         "indexed": false,
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "withdrawalNetwork",
         "type": "uint8"
       }

diff --git a/packages/contracts-bedrock/snapshots/abi/L1FeeVault.json b/packages/contracts-bedrock/snapshots/abi/L1FeeVault.json
@@ -12,7 +12,7 @@
         "type": "uint256"
       },
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "_withdrawalNetwork",
         "type": "uint8"
       }
@@ -55,7 +55,7 @@
     "name": "WITHDRAWAL_NETWORK",
     "outputs": [
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "",
         "type": "uint8"
       }
@@ -127,7 +127,7 @@
     "name": "withdrawalNetwork",
     "outputs": [
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "network_",
         "type": "uint8"
       }
@@ -183,7 +183,7 @@
       },
       {
         "indexed": false,
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "withdrawalNetwork",
         "type": "uint8"
       }

diff --git a/packages/contracts-bedrock/snapshots/abi/SequencerFeeVault.json b/packages/contracts-bedrock/snapshots/abi/SequencerFeeVault.json
@@ -12,7 +12,7 @@
         "type": "uint256"
       },
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "_withdrawalNetwork",
         "type": "uint8"
       }
@@ -55,7 +55,7 @@
     "name": "WITHDRAWAL_NETWORK",
     "outputs": [
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "",
         "type": "uint8"
       }
@@ -140,7 +140,7 @@
     "name": "withdrawalNetwork",
     "outputs": [
       {
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "network_",
         "type": "uint8"
       }
@@ -196,7 +196,7 @@
       },
       {
         "indexed": false,
-        "internalType": "enum FeeVault.WithdrawalNetwork",
+        "internalType": "enum Types.WithdrawalNetwork",
         "name": "withdrawalNetwork",
         "type": "uint8"
       }

diff --git a/packages/contracts-bedrock/src/L2/BaseFeeVault.sol b/packages/contracts-bedrock/src/L2/BaseFeeVault.sol
@@ -2,16 +2,18 @@
 pragma solidity 0.8.15;
 
 import { ISemver } from "src/universal/interfaces/ISemver.sol";
-import { FeeVault } from "src/universal/FeeVault.sol";
+import { FeeVault } from "src/L2/FeeVault.sol";
+
+import { Types } from "src/libraries/Types.sol";
 
 /// @custom:proxied true
 /// @custom:predeploy 0x4200000000000000000000000000000000000019
 /// @title BaseFeeVault
 /// @notice The BaseFeeVault accumulates the base fee that is paid by transactions.
 contract BaseFeeVault is FeeVault, ISemver {
     /// @notice Semantic version.
-    /// @custom:semver 1.5.0-beta.2
-    string public constant version = "1.5.0-beta.2";
+    /// @custom:semver 1.5.0-beta.3
+    string public constant version = "1.5.0-beta.3";
 
     /// @notice Constructs the BaseFeeVault contract.
     /// @param _recipient           Wallet that will receive the fees.
@@ -20,7 +22,7 @@ contract BaseFeeVault is FeeVault, ISemver {
     constructor(
         address _recipient,
         uint256 _minWithdrawalAmount,
-        WithdrawalNetwork _withdrawalNetwork
+        Types.WithdrawalNetwork _withdrawalNetwork
     )
         FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork)
     { }

diff --git a/...tracts-bedrock/src/universal/FeeVault.sol → ...ges/contracts-bedrock/src/L2/FeeVault.sol b/...tracts-bedrock/src/universal/FeeVault.sol → ...ges/contracts-bedrock/src/L2/FeeVault.sol
@@ -8,18 +8,13 @@ import { Predeploys } from "src/libraries/Predeploys.sol";
 // Interfaces
 import { IL2ToL1MessagePasser } from "src/L2/interfaces/IL2ToL1MessagePasser.sol";
 
+// Libraries
+import { Types } from "src/libraries/Types.sol";
+
 /// @title FeeVault
 /// @notice The FeeVault contract contains the basic logic for the various different vault contracts
 ///         used to hold fee revenue generated by the L2 system.
 abstract contract FeeVault {
-    /// @notice Enum representing where the FeeVault withdraws funds to.
-    /// @custom:value L1 FeeVault withdraws funds to L1.
-    /// @custom:value L2 FeeVault withdraws funds to L2.
-    enum WithdrawalNetwork {
-        L1,
-        L2
-    }
-
     /// @notice Minimum balance before a withdrawal can be triggered.
     ///         Use the `minWithdrawalAmount()` getter as this is deprecated
     ///         and is subject to be removed in the future.
@@ -36,7 +31,7 @@ abstract contract FeeVault {
     ///         Use the `withdrawalNetwork()` getter as this is deprecated
     ///         and is subject to be removed in the future.
     /// @custom:legacy
-    WithdrawalNetwork public immutable WITHDRAWAL_NETWORK;
+    Types.WithdrawalNetwork public immutable WITHDRAWAL_NETWORK;
 
     /// @notice The minimum gas limit for the FeeVault withdrawal transaction.
     uint32 internal constant WITHDRAWAL_MIN_GAS = 400_000;
@@ -59,12 +54,12 @@ abstract contract FeeVault {
     /// @param to                Address that the funds were sent to.
     /// @param from              Address that triggered the withdrawal.
     /// @param withdrawalNetwork Network which the to address will receive funds on.
-    event Withdrawal(uint256 value, address to, address from, WithdrawalNetwork withdrawalNetwork);
+    event Withdrawal(uint256 value, address to, address from, Types.WithdrawalNetwork withdrawalNetwork);
 
     /// @param _recipient           Wallet that will receive the fees.
     /// @param _minWithdrawalAmount Minimum balance for withdrawals.
     /// @param _withdrawalNetwork   Network which the recipient will receive fees on.
-    constructor(address _recipient, uint256 _minWithdrawalAmount, WithdrawalNetwork _withdrawalNetwork) {
+    constructor(address _recipient, uint256 _minWithdrawalAmount, Types.WithdrawalNetwork _withdrawalNetwork) {
         RECIPIENT = _recipient;
         MIN_WITHDRAWAL_AMOUNT = _minWithdrawalAmount;
         WITHDRAWAL_NETWORK = _withdrawalNetwork;
@@ -84,7 +79,7 @@ abstract contract FeeVault {
     }
 
     /// @notice Network which the recipient will receive fees on.
-    function withdrawalNetwork() public view returns (WithdrawalNetwork network_) {
+    function withdrawalNetwork() public view returns (Types.WithdrawalNetwork network_) {
         network_ = WITHDRAWAL_NETWORK;
     }
 
@@ -101,7 +96,7 @@ abstract contract FeeVault {
         emit Withdrawal(value, RECIPIENT, msg.sender);
         emit Withdrawal(value, RECIPIENT, msg.sender, WITHDRAWAL_NETWORK);
 
-        if (WITHDRAWAL_NETWORK == WithdrawalNetwork.L2) {
+        if (WITHDRAWAL_NETWORK == Types.WithdrawalNetwork.L2) {
             bool success = SafeCall.send(RECIPIENT, value);
             require(success, "FeeVault: failed to send ETH to L2 fee recipient");
         } else {

diff --git a/packages/contracts-bedrock/src/L2/L1FeeVault.sol b/packages/contracts-bedrock/src/L2/L1FeeVault.sol
@@ -2,16 +2,18 @@
 pragma solidity 0.8.15;
 
 import { ISemver } from "src/universal/interfaces/ISemver.sol";
-import { FeeVault } from "src/universal/FeeVault.sol";
+import { FeeVault } from "src/L2/FeeVault.sol";
+
+import { Types } from "src/libraries/Types.sol";
 
 /// @custom:proxied true
 /// @custom:predeploy 0x420000000000000000000000000000000000001A
 /// @title L1FeeVault
 /// @notice The L1FeeVault accumulates the L1 portion of the transaction fees.
 contract L1FeeVault is FeeVault, ISemver {
     /// @notice Semantic version.
-    /// @custom:semver 1.5.0-beta.2
-    string public constant version = "1.5.0-beta.2";
+    /// @custom:semver 1.5.0-beta.3
+    string public constant version = "1.5.0-beta.3";
 
     /// @notice Constructs the L1FeeVault contract.
     /// @param _recipient           Wallet that will receive the fees.
@@ -20,7 +22,7 @@ contract L1FeeVault is FeeVault, ISemver {
     constructor(
         address _recipient,
         uint256 _minWithdrawalAmount,
-        WithdrawalNetwork _withdrawalNetwork
+        Types.WithdrawalNetwork _withdrawalNetwork
     )
         FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork)
     { }

diff --git a/packages/contracts-bedrock/src/L2/SequencerFeeVault.sol b/packages/contracts-bedrock/src/L2/SequencerFeeVault.sol
@@ -2,16 +2,18 @@
 pragma solidity 0.8.15;
 
 import { ISemver } from "src/universal/interfaces/ISemver.sol";
-import { FeeVault } from "src/universal/FeeVault.sol";
+import { FeeVault } from "src/L2/FeeVault.sol";
+
+import { Types } from "src/libraries/Types.sol";
 
 /// @custom:proxied true
 /// @custom:predeploy 0x4200000000000000000000000000000000000011
 /// @title SequencerFeeVault
 /// @notice The SequencerFeeVault is the contract that holds any fees paid to the Sequencer during
 ///         transaction processing and block production.
 contract SequencerFeeVault is FeeVault, ISemver {
-    /// @custom:semver 1.5.0-beta.2
-    string public constant version = "1.5.0-beta.2";
+    /// @custom:semver 1.5.0-beta.3
+    string public constant version = "1.5.0-beta.3";
 
     /// @notice Constructs the SequencerFeeVault contract.
     /// @param _recipient           Wallet that will receive the fees.
@@ -20,7 +22,7 @@ contract SequencerFeeVault is FeeVault, ISemver {
     constructor(
         address _recipient,
         uint256 _minWithdrawalAmount,
-        WithdrawalNetwork _withdrawalNetwork
+        Types.WithdrawalNetwork _withdrawalNetwork
     )
         FeeVault(_recipient, _minWithdrawalAmount, _withdrawalNetwork)
     { }