fix(fd): start at beginning of challenge period (#2629)

Updates the fault-detector to use a smarter starting height by default.
Specifically, the fault-detector will now start at the first batch that
has not yet crossed the challenge period, since the general trust
assumption is that invalid batches will not pass the challenge period.
Significantly reduces verification time, especially since the fault
detector is stateless and does not keep track of the highest verified
batch.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.changeset/ninety-boxes-melt.md

+---
+'@eth-optimism/fault-detector': patch
+---
+
+Smarter starting height for fault-detector

packages/fault-detector/hardhat.config.ts

+import { HardhatUserConfig } from 'hardhat/types'
+
+// Hardhat plugins
+import '@nomiclabs/hardhat-ethers'
+import '@nomiclabs/hardhat-waffle'
+
+const config: HardhatUserConfig = {
+  mocha: {
+    timeout: 50000,
+  },
+}
+
+export default config

packages/fault-detector/package.json

@@ -10,7 +10,8 @@
   ],
   "scripts": {
     "start": "ts-node ./src/service.ts",
-    "test:coverage": "echo 'No tests defined.'",
+    "test": "hardhat test",
+    "test:coverage": "nyc hardhat test && nyc merge .nyc_output coverage.json",
     "build": "tsc -p tsconfig.json",
     "clean": "rimraf dist/ ./tsconfig.tsbuildinfo",
     "lint": "yarn lint:fix && yarn lint:check",
@@ -32,13 +33,22 @@
     "url": "https://github.com/ethereum-optimism/optimism.git"
   },
   "devDependencies": {
+    "@defi-wonderland/smock": "^2.0.7",
+    "@nomiclabs/hardhat-ethers": "^2.0.6",
+    "@nomiclabs/hardhat-waffle": "^2.0.3",
+    "@types/chai": "^4.3.1",
     "@types/dateformat": "^5.0.0",
+    "chai-as-promised": "^7.1.1",
     "dateformat": "^4.5.1",
+    "ethereum-waffle": "^3.4.4",
     "ethers": "^5.6.8",
+    "hardhat": "^2.9.6",
+    "lodash": "^4.17.21",
     "ts-node": "^10.7.0"
   },
   "dependencies": {
     "@eth-optimism/common-ts": "^0.2.8",
+    "@eth-optimism/contracts": "^0.5.24",
     "@eth-optimism/core-utils": "^0.8.5",
     "@eth-optimism/sdk": "^1.1.6",
     "@ethersproject/abstract-provider": "^5.6.1"

packages/fault-detector/src/helpers.ts

+import { Contract, ethers } from 'ethers'
+
+/**
+ * Finds the Event that corresponds to a given state batch by index.
+ *
+ * @param scc StateCommitmentChain contract.
+ * @param index State batch index to search for.
+ * @returns Event corresponding to the batch.
+ */
+export const findEventForStateBatch = async (
+  scc: Contract,
+  index: number
+): Promise<ethers.Event> => {
+  const events = await scc.queryFilter(scc.filters.StateBatchAppended(index))
+
+  // Only happens if the batch with the given index does not exist yet.
+  if (events.length === 0) {
+    throw new Error(`unable to find event for batch`)
+  }
+
+  // Should never happen.
+  if (events.length > 1) {
+    throw new Error(`found too many events for batch`)
+  }
+
+  return events[0]
+}
+
+/**
+ * Finds the first state batch index that has not yet passed the fault proof window.
+ *
+ * @param scc StateCommitmentChain contract.
+ * @returns Starting state root batch index.
+ */
+export const findFirstUnfinalizedStateBatchIndex = async (
+  scc: Contract
+): Promise<number> => {
+  const fpw = (await scc.FRAUD_PROOF_WINDOW()).toNumber()
+  const latestBlock = await scc.provider.getBlock('latest')
+  const totalBatches = (await scc.getTotalBatches()).toNumber()
+
+  // Perform a binary search to find the next batch that will pass the challenge period.
+  let lo = 0
+  let hi = totalBatches
+  while (lo !== hi) {
+    const mid = Math.floor((lo + hi) / 2)
+    const event = await findEventForStateBatch(scc, mid)
+    const block = await event.getBlock()
+
+    if (block.timestamp + fpw < latestBlock.timestamp) {
+      lo = mid + 1
+    } else {
+      hi = mid
+    }
+  }
+
+  // Result will be zero if the chain is less than FPW seconds old. Only returns undefined in the
+  // case that no batches have been submitted for an entire challenge period.
+  if (lo === totalBatches) {
+    return undefined
+  } else {
+    return lo
+  }
+}

packages/fault-detector/src/index.ts

 export * from './service'
+export * from './helpers'

packages/fault-detector/src/service.ts

 import { BaseServiceV2, Gauge, validators } from '@eth-optimism/common-ts'
-import { sleep, toRpcHexString } from '@eth-optimism/core-utils'
+import { getChainId, sleep, toRpcHexString } from '@eth-optimism/core-utils'
 import { CrossChainMessenger } from '@eth-optimism/sdk'
 import { Provider } from '@ethersproject/abstract-provider'
-import { ethers } from 'ethers'
+import { Contract, ethers } from 'ethers'
 import dateformat from 'dateformat'
 
+import {
+  findFirstUnfinalizedStateBatchIndex,
+  findEventForStateBatch,
+} from './helpers'
+
 type Options = {
   l1RpcProvider: Provider
   l2RpcProvider: Provider
@@ -19,6 +24,7 @@ type Metrics = {
 }
 
 type State = {
+  scc: Contract
   messenger: CrossChainMessenger
   highestCheckedBatchIndex: number
 }
@@ -41,7 +47,7 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
         },
         startBatchIndex: {
           validator: validators.num,
-          default: 0,
+          default: -1,
           desc: 'Batch index to start checking from',
         },
       },
