Merge pull request ethereum-optimism#4913 from ethereum-optimism/feat…

…/ctb-remove-xdm-pause contracts-bedrock: remove crossdomain messenger pausability
hassansazgar · Feb 25, 2023 · 8969e46 · 8969e46
2 parents 10cecba + 608ebc9
commit 8969e46
Show file tree

Hide file tree

Showing 19 changed files with 330 additions and 1,473 deletions.
diff --git a/op-bindings/bindings/l1crossdomainmessenger.go b/op-bindings/bindings/l1crossdomainmessenger.go
diff --git a/op-bindings/bindings/l1crossdomainmessenger_more.go b/op-bindings/bindings/l1crossdomainmessenger_more.go
diff --git a/op-bindings/bindings/l2crossdomainmessenger.go b/op-bindings/bindings/l2crossdomainmessenger.go
diff --git a/op-bindings/bindings/l2crossdomainmessenger_more.go b/op-bindings/bindings/l2crossdomainmessenger_more.go
diff --git a/op-chain-ops/genesis/config.go b/op-chain-ops/genesis/config.go
@@ -414,9 +414,7 @@ func NewL2StorageConfig(config *DeployConfig, block *types.Block) (state.Storage
 	}
 	storage["L2CrossDomainMessenger"] = state.StorageValues{
 		"_initialized":     1,
-		"_owner":           config.ProxyAdminOwner,
 		"_initializing":    false,
-		"_paused":          false,
 		"xDomainMsgSender": "0x000000000000000000000000000000000000dEaD",
 		"msgNonce":         0,
 	}

