Test cleanup and add solcover

package.json

@@ -28,7 +28,7 @@
     "release:alpha": "yarn clean && yarn build && lerna version prerelease --yes && lerna publish from-package --yes --force-publish --dist-tag=alpha -m \"chore(@ethereum-optimism) publish %s release\"",
     "release:beta": "yarn clean && yarn build && lerna version prerelease --yes && lerna publish from-package --yes --force-publish --dist-tag=beta -m \"chore(@ethereum-optimism) publish %s release\"",
     "patch": "yarn run patch:ganache && yarn run patch:waffle",
-    "patch:ganache": "sed -ie 's!import { Provider as Web3Provider } from \"web3/providers\";!import { AbstractProvider as Web3Provider } from \"web3-core\";!' node_modules/ganache-core/typings/index.d.ts",
+    "patch:ganache": "rm node_modules/ganache-core/typings/index.d.ts ||:",
     "patch:waffle": "sed -ie 's!Ganache.GanacheOpts!any!g' node_modules/ethereum-waffle/dist/waffle.d.ts"
   },
   "repository": "git+https://github.com/ethereum-optimism/optimism-monorepo.git",

packages/contracts/.gitignore

@@ -0,0 +1,5 @@
+.coverage_artifacts/
+.coverage_cache/
+.coverage_contracts/
+coverage/
+coverage.json

packages/contracts/.solcover.js

@@ -0,0 +1,69 @@
+/**
+ * SolCover configuration management:
+ * 
+ * SolCover unfortunately doesn't provide us with the ability to exclusively
+ * generate coverage reports for a given file. Although we can use the
+ * `--testfiles`` parameter to limit tests to a particular glob, SolCover will
+ * still try to generate coverage reports for anything not covered within the
+ * `skipFiles` option exported below. `skipFiles` additionally does not parse
+ * globs, creating a mismatch between it and the `--testfiles` option.
+ * 
+ * To address the above issues, we take the following steps:
+ * 1. Parse the `--testfiles` option from our command-line arguments.
+ * 2. Use the `--testfiles` option to find the list of contracts to be tested.
+ * 3. Find *all* contracts and exclude the results of (2).
+ * 4. Add the result of (3) to `skipFiles`.
+ * 
+ * NOTE: The above will *only* work if contract test files follow the
+ * `<ContractName>.spec.ts` convention. Our function will fail to find the
+ * correct contracts otherwise.
+ */ 
+
+const path = require('path')
+const glob = require('glob')
+const ArgumentParser = require('argparse').ArgumentParser
+
+const parser = new ArgumentParser()
+parser.addArgument(
+  ['--testfiles'],
+)
+
+/**
+ * Given a glob, finds all files to skip.
+ * @param skipFiles Path or glob of files to skip.
+ * @returns Paths to files to skip.
+ */
+const parseSkipFiles = (skipFiles) => {
+  return skipFiles.reduce((files, contract) => {
+    return files.concat(glob.sync(contract))
+  }, []).map((contract) => {
+    return contract.replace('contracts/', '')
+  })
+}
+
+/**
+ * Given a glob, finds all contracts to skip that are *not* the given files.
+ * @param includeFiles Path or glob of files not to skip.
+ * @returns Paths to files to skip.
+ */
+const parseIncludeFiles = (includeFiles) => {
+  const allFiles = parseSkipFiles(['contracts/**/*.sol'])
+  const includedFiles = parseSkipFiles(includeFiles)
+  return allFiles.filter((file) => {
+    return !includedFiles.includes(file)
+  })
+}
+
+/**
+ * Parses command-line arguments and picks out the correct contracts to skip.
+ */
+const parseTestFiles = () => {
+  const args = parser.parseKnownArgs()[0]
+  const testFileName = path.basename(args.testfiles)
+  const contractFileName = testFileName.split('.')[0] + '.sol'
+  return parseIncludeFiles(['contracts/**/' + contractFileName])
+}
+
+module.exports = {
+  skipFiles: parseTestFiles()
+}