@@ -67,19 +73,31 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
   }
 
   async init(): Promise<void> {
-    const network = await this.options.l1RpcProvider.getNetwork()
     this.state.messenger = new CrossChainMessenger({
       l1SignerOrProvider: this.options.l1RpcProvider,
       l2SignerOrProvider: this.options.l2RpcProvider,
-      l1ChainId: network.chainId,
+      l1ChainId: await getChainId(this.options.l1RpcProvider),
     })
 
-    this.state.highestCheckedBatchIndex = this.options.startBatchIndex
+    // 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
+
+    // Figure out where to start syncing from.
+    if (this.options.startBatchIndex === -1) {
+      this.logger.info(`finding appropriate starting height`)
+      this.state.highestCheckedBatchIndex =
+        await findFirstUnfinalizedStateBatchIndex(this.state.scc)
+    } else {
+      this.state.highestCheckedBatchIndex = this.options.startBatchIndex
+    }
+
+    this.logger.info(`starting height`, {
+      startBatchIndex: this.state.highestCheckedBatchIndex,
+    })
   }
 
   async main(): Promise<void> {
-    const latestBatchIndex =
-      await this.state.messenger.contracts.l1.StateCommitmentChain.getTotalBatches()
+    const latestBatchIndex = await this.state.scc.getTotalBatches()
     if (this.state.highestCheckedBatchIndex >= latestBatchIndex.toNumber()) {
       await sleep(15000)
       return
@@ -89,41 +107,30 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
 
     this.logger.info(`checking batch`, {
       batchIndex: this.state.highestCheckedBatchIndex,
+      latestIndex: latestBatchIndex.toNumber(),
     })
 
-    const targetEvents =
-      await this.state.messenger.contracts.l1.StateCommitmentChain.queryFilter(
-        this.state.messenger.contracts.l1.StateCommitmentChain.filters.StateBatchAppended(
-          this.state.highestCheckedBatchIndex
-        )
+    let event: ethers.Event
+    try {
+      event = await findEventForStateBatch(
+        this.state.scc,
+        this.state.highestCheckedBatchIndex
       )
-
-    if (targetEvents.length === 0) {
-      this.logger.error(`unable to find event for batch`, {
-        batchIndex: this.state.highestCheckedBatchIndex,
-      })
-      this.metrics.inUnexpectedErrorState.set(1)
-      return
-    }
-
-    if (targetEvents.length > 1) {
-      this.logger.error(`found too many events for batch`, {
+    } catch (err) {
+      this.logger.error(`got unexpected error while searching for batch`, {
         batchIndex: this.state.highestCheckedBatchIndex,
+        error: err,
       })
-      this.metrics.inUnexpectedErrorState.set(1)
-      return
     }
 
-    const targetEvent = targetEvents[0]
-    const batchTransaction = await targetEvent.getTransaction()
-    const [stateRoots] =
-      this.state.messenger.contracts.l1.StateCommitmentChain.interface.decodeFunctionData(
-        'appendStateBatch',
-        batchTransaction.data
-      )
+    const batchTransaction = await event.getTransaction()
+    const [stateRoots] = this.state.scc.interface.decodeFunctionData(
+      'appendStateBatch',
+      batchTransaction.data
+    )
 
-    const batchStart = targetEvent.args._prevTotalElements.toNumber() + 1
-    const batchSize = targetEvent.args._batchSize.toNumber()
+    const batchStart = event.args._prevTotalElements.toNumber() + 1
+    const batchSize = event.args._batchSize.toNumber()
 
     // `getBlockRange` has a limit of 1000 blocks, so we have to break this request out into
     // multiple requests of maximum 1000 blocks in the case that batchSize > 1000.
@@ -143,8 +150,7 @@ export class FaultDetector extends BaseServiceV2<Options, Metrics, State> {
     for (const [i, stateRoot] of stateRoots.entries()) {
       if (blocks[i].stateRoot !== stateRoot) {
         this.metrics.isCurrentlyMismatched.set(1)
-        const fpw =
-          await this.state.messenger.contracts.l1.StateCommitmentChain.FRAUD_PROOF_WINDOW()
+        const fpw = await this.state.scc.FRAUD_PROOF_WINDOW()
         this.logger.error(`state root mismatch`, {
           blockNumber: blocks[i].number,
           expectedStateRoot: blocks[i].stateRoot,

packages/fault-detector/test/helpers.spec.ts

+import hre from 'hardhat'
+import { Contract } from 'ethers'
+import { toRpcHexString } from '@eth-optimism/core-utils'
+import {
+  getContractFactory,
+  getContractInterface,
+} from '@eth-optimism/contracts'
+import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
+import { smock, FakeContract } from '@defi-wonderland/smock'
+
+import { expect } from './setup'
+import {
+  findEventForStateBatch,
+  findFirstUnfinalizedStateBatchIndex,
+} from '../src'
+
+describe('helpers', () => {
+  // Can be any non-zero value, 1000 is fine.
+  const challengeWindowSeconds = 1000
+
+  let signer: SignerWithAddress
+  before(async () => {
+    ;[signer] = await hre.ethers.getSigners()
+  })
+
+  let FakeBondManager: FakeContract<Contract>
+  let FakeCanonicalTransactionChain: FakeContract<Contract>
+  let AddressManager: Contract
+  let ChainStorageContainer: Contract
+  let StateCommitmentChain: Contract
+  beforeEach(async () => {
+    // Set up fakes
+    FakeBondManager = await smock.fake(getContractInterface('BondManager'))
+    FakeCanonicalTransactionChain = await smock.fake(
+      getContractInterface('CanonicalTransactionChain')
+    )
+
+    // Set up contracts
+    AddressManager = await getContractFactory(
+      'Lib_AddressManager',
+      signer
+    ).deploy()
+    ChainStorageContainer = await getContractFactory(
+      'ChainStorageContainer',
+      signer
+    ).deploy(AddressManager.address, 'StateCommitmentChain')
+    StateCommitmentChain = await getContractFactory(
+      'StateCommitmentChain',
+      signer
+    ).deploy(AddressManager.address, challengeWindowSeconds, 10000000)
+
+    // Set addresses in manager
+    await AddressManager.setAddress(
+      'ChainStorageContainer-SCC-batches',
+      ChainStorageContainer.address
+    )
+    await AddressManager.setAddress(
+      'StateCommitmentChain',
+      StateCommitmentChain.address
+    )
+    await AddressManager.setAddress(
+      'CanonicalTransactionChain',
+      FakeCanonicalTransactionChain.address
+    )
+    await AddressManager.setAddress('BondManager', FakeBondManager.address)
+
+    // Set up mock returns
+    FakeCanonicalTransactionChain.getTotalElements.returns(1000000000) // just needs to be large
+    FakeBondManager.isCollateralized.returns(true)
+  })
+
+  describe('findEventForStateBatch', () => {
+    describe('when the event exists once', () => {
+      beforeEach(async () => {
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          0
+        )
+      })
+
+      it('should return the event', async () => {
+        const event = await findEventForStateBatch(StateCommitmentChain, 0)
+
+        expect(event.args._batchIndex).to.equal(0)
+      })
+    })
+
+    describe('when the event does not exist', () => {
+      it('should throw an error', async () => {
+        await expect(
+          findEventForStateBatch(StateCommitmentChain, 0)
+        ).to.eventually.be.rejectedWith('unable to find event for batch')
+      })
+    })
+
+    describe('when more than one event exists', () => {
+      beforeEach(async () => {
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          0
+        )
+        await hre.ethers.provider.send('hardhat_setStorageAt', [
+          ChainStorageContainer.address,
+          '0x2',
+          hre.ethers.constants.HashZero,
+        ])
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          0
+        )
+      })
+
+      it('should throw an error', async () => {
+        await expect(
+          findEventForStateBatch(StateCommitmentChain, 0)
+        ).to.eventually.be.rejectedWith('found too many events for batch')
+      })
+    })
+  })
+
+  describe('findFirstUnfinalizedIndex', () => {
+    describe('when the chain is more then FPW seconds old', () => {
+      beforeEach(async () => {
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          0
+        )
+
+        // Simulate FPW passing
+        await hre.ethers.provider.send('evm_increaseTime', [
+          toRpcHexString(challengeWindowSeconds * 2),
+        ])
+
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          1
+        )
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          2
+        )
+      })
+
+      it('should find the first batch older than the FPW', async () => {
+        const first = await findFirstUnfinalizedStateBatchIndex(
+          StateCommitmentChain
+        )
+
+        expect(first).to.equal(1)
+      })
+    })
+
+    describe('when the chain is less than FPW seconds old', () => {
+      beforeEach(async () => {
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          0
+        )
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          1
+        )
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          2
+        )
+      })
+
+      it('should return zero', async () => {
+        const first = await findFirstUnfinalizedStateBatchIndex(
+          StateCommitmentChain
+        )
+
+        expect(first).to.equal(0)
+      })
+    })
+
+    describe('when no batches submitted for the entire FPW', () => {
+      beforeEach(async () => {
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          0
+        )
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          1
+        )
+        await StateCommitmentChain.appendStateBatch(
+          [hre.ethers.constants.HashZero],
+          2
+        )
+
+        // Simulate FPW passing and no new batches
+        await hre.ethers.provider.send('evm_increaseTime', [
+          toRpcHexString(challengeWindowSeconds * 2),
+        ])
+
+        // Mine a block to force timestamp to update
+        await hre.ethers.provider.send('hardhat_mine', ['0x1'])
+      })
+
+      it('should return undefined', async () => {
+        const first = await findFirstUnfinalizedStateBatchIndex(
+          StateCommitmentChain
+        )
+
+        expect(first).to.equal(undefined)
+      })
+    })
+  })
+})

