Add sequencer role to oracle (#2810)

* bedrock: Add sequencer role to output oracle

* bedrock: Test oracle access controls.

* chore(bedrock): bindings, snapshot, changeset

* bedrock: update oracle storage snapshot

* bedrock: deploy scripts with oracle owner

* test(bedrock): fix warp function in oracle test

* bedrock: Address review feedback

* bedrock: Generate unique deployer key in tests

* kick build
Co-authored-by: Matthew Slipper <me@matthewslipper.com>

.changeset/curvy-coins-push.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Separate the owner and sequencer roles in the OutputOracle

.changeset/famous-sloths-confess.md

+---
+'@eth-optimism/contracts-bedrock': minor
+---
+
+Add separate sequencer role to Oracle

op-e2e/setup.go

@@ -402,7 +402,7 @@ func (cfg SystemConfig) start() (*System, error) {
 	// Deploy Deposit Contract
 	deployerPrivKey, err := sys.wallet.PrivateKey(accounts.Account{
 		URL: accounts.URL{
-			Path: sys.cfg.DeployerHDPath,
+			Path: cfg.DeployerHDPath,
 		},
 	})
 	if err != nil {
@@ -425,6 +425,7 @@ func (cfg SystemConfig) start() (*System, error) {
 		sys.cfg.L2OOCfg.L2StartingTimeStamp,
 		sys.cfg.L2OOCfg.L2BlockTime,
 		l2OutputSubmitterAddr,
+		crypto.PubkeyToAddress(deployerPrivKey.PublicKey),
 	)
 	sys.cfg.DepositCFG.L2Oracle = sys.L2OOContractAddr
 	if err != nil {

op-e2e/system_test.go

@@ -57,6 +57,7 @@ const (
 	l2OutputHDPath     = "m/44'/60'/0'/0/3"
 	bssHDPath          = "m/44'/60'/0'/0/4"
 	p2pSignerHDPath    = "m/44'/60'/0'/0/5"
+	deployerHDPath     = "m/44'/60'/0'/0/6"
 )
 
 var (
@@ -81,6 +82,7 @@ func defaultSystemConfig(t *testing.T) SystemConfig {
 			transactorHDPath:   10000000,
 			l2OutputHDPath:     10000000,
 			bssHDPath:          10000000,
+			deployerHDPath:     10000000,
 		},
 		DepositCFG: DepositContractConfig{
 			FinalizationPeriod: big.NewInt(60 * 60 * 24),
@@ -94,7 +96,7 @@ func defaultSystemConfig(t *testing.T) SystemConfig {
 		L2OutputHDPath:             l2OutputHDPath,
 		BatchSubmitterHDPath:       bssHDPath,
 		P2PSignerHDPath:            p2pSignerHDPath,
-		DeployerHDPath:             l2OutputHDPath,
+		DeployerHDPath:             deployerHDPath,
 		CliqueSignerDerivationPath: cliqueSignerHDPath,
 		L1InfoPredeployAddress:     predeploys.L1BlockAddr,
 		L1BlockTime:                2,

packages/contracts-bedrock/.gas-snapshot

@@ -39,7 +39,7 @@ L1StandardBridge_Test:test_depositERC20() (gas: 452856)
 L1StandardBridge_Test:test_depositERC20To() (gas: 454632)
 L1StandardBridge_Test:test_depositETH() (gas: 247077)
 L1StandardBridge_Test:test_depositETHTo() (gas: 204961)
-L1StandardBridge_Test:test_finalizeERC20Withdrawal() (gas: 438752)
+L1StandardBridge_Test:test_finalizeERC20Withdrawal() (gas: 438745)
 L1StandardBridge_Test:test_finalizeETHWithdrawal() (gas: 48005)
 L1StandardBridge_Test:test_initialize() (gas: 14885)
 L1StandardBridge_Test:test_onlyEOADepositERC20() (gas: 12085)
@@ -57,21 +57,24 @@ 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: 18442)
-L2OutputOracleTest:testCannot_appendFutureTimetamp() (gas: 20072)
-L2OutputOracleTest:testCannot_appendOnWrongFork() (gas: 20710)
-L2OutputOracleTest:testCannot_appendOutputIfNotSequencer() (gas: 17829)
-L2OutputOracleTest:testCannot_appendUnexpectedBlockNumber() (gas: 20313)
-L2OutputOracleTest:testCannot_deleteL2Output_ifNotSequencer() (gas: 18805)
-L2OutputOracleTest:testCannot_deleteWrongL2Output() (gas: 79498)
-L2OutputOracleTest:test_appendWithBlockhashAndHeight() (gas: 69365)
-L2OutputOracleTest:test_appendingAnotherOutput() (gas: 70714)
-L2OutputOracleTest:test_computeL2Timestamp() (gas: 19230)
-L2OutputOracleTest:test_constructor() (gas: 33908)
-L2OutputOracleTest:test_deleteL2Output() (gas: 66081)
-L2OutputOracleTest:test_getL2Output() (gas: 76274)
-L2OutputOracleTest:test_latestBlockNumber() (gas: 70075)
-L2OutputOracleTest:test_nextBlockNumber() (gas: 9279)
+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)
 L2StandardBridge_Test:test_ERC20BridgeFailed_whenLocalTokenIsBridge() (gas: 133097)
 L2StandardBridge_Test:test_cannotWithdrawEthWithoutSendingIt() (gas: 21611)
 L2StandardBridge_Test:test_finalizeDeposit() (gas: 93100)
@@ -110,7 +113,7 @@ OptimismMintableTokenFactory_Test:test_initializeShouldRevert() (gas: 12696)
 OptimismPortal_Test:test_OptimismPortalConstructor() (gas: 11413)
 OptimismPortal_Test:test_OptimismPortalContractCreationReverts() (gas: 9214)
 OptimismPortal_Test:test_OptimismPortalReceiveEth() (gas: 121706)
-OptimismPortal_Test:test_cannotVerifyRecentWithdrawal() (gas: 21886)
+OptimismPortal_Test:test_cannotVerifyRecentWithdrawal() (gas: 21908)
 OptimismPortal_Test:test_depositTransaction_NoValueContract() (gas: 70746)
 OptimismPortal_Test:test_depositTransaction_NoValueEOA() (gas: 71114)
 OptimismPortal_Test:test_depositTransaction_createWithZeroValueForContract() (gas: 70773)
@@ -119,7 +122,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: 33702)
+OptimismPortal_Test:test_invalidWithdrawalProof() (gas: 33746)
 Proxy_Test:test_clashingFunctionSignatures() (gas: 101427)
 Proxy_Test:test_implementationKey() (gas: 20942)
 Proxy_Test:test_implementationProxyCallIfNotAdmin() (gas: 30021)

packages/contracts-bedrock/.storage-layout

@@ -69,9 +69,11 @@
 +======================================================================================================+
 | _owner            | address                                                  | 0    | 0      | 20    |
 |-------------------+----------------------------------------------------------+------+--------+-------|
-| latestBlockNumber | uint256                                                  | 1    | 0      | 32    |
+| sequencer         | address                                                  | 1    | 0      | 20    |
 |-------------------+----------------------------------------------------------+------+--------+-------|
-| l2Outputs         | mapping(uint256 => struct L2OutputOracle.OutputProposal) | 2    | 0      | 32    |
+| latestBlockNumber | uint256                                                  | 2    | 0      | 32    |
+|-------------------+----------------------------------------------------------+------+--------+-------|
+| l2Outputs         | mapping(uint256 => struct L2OutputOracle.OutputProposal) | 3    | 0      | 32    |
 +-------------------+----------------------------------------------------------+------+--------+-------+
 
 =======================

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

@@ -48,6 +48,14 @@ contract L2OutputOracle is Ownable {
         uint256 indexed _l2BlockNumber
     );
 
+    /**
+     * @notice Emitted when the sequencer address is changed.
+     *
+     * @param previousSequencer The previous sequencer address.
+     * @param newSequencer      The new sequencer address.
+     */
+    event SequencerChanged(address indexed previousSequencer, address indexed newSequencer);
+
     /**
      * @notice The interval in L2 blocks at which checkpoints must be submitted.
      */
@@ -73,6 +81,11 @@ contract L2OutputOracle is Ownable {
      */
     uint256 public immutable L2_BLOCK_TIME;
 
+    /**
+     * @notice The address of the sequencer;
+     */
+    address public sequencer;
+
     /**
      * @notice The number of the most recent L2 block recorded in this contract.
      */
@@ -85,6 +98,14 @@ contract L2OutputOracle is Ownable {
      */
     mapping(uint256 => OutputProposal) internal l2Outputs;
 
+    /**
+     * @notice Reverts if called by any account other than the sequencer.
+     */
+    modifier onlySequencer() {
+        require(sequencer == msg.sender, "OutputOracle: caller is not the sequencer");
+        _;
+    }
+
     /**
      * @notice Initialize the L2OutputOracle contract.
      *
@@ -97,6 +118,7 @@ contract L2OutputOracle is Ownable {
      * @param _startingTimestamp     The timestamp of the first L2 block.
      * @param _l2BlockTime           The timestamp of the first L2 block.
      * @param _sequencer             The address of the sequencer.
+     * @param _owner                 The address of the owner.
      */
     constructor(
         uint256 _submissionInterval,
@@ -105,7 +127,8 @@ contract L2OutputOracle is Ownable {
         uint256 _startingBlockNumber,
         uint256 _startingTimestamp,
         uint256 _l2BlockTime,
-        address _sequencer
+        address _sequencer,
+        address _owner
     ) {
         require(
             _l2BlockTime < block.timestamp,
@@ -120,7 +143,8 @@ contract L2OutputOracle is Ownable {
         l2Outputs[_startingBlockNumber] = OutputProposal(_genesisL2Output, block.timestamp);
         latestBlockNumber = _startingBlockNumber;
 
-        _transferOwnership(_sequencer);
+        changeSequencer(_sequencer);
+        _transferOwnership(_owner);
     }
 
     /**
@@ -139,7 +163,7 @@ contract L2OutputOracle is Ownable {
         uint256 _l2BlockNumber,
         bytes32 _l1Blockhash,
         uint256 _l1BlockNumber
-    ) external payable onlyOwner {
+    ) external payable onlySequencer {
         require(
             _l2BlockNumber == nextBlockNumber(),
             "OutputOracle: Block number must be equal to next expected block number."
@@ -172,8 +196,11 @@ contract L2OutputOracle is Ownable {
     }
 
     /**
-     * @notice Deletes the most recent output.
-     *
+     * @notice Deletes the most recent output. This is used to remove the most recent output in the
+     *         event that an erreneous output is submitted. It can only be called by the contract's
+     *         owner, not the sequencer. Longer term, this should be replaced with a more robust
+     *         mechanism which will allow deletion of proposals shown to be invalid by a fault
+     *         proof.
      * @param _proposal Represents the output proposal to delete
      */
     function deleteL2Output(OutputProposal memory _proposal) external onlyOwner {
@@ -230,4 +257,15 @@ contract L2OutputOracle is Ownable {
         return
             STARTING_TIMESTAMP + ((_l2BlockNumber - STARTING_BLOCK_NUMBER) * SUBMISSION_INTERVAL);
     }
+
+    /**
+     * @notice Transfers the sequencer role to a new account (`newSequencer`).
+     *         Can only be called by the current owner.
+     */
+    function changeSequencer(address _newSequencer) public onlyOwner {
+        require(_newSequencer != address(0), "OutputOracle: new sequencer is the zero address");
+        require(_newSequencer != owner(), "OutputOracle: sequencer cannot be same as the owner");
+        emit SequencerChanged(sequencer, _newSequencer);
+        sequencer = _newSequencer;
+    }
 }

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

@@ -42,18 +42,20 @@ contract CommonTest is Test {
         vm.fee(1000000000);
     }
 }
+
 contract L2OutputOracle_Initializer is CommonTest {
     // Test target
     L2OutputOracle oracle;
 
     // Constructor arguments
-    uint256 submissionInterval = 42;
+    address sequencer = 0x000000000000000000000000000000000000AbBa;
+    address owner = 0x000000000000000000000000000000000000ACDC;
+    uint256 submissionInterval = 1800;
+    uint256 l2BlockTime = 2;
     bytes32 genesisL2Output = keccak256(abi.encode(0));
     uint256 historicalTotalBlocks = 199;
     uint256 startingBlockNumber = 200;
     uint256 startingTimestamp = 1000;
-    uint256 l2BlockTime = 2;
-    address sequencer = 0x000000000000000000000000000000000000AbBa;
 
     // Test data
     uint256 initL1Time;
@@ -74,7 +76,8 @@ contract L2OutputOracle_Initializer is CommonTest {
             startingBlockNumber,
             startingTimestamp,
             l2BlockTime,
-            sequencer
+            sequencer,
+            owner
         );
     }
 }
@@ -82,8 +85,10 @@ contract L2OutputOracle_Initializer is CommonTest {
 contract Messenger_Initializer is L2OutputOracle_Initializer {
     OptimismPortal op;
     L1CrossDomainMessenger L1Messenger;
-    L2CrossDomainMessenger L2Messenger = L2CrossDomainMessenger(Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER);
-    L2ToL1MessagePasser messagePasser = L2ToL1MessagePasser(payable(Lib_PredeployAddresses.L2_TO_L1_MESSAGE_PASSER));
+    L2CrossDomainMessenger L2Messenger =
+        L2CrossDomainMessenger(Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER);
+    L2ToL1MessagePasser messagePasser =
+        L2ToL1MessagePasser(payable(Lib_PredeployAddresses.L2_TO_L1_MESSAGE_PASSER));
 
     event SentMessage(
         address indexed target,
@@ -139,20 +144,11 @@ contract Messenger_Initializer is L2OutputOracle_Initializer {
             address(new L2ToL1MessagePasser()).code
         );
 
-        vm.label(
-            Lib_PredeployAddresses.OVM_ETH,
-            "OVM_ETH"
-        );
+        vm.label(Lib_PredeployAddresses.OVM_ETH, "OVM_ETH");
 
-        vm.label(
-            Lib_PredeployAddresses.L2_TO_L1_MESSAGE_PASSER,
-            "L2ToL1MessagePasser"
-        );
+        vm.label(Lib_PredeployAddresses.L2_TO_L1_MESSAGE_PASSER, "L2ToL1MessagePasser");
 
-        vm.label(
-            Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER,
-            "L2CrossDomainMessenger"
-        );
+        vm.label(Lib_PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER, "L2CrossDomainMessenger");
 
         vm.label(
             AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger)),
@@ -274,14 +270,8 @@ contract Bridge_Initializer is Messenger_Initializer {
     function setUp() public virtual override {
         super.setUp();
 
-        vm.label(
-            Lib_PredeployAddresses.L2_STANDARD_BRIDGE,
-            "L2StandardBridge"
-        );
-        vm.label(
-            Lib_PredeployAddresses.L2_STANDARD_TOKEN_FACTORY,
-            "L2StandardTokenFactory"
-        );
+        vm.label(Lib_PredeployAddresses.L2_STANDARD_BRIDGE, "L2StandardBridge");
+        vm.label(Lib_PredeployAddresses.L2_STANDARD_TOKEN_FACTORY, "L2StandardTokenFactory");
 
         // Deploy the L1 bridge and initialize it with the address of the
         // L1CrossDomainMessenger
@@ -299,32 +289,34 @@ contract Bridge_Initializer is Messenger_Initializer {
         // Set up the L2 mintable token factory
         OptimismMintableTokenFactory factory = new OptimismMintableTokenFactory();
         vm.etch(Lib_PredeployAddresses.L2_STANDARD_TOKEN_FACTORY, address(factory).code);
-        L2TokenFactory = OptimismMintableTokenFactory(Lib_PredeployAddresses.L2_STANDARD_TOKEN_FACTORY);
+        L2TokenFactory = OptimismMintableTokenFactory(
+            Lib_PredeployAddresses.L2_STANDARD_TOKEN_FACTORY
+        );
         L2TokenFactory.initialize(Lib_PredeployAddresses.L2_STANDARD_BRIDGE);
 
-        vm.etch(
-            Lib_PredeployAddresses.OVM_ETH,
-            address(new OVM_ETH()).code
-        );
+        vm.etch(Lib_PredeployAddresses.OVM_ETH, address(new OVM_ETH()).code);
 
         L1Token = new ERC20("Native L1 Token", "L1T");
 
         // Deploy the L2 ERC20 now
-        L2Token = OptimismMintableERC20(L2TokenFactory.createStandardL2Token(
-            address(L1Token),
-            string(abi.encodePacked("L2-", L1Token.name())),
-            string(abi.encodePacked("L2-", L1Token.symbol()))
-        ));
+        L2Token = OptimismMintableERC20(
+            L2TokenFactory.createStandardL2Token(
+                address(L1Token),
+                string(abi.encodePacked("L2-", L1Token.name())),
+                string(abi.encodePacked("L2-", L1Token.symbol()))
+            )
+        );
 
         NativeL2Token = new ERC20("Native L2 Token", "L2T");
         L1TokenFactory = new OptimismMintableTokenFactory();
         L1TokenFactory.initialize(address(L1Bridge));
 
-        RemoteL1Token = OptimismMintableERC20(L1TokenFactory.createStandardL2Token(
-            address(NativeL2Token),
-            string(abi.encodePacked("L1-", NativeL2Token.name())),
-            string(abi.encodePacked("L1-", NativeL2Token.symbol()))
-        ));
+        RemoteL1Token = OptimismMintableERC20(
+            L1TokenFactory.createStandardL2Token(
+                address(NativeL2Token),
+                string(abi.encodePacked("L1-", NativeL2Token.name())),
+                string(abi.encodePacked("L1-", NativeL2Token.symbol()))
+            )
+        );
     }
 }
-

packages/contracts-bedrock/deploy-config/devnetL1.ts

@@ -9,6 +9,7 @@ const config = {
     parseInt(process.env.L2OO_STARTING_BLOCK_TIMESTAMP, 10) || Date.now(),
   l2BlockTime: 2,
   sequencerAddress: '0x70997970C51812dc3A010C7d01b50e0d17dc79C8',
+  ownerAddress: '0x70997970C51812dc3A010C7d01b50e0d17dc79C8',
 }
 
 export default config

packages/contracts-bedrock/deploy-config/hardhat.ts

@@ -9,6 +9,7 @@ const config = {
     parseInt(process.env.L2OO_STARTING_BLOCK_TIMESTAMP, 10) || Date.now(),
   l2BlockTime: 2,
   sequencerAddress: '0x70997970C51812dc3A010C7d01b50e0d17dc79C8',
+  ownerAddress: '0x70997970C51812dc3A010C7d01b50e0d17dc79C8',
 }
 
 export default config

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

@@ -25,6 +25,7 @@ const deployFn: DeployFunction = async (hre) => {
       hre.deployConfig.startingTimestamp,
       hre.deployConfig.l2BlockTime,
       hre.deployConfig.sequencerAddress,
+      hre.deployConfig.ownerAddress,
     ],
     log: true,
     waitConfirmations: 1,

packages/contracts-bedrock/hardhat.config.ts

@@ -80,6 +80,9 @@ const config: HardhatUserConfig = {
     sequencerAddress: {
       type: 'address',
     },
+    ownerAddress: {
+      type: 'address',
+    },
   },
   solidity: {
     compilers: [