packages/contracts/buidler.config.ts

@@ -1,17 +1,24 @@
-import * as path from 'path'
 import { usePlugin, BuidlerConfig } from '@nomiclabs/buidler/config'
 
-import { DEFAULT_ACCOUNTS_BUIDLER } from './test/test-helpers/constants'
+import {
+  DEFAULT_ACCOUNTS_BUIDLER,
+  GAS_LIMIT,
+} from './test/test-helpers/constants'
 
 usePlugin('@nomiclabs/buidler-ethers')
 usePlugin('@nomiclabs/buidler-waffle')
+usePlugin('solidity-coverage')
 
 import './plugins/hijack-compiler'
 
 const config: BuidlerConfig = {
   networks: {
     buidlerevm: {
       accounts: DEFAULT_ACCOUNTS_BUIDLER,
+      blockGasLimit: GAS_LIMIT * 2,
+    },
+    coverage: {
+      url: 'http://localhost:8555',
     },
   },
 }

packages/contracts/package.json

@@ -12,6 +12,8 @@
     "nohoist": [
       "**/@nomiclabs",
       "**/@nomiclabs/**",
+      "**/solidity-coverage",
+      "**/solidity-coverage/**",
       "**/typescript",
       "**/typescript/**",
       "**/ts-node",
@@ -22,6 +24,8 @@
     "all": "yarn clean && yarn build && yarn test && yarn fix && yarn lint",
     "test": "yarn run test:contracts",
     "test:contracts": "buidler test --show-stack-traces",
+    "coverage": "yarn run coverage:contracts",
+    "coverage:contracts": "buidler coverage --network coverage --show-stack-traces --testfiles \"test/contracts/**/*.spec.ts\"",
     "build": "yarn run build:contracts && yarn run build:typescript",
     "build:contracts": "buidler compile",
     "build:typescript": "tsc -p .",
@@ -52,5 +56,9 @@
     "seedrandom": "^3.0.5",
     "ts-node": "^8.10.2",
     "typescript": "^3.9.5"
+  },
+  "devDependencies": {
+    "glob": "^7.1.6",
+    "solidity-coverage": "^0.7.9"
   }
-}
+}

packages/contracts/test/contracts/chain/CanonicalTransactionChain.spec.ts

@@ -3,11 +3,14 @@ import '../../setup'
 /* External Imports */
 import { ethers } from '@nomiclabs/buidler'
 import { getLogger, sleep, TestUtils } from '@eth-optimism/core-utils'
-import { Signer, ContractFactory } from 'ethers'
+import { Contract, Signer, ContractFactory } from 'ethers'
 
 /* Internal Imports */
-import { TxChainBatch, TxQueueBatch } from '../../test-helpers/rl-helpers'
-import { makeRandomBatchOfSize } from '../../test-helpers'
+import {
+  makeRandomBatchOfSize,
+  TxQueueBatch,
+  TxChainBatch,
+} from '../../test-helpers'
 
 /* Logging */
 const log = getLogger('canonical-tx-chain', true)
@@ -32,10 +35,10 @@ describe('CanonicalTransactionChain', () => {
     ] = await ethers.getSigners()
   })
 
