Merge pull request #1498 from 0xProject/fix/sol-cov

Sol tracing fixes

packages/devnet/genesis.json

@@ -8,7 +8,7 @@
         "eip158Block": 3,
         "byzantiumBlock": 4,
         "clique": {
-            "period": 0,
+            "period": 1,
             "epoch": 30000
         }
     },

packages/ethereum-types/src/index.ts

@@ -306,6 +306,8 @@ export interface TraceParams {
     disableMemory?: boolean;
     disableStack?: boolean;
     disableStorage?: boolean;
+    tracer?: string;
+    timeout?: string;
 }
 
 export type OutputField =

packages/sol-coverage/src/coverage_subprovider.ts

@@ -74,19 +74,19 @@ export const coverageHandler: SingleFileSubtraceHandler = (
 
     let sourceRanges = _.map(subtrace, structLog => pcToSourceRange[structLog.pc]);
     sourceRanges = _.compact(sourceRanges); // Some PC's don't map to a source range and we just ignore them.
-    // By default lodash does a shallow object comparasion. We JSON.stringify them and compare as strings.
+    // By default lodash does a shallow object comparison. We JSON.stringify them and compare as strings.
     sourceRanges = _.uniqBy(sourceRanges, s => JSON.stringify(s)); // We don't care if one PC was covered multiple times within a single transaction
     sourceRanges = _.filter(sourceRanges, sourceRange => sourceRange.fileName === absoluteFileName);
     const branchCoverage: BranchCoverage = {};
     const branchIds = _.keys(coverageEntriesDescription.branchMap);
     for (const branchId of branchIds) {
         const branchDescription = coverageEntriesDescription.branchMap[branchId];
-        const isBranchCoveredByBranchIndex = _.map(branchDescription.locations, location => {
+        const branchIndexToIsBranchCovered = _.map(branchDescription.locations, location => {
             const isBranchCovered = _.some(sourceRanges, range => utils.isRangeInside(range.location, location));
             const timesBranchCovered = Number(isBranchCovered);
             return timesBranchCovered;
         });
-        branchCoverage[branchId] = isBranchCoveredByBranchIndex;
+        branchCoverage[branchId] = branchIndexToIsBranchCovered;
     }
     const statementCoverage: StatementCoverage = {};
     const statementIds = _.keys(coverageEntriesDescription.statementMap);

packages/sol-coverage/src/index.ts

@@ -4,6 +4,8 @@ export {
     TruffleArtifactAdapter,
     AbstractArtifactAdapter,
     ContractData,
+    SourceCodes,
+    Sources,
 } from '@0x/sol-tracing-utils';
 
 export {

packages/sol-profiler/src/index.ts

@@ -3,6 +3,8 @@ export {
     SolCompilerArtifactAdapter,
     TruffleArtifactAdapter,
     ContractData,
+    SourceCodes,
+    Sources,
 } from '@0x/sol-tracing-utils';
 
 // HACK: ProfilerSubprovider is a hacky way to do profiling using coverage tools. Not production ready

packages/sol-profiler/src/profiler_subprovider.ts

@@ -63,7 +63,7 @@ export const profilerHandler: SingleFileSubtraceHandler = (
 ): Coverage => {
     const absoluteFileName = contractData.sources[fileIndex];
     const profilerEntriesDescription = collectCoverageEntries(contractData.sourceCodes[fileIndex]);
-    const gasConsumedByStatement: { [statementId: string]: number } = {};
+    const statementToGasConsumed: { [statementId: string]: number } = {};
     const statementIds = _.keys(profilerEntriesDescription.statementMap);
     for (const statementId of statementIds) {
         const statementDescription = profilerEntriesDescription.statementMap[statementId];
@@ -83,14 +83,14 @@ export const profilerHandler: SingleFileSubtraceHandler = (
                 }
             }),
         );
-        gasConsumedByStatement[statementId] = totalGasCost;
+        statementToGasConsumed[statementId] = totalGasCost;
     }
     const partialProfilerOutput = {
         [absoluteFileName]: {
             ...profilerEntriesDescription,
             path: absoluteFileName,
             f: {}, // I's meaningless in profiling context
-            s: gasConsumedByStatement,
+            s: statementToGasConsumed,
             b: {}, // I's meaningless in profiling context
         },
     };

packages/sol-trace/src/index.ts

@@ -3,6 +3,8 @@ export {
     TruffleArtifactAdapter,
     SolCompilerArtifactAdapter,
     ContractData,
+    SourceCodes,
+    Sources,
 } from '@0x/sol-tracing-utils';
 
 export { RevertTraceSubprovider } from './revert_trace_subprovider';

packages/sol-trace/src/revert_trace_subprovider.ts

@@ -106,7 +106,8 @@ export class RevertTraceSubprovider extends TraceCollectionSubprovider {
                 continue;
             }
 
-            const fileIndex = contractData.sources.indexOf(sourceRange.fileName);
+            const fileNameToFileIndex = _.invert(contractData.sources);
+            const fileIndex = _.parseInt(fileNameToFileIndex[sourceRange.fileName]);
             const sourceSnippet = getSourceRangeSnippet(sourceRange, contractData.sourceCodes[fileIndex]);
             if (sourceSnippet !== null) {
                 sourceSnippets.push(sourceSnippet);

packages/sol-tracing-utils/CHANGELOG.json

@@ -1,4 +1,29 @@
 [
+    {
+        "version": "4.0.0",
+        "changes": [
+            {
+                "note": "Fix a bug with incorrect parsing of `sourceMaps` due to sources being in an array instead of a map",
+                "pr": 1498
+            },
+            {
+                "note": "Change the types of `ContractData.sources` and `ContractData.sourceCodes` to be objects instead of arrays",
+                "pr": 1498
+            },
+            {
+                "note": "Use custom JS tracer to speed up tracing on clients that support it (e.g., Geth)",
+                "pr": 1498
+            },
+            {
+                "note": "Log errors encountered in `TraceCollectionSubprovider`",
+                "pr": 1498
+            },
+            {
+                "note": "Add support for assembly statements",
+                "pr": 1498
+            }
+        ]
+    },
     {
         "version": "3.0.0",
         "changes": [

packages/sol-tracing-utils/src/artifact_adapters/sol_compiler_artifact_adapter.ts

@@ -5,7 +5,7 @@ import * as glob from 'glob';
 import * as _ from 'lodash';
 import * as path from 'path';
 
-import { ContractData } from '../types';
+import { ContractData, SourceCodes, Sources } from '../types';
 
 import { AbstractArtifactAdapter } from './abstract_artifact_adapter';
 
@@ -43,9 +43,14 @@ export class SolCompilerArtifactAdapter extends AbstractArtifactAdapter {
                 logUtils.warn(`${artifactFileName} doesn't contain bytecode. Skipping...`);
                 continue;
             }
-            let sources = _.keys(artifact.sources);
-            sources = _.map(sources, relativeFilePath => path.resolve(this._sourcesPath, relativeFilePath));
-            const sourceCodes = _.map(sources, (source: string) => fs.readFileSync(source).toString());
+            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 contractData = {
                 sourceCodes,
                 sources,

packages/sol-tracing-utils/src/ast_visitor.ts

@@ -1,7 +1,7 @@
 import * as _ from 'lodash';
 import * as Parser from 'solidity-parser-antlr';
 
-import { BranchMap, FnMap, LocationByOffset, SingleFileSourceRange, StatementMap } from './types';
+import { BranchMap, FnMap, OffsetToLocation, SingleFileSourceRange, StatementMap } from './types';
 
 export interface CoverageEntriesDescription {
     fnMap: FnMap;
@@ -22,13 +22,13 @@ export class ASTVisitor {
     private readonly _branchMap: BranchMap = {};
     private readonly _modifiersStatementIds: number[] = [];
     private readonly _statementMap: StatementMap = {};
-    private readonly _locationByOffset: LocationByOffset;
+    private readonly _offsetToLocation: OffsetToLocation;
     private readonly _ignoreRangesBeginningAt: number[];
     // keep track of contract/function ranges that are to be ignored
     // so we can also ignore any children nodes within the contract/function
     private readonly _ignoreRangesWithin: Array<[number, number]> = [];
-    constructor(locationByOffset: LocationByOffset, ignoreRangesBeginningAt: number[] = []) {
-        this._locationByOffset = locationByOffset;
+    constructor(offsetToLocation: OffsetToLocation, ignoreRangesBeginningAt: number[] = []) {
+        this._offsetToLocation = offsetToLocation;
         this._ignoreRangesBeginningAt = ignoreRangesBeginningAt;
     }
     public getCollectedCoverageEntries(): CoverageEntriesDescription {
@@ -94,6 +94,39 @@ export class ASTVisitor {
     public InlineAssemblyStatement(ast: Parser.InlineAssemblyStatement): void {
         this._visitStatement(ast);
     }
+    public AssemblyLocalDefinition(ast: Parser.AssemblyLocalDefinition): void {
+        this._visitStatement(ast);
+    }
+    public AssemblyCall(ast: Parser.AssemblyCall): void {
+        this._visitStatement(ast);
+    }
+    public AssemblyIf(ast: Parser.AssemblyIf): void {
+        this._visitStatement(ast);
+    }
+    public AssemblyBlock(ast: Parser.AssemblyBlock): void {
+        this._visitStatement(ast);
+    }
+    public AssemblyExpression(ast: Parser.AssemblyExpression): void {
+        this._visitStatement(ast);
+    }
+    public AssemblyAssignment(ast: Parser.AssemblyAssignment): void {
+        this._visitStatement(ast);
+    }
+    public LabelDefinition(ast: Parser.LabelDefinition): void {
+        this._visitStatement(ast);
+    }
+    public AssemblySwitch(ast: Parser.AssemblySwitch): void {
+        this._visitStatement(ast);
+    }
+    public AssemblyFunctionDefinition(ast: Parser.AssemblyFunctionDefinition): void {
+        this._visitStatement(ast);
+    }
+    public AssemblyFor(ast: Parser.AssemblyFor): void {
+        this._visitStatement(ast);
+    }
+    public SubAssembly(ast: Parser.SubAssembly): void {
+        this._visitStatement(ast);
+    }
     public BinaryOperation(ast: Parser.BinaryOperation): void {
         const BRANCHING_BIN_OPS = ['&&', '||'];
         if (_.includes(BRANCHING_BIN_OPS, ast.operator)) {
@@ -136,8 +169,8 @@ export class ASTVisitor {
     }
     private _getExpressionRange(ast: Parser.ASTNode): SingleFileSourceRange {
         const astRange = ast.range as [number, number];
-        const start = this._locationByOffset[astRange[0]];
-        const end = this._locationByOffset[astRange[1] + 1];
+        const start = this._offsetToLocation[astRange[0]];
+        const end = this._offsetToLocation[astRange[1] + 1];
         const range = {
             start,
             end,

packages/sol-tracing-utils/src/collect_coverage_entries.ts

@@ -3,24 +3,24 @@ import * as _ from 'lodash';
 import * as parser from 'solidity-parser-antlr';
 
 import { ASTVisitor, CoverageEntriesDescription } from './ast_visitor';
-import { getLocationByOffset } from './source_maps';
+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 coverageEntriesBySourceHash: { [sourceHash: string]: CoverageEntriesDescription } = {};
+const sourceHashToCoverageEntries: { [sourceHash: string]: CoverageEntriesDescription } = {};
 
 export const collectCoverageEntries = (contractSource: string) => {
     const sourceHash = ethUtil.sha3(contractSource).toString('hex');
-    if (_.isUndefined(coverageEntriesBySourceHash[sourceHash]) && !_.isUndefined(contractSource)) {
+    if (_.isUndefined(sourceHashToCoverageEntries[sourceHash]) && !_.isUndefined(contractSource)) {
         const ast = parser.parse(contractSource, { range: true });
-        const locationByOffset = getLocationByOffset(contractSource);
+        const offsetToLocation = getOffsetToLocation(contractSource);
         const ignoreRangesBegingingAt = gatherRangesToIgnore(contractSource);
-        const visitor = new ASTVisitor(locationByOffset, ignoreRangesBegingingAt);
+        const visitor = new ASTVisitor(offsetToLocation, ignoreRangesBegingingAt);
         parser.visit(ast, visitor);
-        coverageEntriesBySourceHash[sourceHash] = visitor.getCollectedCoverageEntries();
+        sourceHashToCoverageEntries[sourceHash] = visitor.getCollectedCoverageEntries();
     }
-    const coverageEntriesDescription = coverageEntriesBySourceHash[sourceHash];
+    const coverageEntriesDescription = sourceHashToCoverageEntries[sourceHash];
     return coverageEntriesDescription;
 };
 

packages/sol-tracing-utils/src/get_source_range_snippet.ts

@@ -13,7 +13,7 @@ interface ASTInfo {
 }
 
 // Parsing source code for each transaction/code is slow and therefore we cache it
-const parsedSourceByHash: { [sourceHash: string]: Parser.ASTNode } = {};
+const hashToParsedSource: { [sourceHash: string]: Parser.ASTNode } = {};
 
 /**
  * Gets the source range snippet by source range to be used by revert trace.
@@ -22,10 +22,10 @@ const parsedSourceByHash: { [sourceHash: string]: Parser.ASTNode } = {};
  */
 export function getSourceRangeSnippet(sourceRange: SourceRange, sourceCode: string): SourceSnippet | null {
     const sourceHash = ethUtil.sha3(sourceCode).toString('hex');
-    if (_.isUndefined(parsedSourceByHash[sourceHash])) {
-        parsedSourceByHash[sourceHash] = Parser.parse(sourceCode, { loc: true });
+    if (_.isUndefined(hashToParsedSource[sourceHash])) {
+        hashToParsedSource[sourceHash] = Parser.parse(sourceCode, { loc: true });
     }
-    const astNode = parsedSourceByHash[sourceHash];
+    const astNode = hashToParsedSource[sourceHash];
     const visitor = new ASTInfoVisitor();
     Parser.visit(astNode, visitor);
     const astInfo = visitor.getASTInfoForRange(sourceRange);

packages/sol-tracing-utils/src/index.ts

@@ -22,11 +22,13 @@ export {
     BranchMap,
     EvmCallStackEntry,
     FnMap,
-    LocationByOffset,
+    OffsetToLocation,
     StatementMap,
     TraceInfoBase,
     TraceInfoExistingContract,
     TraceInfoNewContract,
+    Sources,
+    SourceCodes,
 } from './types';
 export { collectCoverageEntries } from './collect_coverage_entries';
 export { TraceCollector, SingleFileSubtraceHandler } from './trace_collector';

packages/sol-tracing-utils/src/source_maps.ts

@@ -1,7 +1,7 @@
 import * as _ from 'lodash';
 
 import { getPcToInstructionIndexMapping } from './instructions';
-import { LocationByOffset, SourceRange } from './types';
+import { OffsetToLocation, SourceCodes, SourceRange, Sources } from './types';
 
 const RADIX = 10;
 
@@ -15,38 +15,41 @@ export interface SourceLocation {
  * Receives a string with newlines and returns a map of byte offset to LineColumn
  * @param str A string to process
  */
-export function getLocationByOffset(str: string): LocationByOffset {
-    const locationByOffset: LocationByOffset = { 0: { line: 1, column: 0 } };
+export function getOffsetToLocation(str: string): OffsetToLocation {
+    const offsetToLocation: OffsetToLocation = { 0: { line: 1, column: 0 } };
     let currentOffset = 0;
     for (const char of str.split('')) {
-        const location = locationByOffset[currentOffset];
+        const location = offsetToLocation[currentOffset];
         const isNewline = char === '\n';
-        locationByOffset[currentOffset + 1] = {
+        offsetToLocation[currentOffset + 1] = {
             line: location.line + (isNewline ? 1 : 0),
             column: isNewline ? 0 : location.column + 1,
         };
         currentOffset++;
     }
-    return locationByOffset;
+    return offsetToLocation;
 }
 
 /**
  * Parses a sourcemap string.
  * The solidity sourcemap format is documented here: https://github.com/ethereum/solidity/blob/develop/docs/miscellaneous.rst#source-mappings
- * @param sourceCodes sources contents
+ * @param indexToSourceCode index to source code
  * @param srcMap source map string
  * @param bytecodeHex contract bytecode
- * @param sources sources file names
+ * @param indexToSource index to source file path
  */
 export function parseSourceMap(
-    sourceCodes: string[],
+    sourceCodes: SourceCodes,
     srcMap: string,
     bytecodeHex: string,
-    sources: string[],
+    sources: Sources,
 ): { [programCounter: number]: SourceRange } {
     const bytecode = Uint8Array.from(Buffer.from(bytecodeHex, 'hex'));
     const pcToInstructionIndex: { [programCounter: number]: number } = getPcToInstructionIndexMapping(bytecode);
-    const locationByOffsetByFileIndex = _.map(sourceCodes, s => (_.isUndefined(s) ? {} : getLocationByOffset(s)));
+    const fileIndexToOffsetToLocation: { [fileIndex: number]: OffsetToLocation } = {};
+    _.map(sourceCodes, (sourceCode: string, fileIndex: number) => {
+        fileIndexToOffsetToLocation[fileIndex] = _.isUndefined(sourceCode) ? {} : getOffsetToLocation(sourceCode);
+    });
     const entries = srcMap.split(';');
     let lastParsedEntry: SourceLocation = {} as any;
     const instructionIndexToSourceRange: { [instructionIndex: number]: SourceRange } = {};
@@ -66,14 +69,18 @@ export function parseSourceMap(
             length,
             fileIndex,
         };
-        if (parsedEntry.fileIndex !== -1 && !_.isUndefined(locationByOffsetByFileIndex[parsedEntry.fileIndex])) {
+        if (parsedEntry.fileIndex !== -1 && !_.isUndefined(fileIndexToOffsetToLocation[parsedEntry.fileIndex])) {
+            const offsetToLocation = fileIndexToOffsetToLocation[parsedEntry.fileIndex];
             const sourceRange = {
                 location: {
-                    start: locationByOffsetByFileIndex[parsedEntry.fileIndex][parsedEntry.offset],
-                    end: locationByOffsetByFileIndex[parsedEntry.fileIndex][parsedEntry.offset + parsedEntry.length],
+                    start: offsetToLocation[parsedEntry.offset],
+                    end: offsetToLocation[parsedEntry.offset + parsedEntry.length],
                 },
                 fileName: sources[parsedEntry.fileIndex],
             };
+            if (sourceRange.location.start === undefined || sourceRange.location.end === undefined) {
+                throw new Error(`Error while processing sourcemap: location out of range in ${sourceRange.fileName}`);
+            }
             instructionIndexToSourceRange[i] = sourceRange;
         } else {
             // Some assembly code generated by Solidity can't be mapped back to a line of source code.