From 667e0780e3243475ce700497014a05771c3e851f Mon Sep 17 00:00:00 2001 From: zimpha Date: Thu, 12 Oct 2023 14:34:05 +0800 Subject: [PATCH 1/9] draft version of lido gateway --- contracts/src/lido/L1LidoGateway.sol | 109 ++++++++++++ contracts/src/lido/L2LidoGateway.sol | 131 ++++++++++++++ contracts/src/lido/LidoBridgeableTokens.sol | 73 ++++++++ contracts/src/lido/LidoGatewayManager.sol | 178 ++++++++++++++++++++ contracts/src/lido/README.md | 1 + 5 files changed, 492 insertions(+) create mode 100644 contracts/src/lido/L1LidoGateway.sol create mode 100644 contracts/src/lido/L2LidoGateway.sol create mode 100644 contracts/src/lido/LidoBridgeableTokens.sol create mode 100644 contracts/src/lido/LidoGatewayManager.sol create mode 100644 contracts/src/lido/README.md diff --git a/contracts/src/lido/L1LidoGateway.sol b/contracts/src/lido/L1LidoGateway.sol new file mode 100644 index 0000000000..d8bb3744f3 --- /dev/null +++ b/contracts/src/lido/L1LidoGateway.sol @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {IL1ERC20Gateway} from "../L1/gateways/IL1ERC20Gateway.sol"; +import {IL1ScrollMessenger} from "../L1/IL1ScrollMessenger.sol"; +import {IL2ERC20Gateway} from "../L2/gateways/IL2ERC20Gateway.sol"; + +import {L1ERC20Gateway} from "../L1/gateways/L1ERC20Gateway.sol"; +import {LidoBridgeableTokens} from "./LidoBridgeableTokens.sol"; +import {LidoGatewayManager} from "./LidoGatewayManager.sol"; + +contract L1LidoGateway is L1ERC20Gateway, LidoBridgeableTokens, LidoGatewayManager { + /********** + * Errors * + **********/ + + /// @dev Thrown when deposit zero amount token. + error ErrorDepositZeroAmount(); + + /************* + * Variables * + *************/ + + /// @dev The initial version of `L1LidoGateway` use `L1CustomERC20Gateway`. We keep the storage + /// slot for `tokenMapping` for compatibility. It should no longer be used. + mapping(address => address) private __tokenMapping; + + /*************** + * Constructor * + ***************/ + + /// @param _l1Token The address of the bridged token in the L1 chain + /// @param _l2Token The address of the token minted on the L2 chain when token bridged + constructor(address _l1Token, address _l2Token) LidoBridgeableTokens(_l1Token, _l2Token) { + _disableInitializers(); + } + + /// @notice Initialize the storage of L1LidoGateway. + function initializeV2() external reinitializer(2) { + __LidoGatewayManager_init(); + } + + /************************* + * Public View Functions * + *************************/ + + /// @inheritdoc IL1ERC20Gateway + function getL2ERC20Address(address _l1Token) + external + view + override + onlySupportedL1Token(_l1Token) + returns (address) + { + return l2Token; + } + + /********************** + * Internal Functions * + **********************/ + + /// @inheritdoc L1ERC20Gateway + function _beforeFinalizeWithdrawERC20( + address _l1Token, + address _l2Token, + address, + address, + uint256, + bytes calldata + ) internal virtual override onlySupportedL1Token(_l1Token) onlySupportedL2Token(_l2Token) whenWithdrawalsEnabled { + if (msg.value != 0) revert ErrorNonZeroMsgValue(); + } + + /// @inheritdoc L1ERC20Gateway + function _beforeDropMessage( + address _token, + address, + uint256 + ) internal virtual override onlySupportedL1Token(_token) { + if (msg.value != 0) revert ErrorNonZeroMsgValue(); + } + + /// @inheritdoc L1ERC20Gateway + function _deposit( + address _token, + address _to, + uint256 _amount, + bytes memory _data, + uint256 _gasLimit + ) internal virtual override nonReentrant onlySupportedL1Token(_token) onlyNonZeroAccount(_to) whenDepositsEnabled { + if (_amount == 0) revert ErrorDepositZeroAmount(); + + // 1. Transfer token into this contract. + address _from; + (_from, _amount, _data) = _transferERC20In(_token, _amount, _data); + + // 2. Generate message passed to L2LidoGateway. + bytes memory _message = abi.encodeCall( + IL2ERC20Gateway.finalizeDepositERC20, + (_token, l2Token, _from, _to, _amount, _data) + ); + + // 3. Send message to L1ScrollMessenger. + IL1ScrollMessenger(messenger).sendMessage{value: msg.value}(counterpart, 0, _message, _gasLimit, _from); + + emit DepositERC20(_token, l2Token, _from, _to, _amount, _data); + } +} diff --git a/contracts/src/lido/L2LidoGateway.sol b/contracts/src/lido/L2LidoGateway.sol new file mode 100644 index 0000000000..b3a6214900 --- /dev/null +++ b/contracts/src/lido/L2LidoGateway.sol @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {IL1ERC20Gateway} from "../L1/gateways/IL1ERC20Gateway.sol"; +import {IL2ScrollMessenger} from "../L2/IL2ScrollMessenger.sol"; +import {IL2ERC20Gateway} from "../L2/gateways/IL2ERC20Gateway.sol"; +import {IScrollERC20Upgradeable} from "../libraries/token/IScrollERC20Upgradeable.sol"; + +import {L2ERC20Gateway} from "../L2/gateways/L2ERC20Gateway.sol"; +import {LidoBridgeableTokens} from "./LidoBridgeableTokens.sol"; +import {LidoGatewayManager} from "./LidoGatewayManager.sol"; + +contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManager { + /********** + * Errors * + **********/ + + /// @dev Thrown when withdraw zero amount token. + error ErrorWithdrawZeroAmount(); + + /************* + * Variables * + *************/ + + /// @dev The initial version of `L2LidoGateway` use `L2CustomERC20Gateway`. We keep the storage + /// slot for `tokenMapping` for compatibility. It should no longer be used. + mapping(address => address) private __tokenMapping; + + /*************** + * Constructor * + ***************/ + + /// @param _l1Token The address of the bridged token in the L1 chain + /// @param _l2Token The address of the token minted on the L2 chain when token bridged + constructor(address _l1Token, address _l2Token) LidoBridgeableTokens(_l1Token, _l2Token) { + _disableInitializers(); + } + + /************************* + * Public View Functions * + *************************/ + + /// @inheritdoc IL2ERC20Gateway + function getL1ERC20Address(address _l2Token) + external + view + override + onlySupportedL2Token(_l2Token) + returns (address) + { + return l1Token; + } + + /// @inheritdoc IL2ERC20Gateway + function getL2ERC20Address(address _l1Token) + external + view + override + onlySupportedL1Token(_l1Token) + returns (address) + { + return l2Token; + } + + /***************************** + * Public Mutating Functions * + *****************************/ + + /// @inheritdoc IL2ERC20Gateway + function finalizeDepositERC20( + address _l1Token, + address _l2Token, + address _from, + address _to, + uint256 _amount, + bytes calldata _data + ) + external + payable + override + onlyCallByCounterpart + nonReentrant + onlySupportedL1Token(_l1Token) + onlySupportedL2Token(_l2Token) + whenDepositsEnabled + { + if (msg.value != 0) revert ErrorNonZeroMsgValue(); + + IScrollERC20Upgradeable(_l2Token).mint(_to, _amount); + + _doCallback(_to, _data); + + emit FinalizeDepositERC20(_l1Token, _l2Token, _from, _to, _amount, _data); + } + + /********************** + * Internal Functions * + **********************/ + + /// @inheritdoc L2ERC20Gateway + function _withdraw( + address _l2Token, + address _to, + uint256 _amount, + bytes memory _data, + uint256 _gasLimit + ) internal virtual override nonReentrant onlySupportedL2Token(_l2Token) whenWithdrawalsEnabled { + if (_amount == 0) revert ErrorWithdrawZeroAmount(); + + // 1. Extract real sender if this call is from L2GatewayRouter. + address _from = _msgSender(); + if (router == _from) { + (_from, _data) = abi.decode(_data, (address, bytes)); + } + + // 2. Burn token. + IScrollERC20Upgradeable(_l2Token).burn(_from, _amount); + + // 3. Generate message passed to L1StandardERC20Gateway. + bytes memory _message = abi.encodeCall( + IL1ERC20Gateway.finalizeWithdrawERC20, + (l1Token, _l2Token, _from, _to, _amount, _data) + ); + + // 4. send message to L2ScrollMessenger + IL2ScrollMessenger(messenger).sendMessage{value: msg.value}(counterpart, 0, _message, _gasLimit); + + emit WithdrawERC20(l1Token, _l2Token, _from, _to, _amount, _data); + } +} diff --git a/contracts/src/lido/LidoBridgeableTokens.sol b/contracts/src/lido/LidoBridgeableTokens.sol new file mode 100644 index 0000000000..94079b8761 --- /dev/null +++ b/contracts/src/lido/LidoBridgeableTokens.sol @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +abstract contract LidoBridgeableTokens { + /************* + * Constants * + *************/ + + /// @notice The address of bridged token in L1 chain. + address public immutable l1Token; + + /// @notice The address of the token minted on the L2 chain when token bridged. + address public immutable l2Token; + + /********** + * Errors * + **********/ + + /// @dev Thrown the given `l1Token` is not supported. + error ErrorUnsupportedL1Token(); + + /// @dev Thrown the given `l2Token` is not supported. + error ErrorUnsupportedL2Token(); + + /// @dev Thrown the given account is zero address. + error ErrorAccountIsZeroAddress(); + + /// @dev Thrown the `msg.value` is not zero. + error ErrorNonZeroMsgValue(); + + /// @dev Thrown the `msg.value` is not match with the expected one. + error ErrorMsgValueMismatch(); + + /********************** + * Function Modifiers * + **********************/ + + /// @dev Validates that passed `_l1Token` is supported by the bridge + modifier onlySupportedL1Token(address _l1Token) { + if (_l1Token != l1Token) { + revert ErrorUnsupportedL1Token(); + } + _; + } + + /// @dev Validates that passed `_l2Token` is supported by the bridge + modifier onlySupportedL2Token(address _l2Token) { + if (_l2Token != l2Token) { + revert ErrorUnsupportedL2Token(); + } + _; + } + + /// @dev validates that `_account` is not zero address + modifier onlyNonZeroAccount(address _account) { + if (_account == address(0)) { + revert ErrorAccountIsZeroAddress(); + } + _; + } + + /*************** + * Constructor * + ***************/ + + /// @param _l1Token The address of the bridged token in the L1 chain + /// @param _l2Token The address of the token minted on the L2 chain when token bridged + constructor(address _l1Token, address _l2Token) { + l1Token = _l1Token; + l2Token = _l2Token; + } +} diff --git a/contracts/src/lido/LidoGatewayManager.sol b/contracts/src/lido/LidoGatewayManager.sol new file mode 100644 index 0000000000..9a99a0f759 --- /dev/null +++ b/contracts/src/lido/LidoGatewayManager.sol @@ -0,0 +1,178 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {ScrollGatewayBase} from "../libraries/gateway/ScrollGatewayBase.sol"; + +// solhint-disable func-name-mixedcase + +abstract contract LidoGatewayManager is ScrollGatewayBase { + /********** + * Events * + **********/ + + /// @notice Emitted then caller enable deposits. + /// @param enabler The address of caller. + event DepositsEnabled(address indexed enabler); + + /// @notice Emitted then caller disable deposits. + /// @param disabler The address of caller. + event DepositsDisabled(address indexed disabler); + + /// @notice Emitted then caller enable withdrawals. + /// @param enabler The address of caller. + event WithdrawalsEnabled(address indexed enabler); + + /// @notice Emitted then caller disable withdrawals. + /// @param disabler The address of caller. + event WithdrawalsDisabled(address indexed disabler); + + /********** + * Errors * + **********/ + + /// @dev Thrown when deposits are enabled while caller try to enable it again. + error ErrorDepositsEnabled(); + + /// @dev Thrown when deposits are disable while caller try to deposits related operation. + error ErrorDepositsDisabled(); + + /// @dev Thrown when withdrawals are enabled while caller try to enable it again. + error ErrorWithdrawalsEnabled(); + + /// @dev Thrown when withdrawals are disable while caller try to withdrawals related operation. + error ErrorWithdrawalsDisabled(); + + /// @dev Thrown when caller is not deposits enabler. + error ErrorCallerIsNotDepositsEnabler(); + + /// @dev Thrown when caller is not deposits disabler. + error ErrorCallerIsNotDepositsDisabler(); + + /// @dev Thrown when caller is not withdrawals enabler. + error ErrorCallerIsNotWithdrawalsEnabler(); + + /// @dev Thrown when caller is not withdrawals disabler. + error ErrorCallerIsNotWithdrawalsDisabler(); + + /*********** + * Structs * + ***********/ + + /// @dev Stores the state of the bridging + /// @param isDepositsEnabled Stores the state of the deposits + /// @param isWithdrawalsEnabled Stores the state of the withdrawals + /// @param depositsEnabler The address of user who can enable deposits + /// @param depositsEnabler The address of user who can disable deposits + /// @param withdrawalsEnabler The address of user who can enable withdrawals + /// @param withdrawalsDisabler The address of user who can disable withdrawals + struct State { + bool isDepositsEnabled; + bool isWithdrawalsEnabled; + address depositsEnabler; + address depositsDisabler; + address withdrawalsEnabler; + address withdrawalsDisabler; + } + + /************* + * Constants * + *************/ + + /// @dev The location of the slot with State + bytes32 private constant STATE_SLOT = keccak256("LidoGatewayManager.bridgingState"); + + /********************** + * Function Modifiers * + **********************/ + + /// @dev Validates that deposits are enabled + modifier whenDepositsEnabled() { + if (!isDepositsEnabled()) revert ErrorDepositsDisabled(); + _; + } + + /// @dev Validates that withdrawals are enabled + modifier whenWithdrawalsEnabled() { + if (!isWithdrawalsEnabled()) revert ErrorWithdrawalsDisabled(); + _; + } + + /*************** + * Constructor * + ***************/ + + /// @notice Initialize the storage of LidoGatewayManager. + function __LidoGatewayManager_init() internal onlyInitializing { + _loadState().isDepositsEnabled = true; + emit DepositsEnabled(_msgSender()); + + _loadState().isWithdrawalsEnabled = true; + emit WithdrawalsEnabled(_msgSender()); + } + + /************************* + * Public View Functions * + *************************/ + + /// @notice Returns whether the deposits are enabled or not + function isDepositsEnabled() public view returns (bool) { + return _loadState().isDepositsEnabled; + } + + /// @notice Returns whether the withdrawals are enabled or not + function isWithdrawalsEnabled() public view returns (bool) { + return _loadState().isWithdrawalsEnabled; + } + + /************************ + * Restricted Functions * + ************************/ + + /// @notice Enables the deposits if they are disabled + function enableDeposits() external { + if (isDepositsEnabled()) revert ErrorDepositsEnabled(); + if (_msgSender() != _loadState().depositsEnabler) revert ErrorCallerIsNotDepositsEnabler(); + + _loadState().isDepositsEnabled = true; + emit DepositsEnabled(_msgSender()); + } + + /// @notice Disables the deposits if they aren't disabled yet + function disableDeposits() external whenDepositsEnabled { + if (_msgSender() != _loadState().depositsDisabler) revert ErrorCallerIsNotDepositsDisabler(); + + _loadState().isDepositsEnabled = false; + emit DepositsDisabled(_msgSender()); + } + + /// @notice Enables the withdrawals if they are disabled + function enableWithdrawals() external { + if (isWithdrawalsEnabled()) revert ErrorWithdrawalsEnabled(); + if (_msgSender() != _loadState().withdrawalsEnabler) revert ErrorCallerIsNotWithdrawalsEnabler(); + + _loadState().isWithdrawalsEnabled = true; + emit WithdrawalsEnabled(_msgSender()); + } + + /// @notice Disables the withdrawals if they aren't disabled yet + function disableWithdrawals() external whenWithdrawalsEnabled { + if (_msgSender() != _loadState().withdrawalsDisabler) revert ErrorCallerIsNotWithdrawalsDisabler(); + + _loadState().isWithdrawalsEnabled = false; + emit WithdrawalsDisabled(_msgSender()); + } + + /********************** + * Internal Functions * + **********************/ + + /// @dev Returns the reference to the slot with State struct + function _loadState() private pure returns (State storage r) { + bytes32 slot = STATE_SLOT; + // solhint-disable-next-line no-inline-assembly + assembly { + r.slot := slot + } + } +} diff --git a/contracts/src/lido/README.md b/contracts/src/lido/README.md new file mode 100644 index 0000000000..b45e886200 --- /dev/null +++ b/contracts/src/lido/README.md @@ -0,0 +1 @@ +# Lido's Scroll Bridge From 71c78f514e467c2856f467ae749512bfa8bbe655 Mon Sep 17 00:00:00 2001 From: zimpha Date: Thu, 26 Oct 2023 13:23:01 +0800 Subject: [PATCH 2/9] add unit tests for L1LidoGateway and L2LidoGateway --- contracts/.solcover.js | 10 + contracts/src/lido/L1LidoGateway.sol | 16 +- contracts/src/lido/L2LidoGateway.sol | 33 +- contracts/src/lido/LidoBridgeableTokens.sol | 3 - contracts/src/lido/LidoGatewayManager.sol | 60 ++ contracts/src/test/L1LidoGateway.t.sol | 740 ++++++++++++++++++ contracts/src/test/L2LidoGateway.t.sol | 606 ++++++++++++++ .../src/test/mocks/MockL1LidoGateway.sol | 18 + .../src/test/mocks/MockL2LidoGateway.sol | 18 + 9 files changed, 1495 insertions(+), 9 deletions(-) create mode 100644 contracts/.solcover.js create mode 100644 contracts/src/test/L1LidoGateway.t.sol create mode 100644 contracts/src/test/L2LidoGateway.t.sol create mode 100644 contracts/src/test/mocks/MockL1LidoGateway.sol create mode 100644 contracts/src/test/mocks/MockL2LidoGateway.sol diff --git a/contracts/.solcover.js b/contracts/.solcover.js new file mode 100644 index 0000000000..4df81f9485 --- /dev/null +++ b/contracts/.solcover.js @@ -0,0 +1,10 @@ +module.exports = { + skipFiles: [ + 'mocks', + 'test', + 'L2/predeploys/L1BlockContainer.sol', + 'libraries/verifier/ZkTrieVerifier.sol', + 'libraries/verifier/PatriciaMerkleTrieVerifier.sol' + ], + istanbulReporter: ["lcov", "json"] +}; diff --git a/contracts/src/lido/L1LidoGateway.sol b/contracts/src/lido/L1LidoGateway.sol index d8bb3744f3..ade81f38a0 100644 --- a/contracts/src/lido/L1LidoGateway.sol +++ b/contracts/src/lido/L1LidoGateway.sol @@ -3,10 +3,11 @@ pragma solidity =0.8.16; import {IL1ERC20Gateway} from "../L1/gateways/IL1ERC20Gateway.sol"; +import {L1ERC20Gateway} from "../L1/gateways/L1ERC20Gateway.sol"; import {IL1ScrollMessenger} from "../L1/IL1ScrollMessenger.sol"; import {IL2ERC20Gateway} from "../L2/gateways/IL2ERC20Gateway.sol"; +import {ScrollGatewayBase} from "../libraries/gateway/ScrollGatewayBase.sol"; -import {L1ERC20Gateway} from "../L1/gateways/L1ERC20Gateway.sol"; import {LidoBridgeableTokens} from "./LidoBridgeableTokens.sol"; import {LidoGatewayManager} from "./LidoGatewayManager.sol"; @@ -36,7 +37,18 @@ contract L1LidoGateway is L1ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag _disableInitializers(); } - /// @notice Initialize the storage of L1LidoGateway. + /// @notice Initialize the storage of L1LidoGateway v1. + function initialize( + address _counterpart, + address _router, + address _messenger + ) external initializer { + require(_router != address(0), "zero router address"); + + ScrollGatewayBase._initialize(_counterpart, _router, _messenger); + } + + /// @notice Initialize the storage of L1LidoGateway v2. function initializeV2() external reinitializer(2) { __LidoGatewayManager_init(); } diff --git a/contracts/src/lido/L2LidoGateway.sol b/contracts/src/lido/L2LidoGateway.sol index b3a6214900..b64d98df23 100644 --- a/contracts/src/lido/L2LidoGateway.sol +++ b/contracts/src/lido/L2LidoGateway.sol @@ -3,11 +3,12 @@ pragma solidity =0.8.16; import {IL1ERC20Gateway} from "../L1/gateways/IL1ERC20Gateway.sol"; -import {IL2ScrollMessenger} from "../L2/IL2ScrollMessenger.sol"; import {IL2ERC20Gateway} from "../L2/gateways/IL2ERC20Gateway.sol"; +import {L2ERC20Gateway} from "../L2/gateways/L2ERC20Gateway.sol"; +import {IL2ScrollMessenger} from "../L2/IL2ScrollMessenger.sol"; import {IScrollERC20Upgradeable} from "../libraries/token/IScrollERC20Upgradeable.sol"; +import {ScrollGatewayBase} from "../libraries/gateway/ScrollGatewayBase.sol"; -import {L2ERC20Gateway} from "../L2/gateways/L2ERC20Gateway.sol"; import {LidoBridgeableTokens} from "./LidoBridgeableTokens.sol"; import {LidoGatewayManager} from "./LidoGatewayManager.sol"; @@ -37,6 +38,22 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag _disableInitializers(); } + /// @notice Initialize the storage of L2LidoGateway v1. + function initialize( + address _counterpart, + address _router, + address _messenger + ) external initializer { + require(_router != address(0), "zero router address"); + + ScrollGatewayBase._initialize(_counterpart, _router, _messenger); + } + + /// @notice Initialize the storage of L2LidoGateway v2. + function initializeV2() external reinitializer(2) { + __LidoGatewayManager_init(); + } + /************************* * Public View Functions * *************************/ @@ -105,7 +122,15 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag uint256 _amount, bytes memory _data, uint256 _gasLimit - ) internal virtual override nonReentrant onlySupportedL2Token(_l2Token) whenWithdrawalsEnabled { + ) + internal + virtual + override + nonReentrant + onlySupportedL2Token(_l2Token) + onlyNonZeroAccount(_to) + whenWithdrawalsEnabled + { if (_amount == 0) revert ErrorWithdrawZeroAmount(); // 1. Extract real sender if this call is from L2GatewayRouter. @@ -117,7 +142,7 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag // 2. Burn token. IScrollERC20Upgradeable(_l2Token).burn(_from, _amount); - // 3. Generate message passed to L1StandardERC20Gateway. + // 3. Generate message passed to L1LidoGateway. bytes memory _message = abi.encodeCall( IL1ERC20Gateway.finalizeWithdrawERC20, (l1Token, _l2Token, _from, _to, _amount, _data) diff --git a/contracts/src/lido/LidoBridgeableTokens.sol b/contracts/src/lido/LidoBridgeableTokens.sol index 94079b8761..57bade1dc4 100644 --- a/contracts/src/lido/LidoBridgeableTokens.sol +++ b/contracts/src/lido/LidoBridgeableTokens.sol @@ -29,9 +29,6 @@ abstract contract LidoBridgeableTokens { /// @dev Thrown the `msg.value` is not zero. error ErrorNonZeroMsgValue(); - /// @dev Thrown the `msg.value` is not match with the expected one. - error ErrorMsgValueMismatch(); - /********************** * Function Modifiers * **********************/ diff --git a/contracts/src/lido/LidoGatewayManager.sol b/contracts/src/lido/LidoGatewayManager.sol index 9a99a0f759..0216f5f039 100644 --- a/contracts/src/lido/LidoGatewayManager.sol +++ b/contracts/src/lido/LidoGatewayManager.sol @@ -27,6 +27,26 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { /// @param disabler The address of caller. event WithdrawalsDisabled(address indexed disabler); + /// @notice Emitted when the deposits enabler is updated. + /// @param oldEnabler The address of the previous deposits enabler. + /// @param newEnabler The address of the current deposits enabler. + event UpdateDepositsEnabler(address indexed oldEnabler, address indexed newEnabler); + + /// @notice Emitted when the deposits disabler is updated. + /// @param oldDisabler The address of the previous deposits disabler. + /// @param newDisabler The address of the current deposits disabler. + event UpdateDepositsDisabler(address indexed oldDisabler, address indexed newDisabler); + + /// @notice Emitted when the withdrawals enabler is updated. + /// @param oldEnabler The address of the previous withdrawals enabler. + /// @param newEnabler The address of the current withdrawals enabler. + event UpdateWithdrawalsEnabler(address indexed oldEnabler, address indexed newEnabler); + + /// @notice Emitted when the withdrawals disabler is updated. + /// @param oldDisabler The address of the previous withdrawals disabler. + /// @param newDisabler The address of the current withdrawals disabler. + event UpdateWithdrawalsDisabler(address indexed oldDisabler, address indexed newDisabler); + /********** * Errors * **********/ @@ -163,6 +183,46 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { emit WithdrawalsDisabled(_msgSender()); } + /// @notice Update the address of deposits enabler. + /// @param _newEnabler The address of new deposits enabler. + function updateDepositsEnabler(address _newEnabler) external onlyOwner { + State storage s = _loadState(); + address _oldEnabler = s.depositsEnabler; + s.depositsEnabler = _newEnabler; + + emit UpdateDepositsEnabler(_oldEnabler, _newEnabler); + } + + /// @notice Update the address of deposits disabler. + /// @param _newDisabler The address of new deposits disabler. + function updateDepositsDisabler(address _newDisabler) external onlyOwner { + State storage s = _loadState(); + address _oldDisabler = s.depositsDisabler; + s.depositsDisabler = _newDisabler; + + emit UpdateDepositsDisabler(_oldDisabler, _newDisabler); + } + + /// @notice Update the address of withdrawals enabler. + /// @param _newEnabler The address of new withdrawals enabler. + function updateWithdrawalsEnabler(address _newEnabler) external onlyOwner { + State storage s = _loadState(); + address _oldEnabler = s.withdrawalsEnabler; + s.withdrawalsEnabler = _newEnabler; + + emit UpdateWithdrawalsEnabler(_oldEnabler, _newEnabler); + } + + /// @notice Update the address of withdrawals disabler. + /// @param _newDisabler The address of new withdrawals disabler. + function updateWithdrawalsDisabler(address _newDisabler) external onlyOwner { + State storage s = _loadState(); + address _oldDisabler = s.withdrawalsDisabler; + s.withdrawalsDisabler = _newDisabler; + + emit UpdateWithdrawalsDisabler(_oldDisabler, _newDisabler); + } + /********************** * Internal Functions * **********************/ diff --git a/contracts/src/test/L1LidoGateway.t.sol b/contracts/src/test/L1LidoGateway.t.sol new file mode 100644 index 0000000000..93db65eb00 --- /dev/null +++ b/contracts/src/test/L1LidoGateway.t.sol @@ -0,0 +1,740 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; + +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +import {IL1ERC20Gateway} from "../L1/gateways/IL1ERC20Gateway.sol"; +import {L1GatewayRouter} from "../L1/gateways/L1GatewayRouter.sol"; +import {IL1ScrollMessenger} from "../L1/IL1ScrollMessenger.sol"; +import {IL2ERC20Gateway} from "../L2/gateways/IL2ERC20Gateway.sol"; +import {AddressAliasHelper} from "../libraries/common/AddressAliasHelper.sol"; +import {ScrollConstants} from "../libraries/constants/ScrollConstants.sol"; +import {L2LidoGateway} from "../lido/L2LidoGateway.sol"; + +import {L1GatewayTestBase} from "./L1GatewayTestBase.t.sol"; +import {MockL1LidoGateway} from "./mocks/MockL1LidoGateway.sol"; +import {MockScrollMessenger} from "./mocks/MockScrollMessenger.sol"; +import {MockGatewayRecipient} from "./mocks/MockGatewayRecipient.sol"; + +contract L1LidoGatewayTest is L1GatewayTestBase { + // events from L1LidoGateway + event FinalizeWithdrawERC20( + address indexed _l1Token, + address indexed _l2Token, + address indexed _from, + address _to, + uint256 _amount, + bytes _data + ); + event DepositERC20( + address indexed _l1Token, + address indexed _l2Token, + address indexed _from, + address _to, + uint256 _amount, + bytes _data + ); + event RefundERC20(address indexed token, address indexed recipient, uint256 amount); + event DepositsEnabled(address indexed enabler); + event DepositsDisabled(address indexed disabler); + event WithdrawalsEnabled(address indexed enabler); + event WithdrawalsDisabled(address indexed disabler); + event UpdateDepositsEnabler(address indexed oldEnabler, address indexed newEnabler); + event UpdateDepositsDisabler(address indexed oldDisabler, address indexed newDisabler); + event UpdateWithdrawalsEnabler(address indexed oldEnabler, address indexed newEnabler); + event UpdateWithdrawalsDisabler(address indexed oldDisabler, address indexed newDisabler); + + // errors from L1LidoGateway + error ErrorDepositsEnabled(); + error ErrorDepositsDisabled(); + error ErrorWithdrawalsEnabled(); + error ErrorWithdrawalsDisabled(); + error ErrorCallerIsNotDepositsEnabler(); + error ErrorCallerIsNotDepositsDisabler(); + error ErrorCallerIsNotWithdrawalsEnabler(); + error ErrorCallerIsNotWithdrawalsDisabler(); + error ErrorUnsupportedL1Token(); + error ErrorUnsupportedL2Token(); + error ErrorAccountIsZeroAddress(); + error ErrorNonZeroMsgValue(); + error ErrorDepositZeroAmount(); + + MockL1LidoGateway private gateway; + L1GatewayRouter private router; + + L2LidoGateway private counterpartGateway; + + MockERC20 private l1Token; + MockERC20 private l2Token; + + function setUp() public { + setUpBase(); + // Deploy tokens + l1Token = new MockERC20("Mock L1", "ML1", 18); + l2Token = new MockERC20("Mock L2", "ML2", 18); + + // Deploy L1 contracts + gateway = _deployGateway(); + router = L1GatewayRouter(address(new ERC1967Proxy(address(new L1GatewayRouter()), new bytes(0)))); + + // Deploy L2 contracts + counterpartGateway = new L2LidoGateway(address(l1Token), address(l2Token)); + + // Initialize L1 contracts + gateway.initialize(address(counterpartGateway), address(router), address(l1Messenger)); + gateway.initializeV2(); + router.initialize(address(0), address(gateway)); + + // Prepare token balances + l1Token.mint(address(this), type(uint128).max); + l1Token.approve(address(gateway), type(uint256).max); + l1Token.approve(address(router), type(uint256).max); + } + + function testInitialized() public { + // state in ScrollGatewayBase + assertEq(address(this), gateway.owner()); + assertEq(address(counterpartGateway), gateway.counterpart()); + assertEq(address(router), gateway.router()); + assertEq(address(l1Messenger), gateway.messenger()); + + // state in LidoBridgeableTokens + assertEq(address(l1Token), gateway.l1Token()); + assertEq(address(l2Token), gateway.l2Token()); + + // state in LidoGatewayManager + assertBoolEq(true, gateway.isDepositsEnabled()); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + + // state in L1LidoGateway + assertEq(address(l2Token), gateway.getL2ERC20Address(address(l1Token))); + + hevm.expectRevert("Initializable: contract is already initialized"); + gateway.initialize(address(counterpartGateway), address(router), address(l1Messenger)); + + hevm.expectRevert("Initializable: contract is already initialized"); + gateway.initializeV2(); + } + + /************************************* + * Functions from LidoGatewayManager * + *************************************/ + + function testEnableDeposits() external { + // revert when already enabled + hevm.expectRevert(ErrorDepositsEnabled.selector); + gateway.enableDeposits(); + + // revert when caller is not deposits enabler + gateway.updateDepositsDisabler(address(this)); + gateway.disableDeposits(); + hevm.expectRevert(ErrorCallerIsNotDepositsEnabler.selector); + gateway.enableDeposits(); + + // succeed + gateway.updateDepositsEnabler(address(this)); + assertBoolEq(false, gateway.isDepositsEnabled()); + hevm.expectEmit(true, false, false, true); + emit DepositsEnabled(address(this)); + gateway.enableDeposits(); + assertBoolEq(true, gateway.isDepositsEnabled()); + } + + function testDisableDeposits() external { + // revert when already disabled + gateway.updateDepositsDisabler(address(this)); + gateway.disableDeposits(); + assertBoolEq(false, gateway.isDepositsEnabled()); + hevm.expectRevert(ErrorDepositsDisabled.selector); + gateway.disableDeposits(); + + // revert when caller is not deposits disabler + gateway.updateDepositsEnabler(address(this)); + gateway.enableDeposits(); + assertBoolEq(true, gateway.isDepositsEnabled()); + gateway.updateDepositsDisabler(address(0)); + hevm.expectRevert(ErrorCallerIsNotDepositsDisabler.selector); + gateway.disableDeposits(); + + // succeed + gateway.updateDepositsDisabler(address(this)); + assertBoolEq(true, gateway.isDepositsEnabled()); + hevm.expectEmit(true, false, false, true); + emit DepositsDisabled(address(this)); + gateway.disableDeposits(); + assertBoolEq(false, gateway.isDepositsEnabled()); + } + + function testEnableWithdrawals() external { + // revert when already enabled + hevm.expectRevert(ErrorWithdrawalsEnabled.selector); + gateway.enableWithdrawals(); + + // revert when caller is not deposits enabler + gateway.updateWithdrawalsDisabler(address(this)); + gateway.disableWithdrawals(); + hevm.expectRevert(ErrorCallerIsNotWithdrawalsEnabler.selector); + gateway.enableWithdrawals(); + + // succeed + gateway.updateWithdrawalsEnabler(address(this)); + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + hevm.expectEmit(true, false, false, true); + emit WithdrawalsEnabled(address(this)); + gateway.enableWithdrawals(); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + } + + function testDisableWithdrawals() external { + // revert when already disabled + gateway.updateWithdrawalsDisabler(address(this)); + gateway.disableWithdrawals(); + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + hevm.expectRevert(ErrorWithdrawalsDisabled.selector); + gateway.disableWithdrawals(); + + // revert when caller is not deposits disabler + gateway.updateWithdrawalsEnabler(address(this)); + gateway.enableWithdrawals(); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + gateway.updateWithdrawalsDisabler(address(0)); + hevm.expectRevert(ErrorCallerIsNotWithdrawalsDisabler.selector); + gateway.disableWithdrawals(); + + // succeed + gateway.updateWithdrawalsDisabler(address(this)); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + hevm.expectEmit(true, false, false, true); + emit WithdrawalsDisabled(address(this)); + gateway.disableWithdrawals(); + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + } + + function testUpdateDepositsEnabler(address _enabler) external { + hevm.assume(_enabler != address(0)); + + // revert caller is not owner + hevm.startPrank(address(1)); + hevm.expectRevert("Ownable: caller is not the owner"); + gateway.updateDepositsEnabler(address(0)); + hevm.stopPrank(); + + gateway.updateDepositsDisabler(address(this)); + gateway.disableDeposits(); + + // succeed to set + hevm.startPrank(_enabler); + hevm.expectRevert(ErrorCallerIsNotDepositsEnabler.selector); + gateway.enableDeposits(); + hevm.stopPrank(); + + hevm.expectEmit(true, true, false, true); + emit UpdateDepositsEnabler(address(0), _enabler); + gateway.updateDepositsEnabler(_enabler); + + assertBoolEq(false, gateway.isDepositsEnabled()); + hevm.startPrank(_enabler); + gateway.enableDeposits(); + hevm.stopPrank(); + assertBoolEq(true, gateway.isDepositsEnabled()); + } + + function testUpdateDepositsDisabler(address _disabler) external { + hevm.assume(_disabler != address(0)); + + // revert caller is not owner + hevm.startPrank(address(1)); + hevm.expectRevert("Ownable: caller is not the owner"); + gateway.updateDepositsDisabler(address(0)); + hevm.stopPrank(); + + // succeed to set + hevm.startPrank(_disabler); + hevm.expectRevert(ErrorCallerIsNotDepositsDisabler.selector); + gateway.disableDeposits(); + hevm.stopPrank(); + + hevm.expectEmit(true, true, false, true); + emit UpdateDepositsDisabler(address(0), _disabler); + gateway.updateDepositsDisabler(_disabler); + + assertBoolEq(true, gateway.isDepositsEnabled()); + hevm.startPrank(_disabler); + gateway.disableDeposits(); + hevm.stopPrank(); + assertBoolEq(false, gateway.isDepositsEnabled()); + } + + function testUpdateWithdrawalsEnabler(address _enabler) external { + hevm.assume(_enabler != address(0)); + + // revert caller is not owner + hevm.startPrank(address(1)); + hevm.expectRevert("Ownable: caller is not the owner"); + gateway.updateWithdrawalsEnabler(address(0)); + hevm.stopPrank(); + + gateway.updateWithdrawalsDisabler(address(this)); + gateway.disableWithdrawals(); + + // succeed to set + hevm.startPrank(_enabler); + hevm.expectRevert(ErrorCallerIsNotWithdrawalsEnabler.selector); + gateway.enableWithdrawals(); + hevm.stopPrank(); + + hevm.expectEmit(true, true, false, true); + emit UpdateWithdrawalsEnabler(address(0), _enabler); + gateway.updateWithdrawalsEnabler(_enabler); + + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + hevm.startPrank(_enabler); + gateway.enableWithdrawals(); + hevm.stopPrank(); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + } + + function testUpdateWithdrawalsDisabler(address _disabler) external { + hevm.assume(_disabler != address(0)); + + // revert caller is not owner + hevm.startPrank(address(1)); + hevm.expectRevert("Ownable: caller is not the owner"); + gateway.updateWithdrawalsDisabler(address(0)); + hevm.stopPrank(); + + // succeed to set + hevm.startPrank(_disabler); + hevm.expectRevert(ErrorCallerIsNotWithdrawalsDisabler.selector); + gateway.disableWithdrawals(); + hevm.stopPrank(); + + hevm.expectEmit(true, true, false, true); + emit UpdateWithdrawalsDisabler(address(0), _disabler); + gateway.updateWithdrawalsDisabler(_disabler); + + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + hevm.startPrank(_disabler); + gateway.disableWithdrawals(); + hevm.stopPrank(); + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + } + + /******************************** + * Functions from L1LidoGateway * + ********************************/ + + function testDepositERC20( + uint256 amount, + uint256 gasLimit, + uint256 feePerGas + ) external { + _depositERC20(false, 0, amount, address(this), new bytes(0), gasLimit, feePerGas); + } + + function testDepositERC20WithRecipient( + uint256 amount, + address recipient, + uint256 gasLimit, + uint256 feePerGas + ) external { + _depositERC20(false, 1, amount, recipient, new bytes(0), gasLimit, feePerGas); + } + + function testDepositERC20WithRecipientAndCalldata( + uint256 amount, + address recipient, + bytes memory dataToCall, + uint256 gasLimit, + uint256 feePerGas + ) external { + _depositERC20(false, 2, amount, recipient, dataToCall, gasLimit, feePerGas); + } + + function testDepositERC20ByRouter( + uint256 amount, + uint256 gasLimit, + uint256 feePerGas + ) external { + _depositERC20(true, 0, amount, address(this), new bytes(0), gasLimit, feePerGas); + } + + function testDepositERC20WithRecipientByRouter( + uint256 amount, + address recipient, + uint256 gasLimit, + uint256 feePerGas + ) external { + _depositERC20(true, 1, amount, recipient, new bytes(0), gasLimit, feePerGas); + } + + function testDepositERC20WithRecipientAndCalldataByRouter( + uint256 amount, + address recipient, + bytes memory dataToCall, + uint256 gasLimit, + uint256 feePerGas + ) external { + _depositERC20(true, 2, amount, recipient, dataToCall, gasLimit, feePerGas); + } + + function testDropMessage( + uint256 amount, + address recipient, + bytes memory dataToCall + ) public { + hevm.assume(recipient != address(0)); + + amount = bound(amount, 1, l1Token.balanceOf(address(this))); + bytes memory message = abi.encodeCall( + IL2ERC20Gateway.finalizeDepositERC20, + (address(l1Token), address(l2Token), address(this), recipient, amount, dataToCall) + ); + gateway.depositERC20AndCall(address(l1Token), recipient, amount, dataToCall, defaultGasLimit); + + MockScrollMessenger mockMessenger = new MockScrollMessenger(); + MockL1LidoGateway mockGateway = _deployGateway(); + mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); + mockGateway.initializeV2(); + + // revert caller is not messenger + hevm.expectRevert("only messenger can call"); + mockGateway.onDropMessage(new bytes(0)); + + // revert not in drop context + hevm.expectRevert("only called in drop context"); + mockMessenger.callTarget(address(mockGateway), abi.encodeCall(mockGateway.onDropMessage, (new bytes(0)))); + + // revert when reentrant + mockMessenger.setXDomainMessageSender(ScrollConstants.DROP_XDOMAIN_MESSAGE_SENDER); + hevm.expectRevert("ReentrancyGuard: reentrant call"); + mockGateway.reentrantCall( + address(mockMessenger), + abi.encodeCall( + mockMessenger.callTarget, + (address(mockGateway), abi.encodeCall(mockGateway.onDropMessage, (message))) + ) + ); + + // revert when invalid selector + hevm.expectRevert("invalid selector"); + mockMessenger.callTarget(address(mockGateway), abi.encodeCall(mockGateway.onDropMessage, (new bytes(4)))); + + // revert when l1 token not supported + hevm.expectRevert(ErrorUnsupportedL1Token.selector); + mockMessenger.callTarget( + address(mockGateway), + abi.encodeCall( + mockGateway.onDropMessage, + ( + abi.encodeCall( + IL2ERC20Gateway.finalizeDepositERC20, + (address(l2Token), address(l2Token), address(this), recipient, amount, dataToCall) + ) + ) + ) + ); + + // revert when nonzero msg.value + hevm.expectRevert(ErrorNonZeroMsgValue.selector); + mockMessenger.callTarget{value: 1}( + address(mockGateway), + abi.encodeWithSelector(mockGateway.onDropMessage.selector, message) + ); + + // succeed on drop + // skip message 0 + hevm.startPrank(address(rollup)); + messageQueue.popCrossDomainMessage(0, 1, 0x1); + assertEq(messageQueue.pendingQueueIndex(), 1); + hevm.stopPrank(); + + // should emit RefundERC20 + hevm.expectEmit(true, true, false, true); + emit RefundERC20(address(l1Token), address(this), amount); + + uint256 balance = l1Token.balanceOf(address(this)); + uint256 gatewayBalance = l1Token.balanceOf(address(gateway)); + l1Messenger.dropMessage(address(gateway), address(counterpartGateway), 0, 0, message); + assertEq(gatewayBalance - amount, l1Token.balanceOf(address(gateway))); + assertEq(balance + amount, l1Token.balanceOf(address(this))); + } + + function testFinalizeWithdrawERC20( + address sender, + uint256 amount, + bytes memory dataToCall + ) external { + amount = bound(amount, 1, l1Token.balanceOf(address(this))); + MockGatewayRecipient recipient = new MockGatewayRecipient(); + bytes memory message = abi.encodeCall( + IL1ERC20Gateway.finalizeWithdrawERC20, + (address(l1Token), address(l2Token), sender, address(recipient), amount, dataToCall) + ); + gateway.depositERC20(address(l1Token), amount, defaultGasLimit); // deposit some token to L1LidoGateway + + MockScrollMessenger mockMessenger = new MockScrollMessenger(); + MockL1LidoGateway mockGateway = _deployGateway(); + mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); + mockGateway.initializeV2(); + + // revert caller is not messenger + hevm.expectRevert("only messenger can call"); + mockGateway.finalizeWithdrawERC20( + address(l1Token), + address(l2Token), + sender, + address(recipient), + amount, + dataToCall + ); + + // revert not called by counterpart + hevm.expectRevert("only call by counterpart"); + mockMessenger.callTarget(address(mockGateway), message); + + // revert when reentrant + mockMessenger.setXDomainMessageSender(address(counterpartGateway)); + hevm.expectRevert("ReentrancyGuard: reentrant call"); + mockGateway.reentrantCall( + address(mockMessenger), + abi.encodeCall(mockMessenger.callTarget, (address(mockGateway), message)) + ); + + // revert when l1 token not supported + hevm.expectRevert(ErrorUnsupportedL1Token.selector); + mockMessenger.callTarget( + address(mockGateway), + abi.encodeCall( + IL1ERC20Gateway.finalizeWithdrawERC20, + (address(l2Token), address(l2Token), sender, address(recipient), amount, dataToCall) + ) + ); + + // revert when l2 token not supported + hevm.expectRevert(ErrorUnsupportedL2Token.selector); + mockMessenger.callTarget( + address(mockGateway), + abi.encodeCall( + IL1ERC20Gateway.finalizeWithdrawERC20, + (address(l1Token), address(l1Token), sender, address(recipient), amount, dataToCall) + ) + ); + + // revert when withdrawals disabled + mockGateway.updateWithdrawalsDisabler(address(this)); + mockGateway.disableWithdrawals(); + hevm.expectRevert(ErrorWithdrawalsDisabled.selector); + mockMessenger.callTarget(address(mockGateway), message); + + // revert when nonzero msg.value + mockGateway.updateWithdrawalsEnabler(address(this)); + mockGateway.enableWithdrawals(); + hevm.expectRevert(ErrorNonZeroMsgValue.selector); + mockMessenger.callTarget{value: 1}(address(mockGateway), message); + + // succeed when finialize + bytes memory xDomainCalldata = abi.encodeCall( + l2Messenger.relayMessage, + (address(counterpartGateway), address(gateway), 0, 0, message) + ); + prepareL2MessageRoot(keccak256(xDomainCalldata)); + IL1ScrollMessenger.L2MessageProof memory proof; + proof.batchIndex = rollup.lastFinalizedBatchIndex(); + + // should emit FinalizeWithdrawERC20 from L1StandardERC20Gateway + hevm.expectEmit(true, true, true, true); + emit FinalizeWithdrawERC20(address(l1Token), address(l2Token), sender, address(recipient), amount, dataToCall); + + // should emit RelayedMessage from L1ScrollMessenger + hevm.expectEmit(true, false, false, true); + emit RelayedMessage(keccak256(xDomainCalldata)); + + uint256 gatewayBalance = l1Token.balanceOf(address(gateway)); + uint256 recipientBalance = l1Token.balanceOf(address(recipient)); + assertBoolEq(false, l1Messenger.isL2MessageExecuted(keccak256(xDomainCalldata))); + l1Messenger.relayMessageWithProof(address(counterpartGateway), address(gateway), 0, 0, message, proof); + assertBoolEq(true, l1Messenger.isL2MessageExecuted(keccak256(xDomainCalldata))); + assertEq(recipientBalance + amount, l1Token.balanceOf(address(recipient))); + assertEq(gatewayBalance - amount, l1Token.balanceOf(address(gateway))); + } + + function _depositERC20( + bool useRouter, + uint256 methodType, + uint256 amount, + address recipient, + bytes memory dataToCall, + uint256 gasLimit, + uint256 feePerGas + ) private { + hevm.assume(recipient != address(0)); + amount = bound(amount, 1, l1Token.balanceOf(address(this))); + gasLimit = bound(gasLimit, defaultGasLimit / 2, defaultGasLimit); + feePerGas = bound(feePerGas, 0, 1000); + gasOracle.setL2BaseFee(feePerGas); + feePerGas = feePerGas * gasLimit; + + // revert when reentrant + hevm.expectRevert("ReentrancyGuard: reentrant call"); + { + bytes memory reentrantData; + if (methodType == 0) { + reentrantData = abi.encodeWithSignature( + "depositERC20(address,uint256,uint256)", + address(l1Token), + amount, + gasLimit + ); + } else if (methodType == 1) { + reentrantData = abi.encodeWithSignature( + "depositERC20(address,address,uint256,uint256)", + address(l1Token), + recipient, + amount, + gasLimit + ); + } else if (methodType == 2) { + reentrantData = abi.encodeCall( + IL1ERC20Gateway.depositERC20AndCall, + (address(l1Token), recipient, amount, dataToCall, gasLimit) + ); + } + gateway.reentrantCall(useRouter ? address(router) : address(gateway), reentrantData); + } + + // revert when l1 token not support + hevm.expectRevert(ErrorUnsupportedL1Token.selector); + _invokeDepositERC20Call( + useRouter, + methodType, + address(l2Token), + amount, + recipient, + dataToCall, + gasLimit, + feePerGas + ); + + // revert when to is zero address + if (methodType != 0) { + hevm.expectRevert(ErrorAccountIsZeroAddress.selector); + _invokeDepositERC20Call( + useRouter, + methodType, + address(l1Token), + amount, + address(0), + dataToCall, + gasLimit, + feePerGas + ); + } + + // revert when deposits disabled + gateway.updateDepositsDisabler(address(this)); + gateway.disableDeposits(); + hevm.expectRevert(ErrorDepositsDisabled.selector); + _invokeDepositERC20Call( + useRouter, + methodType, + address(l1Token), + amount, + recipient, + dataToCall, + gasLimit, + feePerGas + ); + + // revert when deposit zero amount + gateway.updateDepositsEnabler(address(this)); + gateway.enableDeposits(); + hevm.expectRevert(ErrorDepositZeroAmount.selector); + _invokeDepositERC20Call(useRouter, methodType, address(l1Token), 0, recipient, dataToCall, gasLimit, feePerGas); + + // succeed to withdraw + bytes memory message = abi.encodeCall( + IL2ERC20Gateway.finalizeDepositERC20, + (address(l1Token), address(l2Token), address(this), recipient, amount, dataToCall) + ); + bytes memory xDomainCalldata = abi.encodeCall( + l2Messenger.relayMessage, + (address(gateway), address(counterpartGateway), 0, 0, message) + ); + // should emit QueueTransaction from L1MessageQueue + hevm.expectEmit(true, true, false, true); + address sender = AddressAliasHelper.applyL1ToL2Alias(address(l1Messenger)); + emit QueueTransaction(sender, address(l2Messenger), 0, 0, gasLimit, xDomainCalldata); + + // should emit SentMessage from L1ScrollMessenger + hevm.expectEmit(true, true, false, true); + emit SentMessage(address(gateway), address(counterpartGateway), 0, 0, gasLimit, message); + + // should emit DepositERC20 from L1CustomERC20Gateway + hevm.expectEmit(true, true, true, true); + emit DepositERC20(address(l1Token), address(l2Token), address(this), recipient, amount, dataToCall); + + uint256 gatewayBalance = l1Token.balanceOf(address(gateway)); + uint256 feeVaultBalance = address(feeVault).balance; + uint256 thisBalance = l1Token.balanceOf(address(this)); + assertEq(l1Messenger.messageSendTimestamp(keccak256(xDomainCalldata)), 0); + uint256 balance = address(this).balance; + _invokeDepositERC20Call( + useRouter, + methodType, + address(l1Token), + amount, + recipient, + dataToCall, + gasLimit, + feePerGas + ); + assertEq(balance - feePerGas, address(this).balance); // extra value is transfered back + assertGt(l1Messenger.messageSendTimestamp(keccak256(xDomainCalldata)), 0); + assertEq(thisBalance - amount, l1Token.balanceOf(address(this))); + assertEq(feeVaultBalance + feePerGas, address(feeVault).balance); + assertEq(gatewayBalance + amount, l1Token.balanceOf(address(gateway))); + } + + function _invokeDepositERC20Call( + bool useRouter, + uint256 methodType, + address token, + uint256 amount, + address recipient, + bytes memory dataToCall, + uint256 gasLimit, + uint256 feeToPay + ) private { + uint256 value = feeToPay + extraValue; + if (useRouter) { + if (methodType == 0) { + router.depositERC20{value: value}(token, amount, gasLimit); + } else if (methodType == 1) { + router.depositERC20{value: value}(token, recipient, amount, gasLimit); + } else if (methodType == 2) { + router.depositERC20AndCall{value: value}(token, recipient, amount, dataToCall, gasLimit); + } + } else { + if (methodType == 0) { + gateway.depositERC20{value: value}(token, amount, gasLimit); + } else if (methodType == 1) { + gateway.depositERC20{value: value}(token, recipient, amount, gasLimit); + } else if (methodType == 2) { + gateway.depositERC20AndCall{value: value}(token, recipient, amount, dataToCall, gasLimit); + } + } + } + + function _deployGateway() internal returns (MockL1LidoGateway) { + return + MockL1LidoGateway( + address( + new ERC1967Proxy(address(new MockL1LidoGateway(address(l1Token), address(l2Token))), new bytes(0)) + ) + ); + } +} diff --git a/contracts/src/test/L2LidoGateway.t.sol b/contracts/src/test/L2LidoGateway.t.sol new file mode 100644 index 0000000000..ed067a5aa9 --- /dev/null +++ b/contracts/src/test/L2LidoGateway.t.sol @@ -0,0 +1,606 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; + +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +import {IL1ERC20Gateway} from "../L1/gateways/IL1ERC20Gateway.sol"; +import {IL2ERC20Gateway} from "../L2/gateways/IL2ERC20Gateway.sol"; +import {L2GatewayRouter} from "../L2/gateways/L2GatewayRouter.sol"; +import {AddressAliasHelper} from "../libraries/common/AddressAliasHelper.sol"; +import {ScrollStandardERC20} from "../libraries/token/ScrollStandardERC20.sol"; +import {L1LidoGateway} from "../lido/L1LidoGateway.sol"; + +import {L2GatewayTestBase} from "./L2GatewayTestBase.t.sol"; +import {MockGatewayRecipient} from "./mocks/MockGatewayRecipient.sol"; +import {MockL2LidoGateway} from "./mocks/MockL2LidoGateway.sol"; +import {MockScrollMessenger} from "./mocks/MockScrollMessenger.sol"; + +contract L2LidoGatewayTest is L2GatewayTestBase { + // events from L2LidoGateway + event WithdrawERC20( + address indexed _l1Token, + address indexed _l2Token, + address indexed _from, + address _to, + uint256 _amount, + bytes _data + ); + event FinalizeDepositERC20( + address indexed _l1Token, + address indexed _l2Token, + address indexed _from, + address _to, + uint256 _amount, + bytes _data + ); + event DepositsEnabled(address indexed enabler); + event DepositsDisabled(address indexed disabler); + event WithdrawalsEnabled(address indexed enabler); + event WithdrawalsDisabled(address indexed disabler); + event UpdateDepositsEnabler(address indexed oldEnabler, address indexed newEnabler); + event UpdateDepositsDisabler(address indexed oldDisabler, address indexed newDisabler); + event UpdateWithdrawalsEnabler(address indexed oldEnabler, address indexed newEnabler); + event UpdateWithdrawalsDisabler(address indexed oldDisabler, address indexed newDisabler); + + // errors from L2LidoGateway + error ErrorDepositsEnabled(); + error ErrorDepositsDisabled(); + error ErrorWithdrawalsEnabled(); + error ErrorWithdrawalsDisabled(); + error ErrorCallerIsNotDepositsEnabler(); + error ErrorCallerIsNotDepositsDisabler(); + error ErrorCallerIsNotWithdrawalsEnabler(); + error ErrorCallerIsNotWithdrawalsDisabler(); + error ErrorUnsupportedL1Token(); + error ErrorUnsupportedL2Token(); + error ErrorAccountIsZeroAddress(); + error ErrorNonZeroMsgValue(); + error ErrorWithdrawZeroAmount(); + + MockL2LidoGateway private gateway; + L2GatewayRouter private router; + + L1LidoGateway private counterpartGateway; + + MockERC20 private l1Token; + ScrollStandardERC20 private l2Token; + + function setUp() public { + setUpBase(); + // Deploy tokens + l1Token = new MockERC20("Mock L1", "ML1", 18); + l2Token = ScrollStandardERC20(address(new ERC1967Proxy(address(new ScrollStandardERC20()), new bytes(0)))); + + // Deploy L2 contracts + gateway = _deployGateway(); + router = L2GatewayRouter(address(new ERC1967Proxy(address(new L2GatewayRouter()), new bytes(0)))); + + // Deploy L1 contracts + counterpartGateway = new L1LidoGateway(address(l1Token), address(l2Token)); + + // Initialize L2 contracts + gateway.initialize(address(counterpartGateway), address(router), address(l2Messenger)); + gateway.initializeV2(); + router.initialize(address(0), address(gateway)); + l2Token.initialize("Mock L2", "ML2", 18, address(gateway), address(l1Token)); + + // Prepare token balances + hevm.startPrank(address(gateway)); + l2Token.mint(address(this), type(uint128).max); + hevm.stopPrank(); + } + + function testInitialized() external { + // state in ScrollGatewayBase + assertEq(address(this), gateway.owner()); + assertEq(address(counterpartGateway), gateway.counterpart()); + assertEq(address(router), gateway.router()); + assertEq(address(l2Messenger), gateway.messenger()); + + // state in LidoBridgeableTokens + assertEq(address(l1Token), gateway.l1Token()); + assertEq(address(l2Token), gateway.l2Token()); + + // state in LidoGatewayManager + assertBoolEq(true, gateway.isDepositsEnabled()); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + + // state in L2LidoGateway + assertEq(address(l1Token), gateway.getL1ERC20Address(address(l2Token))); + assertEq(address(l2Token), gateway.getL2ERC20Address(address(l1Token))); + + hevm.expectRevert("Initializable: contract is already initialized"); + gateway.initialize(address(counterpartGateway), address(router), address(l2Messenger)); + + hevm.expectRevert("Initializable: contract is already initialized"); + gateway.initializeV2(); + } + + /************************************* + * Functions from LidoGatewayManager * + *************************************/ + + function testEnableDeposits() external { + // revert when already enabled + hevm.expectRevert(ErrorDepositsEnabled.selector); + gateway.enableDeposits(); + + // revert when caller is not deposits enabler + gateway.updateDepositsDisabler(address(this)); + gateway.disableDeposits(); + hevm.expectRevert(ErrorCallerIsNotDepositsEnabler.selector); + gateway.enableDeposits(); + + // succeed + gateway.updateDepositsEnabler(address(this)); + assertBoolEq(false, gateway.isDepositsEnabled()); + hevm.expectEmit(true, false, false, true); + emit DepositsEnabled(address(this)); + gateway.enableDeposits(); + assertBoolEq(true, gateway.isDepositsEnabled()); + } + + function testDisableDeposits() external { + // revert when already disabled + gateway.updateDepositsDisabler(address(this)); + gateway.disableDeposits(); + assertBoolEq(false, gateway.isDepositsEnabled()); + hevm.expectRevert(ErrorDepositsDisabled.selector); + gateway.disableDeposits(); + + // revert when caller is not deposits disabler + gateway.updateDepositsEnabler(address(this)); + gateway.enableDeposits(); + assertBoolEq(true, gateway.isDepositsEnabled()); + gateway.updateDepositsDisabler(address(0)); + hevm.expectRevert(ErrorCallerIsNotDepositsDisabler.selector); + gateway.disableDeposits(); + + // succeed + gateway.updateDepositsDisabler(address(this)); + assertBoolEq(true, gateway.isDepositsEnabled()); + hevm.expectEmit(true, false, false, true); + emit DepositsDisabled(address(this)); + gateway.disableDeposits(); + assertBoolEq(false, gateway.isDepositsEnabled()); + } + + function testEnableWithdrawals() external { + // revert when already enabled + hevm.expectRevert(ErrorWithdrawalsEnabled.selector); + gateway.enableWithdrawals(); + + // revert when caller is not deposits enabler + gateway.updateWithdrawalsDisabler(address(this)); + gateway.disableWithdrawals(); + hevm.expectRevert(ErrorCallerIsNotWithdrawalsEnabler.selector); + gateway.enableWithdrawals(); + + // succeed + gateway.updateWithdrawalsEnabler(address(this)); + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + hevm.expectEmit(true, false, false, true); + emit WithdrawalsEnabled(address(this)); + gateway.enableWithdrawals(); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + } + + function testDisableWithdrawals() external { + // revert when already disabled + gateway.updateWithdrawalsDisabler(address(this)); + gateway.disableWithdrawals(); + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + hevm.expectRevert(ErrorWithdrawalsDisabled.selector); + gateway.disableWithdrawals(); + + // revert when caller is not deposits disabler + gateway.updateWithdrawalsEnabler(address(this)); + gateway.enableWithdrawals(); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + gateway.updateWithdrawalsDisabler(address(0)); + hevm.expectRevert(ErrorCallerIsNotWithdrawalsDisabler.selector); + gateway.disableWithdrawals(); + + // succeed + gateway.updateWithdrawalsDisabler(address(this)); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + hevm.expectEmit(true, false, false, true); + emit WithdrawalsDisabled(address(this)); + gateway.disableWithdrawals(); + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + } + + function testUpdateDepositsEnabler(address _enabler) external { + hevm.assume(_enabler != address(0)); + + // revert caller is not owner + hevm.startPrank(address(1)); + hevm.expectRevert("Ownable: caller is not the owner"); + gateway.updateDepositsEnabler(address(0)); + hevm.stopPrank(); + + gateway.updateDepositsDisabler(address(this)); + gateway.disableDeposits(); + + // succeed to set + hevm.startPrank(_enabler); + hevm.expectRevert(ErrorCallerIsNotDepositsEnabler.selector); + gateway.enableDeposits(); + hevm.stopPrank(); + + hevm.expectEmit(true, true, false, true); + emit UpdateDepositsEnabler(address(0), _enabler); + gateway.updateDepositsEnabler(_enabler); + + assertBoolEq(false, gateway.isDepositsEnabled()); + hevm.startPrank(_enabler); + gateway.enableDeposits(); + hevm.stopPrank(); + assertBoolEq(true, gateway.isDepositsEnabled()); + } + + function testUpdateDepositsDisabler(address _disabler) external { + hevm.assume(_disabler != address(0)); + + // revert caller is not owner + hevm.startPrank(address(1)); + hevm.expectRevert("Ownable: caller is not the owner"); + gateway.updateDepositsDisabler(address(0)); + hevm.stopPrank(); + + // succeed to set + hevm.startPrank(_disabler); + hevm.expectRevert(ErrorCallerIsNotDepositsDisabler.selector); + gateway.disableDeposits(); + hevm.stopPrank(); + + hevm.expectEmit(true, true, false, true); + emit UpdateDepositsDisabler(address(0), _disabler); + gateway.updateDepositsDisabler(_disabler); + + assertBoolEq(true, gateway.isDepositsEnabled()); + hevm.startPrank(_disabler); + gateway.disableDeposits(); + hevm.stopPrank(); + assertBoolEq(false, gateway.isDepositsEnabled()); + } + + function testUpdateWithdrawalsEnabler(address _enabler) external { + hevm.assume(_enabler != address(0)); + + // revert caller is not owner + hevm.startPrank(address(1)); + hevm.expectRevert("Ownable: caller is not the owner"); + gateway.updateWithdrawalsEnabler(address(0)); + hevm.stopPrank(); + + gateway.updateWithdrawalsDisabler(address(this)); + gateway.disableWithdrawals(); + + // succeed to set + hevm.startPrank(_enabler); + hevm.expectRevert(ErrorCallerIsNotWithdrawalsEnabler.selector); + gateway.enableWithdrawals(); + hevm.stopPrank(); + + hevm.expectEmit(true, true, false, true); + emit UpdateWithdrawalsEnabler(address(0), _enabler); + gateway.updateWithdrawalsEnabler(_enabler); + + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + hevm.startPrank(_enabler); + gateway.enableWithdrawals(); + hevm.stopPrank(); + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + } + + function testUpdateWithdrawalsDisabler(address _disabler) external { + hevm.assume(_disabler != address(0)); + + // revert caller is not owner + hevm.startPrank(address(1)); + hevm.expectRevert("Ownable: caller is not the owner"); + gateway.updateWithdrawalsDisabler(address(0)); + hevm.stopPrank(); + + // succeed to set + hevm.startPrank(_disabler); + hevm.expectRevert(ErrorCallerIsNotWithdrawalsDisabler.selector); + gateway.disableWithdrawals(); + hevm.stopPrank(); + + hevm.expectEmit(true, true, false, true); + emit UpdateWithdrawalsDisabler(address(0), _disabler); + gateway.updateWithdrawalsDisabler(_disabler); + + assertBoolEq(true, gateway.isWithdrawalsEnabled()); + hevm.startPrank(_disabler); + gateway.disableWithdrawals(); + hevm.stopPrank(); + assertBoolEq(false, gateway.isWithdrawalsEnabled()); + } + + /******************************** + * Functions from L2LidoGateway * + ********************************/ + + function testGetL1ERC20Address(address token) external { + hevm.assume(token != address(l2Token)); + hevm.expectRevert(ErrorUnsupportedL2Token.selector); + gateway.getL1ERC20Address(token); + } + + function testGetL2ERC20Address(address token) external { + hevm.assume(token != address(l1Token)); + hevm.expectRevert(ErrorUnsupportedL1Token.selector); + gateway.getL2ERC20Address(token); + } + + function testWithdrawERC20(uint256 amount, uint256 gasLimit) external { + _withdrawERC20(false, 0, amount, address(this), new bytes(0), gasLimit); + } + + function testWithdrawERC20WithRecipient( + uint256 amount, + address recipient, + uint256 gasLimit + ) external { + _withdrawERC20(false, 1, amount, recipient, new bytes(0), gasLimit); + } + + function testWithdrawERC20WithRecipientAndCalldata( + uint256 amount, + address recipient, + bytes memory dataToCall, + uint256 gasLimit + ) external { + _withdrawERC20(false, 2, amount, recipient, dataToCall, gasLimit); + } + + function testWithdrawERC20ByRouter(uint256 amount, uint256 gasLimit) external { + _withdrawERC20(true, 0, amount, address(this), new bytes(0), gasLimit); + } + + function testWithdrawERC20WithRecipientByRouter( + uint256 amount, + address recipient, + uint256 gasLimit + ) external { + _withdrawERC20(true, 1, amount, recipient, new bytes(0), gasLimit); + } + + function testWithdrawERC20WithRecipientAndCalldataByRouter( + uint256 amount, + address recipient, + bytes memory dataToCall, + uint256 gasLimit + ) external { + _withdrawERC20(true, 2, amount, recipient, dataToCall, gasLimit); + } + + function testFinalizeDepositERC20( + address sender, + uint256 amount, + bytes memory dataToCall + ) external { + amount = bound(amount, 1, l2Token.balanceOf(address(this))); + MockGatewayRecipient recipient = new MockGatewayRecipient(); + bytes memory message = abi.encodeCall( + IL2ERC20Gateway.finalizeDepositERC20, + (address(l1Token), address(l2Token), sender, address(recipient), amount, dataToCall) + ); + + MockScrollMessenger mockMessenger = new MockScrollMessenger(); + MockL2LidoGateway mockGateway = _deployGateway(); + mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); + mockGateway.initializeV2(); + + // revert caller is not messenger + hevm.expectRevert("only messenger can call"); + mockGateway.finalizeDepositERC20( + address(l1Token), + address(l2Token), + sender, + address(recipient), + amount, + dataToCall + ); + + // revert not called by counterpart + hevm.expectRevert("only call by counterpart"); + mockMessenger.callTarget(address(mockGateway), message); + + // revert when reentrant + mockMessenger.setXDomainMessageSender(address(counterpartGateway)); + hevm.expectRevert("ReentrancyGuard: reentrant call"); + mockGateway.reentrantCall( + address(mockMessenger), + abi.encodeCall(mockMessenger.callTarget, (address(mockGateway), message)) + ); + + // revert when l1 token not supported + hevm.expectRevert(ErrorUnsupportedL1Token.selector); + mockMessenger.callTarget( + address(mockGateway), + abi.encodeCall( + IL2ERC20Gateway.finalizeDepositERC20, + (address(l2Token), address(l2Token), sender, address(recipient), amount, dataToCall) + ) + ); + + // revert when l2 token not supported + hevm.expectRevert(ErrorUnsupportedL2Token.selector); + mockMessenger.callTarget( + address(mockGateway), + abi.encodeCall( + IL2ERC20Gateway.finalizeDepositERC20, + (address(l1Token), address(l1Token), sender, address(recipient), amount, dataToCall) + ) + ); + + // revert when deposits disabled + mockGateway.updateDepositsDisabler(address(this)); + mockGateway.disableDeposits(); + hevm.expectRevert(ErrorDepositsDisabled.selector); + mockMessenger.callTarget(address(mockGateway), message); + + // revert when nonzero msg.value + mockGateway.updateDepositsEnabler(address(this)); + mockGateway.enableDeposits(); + hevm.expectRevert(ErrorNonZeroMsgValue.selector); + mockMessenger.callTarget{value: 1}(address(mockGateway), message); + + // succeed when finialize + bytes memory xDomainCalldata = abi.encodeCall( + l2Messenger.relayMessage, + (address(counterpartGateway), address(gateway), 0, 0, message) + ); + + // should emit FinalizeDepositERC20 from L2LidoGateway + hevm.expectEmit(true, true, true, true); + emit FinalizeDepositERC20(address(l1Token), address(l2Token), sender, address(recipient), amount, dataToCall); + + // should emit RelayedMessage from L2ScrollMessenger + hevm.expectEmit(true, false, false, true); + emit RelayedMessage(keccak256(xDomainCalldata)); + + uint256 gatewayBalance = l2Token.balanceOf(address(gateway)); + uint256 recipientBalance = l2Token.balanceOf(address(recipient)); + assertBoolEq(false, l2Messenger.isL1MessageExecuted(keccak256(xDomainCalldata))); + hevm.startPrank(AddressAliasHelper.applyL1ToL2Alias(address(l1Messenger))); + l2Messenger.relayMessage(address(counterpartGateway), address(gateway), 0, 0, message); + hevm.stopPrank(); + assertBoolEq(true, l2Messenger.isL1MessageExecuted(keccak256(xDomainCalldata))); // executed + assertEq(recipientBalance + amount, l2Token.balanceOf(address(recipient))); // mint token + assertEq(gatewayBalance, l2Token.balanceOf(address(gateway))); // gateway balance unchanged + } + + function _withdrawERC20( + bool useRouter, + uint256 methodType, + uint256 amount, + address recipient, + bytes memory dataToCall, + uint256 gasLimit + ) private { + hevm.assume(recipient != address(0)); + amount = bound(amount, 1, l2Token.balanceOf(address(this))); + + // revert when reentrant + hevm.expectRevert("ReentrancyGuard: reentrant call"); + bytes memory reentrantData; + if (methodType == 0) { + reentrantData = abi.encodeWithSignature( + "withdrawERC20(address,uint256,uint256)", + address(l2Token), + amount, + gasLimit + ); + } else if (methodType == 1) { + reentrantData = abi.encodeWithSignature( + "withdrawERC20(address,address,uint256,uint256)", + address(l2Token), + recipient, + amount, + gasLimit + ); + } else if (methodType == 2) { + reentrantData = abi.encodeCall( + IL2ERC20Gateway.withdrawERC20AndCall, + (address(l2Token), recipient, amount, dataToCall, gasLimit) + ); + } + gateway.reentrantCall(useRouter ? address(router) : address(gateway), reentrantData); + + // revert when l2 token not support + hevm.expectRevert(ErrorUnsupportedL2Token.selector); + _invokeWithdrawERC20Call(useRouter, methodType, address(l1Token), amount, recipient, dataToCall, gasLimit); + + // revert when to is zero address + if (methodType != 0) { + hevm.expectRevert(ErrorAccountIsZeroAddress.selector); + _invokeWithdrawERC20Call(useRouter, methodType, address(l2Token), amount, address(0), dataToCall, gasLimit); + } + + // revert when withdrawals disabled + gateway.updateWithdrawalsDisabler(address(this)); + gateway.disableWithdrawals(); + hevm.expectRevert(ErrorWithdrawalsDisabled.selector); + _invokeWithdrawERC20Call(useRouter, methodType, address(l2Token), amount, recipient, dataToCall, gasLimit); + + // revert when withdraw zero amount + gateway.updateWithdrawalsEnabler(address(this)); + gateway.enableWithdrawals(); + hevm.expectRevert(ErrorWithdrawZeroAmount.selector); + _invokeWithdrawERC20Call(useRouter, methodType, address(l2Token), 0, recipient, dataToCall, gasLimit); + + // succeed to withdraw + bytes memory message = abi.encodeCall( + IL1ERC20Gateway.finalizeWithdrawERC20, + (address(l1Token), address(l2Token), address(this), recipient, amount, dataToCall) + ); + bytes memory xDomainCalldata = abi.encodeCall( + l2Messenger.relayMessage, + (address(gateway), address(counterpartGateway), 0, 0, message) + ); + // should emit AppendMessage from L2MessageQueue + hevm.expectEmit(false, false, false, true); + emit AppendMessage(0, keccak256(xDomainCalldata)); + + // should emit SentMessage from L2ScrollMessenger + hevm.expectEmit(true, true, false, true); + emit SentMessage(address(gateway), address(counterpartGateway), 0, 0, gasLimit, message); + + // should emit WithdrawERC20 from L2LidoGateway + hevm.expectEmit(true, true, true, true); + emit WithdrawERC20(address(l1Token), address(l2Token), address(this), recipient, amount, dataToCall); + + uint256 gatewayBalance = l2Token.balanceOf(address(gateway)); + uint256 thisBalance = l2Token.balanceOf(address(this)); + assertEq(l2Messenger.messageSendTimestamp(keccak256(xDomainCalldata)), 0); + _invokeWithdrawERC20Call(useRouter, methodType, address(l2Token), amount, recipient, dataToCall, gasLimit); + assertGt(l2Messenger.messageSendTimestamp(keccak256(xDomainCalldata)), 0); + assertEq(thisBalance - amount, l2Token.balanceOf(address(this))); + assertEq(gatewayBalance, l2Token.balanceOf(address(gateway))); + } + + function _invokeWithdrawERC20Call( + bool useRouter, + uint256 methodType, + address token, + uint256 amount, + address recipient, + bytes memory dataToCall, + uint256 gasLimit + ) private { + if (useRouter) { + if (methodType == 0) { + router.withdrawERC20(token, amount, gasLimit); + } else if (methodType == 1) { + router.withdrawERC20(token, recipient, amount, gasLimit); + } else if (methodType == 2) { + router.withdrawERC20AndCall(token, recipient, amount, dataToCall, gasLimit); + } + } else { + if (methodType == 0) { + gateway.withdrawERC20(token, amount, gasLimit); + } else if (methodType == 1) { + gateway.withdrawERC20(token, recipient, amount, gasLimit); + } else if (methodType == 2) { + gateway.withdrawERC20AndCall(token, recipient, amount, dataToCall, gasLimit); + } + } + } + + function _deployGateway() internal returns (MockL2LidoGateway) { + return + MockL2LidoGateway( + address( + new ERC1967Proxy(address(new MockL2LidoGateway(address(l1Token), address(l2Token))), new bytes(0)) + ) + ); + } +} diff --git a/contracts/src/test/mocks/MockL1LidoGateway.sol b/contracts/src/test/mocks/MockL1LidoGateway.sol new file mode 100644 index 0000000000..56f50b18f8 --- /dev/null +++ b/contracts/src/test/mocks/MockL1LidoGateway.sol @@ -0,0 +1,18 @@ +import {L1LidoGateway} from "../../lido/L1LidoGateway.sol"; + +contract MockL1LidoGateway is L1LidoGateway { + constructor(address _l1Token, address _l2Token) L1LidoGateway(_l1Token, _l2Token) {} + + function reentrantCall(address target, bytes calldata data) external payable nonReentrant { + (bool success, ) = target.call{value: msg.value}(data); + if (!success) { + // solhint-disable-next-line no-inline-assembly + assembly { + let ptr := mload(0x40) + let size := returndatasize() + returndatacopy(ptr, 0, size) + revert(ptr, size) + } + } + } +} diff --git a/contracts/src/test/mocks/MockL2LidoGateway.sol b/contracts/src/test/mocks/MockL2LidoGateway.sol new file mode 100644 index 0000000000..24fb29853b --- /dev/null +++ b/contracts/src/test/mocks/MockL2LidoGateway.sol @@ -0,0 +1,18 @@ +import {L2LidoGateway} from "../../lido/L2LidoGateway.sol"; + +contract MockL2LidoGateway is L2LidoGateway { + constructor(address _l1Token, address _l2Token) L2LidoGateway(_l1Token, _l2Token) {} + + function reentrantCall(address target, bytes calldata data) external payable nonReentrant { + (bool success, ) = target.call{value: msg.value}(data); + if (!success) { + // solhint-disable-next-line no-inline-assembly + assembly { + let ptr := mload(0x40) + let size := returndatasize() + returndatacopy(ptr, 0, size) + revert(ptr, size) + } + } + } +} From 581f2e45863008b80966a8ad4e59dc894a81a04f Mon Sep 17 00:00:00 2001 From: zimpha Date: Wed, 8 Nov 2023 10:59:30 +0800 Subject: [PATCH 3/9] add readme for lido gateway --- contracts/src/lido/README.md | 104 +++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/contracts/src/lido/README.md b/contracts/src/lido/README.md index b45e886200..a70d3ee3bf 100644 --- a/contracts/src/lido/README.md +++ b/contracts/src/lido/README.md @@ -1 +1,105 @@ # Lido's Scroll Bridge + +The document details the implementation of the bridging of the ERC20 compatible tokens[^*] between Ethereum and Scroll chains. + +It's the first step of Lido's integration into the Scroll protocol. The main goal of the current implementation is to be the strong foundation for the long-term goals of the Lido expansion in the Scroll chain. The long-run picture of the Lido's integration into L2s includes: + +- Bridging of Lido's tokens from L1 to L2 chains +- Instant ETH staking on L2 chains with receiving stETH/wstETH on the corresponding L2 immediately +- Keeping UX on L2 as close as possible to the UX on Ethereum mainnet + +At this point, the implementation must provide a scalable and reliable solution for Lido to bridge ERC20 compatible tokens between Scroll and the Ethereum chain. + +[^*]: The current implementation might not support the non-standard functionality of the ERC20 tokens. For example, rebasable tokens or tokens with transfers fee will work incorrectly. In case your token implements some non-typical ERC20 logic, make sure it is compatible with the bridge before usage. + +## Security surface overview + +| Statement | Answer | +| -------------------------------------------------------------------------------------------------------------------------------------------- | ------ | +| It is possible to bridge wstETH forth and back using this bridge | Yes | +| The bridge using a canonical mechanism for message/value passing | Yes | +| The bridge is upgradeable | Yes | +| Upgrade authority for the bridge | TBA | +| Emergency pause/cancel mechanisms and their authorities | TBA | +| The bridged token support permits and ERC-1271 | Yes | +| Are the following things in the scope of this bridge deployment: | | +| - Passing the (w)stETH/USD price feed | No | +| - Passing Lido DAO governance decisions | TBA | +| Bridges are complicated in that the transaction can succeed on one side and fail on the other. What's the handling mechanism for this issue? | TBA | +| Is there a deployment script that sets all the parameters and authorities correctly? | TBA | +| Is there a post-deploy check script that, given a deployment, checks that all parameters and authorities are set correctly? | TBA | + +## Scroll's Bridging Flow + +The default implementation of the Scroll bridging solution consists of two parts: `L1StandardERC20Gateway` and `L2StandardERC20Gateway`. These contracts allow bridging the ERC20 tokens between Ethereum and Scroll chains. + +In the standard bridge, when ERC20 is deposited on L1 and transferred to the bridge contract it remains "locked" there while the equivalent amount is minted in the L2 token. For withdrawals, the opposite happens the L2 token amount is burned then the same amount of L1 tokens is transferred to the recipient. + +The default Scroll bridge is suitable for the short-term goal of the Lido (bridging of the wstETH token into Scroll), but it complicates the achievement of the long-term goals. For example, implementation of the staking from L2's very likely will require extending the token and gateway implementations. + +Additionally, Scroll provides functionality to implement the custom bridge solution utilizing the same cross-domain infrastructure as the Standard bridge. The only constraint for the custom bridge to be compatible with the default Scroll Gateway is the implementation of the `IL1ERC20Gateway` and `IL2ERC20Gateway` interfaces. + +The rest of the document provides a technical specification of the bridge Lido will use to transfer tokens between Ethereum and Scroll chains. + +## Lido's Bridge Implementation + +The current implementation of the tokens bridge provides functionality to bridge the specified type of ERC20 compatible token between Ethereum and Scroll chains. Additionally, the bridge provides some administrative features, like the **temporary disabling of the deposits and withdrawals**. It's necessary when bridging must be disabled fast because of the malicious usage of the bridge or vulnerability in the contracts. Also, it might be helpful in the implementation upgrade process. + +The technical implementation focuses on the following requirements for the contracts: + +- **Scalability** - current implementation must provide the ability to be extended with new functionality in the future. +- **Simplicity** - implemented contracts must be clear, simple, and expressive for developers who will work with code in the future. +- **Gas efficiency** - implemented solution must be efficient in terms of gas costs for the end-user, but at the same time, it must not violate the previous requirement. + +A high-level overview of the proposed solution might be found in the below diagram: + +![](https://i.imgur.com/7UaVuto.png) + +- [**`LidoGatewayManager`**](./LidoGatewayManager.sol) - contains administrative methods to retrieve and control the state of the bridging process. +- [**`LidoBridgeableTokens`**](./LidoBridgeableTokens.sol) - contains the logic for validation of tokens used in the bridging process. +- [**`L1LidoGateway`**](./L1LidoGateway.sol) - Ethereum's counterpart of the bridge to bridge registered ERC20 compatible tokens between Ethereum and Scroll chains. +- [**`L2LidoGateway`**](./L2LidoGateway.sol) - Scroll's counterpart of the bridge to bridge registered ERC20 compatible tokens between Ethereum and Scroll chains +- [**`ScrollStandardERC20`**](../libraries/token/ScrollStandardERC20.sol) - an implementation of the `ERC20` token with administrative methods to mint and burn tokens. +- [**`TransparentUpgradeableProxy`**](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/proxy/transparent/TransparentUpgradeableProxy.sol) - the ERC1967 proxy with extra admin functionality. + +## Scroll's Bridging Flow + +The general process of bridging tokens via Scroll's Lido bridge can be found here: [ETH and ERC20 Token Bridge](https://docs.scroll.io/en/developers/l1-and-l2-bridging/eth-and-erc20-token-bridge/). + +## Deployment Process + +To reduce the gas costs for users, contracts `L1LidoGateway`, `L2LidoGateway`, and `ScrollStandardERC20` contracts use immutable variables as much as possible. But some of those variables are cross-referred. For example, `L1LidoGateway` has reference to `L2LidoGateway` and vice versa. As we use proxies, we can deploy proxies at first and without calling the `initialize` function from each gateway. Then call the `initialize` function with correct contract addresses. + +Another option - pre-calculate the future address of the deployed contract offchain and deployed the implementation using pre-calculated addresses. But it is less fault-tolerant than the solution above. + +## Integration Risks + +As an additional link in the tokens flow chain, the Scroll protocol and bridges add points of failure. Below are the main risks of the current integration: + +### Minting of uncollateralized L2 token + +Such an attack might happen if an attacker obtains the right to call `L2LidoGateway.finalizeDepositERC20()` directly. In such a scenario, an attacker can mint uncollaterized tokens on L2 and initiate withdrawal later. + +The best way to detect such an attack is an offchain monitoring of the minting and depositing/withdrawal events. Based on such events might be tracked following stats: + +- `l1ERC20TokenBridgeBalance` - a total number of locked tokens on the L1 bridge contract +- `l2TokenTotalSupply` - total number of minted L2 tokens +- `l2TokenNotWithdrawn` - total number of burned L2 tokens which aren’t withdrawn from the L1 bridge + +At any time following invariant must be satisfied: `l1ERC20TokenBridgeBalance == l2TokenTotalSupply + l2TokenNotWithdrawn`. + +In the case of invariant violation, Lido will have a dispute period to suspend the L1 and L2 bridges. Disabled bridges forbid the minting of L2Token and withdrawal of minted tokens till the resolution of the issue. + +### Attack on L1ScrollMessenger + +According to the Scroll documentation, `L1ScrollMessenger`: + +> The L1 Scroll Messenger contract sends messages from L1 to L2 and relays messages from L2 onto L1. + +This contract is central in the L2 to L1 communication process since all messages from L2 that passed the challenge period are executed on behalf of this contract. + +In case of a vulnerability in the `L1ScrollMessenger`, which allows the attacker to send arbitrary messages bypassing the dispute period, an attacker can immediately drain tokens from the L1 bridge. + +Additional risk creates the upgradeability of the `L1ScrollMessenger`. Exist a risk of an attack with the replacement of the implementation with some malicious functionality. Such an attack might be reduced to the above vulnerability and steal all locked tokens on the L1 bridge. + +To respond quickly to such an attack, Lido can set up monitoring of the Proxy contract, which will ring the alarm in case of an implementation upgrade. From f08e0d878a8b1bab40ff81376a84e92cc1a09484 Mon Sep 17 00:00:00 2001 From: zimpha Date: Tue, 14 Nov 2023 17:01:22 +0800 Subject: [PATCH 4/9] add fork integration for lido gateways --- contracts/foundry.toml | 1 - contracts/src/test/integration/Domain.t.sol | 40 ++++++ .../integration/GatewayIntegrationBase.t.sol | 129 ++++++++++++++++++ .../integration/LidoGatewayIntegration.t.sol | 114 ++++++++++++++++ .../src/test/mocks/MockL1LidoGateway.sol | 4 + .../src/test/mocks/MockL2LidoGateway.sol | 4 + 6 files changed, 291 insertions(+), 1 deletion(-) create mode 100644 contracts/src/test/integration/Domain.t.sol create mode 100644 contracts/src/test/integration/GatewayIntegrationBase.t.sol create mode 100644 contracts/src/test/integration/LidoGatewayIntegration.t.sol diff --git a/contracts/foundry.toml b/contracts/foundry.toml index ca3af0b164..bff45c75ed 100644 --- a/contracts/foundry.toml +++ b/contracts/foundry.toml @@ -20,7 +20,6 @@ sender = '0x00a329c0648769a73afac7f9381e08fb43dbea72' # the address of ` tx_origin = '0x00a329c0648769a73afac7f9381e08fb43dbea72' # the address of `tx.origin` in tests initial_balance = '0xffffffffffffffffffffffff' # the initial balance of the test contract block_number = 0 # the block number we are at in tests -chain_id = 99 # the chain id we are on in tests gas_limit = 9223372036854775807 # the gas limit in tests gas_price = 0 # the gas price (in wei) in tests block_base_fee_per_gas = 0 # the base fee (in wei) in tests diff --git a/contracts/src/test/integration/Domain.t.sol b/contracts/src/test/integration/Domain.t.sol new file mode 100644 index 0000000000..b5e81cde46 --- /dev/null +++ b/contracts/src/test/integration/Domain.t.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +pragma solidity >=0.8.0; + +import {console2} from "forge-std/console2.sol"; +import {StdChains} from "forge-std/StdChains.sol"; +import {Vm} from "forge-std/Vm.sol"; + +// code from: https://github.com/marsfoundation/xchain-helpers/blob/master/src/testing/Domain.sol + +contract Domain { + // solhint-disable-next-line const-name-snakecase + Vm internal constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code"))))); + + StdChains.Chain private _details; + uint256 public forkId; + + constructor(StdChains.Chain memory _chain) { + _details = _chain; + forkId = vm.createFork(_chain.rpcUrl); + vm.makePersistent(address(this)); + } + + function details() public view returns (StdChains.Chain memory) { + return _details; + } + + function selectFork() public { + vm.selectFork(forkId); + require( + block.chainid == _details.chainId, + string( + abi.encodePacked(_details.chainAlias, " is pointing to the wrong RPC endpoint '", _details.rpcUrl, "'") + ) + ); + } + + function rollFork(uint256 blocknum) public { + vm.rollFork(forkId, blocknum); + } +} diff --git a/contracts/src/test/integration/GatewayIntegrationBase.t.sol b/contracts/src/test/integration/GatewayIntegrationBase.t.sol new file mode 100644 index 0000000000..af9172f9b9 --- /dev/null +++ b/contracts/src/test/integration/GatewayIntegrationBase.t.sol @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {Test} from "forge-std/Test.sol"; +import {Vm} from "forge-std/Vm.sol"; + +import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; +import {ITransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; + +import {Domain} from "./Domain.t.sol"; + +import {IL2ScrollMessenger} from "../../L2/IL2ScrollMessenger.sol"; +import {AddressAliasHelper} from "../../libraries/common/AddressAliasHelper.sol"; + +abstract contract GatewayIntegrationBase is Test { + bytes32 private constant SENT_MESSAGE_TOPIC = + keccak256("SentMessage(address,address,uint256,uint256,uint256,bytes)"); + + address internal constant L1_SCROLL_MESSENGER = 0x6774Bcbd5ceCeF1336b5300fb5186a12DDD8b367; + + address internal constant L1_SCROLL_CHAIN = 0xa13BAF47339d63B743e7Da8741db5456DAc1E556; + + address internal constant L1_MESSAGE_QUEUE = 0x0d7E906BD9cAFa154b048cFa766Cc1E54E39AF9B; + + address internal constant L1_GATEWAY_ROUTER = 0xF8B1378579659D8F7EE5f3C929c2f3E332E41Fd6; + + address internal constant L2_SCROLL_MESSENGER = 0x781e90f1c8Fc4611c9b7497C3B47F99Ef6969CbC; + + address internal constant L2_MESSAGE_QUEUE = 0x5300000000000000000000000000000000000000; + + address internal constant L2_GATEWAY_ROUTER = 0x4C0926FF5252A435FD19e10ED15e5a249Ba19d79; + + Domain internal mainnet; + + Domain internal scroll; + + uint256 internal lastFromMainnetLogIndex; + + uint256 internal lastFromScrollLogIndex; + + receive() external payable {} + + // solhint-disable-next-line func-name-mixedcase + function __GatewayIntegrationBase_setUp() internal { + setChain("scroll", ChainData("Scroll Chain", 534352, "https://rpc.scroll.io")); + setChain("mainnet", ChainData("Mainnet", 1, "https://rpc.ankr.com/eth")); + + mainnet = new Domain(getChain("mainnet")); + scroll = new Domain(getChain("scroll")); + } + + function relayFromMainnet() internal { + scroll.selectFork(); + + address malias = AddressAliasHelper.applyL1ToL2Alias(L1_SCROLL_MESSENGER); + + // Read all L1 -> L2 messages and relay them under Optimism fork + Vm.Log[] memory allLogs = vm.getRecordedLogs(); + for (; lastFromMainnetLogIndex < allLogs.length; lastFromMainnetLogIndex++) { + Vm.Log memory _log = allLogs[lastFromMainnetLogIndex]; + if (_log.topics[0] == SENT_MESSAGE_TOPIC && _log.emitter == address(L1_SCROLL_MESSENGER)) { + address sender = address(uint160(uint256(_log.topics[1]))); + address target = address(uint160(uint256(_log.topics[2]))); + (uint256 value, uint256 nonce, uint256 gasLimit, bytes memory message) = abi.decode( + _log.data, + (uint256, uint256, uint256, bytes) + ); + vm.prank(malias); + IL2ScrollMessenger(L2_SCROLL_MESSENGER).relayMessage{gas: gasLimit}( + sender, + target, + value, + nonce, + message + ); + } + } + } + + function relayFromScroll() internal { + mainnet.selectFork(); + + // Read all L2 -> L1 messages and relay them under Primary fork + // Note: We bypass the L1 messenger relay here because it's easier to not have to generate valid state roots / merkle proofs + Vm.Log[] memory allLogs = vm.getRecordedLogs(); + for (; lastFromScrollLogIndex < allLogs.length; lastFromScrollLogIndex++) { + Vm.Log memory _log = allLogs[lastFromScrollLogIndex]; + if (_log.topics[0] == SENT_MESSAGE_TOPIC && _log.emitter == address(L2_SCROLL_MESSENGER)) { + address sender = address(uint160(uint256(_log.topics[1]))); + address target = address(uint160(uint256(_log.topics[2]))); + (uint256 value, , , bytes memory message) = abi.decode(_log.data, (uint256, uint256, uint256, bytes)); + // Set xDomainMessageSender + vm.store(address(L1_SCROLL_MESSENGER), bytes32(uint256(201)), bytes32(uint256(uint160(sender)))); + vm.startPrank(address(L1_SCROLL_MESSENGER)); + (bool success, bytes memory response) = target.call{value: value}(message); + vm.stopPrank(); + vm.store(address(L1_SCROLL_MESSENGER), bytes32(uint256(201)), bytes32(uint256(1))); + if (!success) { + assembly { + revert(add(response, 32), mload(response)) + } + } + } + } + } + + function upgrade( + bool isMainnet, + address proxy, + address implementation + ) internal { + address admin; + address owner; + if (isMainnet) { + mainnet.selectFork(); + admin = 0xEB803eb3F501998126bf37bB823646Ed3D59d072; + owner = 0x798576400F7D662961BA15C6b3F3d813447a26a6; + } else { + scroll.selectFork(); + admin = 0xA76acF000C890b0DD7AEEf57627d9899F955d026; + owner = 0x13D24a7Ff6F5ec5ff0e9C40Fc3B8C9c01c65437B; + } + + vm.startPrank(owner); + ProxyAdmin(admin).upgrade(ITransparentUpgradeableProxy(proxy), implementation); + vm.stopPrank(); + } +} diff --git a/contracts/src/test/integration/LidoGatewayIntegration.t.sol b/contracts/src/test/integration/LidoGatewayIntegration.t.sol new file mode 100644 index 0000000000..09af34a416 --- /dev/null +++ b/contracts/src/test/integration/LidoGatewayIntegration.t.sol @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; + +import {GatewayIntegrationBase} from "./GatewayIntegrationBase.t.sol"; + +import {IL1ERC20Gateway} from "../../L1/gateways/IL1ERC20Gateway.sol"; +import {IL2ERC20Gateway} from "../../L2/gateways/IL2ERC20Gateway.sol"; +import {L1LidoGateway} from "../../lido/L1LidoGateway.sol"; +import {L2LidoGateway} from "../../lido/L2LidoGateway.sol"; + +interface IWstETH { + function wrap(uint256 _stETHAmount) external returns (uint256); + + function unwrap(uint256 _wstETHAmount) external returns (uint256); + + function getStETHByWstETH(uint256 _wstETHAmount) external view returns (uint256); + + function getWstETHByStETH(uint256 _stETHAmount) external view returns (uint256); + + function stEthPerToken() external view returns (uint256); + + function tokensPerStEth() external view returns (uint256); +} + +contract LidoGatewayIntegrationTest is GatewayIntegrationBase { + address private constant L1_LIDO_GATEWAY = 0x6625C6332c9F91F2D27c304E729B86db87A3f504; + + address private constant L1_STETH = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84; + + address private constant L1_WSTETH = 0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0; + + address private constant L2_LIDO_GATEWAY = 0x8aE8f22226B9d789A36AC81474e633f8bE2856c9; + + address private constant L2_WSTETH = 0xf610A9dfB7C89644979b4A0f27063E9e7d7Cda32; + + function setUp() public { + __GatewayIntegrationBase_setUp(); + + mainnet.selectFork(); + upgrade(true, L1_LIDO_GATEWAY, address(new L1LidoGateway(L1_WSTETH, L2_WSTETH))); + L1LidoGateway(L1_LIDO_GATEWAY).initializeV2(); + + scroll.selectFork(); + upgrade(false, L2_LIDO_GATEWAY, address(new L2LidoGateway(L1_WSTETH, L2_WSTETH))); + L2LidoGateway(L2_LIDO_GATEWAY).initializeV2(); + } + + function testWithoutRouter() public { + depositAndWithdraw(false); + } + + function testWithRouter() public { + depositAndWithdraw(true); + } + + function depositAndWithdraw(bool useRouter) private { + vm.recordLogs(); + + mainnet.selectFork(); + uint256 rate = IWstETH(L1_WSTETH).stEthPerToken(); + + // deposit to get some stETH + (bool succeed, ) = L1_STETH.call{value: 11 * rate}(""); + assertEq(true, succeed); + assertApproxEqAbs(MockERC20(L1_STETH).balanceOf(address(this)), 11 * rate, 10); + + // wrap stETH to wstETH + MockERC20(L1_STETH).approve(L1_WSTETH, 10 * rate); + IWstETH(L1_WSTETH).wrap(10 * rate); + assertApproxEqAbs(MockERC20(L1_WSTETH).balanceOf(address(this)), 10 ether, 10); + + // deposit 1 wstETH + uint256 l1GatewayBalance = MockERC20(L1_WSTETH).balanceOf(L1_LIDO_GATEWAY); + uint256 l1Balance = MockERC20(L1_WSTETH).balanceOf(address(this)); + if (useRouter) { + MockERC20(L1_WSTETH).approve(L1_GATEWAY_ROUTER, 1 ether); + IL1ERC20Gateway(L1_GATEWAY_ROUTER).depositERC20{value: 1 ether}(L1_WSTETH, 1 ether, 400000); + } else { + MockERC20(L1_WSTETH).approve(L1_LIDO_GATEWAY, 1 ether); + IL1ERC20Gateway(L1_LIDO_GATEWAY).depositERC20{value: 1 ether}(L1_WSTETH, 1 ether, 400000); + } + assertEq(l1Balance - 1 ether, MockERC20(L1_WSTETH).balanceOf(address(this))); + assertEq(l1GatewayBalance + 1 ether, MockERC20(L1_WSTETH).balanceOf(L1_LIDO_GATEWAY)); + + // relay message to Scroll and check balance + scroll.selectFork(); + uint256 l2Balance = MockERC20(L2_WSTETH).balanceOf(address(this)); + relayFromMainnet(); + + // withdraw wstETH + scroll.selectFork(); + assertEq(l2Balance + 1 ether, MockERC20(L2_WSTETH).balanceOf(address(this))); + assertEq(0, MockERC20(L2_WSTETH).balanceOf(L2_LIDO_GATEWAY)); + if (useRouter) { + IL2ERC20Gateway(L2_GATEWAY_ROUTER).withdrawERC20(L2_WSTETH, 1 ether, 0); + } else { + IL2ERC20Gateway(L2_LIDO_GATEWAY).withdrawERC20(L2_WSTETH, 1 ether, 0); + } + assertEq(l2Balance, MockERC20(L2_WSTETH).balanceOf(address(this))); + assertEq(0, MockERC20(L2_WSTETH).balanceOf(L2_LIDO_GATEWAY)); + + // relay message to Mainnet and check balance + mainnet.selectFork(); + l1GatewayBalance = MockERC20(L1_WSTETH).balanceOf(L1_LIDO_GATEWAY); + l1Balance = MockERC20(L1_WSTETH).balanceOf(address(this)); + relayFromScroll(); + mainnet.selectFork(); + assertEq(l1Balance + 1 ether, MockERC20(L1_WSTETH).balanceOf(address(this))); + assertEq(l1GatewayBalance - 1 ether, MockERC20(L1_WSTETH).balanceOf(L1_LIDO_GATEWAY)); + } +} diff --git a/contracts/src/test/mocks/MockL1LidoGateway.sol b/contracts/src/test/mocks/MockL1LidoGateway.sol index 56f50b18f8..4b7f8821ab 100644 --- a/contracts/src/test/mocks/MockL1LidoGateway.sol +++ b/contracts/src/test/mocks/MockL1LidoGateway.sol @@ -1,3 +1,7 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + import {L1LidoGateway} from "../../lido/L1LidoGateway.sol"; contract MockL1LidoGateway is L1LidoGateway { diff --git a/contracts/src/test/mocks/MockL2LidoGateway.sol b/contracts/src/test/mocks/MockL2LidoGateway.sol index 24fb29853b..981c82679f 100644 --- a/contracts/src/test/mocks/MockL2LidoGateway.sol +++ b/contracts/src/test/mocks/MockL2LidoGateway.sol @@ -1,3 +1,7 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + import {L2LidoGateway} from "../../lido/L2LidoGateway.sol"; contract MockL2LidoGateway is L2LidoGateway { From 8f04d58b3a22cc479672048a28790f909a3b73dd Mon Sep 17 00:00:00 2001 From: zimpha Date: Fri, 22 Dec 2023 11:18:27 +0800 Subject: [PATCH 5/9] update based on comments; add deploy script --- .../scripts/foundry/DeployLidoGateway.s.sol | 43 +++++++++++++++++++ contracts/src/lido/L1LidoGateway.sol | 18 +++++++- contracts/src/lido/L2LidoGateway.sol | 20 +++++++-- contracts/src/lido/LidoGatewayManager.sol | 29 +++++++++++-- contracts/src/lido/README.md | 34 +++++++-------- contracts/src/test/L1LidoGateway.t.sol | 39 +++++++++++------ contracts/src/test/L2LidoGateway.t.sol | 14 ++++-- .../integration/GatewayIntegrationBase.t.sol | 2 +- .../integration/LidoGatewayIntegration.t.sol | 4 +- 9 files changed, 158 insertions(+), 45 deletions(-) create mode 100644 contracts/scripts/foundry/DeployLidoGateway.s.sol diff --git a/contracts/scripts/foundry/DeployLidoGateway.s.sol b/contracts/scripts/foundry/DeployLidoGateway.s.sol new file mode 100644 index 0000000000..50b5e16c3f --- /dev/null +++ b/contracts/scripts/foundry/DeployLidoGateway.s.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.10; + +import {Script} from "forge-std/Script.sol"; +import {console} from "forge-std/console.sol"; + +import {L1LidoGateway} from "../../src/lido/L1LidoGateway.sol"; +import {L2LidoGateway} from "../../src/lido/L2LidoGateway.sol"; + +// solhint-disable state-visibility +// solhint-disable var-name-mixedcase + +contract DeployLidoGateway is Script { + string NETWORK = vm.envString("NETWORK"); + + uint256 L1_DEPLOYER_PRIVATE_KEY = vm.envUint("L1_DEPLOYER_PRIVATE_KEY"); + + uint256 L2_DEPLOYER_PRIVATE_KEY = vm.envUint("L2_DEPLOYER_PRIVATE_KEY"); + + address L1_WSTETH_ADDR = vm.envAddress("L1_WSTETH_ADDR"); + + address L2_WSTETH_ADDR = vm.envAddress("L2_WSTETH_ADDR"); + + function run() external { + vm.startBroadcast(L2_DEPLOYER_PRIVATE_KEY); + + if (keccak256(abi.encodePacked(NETWORK)) == keccak256(abi.encodePacked("L1"))) { + // depoly l1 lido gateway + L1LidoGateway gateway = new L1LidoGateway(L1_WSTETH_ADDR, L2_WSTETH_ADDR); + logAddress("L1_LIDO_GATEWAY_IMPLEMENTATION_ADDR", address(gateway)); + } else if (keccak256(abi.encodePacked(NETWORK)) == keccak256(abi.encodePacked("L2"))) { + // depoly l2 lido gateway + L2LidoGateway gateway = new L2LidoGateway(L1_WSTETH_ADDR, L2_WSTETH_ADDR); + logAddress("L2_LIDO_GATEWAY_IMPLEMENTATION_ADDR", address(gateway)); + } + + vm.stopBroadcast(); + } + + function logAddress(string memory name, address addr) internal view { + console.log(string(abi.encodePacked(name, "=", vm.toString(address(addr))))); + } +} diff --git a/contracts/src/lido/L1LidoGateway.sol b/contracts/src/lido/L1LidoGateway.sol index ade81f38a0..88eb3b95b8 100644 --- a/contracts/src/lido/L1LidoGateway.sol +++ b/contracts/src/lido/L1LidoGateway.sol @@ -19,6 +19,9 @@ contract L1LidoGateway is L1ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag /// @dev Thrown when deposit zero amount token. error ErrorDepositZeroAmount(); + /// @dev Thrown when deposit erc20 with calldata. + error DepositAndCallIsNotAllowed(); + /************* * Variables * *************/ @@ -49,8 +52,17 @@ contract L1LidoGateway is L1ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag } /// @notice Initialize the storage of L1LidoGateway v2. - function initializeV2() external reinitializer(2) { - __LidoGatewayManager_init(); + /// @param _depositsEnabler The address of user who can enable deposits + /// @param _depositsEnabler The address of user who can disable deposits + /// @param _withdrawalsEnabler The address of user who can enable withdrawals + /// @param _withdrawalsDisabler The address of user who can disable withdrawals + function initializeV2( + address _depositsEnabler, + address _depositsDisabler, + address _withdrawalsEnabler, + address _withdrawalsDisabler + ) external reinitializer(2) { + __LidoGatewayManager_init(_depositsEnabler, _depositsDisabler, _withdrawalsEnabler, _withdrawalsDisabler); } /************************* @@ -73,6 +85,7 @@ contract L1LidoGateway is L1ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag **********************/ /// @inheritdoc L1ERC20Gateway + /// @dev The length of `_data` always be zero, which guarantee by `L2LidoGateway`. function _beforeFinalizeWithdrawERC20( address _l1Token, address _l2Token, @@ -106,6 +119,7 @@ contract L1LidoGateway is L1ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag // 1. Transfer token into this contract. address _from; (_from, _amount, _data) = _transferERC20In(_token, _amount, _data); + if (_data.length != 0) revert DepositAndCallIsNotAllowed(); // 2. Generate message passed to L2LidoGateway. bytes memory _message = abi.encodeCall( diff --git a/contracts/src/lido/L2LidoGateway.sol b/contracts/src/lido/L2LidoGateway.sol index b64d98df23..01e62bc4fe 100644 --- a/contracts/src/lido/L2LidoGateway.sol +++ b/contracts/src/lido/L2LidoGateway.sol @@ -20,6 +20,9 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag /// @dev Thrown when withdraw zero amount token. error ErrorWithdrawZeroAmount(); + /// @dev Thrown when withdraw erc20 with calldata. + error WithdrawAndCallIsNotAllowed(); + /************* * Variables * *************/ @@ -50,8 +53,17 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag } /// @notice Initialize the storage of L2LidoGateway v2. - function initializeV2() external reinitializer(2) { - __LidoGatewayManager_init(); + /// @param _depositsEnabler The address of user who can enable deposits + /// @param _depositsEnabler The address of user who can disable deposits + /// @param _withdrawalsEnabler The address of user who can enable withdrawals + /// @param _withdrawalsDisabler The address of user who can disable withdrawals + function initializeV2( + address _depositsEnabler, + address _depositsDisabler, + address _withdrawalsEnabler, + address _withdrawalsDisabler + ) external reinitializer(2) { + __LidoGatewayManager_init(_depositsEnabler, _depositsDisabler, _withdrawalsEnabler, _withdrawalsDisabler); } /************************* @@ -85,6 +97,7 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag *****************************/ /// @inheritdoc IL2ERC20Gateway + /// @dev The length of `_data` always be zero, which guarantee by `L1LidoGateway`. function finalizeDepositERC20( address _l1Token, address _l2Token, @@ -106,8 +119,6 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag IScrollERC20Upgradeable(_l2Token).mint(_to, _amount); - _doCallback(_to, _data); - emit FinalizeDepositERC20(_l1Token, _l2Token, _from, _to, _amount, _data); } @@ -138,6 +149,7 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag if (router == _from) { (_from, _data) = abi.decode(_data, (address, bytes)); } + if (_data.length != 0) revert WithdrawAndCallIsNotAllowed(); // 2. Burn token. IScrollERC20Upgradeable(_l2Token).burn(_from, _amount); diff --git a/contracts/src/lido/LidoGatewayManager.sol b/contracts/src/lido/LidoGatewayManager.sol index 0216f5f039..a1542d1298 100644 --- a/contracts/src/lido/LidoGatewayManager.sol +++ b/contracts/src/lido/LidoGatewayManager.sol @@ -123,12 +123,35 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { ***************/ /// @notice Initialize the storage of LidoGatewayManager. - function __LidoGatewayManager_init() internal onlyInitializing { - _loadState().isDepositsEnabled = true; + /// @param _depositsEnabler The address of user who can enable deposits + /// @param _depositsEnabler The address of user who can disable deposits + /// @param _withdrawalsEnabler The address of user who can enable withdrawals + /// @param _withdrawalsDisabler The address of user who can disable withdrawals + function __LidoGatewayManager_init( + address _depositsEnabler, + address _depositsDisabler, + address _withdrawalsEnabler, + address _withdrawalsDisabler + ) internal onlyInitializing { + State storage s = _loadState(); + + s.isDepositsEnabled = true; emit DepositsEnabled(_msgSender()); - _loadState().isWithdrawalsEnabled = true; + s.isWithdrawalsEnabled = true; emit WithdrawalsEnabled(_msgSender()); + + s.depositsDisabler = _depositsEnabler; + emit UpdateDepositsEnabler(address(0), _depositsEnabler); + + s.depositsDisabler = _depositsDisabler; + emit UpdateDepositsDisabler(address(0), _depositsDisabler); + + s.withdrawalsEnabler = _withdrawalsEnabler; + emit UpdateWithdrawalsEnabler(address(0), _withdrawalsEnabler); + + s.withdrawalsDisabler = _withdrawalsDisabler; + emit UpdateWithdrawalsDisabler(address(0), _withdrawalsDisabler); } /************************* diff --git a/contracts/src/lido/README.md b/contracts/src/lido/README.md index a70d3ee3bf..28882b60c1 100644 --- a/contracts/src/lido/README.md +++ b/contracts/src/lido/README.md @@ -14,20 +14,20 @@ At this point, the implementation must provide a scalable and reliable solution ## Security surface overview -| Statement | Answer | -| -------------------------------------------------------------------------------------------------------------------------------------------- | ------ | -| It is possible to bridge wstETH forth and back using this bridge | Yes | -| The bridge using a canonical mechanism for message/value passing | Yes | -| The bridge is upgradeable | Yes | -| Upgrade authority for the bridge | TBA | -| Emergency pause/cancel mechanisms and their authorities | TBA | -| The bridged token support permits and ERC-1271 | Yes | -| Are the following things in the scope of this bridge deployment: | | -| - Passing the (w)stETH/USD price feed | No | -| - Passing Lido DAO governance decisions | TBA | -| Bridges are complicated in that the transaction can succeed on one side and fail on the other. What's the handling mechanism for this issue? | TBA | -| Is there a deployment script that sets all the parameters and authorities correctly? | TBA | -| Is there a post-deploy check script that, given a deployment, checks that all parameters and authorities are set correctly? | TBA | +| Statement | Answer | +| -------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| It is possible to bridge wstETH forth and back using this bridge | Yes | +| The bridge using a canonical mechanism for message/value passing | Yes | +| The bridge is upgradeable | Yes | +| Upgrade authority for the bridge | TBA | +| Emergency pause/cancel mechanisms and their authorities | TBA | +| The bridged token support permits and ERC-1271 | No, only permits | +| Are the following things in the scope of this bridge deployment: | | +| - Passing the (w)stETH/USD price feed | No | +| - Passing Lido DAO governance decisions | [Lido DAO Agent](https://etherscan.io/address/0x3e40D73EB977Dc6a537aF587D48316feE66E9C8c) representation [on Scroll (address TBD)] via [ScrollBridgeExecutor](https://github.com/scroll-tech/governance-crosschain-bridges/blob/scroll/contracts/bridges/ScrollBridgeExecutor.sol) | +| Bridges are complicated in that the transaction can succeed on one side and fail on the other. What's the handling mechanism for this issue? | TBA | +| Is there a deployment script that sets all the parameters and authorities correctly? | No, we are upgrading from existing gateway, will need to involve multisig operation by Scroll | +| Is there a post-deploy check script that, given a deployment, checks that all parameters and authorities are set correctly? | No | ## Scroll's Bridging Flow @@ -90,15 +90,15 @@ At any time following invariant must be satisfied: `l1ERC20TokenBridgeBalance == In the case of invariant violation, Lido will have a dispute period to suspend the L1 and L2 bridges. Disabled bridges forbid the minting of L2Token and withdrawal of minted tokens till the resolution of the issue. -### Attack on L1ScrollMessenger +### Attack to L1ScrollMessenger According to the Scroll documentation, `L1ScrollMessenger`: > The L1 Scroll Messenger contract sends messages from L1 to L2 and relays messages from L2 onto L1. -This contract is central in the L2 to L1 communication process since all messages from L2 that passed the challenge period are executed on behalf of this contract. +This contract is central in the L2-to-L1 communication process since all messages from L2 that verified by the zkevm proof are executed on behalf of this contract. -In case of a vulnerability in the `L1ScrollMessenger`, which allows the attacker to send arbitrary messages bypassing the dispute period, an attacker can immediately drain tokens from the L1 bridge. +In case of a vulnerability in the `L1ScrollMessenger`, which allows the attacker to send arbitrary messages bypassing the the zkevm proof, an attacker can immediately drain tokens from the L1 bridge. Additional risk creates the upgradeability of the `L1ScrollMessenger`. Exist a risk of an attack with the replacement of the implementation with some malicious functionality. Such an attack might be reduced to the above vulnerability and steal all locked tokens on the L1 bridge. diff --git a/contracts/src/test/L1LidoGateway.t.sol b/contracts/src/test/L1LidoGateway.t.sol index 93db65eb00..238757f4e8 100644 --- a/contracts/src/test/L1LidoGateway.t.sol +++ b/contracts/src/test/L1LidoGateway.t.sol @@ -61,6 +61,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { error ErrorAccountIsZeroAddress(); error ErrorNonZeroMsgValue(); error ErrorDepositZeroAmount(); + error DepositAndCallIsNotAllowed(); MockL1LidoGateway private gateway; L1GatewayRouter private router; @@ -85,7 +86,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { // Initialize L1 contracts gateway.initialize(address(counterpartGateway), address(router), address(l1Messenger)); - gateway.initializeV2(); + gateway.initializeV2(address(0), address(0), address(0), address(0)); router.initialize(address(0), address(gateway)); // Prepare token balances @@ -116,7 +117,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { gateway.initialize(address(counterpartGateway), address(router), address(l1Messenger)); hevm.expectRevert("Initializable: contract is already initialized"); - gateway.initializeV2(); + gateway.initializeV2(address(0), address(0), address(0), address(0)); } /************************************* @@ -381,24 +382,20 @@ contract L1LidoGatewayTest is L1GatewayTestBase { _depositERC20(true, 2, amount, recipient, dataToCall, gasLimit, feePerGas); } - function testDropMessage( - uint256 amount, - address recipient, - bytes memory dataToCall - ) public { + function testDropMessage(uint256 amount, address recipient) public { hevm.assume(recipient != address(0)); amount = bound(amount, 1, l1Token.balanceOf(address(this))); bytes memory message = abi.encodeCall( IL2ERC20Gateway.finalizeDepositERC20, - (address(l1Token), address(l2Token), address(this), recipient, amount, dataToCall) + (address(l1Token), address(l2Token), address(this), recipient, amount, new bytes(0)) ); - gateway.depositERC20AndCall(address(l1Token), recipient, amount, dataToCall, defaultGasLimit); + gateway.depositERC20AndCall(address(l1Token), recipient, amount, new bytes(0), defaultGasLimit); MockScrollMessenger mockMessenger = new MockScrollMessenger(); MockL1LidoGateway mockGateway = _deployGateway(); mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); - mockGateway.initializeV2(); + mockGateway.initializeV2(address(0), address(0), address(0), address(0)); // revert caller is not messenger hevm.expectRevert("only messenger can call"); @@ -432,7 +429,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { ( abi.encodeCall( IL2ERC20Gateway.finalizeDepositERC20, - (address(l2Token), address(l2Token), address(this), recipient, amount, dataToCall) + (address(l2Token), address(l2Token), address(this), recipient, amount, new bytes(0)) ) ) ) @@ -479,7 +476,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { MockScrollMessenger mockMessenger = new MockScrollMessenger(); MockL1LidoGateway mockGateway = _deployGateway(); mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); - mockGateway.initializeV2(); + mockGateway.initializeV2(address(0), address(0), address(0), address(0)); // revert caller is not messenger hevm.expectRevert("only messenger can call"); @@ -655,7 +652,23 @@ contract L1LidoGatewayTest is L1GatewayTestBase { hevm.expectRevert(ErrorDepositZeroAmount.selector); _invokeDepositERC20Call(useRouter, methodType, address(l1Token), 0, recipient, dataToCall, gasLimit, feePerGas); - // succeed to withdraw + // revert when data is not empty + if (dataToCall.length != 0) { + hevm.expectRevert(DepositAndCallIsNotAllowed.selector); + _invokeDepositERC20Call( + useRouter, + methodType, + address(l1Token), + amount, + recipient, + dataToCall, + gasLimit, + feePerGas + ); + return; + } + + // succeed to deposit bytes memory message = abi.encodeCall( IL2ERC20Gateway.finalizeDepositERC20, (address(l1Token), address(l2Token), address(this), recipient, amount, dataToCall) diff --git a/contracts/src/test/L2LidoGateway.t.sol b/contracts/src/test/L2LidoGateway.t.sol index ed067a5aa9..df061ffc41 100644 --- a/contracts/src/test/L2LidoGateway.t.sol +++ b/contracts/src/test/L2LidoGateway.t.sol @@ -59,6 +59,7 @@ contract L2LidoGatewayTest is L2GatewayTestBase { error ErrorAccountIsZeroAddress(); error ErrorNonZeroMsgValue(); error ErrorWithdrawZeroAmount(); + error WithdrawAndCallIsNotAllowed(); MockL2LidoGateway private gateway; L2GatewayRouter private router; @@ -83,7 +84,7 @@ contract L2LidoGatewayTest is L2GatewayTestBase { // Initialize L2 contracts gateway.initialize(address(counterpartGateway), address(router), address(l2Messenger)); - gateway.initializeV2(); + gateway.initializeV2(address(0), address(0), address(0), address(0)); router.initialize(address(0), address(gateway)); l2Token.initialize("Mock L2", "ML2", 18, address(gateway), address(l1Token)); @@ -116,7 +117,7 @@ contract L2LidoGatewayTest is L2GatewayTestBase { gateway.initialize(address(counterpartGateway), address(router), address(l2Messenger)); hevm.expectRevert("Initializable: contract is already initialized"); - gateway.initializeV2(); + gateway.initializeV2(address(0), address(0), address(0), address(0)); } /************************************* @@ -396,7 +397,7 @@ contract L2LidoGatewayTest is L2GatewayTestBase { MockScrollMessenger mockMessenger = new MockScrollMessenger(); MockL2LidoGateway mockGateway = _deployGateway(); mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); - mockGateway.initializeV2(); + mockGateway.initializeV2(address(0), address(0), address(0), address(0)); // revert caller is not messenger hevm.expectRevert("only messenger can call"); @@ -537,6 +538,13 @@ contract L2LidoGatewayTest is L2GatewayTestBase { hevm.expectRevert(ErrorWithdrawZeroAmount.selector); _invokeWithdrawERC20Call(useRouter, methodType, address(l2Token), 0, recipient, dataToCall, gasLimit); + // revert when data is not empty + if (dataToCall.length != 0) { + hevm.expectRevert(WithdrawAndCallIsNotAllowed.selector); + _invokeWithdrawERC20Call(useRouter, methodType, address(l2Token), amount, recipient, dataToCall, gasLimit); + return; + } + // succeed to withdraw bytes memory message = abi.encodeCall( IL1ERC20Gateway.finalizeWithdrawERC20, diff --git a/contracts/src/test/integration/GatewayIntegrationBase.t.sol b/contracts/src/test/integration/GatewayIntegrationBase.t.sol index af9172f9b9..6ef694952b 100644 --- a/contracts/src/test/integration/GatewayIntegrationBase.t.sol +++ b/contracts/src/test/integration/GatewayIntegrationBase.t.sol @@ -55,7 +55,7 @@ abstract contract GatewayIntegrationBase is Test { address malias = AddressAliasHelper.applyL1ToL2Alias(L1_SCROLL_MESSENGER); - // Read all L1 -> L2 messages and relay them under Optimism fork + // Read all L1 -> L2 messages and relay them under Scroll fork Vm.Log[] memory allLogs = vm.getRecordedLogs(); for (; lastFromMainnetLogIndex < allLogs.length; lastFromMainnetLogIndex++) { Vm.Log memory _log = allLogs[lastFromMainnetLogIndex]; diff --git a/contracts/src/test/integration/LidoGatewayIntegration.t.sol b/contracts/src/test/integration/LidoGatewayIntegration.t.sol index 09af34a416..3d8c4577d1 100644 --- a/contracts/src/test/integration/LidoGatewayIntegration.t.sol +++ b/contracts/src/test/integration/LidoGatewayIntegration.t.sol @@ -41,11 +41,11 @@ contract LidoGatewayIntegrationTest is GatewayIntegrationBase { mainnet.selectFork(); upgrade(true, L1_LIDO_GATEWAY, address(new L1LidoGateway(L1_WSTETH, L2_WSTETH))); - L1LidoGateway(L1_LIDO_GATEWAY).initializeV2(); + L1LidoGateway(L1_LIDO_GATEWAY).initializeV2(address(0), address(0), address(0), address(0)); scroll.selectFork(); upgrade(false, L2_LIDO_GATEWAY, address(new L2LidoGateway(L1_WSTETH, L2_WSTETH))); - L2LidoGateway(L2_LIDO_GATEWAY).initializeV2(); + L2LidoGateway(L2_LIDO_GATEWAY).initializeV2(address(0), address(0), address(0), address(0)); } function testWithoutRouter() public { From 6a8a70292a571625e81795a2994ddebd6b052672 Mon Sep 17 00:00:00 2001 From: zimpha Date: Fri, 22 Dec 2023 17:13:13 +0800 Subject: [PATCH 6/9] add EIP-1271 and role access control --- contracts/src/lido/L2WstETHToken.sol | 49 +++++ contracts/src/lido/LidoGatewayManager.sol | 176 ++++++++------- .../src/test/{ => lido}/L1LidoGateway.t.sol | 200 +++++++----------- .../src/test/{ => lido}/L2LidoGateway.t.sol | 198 ++++++----------- contracts/src/test/lido/L2WstETHToken.t.sol | 69 ++++++ 5 files changed, 362 insertions(+), 330 deletions(-) create mode 100644 contracts/src/lido/L2WstETHToken.sol rename contracts/src/test/{ => lido}/L1LidoGateway.t.sol (80%) rename contracts/src/test/{ => lido}/L2LidoGateway.t.sol (77%) create mode 100644 contracts/src/test/lido/L2WstETHToken.t.sol diff --git a/contracts/src/lido/L2WstETHToken.sol b/contracts/src/lido/L2WstETHToken.sol new file mode 100644 index 0000000000..9b53e69fbf --- /dev/null +++ b/contracts/src/lido/L2WstETHToken.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {IERC20PermitUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/IERC20PermitUpgradeable.sol"; +import {ERC20PermitUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20PermitUpgradeable.sol"; +import {SignatureCheckerUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/cryptography/SignatureCheckerUpgradeable.sol"; + +import {ScrollStandardERC20} from "../libraries/token/ScrollStandardERC20.sol"; + +contract L2WstETHToken is ScrollStandardERC20 { + /************* + * Constants * + *************/ + + /// @dev See {ERC20PermitUpgradeable-_PERMIT_TYPEHASH} + bytes32 private constant _PERMIT_TYPEHASH = + keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); + + /***************************** + * Public Mutating Functions * + *****************************/ + + /// @inheritdoc IERC20PermitUpgradeable + /// + /// @dev The code is copied from `ERC20PermitUpgradeable` with modifications to support ERC-1271. + function permit( + address owner, + address spender, + uint256 value, + uint256 deadline, + uint8 v, + bytes32 r, + bytes32 s + ) public virtual override(ERC20PermitUpgradeable, IERC20PermitUpgradeable) { + require(block.timestamp <= deadline, "ERC20Permit: expired deadline"); + + bytes32 structHash = keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline)); + + bytes32 hash = _hashTypedDataV4(structHash); + + require( + SignatureCheckerUpgradeable.isValidSignatureNow(owner, hash, abi.encodePacked(r, s, v)), + "ERC20Permit: invalid signature" + ); + + _approve(owner, spender, value); + } +} diff --git a/contracts/src/lido/LidoGatewayManager.sol b/contracts/src/lido/LidoGatewayManager.sol index a1542d1298..cb3c228d40 100644 --- a/contracts/src/lido/LidoGatewayManager.sol +++ b/contracts/src/lido/LidoGatewayManager.sol @@ -2,11 +2,15 @@ pragma solidity =0.8.16; +import {EnumerableSetUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/structs/EnumerableSetUpgradeable.sol"; + import {ScrollGatewayBase} from "../libraries/gateway/ScrollGatewayBase.sol"; // solhint-disable func-name-mixedcase abstract contract LidoGatewayManager is ScrollGatewayBase { + using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet; + /********** * Events * **********/ @@ -27,25 +31,19 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { /// @param disabler The address of caller. event WithdrawalsDisabled(address indexed disabler); - /// @notice Emitted when the deposits enabler is updated. - /// @param oldEnabler The address of the previous deposits enabler. - /// @param newEnabler The address of the current deposits enabler. - event UpdateDepositsEnabler(address indexed oldEnabler, address indexed newEnabler); - - /// @notice Emitted when the deposits disabler is updated. - /// @param oldDisabler The address of the previous deposits disabler. - /// @param newDisabler The address of the current deposits disabler. - event UpdateDepositsDisabler(address indexed oldDisabler, address indexed newDisabler); + /// @notice Emitted when `account` is granted `role`. + /// + /// @param role The role granted. + /// @param account The address of account to grant the role. + /// @param sender The address of owner. + event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender); - /// @notice Emitted when the withdrawals enabler is updated. - /// @param oldEnabler The address of the previous withdrawals enabler. - /// @param newEnabler The address of the current withdrawals enabler. - event UpdateWithdrawalsEnabler(address indexed oldEnabler, address indexed newEnabler); - - /// @notice Emitted when the withdrawals disabler is updated. - /// @param oldDisabler The address of the previous withdrawals disabler. - /// @param newDisabler The address of the current withdrawals disabler. - event UpdateWithdrawalsDisabler(address indexed oldDisabler, address indexed newDisabler); + /// @notice Emitted when `account` is revoked `role`. + /// + /// @param role The role revoked. + /// @param account The address of account to revoke the role. + /// @param sender The address of owner. + event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender); /********** * Errors * @@ -89,10 +87,7 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { struct State { bool isDepositsEnabled; bool isWithdrawalsEnabled; - address depositsEnabler; - address depositsDisabler; - address withdrawalsEnabler; - address withdrawalsDisabler; + mapping(bytes32 => EnumerableSetUpgradeable.AddressSet) roles; } /************* @@ -102,6 +97,18 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { /// @dev The location of the slot with State bytes32 private constant STATE_SLOT = keccak256("LidoGatewayManager.bridgingState"); + /// @notice The role for deposits enabler. + bytes32 public constant DEPOSITS_ENABLER_ROLE = keccak256("BridgingManager.DEPOSITS_ENABLER_ROLE"); + + /// @notice The role for deposits disabler. + bytes32 public constant DEPOSITS_DISABLER_ROLE = keccak256("BridgingManager.DEPOSITS_DISABLER_ROLE"); + + /// @notice The role for withdrawals enabler. + bytes32 public constant WITHDRAWALS_ENABLER_ROLE = keccak256("BridgingManager.WITHDRAWALS_ENABLER_ROLE"); + + /// @notice The role for withdrawals disabler. + bytes32 public constant WITHDRAWALS_DISABLER_ROLE = keccak256("BridgingManager.WITHDRAWALS_DISABLER_ROLE"); + /********************** * Function Modifiers * **********************/ @@ -141,17 +148,10 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { s.isWithdrawalsEnabled = true; emit WithdrawalsEnabled(_msgSender()); - s.depositsDisabler = _depositsEnabler; - emit UpdateDepositsEnabler(address(0), _depositsEnabler); - - s.depositsDisabler = _depositsDisabler; - emit UpdateDepositsDisabler(address(0), _depositsDisabler); - - s.withdrawalsEnabler = _withdrawalsEnabler; - emit UpdateWithdrawalsEnabler(address(0), _withdrawalsEnabler); - - s.withdrawalsDisabler = _withdrawalsDisabler; - emit UpdateWithdrawalsDisabler(address(0), _withdrawalsDisabler); + _grantRole(DEPOSITS_ENABLER_ROLE, _depositsEnabler); + _grantRole(DEPOSITS_DISABLER_ROLE, _depositsDisabler); + _grantRole(WITHDRAWALS_ENABLER_ROLE, _withdrawalsEnabler); + _grantRole(WITHDRAWALS_DISABLER_ROLE, _withdrawalsDisabler); } /************************* @@ -168,6 +168,28 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { return _loadState().isWithdrawalsEnabled; } + /// @notice Returns `true` if `_account` has been granted `_role`. + function hasRole(bytes32 _role, address _account) public view returns (bool) { + return _loadState().roles[_role].contains(_account); + } + + /// @notice Returns one of the accounts that have `_role`. + /// + /// @param _role The role to query. + /// @param _index The index of account to query. It must be a value between 0 and {getRoleMemberCount}, non-inclusive. + function getRoleMember(bytes32 _role, uint256 _index) external view returns (address) { + return _loadState().roles[_role].at(_index); + } + + /// @notice Returns the number of accounts that have `role`. + /// + /// @dev Can be used together with {getRoleMember} to enumerate all bearers of a role. + /// + /// @param _role The role to query. + function getRoleMemberCount(bytes32 _role) external view returns (uint256) { + return _loadState().roles[_role].length(); + } + /************************ * Restricted Functions * ************************/ @@ -175,7 +197,9 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { /// @notice Enables the deposits if they are disabled function enableDeposits() external { if (isDepositsEnabled()) revert ErrorDepositsEnabled(); - if (_msgSender() != _loadState().depositsEnabler) revert ErrorCallerIsNotDepositsEnabler(); + if (!hasRole(DEPOSITS_ENABLER_ROLE, _msgSender())) { + revert ErrorCallerIsNotDepositsEnabler(); + } _loadState().isDepositsEnabled = true; emit DepositsEnabled(_msgSender()); @@ -183,7 +207,9 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { /// @notice Disables the deposits if they aren't disabled yet function disableDeposits() external whenDepositsEnabled { - if (_msgSender() != _loadState().depositsDisabler) revert ErrorCallerIsNotDepositsDisabler(); + if (!hasRole(DEPOSITS_DISABLER_ROLE, _msgSender())) { + revert ErrorCallerIsNotDepositsDisabler(); + } _loadState().isDepositsEnabled = false; emit DepositsDisabled(_msgSender()); @@ -192,7 +218,9 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { /// @notice Enables the withdrawals if they are disabled function enableWithdrawals() external { if (isWithdrawalsEnabled()) revert ErrorWithdrawalsEnabled(); - if (_msgSender() != _loadState().withdrawalsEnabler) revert ErrorCallerIsNotWithdrawalsEnabler(); + if (!hasRole(WITHDRAWALS_ENABLER_ROLE, _msgSender())) { + revert ErrorCallerIsNotWithdrawalsEnabler(); + } _loadState().isWithdrawalsEnabled = true; emit WithdrawalsEnabled(_msgSender()); @@ -200,50 +228,30 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { /// @notice Disables the withdrawals if they aren't disabled yet function disableWithdrawals() external whenWithdrawalsEnabled { - if (_msgSender() != _loadState().withdrawalsDisabler) revert ErrorCallerIsNotWithdrawalsDisabler(); + if (!hasRole(WITHDRAWALS_DISABLER_ROLE, _msgSender())) { + revert ErrorCallerIsNotWithdrawalsDisabler(); + } _loadState().isWithdrawalsEnabled = false; emit WithdrawalsDisabled(_msgSender()); } - /// @notice Update the address of deposits enabler. - /// @param _newEnabler The address of new deposits enabler. - function updateDepositsEnabler(address _newEnabler) external onlyOwner { - State storage s = _loadState(); - address _oldEnabler = s.depositsEnabler; - s.depositsEnabler = _newEnabler; - - emit UpdateDepositsEnabler(_oldEnabler, _newEnabler); - } - - /// @notice Update the address of deposits disabler. - /// @param _newDisabler The address of new deposits disabler. - function updateDepositsDisabler(address _newDisabler) external onlyOwner { - State storage s = _loadState(); - address _oldDisabler = s.depositsDisabler; - s.depositsDisabler = _newDisabler; - - emit UpdateDepositsDisabler(_oldDisabler, _newDisabler); + /// @notice Grants `_role` from `_account`. + /// If `account` had been granted `role`, emits a {RoleGranted} event. + /// + /// @param _role The role to grant. + /// @param _account The address of account to grant. + function grantRole(bytes32 _role, address _account) external onlyOwner { + _grantRole(_role, _account); } - /// @notice Update the address of withdrawals enabler. - /// @param _newEnabler The address of new withdrawals enabler. - function updateWithdrawalsEnabler(address _newEnabler) external onlyOwner { - State storage s = _loadState(); - address _oldEnabler = s.withdrawalsEnabler; - s.withdrawalsEnabler = _newEnabler; - - emit UpdateWithdrawalsEnabler(_oldEnabler, _newEnabler); - } - - /// @notice Update the address of withdrawals disabler. - /// @param _newDisabler The address of new withdrawals disabler. - function updateWithdrawalsDisabler(address _newDisabler) external onlyOwner { - State storage s = _loadState(); - address _oldDisabler = s.withdrawalsDisabler; - s.withdrawalsDisabler = _newDisabler; - - emit UpdateWithdrawalsDisabler(_oldDisabler, _newDisabler); + /// @notice Revokes `_role` from `_account`. + /// If `account` had been granted `role`, emits a {RoleRevoked} event. + /// + /// @param _role The role to revoke. + /// @param _account The address of account to revoke. + function revokeRole(bytes32 _role, address _account) external onlyOwner { + _revokeRole(_role, _account); } /********************** @@ -258,4 +266,26 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { r.slot := slot } } + + /// @dev Internal function to grant `_role` from `_account`. + /// If `account` had been granted `role`, emits a {RoleGranted} event. + /// + /// @param _role The role to grant. + /// @param _account The address of account to grant. + function _grantRole(bytes32 _role, address _account) internal { + if (_loadState().roles[_role].add(_account)) { + emit RoleGranted(_role, _account, _msgSender()); + } + } + + /// @dev Internal function to revoke `_role` from `_account`. + /// If `account` had been granted `role`, emits a {RoleRevoked} event. + /// + /// @param _role The role to revoke. + /// @param _account The address of account to revoke. + function _revokeRole(bytes32 _role, address _account) internal { + if (_loadState().roles[_role].remove(_account)) { + emit RoleRevoked(_role, _account, _msgSender()); + } + } } diff --git a/contracts/src/test/L1LidoGateway.t.sol b/contracts/src/test/lido/L1LidoGateway.t.sol similarity index 80% rename from contracts/src/test/L1LidoGateway.t.sol rename to contracts/src/test/lido/L1LidoGateway.t.sol index 238757f4e8..5e276554a0 100644 --- a/contracts/src/test/L1LidoGateway.t.sol +++ b/contracts/src/test/lido/L1LidoGateway.t.sol @@ -6,18 +6,18 @@ import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; -import {IL1ERC20Gateway} from "../L1/gateways/IL1ERC20Gateway.sol"; -import {L1GatewayRouter} from "../L1/gateways/L1GatewayRouter.sol"; -import {IL1ScrollMessenger} from "../L1/IL1ScrollMessenger.sol"; -import {IL2ERC20Gateway} from "../L2/gateways/IL2ERC20Gateway.sol"; -import {AddressAliasHelper} from "../libraries/common/AddressAliasHelper.sol"; -import {ScrollConstants} from "../libraries/constants/ScrollConstants.sol"; -import {L2LidoGateway} from "../lido/L2LidoGateway.sol"; - -import {L1GatewayTestBase} from "./L1GatewayTestBase.t.sol"; -import {MockL1LidoGateway} from "./mocks/MockL1LidoGateway.sol"; -import {MockScrollMessenger} from "./mocks/MockScrollMessenger.sol"; -import {MockGatewayRecipient} from "./mocks/MockGatewayRecipient.sol"; +import {IL1ERC20Gateway} from "../../L1/gateways/IL1ERC20Gateway.sol"; +import {L1GatewayRouter} from "../../L1/gateways/L1GatewayRouter.sol"; +import {IL1ScrollMessenger} from "../../L1/IL1ScrollMessenger.sol"; +import {IL2ERC20Gateway} from "../../L2/gateways/IL2ERC20Gateway.sol"; +import {AddressAliasHelper} from "../../libraries/common/AddressAliasHelper.sol"; +import {ScrollConstants} from "../../libraries/constants/ScrollConstants.sol"; +import {L2LidoGateway} from "../../lido/L2LidoGateway.sol"; + +import {L1GatewayTestBase} from "../L1GatewayTestBase.t.sol"; +import {MockL1LidoGateway} from "../mocks/MockL1LidoGateway.sol"; +import {MockScrollMessenger} from "../mocks/MockScrollMessenger.sol"; +import {MockGatewayRecipient} from "../mocks/MockGatewayRecipient.sol"; contract L1LidoGatewayTest is L1GatewayTestBase { // events from L1LidoGateway @@ -42,10 +42,8 @@ contract L1LidoGatewayTest is L1GatewayTestBase { event DepositsDisabled(address indexed disabler); event WithdrawalsEnabled(address indexed enabler); event WithdrawalsDisabled(address indexed disabler); - event UpdateDepositsEnabler(address indexed oldEnabler, address indexed newEnabler); - event UpdateDepositsDisabler(address indexed oldDisabler, address indexed newDisabler); - event UpdateWithdrawalsEnabler(address indexed oldEnabler, address indexed newEnabler); - event UpdateWithdrawalsDisabler(address indexed oldDisabler, address indexed newDisabler); + event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender); + event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender); // errors from L1LidoGateway error ErrorDepositsEnabled(); @@ -118,6 +116,11 @@ contract L1LidoGatewayTest is L1GatewayTestBase { hevm.expectRevert("Initializable: contract is already initialized"); gateway.initializeV2(address(0), address(0), address(0), address(0)); + + gateway.revokeRole(gateway.DEPOSITS_ENABLER_ROLE(), address(0)); + gateway.revokeRole(gateway.DEPOSITS_DISABLER_ROLE(), address(0)); + gateway.revokeRole(gateway.WITHDRAWALS_ENABLER_ROLE(), address(0)); + gateway.revokeRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(0)); } /************************************* @@ -130,13 +133,13 @@ contract L1LidoGatewayTest is L1GatewayTestBase { gateway.enableDeposits(); // revert when caller is not deposits enabler - gateway.updateDepositsDisabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); gateway.disableDeposits(); hevm.expectRevert(ErrorCallerIsNotDepositsEnabler.selector); gateway.enableDeposits(); // succeed - gateway.updateDepositsEnabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_ENABLER_ROLE(), address(this)); assertBoolEq(false, gateway.isDepositsEnabled()); hevm.expectEmit(true, false, false, true); emit DepositsEnabled(address(this)); @@ -146,22 +149,22 @@ contract L1LidoGatewayTest is L1GatewayTestBase { function testDisableDeposits() external { // revert when already disabled - gateway.updateDepositsDisabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); gateway.disableDeposits(); assertBoolEq(false, gateway.isDepositsEnabled()); hevm.expectRevert(ErrorDepositsDisabled.selector); gateway.disableDeposits(); // revert when caller is not deposits disabler - gateway.updateDepositsEnabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_ENABLER_ROLE(), address(this)); gateway.enableDeposits(); assertBoolEq(true, gateway.isDepositsEnabled()); - gateway.updateDepositsDisabler(address(0)); + gateway.revokeRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); hevm.expectRevert(ErrorCallerIsNotDepositsDisabler.selector); gateway.disableDeposits(); // succeed - gateway.updateDepositsDisabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); assertBoolEq(true, gateway.isDepositsEnabled()); hevm.expectEmit(true, false, false, true); emit DepositsDisabled(address(this)); @@ -175,13 +178,13 @@ contract L1LidoGatewayTest is L1GatewayTestBase { gateway.enableWithdrawals(); // revert when caller is not deposits enabler - gateway.updateWithdrawalsDisabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); gateway.disableWithdrawals(); hevm.expectRevert(ErrorCallerIsNotWithdrawalsEnabler.selector); gateway.enableWithdrawals(); // succeed - gateway.updateWithdrawalsEnabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_ENABLER_ROLE(), address(this)); assertBoolEq(false, gateway.isWithdrawalsEnabled()); hevm.expectEmit(true, false, false, true); emit WithdrawalsEnabled(address(this)); @@ -191,22 +194,22 @@ contract L1LidoGatewayTest is L1GatewayTestBase { function testDisableWithdrawals() external { // revert when already disabled - gateway.updateWithdrawalsDisabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); gateway.disableWithdrawals(); assertBoolEq(false, gateway.isWithdrawalsEnabled()); hevm.expectRevert(ErrorWithdrawalsDisabled.selector); gateway.disableWithdrawals(); // revert when caller is not deposits disabler - gateway.updateWithdrawalsEnabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_ENABLER_ROLE(), address(this)); gateway.enableWithdrawals(); assertBoolEq(true, gateway.isWithdrawalsEnabled()); - gateway.updateWithdrawalsDisabler(address(0)); + gateway.revokeRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); hevm.expectRevert(ErrorCallerIsNotWithdrawalsDisabler.selector); gateway.disableWithdrawals(); // succeed - gateway.updateWithdrawalsDisabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); assertBoolEq(true, gateway.isWithdrawalsEnabled()); hevm.expectEmit(true, false, false, true); emit WithdrawalsDisabled(address(this)); @@ -214,114 +217,53 @@ contract L1LidoGatewayTest is L1GatewayTestBase { assertBoolEq(false, gateway.isWithdrawalsEnabled()); } - function testUpdateDepositsEnabler(address _enabler) external { - hevm.assume(_enabler != address(0)); - - // revert caller is not owner - hevm.startPrank(address(1)); - hevm.expectRevert("Ownable: caller is not the owner"); - gateway.updateDepositsEnabler(address(0)); - hevm.stopPrank(); - - gateway.updateDepositsDisabler(address(this)); - gateway.disableDeposits(); - - // succeed to set - hevm.startPrank(_enabler); - hevm.expectRevert(ErrorCallerIsNotDepositsEnabler.selector); - gateway.enableDeposits(); - hevm.stopPrank(); - - hevm.expectEmit(true, true, false, true); - emit UpdateDepositsEnabler(address(0), _enabler); - gateway.updateDepositsEnabler(_enabler); - - assertBoolEq(false, gateway.isDepositsEnabled()); - hevm.startPrank(_enabler); - gateway.enableDeposits(); - hevm.stopPrank(); - assertBoolEq(true, gateway.isDepositsEnabled()); - } - - function testUpdateDepositsDisabler(address _disabler) external { - hevm.assume(_disabler != address(0)); - - // revert caller is not owner + function testGrantRole(bytes32 _role, address _account) external { + // revert not owner hevm.startPrank(address(1)); hevm.expectRevert("Ownable: caller is not the owner"); - gateway.updateDepositsDisabler(address(0)); - hevm.stopPrank(); - - // succeed to set - hevm.startPrank(_disabler); - hevm.expectRevert(ErrorCallerIsNotDepositsDisabler.selector); - gateway.disableDeposits(); + gateway.grantRole(_role, _account); hevm.stopPrank(); - hevm.expectEmit(true, true, false, true); - emit UpdateDepositsDisabler(address(0), _disabler); - gateway.updateDepositsDisabler(_disabler); - - assertBoolEq(true, gateway.isDepositsEnabled()); - hevm.startPrank(_disabler); - gateway.disableDeposits(); - hevm.stopPrank(); - assertBoolEq(false, gateway.isDepositsEnabled()); - } - - function testUpdateWithdrawalsEnabler(address _enabler) external { - hevm.assume(_enabler != address(0)); - - // revert caller is not owner - hevm.startPrank(address(1)); - hevm.expectRevert("Ownable: caller is not the owner"); - gateway.updateWithdrawalsEnabler(address(0)); - hevm.stopPrank(); - - gateway.updateWithdrawalsDisabler(address(this)); - gateway.disableWithdrawals(); - - // succeed to set - hevm.startPrank(_enabler); - hevm.expectRevert(ErrorCallerIsNotWithdrawalsEnabler.selector); - gateway.enableWithdrawals(); - hevm.stopPrank(); - - hevm.expectEmit(true, true, false, true); - emit UpdateWithdrawalsEnabler(address(0), _enabler); - gateway.updateWithdrawalsEnabler(_enabler); - - assertBoolEq(false, gateway.isWithdrawalsEnabled()); - hevm.startPrank(_enabler); - gateway.enableWithdrawals(); - hevm.stopPrank(); - assertBoolEq(true, gateway.isWithdrawalsEnabled()); + // succeed + assertBoolEq(gateway.hasRole(_role, _account), false); + hevm.expectEmit(true, true, true, true); + emit RoleGranted(_role, _account, address(this)); + gateway.grantRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), true); + assertEq(gateway.getRoleMemberCount(_role), 1); + assertEq(gateway.getRoleMember(_role, 0), _account); + + // do nothing regrant + gateway.grantRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), true); + assertEq(gateway.getRoleMemberCount(_role), 1); + assertEq(gateway.getRoleMember(_role, 0), _account); } - function testUpdateWithdrawalsDisabler(address _disabler) external { - hevm.assume(_disabler != address(0)); - - // revert caller is not owner + function testRevokeRole(bytes32 _role, address _account) external { + // revert not owner hevm.startPrank(address(1)); hevm.expectRevert("Ownable: caller is not the owner"); - gateway.updateWithdrawalsDisabler(address(0)); + gateway.revokeRole(_role, _account); hevm.stopPrank(); - // succeed to set - hevm.startPrank(_disabler); - hevm.expectRevert(ErrorCallerIsNotWithdrawalsDisabler.selector); - gateway.disableWithdrawals(); - hevm.stopPrank(); + // grant first + gateway.grantRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), true); + assertEq(gateway.getRoleMemberCount(_role), 1); + assertEq(gateway.getRoleMember(_role, 0), _account); - hevm.expectEmit(true, true, false, true); - emit UpdateWithdrawalsDisabler(address(0), _disabler); - gateway.updateWithdrawalsDisabler(_disabler); - - assertBoolEq(true, gateway.isWithdrawalsEnabled()); - hevm.startPrank(_disabler); - gateway.disableWithdrawals(); - hevm.stopPrank(); - assertBoolEq(false, gateway.isWithdrawalsEnabled()); + // revoke + hevm.expectEmit(true, true, true, true); + emit RoleRevoked(_role, _account, address(this)); + gateway.revokeRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), false); + assertEq(gateway.getRoleMemberCount(_role), 0); + + // revoke again + gateway.revokeRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), false); + assertEq(gateway.getRoleMemberCount(_role), 0); } /******************************** @@ -522,13 +464,13 @@ contract L1LidoGatewayTest is L1GatewayTestBase { ); // revert when withdrawals disabled - mockGateway.updateWithdrawalsDisabler(address(this)); + mockGateway.grantRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); mockGateway.disableWithdrawals(); hevm.expectRevert(ErrorWithdrawalsDisabled.selector); mockMessenger.callTarget(address(mockGateway), message); // revert when nonzero msg.value - mockGateway.updateWithdrawalsEnabler(address(this)); + mockGateway.grantRole(gateway.WITHDRAWALS_ENABLER_ROLE(), address(this)); mockGateway.enableWithdrawals(); hevm.expectRevert(ErrorNonZeroMsgValue.selector); mockMessenger.callTarget{value: 1}(address(mockGateway), message); @@ -632,7 +574,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { } // revert when deposits disabled - gateway.updateDepositsDisabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); gateway.disableDeposits(); hevm.expectRevert(ErrorDepositsDisabled.selector); _invokeDepositERC20Call( @@ -647,7 +589,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { ); // revert when deposit zero amount - gateway.updateDepositsEnabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_ENABLER_ROLE(), address(this)); gateway.enableDeposits(); hevm.expectRevert(ErrorDepositZeroAmount.selector); _invokeDepositERC20Call(useRouter, methodType, address(l1Token), 0, recipient, dataToCall, gasLimit, feePerGas); diff --git a/contracts/src/test/L2LidoGateway.t.sol b/contracts/src/test/lido/L2LidoGateway.t.sol similarity index 77% rename from contracts/src/test/L2LidoGateway.t.sol rename to contracts/src/test/lido/L2LidoGateway.t.sol index df061ffc41..1ede36c51a 100644 --- a/contracts/src/test/L2LidoGateway.t.sol +++ b/contracts/src/test/lido/L2LidoGateway.t.sol @@ -6,17 +6,17 @@ import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; -import {IL1ERC20Gateway} from "../L1/gateways/IL1ERC20Gateway.sol"; -import {IL2ERC20Gateway} from "../L2/gateways/IL2ERC20Gateway.sol"; -import {L2GatewayRouter} from "../L2/gateways/L2GatewayRouter.sol"; -import {AddressAliasHelper} from "../libraries/common/AddressAliasHelper.sol"; -import {ScrollStandardERC20} from "../libraries/token/ScrollStandardERC20.sol"; -import {L1LidoGateway} from "../lido/L1LidoGateway.sol"; - -import {L2GatewayTestBase} from "./L2GatewayTestBase.t.sol"; -import {MockGatewayRecipient} from "./mocks/MockGatewayRecipient.sol"; -import {MockL2LidoGateway} from "./mocks/MockL2LidoGateway.sol"; -import {MockScrollMessenger} from "./mocks/MockScrollMessenger.sol"; +import {IL1ERC20Gateway} from "../../L1/gateways/IL1ERC20Gateway.sol"; +import {IL2ERC20Gateway} from "../../L2/gateways/IL2ERC20Gateway.sol"; +import {L2GatewayRouter} from "../../L2/gateways/L2GatewayRouter.sol"; +import {AddressAliasHelper} from "../../libraries/common/AddressAliasHelper.sol"; +import {ScrollStandardERC20} from "../../libraries/token/ScrollStandardERC20.sol"; +import {L1LidoGateway} from "../../lido/L1LidoGateway.sol"; + +import {L2GatewayTestBase} from "../L2GatewayTestBase.t.sol"; +import {MockGatewayRecipient} from "../mocks/MockGatewayRecipient.sol"; +import {MockL2LidoGateway} from "../mocks/MockL2LidoGateway.sol"; +import {MockScrollMessenger} from "../mocks/MockScrollMessenger.sol"; contract L2LidoGatewayTest is L2GatewayTestBase { // events from L2LidoGateway @@ -40,10 +40,8 @@ contract L2LidoGatewayTest is L2GatewayTestBase { event DepositsDisabled(address indexed disabler); event WithdrawalsEnabled(address indexed enabler); event WithdrawalsDisabled(address indexed disabler); - event UpdateDepositsEnabler(address indexed oldEnabler, address indexed newEnabler); - event UpdateDepositsDisabler(address indexed oldDisabler, address indexed newDisabler); - event UpdateWithdrawalsEnabler(address indexed oldEnabler, address indexed newEnabler); - event UpdateWithdrawalsDisabler(address indexed oldDisabler, address indexed newDisabler); + event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender); + event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender); // errors from L2LidoGateway error ErrorDepositsEnabled(); @@ -92,6 +90,11 @@ contract L2LidoGatewayTest is L2GatewayTestBase { hevm.startPrank(address(gateway)); l2Token.mint(address(this), type(uint128).max); hevm.stopPrank(); + + gateway.revokeRole(gateway.DEPOSITS_ENABLER_ROLE(), address(0)); + gateway.revokeRole(gateway.DEPOSITS_DISABLER_ROLE(), address(0)); + gateway.revokeRole(gateway.WITHDRAWALS_ENABLER_ROLE(), address(0)); + gateway.revokeRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(0)); } function testInitialized() external { @@ -130,13 +133,13 @@ contract L2LidoGatewayTest is L2GatewayTestBase { gateway.enableDeposits(); // revert when caller is not deposits enabler - gateway.updateDepositsDisabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); gateway.disableDeposits(); hevm.expectRevert(ErrorCallerIsNotDepositsEnabler.selector); gateway.enableDeposits(); // succeed - gateway.updateDepositsEnabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_ENABLER_ROLE(), address(this)); assertBoolEq(false, gateway.isDepositsEnabled()); hevm.expectEmit(true, false, false, true); emit DepositsEnabled(address(this)); @@ -146,22 +149,22 @@ contract L2LidoGatewayTest is L2GatewayTestBase { function testDisableDeposits() external { // revert when already disabled - gateway.updateDepositsDisabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); gateway.disableDeposits(); assertBoolEq(false, gateway.isDepositsEnabled()); hevm.expectRevert(ErrorDepositsDisabled.selector); gateway.disableDeposits(); // revert when caller is not deposits disabler - gateway.updateDepositsEnabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_ENABLER_ROLE(), address(this)); gateway.enableDeposits(); assertBoolEq(true, gateway.isDepositsEnabled()); - gateway.updateDepositsDisabler(address(0)); + gateway.revokeRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); hevm.expectRevert(ErrorCallerIsNotDepositsDisabler.selector); gateway.disableDeposits(); // succeed - gateway.updateDepositsDisabler(address(this)); + gateway.grantRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); assertBoolEq(true, gateway.isDepositsEnabled()); hevm.expectEmit(true, false, false, true); emit DepositsDisabled(address(this)); @@ -175,13 +178,13 @@ contract L2LidoGatewayTest is L2GatewayTestBase { gateway.enableWithdrawals(); // revert when caller is not deposits enabler - gateway.updateWithdrawalsDisabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); gateway.disableWithdrawals(); hevm.expectRevert(ErrorCallerIsNotWithdrawalsEnabler.selector); gateway.enableWithdrawals(); // succeed - gateway.updateWithdrawalsEnabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_ENABLER_ROLE(), address(this)); assertBoolEq(false, gateway.isWithdrawalsEnabled()); hevm.expectEmit(true, false, false, true); emit WithdrawalsEnabled(address(this)); @@ -191,22 +194,22 @@ contract L2LidoGatewayTest is L2GatewayTestBase { function testDisableWithdrawals() external { // revert when already disabled - gateway.updateWithdrawalsDisabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); gateway.disableWithdrawals(); assertBoolEq(false, gateway.isWithdrawalsEnabled()); hevm.expectRevert(ErrorWithdrawalsDisabled.selector); gateway.disableWithdrawals(); // revert when caller is not deposits disabler - gateway.updateWithdrawalsEnabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_ENABLER_ROLE(), address(this)); gateway.enableWithdrawals(); assertBoolEq(true, gateway.isWithdrawalsEnabled()); - gateway.updateWithdrawalsDisabler(address(0)); + gateway.revokeRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); hevm.expectRevert(ErrorCallerIsNotWithdrawalsDisabler.selector); gateway.disableWithdrawals(); // succeed - gateway.updateWithdrawalsDisabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); assertBoolEq(true, gateway.isWithdrawalsEnabled()); hevm.expectEmit(true, false, false, true); emit WithdrawalsDisabled(address(this)); @@ -214,114 +217,53 @@ contract L2LidoGatewayTest is L2GatewayTestBase { assertBoolEq(false, gateway.isWithdrawalsEnabled()); } - function testUpdateDepositsEnabler(address _enabler) external { - hevm.assume(_enabler != address(0)); - - // revert caller is not owner - hevm.startPrank(address(1)); - hevm.expectRevert("Ownable: caller is not the owner"); - gateway.updateDepositsEnabler(address(0)); - hevm.stopPrank(); - - gateway.updateDepositsDisabler(address(this)); - gateway.disableDeposits(); - - // succeed to set - hevm.startPrank(_enabler); - hevm.expectRevert(ErrorCallerIsNotDepositsEnabler.selector); - gateway.enableDeposits(); - hevm.stopPrank(); - - hevm.expectEmit(true, true, false, true); - emit UpdateDepositsEnabler(address(0), _enabler); - gateway.updateDepositsEnabler(_enabler); - - assertBoolEq(false, gateway.isDepositsEnabled()); - hevm.startPrank(_enabler); - gateway.enableDeposits(); - hevm.stopPrank(); - assertBoolEq(true, gateway.isDepositsEnabled()); - } - - function testUpdateDepositsDisabler(address _disabler) external { - hevm.assume(_disabler != address(0)); - - // revert caller is not owner + function testGrantRole(bytes32 _role, address _account) external { + // revert not owner hevm.startPrank(address(1)); hevm.expectRevert("Ownable: caller is not the owner"); - gateway.updateDepositsDisabler(address(0)); - hevm.stopPrank(); - - // succeed to set - hevm.startPrank(_disabler); - hevm.expectRevert(ErrorCallerIsNotDepositsDisabler.selector); - gateway.disableDeposits(); + gateway.grantRole(_role, _account); hevm.stopPrank(); - hevm.expectEmit(true, true, false, true); - emit UpdateDepositsDisabler(address(0), _disabler); - gateway.updateDepositsDisabler(_disabler); - - assertBoolEq(true, gateway.isDepositsEnabled()); - hevm.startPrank(_disabler); - gateway.disableDeposits(); - hevm.stopPrank(); - assertBoolEq(false, gateway.isDepositsEnabled()); - } - - function testUpdateWithdrawalsEnabler(address _enabler) external { - hevm.assume(_enabler != address(0)); - - // revert caller is not owner - hevm.startPrank(address(1)); - hevm.expectRevert("Ownable: caller is not the owner"); - gateway.updateWithdrawalsEnabler(address(0)); - hevm.stopPrank(); - - gateway.updateWithdrawalsDisabler(address(this)); - gateway.disableWithdrawals(); - - // succeed to set - hevm.startPrank(_enabler); - hevm.expectRevert(ErrorCallerIsNotWithdrawalsEnabler.selector); - gateway.enableWithdrawals(); - hevm.stopPrank(); - - hevm.expectEmit(true, true, false, true); - emit UpdateWithdrawalsEnabler(address(0), _enabler); - gateway.updateWithdrawalsEnabler(_enabler); - - assertBoolEq(false, gateway.isWithdrawalsEnabled()); - hevm.startPrank(_enabler); - gateway.enableWithdrawals(); - hevm.stopPrank(); - assertBoolEq(true, gateway.isWithdrawalsEnabled()); + // succeed + assertBoolEq(gateway.hasRole(_role, _account), false); + hevm.expectEmit(true, true, true, true); + emit RoleGranted(_role, _account, address(this)); + gateway.grantRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), true); + assertEq(gateway.getRoleMemberCount(_role), 1); + assertEq(gateway.getRoleMember(_role, 0), _account); + + // do nothing regrant + gateway.grantRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), true); + assertEq(gateway.getRoleMemberCount(_role), 1); + assertEq(gateway.getRoleMember(_role, 0), _account); } - function testUpdateWithdrawalsDisabler(address _disabler) external { - hevm.assume(_disabler != address(0)); - - // revert caller is not owner + function testRevokeRole(bytes32 _role, address _account) external { + // revert not owner hevm.startPrank(address(1)); hevm.expectRevert("Ownable: caller is not the owner"); - gateway.updateWithdrawalsDisabler(address(0)); + gateway.revokeRole(_role, _account); hevm.stopPrank(); - // succeed to set - hevm.startPrank(_disabler); - hevm.expectRevert(ErrorCallerIsNotWithdrawalsDisabler.selector); - gateway.disableWithdrawals(); - hevm.stopPrank(); + // grant first + gateway.grantRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), true); + assertEq(gateway.getRoleMemberCount(_role), 1); + assertEq(gateway.getRoleMember(_role, 0), _account); - hevm.expectEmit(true, true, false, true); - emit UpdateWithdrawalsDisabler(address(0), _disabler); - gateway.updateWithdrawalsDisabler(_disabler); - - assertBoolEq(true, gateway.isWithdrawalsEnabled()); - hevm.startPrank(_disabler); - gateway.disableWithdrawals(); - hevm.stopPrank(); - assertBoolEq(false, gateway.isWithdrawalsEnabled()); + // revoke + hevm.expectEmit(true, true, true, true); + emit RoleRevoked(_role, _account, address(this)); + gateway.revokeRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), false); + assertEq(gateway.getRoleMemberCount(_role), 0); + + // revoke again + gateway.revokeRole(_role, _account); + assertBoolEq(gateway.hasRole(_role, _account), false); + assertEq(gateway.getRoleMemberCount(_role), 0); } /******************************** @@ -443,13 +385,13 @@ contract L2LidoGatewayTest is L2GatewayTestBase { ); // revert when deposits disabled - mockGateway.updateDepositsDisabler(address(this)); + mockGateway.grantRole(gateway.DEPOSITS_DISABLER_ROLE(), address(this)); mockGateway.disableDeposits(); hevm.expectRevert(ErrorDepositsDisabled.selector); mockMessenger.callTarget(address(mockGateway), message); // revert when nonzero msg.value - mockGateway.updateDepositsEnabler(address(this)); + mockGateway.grantRole(gateway.DEPOSITS_ENABLER_ROLE(), address(this)); mockGateway.enableDeposits(); hevm.expectRevert(ErrorNonZeroMsgValue.selector); mockMessenger.callTarget{value: 1}(address(mockGateway), message); @@ -527,13 +469,13 @@ contract L2LidoGatewayTest is L2GatewayTestBase { } // revert when withdrawals disabled - gateway.updateWithdrawalsDisabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_DISABLER_ROLE(), address(this)); gateway.disableWithdrawals(); hevm.expectRevert(ErrorWithdrawalsDisabled.selector); _invokeWithdrawERC20Call(useRouter, methodType, address(l2Token), amount, recipient, dataToCall, gasLimit); // revert when withdraw zero amount - gateway.updateWithdrawalsEnabler(address(this)); + gateway.grantRole(gateway.WITHDRAWALS_ENABLER_ROLE(), address(this)); gateway.enableWithdrawals(); hevm.expectRevert(ErrorWithdrawZeroAmount.selector); _invokeWithdrawERC20Call(useRouter, methodType, address(l2Token), 0, recipient, dataToCall, gasLimit); diff --git a/contracts/src/test/lido/L2WstETHToken.t.sol b/contracts/src/test/lido/L2WstETHToken.t.sol new file mode 100644 index 0000000000..612d42ee03 --- /dev/null +++ b/contracts/src/test/lido/L2WstETHToken.t.sol @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: MIT + +pragma solidity =0.8.16; + +import {DSTestPlus} from "solmate/test/utils/DSTestPlus.sol"; + +import {IERC1271Upgradeable} from "@openzeppelin/contracts-upgradeable/interfaces/IERC1271Upgradeable.sol"; +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; + +import {L2WstETHToken} from "../../lido/L2WstETHToken.sol"; + +contract L2WstETHTokenTest is DSTestPlus { + L2WstETHToken private counterpart; + L2WstETHToken private token; + + bytes4 private _magicValue; + bool private revertOnSignature; + + function setUp() public { + hevm.warp(1000); // make block timestamp nonzero + + counterpart = new L2WstETHToken(); + token = L2WstETHToken(address(new ERC1967Proxy(address(new L2WstETHToken()), new bytes(0)))); + + token.initialize("Wrapped liquid staked Ether 2.0", "wstETH", 18, address(this), address(counterpart)); + } + + function testInitialize() external { + assertEq(token.name(), "Wrapped liquid staked Ether 2.0"); + assertEq(token.symbol(), "wstETH"); + assertEq(token.decimals(), 18); + assertEq(token.gateway(), address(this)); + assertEq(token.counterpart(), address(counterpart)); + } + + function testPermit(uint256 amount) external { + uint256 timestamp = block.timestamp; + // revert when expire + hevm.expectRevert("ERC20Permit: expired deadline"); + token.permit(address(this), address(counterpart), 1, timestamp - 1, 0, 0, 0); + + // revert when invalid contract signature + hevm.expectRevert("ERC20Permit: invalid signature"); + _magicValue = bytes4(0); + revertOnSignature = false; + token.permit(address(this), address(counterpart), 1, timestamp, 0, 0, 0); + + // revert when invalid contract signature + hevm.expectRevert("ERC20Permit: invalid signature"); + _magicValue = IERC1271Upgradeable.isValidSignature.selector; + revertOnSignature = true; + token.permit(address(this), address(counterpart), 1, timestamp, 0, 0, 0); + + // succeed on contract signer + _magicValue = IERC1271Upgradeable.isValidSignature.selector; + revertOnSignature = false; + assertEq(token.allowance(address(this), address(counterpart)), 0); + token.permit(address(this), address(counterpart), amount, timestamp, 0, 0, 0); + assertEq(token.allowance(address(this), address(counterpart)), amount); + } + + function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4 magicValue) { + if (revertOnSignature) { + revert("revert"); + } + + magicValue = _magicValue; + } +} From 1c04129b40b6f911548e4a2ce9bab9bb6a3e5613 Mon Sep 17 00:00:00 2001 From: zimpha Date: Fri, 22 Dec 2023 17:22:51 +0800 Subject: [PATCH 7/9] update readme --- contracts/src/lido/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/src/lido/README.md b/contracts/src/lido/README.md index 28882b60c1..7aa2145bec 100644 --- a/contracts/src/lido/README.md +++ b/contracts/src/lido/README.md @@ -21,7 +21,7 @@ At this point, the implementation must provide a scalable and reliable solution | The bridge is upgradeable | Yes | | Upgrade authority for the bridge | TBA | | Emergency pause/cancel mechanisms and their authorities | TBA | -| The bridged token support permits and ERC-1271 | No, only permits | +| The bridged token support permits and ERC-1271 | Yes | | Are the following things in the scope of this bridge deployment: | | | - Passing the (w)stETH/USD price feed | No | | - Passing Lido DAO governance decisions | [Lido DAO Agent](https://etherscan.io/address/0x3e40D73EB977Dc6a537aF587D48316feE66E9C8c) representation [on Scroll (address TBD)] via [ScrollBridgeExecutor](https://github.com/scroll-tech/governance-crosschain-bridges/blob/scroll/contracts/bridges/ScrollBridgeExecutor.sol) | From d2a2b053fc071414c35e4c061e866cdde87ca99f Mon Sep 17 00:00:00 2001 From: zimpha Date: Mon, 8 Jan 2024 18:08:42 +0800 Subject: [PATCH 8/9] update unit tests --- .../GasOptimizationUpgrade.spec.ts | 79 +++++++++++++++++++ .../scripts/foundry/DeployLidoGateway.s.sol | 24 +++++- contracts/src/lido/L1LidoGateway.sol | 25 +++++- contracts/src/lido/L2LidoGateway.sol | 25 +++++- .../integration/LidoGatewayIntegration.t.sol | 12 ++- contracts/src/test/lido/L1LidoGateway.t.sol | 48 ++++++----- contracts/src/test/lido/L2LidoGateway.t.sol | 37 +++++---- .../src/test/mocks/MockL1LidoGateway.sol | 8 +- .../src/test/mocks/MockL2LidoGateway.sol | 8 +- 9 files changed, 221 insertions(+), 45 deletions(-) diff --git a/contracts/integration-test/GasOptimizationUpgrade.spec.ts b/contracts/integration-test/GasOptimizationUpgrade.spec.ts index 7b00a14097..c8def13260 100644 --- a/contracts/integration-test/GasOptimizationUpgrade.spec.ts +++ b/contracts/integration-test/GasOptimizationUpgrade.spec.ts @@ -334,6 +334,46 @@ describe("GasOptimizationUpgrade.spec", async () => { "L1GatewayRouter.depositERC20 USDC after upgrade" ); }); + + it.skip("should succeed on L1LidoGateway", async () => { + const L1_WSTETH = "0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0"; + const L2_WSTETH = "0xf610A9dfB7C89644979b4A0f27063E9e7d7Cda32"; + const L1_GATEWAY = "0x6625C6332c9F91F2D27c304E729B86db87A3f504"; + const L2_GATEWAY = "0x8aE8f22226B9d789A36AC81474e633f8bE2856c9"; + const L1LidoGateway = await ethers.getContractFactory("L1LidoGateway", deployer); + const impl = await L1LidoGateway.deploy(L1_WSTETH, L2_WSTETH, L2_GATEWAY, L1_ROUTER, L1_MESSENGER); + const gateway = await ethers.getContractAt("L1LidoGateway", L1_GATEWAY, deployer); + const amountIn = ethers.utils.parseUnits("1", 6); + const fee = await queue.estimateCrossDomainMessageFee(1e6); + const token = await ethers.getContractAt("MockERC20", L1_WSTETH, deployer); + await mockERC20Balance(token.address, amountIn.mul(10), 0); + await token.approve(L1_GATEWAY, constants.MaxUint256); + await token.approve(L1_ROUTER, constants.MaxUint256); + + // before upgrade + await showGasUsage( + await gateway["depositERC20(address,uint256,uint256)"](L1_WSTETH, amountIn, 1e6, { value: fee }), + "L1LidoGateway.depositERC20 wstETH before upgrade" + ); + await showGasUsage( + await router["depositERC20(address,uint256,uint256)"](L1_WSTETH, amountIn, 1e6, { value: fee }), + "L1GatewayRouter.depositERC20 wstETH before upgrade" + ); + + // do upgrade + await upgradeL1(L1_GATEWAY, impl.address); + await gateway.initializeV2(deployer.address, deployer.address, deployer.address, deployer.address); + + // after upgrade + await showGasUsage( + await gateway["depositERC20(address,uint256,uint256)"](L1_WSTETH, amountIn, 1e6, { value: fee }), + "L1LidoGateway.depositERC20 wstETH after upgrade" + ); + await showGasUsage( + await router["depositERC20(address,uint256,uint256)"](L1_WSTETH, amountIn, 1e6, { value: fee }), + "L1GatewayRouter.depositERC20 wstETH after upgrade" + ); + }); }); context("L2 upgrade", async () => { @@ -584,5 +624,44 @@ describe("GasOptimizationUpgrade.spec", async () => { "L2GatewayRouter.withdrawERC20 USDC after upgrade" ); }); + + it.skip("should succeed on L2LidoGateway", async () => { + const L1_WSTETH = "0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0"; + const L2_WSTETH = "0xf610A9dfB7C89644979b4A0f27063E9e7d7Cda32"; + const L1_GATEWAY = "0x6625C6332c9F91F2D27c304E729B86db87A3f504"; + const L2_GATEWAY = "0x8aE8f22226B9d789A36AC81474e633f8bE2856c9"; + const L2LidoGateway = await ethers.getContractFactory("L2LidoGateway", deployer); + const impl = await L2LidoGateway.deploy(L1_WSTETH, L2_WSTETH, L1_GATEWAY, L2_ROUTER, L2_MESSENGER); + const gateway = await ethers.getContractAt("L2LidoGateway", L2_GATEWAY, deployer); + const amountIn = ethers.utils.parseUnits("1", 6); + const token = await ethers.getContractAt("MockERC20", L2_WSTETH, deployer); + await mockERC20Balance(token.address, amountIn.mul(10), 51); + await token.approve(L2_GATEWAY, constants.MaxUint256); + await token.approve(L2_ROUTER, constants.MaxUint256); + + // before upgrade + await showGasUsage( + await gateway["withdrawERC20(address,uint256,uint256)"](L2_WSTETH, amountIn, 1e6), + "L2LidoGateway.withdrawERC20 wstETH before upgrade" + ); + await showGasUsage( + await router["withdrawERC20(address,uint256,uint256)"](L2_WSTETH, amountIn, 1e6), + "L2GatewayRouter.withdrawERC20 wstETH before upgrade" + ); + + // do upgrade + await upgradeL2(L2_GATEWAY, impl.address); + await gateway.initializeV2(deployer.address, deployer.address, deployer.address, deployer.address); + + // after upgrade + await showGasUsage( + await gateway["withdrawERC20(address,uint256,uint256)"](L2_WSTETH, amountIn, 1e6), + "L2LidoGateway.withdrawERC20 wstETH after upgrade" + ); + await showGasUsage( + await router["withdrawERC20(address,uint256,uint256)"](L2_WSTETH, amountIn, 1e6), + "L2GatewayRouter.withdrawERC20 wstETH after upgrade" + ); + }); }); }); diff --git a/contracts/scripts/foundry/DeployLidoGateway.s.sol b/contracts/scripts/foundry/DeployLidoGateway.s.sol index 50b5e16c3f..49e40078b1 100644 --- a/contracts/scripts/foundry/DeployLidoGateway.s.sol +++ b/contracts/scripts/foundry/DeployLidoGateway.s.sol @@ -21,16 +21,36 @@ contract DeployLidoGateway is Script { address L2_WSTETH_ADDR = vm.envAddress("L2_WSTETH_ADDR"); + address L1_SCROLL_MESSENGER_PROXY_ADDR = vm.envAddress("L1_SCROLL_MESSENGER_PROXY_ADDR"); + address L1_GATEWAY_ROUTER_PROXY_ADDR = vm.envAddress("L1_GATEWAY_ROUTER_PROXY_ADDR"); + address L1_LIDO_GATEWAY_PROXY_ADDR = vm.envAddress("L1_LIDO_GATEWAY_PROXY_ADDR"); + + address L2_SCROLL_MESSENGER_PROXY_ADDR = vm.envAddress("L2_SCROLL_MESSENGER_PROXY_ADDR"); + address L2_GATEWAY_ROUTER_PROXY_ADDR = vm.envAddress("L2_GATEWAY_ROUTER_PROXY_ADDR"); + address L2_LIDO_GATEWAY_PROXY_ADDR = vm.envAddress("L2_LIDO_GATEWAY_PROXY_ADDR"); + function run() external { vm.startBroadcast(L2_DEPLOYER_PRIVATE_KEY); if (keccak256(abi.encodePacked(NETWORK)) == keccak256(abi.encodePacked("L1"))) { // depoly l1 lido gateway - L1LidoGateway gateway = new L1LidoGateway(L1_WSTETH_ADDR, L2_WSTETH_ADDR); + L1LidoGateway gateway = new L1LidoGateway( + L1_WSTETH_ADDR, + L2_WSTETH_ADDR, + L2_LIDO_GATEWAY_PROXY_ADDR, + L1_GATEWAY_ROUTER_PROXY_ADDR, + L1_SCROLL_MESSENGER_PROXY_ADDR + ); logAddress("L1_LIDO_GATEWAY_IMPLEMENTATION_ADDR", address(gateway)); } else if (keccak256(abi.encodePacked(NETWORK)) == keccak256(abi.encodePacked("L2"))) { // depoly l2 lido gateway - L2LidoGateway gateway = new L2LidoGateway(L1_WSTETH_ADDR, L2_WSTETH_ADDR); + L2LidoGateway gateway = new L2LidoGateway( + L1_WSTETH_ADDR, + L2_WSTETH_ADDR, + L1_LIDO_GATEWAY_PROXY_ADDR, + L2_GATEWAY_ROUTER_PROXY_ADDR, + L2_SCROLL_MESSENGER_PROXY_ADDR + ); logAddress("L2_LIDO_GATEWAY_IMPLEMENTATION_ADDR", address(gateway)); } diff --git a/contracts/src/lido/L1LidoGateway.sol b/contracts/src/lido/L1LidoGateway.sol index 88eb3b95b8..416064fd94 100644 --- a/contracts/src/lido/L1LidoGateway.sol +++ b/contracts/src/lido/L1LidoGateway.sol @@ -34,20 +34,39 @@ contract L1LidoGateway is L1ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag * Constructor * ***************/ + /// @notice Constructor for `L1LidoGateway` implementation contract. + /// /// @param _l1Token The address of the bridged token in the L1 chain /// @param _l2Token The address of the token minted on the L2 chain when token bridged - constructor(address _l1Token, address _l2Token) LidoBridgeableTokens(_l1Token, _l2Token) { + /// @param _counterpart The address of `L2LidoGateway` contract in L2. + /// @param _router The address of `L1GatewayRouter` contract. + /// @param _messenger The address of `L1ScrollMessenger` contract. + constructor( + address _l1Token, + address _l2Token, + address _counterpart, + address _router, + address _messenger + ) LidoBridgeableTokens(_l1Token, _l2Token) ScrollGatewayBase(_counterpart, _router, _messenger) { + if (_l1Token == address(0) || _l2Token == address(0) || _router == address(0)) { + revert ErrorZeroAddress(); + } + _disableInitializers(); } /// @notice Initialize the storage of L1LidoGateway v1. + /// + /// @dev The parameters `_counterpart`, `_router` and `_messenger` are no longer used. + /// + /// @param _counterpart The address of `L2LidoGateway` contract in L2. + /// @param _router The address of `L1GatewayRouter` contract. + /// @param _messenger The address of `L1ScrollMessenger` contract. function initialize( address _counterpart, address _router, address _messenger ) external initializer { - require(_router != address(0), "zero router address"); - ScrollGatewayBase._initialize(_counterpart, _router, _messenger); } diff --git a/contracts/src/lido/L2LidoGateway.sol b/contracts/src/lido/L2LidoGateway.sol index 01e62bc4fe..991ea00627 100644 --- a/contracts/src/lido/L2LidoGateway.sol +++ b/contracts/src/lido/L2LidoGateway.sol @@ -35,20 +35,39 @@ contract L2LidoGateway is L2ERC20Gateway, LidoBridgeableTokens, LidoGatewayManag * Constructor * ***************/ + /// @notice Constructor for `L2LidoGateway` implementation contract. + /// /// @param _l1Token The address of the bridged token in the L1 chain /// @param _l2Token The address of the token minted on the L2 chain when token bridged - constructor(address _l1Token, address _l2Token) LidoBridgeableTokens(_l1Token, _l2Token) { + /// @param _counterpart The address of `L1LidoGateway` contract in L1. + /// @param _router The address of `L2GatewayRouter` contract in L2. + /// @param _messenger The address of `L2ScrollMessenger` contract in L2. + constructor( + address _l1Token, + address _l2Token, + address _counterpart, + address _router, + address _messenger + ) LidoBridgeableTokens(_l1Token, _l2Token) ScrollGatewayBase(_counterpart, _router, _messenger) { + if (_l1Token == address(0) || _l2Token == address(0) || _router == address(0)) { + revert ErrorZeroAddress(); + } + _disableInitializers(); } /// @notice Initialize the storage of L2LidoGateway v1. + /// + /// @dev The parameters `_counterpart`, `_router` and `_messenger` are no longer used. + /// + /// @param _counterpart The address of `L1LidoGateway` contract in L1. + /// @param _router The address of `L2GatewayRouter` contract in L2. + /// @param _messenger The address of `L2ScrollMessenger` contract in L2. function initialize( address _counterpart, address _router, address _messenger ) external initializer { - require(_router != address(0), "zero router address"); - ScrollGatewayBase._initialize(_counterpart, _router, _messenger); } diff --git a/contracts/src/test/integration/LidoGatewayIntegration.t.sol b/contracts/src/test/integration/LidoGatewayIntegration.t.sol index 3d8c4577d1..6d65b8e351 100644 --- a/contracts/src/test/integration/LidoGatewayIntegration.t.sol +++ b/contracts/src/test/integration/LidoGatewayIntegration.t.sol @@ -40,11 +40,19 @@ contract LidoGatewayIntegrationTest is GatewayIntegrationBase { __GatewayIntegrationBase_setUp(); mainnet.selectFork(); - upgrade(true, L1_LIDO_GATEWAY, address(new L1LidoGateway(L1_WSTETH, L2_WSTETH))); + upgrade( + true, + L1_LIDO_GATEWAY, + address(new L1LidoGateway(L1_WSTETH, L2_WSTETH, L2_LIDO_GATEWAY, L1_GATEWAY_ROUTER, L1_SCROLL_MESSENGER)) + ); L1LidoGateway(L1_LIDO_GATEWAY).initializeV2(address(0), address(0), address(0), address(0)); scroll.selectFork(); - upgrade(false, L2_LIDO_GATEWAY, address(new L2LidoGateway(L1_WSTETH, L2_WSTETH))); + upgrade( + false, + L2_LIDO_GATEWAY, + address(new L2LidoGateway(L1_WSTETH, L2_WSTETH, L1_LIDO_GATEWAY, L2_GATEWAY_ROUTER, L2_SCROLL_MESSENGER)) + ); L2LidoGateway(L2_LIDO_GATEWAY).initializeV2(address(0), address(0), address(0), address(0)); } diff --git a/contracts/src/test/lido/L1LidoGateway.t.sol b/contracts/src/test/lido/L1LidoGateway.t.sol index 5e276554a0..6014723ecb 100644 --- a/contracts/src/test/lido/L1LidoGateway.t.sol +++ b/contracts/src/test/lido/L1LidoGateway.t.sol @@ -5,6 +5,7 @@ pragma solidity =0.8.16; import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {ITransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {IL1ERC20Gateway} from "../../L1/gateways/IL1ERC20Gateway.sol"; import {L1GatewayRouter} from "../../L1/gateways/L1GatewayRouter.sol"; @@ -70,17 +71,18 @@ contract L1LidoGatewayTest is L1GatewayTestBase { MockERC20 private l2Token; function setUp() public { - setUpBase(); + __L1GatewayTestBase_setUp(); + // Deploy tokens l1Token = new MockERC20("Mock L1", "ML1", 18); l2Token = new MockERC20("Mock L2", "ML2", 18); - // Deploy L1 contracts - gateway = _deployGateway(); - router = L1GatewayRouter(address(new ERC1967Proxy(address(new L1GatewayRouter()), new bytes(0)))); - // Deploy L2 contracts - counterpartGateway = new L2LidoGateway(address(l1Token), address(l2Token)); + counterpartGateway = new L2LidoGateway(address(l1Token), address(l2Token), address(1), address(1), address(1)); + + // Deploy L1 contracts + router = L1GatewayRouter(_deployProxy(address(new L1GatewayRouter(address(l1Messenger))))); + gateway = _deployGateway(address(l1Messenger)); // Initialize L1 contracts gateway.initialize(address(counterpartGateway), address(router), address(l1Messenger)); @@ -335,16 +337,16 @@ contract L1LidoGatewayTest is L1GatewayTestBase { gateway.depositERC20AndCall(address(l1Token), recipient, amount, new bytes(0), defaultGasLimit); MockScrollMessenger mockMessenger = new MockScrollMessenger(); - MockL1LidoGateway mockGateway = _deployGateway(); + MockL1LidoGateway mockGateway = _deployGateway(address(mockMessenger)); mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); mockGateway.initializeV2(address(0), address(0), address(0), address(0)); // revert caller is not messenger - hevm.expectRevert("only messenger can call"); + hevm.expectRevert(ErrorCallerIsNotMessenger.selector); mockGateway.onDropMessage(new bytes(0)); // revert not in drop context - hevm.expectRevert("only called in drop context"); + hevm.expectRevert(ErrorNotInDropMessageContext.selector); mockMessenger.callTarget(address(mockGateway), abi.encodeCall(mockGateway.onDropMessage, (new bytes(0)))); // revert when reentrant @@ -416,12 +418,12 @@ contract L1LidoGatewayTest is L1GatewayTestBase { gateway.depositERC20(address(l1Token), amount, defaultGasLimit); // deposit some token to L1LidoGateway MockScrollMessenger mockMessenger = new MockScrollMessenger(); - MockL1LidoGateway mockGateway = _deployGateway(); + MockL1LidoGateway mockGateway = _deployGateway(address(mockMessenger)); mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); mockGateway.initializeV2(address(0), address(0), address(0), address(0)); // revert caller is not messenger - hevm.expectRevert("only messenger can call"); + hevm.expectRevert(ErrorCallerIsNotMessenger.selector); mockGateway.finalizeWithdrawERC20( address(l1Token), address(l2Token), @@ -432,7 +434,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { ); // revert not called by counterpart - hevm.expectRevert("only call by counterpart"); + hevm.expectRevert(ErrorCallerIsNotCounterpartGateway.selector); mockMessenger.callTarget(address(mockGateway), message); // revert when reentrant @@ -514,7 +516,7 @@ contract L1LidoGatewayTest is L1GatewayTestBase { amount = bound(amount, 1, l1Token.balanceOf(address(this))); gasLimit = bound(gasLimit, defaultGasLimit / 2, defaultGasLimit); feePerGas = bound(feePerGas, 0, 1000); - gasOracle.setL2BaseFee(feePerGas); + messageQueue.setL2BaseFee(feePerGas); feePerGas = feePerGas * gasLimit; // revert when reentrant @@ -684,12 +686,20 @@ contract L1LidoGatewayTest is L1GatewayTestBase { } } - function _deployGateway() internal returns (MockL1LidoGateway) { - return - MockL1LidoGateway( - address( - new ERC1967Proxy(address(new MockL1LidoGateway(address(l1Token), address(l2Token))), new bytes(0)) + function _deployGateway(address messenger) internal returns (MockL1LidoGateway _gateway) { + _gateway = MockL1LidoGateway(_deployProxy(address(0))); + + admin.upgrade( + ITransparentUpgradeableProxy(address(_gateway)), + address( + new MockL1LidoGateway( + address(l1Token), + address(l2Token), + address(counterpartGateway), + address(router), + address(messenger) ) - ); + ) + ); } } diff --git a/contracts/src/test/lido/L2LidoGateway.t.sol b/contracts/src/test/lido/L2LidoGateway.t.sol index 1ede36c51a..fbbfb025ec 100644 --- a/contracts/src/test/lido/L2LidoGateway.t.sol +++ b/contracts/src/test/lido/L2LidoGateway.t.sol @@ -5,6 +5,7 @@ pragma solidity =0.8.16; import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {ITransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {IL1ERC20Gateway} from "../../L1/gateways/IL1ERC20Gateway.sol"; import {IL2ERC20Gateway} from "../../L2/gateways/IL2ERC20Gateway.sol"; @@ -73,12 +74,12 @@ contract L2LidoGatewayTest is L2GatewayTestBase { l1Token = new MockERC20("Mock L1", "ML1", 18); l2Token = ScrollStandardERC20(address(new ERC1967Proxy(address(new ScrollStandardERC20()), new bytes(0)))); - // Deploy L2 contracts - gateway = _deployGateway(); - router = L2GatewayRouter(address(new ERC1967Proxy(address(new L2GatewayRouter()), new bytes(0)))); - // Deploy L1 contracts - counterpartGateway = new L1LidoGateway(address(l1Token), address(l2Token)); + counterpartGateway = new L1LidoGateway(address(l1Token), address(l2Token), address(1), address(1), address(1)); + + // Deploy L2 contracts + router = L2GatewayRouter(_deployProxy(address(new L2GatewayRouter(address(l2Messenger))))); + gateway = _deployGateway(address(l2Messenger)); // Initialize L2 contracts gateway.initialize(address(counterpartGateway), address(router), address(l2Messenger)); @@ -337,12 +338,12 @@ contract L2LidoGatewayTest is L2GatewayTestBase { ); MockScrollMessenger mockMessenger = new MockScrollMessenger(); - MockL2LidoGateway mockGateway = _deployGateway(); + MockL2LidoGateway mockGateway = _deployGateway(address(mockMessenger)); mockGateway.initialize(address(counterpartGateway), address(router), address(mockMessenger)); mockGateway.initializeV2(address(0), address(0), address(0), address(0)); // revert caller is not messenger - hevm.expectRevert("only messenger can call"); + hevm.expectRevert(ErrorCallerIsNotMessenger.selector); mockGateway.finalizeDepositERC20( address(l1Token), address(l2Token), @@ -353,7 +354,7 @@ contract L2LidoGatewayTest is L2GatewayTestBase { ); // revert not called by counterpart - hevm.expectRevert("only call by counterpart"); + hevm.expectRevert(ErrorCallerIsNotCounterpartGateway.selector); mockMessenger.callTarget(address(mockGateway), message); // revert when reentrant @@ -545,12 +546,20 @@ contract L2LidoGatewayTest is L2GatewayTestBase { } } - function _deployGateway() internal returns (MockL2LidoGateway) { - return - MockL2LidoGateway( - address( - new ERC1967Proxy(address(new MockL2LidoGateway(address(l1Token), address(l2Token))), new bytes(0)) + function _deployGateway(address messenger) internal returns (MockL2LidoGateway _gateway) { + _gateway = MockL2LidoGateway(_deployProxy(address(0))); + + admin.upgrade( + ITransparentUpgradeableProxy(address(_gateway)), + address( + new MockL2LidoGateway( + address(l1Token), + address(l2Token), + address(counterpartGateway), + address(router), + address(messenger) ) - ); + ) + ); } } diff --git a/contracts/src/test/mocks/MockL1LidoGateway.sol b/contracts/src/test/mocks/MockL1LidoGateway.sol index 4b7f8821ab..cb1d30f7dc 100644 --- a/contracts/src/test/mocks/MockL1LidoGateway.sol +++ b/contracts/src/test/mocks/MockL1LidoGateway.sol @@ -5,7 +5,13 @@ pragma solidity =0.8.16; import {L1LidoGateway} from "../../lido/L1LidoGateway.sol"; contract MockL1LidoGateway is L1LidoGateway { - constructor(address _l1Token, address _l2Token) L1LidoGateway(_l1Token, _l2Token) {} + constructor( + address _l1Token, + address _l2Token, + address _counterpart, + address _router, + address _messenger + ) L1LidoGateway(_l1Token, _l2Token, _counterpart, _router, _messenger) {} function reentrantCall(address target, bytes calldata data) external payable nonReentrant { (bool success, ) = target.call{value: msg.value}(data); diff --git a/contracts/src/test/mocks/MockL2LidoGateway.sol b/contracts/src/test/mocks/MockL2LidoGateway.sol index 981c82679f..0b2fdd4c93 100644 --- a/contracts/src/test/mocks/MockL2LidoGateway.sol +++ b/contracts/src/test/mocks/MockL2LidoGateway.sol @@ -5,7 +5,13 @@ pragma solidity =0.8.16; import {L2LidoGateway} from "../../lido/L2LidoGateway.sol"; contract MockL2LidoGateway is L2LidoGateway { - constructor(address _l1Token, address _l2Token) L2LidoGateway(_l1Token, _l2Token) {} + constructor( + address _l1Token, + address _l2Token, + address _counterpart, + address _router, + address _messenger + ) L2LidoGateway(_l1Token, _l2Token, _counterpart, _router, _messenger) {} function reentrantCall(address target, bytes calldata data) external payable nonReentrant { (bool success, ) = target.call{value: msg.value}(data); From b644d4e3e85cdb04260cd32a3eb1caf1ab4bf362 Mon Sep 17 00:00:00 2001 From: zimpha Date: Tue, 16 Jan 2024 22:20:13 +0800 Subject: [PATCH 9/9] fix typo and comments --- contracts/scripts/foundry/DeployLidoGateway.s.sol | 4 ++-- contracts/src/lido/LidoGatewayManager.sol | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/contracts/scripts/foundry/DeployLidoGateway.s.sol b/contracts/scripts/foundry/DeployLidoGateway.s.sol index 49e40078b1..2619bd8579 100644 --- a/contracts/scripts/foundry/DeployLidoGateway.s.sol +++ b/contracts/scripts/foundry/DeployLidoGateway.s.sol @@ -33,7 +33,7 @@ contract DeployLidoGateway is Script { vm.startBroadcast(L2_DEPLOYER_PRIVATE_KEY); if (keccak256(abi.encodePacked(NETWORK)) == keccak256(abi.encodePacked("L1"))) { - // depoly l1 lido gateway + // deploy l1 lido gateway L1LidoGateway gateway = new L1LidoGateway( L1_WSTETH_ADDR, L2_WSTETH_ADDR, @@ -43,7 +43,7 @@ contract DeployLidoGateway is Script { ); logAddress("L1_LIDO_GATEWAY_IMPLEMENTATION_ADDR", address(gateway)); } else if (keccak256(abi.encodePacked(NETWORK)) == keccak256(abi.encodePacked("L2"))) { - // depoly l2 lido gateway + // deploy l2 lido gateway L2LidoGateway gateway = new L2LidoGateway( L1_WSTETH_ADDR, L2_WSTETH_ADDR, diff --git a/contracts/src/lido/LidoGatewayManager.sol b/contracts/src/lido/LidoGatewayManager.sol index cb3c228d40..08ccb2135c 100644 --- a/contracts/src/lido/LidoGatewayManager.sol +++ b/contracts/src/lido/LidoGatewayManager.sol @@ -80,10 +80,7 @@ abstract contract LidoGatewayManager is ScrollGatewayBase { /// @dev Stores the state of the bridging /// @param isDepositsEnabled Stores the state of the deposits /// @param isWithdrawalsEnabled Stores the state of the withdrawals - /// @param depositsEnabler The address of user who can enable deposits - /// @param depositsEnabler The address of user who can disable deposits - /// @param withdrawalsEnabler The address of user who can enable withdrawals - /// @param withdrawalsDisabler The address of user who can disable withdrawals + /// @param roles Mapping from role to list of role members. struct State { bool isDepositsEnabled; bool isWithdrawalsEnabled;