-  let canonicalTxChain
-  let rollupMerkleUtils
-  let l1ToL2Queue
-  let safetyQueue
+  let canonicalTxChain: Contract
+  let rollupMerkleUtils: Contract
+  let l1ToL2Queue: Contract
+  let safetyQueue: Contract
 
   const appendSequencerBatch = async (batch: string[]): Promise<number> => {
     const timestamp = Math.floor(Date.now() / 1000)

packages/contracts/test/contracts/chain/SequencerBatchSubmitter.spec.ts

@@ -6,14 +6,13 @@ import { getLogger, TestUtils } from '@eth-optimism/core-utils'
 import { Contract, Signer, ContractFactory } from 'ethers'
 
 /* Internal Imports */
-import { StateChainBatch, TxChainBatch } from '../../test-helpers/rl-helpers'
+import { StateChainBatch, TxChainBatch } from '../../test-helpers'
 
 /* Logging */
 const log = getLogger('batch-submitter', true)
 
 /* Tests */
 describe('SequencerBatchSubmitter', () => {
-  const provider = ethers.provider
   const DEFAULT_STATE_BATCH = ['0x1234', '0x5678']
   const DEFAULT_TX_BATCH = ['0xabcd', '0xef12']
   const FORCE_INCLUSION_PERIOD = 600

packages/contracts/test/contracts/chain/StateCommitmentChain.spec.ts

@@ -6,8 +6,7 @@ import { getLogger, sleep, TestUtils } from '@eth-optimism/core-utils'
 import { Contract, Signer, ContractFactory } from 'ethers'
 
 /* Internal Imports */
-import { StateChainBatch } from '../../test-helpers/rl-helpers'
-import { makeRandomBatchOfSize } from '../../test-helpers'
+import { makeRandomBatchOfSize, StateChainBatch } from '../../test-helpers'
 
 /* Logging */
 const log = getLogger('state-commitment-chain', true)

packages/contracts/test/contracts/ovm/L2ExecutionManager.spec.ts

@@ -6,11 +6,7 @@ import { add0x, getLogger } from '@eth-optimism/core-utils'
 import { Contract, Signer, ContractFactory } from 'ethers'
 
 /* Internal Imports */
-import {
-  DEFAULT_OPCODE_WHITELIST_MASK,
-  GAS_LIMIT,
-  DEFAULT_ETHNODE_GAS_LIMIT,
-} from '../../test-helpers/core-helpers'
+import { DEFAULT_OPCODE_WHITELIST_MASK, GAS_LIMIT } from '../../test-helpers'
 
 /* Logging */
 const log = getLogger('l2-execution-manager-calls', true)

packages/contracts/test/contracts/ovm/PartialStateManager.spec.ts

@@ -2,7 +2,7 @@ import '../../setup'
 
 /* External Imports */
 import { ethers } from '@nomiclabs/buidler'
-import { getLogger, TestUtils } from '@eth-optimism/core-utils'
+import { getLogger } from '@eth-optimism/core-utils'
 import { Contract, ContractFactory, Signer } from 'ethers'
 
 /* Logging */
@@ -20,7 +20,7 @@ describe('PartialStateManager', () => {
     PartialStateManager = await ethers.getContractFactory('PartialStateManager')
   })
 
-  let partialStateManager
+  let partialStateManager: Contract
   beforeEach(async () => {
     partialStateManager = await PartialStateManager.deploy(
       await wallet.getAddress(),

packages/contracts/test/contracts/ovm/StateTransitioner.spec.ts

@@ -2,7 +2,7 @@ import '../../setup'
 
 /* External Imports */
 import { ethers } from '@nomiclabs/buidler'
-import { getLogger, TestUtils } from '@eth-optimism/core-utils'
+import { getLogger } from '@eth-optimism/core-utils'
 import { Contract, ContractFactory, Signer } from 'ethers'
 
 /* Logging */

packages/contracts/test/contracts/ovm/execution-manager/ExecutionManager.call-opcodes.spec.ts

@@ -15,16 +15,12 @@ import { fromPairs } from 'lodash'
 
 /* Internal Imports */
 import {
-  Address,
   GAS_LIMIT,
   DEFAULT_OPCODE_WHITELIST_MASK,
-  DEFAULT_ETHNODE_GAS_LIMIT,
-} from '../../../test-helpers/core-helpers'
-import {
+  Address,
   manuallyDeployOvmContract,
   addressToBytes32Address,
   didCreateSucceed,
-  gasLimit,
   encodeMethodId,
   encodeRawArguments,
 } from '../../../test-helpers'
@@ -450,7 +446,7 @@ describe('Execution Manager -- Call opcodes', () => {
     const receipt = await wallet.sendTransaction({
       to: executionManager.address,
       data: add0x(data),
-      gasLimit,
+      gasLimit: GAS_LIMIT,
     })
 
     return receipt.hash
@@ -477,7 +473,7 @@ describe('Execution Manager -- Call opcodes', () => {
     return executionManager.provider.call({
       to: executionManager.address,
       data,
-      gasLimit,
+      gasLimit: GAS_LIMIT,
     })
   }
 })

packages/contracts/test/contracts/ovm/execution-manager/ExecutionManager.code-opcodes.spec.ts

@@ -13,12 +13,9 @@ import { Contract, ContractFactory, Signer } from 'ethers'
 
 /* Internal Imports */
 import {
-  Address,
   GAS_LIMIT,
   DEFAULT_OPCODE_WHITELIST_MASK,
-  DEFAULT_ETHNODE_GAS_LIMIT,
-} from '../../../test-helpers/core-helpers'
-import {
+  Address,
   manuallyDeployOvmContract,
   executeOVMCall,
   addressToBytes32Address,

packages/contracts/test/contracts/ovm/execution-manager/ExecutionManager.context-opcodes.spec.ts

@@ -16,17 +16,13 @@ import { fromPairs } from 'lodash'
 
 /* Internal Imports */
 import {
-  Address,
   GAS_LIMIT,
   DEFAULT_OPCODE_WHITELIST_MASK,
-  DEFAULT_ETHNODE_GAS_LIMIT,
-} from '../../../test-helpers/core-helpers'
-import {
+  Address,
   manuallyDeployOvmContract,
   addressToBytes32Address,
   encodeRawArguments,
   encodeMethodId,
-  gasLimit,
 } from '../../../test-helpers'
 
 /* Logging */
@@ -229,7 +225,7 @@ describe('Execution Manager -- Context opcodes', () => {
     return executionManager.provider.call({
       to: executionManager.address,
       data,
-      gasLimit,
+      gasLimit: GAS_LIMIT,
     })
   }
 })

packages/contracts/test/contracts/ovm/execution-manager/ExecutionManager.create-opcodes.spec.ts

@@ -10,10 +10,6 @@ import { fromPairs } from 'lodash'
 import {
   DEFAULT_OPCODE_WHITELIST_MASK,
   GAS_LIMIT,
-  DEFAULT_ETHNODE_GAS_LIMIT,
-} from '../../../test-helpers/core-helpers'
-import {
-  gasLimit,
   executeOVMCall,
   encodeMethodId,
   encodeRawArguments,
@@ -31,11 +27,6 @@ const methodIds = fromPairs(
 
 /* Tests */
 describe('ExecutionManager -- Create opcodes', () => {
-  let wallet: Signer
-  before(async () => {
-    ;[wallet] = await ethers.getSigners()
-  })
-
   let ExecutionManager: ContractFactory
   let SimpleStorage: ContractFactory
   let InvalidOpcodes: ContractFactory
@@ -91,7 +82,7 @@ describe('ExecutionManager -- Create opcodes', () => {
       const result = await executionManager.provider.call({
         to: safetyCheckedExecutionManager.address,
         data,
-        gasLimit,
+        gasLimit: GAS_LIMIT,
       })
 
       log.debug(`Result: [${result}]`)
@@ -112,7 +103,7 @@ describe('ExecutionManager -- Create opcodes', () => {
       const result = await executionManager.provider.call({
         to: executionManager.address,
         data,
-        gasLimit,
+        gasLimit: GAS_LIMIT,
       })
 
       log.debug(`Result: [${result}]`)
@@ -131,7 +122,7 @@ describe('ExecutionManager -- Create opcodes', () => {
       const result = await executionManager.provider.call({
         to: safetyCheckedExecutionManager.address,
         data,
-        gasLimit,
+        gasLimit: GAS_LIMIT,
       })
 
       log.debug(`Result: [${result}]`)