diff --git a/op-chain-ops/genesis/layer_one.go b/op-chain-ops/genesis/layer_one.go
@@ -141,10 +141,7 @@ func BuildL1DeveloperGenesis(config *DeployConfig) (*core.Genesis, error) {
 	if err != nil {
 		return nil, err
 	}
-	data, err = l1XDMABI.Pack(
-		"initialize",
-		config.FinalSystemOwner,
-	)
+	data, err = l1XDMABI.Pack("initialize")
 	if err != nil {
 		return nil, fmt.Errorf("cannot abi encode initialize for L1CrossDomainMessenger: %w", err)
 	}

diff --git a/packages/contracts-bedrock/.gas-snapshot b/packages/contracts-bedrock/.gas-snapshot
diff --git a/packages/contracts-bedrock/.storage-layout b/packages/contracts-bedrock/.storage-layout
@@ -11,11 +11,11 @@
 | spacer_0_0_20          | address                  | 0    | 0      | 20    | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
 | _initialized           | uint8                    | 0    | 20     | 1     | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
 | _initializing          | bool                     | 0    | 21     | 1     | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
-| __gap                  | uint256[50]              | 1    | 0      | 1600  | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
-| _owner                 | address                  | 51   | 0      | 20    | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
-| __gap                  | uint256[49]              | 52   | 0      | 1568  | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
-| _paused                | bool                     | 101  | 0      | 1     | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
-| __gap                  | uint256[49]              | 102  | 0      | 1568  | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
+| spacer_1_0_1600        | uint256[50]              | 1    | 0      | 1600  | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
+| spacer_51_0_20         | address                  | 51   | 0      | 20    | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
+| spacer_52_0_1568       | uint256[49]              | 52   | 0      | 1568  | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
+| spacer_101_0_1         | bool                     | 101  | 0      | 1     | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
+| spacer_102_0_1568      | uint256[49]              | 102  | 0      | 1568  | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
 | spacer_151_0_32        | uint256                  | 151  | 0      | 32    | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
 | __gap_reentrancy_guard | uint256[49]              | 152  | 0      | 1568  | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
 | spacer_201_0_32        | mapping(bytes32 => bool) | 201  | 0      | 32    | contracts/L1/L1CrossDomainMessenger.sol:L1CrossDomainMessenger |
@@ -121,11 +121,11 @@
 | spacer_0_0_20          | address                  | 0    | 0      | 20    | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
 | _initialized           | uint8                    | 0    | 20     | 1     | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
 | _initializing          | bool                     | 0    | 21     | 1     | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
-| __gap                  | uint256[50]              | 1    | 0      | 1600  | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
-| _owner                 | address                  | 51   | 0      | 20    | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
-| __gap                  | uint256[49]              | 52   | 0      | 1568  | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
-| _paused                | bool                     | 101  | 0      | 1     | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
-| __gap                  | uint256[49]              | 102  | 0      | 1568  | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
+| spacer_1_0_1600        | uint256[50]              | 1    | 0      | 1600  | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
+| spacer_51_0_20         | address                  | 51   | 0      | 20    | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
+| spacer_52_0_1568       | uint256[49]              | 52   | 0      | 1568  | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
+| spacer_101_0_1         | bool                     | 101  | 0      | 1     | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
+| spacer_102_0_1568      | uint256[49]              | 102  | 0      | 1568  | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
 | spacer_151_0_32        | uint256                  | 151  | 0      | 32    | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
 | __gap_reentrancy_guard | uint256[49]              | 152  | 0      | 1568  | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |
 | spacer_201_0_32        | mapping(bytes32 => bool) | 201  | 0      | 32    | contracts/L2/L2CrossDomainMessenger.sol:L2CrossDomainMessenger |

diff --git a/packages/contracts-bedrock/contracts/L1/L1CrossDomainMessenger.sol b/packages/contracts-bedrock/contracts/L1/L1CrossDomainMessenger.sol
@@ -29,17 +29,14 @@ contract L1CrossDomainMessenger is CrossDomainMessenger, Semver {
         CrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER)
     {
         PORTAL = _portal;
-        initialize(address(0));
+        initialize();
     }
 
     /**
      * @notice Initializer.
-     *
-     * @param _owner Address of the initial owner of this contract.
      */
-    function initialize(address _owner) public initializer {
+    function initialize() public initializer {
         __CrossDomainMessenger_init();
-        _transferOwnership(_owner);
     }
 
     /**

diff --git a/packages/contracts-bedrock/contracts/deployment/SystemDictator.sol b/packages/contracts-bedrock/contracts/deployment/SystemDictator.sol
@@ -335,7 +335,7 @@ contract SystemDictator is OwnableUpgradeable {
         // Try to initialize the L1CrossDomainMessenger, only fail if it's already been initialized.
         try
             L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy)
-                .initialize(address(this))
+                .initialize()
         {
             // L1CrossDomainMessenger is the one annoying edge case difference between existing
             // networks and fresh networks because in existing networks it'll already be
@@ -375,27 +375,12 @@ contract SystemDictator is OwnableUpgradeable {
             payable(config.proxyAddressConfig.l1ERC721BridgeProxy),
             address(config.implementationAddressConfig.l1ERC721BridgeImpl)
         );
-
-        // Pause the L1CrossDomainMessenger, chance to check that everything is OK.
-        L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy).pause();
-    }
-
-    /**
-     * @notice Unpauses the system at which point the system should be fully operational.
-     */
-    function step6() external onlyOwner step(6) {
-        // Unpause the L1CrossDomainMessenger.
-        L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy).unpause();
     }
 
     /**
      * @notice Tranfers admin ownership to the final owner.
      */
     function finalize() external onlyOwner {
-        // Transfer ownership of the L1CrossDomainMessenger to the final owner.
-        L1CrossDomainMessenger(config.proxyAddressConfig.l1CrossDomainMessengerProxy)
-            .transferOwnership(config.globalConfig.finalOwner);
-
         // Transfer ownership of the ProxyAdmin to the final owner.
         config.globalConfig.proxyAdmin.transferOwnership(config.globalConfig.finalOwner);
 

diff --git a/packages/contracts-bedrock/contracts/test/CommonTest.t.sol b/packages/contracts-bedrock/contracts/test/CommonTest.t.sol
@@ -254,7 +254,7 @@ contract Messenger_Initializer is L2OutputOracle_Initializer {
             "OVM_L1CrossDomainMessenger"
         );
         L1Messenger = L1CrossDomainMessenger(address(proxy));
-        L1Messenger.initialize(alice);
+        L1Messenger.initialize();
 
         vm.etch(
             Predeploys.L2_CROSS_DOMAIN_MESSENGER,

diff --git a/packages/contracts-bedrock/contracts/test/L1CrossDomainMessenger.t.sol b/packages/contracts-bedrock/contracts/test/L1CrossDomainMessenger.t.sol
@@ -25,42 +25,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
     // Storage slot of the l2Sender
     uint256 constant senderSlotIndex = 50;
 
-    function setUp() public override {
-        super.setUp();
-    }
-
-    // pause: should pause the contract when called by the current owner
-    function test_pause_succeeds() external {
-        vm.prank(alice);
-        L1Messenger.pause();
-        assert(L1Messenger.paused());
-    }
-
-    // pause: should not pause the contract when called by account other than the owner
-    function test_pause_callerIsNotOwner_reverts() external {
-        vm.expectRevert("Ownable: caller is not the owner");
-        vm.prank(address(0xABBA));
-        L1Messenger.pause();
-    }
-
-    // unpause: should unpause the contract when called by the current owner
-    function test_unpause_succeeds() external {
-        vm.prank(alice);
-        L1Messenger.pause();
-        assert(L1Messenger.paused());
-
-        vm.prank(alice);
-        L1Messenger.unpause();
-        assert(!L1Messenger.paused());
-    }
-
-    // unpause: should not unpause the contract when called by account other than the owner
-    function test_unpause_callerIsNotOwner_reverts() external {
-        vm.expectRevert("Ownable: caller is not the owner");
-        vm.prank(address(0xABBA));
-        L1Messenger.unpause();
-    }
-
     // the version is encoded in the nonce
     function test_messageVersion_succeeds() external {
         (, uint16 version) = Encoding.decodeVersionedNonce(L1Messenger.messageNonce());
@@ -274,15 +238,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         L1Messenger.xDomainMessageSender();
     }
 
-    // relayMessage: should revert if paused
-    function test_relayMessage_paused_reverts() external {
-        vm.prank(L1Messenger.owner());
-        L1Messenger.pause();
-
-        vm.expectRevert("Pausable: paused");
-        L1Messenger.relayMessage(0, address(0), address(0), 0, 0, hex"");
-    }
-
     // relayMessage: should send a successful call to the target contract after the first message
     // fails and ETH gets stuck, but the second message succeeds
     function test_relayMessage_retryAfterFailure_succeeds() external {

diff --git a/packages/contracts-bedrock/contracts/test/L2CrossDomainMessenger.t.sol b/packages/contracts-bedrock/contracts/test/L2CrossDomainMessenger.t.sol
@@ -16,21 +16,6 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
     // Receiver address for testing
     address recipient = address(0xabbaacdc);
 
-    function setUp() public override {
-        super.setUp();
-    }
-
-    function test_pause_succeeds() external {
-        L2Messenger.pause();
-        assert(L2Messenger.paused());
-    }
-
-    function test_pause_notOwner_reverts() external {
-        vm.expectRevert("Ownable: caller is not the owner");
-        vm.prank(address(0xABBA));
-        L2Messenger.pause();
-    }
-
     function test_messageVersion_succeeds() external {
         (, uint16 version) = Encoding.decodeVersionedNonce(L2Messenger.messageNonce());
         assertEq(version, L2Messenger.MESSAGE_VERSION());
@@ -191,15 +176,6 @@ contract L2CrossDomainMessenger_Test is Messenger_Initializer {
         L2Messenger.xDomainMessageSender();
     }
 
-    // relayMessage: should revert if paused
-    function test_relayMessage_paused_reverts() external {
-        vm.prank(L2Messenger.owner());
-        L2Messenger.pause();
-
-        vm.expectRevert("Pausable: paused");
-        L2Messenger.relayMessage(0, address(0), address(0), 0, 0, hex"");
-    }
-
     // relayMessage: should send a successful call to the target contract after the first message
     // fails and ETH gets stuck, but the second message succeeds
     function test_relayMessage_retry_succeeds() external {