From fcdd0de9eea6b06449aaa9ac71dadfcd4faf4968 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 17 Jan 2019 14:37:15 +0100 Subject: Fix/simplify handling of revert trace snippets --- packages/sol-trace/src/revert_trace_subprovider.ts | 17 +- packages/sol-tracing-utils/CHANGELOG.json | 4 + .../src/get_source_range_snippet.ts | 175 +-------------------- packages/sol-tracing-utils/src/types.ts | 3 - packages/sol-tracing-utils/src/utils.ts | 16 +- 5 files changed, 22 insertions(+), 193 deletions(-) diff --git a/packages/sol-trace/src/revert_trace_subprovider.ts b/packages/sol-trace/src/revert_trace_subprovider.ts index fa065cfcb..046dad812 100644 --- a/packages/sol-trace/src/revert_trace_subprovider.ts +++ b/packages/sol-trace/src/revert_trace_subprovider.ts @@ -109,9 +109,7 @@ export class RevertTraceSubprovider extends TraceCollectionSubprovider { const fileNameToFileIndex = _.invert(contractData.sources); const fileIndex = _.parseInt(fileNameToFileIndex[sourceRange.fileName]); const sourceSnippet = getSourceRangeSnippet(sourceRange, contractData.sourceCodes[fileIndex]); - if (sourceSnippet !== null) { - sourceSnippets.push(sourceSnippet); - } + sourceSnippets.push(sourceSnippet); } const filteredSnippets = filterSnippets(sourceSnippets); if (filteredSnippets.length > 0) { @@ -135,9 +133,7 @@ function filterSnippets(sourceSnippets: SourceSnippet[]): SourceSnippet[] { const results: SourceSnippet[] = [sourceSnippets[0]]; let prev = sourceSnippets[0]; for (const sourceSnippet of sourceSnippets) { - if (sourceSnippet.type === 'IfStatement') { - continue; - } else if (sourceSnippet.source === prev.source) { + if (sourceSnippet.source === prev.source) { prev = sourceSnippet; continue; } @@ -157,12 +153,5 @@ function getStackTraceString(sourceSnippet: SourceSnippet): string { } function getSourceSnippetString(sourceSnippet: SourceSnippet): string { - switch (sourceSnippet.type) { - case 'ContractDefinition': - return `contract ${sourceSnippet.name}`; - case 'FunctionDefinition': - return `function ${sourceSnippet.name}`; - default: - return `${sourceSnippet.source}`; - } + return `${sourceSnippet.source}`; } diff --git a/packages/sol-tracing-utils/CHANGELOG.json b/packages/sol-tracing-utils/CHANGELOG.json index 17f9466bb..d04678755 100644 --- a/packages/sol-tracing-utils/CHANGELOG.json +++ b/packages/sol-tracing-utils/CHANGELOG.json @@ -9,6 +9,10 @@ { "note": "Fix a bug when `TraceCollectionSubprovider` was hanging on the fake `Geth` snapshot transaction", "pr": "TODO" + }, + { + "note": "Fix/simplify handling of revert trace snippets", + "pr": "TODO" } ] }, 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/types.ts b/packages/sol-tracing-utils/src/types.ts index 27568ae03..3cab6f7f1 100644 --- a/packages/sol-tracing-utils/src/types.ts +++ b/packages/sol-tracing-utils/src/types.ts @@ -126,8 +126,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..89c158ee7 100644 --- a/packages/sol-tracing-utils/src/utils.ts +++ b/packages/sol-tracing-utils/src/utils.ts @@ -8,6 +8,7 @@ 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; export const utils = { compareLineColumn(lhs: LineColumn, rhs: LineColumn): number { @@ -76,10 +77,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; -- cgit v1.2.3