diff --git a/packages/contracts-bedrock/src/L2/L1Block.sol b/packages/contracts-bedrock/src/L2/L1Block.sol index fac5c0893a6..2476e686277 100644 --- a/packages/contracts-bedrock/src/L2/L1Block.sol +++ b/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 }); - } } diff --git a/packages/contracts-bedrock/src/L2/L1BlockInterop.sol b/packages/contracts-bedrock/src/L2/L1BlockInterop.sol index 8dbf986fb7e..8777ccbcded 100644 --- a/packages/contracts-bedrock/src/L2/L1BlockInterop.sol +++ b/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. diff --git a/packages/contracts-bedrock/test/L2/L1Block.t.sol b/packages/contracts-bedrock/test/L2/L1Block.t.sol index ffa5e0036b5..1c004a6d429 100644 --- a/packages/contracts-bedrock/test/L2/L1Block.t.sol +++ b/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); - } -} diff --git a/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol b/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol index 159021e77dc..7e96670de1f 100644 --- a/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol +++ b/packages/contracts-bedrock/test/L2/L1BlockInterop.t.sol @@ -9,6 +9,7 @@ import { StaticConfig } from "src/libraries/StaticConfig.sol"; // Target contract dependencies import { L1BlockInterop, ConfigType } from "src/L2/L1BlockInterop.sol"; +import { Predeploys } from "src/libraries/Predeploys.sol"; import "src/libraries/L1BlockErrors.sol"; contract L1BlockInteropTest is CommonTest { @@ -17,7 +18,7 @@ contract L1BlockInteropTest is CommonTest { event DependencyRemoved(uint256 indexed chainId); modifier prankDepositor() { - vm.startPrank(l1Block.DEPOSITOR_ACCOUNT()); + vm.startPrank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); _; vm.stopPrank(); } @@ -202,3 +203,110 @@ contract L1BlockInteropTest is CommonTest { return L1BlockInterop(address(l1Block)); } } + +contract L1BlockInteropIsDeposit_Test is L1BlockInteropTest { + /// @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); + _l1BlockInterop().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(_l1BlockInterop().isDeposit(), false); + + /// @dev Assuming that `setL1BlockValuesIsthmus` will set the proper value. That function is tested as well + vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + _l1BlockInterop().setL1BlockValuesIsthmus(); + + // Assert is true if the value is updated + vm.prank(Predeploys.CROSS_L2_INBOX); + assertEq(_l1BlockInterop().isDeposit(), true); + } +} + +contract L1BlockInteropSetL1BlockValuesIsthmus_Test is L1BlockInteropTest { + /// @dev Tests that `setL1BlockValuesIsthmus` reverts if sender address is not the depositor + function test_setL1BlockValuesIsthmus_notDepositor_reverts(address _caller) external { + vm.assume(_caller != _l1BlockInterop().DEPOSITOR_ACCOUNT()); + vm.prank(_caller); + vm.expectRevert(NotDepositor.selector); + _l1BlockInterop().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(_l1BlockInterop().isDeposit(), false); + + bytes memory setValuesEcotoneCalldata = abi.encodePacked( + baseFeeScalar, blobBaseFeeScalar, sequenceNumber, timestamp, number, baseFee, blobBaseFee, hash, batcherHash + ); + + vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + (bool success,) = address(l1Block).call( + abi.encodePacked(L1BlockInterop.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(_l1BlockInterop().isDeposit(), true); + + // Assert `setL1BlockValuesEcotone` was properly called, forwarding the calldata to it + assertEq(_l1BlockInterop().baseFeeScalar(), baseFeeScalar, "base fee scalar not properly set"); + assertEq(_l1BlockInterop().blobBaseFeeScalar(), blobBaseFeeScalar, "blob base fee scalar not properly set"); + assertEq(_l1BlockInterop().sequenceNumber(), sequenceNumber, "sequence number not properly set"); + assertEq(_l1BlockInterop().timestamp(), timestamp, "timestamp not properly set"); + assertEq(_l1BlockInterop().number(), number, "number not properly set"); + assertEq(_l1BlockInterop().basefee(), baseFee, "base fee not properly set"); + assertEq(_l1BlockInterop().blobBaseFee(), blobBaseFee, "blob base fee not properly set"); + assertEq(_l1BlockInterop().hash(), hash, "hash not properly set"); + assertEq(_l1BlockInterop().batcherHash(), batcherHash, "batcher hash not properly set"); + } +} + +contract L1BlockDepositsComplete_Test is L1BlockInteropTest { + // @dev Tests that `depositsComplete` reverts if the caller is not the depositor. + function test_deposits_is_depositor_reverts(address _caller) external { + vm.assume(_caller != _l1BlockInterop().DEPOSITOR_ACCOUNT()); + vm.expectRevert(NotDepositor.selector); + _l1BlockInterop().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(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + _l1BlockInterop().setL1BlockValuesIsthmus(); + + // Assert that the `isDeposit` flag was properly set to true + vm.prank(Predeploys.CROSS_L2_INBOX); + assertTrue(_l1BlockInterop().isDeposit()); + + // Call `depositsComplete` + vm.prank(_l1BlockInterop().DEPOSITOR_ACCOUNT()); + _l1BlockInterop().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(_l1BlockInterop().isDeposit(), false); + } +}