Make OuputOracle upgradeable (#2826)

* bedrock: Add sequencer role to output oracle

* bedrock: Test oracle access controls.

* bedrock: Make the Output Oracle upgradeable

* chore(bedrock): bindings, snapshot, changeset

* update storage layout

* kick CI

* kick CI

* Archive personal fork then kick again

* kick build

* contracts-bedrock: new storage layout

* contracts-bedrock: fix deploy script
Co-authored-by: Matthew Slipper <me@matthewslipper.com>
Co-authored-by: Mark Tyneway <mark.tyneway@gmail.com>

.changeset/real-trainers-tie.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Make the output oracle upgradeable.

packages/contracts-bedrock/.gas-snapshot

@@ -57,24 +57,27 @@ L2CrossDomainMessenger_Test:test_L2MessengerSendMessage() (gas: 119682)
 L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 133142)
 L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10599)
 L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 54881)
-L2OutputOracleTest:testCannot_appendEmptyOutput() (gas: 18193)
-L2OutputOracleTest:testCannot_appendFutureTimetamp() (gas: 20160)
-L2OutputOracleTest:testCannot_appendOnWrongFork() (gas: 20439)
-L2OutputOracleTest:testCannot_appendOutputIfNotSequencer() (gas: 17584)
-L2OutputOracleTest:testCannot_appendUnexpectedBlockNumber() (gas: 20063)
-L2OutputOracleTest:testCannot_deleteL2Output_ifNotOwner() (gas: 18870)
-L2OutputOracleTest:testCannot_deleteL2Output_withWrongRoot() (gas: 83284)
-L2OutputOracleTest:testCannot_deleteL2Output_withWrongTime() (gas: 79277)
-L2OutputOracleTest:test_appendWithBlockhashAndHeight() (gas: 69103)
-L2OutputOracleTest:test_appendingAnotherOutput() (gas: 70429)
-L2OutputOracleTest:test_changeSequencer() (gas: 35175)
-L2OutputOracleTest:test_computeL2Timestamp() (gas: 19231)
-L2OutputOracleTest:test_constructor() (gas: 40084)
-L2OutputOracleTest:test_deleteL2Output() (gas: 69164)
-L2OutputOracleTest:test_getL2Output() (gas: 76076)
-L2OutputOracleTest:test_latestBlockNumber() (gas: 69832)
-L2OutputOracleTest:test_nextBlockNumber() (gas: 9280)
-L2OutputOracleTest:test_updateOwner() (gas: 24112)
+L2OutputOracleTest:testCannot_appendEmptyOutput() (gas: 18216)
+L2OutputOracleTest:testCannot_appendFutureTimetamp() (gas: 20183)
+L2OutputOracleTest:testCannot_appendOnWrongFork() (gas: 20462)
+L2OutputOracleTest:testCannot_appendOutputIfNotSequencer() (gas: 17607)
+L2OutputOracleTest:testCannot_appendUnexpectedBlockNumber() (gas: 20086)
+L2OutputOracleTest:testCannot_deleteL2Output_ifNotOwner() (gas: 18893)
+L2OutputOracleTest:testCannot_deleteL2Output_withWrongRoot() (gas: 83353)
+L2OutputOracleTest:testCannot_deleteL2Output_withWrongTime() (gas: 79346)
+L2OutputOracleTest:test_appendWithBlockhashAndHeight() (gas: 69126)
+L2OutputOracleTest:test_appendingAnotherOutput() (gas: 70475)
+L2OutputOracleTest:test_changeSequencer() (gas: 35473)
+L2OutputOracleTest:test_computeL2Timestamp() (gas: 19323)
+L2OutputOracleTest:test_constructor() (gas: 40152)
+L2OutputOracleTest:test_deleteL2Output() (gas: 69237)
+L2OutputOracleTest:test_getL2Output() (gas: 76099)
+L2OutputOracleTest:test_latestBlockNumber() (gas: 69878)
+L2OutputOracleTest:test_nextBlockNumber() (gas: 9281)
+L2OutputOracleTest:test_updateOwner() (gas: 24185)
+L2OutputOracleUpgradeable_Test:test_cannotInitImpl() (gas: 8431)
+L2OutputOracleUpgradeable_Test:test_cannotInitProxy() (gas: 13430)
+L2OutputOracleUpgradeable_Test:test_initValuesOnProxy() (gas: 38906)
 L2StandardBridge_Test:test_ERC20BridgeFailed_whenLocalTokenIsBridge() (gas: 133097)
 L2StandardBridge_Test:test_cannotWithdrawEthWithoutSendingIt() (gas: 21611)
 L2StandardBridge_Test:test_finalizeDeposit() (gas: 93100)
