fix(contracts): OZ-L2-L06 Initialization Performed Outside of Initial…

…ization Function (#652) Co-authored-by: Haichen Shen <shenhaichen@gmail.com>
scroll-tech · Jul 25, 2023 · 3832422 · 3832422
1 parent 4d96c12
commit 3832422
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 30 deletions.
diff --git a/contracts/scripts/foundry/InitializeL2BridgeContracts.s.sol b/contracts/scripts/foundry/InitializeL2BridgeContracts.s.sol
@@ -50,8 +50,7 @@ contract InitializeL2BridgeContracts is Script {
         vm.startBroadcast(deployerPrivateKey);
 
         // initialize L2MessageQueue
-        L2MessageQueue(L2_MESSAGE_QUEUE_ADDR).initialize();
-        L2MessageQueue(L2_MESSAGE_QUEUE_ADDR).updateMessenger(L2_SCROLL_MESSENGER_PROXY_ADDR);
+        L2MessageQueue(L2_MESSAGE_QUEUE_ADDR).initialize(L2_SCROLL_MESSENGER_PROXY_ADDR);
 
         // initialize L2TxFeeVault
         L2TxFeeVault(payable(L2_TX_FEE_VAULT_ADDR)).updateMessenger(L2_SCROLL_MESSENGER_PROXY_ADDR);

diff --git a/contracts/src/L2/predeploys/L2MessageQueue.sol b/contracts/src/L2/predeploys/L2MessageQueue.sol
@@ -21,11 +21,6 @@ contract L2MessageQueue is AppendOnlyMerkleTree, OwnableBase {
     /// @param messageHash The hash of the corresponding message.
     event AppendMessage(uint256 index, bytes32 messageHash);
 
-    /// @notice Emits each time the owner updates the address of `messenger`.
-    /// @param oldMessenger The address of old messenger.
-    /// @param newMessenger The address of new messenger.
-    event UpdateMessenger(address indexed oldMessenger, address indexed newMessenger);
-
     /*************
      * Variables *
      *************/
@@ -41,8 +36,15 @@ contract L2MessageQueue is AppendOnlyMerkleTree, OwnableBase {
         _transferOwnership(_owner);
     }
 
-    function initialize() external {
+    /// @notice Initialize the state of `L2MessageQueue`
+    /// @dev You are not allowed to initialize when there are some messages appended.
+    /// @param _messenger The address of messenger to update.
+    function initialize(address _messenger) external onlyOwner {
+        require(nextMessageIndex == 0, "cannot initialize");
+
         _initializeMerkleTree();
+
+        messenger = _messenger;
     }
 
     /*****************************
@@ -61,20 +63,4 @@ contract L2MessageQueue is AppendOnlyMerkleTree, OwnableBase {
 
         return _currentRoot;
     }
-
-    /************************
-     * Restricted Functions *
-     ************************/
-
-    /// @notice Update the address of messenger.
-    /// @dev You are not allowed to update messenger when there are some messages appended.
-    /// @param _newMessenger The address of messenger to update.
-    function updateMessenger(address _newMessenger) external onlyOwner {
-        require(nextMessageIndex == 0, "cannot update messenger");
-
-        address _oldMessenger = messenger;
-        messenger = _newMessenger;
-
-        emit UpdateMessenger(_oldMessenger, _newMessenger);
-    }
 }
diff --git a/contracts/src/test/L2GatewayTestBase.t.sol b/contracts/src/test/L2GatewayTestBase.t.sol
@@ -59,8 +59,7 @@ abstract contract L2GatewayTestBase is DSTestPlus {
 
         // Initialize L2 contracts
         l2Messenger.initialize(address(l1Messenger), feeVault);
-        l2MessageQueue.initialize();
-        l2MessageQueue.updateMessenger(address(l2Messenger));
+        l2MessageQueue.initialize(address(l2Messenger));
         l1GasOracle.updateWhitelist(address(whitelist));
 
         address[] memory _accounts = new address[](1);

diff --git a/contracts/src/test/L2MessageQueue.t.sol b/contracts/src/test/L2MessageQueue.t.sol
@@ -11,8 +11,7 @@ contract L2MessageQueueTest is DSTestPlus {
 
     function setUp() public {
         queue = new L2MessageQueue(address(this));
-        queue.initialize();
-        queue.updateMessenger(address(this));
+        queue.initialize(address(this));
     }
 
     function testConstructor() external {

diff --git a/contracts/src/test/L2ScrollMessenger.t.sol b/contracts/src/test/L2ScrollMessenger.t.sol
@@ -41,8 +41,7 @@ contract L2ScrollMessengerTest is DSTestPlus {
 
         // Initialize L2 contracts
         l2Messenger.initialize(address(l1Messenger), feeVault);
-        l2MessageQueue.initialize();
-        l2MessageQueue.updateMessenger(address(l2Messenger));
+        l2MessageQueue.initialize(address(l2Messenger));
         l1GasOracle.updateWhitelist(address(whitelist));
     }