From 7ab921669bf52c1cb2d43350b2cccc8efe91bdbd Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 13:58:28 -0700 Subject: Introduce subprovider for printing revert stack traces --- packages/contracts/package.json | 1 + packages/contracts/src/utils/revert_trace.ts | 21 ++ packages/contracts/src/utils/web3_wrapper.ts | 51 ++--- packages/dev-utils/src/env.ts | 1 + packages/sol-cov/src/index.ts | 1 + packages/sol-cov/src/revert_trace.ts | 82 ++++++++ packages/sol-cov/src/revert_trace_subprovider.ts | 237 +++++++++++++++++++++++ packages/sol-cov/src/trace.ts | 41 ++-- packages/sol-cov/src/types.ts | 7 + packages/sol-cov/src/utils.ts | 24 +++ 10 files changed, 416 insertions(+), 50 deletions(-) create mode 100644 packages/contracts/src/utils/revert_trace.ts create mode 100644 packages/sol-cov/src/revert_trace.ts create mode 100644 packages/sol-cov/src/revert_trace_subprovider.ts (limited to 'packages') diff --git a/packages/contracts/package.json b/packages/contracts/package.json index d6ca3727b..ad7cbf476 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -19,6 +19,7 @@ "rebuild_and_test": "run-s build test", "test:coverage": "SOLIDITY_COVERAGE=true run-s build run_mocha coverage:report:text coverage:report:lcov", "test:profiler": "SOLIDITY_PROFILER=true run-s build run_mocha profiler:report:html", + "test:trace": "SOLIDITY_REVERT_TRACE=true run-s run_mocha", "run_mocha": "mocha --require source-map-support/register 'lib/test/**/*.js' --timeout 100000 --bail --exit", "compile": "sol-compiler", "clean": "shx rm -rf lib src/generated_contract_wrappers", diff --git a/packages/contracts/src/utils/revert_trace.ts b/packages/contracts/src/utils/revert_trace.ts new file mode 100644 index 000000000..0bf8384bc --- /dev/null +++ b/packages/contracts/src/utils/revert_trace.ts @@ -0,0 +1,21 @@ +import { devConstants } from '@0xproject/dev-utils'; +import { RevertTraceSubprovider, SolCompilerArtifactAdapter } from '@0xproject/sol-cov'; +import * as _ from 'lodash'; + +let revertTraceSubprovider: RevertTraceSubprovider; + +export const revertTrace = { + getRevertTraceSubproviderSingleton(): RevertTraceSubprovider { + if (_.isUndefined(revertTraceSubprovider)) { + revertTraceSubprovider = revertTrace._getRevertTraceSubprovider(); + } + return revertTraceSubprovider; + }, + _getRevertTraceSubprovider(): RevertTraceSubprovider { + const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; + const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); + const isVerbose = true; + const subprovider = new RevertTraceSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); + return subprovider; + }, +}; diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index c475d96a9..b8e8ed8ce 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -5,6 +5,7 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { coverage } from './coverage'; import { profiler } from './profiler'; +import { revertTrace } from './revert_trace'; enum ProviderType { Ganache = 'ganache', @@ -48,28 +49,34 @@ const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); -if (isCoverageEnabled && isProfilerEnabled) { - throw new Error( - `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, - ); -} -if (isCoverageEnabled) { - const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); - prependSubprovider(provider, coverageSubprovider); -} -if (isProfilerEnabled) { - if (testProvider === ProviderType.Ganache) { - logUtils.warn( - "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", - ); - process.exit(1); - } - const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); - logUtils.log( - "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", - ); - profilerSubprovider.stop(); - prependSubprovider(provider, profilerSubprovider); +const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); +// TODO(albrow): Include revertTrace checks in the warnings below. +// if (isCoverageEnabled && isProfilerEnabled) { +// throw new Error( +// `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, +// ); +// } +// if (isCoverageEnabled) { +// const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); +// prependSubprovider(provider, coverageSubprovider); +// } +// if (isProfilerEnabled) { +// if (testProvider === ProviderType.Ganache) { +// logUtils.warn( +// "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", +// ); +// process.exit(1); +// } +// const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); +// logUtils.log( +// "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", +// ); +// profilerSubprovider.stop(); +// prependSubprovider(provider, profilerSubprovider); +// } +if (isRevertTraceEnabled) { + const revertTraceSubprovider = revertTrace.getRevertTraceSubproviderSingleton(); + prependSubprovider(provider, revertTraceSubprovider); } export const web3Wrapper = new Web3Wrapper(provider); diff --git a/packages/dev-utils/src/env.ts b/packages/dev-utils/src/env.ts index e74ef3d23..024162c2f 100644 --- a/packages/dev-utils/src/env.ts +++ b/packages/dev-utils/src/env.ts @@ -4,6 +4,7 @@ import * as process from 'process'; export enum EnvVars { SolidityCoverage = 'SOLIDITY_COVERAGE', SolidityProfiler = 'SOLIDITY_PROFILER', + SolidityRevertTrace = 'SOLIDITY_REVERT_TRACE', VerboseGanache = 'VERBOSE_GANACHE', } diff --git a/packages/sol-cov/src/index.ts b/packages/sol-cov/src/index.ts index 10f6d9597..003a27374 100644 --- a/packages/sol-cov/src/index.ts +++ b/packages/sol-cov/src/index.ts @@ -1,6 +1,7 @@ export { CoverageSubprovider } from './coverage_subprovider'; // HACK: ProfilerSubprovider is a hacky way to do profiling using coverage tools. Not production ready export { ProfilerSubprovider } from './profiler_subprovider'; +export { RevertTraceSubprovider } from './revert_trace_subprovider'; export { SolCompilerArtifactAdapter } from './artifact_adapters/sol_compiler_artifact_adapter'; export { TruffleArtifactAdapter } from './artifact_adapters/truffle_artifact_adapter'; export { AbstractArtifactAdapter } from './artifact_adapters/abstract_artifact_adapter'; diff --git a/packages/sol-cov/src/revert_trace.ts b/packages/sol-cov/src/revert_trace.ts new file mode 100644 index 000000000..59edf5db4 --- /dev/null +++ b/packages/sol-cov/src/revert_trace.ts @@ -0,0 +1,82 @@ +import { logUtils } from '@0xproject/utils'; +import { OpCode, StructLog } from 'ethereum-types'; + +import * as _ from 'lodash'; + +import { EvmCallStack, EvmCallStackEntry } from './types'; +import { utils } from './utils'; + +export function getRevertTrace(structLogs: StructLog[], startAddress: string): EvmCallStack { + const evmCallStack: EvmCallStack = []; + let currentAddress = startAddress; + if (_.isEmpty(structLogs)) { + return []; + } + const normalizedStructLogs = utils.normalizeStructLogs(structLogs); + // tslint:disable-next-line:prefer-for-of + for (let i = 0; i < normalizedStructLogs.length; i++) { + const structLog = normalizedStructLogs[i]; + if (structLog.depth !== evmCallStack.length) { + throw new Error("Malformed trace. Trace depth doesn't match call stack depth"); + } + // After that check we have a guarantee that call stack is never empty + // If it would: callStack.length - 1 === structLog.depth === -1 + // That means that we can always safely pop from it + + // TODO(albrow): split out isCallLike and isEndOpcode + if (utils.isCallLike(structLog.op)) { + const evmCallStackEntry = _.last(evmCallStack) as EvmCallStackEntry; + const prevAddress = _.isUndefined(evmCallStackEntry) ? currentAddress : evmCallStackEntry.address; + const jumpAddressOffset = 1; + currentAddress = utils.getAddressFromStackEntry( + structLog.stack[structLog.stack.length - jumpAddressOffset - 1], + ); + + if (structLog === _.last(normalizedStructLogs)) { + throw new Error('Malformed trace. CALL-like opcode can not be the last one'); + } + // Sometimes calls don't change the execution context (current address). When we do a transfer to an + // externally owned account - it does the call and immediately returns because there is no fallback + // function. We manually check if the call depth had changed to handle that case. + const nextStructLog = normalizedStructLogs[i + 1]; + if (nextStructLog.depth !== structLog.depth) { + evmCallStack.push({ + address: prevAddress, + structLog, + }); + } + } else if (utils.isEndOpcode(structLog.op) && structLog.op !== OpCode.Revert) { + // Just like with calls, sometimes returns/stops don't change the execution context (current address). + const nextStructLog = normalizedStructLogs[i + 1]; + if (_.isUndefined(nextStructLog) || nextStructLog.depth !== structLog.depth) { + evmCallStack.pop(); + } + if (structLog.op === OpCode.SelfDestruct) { + // After contract execution, we look at all sub-calls to external contracts, and for each one, fetch + // the bytecode and compute the coverage for the call. If the contract is destroyed with a call + // to `selfdestruct`, we are unable to fetch it's bytecode and compute coverage. + // TODO: Refactor this logic to fetch the sub-called contract bytecode before the selfdestruct is called + // in order to handle this edge-case. + logUtils.warn( + "Detected a selfdestruct. Sol-cov currently doesn't support that scenario. We'll just skip the trace part for a destructed contract", + ); + } + } else if (structLog.op === OpCode.Revert) { + evmCallStack.push({ + address: currentAddress, + structLog, + }); + return evmCallStack; + } else if (structLog.op === OpCode.Create) { + // TODO: Extract the new contract address from the stack and handle that scenario + logUtils.warn( + "Detected a contract created from within another contract. Sol-cov currently doesn't support that scenario. We'll just skip that trace", + ); + return []; + } + } + if (evmCallStack.length !== 0) { + logUtils.warn('Malformed trace. Call stack non empty at the end. (probably out of gas)'); + } + return []; +} diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts new file mode 100644 index 000000000..ea878058c --- /dev/null +++ b/packages/sol-cov/src/revert_trace_subprovider.ts @@ -0,0 +1,237 @@ +import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { Callback, ErrorCallback, NextCallback, Subprovider } from '@0xproject/subproviders'; +import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import { CallData, JSONRPCRequestPayload, Provider, TxData } from 'ethereum-types'; +import { stripHexPrefix } from 'ethereumjs-util'; +import * as _ from 'lodash'; +import { getLogger, levels, Logger } from 'loglevel'; +import { Lock } from 'semaphore-async-await'; + +import { AbstractArtifactAdapter } from './artifact_adapters/abstract_artifact_adapter'; +import { constants } from './constants'; +import { getRevertTrace } from './revert_trace'; +import { parseSourceMap } from './source_maps'; +import { BlockParamLiteral, ContractData, EvmCallStack, SourceRange } from './types'; +import { utils } from './utils'; + +interface MaybeFakeTxData extends TxData { + isFakeTransaction?: boolean; +} + +const BLOCK_GAS_LIMIT = 6000000; + +/** + * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. + * It is used to report call stack traces whenever a revert occurs. + */ +export class RevertTraceSubprovider extends Subprovider { + // Lock is used to not accept normal transactions while doing call/snapshot magic because they'll be reverted later otherwise + private _lock = new Lock(); + private _defaultFromAddress: string; + private _web3Wrapper!: Web3Wrapper; + private _isEnabled = true; + private _artifactAdapter: AbstractArtifactAdapter; + private _contractsData!: ContractData[]; + private _logger: Logger; + + /** + * Instantiates a TraceCollectionSubprovider instance + * @param defaultFromAddress default from address to use when sending transactions + */ + constructor(artifactAdapter: AbstractArtifactAdapter, defaultFromAddress: string, isVerbose: boolean) { + super(); + this._artifactAdapter = artifactAdapter; + this._defaultFromAddress = defaultFromAddress; + this._logger = getLogger('sol-cov'); + this._logger.setLevel(isVerbose ? levels.TRACE : levels.ERROR); + } + /** + * Starts trace collection + */ + public start(): void { + this._isEnabled = true; + } + /** + * Stops trace collection + */ + public stop(): void { + this._isEnabled = false; + } + /** + * This method conforms to the web3-provider-engine interface. + * It is called internally by the ProviderEngine when it is this subproviders + * turn to handle a JSON RPC request. + * @param payload JSON RPC payload + * @param next Callback to call if this subprovider decides not to handle the request + * @param end Callback to call if subprovider handled the request and wants to pass back the request. + */ + // tslint:disable-next-line:prefer-function-over-method async-suffix + public async handleRequest(payload: JSONRPCRequestPayload, next: NextCallback, _end: ErrorCallback): Promise { + if (this._isEnabled) { + switch (payload.method) { + case 'eth_sendTransaction': + const txData = payload.params[0]; + next(this._onTransactionSentAsync.bind(this, txData)); + return; + + case 'eth_call': + const callData = payload.params[0]; + next(this._onCallOrGasEstimateExecutedAsync.bind(this, callData)); + return; + + case 'eth_estimateGas': + const estimateGasData = payload.params[0]; + next(this._onCallOrGasEstimateExecutedAsync.bind(this, estimateGasData)); + return; + + default: + next(); + return; + } + } else { + next(); + return; + } + } + /** + * Set's the subprovider's engine to the ProviderEngine it is added to. + * This is only called within the ProviderEngine source code, do not call + * directly. + */ + public setEngine(engine: Provider): void { + super.setEngine(engine); + this._web3Wrapper = new Web3Wrapper(engine); + } + private async _onTransactionSentAsync( + txData: MaybeFakeTxData, + err: Error | null, + txHash: string | undefined, + cb: Callback, + ): Promise { + if (!txData.isFakeTransaction) { + // This transaction is a usual transaction. Not a call executed as one. + // And we don't want it to be executed within a snapshotting period + await this._lock.acquire(); + } + const NULL_ADDRESS = '0x0'; + if (_.isNull(err)) { + const toAddress = + _.isUndefined(txData.to) || txData.to === NULL_ADDRESS ? constants.NEW_CONTRACT : txData.to; + await this._recordTxTraceAsync(toAddress, txData.data, txHash as string); + } else { + const latestBlock = await this._web3Wrapper.getBlockWithTransactionDataAsync(BlockParamLiteral.Latest); + const transactions = latestBlock.transactions; + for (const transaction of transactions) { + const toAddress = + _.isUndefined(txData.to) || txData.to === NULL_ADDRESS ? constants.NEW_CONTRACT : txData.to; + await this._recordTxTraceAsync(toAddress, transaction.input, transaction.hash); + } + } + if (!txData.isFakeTransaction) { + // This transaction is a usual transaction. Not a call executed as one. + // And we don't want it to be executed within a snapshotting period + this._lock.release(); + } + cb(); + } + private async _onCallOrGasEstimateExecutedAsync( + callData: Partial, + _err: Error | null, + _callResult: string, + cb: Callback, + ): Promise { + await this._recordCallOrGasEstimateTraceAsync(callData); + cb(); + } + private async _recordTxTraceAsync(address: string, data: string | undefined, txHash: string): Promise { + await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0); + const trace = await this._web3Wrapper.getTransactionTraceAsync(txHash, { + disableMemory: true, + disableStack: false, + disableStorage: true, + }); + const evmCallStack = getRevertTrace(trace.structLogs, address); + if (evmCallStack.length > 0) { + // if getRevertTrace returns a call stack it means there was a + // revert. + await this._printStackTraceAsync(evmCallStack); + } + } + private async _recordCallOrGasEstimateTraceAsync(callData: Partial): Promise { + // We don't want other transactions to be exeucted during snashotting period, that's why we lock the + // transaction execution for all transactions except our fake ones. + await this._lock.acquire(); + const blockchainLifecycle = new BlockchainLifecycle(this._web3Wrapper); + await blockchainLifecycle.startAsync(); + const fakeTxData: MaybeFakeTxData = { + gas: BLOCK_GAS_LIMIT, + isFakeTransaction: true, // This transaction (and only it) is allowed to come through when the lock is locked + ...callData, + from: callData.from || this._defaultFromAddress, + }; + try { + const txHash = await this._web3Wrapper.sendTransactionAsync(fakeTxData); + await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0); + } catch (err) { + // Even if this transaction failed - we've already recorded it's trace. + _.noop(); + } + await blockchainLifecycle.revertAsync(); + this._lock.release(); + } + private async _printStackTraceAsync(evmCallStack: EvmCallStack): Promise { + const sourceRanges: SourceRange[] = []; + if (_.isUndefined(this._contractsData)) { + this._contractsData = await this._artifactAdapter.collectContractsDataAsync(); + } + for (const evmCallStackEntry of evmCallStack) { + const isContractCreation = evmCallStackEntry.address === constants.NEW_CONTRACT; + const bytecode = await this._web3Wrapper.getContractCodeAsync(evmCallStackEntry.address); + const contractData = utils.getContractDataIfExists(this._contractsData, bytecode); + if (_.isUndefined(contractData)) { + const errMsg = isContractCreation + ? `Unknown contract creation transaction` + : `Transaction to an unknown address: ${evmCallStackEntry.address}`; + this._logger.warn(errMsg); + continue; + } + const bytecodeHex = stripHexPrefix(bytecode); + const sourceMap = isContractCreation ? contractData.sourceMap : contractData.sourceMapRuntime; + const pcToSourceRange = parseSourceMap( + contractData.sourceCodes, + sourceMap, + bytecodeHex, + contractData.sources, + ); + // tslint:disable-next-line:no-unnecessary-initializer + let sourceRange: SourceRange | undefined = undefined; + let pc = evmCallStackEntry.structLog.pc; + // Sometimes there is not a mapping for this pc (e.g. if the revert + // actually happens in assembly). In that case, we want to keep + // searching backwards by decrementing the pc until we find a + // mapped source range. + while (_.isUndefined(sourceRange)) { + sourceRange = pcToSourceRange[pc]; + pc -= 1; + if (pc <= 0) { + this._logger.warn( + `could not find matching sourceRange for structLog: ${evmCallStackEntry.structLog}`, + ); + continue; + } + } + sourceRanges.push(sourceRange); + } + if (sourceRanges.length > 0) { + this._logger.error('\n\nStack trace:\n'); + _.forEach(sourceRanges, sourceRange => { + this._logger.error( + `${sourceRange.fileName}:${sourceRange.location.start.line}:${sourceRange.location.start.column}`, + ); + }); + this._logger.error('\n'); + } else { + this._logger.error('Could not determine stack trace'); + } + } +} diff --git a/packages/sol-cov/src/trace.ts b/packages/sol-cov/src/trace.ts index 45e45e9c5..b43fd19a2 100644 --- a/packages/sol-cov/src/trace.ts +++ b/packages/sol-cov/src/trace.ts @@ -1,17 +1,13 @@ -import { addressUtils, BigNumber, logUtils } from '@0xproject/utils'; +import { logUtils } from '@0xproject/utils'; import { OpCode, StructLog } from 'ethereum-types'; -import { addHexPrefix } from 'ethereumjs-util'; import * as _ from 'lodash'; +import { utils } from './utils'; + export interface TraceByContractAddress { [contractAddress: string]: StructLog[]; } -function getAddressFromStackEntry(stackEntry: string): string { - const hexBase = 16; - return addressUtils.padZeros(new BigNumber(addHexPrefix(stackEntry)).toString(hexBase)); -} - export function getTracesByContractAddress(structLogs: StructLog[], startAddress: string): TraceByContractAddress { const traceByContractAddress: TraceByContractAddress = {}; let currentTraceSegment = []; @@ -19,13 +15,10 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress if (_.isEmpty(structLogs)) { return traceByContractAddress; } - if (structLogs[0].depth === 1) { - // Geth uses 1-indexed depth counter whilst ganache starts from 0 - _.forEach(structLogs, structLog => structLog.depth--); - } + const normalizedStructLogs = utils.normalizeStructLogs(structLogs); // tslint:disable-next-line:prefer-for-of - for (let i = 0; i < structLogs.length; i++) { - const structLog = structLogs[i]; + for (let i = 0; i < normalizedStructLogs.length; i++) { + const structLog = normalizedStructLogs[i]; if (structLog.depth !== callStack.length - 1) { throw new Error("Malformed trace. Trace depth doesn't match call stack depth"); } @@ -34,27 +27,19 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress // That means that we can always safely pop from it currentTraceSegment.push(structLog); - const isCallLike = _.includes( - [OpCode.CallCode, OpCode.StaticCall, OpCode.Call, OpCode.DelegateCall], - structLog.op, - ); - const isEndOpcode = _.includes( - [OpCode.Return, OpCode.Stop, OpCode.Revert, OpCode.Invalid, OpCode.SelfDestruct], - structLog.op, - ); - if (isCallLike) { + if (utils.isCallLike(structLog.op)) { const currentAddress = _.last(callStack) as string; const jumpAddressOffset = 1; - const newAddress = getAddressFromStackEntry( + const newAddress = utils.getAddressFromStackEntry( structLog.stack[structLog.stack.length - jumpAddressOffset - 1], ); - if (structLog === _.last(structLogs)) { + if (structLog === _.last(normalizedStructLogs)) { throw new Error('Malformed trace. CALL-like opcode can not be the last one'); } // Sometimes calls don't change the execution context (current address). When we do a transfer to an // externally owned account - it does the call and immediately returns because there is no fallback // function. We manually check if the call depth had changed to handle that case. - const nextStructLog = structLogs[i + 1]; + const nextStructLog = normalizedStructLogs[i + 1]; if (nextStructLog.depth !== structLog.depth) { callStack.push(newAddress); traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat( @@ -62,7 +47,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress ); currentTraceSegment = []; } - } else if (isEndOpcode) { + } else if (utils.isEndOpcode(structLog.op)) { const currentAddress = callStack.pop() as string; traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat( currentTraceSegment, @@ -85,8 +70,8 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress ); return traceByContractAddress; } else { - if (structLog !== _.last(structLogs)) { - const nextStructLog = structLogs[i + 1]; + if (structLog !== _.last(normalizedStructLogs)) { + const nextStructLog = normalizedStructLogs[i + 1]; if (nextStructLog.depth === structLog.depth) { continue; } else if (nextStructLog.depth === structLog.depth - 1) { diff --git a/packages/sol-cov/src/types.ts b/packages/sol-cov/src/types.ts index 896d4a7b5..cef7141cb 100644 --- a/packages/sol-cov/src/types.ts +++ b/packages/sol-cov/src/types.ts @@ -107,3 +107,10 @@ export type TraceInfo = TraceInfoNewContract | TraceInfoExistingContract; export enum BlockParamLiteral { Latest = 'latest', } + +export interface EvmCallStackEntry { + structLog: StructLog; + address: string; +} + +export type EvmCallStack = EvmCallStackEntry[]; diff --git a/packages/sol-cov/src/utils.ts b/packages/sol-cov/src/utils.ts index 0b32df02e..4f16a1cda 100644 --- a/packages/sol-cov/src/utils.ts +++ b/packages/sol-cov/src/utils.ts @@ -1,3 +1,6 @@ +import { addressUtils, BigNumber } from '@0xproject/utils'; +import { OpCode, StructLog } from 'ethereum-types'; +import { addHexPrefix } from 'ethereumjs-util'; import * as _ from 'lodash'; import { ContractData, LineColumn, SingleFileSourceRange } from './types'; @@ -42,4 +45,25 @@ export const utils = { }); return contractData; }, + isCallLike(op: OpCode): boolean { + return _.includes([OpCode.CallCode, OpCode.StaticCall, OpCode.Call, OpCode.DelegateCall], op); + }, + isEndOpcode(op: OpCode): boolean { + return _.includes([OpCode.Return, OpCode.Stop, OpCode.Revert, OpCode.Invalid, OpCode.SelfDestruct], op); + }, + getAddressFromStackEntry(stackEntry: string): string { + const hexBase = 16; + return addressUtils.padZeros(new BigNumber(addHexPrefix(stackEntry)).toString(hexBase)); + }, + normalizeStructLogs(structLogs: StructLog[]): StructLog[] { + if (structLogs[0].depth === 1) { + // Geth uses 1-indexed depth counter whilst ganache starts from 0 + const newStructLogs = _.map(structLogs, structLog => ({ + ...structLog, + depth: structLog.depth - 1, + })); + return newStructLogs; + } + return structLogs; + }, }; -- cgit From 263bfb1bdad8acdb9b534edf6e79024e8da35721 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 15:46:59 -0700 Subject: Fix a bug in revert_trace.ts --- packages/contracts/src/utils/web3_wrapper.ts | 46 ++++++++++++------------ packages/sol-cov/src/revert_trace.ts | 27 +++++++++----- packages/sol-cov/src/revert_trace_subprovider.ts | 6 +++- packages/sol-cov/src/trace.ts | 16 ++++----- 4 files changed, 55 insertions(+), 40 deletions(-) (limited to 'packages') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index b8e8ed8ce..f51ad435b 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -51,29 +51,29 @@ const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); // TODO(albrow): Include revertTrace checks in the warnings below. -// if (isCoverageEnabled && isProfilerEnabled) { -// throw new Error( -// `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, -// ); -// } -// if (isCoverageEnabled) { -// const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); -// prependSubprovider(provider, coverageSubprovider); -// } -// if (isProfilerEnabled) { -// if (testProvider === ProviderType.Ganache) { -// logUtils.warn( -// "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", -// ); -// process.exit(1); -// } -// const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); -// logUtils.log( -// "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", -// ); -// profilerSubprovider.stop(); -// prependSubprovider(provider, profilerSubprovider); -// } +if (isCoverageEnabled && isProfilerEnabled) { + throw new Error( + `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, + ); +} +if (isCoverageEnabled) { + const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); + prependSubprovider(provider, coverageSubprovider); +} +if (isProfilerEnabled) { + if (testProvider === ProviderType.Ganache) { + logUtils.warn( + "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", + ); + process.exit(1); + } + const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); + logUtils.log( + "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", + ); + profilerSubprovider.stop(); + prependSubprovider(provider, profilerSubprovider); +} if (isRevertTraceEnabled) { const revertTraceSubprovider = revertTrace.getRevertTraceSubproviderSingleton(); prependSubprovider(provider, revertTraceSubprovider); diff --git a/packages/sol-cov/src/revert_trace.ts b/packages/sol-cov/src/revert_trace.ts index 59edf5db4..04d410f0a 100644 --- a/packages/sol-cov/src/revert_trace.ts +++ b/packages/sol-cov/src/revert_trace.ts @@ -8,7 +8,7 @@ import { utils } from './utils'; export function getRevertTrace(structLogs: StructLog[], startAddress: string): EvmCallStack { const evmCallStack: EvmCallStack = []; - let currentAddress = startAddress; + const addressStack = [startAddress]; if (_.isEmpty(structLogs)) { return []; } @@ -16,19 +16,17 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E // tslint:disable-next-line:prefer-for-of for (let i = 0; i < normalizedStructLogs.length; i++) { const structLog = normalizedStructLogs[i]; - if (structLog.depth !== evmCallStack.length) { + if (structLog.depth !== addressStack.length - 1) { throw new Error("Malformed trace. Trace depth doesn't match call stack depth"); } // After that check we have a guarantee that call stack is never empty // If it would: callStack.length - 1 === structLog.depth === -1 // That means that we can always safely pop from it - // TODO(albrow): split out isCallLike and isEndOpcode if (utils.isCallLike(structLog.op)) { - const evmCallStackEntry = _.last(evmCallStack) as EvmCallStackEntry; - const prevAddress = _.isUndefined(evmCallStackEntry) ? currentAddress : evmCallStackEntry.address; + const currentAddress = _.last(addressStack) as string; const jumpAddressOffset = 1; - currentAddress = utils.getAddressFromStackEntry( + const newAddress = utils.getAddressFromStackEntry( structLog.stack[structLog.stack.length - jumpAddressOffset - 1], ); @@ -40,8 +38,9 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E // function. We manually check if the call depth had changed to handle that case. const nextStructLog = normalizedStructLogs[i + 1]; if (nextStructLog.depth !== structLog.depth) { + addressStack.push(newAddress); evmCallStack.push({ - address: prevAddress, + address: currentAddress, structLog, }); } @@ -50,6 +49,7 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E const nextStructLog = normalizedStructLogs[i + 1]; if (_.isUndefined(nextStructLog) || nextStructLog.depth !== structLog.depth) { evmCallStack.pop(); + addressStack.pop(); } if (structLog.op === OpCode.SelfDestruct) { // After contract execution, we look at all sub-calls to external contracts, and for each one, fetch @@ -63,7 +63,7 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E } } else if (structLog.op === OpCode.Revert) { evmCallStack.push({ - address: currentAddress, + address: _.last(addressStack) as string, structLog, }); return evmCallStack; @@ -73,6 +73,17 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E "Detected a contract created from within another contract. Sol-cov currently doesn't support that scenario. We'll just skip that trace", ); return []; + } else { + if (structLog !== _.last(normalizedStructLogs)) { + const nextStructLog = normalizedStructLogs[i + 1]; + if (nextStructLog.depth === structLog.depth) { + continue; + } else if (nextStructLog.depth === structLog.depth - 1) { + addressStack.pop(); + } else { + throw new Error('Malformed trace. Unexpected call depth change'); + } + } } } if (evmCallStack.length !== 0) { diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts index ea878058c..aef9c7568 100644 --- a/packages/sol-cov/src/revert_trace_subprovider.ts +++ b/packages/sol-cov/src/revert_trace_subprovider.ts @@ -186,6 +186,10 @@ export class RevertTraceSubprovider extends Subprovider { } for (const evmCallStackEntry of evmCallStack) { const isContractCreation = evmCallStackEntry.address === constants.NEW_CONTRACT; + if (isContractCreation) { + this._logger.error('Contract creation not supported'); + continue; + } const bytecode = await this._web3Wrapper.getContractCodeAsync(evmCallStackEntry.address); const contractData = utils.getContractDataIfExists(this._contractsData, bytecode); if (_.isUndefined(contractData)) { @@ -223,7 +227,7 @@ export class RevertTraceSubprovider extends Subprovider { sourceRanges.push(sourceRange); } if (sourceRanges.length > 0) { - this._logger.error('\n\nStack trace:\n'); + this._logger.error('\n\nStack trace for REVERT:\n'); _.forEach(sourceRanges, sourceRange => { this._logger.error( `${sourceRange.fileName}:${sourceRange.location.start.line}:${sourceRange.location.start.column}`, diff --git a/packages/sol-cov/src/trace.ts b/packages/sol-cov/src/trace.ts index b43fd19a2..fad2e5e08 100644 --- a/packages/sol-cov/src/trace.ts +++ b/packages/sol-cov/src/trace.ts @@ -11,7 +11,7 @@ export interface TraceByContractAddress { export function getTracesByContractAddress(structLogs: StructLog[], startAddress: string): TraceByContractAddress { const traceByContractAddress: TraceByContractAddress = {}; let currentTraceSegment = []; - const callStack = [startAddress]; + const addressStack = [startAddress]; if (_.isEmpty(structLogs)) { return traceByContractAddress; } @@ -19,7 +19,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress // tslint:disable-next-line:prefer-for-of for (let i = 0; i < normalizedStructLogs.length; i++) { const structLog = normalizedStructLogs[i]; - if (structLog.depth !== callStack.length - 1) { + if (structLog.depth !== addressStack.length - 1) { throw new Error("Malformed trace. Trace depth doesn't match call stack depth"); } // After that check we have a guarantee that call stack is never empty @@ -28,7 +28,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress currentTraceSegment.push(structLog); if (utils.isCallLike(structLog.op)) { - const currentAddress = _.last(callStack) as string; + const currentAddress = _.last(addressStack) as string; const jumpAddressOffset = 1; const newAddress = utils.getAddressFromStackEntry( structLog.stack[structLog.stack.length - jumpAddressOffset - 1], @@ -41,14 +41,14 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress // function. We manually check if the call depth had changed to handle that case. const nextStructLog = normalizedStructLogs[i + 1]; if (nextStructLog.depth !== structLog.depth) { - callStack.push(newAddress); + addressStack.push(newAddress); traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat( currentTraceSegment, ); currentTraceSegment = []; } } else if (utils.isEndOpcode(structLog.op)) { - const currentAddress = callStack.pop() as string; + const currentAddress = addressStack.pop() as string; traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat( currentTraceSegment, ); @@ -75,7 +75,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress if (nextStructLog.depth === structLog.depth) { continue; } else if (nextStructLog.depth === structLog.depth - 1) { - const currentAddress = callStack.pop() as string; + const currentAddress = addressStack.pop() as string; traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat( currentTraceSegment, ); @@ -86,11 +86,11 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress } } } - if (callStack.length !== 0) { + if (addressStack.length !== 0) { logUtils.warn('Malformed trace. Call stack non empty at the end'); } if (currentTraceSegment.length !== 0) { - const currentAddress = callStack.pop() as string; + const currentAddress = addressStack.pop() as string; traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat( currentTraceSegment, ); -- cgit From a9c23b7c2857a826756b919e6be73456536e344d Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 15:50:54 -0700 Subject: Reverse order of stack trace to match behavior of most other language stack traces --- packages/sol-cov/src/revert_trace_subprovider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages') diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts index aef9c7568..1889f13c4 100644 --- a/packages/sol-cov/src/revert_trace_subprovider.ts +++ b/packages/sol-cov/src/revert_trace_subprovider.ts @@ -228,7 +228,7 @@ export class RevertTraceSubprovider extends Subprovider { } if (sourceRanges.length > 0) { this._logger.error('\n\nStack trace for REVERT:\n'); - _.forEach(sourceRanges, sourceRange => { + _.forEach(_.reverse(sourceRanges), sourceRange => { this._logger.error( `${sourceRange.fileName}:${sourceRange.location.start.line}:${sourceRange.location.start.column}`, ); -- cgit From d9292a70bfa00715b6b007b0ecd696db02b1d042 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:00:24 -0700 Subject: Remove unused variables and other small fixes --- packages/contracts/package.json | 2 +- packages/contracts/src/utils/web3_wrapper.ts | 11 ++++++----- packages/sol-cov/src/revert_trace.ts | 2 +- packages/sol-cov/src/revert_trace_subprovider.ts | 6 +++--- 4 files changed, 11 insertions(+), 10 deletions(-) (limited to 'packages') diff --git a/packages/contracts/package.json b/packages/contracts/package.json index ad7cbf476..2495795dc 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -19,7 +19,7 @@ "rebuild_and_test": "run-s build test", "test:coverage": "SOLIDITY_COVERAGE=true run-s build run_mocha coverage:report:text coverage:report:lcov", "test:profiler": "SOLIDITY_PROFILER=true run-s build run_mocha profiler:report:html", - "test:trace": "SOLIDITY_REVERT_TRACE=true run-s run_mocha", + "test:trace": "SOLIDITY_REVERT_TRACE=true run-s build run_mocha", "run_mocha": "mocha --require source-map-support/register 'lib/test/**/*.js' --timeout 100000 --bail --exit", "compile": "sol-compiler", "clean": "shx rm -rf lib src/generated_contract_wrappers", diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index f51ad435b..772e4c613 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -7,6 +7,8 @@ import { coverage } from './coverage'; import { profiler } from './profiler'; import { revertTrace } from './revert_trace'; +import * as _ from 'lodash'; + enum ProviderType { Ganache = 'ganache', Geth = 'geth', @@ -50,11 +52,10 @@ export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); -// TODO(albrow): Include revertTrace checks in the warnings below. -if (isCoverageEnabled && isProfilerEnabled) { - throw new Error( - `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, - ); +const enabledSubproviderCount = _.filter([isCoverageEnabled, isProfilerEnabled, isRevertTraceEnabled], _.identity) + .length; +if (enabledSubproviderCount > 1) { + throw new Error(`Only one of coverage, profiler, and revert trace subproviders can be enabled at a time`); } if (isCoverageEnabled) { const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); diff --git a/packages/sol-cov/src/revert_trace.ts b/packages/sol-cov/src/revert_trace.ts index 04d410f0a..1d52b969e 100644 --- a/packages/sol-cov/src/revert_trace.ts +++ b/packages/sol-cov/src/revert_trace.ts @@ -3,7 +3,7 @@ import { OpCode, StructLog } from 'ethereum-types'; import * as _ from 'lodash'; -import { EvmCallStack, EvmCallStackEntry } from './types'; +import { EvmCallStack } from './types'; import { utils } from './utils'; export function getRevertTrace(structLogs: StructLog[], startAddress: string): EvmCallStack { diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts index 1889f13c4..68e752aee 100644 --- a/packages/sol-cov/src/revert_trace_subprovider.ts +++ b/packages/sol-cov/src/revert_trace_subprovider.ts @@ -117,14 +117,14 @@ export class RevertTraceSubprovider extends Subprovider { if (_.isNull(err)) { const toAddress = _.isUndefined(txData.to) || txData.to === NULL_ADDRESS ? constants.NEW_CONTRACT : txData.to; - await this._recordTxTraceAsync(toAddress, txData.data, txHash as string); + await this._recordTxTraceAsync(toAddress, txHash as string); } else { const latestBlock = await this._web3Wrapper.getBlockWithTransactionDataAsync(BlockParamLiteral.Latest); const transactions = latestBlock.transactions; for (const transaction of transactions) { const toAddress = _.isUndefined(txData.to) || txData.to === NULL_ADDRESS ? constants.NEW_CONTRACT : txData.to; - await this._recordTxTraceAsync(toAddress, transaction.input, transaction.hash); + await this._recordTxTraceAsync(toAddress, transaction.hash); } } if (!txData.isFakeTransaction) { @@ -143,7 +143,7 @@ export class RevertTraceSubprovider extends Subprovider { await this._recordCallOrGasEstimateTraceAsync(callData); cb(); } - private async _recordTxTraceAsync(address: string, data: string | undefined, txHash: string): Promise { + private async _recordTxTraceAsync(address: string, txHash: string): Promise { await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0); const trace = await this._web3Wrapper.getTransactionTraceAsync(txHash, { disableMemory: true, -- cgit From 5a8539a1228baeb085ed7851245337f27ee1d974 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:04:08 -0700 Subject: Fix linter errors and remove coverage.json --- packages/contracts/src/utils/web3_wrapper.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'packages') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 772e4c613..67b71cdeb 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -2,13 +2,12 @@ import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils'; import { prependSubprovider } from '@0xproject/subproviders'; import { logUtils } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import * as _ from 'lodash'; import { coverage } from './coverage'; import { profiler } from './profiler'; import { revertTrace } from './revert_trace'; -import * as _ from 'lodash'; - enum ProviderType { Ganache = 'ganache', Geth = 'geth', -- cgit From 897560745a7e528691e03ecfa99ca25da26135ba Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:33:09 -0700 Subject: De-duplicate code by refactoring subprovider classes --- packages/sol-cov/src/coverage_subprovider.ts | 6 +- packages/sol-cov/src/profiler_subprovider.ts | 6 +- packages/sol-cov/src/revert_trace_subprovider.ts | 158 ++------------------- .../sol-cov/src/trace_collection_subprovider.ts | 64 ++------- packages/sol-cov/src/trace_info_subprovider.ts | 59 ++++++++ 5 files changed, 89 insertions(+), 204 deletions(-) create mode 100644 packages/sol-cov/src/trace_info_subprovider.ts (limited to 'packages') diff --git a/packages/sol-cov/src/coverage_subprovider.ts b/packages/sol-cov/src/coverage_subprovider.ts index 0fa7f873e..065a48434 100644 --- a/packages/sol-cov/src/coverage_subprovider.ts +++ b/packages/sol-cov/src/coverage_subprovider.ts @@ -2,8 +2,8 @@ import * as _ from 'lodash'; import { AbstractArtifactAdapter } from './artifact_adapters/abstract_artifact_adapter'; import { collectCoverageEntries } from './collect_coverage_entries'; -import { TraceCollectionSubprovider } from './trace_collection_subprovider'; import { SingleFileSubtraceHandler, TraceCollector } from './trace_collector'; +import { TraceInfoSubprovider } from './trace_info_subprovider'; import { BranchCoverage, ContractData, @@ -22,7 +22,7 @@ import { utils } from './utils'; * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. * It's used to compute your code coverage while running solidity tests. */ -export class CoverageSubprovider extends TraceCollectionSubprovider { +export class CoverageSubprovider extends TraceInfoSubprovider { private _coverageCollector: TraceCollector; /** * Instantiates a CoverageSubprovider instance @@ -39,7 +39,7 @@ export class CoverageSubprovider extends TraceCollectionSubprovider { super(defaultFromAddress, traceCollectionSubproviderConfig); this._coverageCollector = new TraceCollector(artifactAdapter, isVerbose, coverageHandler); } - public async handleTraceInfoAsync(traceInfo: TraceInfo): Promise { + protected async _handleTraceInfoAsync(traceInfo: TraceInfo): Promise { await this._coverageCollector.computeSingleTraceCoverageAsync(traceInfo); } /** diff --git a/packages/sol-cov/src/profiler_subprovider.ts b/packages/sol-cov/src/profiler_subprovider.ts index 62ed1b472..9f98da524 100644 --- a/packages/sol-cov/src/profiler_subprovider.ts +++ b/packages/sol-cov/src/profiler_subprovider.ts @@ -2,8 +2,8 @@ import * as _ from 'lodash'; import { AbstractArtifactAdapter } from './artifact_adapters/abstract_artifact_adapter'; import { collectCoverageEntries } from './collect_coverage_entries'; -import { TraceCollectionSubprovider } from './trace_collection_subprovider'; import { SingleFileSubtraceHandler, TraceCollector } from './trace_collector'; +import { TraceInfoSubprovider } from './trace_info_subprovider'; import { ContractData, Coverage, SourceRange, Subtrace, TraceInfo } from './types'; import { utils } from './utils'; @@ -11,7 +11,7 @@ import { utils } from './utils'; * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. * ProfilerSubprovider is used to profile Solidity code while running tests. */ -export class ProfilerSubprovider extends TraceCollectionSubprovider { +export class ProfilerSubprovider extends TraceInfoSubprovider { private _profilerCollector: TraceCollector; /** * Instantiates a ProfilerSubprovider instance @@ -28,7 +28,7 @@ export class ProfilerSubprovider extends TraceCollectionSubprovider { super(defaultFromAddress, traceCollectionSubproviderConfig); this._profilerCollector = new TraceCollector(artifactAdapter, isVerbose, profilerHandler); } - public async handleTraceInfoAsync(traceInfo: TraceInfo): Promise { + protected async _handleTraceInfoAsync(traceInfo: TraceInfo): Promise { await this._profilerCollector.computeSingleTraceCoverageAsync(traceInfo); } /** diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts index 68e752aee..f6501757a 100644 --- a/packages/sol-cov/src/revert_trace_subprovider.ts +++ b/packages/sol-cov/src/revert_trace_subprovider.ts @@ -1,149 +1,43 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { Callback, ErrorCallback, NextCallback, Subprovider } from '@0xproject/subproviders'; -import { Web3Wrapper } from '@0xproject/web3-wrapper'; -import { CallData, JSONRPCRequestPayload, Provider, TxData } from 'ethereum-types'; import { stripHexPrefix } from 'ethereumjs-util'; import * as _ from 'lodash'; import { getLogger, levels, Logger } from 'loglevel'; -import { Lock } from 'semaphore-async-await'; import { AbstractArtifactAdapter } from './artifact_adapters/abstract_artifact_adapter'; import { constants } from './constants'; import { getRevertTrace } from './revert_trace'; import { parseSourceMap } from './source_maps'; -import { BlockParamLiteral, ContractData, EvmCallStack, SourceRange } from './types'; +import { TraceCollectionSubprovider } from './trace_collection_subprovider'; +import { ContractData, EvmCallStack, SourceRange } from './types'; import { utils } from './utils'; -interface MaybeFakeTxData extends TxData { - isFakeTransaction?: boolean; -} - -const BLOCK_GAS_LIMIT = 6000000; - /** * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. * It is used to report call stack traces whenever a revert occurs. */ -export class RevertTraceSubprovider extends Subprovider { +export class RevertTraceSubprovider extends TraceCollectionSubprovider { // Lock is used to not accept normal transactions while doing call/snapshot magic because they'll be reverted later otherwise - private _lock = new Lock(); - private _defaultFromAddress: string; - private _web3Wrapper!: Web3Wrapper; - private _isEnabled = true; - private _artifactAdapter: AbstractArtifactAdapter; private _contractsData!: ContractData[]; + private _artifactAdapter: AbstractArtifactAdapter; private _logger: Logger; /** - * Instantiates a TraceCollectionSubprovider instance + * Instantiates a RevertTraceSubprovider instance + * @param artifactAdapter Adapter for used artifacts format (0x, truffle, giveth, etc.) * @param defaultFromAddress default from address to use when sending transactions + * @param isVerbose If true, we will log any unknown transactions. Otherwise we will ignore them */ - constructor(artifactAdapter: AbstractArtifactAdapter, defaultFromAddress: string, isVerbose: boolean) { - super(); + constructor(artifactAdapter: AbstractArtifactAdapter, defaultFromAddress: string, isVerbose: boolean = true) { + const traceCollectionSubproviderConfig = { + shouldCollectTransactionTraces: true, + shouldCollectGasEstimateTraces: true, + shouldCollectCallTraces: true, + }; + super(defaultFromAddress, traceCollectionSubproviderConfig); this._artifactAdapter = artifactAdapter; - this._defaultFromAddress = defaultFromAddress; this._logger = getLogger('sol-cov'); this._logger.setLevel(isVerbose ? levels.TRACE : levels.ERROR); } - /** - * Starts trace collection - */ - public start(): void { - this._isEnabled = true; - } - /** - * Stops trace collection - */ - public stop(): void { - this._isEnabled = false; - } - /** - * This method conforms to the web3-provider-engine interface. - * It is called internally by the ProviderEngine when it is this subproviders - * turn to handle a JSON RPC request. - * @param payload JSON RPC payload - * @param next Callback to call if this subprovider decides not to handle the request - * @param end Callback to call if subprovider handled the request and wants to pass back the request. - */ - // tslint:disable-next-line:prefer-function-over-method async-suffix - public async handleRequest(payload: JSONRPCRequestPayload, next: NextCallback, _end: ErrorCallback): Promise { - if (this._isEnabled) { - switch (payload.method) { - case 'eth_sendTransaction': - const txData = payload.params[0]; - next(this._onTransactionSentAsync.bind(this, txData)); - return; - - case 'eth_call': - const callData = payload.params[0]; - next(this._onCallOrGasEstimateExecutedAsync.bind(this, callData)); - return; - - case 'eth_estimateGas': - const estimateGasData = payload.params[0]; - next(this._onCallOrGasEstimateExecutedAsync.bind(this, estimateGasData)); - return; - - default: - next(); - return; - } - } else { - next(); - return; - } - } - /** - * Set's the subprovider's engine to the ProviderEngine it is added to. - * This is only called within the ProviderEngine source code, do not call - * directly. - */ - public setEngine(engine: Provider): void { - super.setEngine(engine); - this._web3Wrapper = new Web3Wrapper(engine); - } - private async _onTransactionSentAsync( - txData: MaybeFakeTxData, - err: Error | null, - txHash: string | undefined, - cb: Callback, - ): Promise { - if (!txData.isFakeTransaction) { - // This transaction is a usual transaction. Not a call executed as one. - // And we don't want it to be executed within a snapshotting period - await this._lock.acquire(); - } - const NULL_ADDRESS = '0x0'; - if (_.isNull(err)) { - const toAddress = - _.isUndefined(txData.to) || txData.to === NULL_ADDRESS ? constants.NEW_CONTRACT : txData.to; - await this._recordTxTraceAsync(toAddress, txHash as string); - } else { - const latestBlock = await this._web3Wrapper.getBlockWithTransactionDataAsync(BlockParamLiteral.Latest); - const transactions = latestBlock.transactions; - for (const transaction of transactions) { - const toAddress = - _.isUndefined(txData.to) || txData.to === NULL_ADDRESS ? constants.NEW_CONTRACT : txData.to; - await this._recordTxTraceAsync(toAddress, transaction.hash); - } - } - if (!txData.isFakeTransaction) { - // This transaction is a usual transaction. Not a call executed as one. - // And we don't want it to be executed within a snapshotting period - this._lock.release(); - } - cb(); - } - private async _onCallOrGasEstimateExecutedAsync( - callData: Partial, - _err: Error | null, - _callResult: string, - cb: Callback, - ): Promise { - await this._recordCallOrGasEstimateTraceAsync(callData); - cb(); - } - private async _recordTxTraceAsync(address: string, txHash: string): Promise { + protected async _recordTxTraceAsync(address: string, data: string | undefined, txHash: string): Promise { await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0); const trace = await this._web3Wrapper.getTransactionTraceAsync(txHash, { disableMemory: true, @@ -157,28 +51,6 @@ export class RevertTraceSubprovider extends Subprovider { await this._printStackTraceAsync(evmCallStack); } } - private async _recordCallOrGasEstimateTraceAsync(callData: Partial): Promise { - // We don't want other transactions to be exeucted during snashotting period, that's why we lock the - // transaction execution for all transactions except our fake ones. - await this._lock.acquire(); - const blockchainLifecycle = new BlockchainLifecycle(this._web3Wrapper); - await blockchainLifecycle.startAsync(); - const fakeTxData: MaybeFakeTxData = { - gas: BLOCK_GAS_LIMIT, - isFakeTransaction: true, // This transaction (and only it) is allowed to come through when the lock is locked - ...callData, - from: callData.from || this._defaultFromAddress, - }; - try { - const txHash = await this._web3Wrapper.sendTransactionAsync(fakeTxData); - await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0); - } catch (err) { - // Even if this transaction failed - we've already recorded it's trace. - _.noop(); - } - await blockchainLifecycle.revertAsync(); - this._lock.release(); - } private async _printStackTraceAsync(evmCallStack: EvmCallStack): Promise { const sourceRanges: SourceRange[] = []; if (_.isUndefined(this._contractsData)) { diff --git a/packages/sol-cov/src/trace_collection_subprovider.ts b/packages/sol-cov/src/trace_collection_subprovider.ts index 742735935..9866472b9 100644 --- a/packages/sol-cov/src/trace_collection_subprovider.ts +++ b/packages/sol-cov/src/trace_collection_subprovider.ts @@ -6,8 +6,7 @@ import * as _ from 'lodash'; import { Lock } from 'semaphore-async-await'; import { constants } from './constants'; -import { getTracesByContractAddress } from './trace'; -import { BlockParamLiteral, TraceInfo, TraceInfoExistingContract, TraceInfoNewContract } from './types'; +import { BlockParamLiteral } from './types'; interface MaybeFakeTxData extends TxData { isFakeTransaction?: boolean; @@ -27,13 +26,14 @@ export interface TraceCollectionSubproviderConfig { /** * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface. - * It collects traces of all transactions that were sent and all calls that were executed through JSON RPC. + * It collects traces of all transactions that were sent and all calls that were executed through JSON RPC. It must + * be extended by implementing the _recordTxTraceAsync method which is called for every transaction. */ export abstract class TraceCollectionSubprovider extends Subprovider { + protected _web3Wrapper!: Web3Wrapper; // Lock is used to not accept normal transactions while doing call/snapshot magic because they'll be reverted later otherwise private _lock = new Lock(); private _defaultFromAddress: string; - private _web3Wrapper!: Web3Wrapper; private _isEnabled = true; private _config: TraceCollectionSubproviderConfig; /** @@ -57,11 +57,6 @@ export abstract class TraceCollectionSubprovider extends Subprovider { public stop(): void { this._isEnabled = false; } - /** - * Called for each subtrace. - * @param traceInfo Trace info for this subtrace. - */ - public abstract handleTraceInfoAsync(traceInfo: TraceInfo): Promise; /** * This method conforms to the web3-provider-engine interface. * It is called internally by the ProviderEngine when it is this subproviders @@ -119,6 +114,11 @@ export abstract class TraceCollectionSubprovider extends Subprovider { super.setEngine(engine); this._web3Wrapper = new Web3Wrapper(engine); } + protected abstract async _recordTxTraceAsync( + address: string, + data: string | undefined, + txHash: string, + ): Promise; private async _onTransactionSentAsync( txData: MaybeFakeTxData, err: Error | null, @@ -160,52 +160,6 @@ export abstract class TraceCollectionSubprovider extends Subprovider { await this._recordCallOrGasEstimateTraceAsync(callData); cb(); } - private async _recordTxTraceAsync(address: string, data: string | undefined, txHash: string): Promise { - await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0); - const trace = await this._web3Wrapper.getTransactionTraceAsync(txHash, { - disableMemory: true, - disableStack: false, - disableStorage: true, - }); - const tracesByContractAddress = getTracesByContractAddress(trace.structLogs, address); - const subcallAddresses = _.keys(tracesByContractAddress); - if (address === constants.NEW_CONTRACT) { - for (const subcallAddress of subcallAddresses) { - let traceInfo: TraceInfoNewContract | TraceInfoExistingContract; - if (subcallAddress === 'NEW_CONTRACT') { - const traceForThatSubcall = tracesByContractAddress[subcallAddress]; - traceInfo = { - subtrace: traceForThatSubcall, - txHash, - address: subcallAddress, - bytecode: data as string, - }; - } else { - const runtimeBytecode = await this._web3Wrapper.getContractCodeAsync(subcallAddress); - const traceForThatSubcall = tracesByContractAddress[subcallAddress]; - traceInfo = { - subtrace: traceForThatSubcall, - txHash, - address: subcallAddress, - runtimeBytecode, - }; - } - await this.handleTraceInfoAsync(traceInfo); - } - } else { - for (const subcallAddress of subcallAddresses) { - const runtimeBytecode = await this._web3Wrapper.getContractCodeAsync(subcallAddress); - const traceForThatSubcall = tracesByContractAddress[subcallAddress]; - const traceInfo: TraceInfoExistingContract = { - subtrace: traceForThatSubcall, - txHash, - address: subcallAddress, - runtimeBytecode, - }; - await this.handleTraceInfoAsync(traceInfo); - } - } - } private async _recordCallOrGasEstimateTraceAsync(callData: Partial): Promise { // We don't want other transactions to be exeucted during snashotting period, that's why we lock the // transaction execution for all transactions except our fake ones. diff --git a/packages/sol-cov/src/trace_info_subprovider.ts b/packages/sol-cov/src/trace_info_subprovider.ts new file mode 100644 index 000000000..635a68f58 --- /dev/null +++ b/packages/sol-cov/src/trace_info_subprovider.ts @@ -0,0 +1,59 @@ +import * as _ from 'lodash'; + +import { constants } from './constants'; +import { getTracesByContractAddress } from './trace'; +import { TraceCollectionSubprovider } from './trace_collection_subprovider'; +import { TraceInfo, TraceInfoExistingContract, TraceInfoNewContract } from './types'; + +// TraceInfoSubprovider is extended by subproviders which need to work with one +// TraceInfo at a time. It has one abstract method: _handleTraceInfoAsync, which +// is called for each TraceInfo. +export abstract class TraceInfoSubprovider extends TraceCollectionSubprovider { + protected abstract _handleTraceInfoAsync(traceInfo: TraceInfo): Promise; + protected async _recordTxTraceAsync(address: string, data: string | undefined, txHash: string): Promise { + await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0); + const trace = await this._web3Wrapper.getTransactionTraceAsync(txHash, { + disableMemory: true, + disableStack: false, + disableStorage: true, + }); + const tracesByContractAddress = getTracesByContractAddress(trace.structLogs, address); + const subcallAddresses = _.keys(tracesByContractAddress); + if (address === constants.NEW_CONTRACT) { + for (const subcallAddress of subcallAddresses) { + let traceInfo: TraceInfoNewContract | TraceInfoExistingContract; + if (subcallAddress === 'NEW_CONTRACT') { + const traceForThatSubcall = tracesByContractAddress[subcallAddress]; + traceInfo = { + subtrace: traceForThatSubcall, + txHash, + address: subcallAddress, + bytecode: data as string, + }; + } else { + const runtimeBytecode = await this._web3Wrapper.getContractCodeAsync(subcallAddress); + const traceForThatSubcall = tracesByContractAddress[subcallAddress]; + traceInfo = { + subtrace: traceForThatSubcall, + txHash, + address: subcallAddress, + runtimeBytecode, + }; + } + await this._handleTraceInfoAsync(traceInfo); + } + } else { + for (const subcallAddress of subcallAddresses) { + const runtimeBytecode = await this._web3Wrapper.getContractCodeAsync(subcallAddress); + const traceForThatSubcall = tracesByContractAddress[subcallAddress]; + const traceInfo: TraceInfoExistingContract = { + subtrace: traceForThatSubcall, + txHash, + address: subcallAddress, + runtimeBytecode, + }; + await this._handleTraceInfoAsync(traceInfo); + } + } + } +} -- cgit From ef61c3543fba2030515002ec418afc388e0e1033 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:38:21 -0700 Subject: Fix linter errors --- packages/sol-cov/src/revert_trace_subprovider.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages') diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts index f6501757a..b1d4da10c 100644 --- a/packages/sol-cov/src/revert_trace_subprovider.ts +++ b/packages/sol-cov/src/revert_trace_subprovider.ts @@ -37,6 +37,7 @@ export class RevertTraceSubprovider extends TraceCollectionSubprovider { this._logger = getLogger('sol-cov'); this._logger.setLevel(isVerbose ? levels.TRACE : levels.ERROR); } + // tslint:disable-next-line:no-unused-variable protected async _recordTxTraceAsync(address: string, data: string | undefined, txHash: string): Promise { await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0); const trace = await this._web3Wrapper.getTransactionTraceAsync(txHash, { -- cgit From d118533d876fdf290af5e0e5a08dd84ff001eee4 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:53:29 -0700 Subject: Remove redundant check in trace.ts and revert_trace.ts --- packages/sol-cov/src/revert_trace.ts | 3 --- packages/sol-cov/src/trace.ts | 4 +--- 2 files changed, 1 insertion(+), 6 deletions(-) (limited to 'packages') diff --git a/packages/sol-cov/src/revert_trace.ts b/packages/sol-cov/src/revert_trace.ts index 1d52b969e..a78d1afa8 100644 --- a/packages/sol-cov/src/revert_trace.ts +++ b/packages/sol-cov/src/revert_trace.ts @@ -30,9 +30,6 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E structLog.stack[structLog.stack.length - jumpAddressOffset - 1], ); - if (structLog === _.last(normalizedStructLogs)) { - throw new Error('Malformed trace. CALL-like opcode can not be the last one'); - } // Sometimes calls don't change the execution context (current address). When we do a transfer to an // externally owned account - it does the call and immediately returns because there is no fallback // function. We manually check if the call depth had changed to handle that case. diff --git a/packages/sol-cov/src/trace.ts b/packages/sol-cov/src/trace.ts index fad2e5e08..635019fc0 100644 --- a/packages/sol-cov/src/trace.ts +++ b/packages/sol-cov/src/trace.ts @@ -33,9 +33,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress const newAddress = utils.getAddressFromStackEntry( structLog.stack[structLog.stack.length - jumpAddressOffset - 1], ); - if (structLog === _.last(normalizedStructLogs)) { - throw new Error('Malformed trace. CALL-like opcode can not be the last one'); - } + // Sometimes calls don't change the execution context (current address). When we do a transfer to an // externally owned account - it does the call and immediately returns because there is no fallback // function. We manually check if the call depth had changed to handle that case. -- cgit From 7032825e35e825e42196d4ccc23b1a1f7fb8038a Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 14 Jun 2018 16:53:48 -0700 Subject: Change wording of error message when you try to use more than one subprovider --- packages/contracts/src/utils/web3_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages') diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index 67b71cdeb..c9d83a02d 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -54,7 +54,7 @@ const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace); const enabledSubproviderCount = _.filter([isCoverageEnabled, isProfilerEnabled, isRevertTraceEnabled], _.identity) .length; if (enabledSubproviderCount > 1) { - throw new Error(`Only one of coverage, profiler, and revert trace subproviders can be enabled at a time`); + throw new Error(`Only one of coverage, profiler, or revert trace subproviders can be enabled at a time`); } if (isCoverageEnabled) { const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); -- cgit