feat: ban deposits interop

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

@@ -4,7 +4,6 @@ pragma solidity 0.8.15;
 import { ISemver } from "src/universal/ISemver.sol";
 import { Constants } from "src/libraries/Constants.sol";
 import { GasPayingToken, IGasToken } from "src/libraries/GasPayingToken.sol";
-import { Predeploys } from "src/libraries/Predeploys.sol";
 import "src/libraries/L1BlockErrors.sol";
 
 /// @custom:proxied
@@ -58,11 +57,6 @@ contract L1Block is ISemver, IGasToken {
     /// @notice The latest L1 blob base fee.
     uint256 public blobBaseFee;
 
-    /// @notice Storage slot that the isDeposit is stored at.
-    ///         This is a custom slot that is not part of the standard storage layout.
-    /// keccak256(abi.encode(uint256(keccak256("l1Block.identifier.isDeposit")) - 1)) & ~bytes32(uint256(0xff))
-    uint256 internal constant IS_DEPOSIT_SLOT = 0x921bd3a089295c6e5540e8fba8195448d253efd6f2e3e495b499b627dc36a300;
-
     /// @custom:semver 1.5.1-beta.1
     function version() public pure virtual returns (string memory) {
         return "1.5.1-beta.1";
@@ -93,15 +87,6 @@ contract L1Block is ISemver, IGasToken {
         return token != Constants.ETHER;
     }
 
-    /// @notice Returns whether the call was triggered from a a deposit or not.
-    /// @notice This function is only callable by the CrossL2Inbox contract.
-    function isDeposit() external view returns (bool isDeposit_) {
-        if (msg.sender != Predeploys.CROSS_L2_INBOX) revert NotCrossL2Inbox();
-        assembly {
-            isDeposit_ := sload(IS_DEPOSIT_SLOT)
-        }
-    }
-
     /// @custom:legacy
     /// @notice Updates the L1 block values.
     /// @param _number         L1 blocknumber.
@@ -136,16 +121,31 @@ contract L1Block is ISemver, IGasToken {
         l1FeeScalar = _l1FeeScalar;
     }
 
-    /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain.
-    ///         It updates the L1 block values through the `setL1BlockValuesEcotone` function.
-    ///         It forwards the calldata to the internally-used `setL1BlockValuesEcotone` function.
-    function setL1BlockValuesIsthmus() external {
-        // Set the isDeposit flag to true.
-        assembly {
-            sstore(IS_DEPOSIT_SLOT, 1)
-        }
+    /// @notice Updates the L1 block values for an Ecotone upgraded chain.
+    /// Params are packed and passed in as raw msg.data instead of ABI to reduce calldata size.
+    /// Params are expected to be in the following order:
+    ///   1. _baseFeeScalar      L1 base fee scalar
+    ///   2. _blobBaseFeeScalar  L1 blob base fee scalar
+    ///   3. _sequenceNumber     Number of L2 blocks since epoch start.
+    ///   4. _timestamp          L1 timestamp.
+    ///   5. _number             L1 blocknumber.
+    ///   6. _basefee            L1 base fee.
+    ///   7. _blobBaseFee        L1 blob base fee.
+    ///   8. _hash               L1 blockhash.
+    ///   9. _batcherHash        Versioned hash to authenticate batcher by.
+    function setL1BlockValuesEcotone() public {
+        _setL1BlockValuesEcotone();
+    }
+
+    /// @notice Sets the gas paying token for the L2 system. Can only be called by the special
+    ///         depositor account. This function is not called on every L2 block but instead
+    ///         only called by specially crafted L1 deposit transactions.
+    function setGasPayingToken(address _token, uint8 _decimals, bytes32 _name, bytes32 _symbol) external {
+        if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor();
 
-        setL1BlockValuesEcotone();
+        GasPayingToken.set({ _token: _token, _decimals: _decimals, _name: _name, _symbol: _symbol });
+
+        emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol });
     }
 
     /// @notice Updates the L1 block values for an Ecotone upgraded chain.
@@ -160,7 +160,7 @@ contract L1Block is ISemver, IGasToken {
     ///   7. _blobBaseFee        L1 blob base fee.
     ///   8. _hash               L1 blockhash.
     ///   9. _batcherHash        Versioned hash to authenticate batcher by.
-    function setL1BlockValuesEcotone() public {
+    function _setL1BlockValuesEcotone() internal {
         address depositor = DEPOSITOR_ACCOUNT();
         assembly {
             // Revert if the caller is not the depositor account.
@@ -178,26 +178,4 @@ contract L1Block is ISemver, IGasToken {
             sstore(batcherHash.slot, calldataload(132)) // bytes32
         }
     }
-
-    /// @notice Resets the isDeposit flag.
-    ///         Should only be called by the depositor account after the deposits are complete.
-    function depositsComplete() external {
-        if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor();
-
-        // Set the isDeposit flag to false.
-        assembly {
-            sstore(IS_DEPOSIT_SLOT, 0)
-        }
-    }
-
-    /// @notice Sets the gas paying token for the L2 system. Can only be called by the special
-    ///         depositor account. This function is not called on every L2 block but instead
-    ///         only called by specially crafted L1 deposit transactions.
-    function setGasPayingToken(address _token, uint8 _decimals, bytes32 _name, bytes32 _symbol) external {
-        if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor();
-
-        GasPayingToken.set({ _token: _token, _decimals: _decimals, _name: _name, _symbol: _symbol });
-
-        emit GasPayingTokenSet({ token: _token, decimals: _decimals, name: _name, symbol: _symbol });
-    }
 }

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

@@ -5,6 +5,7 @@ import { L1Block } from "src/L2/L1Block.sol";
 import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 import { GasPayingToken } from "src/libraries/GasPayingToken.sol";
 import { StaticConfig } from "src/libraries/StaticConfig.sol";
+import { Predeploys } from "src/libraries/Predeploys.sol";
 import "src/libraries/L1BlockErrors.sol";
 
 /// @notice Enum representing different types of configurations that can be set on L1BlockInterop.
@@ -33,11 +34,25 @@ contract L1BlockInterop is L1Block {
     /// @notice The interop dependency set, containing the chain IDs in it.
     EnumerableSet.UintSet dependencySet;
 
+    /// @notice Storage slot that the isDeposit is stored at.
+    ///         This is a custom slot that is not part of the standard storage layout.
+    /// keccak256(abi.encode(uint256(keccak256("l1Block.identifier.isDeposit")) - 1)) & ~bytes32(uint256(0xff))
+    uint256 internal constant IS_DEPOSIT_SLOT = 0x921bd3a089295c6e5540e8fba8195448d253efd6f2e3e495b499b627dc36a300;
+
     /// @custom:semver +interop
     function version() public pure override returns (string memory) {
         return string.concat(super.version(), "+interop");
     }
 
+    /// @notice Returns whether the call was triggered from a a deposit or not.
+    /// @notice This function is only callable by the CrossL2Inbox contract.
+    function isDeposit() external view returns (bool isDeposit_) {
+        if (msg.sender != Predeploys.CROSS_L2_INBOX) revert NotCrossL2Inbox();
+        assembly {
+            isDeposit_ := sload(IS_DEPOSIT_SLOT)
+        }
+    }
+
     /// @notice Returns true if a chain ID is in the interop dependency set and false otherwise.
     ///         The chain's chain ID is always considered to be in the dependency set.
     /// @param _chainId The chain ID to check.
@@ -52,6 +67,29 @@ contract L1BlockInterop is L1Block {
         return uint8(dependencySet.length());
     }
 
+    /// @notice Updates the `isDeposit` flag and sets the L1 block values for an Isthmus upgraded chain.
+    ///         It updates the L1 block values through the `setL1BlockValuesEcotone` function.
+    ///         It forwards the calldata to the internally-used `setL1BlockValuesEcotone` function.
+    function setL1BlockValuesIsthmus() external {
+        // Set the isDeposit flag to true.
+        assembly {
+            sstore(IS_DEPOSIT_SLOT, 1)
+        }
+
+        _setL1BlockValuesEcotone();
+    }
+
+    /// @notice Resets the isDeposit flag.
+    ///         Should only be called by the depositor account after the deposits are complete.
+    function depositsComplete() external {
+        if (msg.sender != DEPOSITOR_ACCOUNT()) revert NotDepositor();
+
+        // Set the isDeposit flag to false.
+        assembly {
+            sstore(IS_DEPOSIT_SLOT, 0)
+        }
+    }
+
     /// @notice Sets static configuration options for the L2 system. Can only be called by the special
     ///         depositor account.
     /// @param _type  The type of configuration to set.

packages/contracts-bedrock/test/L2/L1Block.t.sol

@@ -8,10 +8,9 @@ import { CommonTest } from "test/setup/CommonTest.sol";
 import { Encoding } from "src/libraries/Encoding.sol";
 import { Constants } from "src/libraries/Constants.sol";
 
-// Target contract
+// Target contract dependencies
 import { L1Block } from "src/L2/L1Block.sol";
-import { NotDepositor, NotCrossL2Inbox } from "src/libraries/L1BlockErrors.sol";
-import { Predeploys } from "src/libraries/Predeploys.sol";
+import "src/libraries/L1BlockErrors.sol";
 
 contract L1BlockTest is CommonTest {
     address depositor;
@@ -82,59 +81,6 @@ contract L1BlockBedrock_Test is L1BlockTest {
     }
 }
 
-contract L1BlockSetL1BlockValuesIsthmus_Test is L1BlockTest {
-    /// @dev Tests that `setL1BlockValuesIsthmus` reverts if sender address is not the depositor
-    function test_setL1BlockValuesIsthmus_notDepositor_reverts(address _caller) external {
-        vm.assume(_caller != depositor);
-        vm.prank(_caller);
-        vm.expectRevert(NotDepositor.selector);
-        l1Block.setL1BlockValuesIsthmus();
-    }
-
-    /// @dev Tests that `setL1BlockValuesIsthmus` succeeds if sender address is the depositor
-    function test_setL1BlockValuesIsthmus_succeeds(
-        uint32 baseFeeScalar,
-        uint32 blobBaseFeeScalar,
-        uint64 sequenceNumber,
-        uint64 timestamp,
-        uint64 number,
-        uint256 baseFee,
-        uint256 blobBaseFee,
-        bytes32 hash,
-        bytes32 batcherHash
-    )
-        external
-    {
-        // Ensure the `isDepositTransaction` flag is false before calling `setL1BlockValuesIsthmus`
-        vm.prank(Predeploys.CROSS_L2_INBOX);
-        assertEq(l1Block.isDeposit(), false);
-
-        bytes memory setValuesEcotoneCalldata = abi.encodePacked(
-            baseFeeScalar, blobBaseFeeScalar, sequenceNumber, timestamp, number, baseFee, blobBaseFee, hash, batcherHash
-        );
-
-        vm.prank(depositor);
-        (bool success,) =
-            address(l1Block).call(abi.encodePacked(L1Block.setL1BlockValuesIsthmus.selector, setValuesEcotoneCalldata));
-        assertTrue(success, "function call failed");
-
-        // Assert that the `isDepositTransaction` flag was properly set to true
-        vm.prank(Predeploys.CROSS_L2_INBOX);
-        assertEq(l1Block.isDeposit(), true);
-
-        // Assert `setL1BlockValuesEcotone` was properly called, forwarding the calldata to it
-        assertEq(l1Block.baseFeeScalar(), baseFeeScalar, "1");
-        assertEq(l1Block.blobBaseFeeScalar(), blobBaseFeeScalar, "2");
-        assertEq(l1Block.sequenceNumber(), sequenceNumber, "3");
-        assertEq(l1Block.timestamp(), timestamp, "4");
-        assertEq(l1Block.number(), number, "5");
-        assertEq(l1Block.basefee(), baseFee, "6");
-        assertEq(l1Block.blobBaseFee(), blobBaseFee, "7");
-        assertEq(l1Block.hash(), hash, "8");
-        assertEq(l1Block.batcherHash(), batcherHash, "9");
-    }
-}
-
 contract L1BlockEcotone_Test is L1BlockTest {
     /// @dev Tests that setL1BlockValuesEcotone updates the values appropriately.
     function testFuzz_setL1BlockValuesEcotone_succeeds(
@@ -222,35 +168,6 @@ contract L1BlockEcotone_Test is L1BlockTest {
     }
 }
 
-contract L1BlockDepositsComplete_Test is L1BlockTest {
-    // @dev Tests that `depositsComplete` reverts if the caller is not the depositor.
-    function test_deposits_is_depositor_reverts(address _caller) external {
-        vm.assume(_caller != depositor);
-        vm.expectRevert(NotDepositor.selector);
-        l1Block.depositsComplete();
-    }
-
-    // @dev Tests that `depositsComplete` succeeds if the caller is the depositor.
-    function test_depositsComplete_succeeds() external {
-        // Set the `isDeposit` flag to true
-        vm.prank(depositor);
-        l1Block.setL1BlockValuesIsthmus();
-
-        // Assert that the `isDeposit` flag was properly set to true
-        vm.prank(Predeploys.CROSS_L2_INBOX);
-        assertTrue(l1Block.isDeposit());
-
-        // Call `depositsComplete`
-        vm.prank(depositor);
-        l1Block.depositsComplete();
-
-        // Assert that the `isDeposit` flag was properly set to false
-        /// @dev Assuming that `isDeposit()` wil return the proper value. That function is tested as well
-        vm.prank(Predeploys.CROSS_L2_INBOX);
-        assertEq(l1Block.isDeposit(), false);
-    }
-}
-
 contract L1BlockCustomGasToken_Test is L1BlockTest {
     function testFuzz_setGasPayingToken_succeeds(
         address _token,
@@ -288,27 +205,3 @@ contract L1BlockCustomGasToken_Test is L1BlockTest {
         l1Block.setGasPayingToken(address(this), 18, "Test", "TST");
     }
 }
-
-contract L1BlockIsDeposit_Test is L1BlockTest {
-    /// @dev Tests that `isDeposit` reverts if the caller is not the cross L2 inbox.
-    function test_isDeposit_notCrossL2Inbox_reverts(address _caller) external {
-        vm.assume(_caller != Predeploys.CROSS_L2_INBOX);
-        vm.expectRevert(NotCrossL2Inbox.selector);
-        l1Block.isDeposit();
-    }
-
-    /// @dev Tests that `isDeposit` always returns the correct value.
-    function test_isDeposit_succeeds() external {
-        // Assert is false if the value is not updated
-        vm.prank(Predeploys.CROSS_L2_INBOX);
-        assertEq(l1Block.isDeposit(), false);
-
-        /// @dev Assuming that `setL1BlockValuesIsthmus` will set the proper value. That function is tested as well
-        vm.prank(depositor);
-        l1Block.setL1BlockValuesIsthmus();
-
-        // Assert is true if the value is updated
-        vm.prank(Predeploys.CROSS_L2_INBOX);
-        assertEq(l1Block.isDeposit(), true);
-    }
-}