Clean up logging

packages/core-utils/src/app/log.ts

@@ -1,16 +1,20 @@
 import debug from 'debug'
 import { Logger } from '../types'
 
+export const LOG_NEWLINE_STRING = '<\n>'
+export const joinNewlinesAndDebug = (logs: string) =>
+  debug(logs.replace('\n', LOG_NEWLINE_STRING))
+
 export const getLogger = (
   identifier: string,
   isTest: boolean = false
 ): Logger => {
   const testString = isTest ? 'test:' : ''
   return {
-    debug: debug(`${testString}debug:${identifier}`),
-    info: debug(`${testString}info:${identifier}`),
-    warn: debug(`${testString}warn:${identifier}`),
-    error: debug(`${testString}error:${identifier}`),
+    debug: joinNewlinesAndDebug(`${testString}debug:${identifier}`),
+    info: joinNewlinesAndDebug(`${testString}info:${identifier}`),
+    warn: joinNewlinesAndDebug(`${testString}warn:${identifier}`),
+    error: joinNewlinesAndDebug(`${testString}error:${identifier}`),
   }
 }
 

packages/ovm/src/app/utils.ts

@@ -9,6 +9,7 @@ import {
   logError,
   remove0x,
   ZERO_ADDRESS,
+  LOG_NEWLINE_STRING,
   BloomFilter,
 } from '@eth-optimism/core-utils'
 import { ethers } from 'ethers'