packages/fault-detector/test/setup.ts

+import chai = require('chai')
+import chaiAsPromised from 'chai-as-promised'
+
+// Chai plugins go here.
+chai.use(chaiAsPromised)
+
+const should = chai.should()
+const expect = chai.expect
+
+export { should, expect }

yarn.lock

@@ -2572,7 +2572,7 @@
     safe-buffer "^5.1.1"
     util.promisify "^1.0.0"
 
-"@nomiclabs/hardhat-ethers@^2.0.0":
+"@nomiclabs/hardhat-ethers@^2.0.0", "@nomiclabs/hardhat-ethers@^2.0.6":
   version "2.0.6"
   resolved "https://registry.yarnpkg.com/@nomiclabs/hardhat-ethers/-/hardhat-ethers-2.0.6.tgz#1c695263d5b46a375dcda48c248c4fba9dfe2fc2"
   integrity sha512-q2Cjp20IB48rEn2NPjR1qxsIQBvFVYW9rFRCFq+bC4RUrn1Ljz3g4wM8uSlgIBZYBi2JMXxmOzFqHraczxq4Ng==
@@ -2621,7 +2621,7 @@
     semver "^6.3.0"
     undici "^4.14.1"
 
-"@nomiclabs/hardhat-waffle@^2.0.0", "@nomiclabs/hardhat-waffle@^2.0.2":
+"@nomiclabs/hardhat-waffle@^2.0.0", "@nomiclabs/hardhat-waffle@^2.0.2", "@nomiclabs/hardhat-waffle@^2.0.3":
   version "2.0.3"
   resolved "https://registry.yarnpkg.com/@nomiclabs/hardhat-waffle/-/hardhat-waffle-2.0.3.tgz#9c538a09c5ed89f68f5fd2dc3f78f16ed1d6e0b1"
   integrity sha512-049PHSnI1CZq6+XTbrMbMv5NaL7cednTfPenx02k3cEh8wBMLa6ys++dBETJa6JjfwgA9nBhhHQ173LJv6k2Pg==