@@ -122,7 +125,7 @@ OptimismPortal_Test:test_depositTransaction_withEthValueAndContractContractCreat
 OptimismPortal_Test:test_depositTransaction_withEthValueAndEOAContractCreation() (gas: 69947)
 OptimismPortal_Test:test_depositTransaction_withEthValueFromContract() (gas: 77478)
 OptimismPortal_Test:test_depositTransaction_withEthValueFromEOA() (gas: 78049)
-OptimismPortal_Test:test_invalidWithdrawalProof() (gas: 33746)
+OptimismPortal_Test:test_invalidWithdrawalProof() (gas: 33769)
 Proxy_Test:test_clashingFunctionSignatures() (gas: 101427)
 Proxy_Test:test_implementationKey() (gas: 20942)
 Proxy_Test:test_implementationProxyCallIfNotAdmin() (gas: 30021)

packages/contracts-bedrock/.storage-layout

@@ -67,13 +67,21 @@
 +-------------------+----------------------------------------------------------+------+--------+-------+
 | Name              | Type                                                     | Slot | Offset | Bytes |
 +======================================================================================================+
-| _owner            | address                                                  | 0    | 0      | 20    |
+| _initialized      | uint8                                                    | 0    | 0      | 1     |
 |-------------------+----------------------------------------------------------+------+--------+-------|
-| sequencer         | address                                                  | 1    | 0      | 20    |
+| _initializing     | bool                                                     | 0    | 1      | 1     |
 |-------------------+----------------------------------------------------------+------+--------+-------|
-| latestBlockNumber | uint256                                                  | 2    | 0      | 32    |
+| __gap             | uint256[50]                                              | 1    | 0      | 1600  |
 |-------------------+----------------------------------------------------------+------+--------+-------|
-| l2Outputs         | mapping(uint256 => struct L2OutputOracle.OutputProposal) | 3    | 0      | 32    |
+| _owner            | address                                                  | 51   | 0      | 20    |
+|-------------------+----------------------------------------------------------+------+--------+-------|
+| __gap             | uint256[49]                                              | 52   | 0      | 1568  |
+|-------------------+----------------------------------------------------------+------+--------+-------|
+| sequencer         | address                                                  | 101  | 0      | 20    |
+|-------------------+----------------------------------------------------------+------+--------+-------|
+| latestBlockNumber | uint256                                                  | 102  | 0      | 32    |
+|-------------------+----------------------------------------------------------+------+--------+-------|
+| l2Outputs         | mapping(uint256 => struct L2OutputOracle.OutputProposal) | 103  | 0      | 32    |
 +-------------------+----------------------------------------------------------+------+--------+-------+
 
 =======================

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

 //SPDX-License-Identifier: MIT
 pragma solidity 0.8.10;
 
-import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
+import {
+    OwnableUpgradeable
+} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 
 /**
  * @title L2OutputOracle
@@ -10,7 +12,12 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
  *         This contract should be deployed behind an upgradable proxy
  */
 // slither-disable-next-line locked-ether
