diff --git a/src/bridge/AbsBridge.sol b/src/bridge/AbsBridge.sol index bafd56f95..40f2b45cc 100644 --- a/src/bridge/AbsBridge.sol +++ b/src/bridge/AbsBridge.sol @@ -6,6 +6,7 @@ pragma solidity ^0.8.4; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol"; +import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; import { NotContract, @@ -22,22 +23,9 @@ import "../libraries/DelegateCallAware.sol"; import {L1MessageType_batchPostingReport} from "../libraries/MessageTypes.sol"; -/** - * @title Staging ground for incoming and outgoing messages - * @notice Holds the inbox accumulator for sequenced and delayed messages. - * Since the escrow is held here, this contract also contains a list of allowed - * outboxes that can make calls from here and withdraw this escrow. - */ -abstract contract AbsBridge is Initializable, DelegateCallAware, IBridge { - using AddressUpgradeable for address; - - struct InOutInfo { - uint256 index; - bool allowed; - } - - mapping(address => InOutInfo) private allowedDelayedInboxesMap; - mapping(address => InOutInfo) private allowedOutboxesMap; +abstract contract AbsBridgeStorage is IBridge { + mapping(address => InOutInfo) internal allowedDelayedInboxesMap; + mapping(address => InOutInfo) internal allowedOutboxesMap; address[] public allowedDelayedInboxList; address[] public allowedOutboxList; @@ -55,6 +43,32 @@ abstract contract AbsBridge is Initializable, DelegateCallAware, IBridge { uint256 public override sequencerReportedSubMessageCount; + /** + * @dev This empty reserved space is put in place to allow future versions to add new + * variables without shifting down storage in the inheritance chain. + * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps + */ + uint256[40] private __gap; + + /// @inheritdoc IBridge + address public nativeToken; +} + +/** + * @title Staging ground for incoming and outgoing messages + * @notice Holds the inbox accumulator for sequenced and delayed messages. + * Since the escrow is held here, this contract also contains a list of allowed + * outboxes that can make calls from here and withdraw this escrow. + */ +abstract contract AbsBridge is + Initializable, + DelegateCallAware, + IBridge, + AbsBridgeStorage, + AccessControlUpgradeable +{ + using AddressUpgradeable for address; + address internal constant EMPTY_ACTIVEOUTBOX = address(type(uint160).max); modifier onlyRollupOrOwner() { @@ -298,11 +312,4 @@ abstract contract AbsBridge is Initializable, DelegateCallAware, IBridge { /// @dev get base fee which is emitted in `MessageDelivered` event and then picked up and /// used in ArbOs to calculate the submission fee for retryable ticket function _baseFeeToReport() internal view virtual returns (uint256); - - /** - * @dev This empty reserved space is put in place to allow future versions to add new - * variables without shifting down storage in the inheritance chain. - * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps - */ - uint256[40] private __gap; } diff --git a/src/bridge/ERC20Bridge.sol b/src/bridge/ERC20Bridge.sol index c67e1ba06..cb62d5bb5 100644 --- a/src/bridge/ERC20Bridge.sol +++ b/src/bridge/ERC20Bridge.sol @@ -24,9 +24,6 @@ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; contract ERC20Bridge is AbsBridge, IERC20Bridge { using SafeERC20 for IERC20; - /// @inheritdoc IERC20Bridge - address public nativeToken; - /// @inheritdoc IERC20Bridge function initialize(IOwnable rollup_, address nativeToken_) external initializer onlyDelegated { if (nativeToken_ == address(0)) revert InvalidTokenSet(nativeToken_); diff --git a/src/bridge/IBridge.sol b/src/bridge/IBridge.sol index 22388b4bd..0c91d6c5b 100644 --- a/src/bridge/IBridge.sol +++ b/src/bridge/IBridge.sol @@ -8,6 +8,11 @@ pragma solidity >=0.6.9 <0.9.0; import "./IOwnable.sol"; interface IBridge { + struct InOutInfo { + uint256 index; + bool allowed; + } + event MessageDelivered( uint256 indexed messageIndex, bytes32 indexed beforeInboxAcc, @@ -34,6 +39,17 @@ interface IBridge { event RollupUpdated(address rollup); + /** + * @dev Token that is escrowed in bridge on L1 side and minted on L2 as native currency. + * When set to address(0), the bridge use the L1 native token (e.g. ETH) + * Fees are paid in this token. There are certain restrictions on the native token: + * - The token can't be rebasing or have a transfer fee + * - The token must only be transferrable via a call to the token address itself + * - The token must only be able to set allowance via a call to the token address itself + * - The token must not have a callback on transfer, and more generally a user must not be able to make a transfer to themselves revert + */ + function nativeToken() external view returns (address); + function allowedDelayedInboxList(uint256) external returns (address); function allowedOutboxList(uint256) external returns (address); diff --git a/src/bridge/IERC20Bridge.sol b/src/bridge/IERC20Bridge.sol index b2b4551db..878598b7c 100644 --- a/src/bridge/IERC20Bridge.sol +++ b/src/bridge/IERC20Bridge.sol @@ -9,16 +9,6 @@ import "./IOwnable.sol"; import "./IBridge.sol"; interface IERC20Bridge is IBridge { - /** - * @dev token that is escrowed in bridge on L1 side and minted on L2 as native currency. - * Fees are paid in this token. There are certain restrictions on the native token: - * - The token can't be rebasing or have a transfer fee - * - The token must only be transferrable via a call to the token address itself - * - The token must only be able to set allowance via a call to the token address itself - * - The token must not have a callback on transfer, and more generally a user must not be able to make a transfer to themselves revert - */ - function nativeToken() external view returns (address); - /** * @dev Enqueue a message in the delayed inbox accumulator. * These messages are later sequenced in the SequencerInbox, either diff --git a/src/mocks/BridgeStub.sol b/src/mocks/BridgeStub.sol index 2e2dee05e..05361b449 100644 --- a/src/mocks/BridgeStub.sol +++ b/src/mocks/BridgeStub.sol @@ -11,11 +11,6 @@ import "../bridge/IBridge.sol"; import "../bridge/IEthBridge.sol"; contract BridgeStub is IBridge, IEthBridge { - struct InOutInfo { - uint256 index; - bool allowed; - } - mapping(address => InOutInfo) private allowedDelayedInboxesMap; //mapping(address => InOutInfo) private allowedOutboxesMap; @@ -32,6 +27,8 @@ contract BridgeStub is IBridge, IEthBridge { address public sequencerInbox; uint256 public override sequencerReportedSubMessageCount; + address public nativeToken; + function setSequencerInbox(address _sequencerInbox) external override { sequencerInbox = _sequencerInbox; emit SequencerInboxUpdated(_sequencerInbox); diff --git a/src/test-helpers/BridgeTester.sol b/src/test-helpers/BridgeTester.sol index 0bcbe00fc..2de8f0afa 100644 --- a/src/test-helpers/BridgeTester.sol +++ b/src/test-helpers/BridgeTester.sol @@ -30,11 +30,6 @@ import "../libraries/DelegateCallAware.sol"; contract BridgeTester is Initializable, DelegateCallAware, IBridge, IEthBridge { using AddressUpgradeable for address; - struct InOutInfo { - uint256 index; - bool allowed; - } - mapping(address => InOutInfo) private allowedInboxesMap; mapping(address => InOutInfo) private allowedOutboxesMap; @@ -46,6 +41,8 @@ contract BridgeTester is Initializable, DelegateCallAware, IBridge, IEthBridge { IOwnable public rollup; address public sequencerInbox; + address public nativeToken; + modifier onlyRollupOrOwner() { if (msg.sender != address(rollup)) { address rollupOwner = rollup.owner(); diff --git a/test/storage/Bridge b/test/storage/Bridge index 9ed7f48a2..37dc4bf9a 100644 --- a/test/storage/Bridge +++ b/test/storage/Bridge @@ -1,15 +1,20 @@ -| Name | Type | Slot | Offset | Bytes | Contract | -|----------------------------------|------------------------------------------------|------|--------|-------|------------------------------| -| _initialized | bool | 0 | 0 | 1 | src/bridge/Bridge.sol:Bridge | -| _initializing | bool | 0 | 1 | 1 | src/bridge/Bridge.sol:Bridge | -| allowedDelayedInboxesMap | mapping(address => struct AbsBridge.InOutInfo) | 1 | 0 | 32 | src/bridge/Bridge.sol:Bridge | -| allowedOutboxesMap | mapping(address => struct AbsBridge.InOutInfo) | 2 | 0 | 32 | src/bridge/Bridge.sol:Bridge | -| allowedDelayedInboxList | address[] | 3 | 0 | 32 | src/bridge/Bridge.sol:Bridge | -| allowedOutboxList | address[] | 4 | 0 | 32 | src/bridge/Bridge.sol:Bridge | -| _activeOutbox | address | 5 | 0 | 20 | src/bridge/Bridge.sol:Bridge | -| delayedInboxAccs | bytes32[] | 6 | 0 | 32 | src/bridge/Bridge.sol:Bridge | -| sequencerInboxAccs | bytes32[] | 7 | 0 | 32 | src/bridge/Bridge.sol:Bridge | -| rollup | contract IOwnable | 8 | 0 | 20 | src/bridge/Bridge.sol:Bridge | -| sequencerInbox | address | 9 | 0 | 20 | src/bridge/Bridge.sol:Bridge | -| sequencerReportedSubMessageCount | uint256 | 10 | 0 | 32 | src/bridge/Bridge.sol:Bridge | -| __gap | uint256[40] | 11 | 0 | 1280 | src/bridge/Bridge.sol:Bridge | +| Name | Type | Slot | Offset | Bytes | Contract | +|----------------------------------|--------------------------------------------------------------|------|--------|-------|------------------------------| +| _initialized | bool | 0 | 0 | 1 | src/bridge/Bridge.sol:Bridge | +| _initializing | bool | 0 | 1 | 1 | src/bridge/Bridge.sol:Bridge | +| allowedDelayedInboxesMap | mapping(address => struct IBridge.InOutInfo) | 1 | 0 | 32 | src/bridge/Bridge.sol:Bridge | +| allowedOutboxesMap | mapping(address => struct IBridge.InOutInfo) | 2 | 0 | 32 | src/bridge/Bridge.sol:Bridge | +| allowedDelayedInboxList | address[] | 3 | 0 | 32 | src/bridge/Bridge.sol:Bridge | +| allowedOutboxList | address[] | 4 | 0 | 32 | src/bridge/Bridge.sol:Bridge | +| _activeOutbox | address | 5 | 0 | 20 | src/bridge/Bridge.sol:Bridge | +| delayedInboxAccs | bytes32[] | 6 | 0 | 32 | src/bridge/Bridge.sol:Bridge | +| sequencerInboxAccs | bytes32[] | 7 | 0 | 32 | src/bridge/Bridge.sol:Bridge | +| rollup | contract IOwnable | 8 | 0 | 20 | src/bridge/Bridge.sol:Bridge | +| sequencerInbox | address | 9 | 0 | 20 | src/bridge/Bridge.sol:Bridge | +| sequencerReportedSubMessageCount | uint256 | 10 | 0 | 32 | src/bridge/Bridge.sol:Bridge | +| __gap | uint256[40] | 11 | 0 | 1280 | src/bridge/Bridge.sol:Bridge | +| nativeToken | address | 51 | 0 | 20 | src/bridge/Bridge.sol:Bridge | +| __gap | uint256[50] | 52 | 0 | 1600 | src/bridge/Bridge.sol:Bridge | +| __gap | uint256[50] | 102 | 0 | 1600 | src/bridge/Bridge.sol:Bridge | +| _roles | mapping(bytes32 => struct AccessControlUpgradeable.RoleData) | 152 | 0 | 32 | src/bridge/Bridge.sol:Bridge | +| __gap | uint256[49] | 153 | 0 | 1568 | src/bridge/Bridge.sol:Bridge | diff --git a/test/storage/ERC20Bridge b/test/storage/ERC20Bridge index 1b0d838b4..1effcfbbf 100644 --- a/test/storage/ERC20Bridge +++ b/test/storage/ERC20Bridge @@ -1,16 +1,20 @@ -| Name | Type | Slot | Offset | Bytes | Contract | -|----------------------------------|------------------------------------------------|------|--------|-------|----------------------------------------| -| _initialized | bool | 0 | 0 | 1 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| _initializing | bool | 0 | 1 | 1 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| allowedDelayedInboxesMap | mapping(address => struct AbsBridge.InOutInfo) | 1 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| allowedOutboxesMap | mapping(address => struct AbsBridge.InOutInfo) | 2 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| allowedDelayedInboxList | address[] | 3 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| allowedOutboxList | address[] | 4 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| _activeOutbox | address | 5 | 0 | 20 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| delayedInboxAccs | bytes32[] | 6 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| sequencerInboxAccs | bytes32[] | 7 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| rollup | contract IOwnable | 8 | 0 | 20 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| sequencerInbox | address | 9 | 0 | 20 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| sequencerReportedSubMessageCount | uint256 | 10 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| __gap | uint256[40] | 11 | 0 | 1280 | src/bridge/ERC20Bridge.sol:ERC20Bridge | -| nativeToken | address | 51 | 0 | 20 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| Name | Type | Slot | Offset | Bytes | Contract | +|----------------------------------|--------------------------------------------------------------|------|--------|-------|----------------------------------------| +| _initialized | bool | 0 | 0 | 1 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| _initializing | bool | 0 | 1 | 1 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| allowedDelayedInboxesMap | mapping(address => struct IBridge.InOutInfo) | 1 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| allowedOutboxesMap | mapping(address => struct IBridge.InOutInfo) | 2 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| allowedDelayedInboxList | address[] | 3 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| allowedOutboxList | address[] | 4 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| _activeOutbox | address | 5 | 0 | 20 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| delayedInboxAccs | bytes32[] | 6 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| sequencerInboxAccs | bytes32[] | 7 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| rollup | contract IOwnable | 8 | 0 | 20 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| sequencerInbox | address | 9 | 0 | 20 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| sequencerReportedSubMessageCount | uint256 | 10 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| __gap | uint256[40] | 11 | 0 | 1280 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| nativeToken | address | 51 | 0 | 20 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| __gap | uint256[50] | 52 | 0 | 1600 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| __gap | uint256[50] | 102 | 0 | 1600 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| _roles | mapping(bytes32 => struct AccessControlUpgradeable.RoleData) | 152 | 0 | 32 | src/bridge/ERC20Bridge.sol:ERC20Bridge | +| __gap | uint256[49] | 153 | 0 | 1568 | src/bridge/ERC20Bridge.sol:ERC20Bridge |