Merge branch 'develop' into jg/fix_ci

.changeset/lazy-seahorses-hug.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Cleans up various compiler warnings

.changeset/wild-rivers-guess.md

+---
+'@eth-optimism/fault-detector': patch
+---
+
+Properly handle connection failures for L2 node

op-e2e/system_test.go

@@ -165,7 +165,7 @@ func TestL2OutputSubmitter(t *testing.T) {
 
 	l1Client := sys.Clients["l1"]
 
-	rollupRPCClient, err := rpc.DialContext(context.Background(), fmt.Sprintf("http://%s:%d", cfg.Nodes["sequencer"].RPC.ListenAddr, cfg.Nodes["sequencer"].RPC.ListenPort))
+	rollupRPCClient, err := rpc.DialContext(context.Background(), cfg.Nodes["sequencer"].RPC.HttpEndpoint())
 	require.Nil(t, err)
 	rollupClient := rollupclient.NewRollupClient(rollupRPCClient)
 
@@ -326,6 +326,18 @@ func TestSystemE2E(t *testing.T) {
 	require.Equal(t, verifBlock.NumberU64(), seqBlock.NumberU64(), "Verifier and sequencer blocks not the same after including a batch tx")
 	require.Equal(t, verifBlock.ParentHash(), seqBlock.ParentHash(), "Verifier and sequencer blocks parent hashes not the same after including a batch tx")
 	require.Equal(t, verifBlock.Hash(), seqBlock.Hash(), "Verifier and sequencer blocks not the same after including a batch tx")
+
+	rollupRPCClient, err := rpc.DialContext(context.Background(), cfg.Nodes["sequencer"].RPC.HttpEndpoint())
+	require.Nil(t, err)
+	rollupClient := rollupclient.NewRollupClient(rollupRPCClient)
+	// basic check that sync status works
+	seqStatus, err := rollupClient.SyncStatus(context.Background())
+	require.Nil(t, err)
+	require.LessOrEqual(t, seqBlock.NumberU64(), seqStatus.UnsafeL2.Number)
+	// basic check that version endpoint works
+	seqVersion, err := rollupClient.Version(context.Background())
+	require.Nil(t, err)
+	require.NotEqual(t, "", seqVersion)
 }
 
 // TestConfirmationDepth runs the rollup with both sequencer and verifier not immediately processing the tip of the chain.

op-node/node/api.go

@@ -87,9 +87,17 @@ func (n *nodeAPI) OutputAtBlock(ctx context.Context, number rpc.BlockNumber) ([]
 }
 
 func (n *nodeAPI) SyncStatus(ctx context.Context) (*driver.SyncStatus, error) {
+	recordDur := n.m.RecordRPCServerRequest("optimism_syncStatus")
+	defer recordDur()
 	return n.dr.SyncStatus(ctx)
 }
 
+func (n *nodeAPI) RollupConfig(_ context.Context) (*rollup.Config, error) {
+	recordDur := n.m.RecordRPCServerRequest("optimism_rollupConfig")
+	defer recordDur()
+	return n.config, nil
+}
+
 func (n *nodeAPI) Version(ctx context.Context) (string, error) {
 	recordDur := n.m.RecordRPCServerRequest("optimism_version")
 	defer recordDur()

op-node/node/config.go

@@ -37,6 +37,10 @@ type RPCConfig struct {
 	ListenPort int
 }
 
+func (cfg *RPCConfig) HttpEndpoint() string {
+	return fmt.Sprintf("http://%s:%d", cfg.ListenAddr, cfg.ListenPort)
+}
+
 type MetricsConfig struct {
 	Enabled    bool
 	ListenAddr string

op-proposer/rollupclient/rollupclient.go

@@ -5,6 +5,7 @@ import (
 	"math/big"
 
 	"github.com/ethereum-optimism/optimism/op-node/eth"
+	"github.com/ethereum-optimism/optimism/op-node/rollup"
 	"github.com/ethereum-optimism/optimism/op-node/rollup/driver"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/rpc"
@@ -30,6 +31,12 @@ func (r *RollupClient) SyncStatus(ctx context.Context) (*driver.SyncStatus, erro
 	return output, err
 }
 
+func (r *RollupClient) RollupConfig(ctx context.Context) (*rollup.Config, error) {
+	var output *rollup.Config
+	err := r.rpc.CallContext(ctx, &output, "optimism_rollupConfig")
+	return output, err
+}
+
 func (r *RollupClient) Version(ctx context.Context) (string, error) {
 	var output string
 	err := r.rpc.CallContext(ctx, &output, "optimism_version")

packages/contracts-bedrock/contracts/legacy/LegacyERC20ETH.sol

@@ -23,87 +23,62 @@ contract LegacyERC20ETH is OptimismMintableERC20 {
     {}
 
     /**
+     * @custom:blocked
      * @notice Mints some amount of ETH.
-     *
-     * @param _to     Address of the recipient.
-     * @param _amount Amount of ETH to mint.
      */
-    function mint(address _to, uint256 _amount) public virtual override {
+    function mint(address, uint256) public virtual override {
         revert("LegacyERC20ETH: mint is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Burns some amount of ETH.
-     *
-     * @param _from   Address to burn from.
-     * @param _amount Amount of ETH to burn.
      */
-    function burn(address _from, uint256 _amount) public virtual override {
+    function burn(address, uint256) public virtual override {
         revert("LegacyERC20ETH: burn is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Transfers some amount of ETH.
-     *
-     * @param _recipient Address to send to.
-     * @param _amount    Amount of ETH to send.
      */
-    function transfer(address _recipient, uint256 _amount) public virtual override returns (bool) {
+    function transfer(address, uint256) public virtual override returns (bool) {
         revert("LegacyERC20ETH: transfer is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Approves a spender to spend some amount of ETH.
-     *
-     * @param _spender Address of the spender.
-     * @param _amount  Amount of ETH to approve.
      */
-    function approve(address _spender, uint256 _amount) public virtual override returns (bool) {
+    function approve(address, uint256) public virtual override returns (bool) {
         revert("LegacyERC20ETH: approve is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Transfers funds from some sender account.
-     *
-     * @param _sender    Address of the sender.
-     * @param _recipient Address of the recipient.
-     * @param _amount    Amount of ETH to transfer.
      */
     function transferFrom(
-        address _sender,
-        address _recipient,
-        uint256 _amount
+        address,
+        address,
+        uint256
     ) public virtual override returns (bool) {
         revert("LegacyERC20ETH: transferFrom is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Increases the allowance of a spender.
-     *
-     * @param _spender    Address of the spender.
-     * @param _addedValue Amount of ETH to increase the allowance by.
      */
-    function increaseAllowance(address _spender, uint256 _addedValue)
-        public
-        virtual
-        override
-        returns (bool)
-    {
+    function increaseAllowance(address, uint256) public virtual override returns (bool) {
         revert("LegacyERC20ETH: increaseAllowance is disabled");
     }
 
     /**
+     * @custom:blocked
      * @notice Decreases the allowance of a spender.
-     *
-     * @param _spender         Address of the spender.
-     * @param _subtractedValue Amount of ETH to decrease the allowance by.
      */
-    function decreaseAllowance(address _spender, uint256 _subtractedValue)
-        public
-        virtual
-        override
-        returns (bool)
-    {
+    function decreaseAllowance(address, uint256) public virtual override returns (bool) {
         revert("LegacyERC20ETH: decreaseAllowance is disabled");
     }
 }

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

@@ -198,7 +198,6 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         address sender = PredeployAddresses.L2_CROSS_DOMAIN_MESSENGER;
 
         uint256 senderSlotIndex = 51;
-        bytes32 slotValue = vm.load(address(op), bytes32(senderSlotIndex));
 
         vm.store(address(op), bytes32(senderSlotIndex), bytes32(abi.encode(sender)));
         vm.prank(address(op));

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

@@ -59,7 +59,8 @@ contract L1StandardBridge_Test is Bridge_Initializer {
         );
 
         vm.prank(alice, alice);
-        address(L1Bridge).call{ value: 100 }(hex"");
+        (bool success,) = address(L1Bridge).call{ value: 100 }(hex"");
+        assertEq(success, true);
         assertEq(address(op).balance, 100);
     }
 

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

@@ -357,12 +357,22 @@ contract L2OutputOracleUpgradeable_Test is L2OutputOracle_Initializer {
 
     function test_cannotInitProxy() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        address(proxy).call(abi.encodeWithSelector(L2OutputOracle.initialize.selector));
+        L2OutputOracle(payable(proxy)).initialize(
+            genesisL2Output,
+            startingBlockNumber,
+            sequencer,
+            owner
+        );
     }
 
     function test_cannotInitImpl() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        address(oracleImpl).call(abi.encodeWithSelector(L2OutputOracle.initialize.selector));
+        L2OutputOracle(oracleImpl).initialize(
+            genesisL2Output,
+            startingBlockNumber,
+            sequencer,
+            owner
+        );
     }
 
     function test_upgrading() external {

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

@@ -40,7 +40,8 @@ contract L2StandardBridge_Test is Bridge_Initializer {
         // TODO: events from each contract
 
         vm.prank(alice, alice);
-        address(L2Bridge).call{ value: 100 }(hex"");
+        (bool success,) = address(L2Bridge).call{ value: 100 }(hex"");
+        assertEq(success, true);
         assertEq(address(messagePasser).balance, 100);
     }
 

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

@@ -334,12 +334,12 @@ contract OptimismPortalUpgradeable_Test is Portal_Initializer {
 
     function test_cannotInitProxy() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        address(proxy).call(abi.encodeWithSelector(OptimismPortal.initialize.selector));
+        OptimismPortal(payable(proxy)).initialize();
     }
 
     function test_cannotInitImpl() external {
         vm.expectRevert("Initializable: contract is already initialized");
-        address(opImpl).call(abi.encodeWithSelector(OptimismPortal.initialize.selector));
+        OptimismPortal(opImpl).initialize();
     }
 
     function test_upgrading() external {

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

@@ -18,7 +18,7 @@ contract SimpleStorage {
 }
 
 contract Clasher {
-    function upgradeTo(address _implementation) external view {
+    function upgradeTo(address) external pure {
         revert("upgradeTo");
     }
 }
@@ -150,8 +150,8 @@ contract Proxy_Test is Test {
     function test_upgradeToAndCall() external {
         {
             // There should be nothing in the current proxy storage
-            uint256 result = SimpleStorage(address(proxy)).get(1);
-            assertEq(result, 0);
+            uint256 expect = SimpleStorage(address(proxy)).get(1);
+            assertEq(expect, 0);
         }
 
         // Deploy a new SimpleStorage

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

@@ -92,12 +92,12 @@ contract ResourceMetering_Test is CommonTest {
         uint64 elasticity = uint64(uint256(meter.ELASTICITY_MULTIPLIER()));
         meter.use(target * elasticity);
 
-        (uint128 prevBaseFee, uint64 prevBoughtGas, uint64 prevBlockNum) = meter.params();
+        (, uint64 prevBoughtGas, ) = meter.params();
         assertEq(prevBoughtGas, target * elasticity);
 
         vm.roll(initialBlockNum + 1);
         meter.use(0);
-        (uint128 postBaseFee, uint64 postBoughtGas, uint64 postBlockNum) = meter.params();
+        (uint128 postBaseFee,,) = meter.params();
         // Base fee increases by 1/8 the difference
         assertEq(postBaseFee, 1375000000);
     }

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

@@ -48,8 +48,9 @@ contract SequencerFeeVault_Test is Bridge_Initializer {
         );
 
         vm.prank(alice);
-        address(vault).call{ value: 100 }(hex"");
+        (bool success,) = address(vault).call{ value: 100 }(hex"");
 
+        assertEq(success, true);
         assertEq(
             address(vault).balance,
             100

packages/fault-detector/src/service.ts

@@ -2,7 +2,7 @@ import { BaseServiceV2, Gauge, validators } from '@eth-optimism/common-ts'
 import { getChainId, sleep, toRpcHexString } from '@eth-optimism/core-utils'
 import { CrossChainMessenger } from '@eth-optimism/sdk'
 import { Provider } from '@ethersproject/abstract-provider'
-import { Contract, ethers } from 'ethers'
+import { Contract, ethers, Transaction } from 'ethers'
 import dateformat from 'dateformat'
 
 import {
@@ -20,10 +20,12 @@ type Metrics = {
   highestCheckedBatchIndex: Gauge
   highestKnownBatchIndex: Gauge
   isCurrentlyMismatched: Gauge
-  inUnexpectedErrorState: Gauge
+  l1NodeConnectionFailures: Gauge
+  l2NodeConnectionFailures: Gauge
 }
 
 type State = {
+  fpw: number
   scc: Contract
   messenger: CrossChainMessenger
   highestCheckedBatchIndex: number
@@ -68,9 +70,13 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
           type: Gauge,
           desc: '0 if state is ok, 1 if state is mismatched',
         },
-        inUnexpectedErrorState: {
+        l1NodeConnectionFailures: {
           type: Gauge,
-          desc: '0 if service is ok, 1 service is in unexpected error state',
+          desc: 'Number of times L1 node connection has failed',
+        },
+        l2NodeConnectionFailures: {
+          type: Gauge,
+          desc: 'Number of times L2 node connection has failed',
         },
       },
     })
@@ -86,6 +92,7 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
 
     // We use this a lot, a bit cleaner to pull out to the top level of the state object.
     this.state.scc = this.state.messenger.contracts.l1.StateCommitmentChain
+    this.state.fpw = (await this.state.scc.FRAUD_PROOF_WINDOW()).toNumber()
 
     // Figure out where to start syncing from.
     if (this.options.startBatchIndex === -1) {
@@ -102,17 +109,30 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
   }
 
   async main(): Promise<void> {
-    const latestBatchIndex = await this.state.scc.getTotalBatches()
-    if (this.state.highestCheckedBatchIndex >= latestBatchIndex.toNumber()) {
+    let latestBatchIndex: number
+    try {
+      latestBatchIndex = (await this.state.scc.getTotalBatches()).toNumber()
+    } catch (err) {
+      this.logger.error(`got error when connecting to node`, {
+        error: err,
+        node: 'l1',
+        section: 'getTotalBatches',
+      })
+      this.metrics.l1NodeConnectionFailures.inc()
       await sleep(15000)
       return
     }
 
-    this.metrics.highestKnownBatchIndex.set(latestBatchIndex.toNumber())
+    if (this.state.highestCheckedBatchIndex >= latestBatchIndex) {
+      await sleep(15000)
+      return
+    } else {
+      this.metrics.highestKnownBatchIndex.set(latestBatchIndex)
+    }
 
     this.logger.info(`checking batch`, {
       batchIndex: this.state.highestCheckedBatchIndex,
-      latestIndex: latestBatchIndex.toNumber(),
+      latestIndex: latestBatchIndex,
     })
 
     let event: ethers.Event
@@ -122,13 +142,30 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
         this.state.highestCheckedBatchIndex
       )
     } catch (err) {
-      this.logger.error(`got unexpected error while searching for batch`, {
-        batchIndex: this.state.highestCheckedBatchIndex,
+      this.logger.error(`got error when connecting to node`, {
+        error: err,
+        node: 'l1',
+        section: 'findEventForStateBatch',
+      })
+      this.metrics.l1NodeConnectionFailures.inc()
+      await sleep(15000)
+      return
+    }
+
+    let batchTransaction: Transaction
+    try {
+      batchTransaction = await event.getTransaction()
+    } catch (err) {
+      this.logger.error(`got error when connecting to node`, {
         error: err,
+        node: 'l1',
+        section: 'getTransaction',
       })
+      this.metrics.l1NodeConnectionFailures.inc()
+      await sleep(15000)
+      return
     }
 
-    const batchTransaction = await event.getTransaction()
     const [stateRoots] = this.state.scc.interface.decodeFunctionData(
       'appendStateBatch',
       batchTransaction.data
@@ -138,7 +175,20 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
     const batchSize = event.args._batchSize.toNumber()
     const batchEnd = batchStart + batchSize
 
-    const latestBlock = await this.options.l2RpcProvider.getBlockNumber()
+    let latestBlock: number
+    try {
+      latestBlock = await this.options.l2RpcProvider.getBlockNumber()
+    } catch (err) {
+      this.logger.error(`got error when connecting to node`, {
+        error: err,
+        node: 'l2',
+        section: 'getBlockNumber',
+      })
+      this.metrics.l2NodeConnectionFailures.inc()
+      await sleep(15000)
+      return
+    }
+
     if (latestBlock < batchEnd) {
       this.logger.info(`node is behind, waiting for sync`, {
         batchEnd,
@@ -151,21 +201,32 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
     // multiple requests of maximum 1000 blocks in the case that batchSize > 1000.
     let blocks: any[] = []
     for (let i = 0; i < batchSize; i += 1000) {
-      const provider = this.options
-        .l2RpcProvider as ethers.providers.JsonRpcProvider
-      blocks = blocks.concat(
-        await provider.send('eth_getBlockRange', [
+      let newBlocks: any[]
+      try {
+        newBlocks = await (
+          this.options.l2RpcProvider as ethers.providers.JsonRpcProvider
+        ).send('eth_getBlockRange', [
           toRpcHexString(batchStart + i),
           toRpcHexString(batchStart + i + Math.min(batchSize - i, 1000) - 1),
           false,
         ])
-      )
+      } catch (err) {
+        this.logger.error(`got error when connecting to node`, {
+          error: err,
+          node: 'l2',
+          section: 'getBlockRange',
+        })
+        this.metrics.l2NodeConnectionFailures.inc()
+        await sleep(15000)
+        return
+      }
+
+      blocks = blocks.concat(newBlocks)
     }
 
     for (const [i, stateRoot] of stateRoots.entries()) {
       if (blocks[i].stateRoot !== stateRoot) {
         this.metrics.isCurrentlyMismatched.set(1)
-        const fpw = await this.state.scc.FRAUD_PROOF_WINDOW()
         this.logger.error(`state root mismatch`, {
           blockNumber: blocks[i].number,
           expectedStateRoot: blocks[i].stateRoot,
@@ -173,7 +234,7 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
           finalizationTime: dateformat(
             new Date(
               (ethers.BigNumber.from(blocks[i].timestamp).toNumber() +
-                fpw.toNumber()) *
+                this.state.fpw) *
                 1000
             ),
             'mmmm dS, yyyy, h:MM:ss TT'
@@ -190,7 +251,6 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
 
     // If we got through the above without throwing an error, we should be fine to reset.
     this.metrics.isCurrentlyMismatched.set(0)
-    this.metrics.inUnexpectedErrorState.set(0)
   }
 }