@@ -3257,6 +3257,11 @@
   resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.2.21.tgz#9f35a5643129df132cf3b5c1ec64046ea1af0650"
   integrity sha512-yd+9qKmJxm496BOV9CMNaey8TWsikaZOwMRwPHQIjcOJM9oV+fi9ZMNw3JsVnbEEbo2gRTDnGEBv8pjyn67hNg==
 
+"@types/chai@^4.3.1":
+  version "4.3.1"
+  resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.3.1.tgz#e2c6e73e0bdeb2521d00756d099218e9f5d90a04"
+  integrity sha512-/zPMqDkzSZ8t3VtxOa4KPq7uzzW978M9Tvh+j7GHKuo6k6GTLxPJ4J5gE5cjfJ26pnXst0N5Hax8Sr0T2Mi9zQ==
+
 "@types/concat-stream@^1.6.0":
   version "1.6.1"
   resolved "https://registry.yarnpkg.com/@types/concat-stream/-/concat-stream-1.6.1.tgz#24bcfc101ecf68e886aaedce60dfd74b632a1b74"
@@ -8022,7 +8027,7 @@ ethereum-cryptography@^1.0.3:
     "@scure/bip32" "1.0.1"
     "@scure/bip39" "1.0.0"
 
-ethereum-waffle@^3.0.0:
+ethereum-waffle@^3.0.0, ethereum-waffle@^3.4.4:
   version "3.4.4"
   resolved "https://registry.yarnpkg.com/ethereum-waffle/-/ethereum-waffle-3.4.4.tgz#1378b72040697857b7f5e8f473ca8f97a37b5840"
   integrity sha512-PA9+jCjw4WC3Oc5ocSMBj5sXvueWQeAbvCA+hUlb6oFgwwKyq5ka3bWQ7QZcjzIX+TdFkxP4IbFmoY2D8Dkj9Q==