@@ -71,24 +72,26 @@ export interface OvmTransactionMetadata {
  */
 export const convertInternalLogsToOvmLogs = (logs: Log[]): Log[] => {
   let activeContract = logs[0] ? logs[0].address : ZERO_ADDRESS
+  const loggerLogs = [`Parsing logs from contract ${activeContract}: `]
   const ovmLogs = []
   logs.forEach((log) => {
     const executionManagerLog = executionManagerInterface.parseLog(log)
     if (executionManagerLog) {
       if (executionManagerLog.name === 'ActiveContract') {
         activeContract = executionManagerLog.values['_activeContract']
       } else {
-        logger.debug(
+        loggerLogs.push(
           `${executionManagerLog.name}, values: ${JSON.stringify(
             executionManagerLog.values
           )}`
         )
       }
     } else {
-      logger.debug(`Non-EM log: ${JSON.stringify(log)}`)
+      loggerLogs.push(`Non-EM log: ${JSON.stringify(log)}`)
       ovmLogs.push({ ...log, address: activeContract })
     }
   })
+  logger.debug(loggerLogs.join(LOG_NEWLINE_STRING))
   return ovmLogs
 }
 

packages/rollup-full-node/package.json

@@ -49,7 +49,8 @@
     "ethers": "^4.0.39",
     "fastpriorityqueue": "^0.6.3",
     "level": "^6.0.1",
-    "rimraf": "^2.6.3"
+    "rimraf": "^2.6.3",
+    "web3": "^1.2.7"
   },
   "devDependencies": {
     "@eth-optimism/solc-transpiler": "^0.0.1-alpha.25",

packages/rollup-full-node/src/app/test-web3-rpc-handler.ts

@@ -15,6 +15,7 @@ import {
   L2ToL1MessageSubmitter,
   UnsupportedMethodError,
   Web3RpcMethods,
+  Web3RpcTypes,
 } from '../types'
 import { getCurrentTime, initializeL2Node } from './util'
 import { NoOpL2ToL1MessageSubmitter } from './message-submitter'
@@ -66,24 +67,24 @@ export class TestWeb3Handler extends DefaultWeb3Handler {
   public async handleRequest(method: string, params: any[]): Promise<string> {
     switch (method) {
       case Web3RpcMethods.increaseTimestamp:
-        this.assertParameters(params, 1)
+        this.assertParameters(params, [Web3RpcTypes.quantity])
         this.increaseTimestamp(params[0])
         log.debug(`Set increased timestamp by ${params[0]} seconds.`)
         return TestWeb3Handler.successString
       case Web3RpcMethods.mine:
         return this.context.provider.send(Web3RpcMethods.mine, params)
       case Web3RpcMethods.sendTransaction:
-        this.assertParameters(params, 1)
+        this.assertParameters(params, [Web3RpcTypes.object])
         return this.sendTransaction(params[0])
         break
       case Web3RpcMethods.snapshot:
-        this.assertParameters(params, 0)
+        this.assertParameters(params, [])
         return this.snapshot()
       case Web3RpcMethods.revert:
-        this.assertParameters(params, 1)
+        this.assertParameters(params, [Web3RpcTypes.quantity])
         return this.revert(params[0])
       case Web3RpcMethods.accounts:
-        this.assertParameters(params, 0)
+        this.assertParameters(params, [])
         return this.accounts()
       default:
         return super.handleRequest(method, params)

packages/rollup-full-node/src/app/web3-rpc-handler.ts

@@ -27,7 +27,8 @@ import {
 } from '@eth-optimism/ovm'
 
 import AsyncLock from 'async-lock'
-import { utils, Wallet } from 'ethers'
+import { Contract, utils, Wallet } from 'ethers'
+import Web3 from 'web3'
 import { JsonRpcProvider, TransactionReceipt } from 'ethers/providers'
 
 /* Internal Imports */
@@ -38,6 +39,7 @@ import {
   L2ToL1MessageSubmitter,
   UnsupportedMethodError,
   Web3Handler,
+  Web3RpcTypes,
   Web3RpcMethods,
   RevertError,
 } from '../types'
@@ -116,70 +118,91 @@ export class DefaultWeb3Handler
 
     // Make sure the method is available
     let response: any
-    let args: any[]
     switch (method) {
       case Web3RpcMethods.blockNumber:
-        this.assertParameters(params, 0)
+        this.assertParameters(params, [])
         response = await this.blockNumber()
         break
       case Web3RpcMethods.call:
-        args = this.assertParameters(params, 2, latestBlock)
-        response = await this.call(args[0], args[1])
+        this.assertParameters(params, [
+          Web3RpcTypes.object,
+          Web3RpcTypes.quantityOrTag,
+        ])
+        response = await this.call(params[0], params[1] || latestBlock)
         break
       case Web3RpcMethods.estimateGas:
-        args = this.assertParameters(params, 2, latestBlock)
-        response = await this.estimateGas(args[0], args[1])
+        this.assertParameters(params, [
+          Web3RpcTypes.object,
+          Web3RpcTypes.quantityOrTag,
+        ])
+        response = await this.estimateGas(params[0], params[1] || latestBlock)
         break
       case Web3RpcMethods.gasPrice:
-        this.assertParameters(params, 0)
+        this.assertParameters(params, [])
         response = await this.gasPrice()
         break
       case Web3RpcMethods.getBlockByNumber:
-        args = this.assertParameters(params, 2)
-        response = await this.getBlockByNumber(args[0], args[1])
+        this.assertParameters(params, [
+          Web3RpcTypes.quantityOrTag,
+          Web3RpcTypes.boolean,
+        ])
+        response = await this.getBlockByNumber(params[0], params[1])
         break
       case Web3RpcMethods.getBlockByHash:
-        args = this.assertParameters(params, 2)
-        response = await this.getBlockByHash(args[0], args[1])
+        this.assertParameters(params, [Web3RpcTypes.data, Web3RpcTypes.boolean])
+        response = await this.getBlockByHash(params[0], params[1])
         break
       case Web3RpcMethods.getBalance:
-        this.assertParameters(params, 2, latestBlock)
+        this.assertParameters(
+          params,
+          [Web3RpcTypes.address, Web3RpcTypes.quantityOrTag],
+          latestBlock
+        )
         response = await this.getBalance()
         break
       case Web3RpcMethods.getCode:
-        args = this.assertParameters(params, 2, latestBlock)
-        response = await this.getCode(args[0], args[1])
+        this.assertParameters(params, [
+          Web3RpcTypes.data,
+          Web3RpcTypes.quantityOrTag,
+        ])
+        response = await this.getCode(params[0], params[1] || latestBlock)
         break
       case Web3RpcMethods.getExecutionManagerAddress:
-        this.assertParameters(params, 0)
+        this.assertParameters(params, [])
         response = await this.getExecutionManagerAddress()
         break
       case Web3RpcMethods.getLogs:
-        args = this.assertParameters(params, 1)
-        response = await this.getLogs(args[0])
+        this.assertParameters(params, [Web3RpcTypes.object])
+        response = await this.getLogs(params[0])
         break
       case Web3RpcMethods.getTransactionByHash:
-        args = this.assertParameters(params, 1)
-        response = await this.getTransactionByHash(args[0])
+        this.assertParameters(params, [Web3RpcTypes.data])
+        response = await this.getTransactionByHash(params[0])
         break
       case Web3RpcMethods.getTransactionCount:
-        args = this.assertParameters(params, 2, latestBlock)
-        response = await this.getTransactionCount(args[0], args[1])
+        this.assertParameters(params, [
+          Web3RpcTypes.data,
+          Web3RpcTypes.quantityOrTag,
+        ])
+        response = await this.getTransactionCount(
+          params[0],
+          params[1] || latestBlock
+        )
         break
       case Web3RpcMethods.getTransactionReceipt:
-        args = this.assertParameters(params, 1)
-        response = await this.getTransactionReceipt(args[0])
+        this.assertParameters(params, [Web3RpcTypes.data])
+        response = await this.getTransactionReceipt(params[0])
         break
       case Web3RpcMethods.sendRawTransaction:
-        args = this.assertParameters(params, 1)
-        response = await this.sendRawTransaction(args[0])
+        this.assertParameters(params, [Web3RpcTypes.data])
+        response = await this.sendRawTransaction(params[0])
         break
       case Web3RpcMethods.networkVersion:
-        this.assertParameters(params, 0)
+        this.assertParameters(params, [])
         response = await this.networkVersion()
         break
       case Web3RpcMethods.chainId:
-        this.assertParameters(params, 0)
+        this.assertParameters(params, [])
         response = await this.chainId()
         break
       default:
@@ -443,7 +466,7 @@ export class DefaultWeb3Handler
           defaultBlock
         )}] curentBlockNumber:[${curentBlockNumber}]`
       )
-      throw new Error(
+      throw new InvalidParametersError(
         `Historical code lookups aren't supported. Requested Block: ${hexStrToNumber(
           defaultBlock
         )} Current Block: ${curentBlockNumber}`
@@ -498,6 +521,7 @@ export class DefaultWeb3Handler
           const receipt = await this.getTransactionReceipt(transaction.hash)
           transaction['to'] = receipt.contractAddress
         }
+        logItem['address'] = transaction['to']
 
         return logItem
       })
@@ -940,7 +964,7 @@ export class DefaultWeb3Handler
   private async ovmTxToInternalTx(ovmTx: any): Promise<string> {
     // Verify that the transaction is not accidentally sending to the ZERO_ADDRESS
     if (ovmTx.to === ZERO_ADDRESS) {
-      throw new Error('Sending to Zero Address disallowed')
+      throw new InvalidParametersError('Sending to Zero Address disallowed')
     }
     // Get the nonce of the account that we will use to send everything
     // Note: + 1 because all transactions will have a tx hash mapping tx sent before them.
@@ -950,7 +974,7 @@ export class DefaultWeb3Handler
       log.error(
         'Transaction does not have a valid signature! For now we only support calls from EOAs'
       )
-      throw new Error('Non-EOA transaction detected')
+      throw new InvalidParametersError('Non-EOA transaction detected')
     }
     // Generate the calldata which we'll use to call our internal execution manager
     // First pull out the `to` field (we just need to check if it's null & if so set ovmTo to the zero address as that's how we deploy contracts)
@@ -961,7 +985,7 @@ export class DefaultWeb3Handler
       await this.context.executionManager.getOvmContractNonce(ovmFrom)
     ).toNumber()
     if (expectedNonce !== ovmTx.nonce) {
-      throw new Error(
+      throw new InvalidParametersError(
         `Incorrect nonce! Expected nonce: ${expectedNonce} but received nonce: ${ovmTx.nonce}`
       )
     }
@@ -1037,23 +1061,44 @@ export class DefaultWeb3Handler
 
   protected assertParameters(
     params: any[],
-    expected: number,
+    expected: Web3RpcTypes[],
     defaultLast?: any
-  ): any[] {
-    if (!params) {
-      if (!expected) {
-        return []
-      }
-    } else if (params.length === expected - 1 || params.length === expected) {
-      const nonEmptyParams = params.filter((x) => !!x)
-      return nonEmptyParams.length === expected
-        ? nonEmptyParams
-        : [...nonEmptyParams, defaultLast]
+  ) {
+    if (
+      !(
+        !params ||
+        params.length === expected.length - 1 ||
+        params.length === expected.length
+      )
+    ) {
+      throw new InvalidParametersError(
+        `Expected ${expected} parameters but received ${params.length}.`
+      )
     }
-    throw new InvalidParametersError(
-      `Expected ${expected} parameters but received ${
-        !params ? 0 : params.length
-      }.`
-    )
+    expected.forEach((expectedType, index) => {
+      const param = params[index]
+      const typeChecks = {
+        [Web3RpcTypes.quantityOrTag]: (value) => {
+          return (
+            value === undefined ||
+            !isNaN(value) ||
+            ['latest', 'earliest', 'pending'].includes(value)
+          )
+        },
+        [Web3RpcTypes.boolean]: (value) => [true, false].includes(value),
+        [Web3RpcTypes.quantity]: (value) => !isNaN(value),
+        [Web3RpcTypes.data]: Web3.utils.isHex,
+        [Web3RpcTypes.address]: Web3.utils.isAddress,
+        [Web3RpcTypes.object]: (value) => {
+          return value instanceof Object
+        },
+      }
+
+      if (!typeChecks[expectedType](param)) {
+        throw new InvalidParametersError(
+          `Expected ${expectedType} but got ${param}`
+        )
+      }
+    })
   }
 }

packages/rollup-full-node/src/types/web3-rpc-handler.ts

@@ -30,6 +30,15 @@ export interface Web3Handler {
   chainId(): Promise<string>
 }
 
+export enum Web3RpcTypes {
+  quantity = 'quantity',
+  boolean = 'boolean',
+  data = 'data',
+  address = 'address',
+  object = 'object',
+  quantityOrTag = 'quantityOrTag',
+}
+
 // Enum of supported web3 rpc methods
 export enum Web3RpcMethods {
   blockNumber = 'eth_blockNumber',

packages/rollup-full-node/test/app/web-rpc-handler.spec.ts

@@ -6,6 +6,7 @@ import {
   getLogger,
   keccak256,
   numberToHexString,
+  JSONRPC_ERRORS,
   ZERO_ADDRESS,
   hexStrToBuf,
 } from '@eth-optimism/core-utils'
@@ -178,6 +179,14 @@ describe('Web3Handler', () => {
 
         balance.toNumber().should.eq(0)
       })
+
+      it('should return a parameter error if an invalid parameter is passed', async () => {
+        const wallet = getWallet(httpProvider)
+
+        await assertAsyncThrowsWithMessage(async () => {
+          await httpProvider.send('eth_getBalance', [1])
+        }, JSONRPC_ERRORS.INVALID_PARAMS.message)
+      })
     })
 
     describe('EVM reversion handling', async () => {