-contract L2OutputOracle is Ownable {
+contract L2OutputOracle is OwnableUpgradeable {
+    /**
+     * @notice Contract version number.
+     */
+    uint8 public constant L2_OUTPUT_ORACLE_VERSION = 1;
+
     /**
      * @notice OutputProposal represents a commitment to the L2 state.
      *         The timestamp is the L1 timestamp that the output root is posted.
@@ -140,8 +147,26 @@ contract L2OutputOracle is Ownable {
         STARTING_TIMESTAMP = _startingTimestamp;
         L2_BLOCK_TIME = _l2BlockTime;
 
+        initialize(_genesisL2Output, _startingBlockNumber, _sequencer, _owner);
+    }
+
+    /**
+     * @notice Initialize the L2OutputOracle contract.
+     *
+     * @param _genesisL2Output        The initial L2 output of the L2 chain.
+     * @param _startingBlockNumber The timestamp to start L2 block at.
+     * @param _sequencer              The address of the sequencer.
+     * @param _owner                 The address of the owner.
+     */
+    function initialize(
+        bytes32 _genesisL2Output,
+        uint256 _startingBlockNumber,
+        address _sequencer,
+        address _owner
+    ) public reinitializer(L2_OUTPUT_ORACLE_VERSION) {
         l2Outputs[_startingBlockNumber] = OutputProposal(_genesisL2Output, block.timestamp);
         latestBlockNumber = _startingBlockNumber;
+        __Ownable_init();
 
         changeSequencer(_sequencer);
         _transferOwnership(_owner);

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

@@ -3,6 +3,7 @@ pragma solidity 0.8.10;
 
 import { L2OutputOracle_Initializer } from "./CommonTest.t.sol";
 import { L2OutputOracle } from "../L1/L2OutputOracle.sol";
+import { Proxy } from "../universal/Proxy.sol";
 
 contract L2OutputOracleTest is L2OutputOracle_Initializer {
     bytes32 appendedOutput1 = keccak256(abi.encode(1));
@@ -138,8 +139,6 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         vm.stopPrank();
     }
 
-    // Test updating owner
-
     /*****************************
      * Append Tests - Happy Path *
      *****************************/
@@ -335,3 +334,54 @@ contract L2OutputOracleTest is L2OutputOracle_Initializer {
         oracle.deleteL2Output(proposalToDelete);
     }
 }
+
+contract L2OutputOracleUpgradeable_Test is L2OutputOracle_Initializer {
+    L2OutputOracle oracleImpl;
+    Proxy internal proxy;
+    uint64 initialBlockNum;
+
+    function setUp() override public {
+        super.setUp();
+        initialBlockNum = uint64(block.number);
+
+        // Rename the deployed oracle instance for the purposes of this test.
+        oracleImpl = oracle;
+        proxy = new Proxy(alice);
+        vm.prank(alice);
+        proxy.upgradeToAndCall(
+            address(oracleImpl),
+            abi.encodeWithSelector(
+                L2OutputOracle.initialize.selector,
+                genesisL2Output,
+                startingBlockNumber,
+                sequencer,
+                owner
+            )
+        );
+    }
+
+    function test_initValuesOnProxy() external {
+        assertEq(submissionInterval, oracleImpl.SUBMISSION_INTERVAL());
+        assertEq(historicalTotalBlocks, oracleImpl.HISTORICAL_TOTAL_BLOCKS());
+        assertEq(startingBlockNumber, oracleImpl.STARTING_BLOCK_NUMBER());
+        assertEq(startingTimestamp, oracleImpl.STARTING_TIMESTAMP());
+        assertEq(l2BlockTime, oracleImpl.L2_BLOCK_TIME());
+
+        L2OutputOracle.OutputProposal memory initOutput = oracleImpl.getL2Output(startingBlockNumber);
+        assertEq(genesisL2Output, initOutput.outputRoot);
+        assertEq(initL1Time, initOutput.timestamp);
+
+        assertEq(sequencer, oracleImpl.sequencer());
+        assertEq(owner, oracleImpl.owner());
+    }
+
+    function test_cannotInitProxy() external {
+        vm.expectRevert("Initializable: contract is already initialized");
+        address(proxy).call(abi.encodeWithSelector(L2OutputOracle.initialize.selector));
+    }
+
+    function test_cannotInitImpl() external {
+        vm.expectRevert("Initializable: contract is already initialized");
+        address(oracleImpl).call(abi.encodeWithSelector(L2OutputOracle.initialize.selector));
+    }
+}

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

@@ -19,8 +19,7 @@ contract OptimismPortal_Test is L2OutputOracle_Initializer {
         bytes data
     );
 
-    // Dependencies
-    // L2OutputOracle oracle;
+    // Test target
     OptimismPortal op;
 
     function setUp() public override {

packages/contracts-bedrock/deploy/000-L2OutputOracle.deploy.ts

@@ -46,14 +46,26 @@ const deployFn: DeployFunction = async (hre) => {
   const oracle = await hre.deployments.get('L2OutputOracle')
   const proxy = await hre.deployments.get('L2OutputOracleProxy')
   const Proxy = await hre.ethers.getContractAt('Proxy', proxy.address)
-  const tx = await Proxy.upgradeTo(oracle.address)
-  await tx.wait()
 
   const L2OutputOracle = await hre.ethers.getContractAt(
     'L2OutputOracle',
     proxy.address
   )
 
+  const tx = await Proxy.upgradeToAndCall(
+    oracle.address,
+    L2OutputOracle.interface.encodeFunctionData(
+      'initialize(bytes32,uint256,address,address)',
+      [
+        deployConfig.genesisOutput,
+        deployConfig.startingBlockNumber,
+        deployConfig.sequencerAddress,
+        deployConfig.ownerAddress,
+      ]
+    )
+  )
+  await tx.wait()
+
   const submissionInterval = await L2OutputOracle.SUBMISSION_INTERVAL()
   if (!submissionInterval.eq(BigNumber.from(deployConfig.submissionInterval))) {
     throw new Error('submission internal misconfigured')