contracts-bedrock: cleanup

contracts-bedrock: fix deployment

lint: fix

contracts-bedrock: snapshot regenerate

contracts-bedrock: portal pausability in constructor

contracts-bedrock: cleanup

contracts-bedrock: better comments

contracts-bedrock: regenerate invariant docs

op-chain-ops/genesis/layer_one.go

@@ -123,6 +123,7 @@ func BuildL1DeveloperGenesis(config *DeployConfig) (*core.Genesis, error) {
 	if err != nil {
 		return nil, err
 	}
+	// Initialize the OptimismPortal without being paused
 	data, err = portalABI.Pack("initialize", false)
 	if err != nil {
 		return nil, fmt.Errorf("cannot abi encode initialize for OptimismPortal: %w", err)
@@ -290,11 +291,15 @@ func deployL1Contracts(config *DeployConfig, backend *backends.SimulatedBackend)
 			},
 		},
 		{
+			// The implementation of the OptimismPortal is deployed
+			// as being paused to prevent invalid usage of the network
+			// as only the proxy should be used
 			Name: "OptimismPortal",
 			Args: []interface{}{
 				predeploys.DevL2OutputOracleAddr,
 				config.FinalSystemOwner,
 				uint642Big(config.FinalizationPeriodSeconds),
+				true, // _paused
 			},
 		},
 		{
@@ -359,6 +364,7 @@ func l1Deployer(backend *backends.SimulatedBackend, opts *bind.TransactOpts, dep
 			deployment.Args[0].(common.Address),
 			deployment.Args[1].(common.Address),
 			deployment.Args[2].(*big.Int),
+			deployment.Args[3].(bool),
 		)
 	case "L1CrossDomainMessenger":
 		_, tx, _, err = bindings.DeployL1CrossDomainMessenger(

packages/contracts-bedrock/.storage-layout

@@ -63,6 +63,7 @@
 | l2Sender             | address                                                    | 50   | 0      | 20    | contracts/L1/OptimismPortal.sol:OptimismPortal |
 | finalizedWithdrawals | mapping(bytes32 => bool)                                   | 51   | 0      | 32    | contracts/L1/OptimismPortal.sol:OptimismPortal |
 | provenWithdrawals    | mapping(bytes32 => struct OptimismPortal.ProvenWithdrawal) | 52   | 0      | 32    | contracts/L1/OptimismPortal.sol:OptimismPortal |
+| paused               | bool                                                       | 53   | 0      | 1     | contracts/L1/OptimismPortal.sol:OptimismPortal |
 
 =======================
 ➡ contracts/L1/SystemConfig.sol:SystemConfig

packages/contracts-bedrock/contracts/L1/OptimismPortal.sol

@@ -85,7 +85,7 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
      *         deposits and withdrawals are paused. This may be removed in the
      *         future.
      */
-    bool public paused = false;
+    bool public paused;
 
     /**
      * @notice Emitted when a transaction is deposited from L1 to L2. The parameters of this event
@@ -145,21 +145,23 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
     }
 
     /**
-     * @custom:semver 1.0.0
+     * @custom:semver 1.1.0
      *
      * @param _l2Oracle                  Address of the L2OutputOracle contract.
      * @param _guardian                  Address that can pause deposits and withdrawals.
      * @param _finalizationPeriodSeconds Output finalization time in seconds.
+     * @param _paused                    Sets the contract's pausability state.
      */
     constructor(
         L2OutputOracle _l2Oracle,
         address _guardian,
-        uint256 _finalizationPeriodSeconds
+        uint256 _finalizationPeriodSeconds,
+        bool _paused
     ) Semver(1, 1, 0) {
         L2_ORACLE = _l2Oracle;
         GUARDIAN = _guardian;
         FINALIZATION_PERIOD_SECONDS = _finalizationPeriodSeconds;
-        initialize(true);
+        initialize(_paused);
     }
 
     /**

packages/contracts-bedrock/contracts/deployment/SystemDictator.sol

@@ -319,7 +319,7 @@ contract SystemDictator is OwnableUpgradeable {
             )
         );
 
-        // Upgrade and initialize the OptimismPortal as paused.
+        // Upgrade and initialize the OptimismPortal.
         config.globalConfig.proxyAdmin.upgradeAndCall(
             payable(config.proxyAddressConfig.optimismPortalProxy),
             address(config.implementationAddressConfig.optimismPortalImpl),

packages/contracts-bedrock/contracts/echidna/FuzzOptimismPortal.sol

@@ -12,7 +12,8 @@ contract EchidnaFuzzOptimismPortal {
         portal = new OptimismPortal({
             _l2Oracle: L2OutputOracle(address(0)),
             _guardian: address(0),
-            _finalizationPeriodSeconds: 10
+            _finalizationPeriodSeconds: 10,
+            _paused: false
         });
     }
 

packages/contracts-bedrock/contracts/test/CommonTest.t.sol

@@ -119,6 +119,7 @@ contract L2OutputOracle_Initializer is CommonTest {
     }
 
     function setUp() public virtual override {
+        super.setUp();
         guardian = makeAddr("guardian");
 
         // By default the first block has timestamp and number zero, which will cause underflows in the
@@ -169,7 +170,8 @@ contract Portal_Initializer is L2OutputOracle_Initializer {
         opImpl = new OptimismPortal({
             _l2Oracle: oracle,
             _guardian: guardian,
-            _finalizationPeriodSeconds: 7 days
+            _finalizationPeriodSeconds: 7 days,
+            _paused: true
         });
         Proxy proxy = new Proxy(multisig);
         vm.prank(multisig);
@@ -232,7 +234,8 @@ contract Messenger_Initializer is L2OutputOracle_Initializer {
         op = new OptimismPortal({
             _l2Oracle: oracle,
             _guardian: guardian,
-            _finalizationPeriodSeconds: 7 days
+            _finalizationPeriodSeconds: 7 days,
+            _paused: false
         });
         vm.label(address(op), "OptimismPortal");
         vm.prank(op.GUARDIAN());

packages/contracts-bedrock/contracts/test/OptimismPortal.t.sol

@@ -21,6 +21,9 @@ contract OptimismPortal_Test is Portal_Initializer {
         assertEq(op.paused(), false);
     }
 
+    /**
+     * @notice The OptimismPortal can be paused by the GUARDIAN
+     */
     function test_pause_succeeds() external {
         address guardian = op.GUARDIAN();
 
@@ -35,6 +38,10 @@ contract OptimismPortal_Test is Portal_Initializer {
         assertEq(op.paused(), true);
     }
 
+    /**
+     * @notice The OptimismPortal reverts when an account that is not the
+     *         GUARDIAN calls `pause()`
+     */
     function test_pause_onlyOwner_reverts() external {
         assertEq(op.paused(), false);
 
@@ -46,6 +53,9 @@ contract OptimismPortal_Test is Portal_Initializer {
         assertEq(op.paused(), false);
     }
 
+    /**
+     * @notice The OptimismPortal can be unpaused by the GUARDIAN
+     */
     function test_unpause_succeeds() external {
         address guardian = op.GUARDIAN();
 
@@ -61,6 +71,10 @@ contract OptimismPortal_Test is Portal_Initializer {
         assertEq(op.paused(), false);
     }
 
+    /**
+     * @notice The OptimismPortal reverts when an account that is not
+     *         the GUARDIAN calls `unpause()`
+     */
     function test_unpause_onlyOwner_reverts() external {
         address guardian = op.GUARDIAN();
 

packages/contracts-bedrock/deploy/013-OptimismPortalImpl.ts

@@ -13,13 +13,28 @@ const deployFn: DeployFunction = async (hre) => {
     'L2OutputOracleProxy'
   )
 
+  const finalSystemOwner = hre.deployConfig.finalSystemOwner
+  const finalSystemOwnerCode = await hre.ethers.provider.getCode(
+    finalSystemOwner
+  )
+  if (finalSystemOwnerCode === '0x') {
+    console.log(
+      `WARNING: setting OptimismPortal.GUARDIAN to ${finalSystemOwner} and it has no code`
+    )
+  }
+
+  // Deploy the OptimismPortal implementation as paused to
+  // ensure that users do not interact with it and instead
+  // interact with the proxied contract.
+  // The `finalSystemOwner` is set at the GUARDIAN.
   await deploy({
     hre,
     name: 'OptimismPortal',
     args: [
       L2OutputOracleProxy.address,
-      hre.deployConfig.finalSystemOwner,
+      finalSystemOwner,
       hre.deployConfig.finalizationPeriodSeconds,
+      true, // paused
     ],
     postDeployAction: async (contract) => {
       await assertContractVariable(

packages/contracts-bedrock/deploy/021-SystemDictatorSteps-2.ts

@@ -294,11 +294,15 @@ const deployFn: DeployFunction = async (hre) => {
 
   if (await isStep(SystemDictator, 6)) {
     console.log(`
-      Unpause the OptimismPortal
+      Unpause the OptimismPortal. The GUARDIAN account should be used. In practice
+      this is the multisig. In test networks, the OptimismPortal is initialized
+      without being paused.
     `)
 
     if (isLiveDeployer) {
-      console.log('OptimismPortal already not paused.')
+      console.log('WARNING: OptimismPortal configured to not be paused')
+      console.log('This should only happen for test environments')
+      await assertContractVariable(OptimismPortal, 'paused', false)
     } else {
       const tx = await OptimismPortal.populateTransaction.unpause()
       console.log(`Please unpause the OptimismPortal...`)
@@ -309,7 +313,8 @@ const deployFn: DeployFunction = async (hre) => {
 
     await awaitCondition(
       async () => {
-        return !OptimismPortal.paused()
+        const paused = await OptimismPortal.paused()
+        return !paused
       },
       30000,
       1000

packages/contracts-bedrock/invariant-docs/OptimismPortal.md

@@ -19,6 +19,6 @@ This invariant asserts that there is no chain of calls that can be made that wil
 
 
 ## Deposits of any value should always succeed unless `_to` = `address(0)` or `_isCreation` = `true`.
-**Test:** [`FuzzOptimismPortal.sol#L37`](../contracts/echidna/FuzzOptimismPortal.sol#L37)
+**Test:** [`FuzzOptimismPortal.sol#L42`](../contracts/echidna/FuzzOptimismPortal.sol#L42)
 
 All deposits, barring creation transactions and transactions sent to `address(0)`, should always succeed.