Merge branch 'development' of https://github.com/0xProject/0x-monorepo into development

11 files changed, 117 insertions, 209 deletions

diff --git a/packages/sol-tracing-utils/CHANGELOG.json b/packages/sol-tracing-utils/CHANGELOG.json
index b470d3e87..16a12ca63 100644
--- a/packages/sol-tracing-utils/CHANGELOG.json
+++ b/packages/sol-tracing-utils/CHANGELOG.json
@@ -1,5 +1,53 @@
 [
     {
+        "version": "6.0.0",
+        "changes": [
+            {
+                "note": "`SolCompilerArtifactAdapter` now uses `SolResolver` under the hood which allows to resolve `NPM` dependencies properly",
+                "pr": 1535
+            },
+            {
+                "note": "Cache the `utils.getContractDataIfExists` leading to faster execution",
+                "pr": 1535
+            },
+            {
+                "note": "`SolCompilerArtifactAdapter` now doesn't return the `ContractData` for interfaces",
+                "pr": 1535
+            },
+            {
+                "note": "Print resasonable error message on bytecode collision",
+                "pr": 1535
+            }
+        ]
+    },
+    {
+        "version": "5.0.0",
+        "changes": [
+            {
+                "note": "Upgrade the bignumber.js to v8.0.2",
+                "pr": 1517
+            }
+        ]
+    },
+    {
+        "version": "4.0.1",
+        "changes": [
+            {
+                "note": "Fix a bug where a custom `Geth` tracer didn't return stack entries for `DELEGATECALL`",
+                "pr": 1521
+            },
+            {
+                "note": "Fix a bug where `TraceCollectionSubprovider` was hanging on the fake `Geth` snapshot transaction",
+                "pr": 1521
+            },
+            {
+                "note": "Fix/simplify handling of revert trace snippets",
+                "pr": 1521
+            }
+        ],
+        "timestamp": 1547747677
+    },
+    {
         "version": "4.0.0",
         "changes": [
             {
diff --git a/packages/sol-tracing-utils/CHANGELOG.md b/packages/sol-tracing-utils/CHANGELOG.md
index 00d36844c..9bffb2a6d 100644
--- a/packages/sol-tracing-utils/CHANGELOG.md
+++ b/packages/sol-tracing-utils/CHANGELOG.md
@@ -5,6 +5,12 @@ Edit the package's CHANGELOG.json file only.
 
 CHANGELOG
 
+## v4.0.1 - _January 17, 2019_
+
+    * Fix a bug where a custom `Geth` tracer didn't return stack entries for `DELEGATECALL` (#1521)
+    * Fix a bug where `TraceCollectionSubprovider` was hanging on the fake `Geth` snapshot transaction (#1521)
+    * Fix/simplify handling of revert trace snippets (#1521)
+
 ## v4.0.0 - _January 15, 2019_
 
     * Fix a bug with incorrect parsing of `sourceMaps` due to sources being in an array instead of a map (#1498)
diff --git a/packages/sol-tracing-utils/package.json b/packages/sol-tracing-utils/package.json
index e3e52f42c..4b0fff222 100644
--- a/packages/sol-tracing-utils/package.json
+++ b/packages/sol-tracing-utils/package.json
@@ -1,6 +1,6 @@
 {
     "name": "@0x/sol-tracing-utils",
-    "version": "4.0.0",
+    "version": "4.0.1",
     "engines": {
         "node": ">=6.12"
     },
@@ -44,6 +44,7 @@
     "dependencies": {
         "@0x/dev-utils": "^1.0.24",
         "@0x/sol-compiler": "^2.0.2",
+        "@0x/sol-resolver": "^1.2.3",
         "@0x/subproviders": "^2.1.11",
         "@0x/typescript-typings": "^3.0.8",
         "@0x/utils": "^3.0.1",
diff --git a/packages/sol-tracing-utils/src/artifact_adapters/sol_compiler_artifact_adapter.ts b/packages/sol-tracing-utils/src/artifact_adapters/sol_compiler_artifact_adapter.ts
index d52587f2c..bfd3a504a 100644
--- a/packages/sol-tracing-utils/src/artifact_adapters/sol_compiler_artifact_adapter.ts
+++ b/packages/sol-tracing-utils/src/artifact_adapters/sol_compiler_artifact_adapter.ts
@@ -1,3 +1,4 @@
+import { FallthroughResolver, FSResolver, NPMResolver, RelativeFSResolver, URLResolver } from '@0x/sol-resolver';
 import { logUtils } from '@0x/utils';
 import { CompilerOptions, ContractArtifact } from 'ethereum-types';
 import * as fs from 'fs';
@@ -14,6 +15,7 @@ const CONFIG_FILE = 'compiler.json';
 export class SolCompilerArtifactAdapter extends AbstractArtifactAdapter {
     private readonly _artifactsPath: string;
     private readonly _sourcesPath: string;
+    private readonly _resolver: FallthroughResolver;
     /**
      * Instantiates a SolCompilerArtifactAdapter
      * @param artifactsPath Path to your artifacts directory
@@ -32,6 +34,12 @@ export class SolCompilerArtifactAdapter extends AbstractArtifactAdapter {
             throw new Error(`contractsDir not found in ${CONFIG_FILE}`);
         }
         this._sourcesPath = (sourcesPath || config.contractsDir) as string;
+        this._resolver = new FallthroughResolver();
+        this._resolver.appendResolver(new URLResolver());
+        const packagePath = path.resolve('');
+        this._resolver.appendResolver(new NPMResolver(packagePath));
+        this._resolver.appendResolver(new RelativeFSResolver(this._sourcesPath));
+        this._resolver.appendResolver(new FSResolver());
     }
     public async collectContractsDataAsync(): Promise<ContractData[]> {
         const artifactsGlob = `${this._artifactsPath}/**/*.json`;
@@ -46,10 +54,9 @@ export class SolCompilerArtifactAdapter extends AbstractArtifactAdapter {
             const sources: Sources = {};
             const sourceCodes: SourceCodes = {};
             _.map(artifact.sources, (value: { id: number }, relativeFilePath: string) => {
-                const filePath = path.resolve(this._sourcesPath, relativeFilePath);
-                const fileContent = fs.readFileSync(filePath).toString();
-                sources[value.id] = filePath;
-                sourceCodes[value.id] = fileContent;
+                const source = this._resolver.resolve(relativeFilePath);
+                sources[value.id] = source.absolutePath;
+                sourceCodes[value.id] = source.source;
             });
             const contractData = {
                 sourceCodes,
@@ -59,6 +66,10 @@ export class SolCompilerArtifactAdapter extends AbstractArtifactAdapter {
                 runtimeBytecode: artifact.compilerOutput.evm.deployedBytecode.object,
                 sourceMapRuntime: artifact.compilerOutput.evm.deployedBytecode.sourceMap,
             };
+            const isInterfaceContract = contractData.bytecode === '0x' && contractData.runtimeBytecode === '0x';
+            if (isInterfaceContract) {
+                continue;
+            }
             contractsData.push(contractData);
         }
         return contractsData;
diff --git a/packages/sol-tracing-utils/src/collect_coverage_entries.ts b/packages/sol-tracing-utils/src/collect_coverage_entries.ts
index 9e3591d74..d5045b106 100644
--- a/packages/sol-tracing-utils/src/collect_coverage_entries.ts
+++ b/packages/sol-tracing-utils/src/collect_coverage_entries.ts
@@ -5,18 +5,18 @@ import * as parser from 'solidity-parser-antlr';
 import { ASTVisitor, CoverageEntriesDescription } from './ast_visitor';
 import { getOffsetToLocation } from './source_maps';
 
-const IGNORE_RE = /\/\*\s*solcov\s+ignore\s+next\s*\*\/\s*/gm;
-
 // Parsing source code for each transaction/code is slow and therefore we cache it
 const sourceHashToCoverageEntries: { [sourceHash: string]: CoverageEntriesDescription } = {};
 
-export const collectCoverageEntries = (contractSource: string) => {
+export const collectCoverageEntries = (contractSource: string, ignoreRegexp?: RegExp) => {
     const sourceHash = ethUtil.sha3(contractSource).toString('hex');
     if (_.isUndefined(sourceHashToCoverageEntries[sourceHash]) && !_.isUndefined(contractSource)) {
         const ast = parser.parse(contractSource, { range: true });
         const offsetToLocation = getOffsetToLocation(contractSource);
-        const ignoreRangesBegingingAt = gatherRangesToIgnore(contractSource);
-        const visitor = new ASTVisitor(offsetToLocation, ignoreRangesBegingingAt);
+        const ignoreRangesBeginningAt = _.isUndefined(ignoreRegexp)
+            ? []
+            : gatherRangesToIgnore(contractSource, ignoreRegexp);
+        const visitor = new ASTVisitor(offsetToLocation, ignoreRangesBeginningAt);
         parser.visit(ast, visitor);
         sourceHashToCoverageEntries[sourceHash] = visitor.getCollectedCoverageEntries();
     }
@@ -25,12 +25,12 @@ export const collectCoverageEntries = (contractSource: string) => {
 };
 
 // Gather the start index of all code blocks preceeded by "/* solcov ignore next */"
-function gatherRangesToIgnore(contractSource: string): number[] {
+function gatherRangesToIgnore(contractSource: string, ignoreRegexp: RegExp): number[] {
     const ignoreRangesStart = [];
 
     let match;
     do {
-        match = IGNORE_RE.exec(contractSource);
+        match = ignoreRegexp.exec(contractSource);
         if (match) {
             const matchLen = match[0].length;
             ignoreRangesStart.push(match.index + matchLen);
diff --git a/packages/sol-tracing-utils/src/get_source_range_snippet.ts b/packages/sol-tracing-utils/src/get_source_range_snippet.ts
index 7aef00fee..c1f6e3a4e 100644
--- a/packages/sol-tracing-utils/src/get_source_range_snippet.ts
+++ b/packages/sol-tracing-utils/src/get_source_range_snippet.ts
@@ -1,185 +1,16 @@
-import * as ethUtil from 'ethereumjs-util';
-import * as _ from 'lodash';
-import * as Parser from 'solidity-parser-antlr';
-
-import { SingleFileSourceRange, SourceRange, SourceSnippet } from './types';
+import { SourceRange, SourceSnippet } from './types';
 import { utils } from './utils';
 
-interface ASTInfo {
-    type: string;
-    node: Parser.ASTNode;
-    name: string | null;
-    range?: SingleFileSourceRange;
-}
-
-// Parsing source code for each transaction/code is slow and therefore we cache it
-const hashToParsedSource: { [sourceHash: string]: Parser.ASTNode } = {};
-
 /**
  * Gets the source range snippet by source range to be used by revert trace.
  * @param sourceRange source range
  * @param sourceCode source code
  */
-export function getSourceRangeSnippet(sourceRange: SourceRange, sourceCode: string): SourceSnippet | null {
-    const sourceHash = ethUtil.sha3(sourceCode).toString('hex');
-    if (_.isUndefined(hashToParsedSource[sourceHash])) {
-        hashToParsedSource[sourceHash] = Parser.parse(sourceCode, { loc: true });
-    }
-    const astNode = hashToParsedSource[sourceHash];
-    const visitor = new ASTInfoVisitor();
-    Parser.visit(astNode, visitor);
-    const astInfo = visitor.getASTInfoForRange(sourceRange);
-    if (astInfo === null) {
-        return null;
-    }
+export function getSourceRangeSnippet(sourceRange: SourceRange, sourceCode: string): SourceSnippet {
     const sourceCodeInRange = utils.getRange(sourceCode, sourceRange.location);
     return {
-        ...astInfo,
-        range: astInfo.range as SingleFileSourceRange,
+        range: sourceRange.location,
         source: sourceCodeInRange,
         fileName: sourceRange.fileName,
     };
 }
-
-// A visitor which collects ASTInfo for most nodes in the AST.
-class ASTInfoVisitor {
-    private readonly _astInfos: ASTInfo[] = [];
-    public getASTInfoForRange(sourceRange: SourceRange): ASTInfo | null {
-        // HACK(albrow): Sometimes the source range doesn't exactly match that
-        // of astInfo. To work around that we try with a +/-1 offset on
-        // end.column. If nothing matches even with the offset, we return null.
-        const offset = {
-            start: {
-                line: 0,
-                column: 0,
-            },
-            end: {
-                line: 0,
-                column: 0,
-            },
-        };
-        let astInfo = this._getASTInfoForRange(sourceRange, offset);
-        if (astInfo !== null) {
-            return astInfo;
-        }
-        offset.end.column += 1;
-        astInfo = this._getASTInfoForRange(sourceRange, offset);
-        if (astInfo !== null) {
-            return astInfo;
-        }
-        offset.end.column -= 2;
-        astInfo = this._getASTInfoForRange(sourceRange, offset);
-        if (astInfo !== null) {
-            return astInfo;
-        }
-        return null;
-    }
-    public ContractDefinition(ast: Parser.ContractDefinition): void {
-        this._visitContractDefinition(ast);
-    }
-    public IfStatement(ast: Parser.IfStatement): void {
-        this._visitStatement(ast);
-    }
-    public FunctionDefinition(ast: Parser.FunctionDefinition): void {
-        this._visitFunctionLikeDefinition(ast);
-    }
-    public ModifierDefinition(ast: Parser.ModifierDefinition): void {
-        this._visitFunctionLikeDefinition(ast);
-    }
-    public ForStatement(ast: Parser.ForStatement): void {
-        this._visitStatement(ast);
-    }
-    public ReturnStatement(ast: Parser.ReturnStatement): void {
-        this._visitStatement(ast);
-    }
-    public BreakStatement(ast: Parser.BreakStatement): void {
-        this._visitStatement(ast);
-    }
-    public ContinueStatement(ast: Parser.ContinueStatement): void {
-        this._visitStatement(ast);
-    }
-    public EmitStatement(ast: any /* TODO: Parser.EmitStatement */): void {
-        this._visitStatement(ast);
-    }
-    public VariableDeclarationStatement(ast: Parser.VariableDeclarationStatement): void {
-        this._visitStatement(ast);
-    }
-    public Statement(ast: Parser.Statement): void {
-        this._visitStatement(ast);
-    }
-    public WhileStatement(ast: Parser.WhileStatement): void {
-        this._visitStatement(ast);
-    }
-    public SimpleStatement(ast: Parser.SimpleStatement): void {
-        this._visitStatement(ast);
-    }
-    public ThrowStatement(ast: Parser.ThrowStatement): void {
-        this._visitStatement(ast);
-    }
-    public DoWhileStatement(ast: Parser.DoWhileStatement): void {
-        this._visitStatement(ast);
-    }
-    public ExpressionStatement(ast: Parser.ExpressionStatement): void {
-        this._visitStatement(ast.expression);
-    }
-    public InlineAssemblyStatement(ast: Parser.InlineAssemblyStatement): void {
-        this._visitStatement(ast);
-    }
-    public ModifierInvocation(ast: Parser.ModifierInvocation): void {
-        const BUILTIN_MODIFIERS = ['public', 'view', 'payable', 'external', 'internal', 'pure', 'constant'];
-        if (!_.includes(BUILTIN_MODIFIERS, ast.name)) {
-            this._visitStatement(ast);
-        }
-    }
-    private _visitStatement(ast: Parser.ASTNode): void {
-        this._astInfos.push({
-            type: ast.type,
-            node: ast,
-            name: null,
-            range: ast.loc,
-        });
-    }
-    private _visitFunctionLikeDefinition(ast: Parser.ModifierDefinition | Parser.FunctionDefinition): void {
-        this._astInfos.push({
-            type: ast.type,
-            node: ast,
-            name: ast.name,
-            range: ast.loc,
-        });
-    }
-    private _visitContractDefinition(ast: Parser.ContractDefinition): void {
-        this._astInfos.push({
-            type: ast.type,
-            node: ast,
-            name: ast.name,
-            range: ast.loc,
-        });
-    }
-    private _getASTInfoForRange(sourceRange: SourceRange, offset: SingleFileSourceRange): ASTInfo | null {
-        const offsetSourceRange = {
-            ...sourceRange,
-            location: {
-                start: {
-                    line: sourceRange.location.start.line + offset.start.line,
-                    column: sourceRange.location.start.column + offset.start.column,
-                },
-                end: {
-                    line: sourceRange.location.end.line + offset.end.line,
-                    column: sourceRange.location.end.column + offset.end.column,
-                },
-            },
-        };
-        for (const astInfo of this._astInfos) {
-            const astInfoRange = astInfo.range as SingleFileSourceRange;
-            if (
-                astInfoRange.start.column === offsetSourceRange.location.start.column &&
-                astInfoRange.start.line === offsetSourceRange.location.start.line &&
-                astInfoRange.end.column === offsetSourceRange.location.end.column &&
-                astInfoRange.end.line === offsetSourceRange.location.end.line
-            ) {
-                return astInfo;
-            }
-        }
-        return null;
-    }
-}
diff --git a/packages/sol-tracing-utils/src/trace_collection_subprovider.ts b/packages/sol-tracing-utils/src/trace_collection_subprovider.ts
index 323e1523c..5118921fa 100644
--- a/packages/sol-tracing-utils/src/trace_collection_subprovider.ts
+++ b/packages/sol-tracing-utils/src/trace_collection_subprovider.ts
@@ -144,7 +144,7 @@ export abstract class TraceCollectionSubprovider extends Subprovider {
         txHash: string | undefined,
         cb: Callback,
     ): Promise<void> {
-        if (!txData.isFakeTransaction) {
+        if (!(txData.isFakeTransaction || txData.from === txData.to)) {
             // 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();
diff --git a/packages/sol-tracing-utils/src/trace_info_subprovider.ts b/packages/sol-tracing-utils/src/trace_info_subprovider.ts
index b75fc7bf7..de42e1862 100644
--- a/packages/sol-tracing-utils/src/trace_info_subprovider.ts
+++ b/packages/sol-tracing-utils/src/trace_info_subprovider.ts
@@ -31,7 +31,7 @@ export abstract class TraceInfoSubprovider extends TraceCollectionSubprovider {
                         const depth = 0 | log.getDepth();
                         const gasCost = 0 | log.getCost();
                         const gas = 0 | log.getGas();
-                        const isCall = opn == 0xf1 || opn == 0xf2 || opn == 0xf4 || opn == 0xf5;
+                        const isCall = opn == 0xf1 || opn == 0xf2 || opn == 0xf4 || opn == 0xf5 || opn == 0xfa;
                         const stack = isCall ? ['0x'+log.stack.peek(1).toString(16), null] : null;
                         this.data.push({ pc, gasCost, depth, op, stack, gas });
                     },
diff --git a/packages/sol-tracing-utils/src/types.ts b/packages/sol-tracing-utils/src/types.ts
index 27568ae03..97b5e6b37 100644
--- a/packages/sol-tracing-utils/src/types.ts
+++ b/packages/sol-tracing-utils/src/types.ts
@@ -1,5 +1,4 @@
 import { StructLog } from 'ethereum-types';
-import * as Parser from 'solidity-parser-antlr';
 
 export interface LineColumn {
     line: number;
@@ -126,8 +125,5 @@ export type EvmCallStack = EvmCallStackEntry[];
 export interface SourceSnippet {
     source: string;
     fileName: string;
-    type: string;
-    node: Parser.ASTNode;
-    name: string | null;
     range: SingleFileSourceRange;
 }
diff --git a/packages/sol-tracing-utils/src/utils.ts b/packages/sol-tracing-utils/src/utils.ts
index 644321f32..7dc1844a5 100644
--- a/packages/sol-tracing-utils/src/utils.ts
+++ b/packages/sol-tracing-utils/src/utils.ts
@@ -1,13 +1,13 @@
-import { addressUtils, BigNumber } from '@0x/utils';
+import { addressUtils, BigNumber, logUtils } from '@0x/utils';
 import { OpCode, StructLog } from 'ethereum-types';
 import { addHexPrefix } from 'ethereumjs-util';
 import * as _ from 'lodash';
 
 import { ContractData, LineColumn, SingleFileSourceRange } from './types';
 
-// This is the minimum length of valid contract bytecode. The Solidity compiler
-// metadata is 86 bytes. If you add the '0x' prefix, we get 88.
-const MIN_CONTRACT_BYTECODE_LENGTH = 88;
+const STATICCALL_GAS_COST = 40;
+
+const bytecodeToContractDataIfExists: { [bytecode: string]: ContractData | undefined } = {};
 
 export const utils = {
     compareLineColumn(lhs: LineColumn, rhs: LineColumn): number {
@@ -46,22 +46,29 @@ export const utils = {
         if (!bytecode.startsWith('0x')) {
             throw new Error(`0x hex prefix missing: ${bytecode}`);
         }
-        const contractData = _.find(contractsData, contractDataCandidate => {
+        // HACK(leo): We want to cache the values that are possibly undefined.
+        // That's why we can't check for undefined as we usually do, but need to use `hasOwnProperty`.
+        if (bytecodeToContractDataIfExists.hasOwnProperty(bytecode)) {
+            return bytecodeToContractDataIfExists[bytecode];
+        }
+        const contractDataCandidates = _.filter(contractsData, contractDataCandidate => {
             const bytecodeRegex = utils.bytecodeToBytecodeRegex(contractDataCandidate.bytecode);
-            // If the bytecode is less than the minimum length, we are probably
-            // dealing with an interface. This isn't what we're looking for.
-            if (bytecodeRegex.length < MIN_CONTRACT_BYTECODE_LENGTH) {
-                return false;
-            }
             const runtimeBytecodeRegex = utils.bytecodeToBytecodeRegex(contractDataCandidate.runtimeBytecode);
-            if (runtimeBytecodeRegex.length < MIN_CONTRACT_BYTECODE_LENGTH) {
-                return false;
-            }
             // We use that function to find by bytecode or runtimeBytecode. Those are quasi-random strings so
             // collisions are practically impossible and it allows us to reuse that code
             return !_.isNull(bytecode.match(bytecodeRegex)) || !_.isNull(bytecode.match(runtimeBytecodeRegex));
         });
-        return contractData;
+        if (contractDataCandidates.length > 1) {
+            const candidates = contractDataCandidates.map(
+                contractDataCandidate => _.values(contractDataCandidate.sources)[0],
+            );
+            const errMsg =
+                "We've found more than one artifact that contains the exact same bytecode and therefore are unable to detect which contract was executed. " +
+                "We'll be assigning all traces to the first one.";
+            logUtils.warn(errMsg);
+            logUtils.warn(candidates);
+        }
+        return (bytecodeToContractDataIfExists[bytecode] = contractDataCandidates[0]);
     },
     isCallLike(op: OpCode): boolean {
         return _.includes([OpCode.CallCode, OpCode.StaticCall, OpCode.Call, OpCode.DelegateCall], op);
@@ -76,10 +83,17 @@ export const utils = {
     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,
-            }));
+            const newStructLogs = _.map(structLogs, structLog => {
+                const newStructLog = {
+                    ...structLog,
+                    depth: structLog.depth - 1,
+                };
+                if (newStructLog.op === 'STATICCALL') {
+                    // HACK(leo): Geth traces sometimes returns those gas costs incorrectly as very big numbers so we manually fix them.
+                    newStructLog.gasCost = STATICCALL_GAS_COST;
+                }
+                return newStructLog;
+            });
             return newStructLogs;
         }
         return structLogs;
diff --git a/packages/sol-tracing-utils/test/collect_coverage_entries_test.ts b/packages/sol-tracing-utils/test/collect_coverage_entries_test.ts
index 7832ec316..d3ca8930c 100644
--- a/packages/sol-tracing-utils/test/collect_coverage_entries_test.ts
+++ b/packages/sol-tracing-utils/test/collect_coverage_entries_test.ts
@@ -130,7 +130,8 @@ describe('Collect coverage entries', () => {
                 solcovIgnoreContractBaseName,
             );
             const solcovIgnoreContract = fs.readFileSync(solcovIgnoreContractFileName).toString();
-            const coverageEntries = collectCoverageEntries(solcovIgnoreContract);
+            const IGNORE_REGEXP = /\/\*\s*solcov\s+ignore\s+next\s*\*\/\s*/gm;
+            const coverageEntries = collectCoverageEntries(solcovIgnoreContract, IGNORE_REGEXP);
             const fnIds = _.keys(coverageEntries.fnMap);
 
             expect(fnIds.length).to.be.equal(1);