From cdac2d210e2a43c35cbcc2f046010e972640c89c Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 17 Jan 2019 14:23:03 +0100 Subject: Revert devnet mining period from 1 to 0 --- packages/devnet/genesis.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devnet/genesis.json b/packages/devnet/genesis.json index 03dc5d623..073672dd9 100644 --- a/packages/devnet/genesis.json +++ b/packages/devnet/genesis.json @@ -8,7 +8,7 @@ "eip158Block": 3, "byzantiumBlock": 4, "clique": { - "period": 1, + "period": 0, "epoch": 30000 } }, -- cgit v1.2.3 From 83f77a2d561780c97790fe44a3d34a3c7e847273 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 17 Jan 2019 14:27:39 +0100 Subject: Fix a bug when a custom Geth tracer didn't return stack entries for DELEGATECALL --- packages/sol-tracing-utils/CHANGELOG.json | 9 +++++++++ packages/sol-tracing-utils/src/trace_info_subprovider.ts | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/sol-tracing-utils/CHANGELOG.json b/packages/sol-tracing-utils/CHANGELOG.json index b470d3e87..9ee298e22 100644 --- a/packages/sol-tracing-utils/CHANGELOG.json +++ b/packages/sol-tracing-utils/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "4.0.1", + "changes": [ + { + "note": "Fix a bug when a custom Geth tracer didn't return stack entries for `DELEGATECALL`", + "pr": "TODO" + } + ] + }, { "version": "4.0.0", "changes": [ 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 }); }, -- cgit v1.2.3 From 4c5bde1b544f66d70957824658ad37230735d527 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 17 Jan 2019 14:36:12 +0100 Subject: Fix a bug when TraceCollectionSubprovider was hanging on the fake Geth snapshot transaction --- packages/sol-tracing-utils/CHANGELOG.json | 6 +++++- packages/sol-tracing-utils/src/trace_collection_subprovider.ts | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/sol-tracing-utils/CHANGELOG.json b/packages/sol-tracing-utils/CHANGELOG.json index 9ee298e22..17f9466bb 100644 --- a/packages/sol-tracing-utils/CHANGELOG.json +++ b/packages/sol-tracing-utils/CHANGELOG.json @@ -3,7 +3,11 @@ "version": "4.0.1", "changes": [ { - "note": "Fix a bug when a custom Geth tracer didn't return stack entries for `DELEGATECALL`", + "note": "Fix a bug when a custom `Geth` tracer didn't return stack entries for `DELEGATECALL`", + "pr": "TODO" + }, + { + "note": "Fix a bug when `TraceCollectionSubprovider` was hanging on the fake `Geth` snapshot transaction", "pr": "TODO" } ] diff --git a/packages/sol-tracing-utils/src/trace_collection_subprovider.ts b/packages/sol-tracing-utils/src/trace_collection_subprovider.ts index 323e1523c..d34707a13 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 { - 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(); -- cgit v1.2.3 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 From 69aa1c2e91bb7497f5b9f346e15973197082b4bd Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 17 Jan 2019 14:40:11 +0100 Subject: Add PR numbers --- packages/sol-tracing-utils/CHANGELOG.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/sol-tracing-utils/CHANGELOG.json b/packages/sol-tracing-utils/CHANGELOG.json index d04678755..938d33eb8 100644 --- a/packages/sol-tracing-utils/CHANGELOG.json +++ b/packages/sol-tracing-utils/CHANGELOG.json @@ -4,15 +4,15 @@ "changes": [ { "note": "Fix a bug when a custom `Geth` tracer didn't return stack entries for `DELEGATECALL`", - "pr": "TODO" + "pr": 1521 }, { "note": "Fix a bug when `TraceCollectionSubprovider` was hanging on the fake `Geth` snapshot transaction", - "pr": "TODO" + "pr": 1521 }, { "note": "Fix/simplify handling of revert trace snippets", - "pr": "TODO" + "pr": 1521 } ] }, -- cgit v1.2.3 From edd4370cdb25a2feb5b9671b1fcf351c3ca9df73 Mon Sep 17 00:00:00 2001 From: Fabio B Date: Thu, 17 Jan 2019 15:06:29 +0100 Subject: Update packages/sol-tracing-utils/CHANGELOG.json Co-Authored-By: LogvinovLeon --- packages/sol-tracing-utils/CHANGELOG.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sol-tracing-utils/CHANGELOG.json b/packages/sol-tracing-utils/CHANGELOG.json index 938d33eb8..66c065018 100644 --- a/packages/sol-tracing-utils/CHANGELOG.json +++ b/packages/sol-tracing-utils/CHANGELOG.json @@ -3,7 +3,7 @@ "version": "4.0.1", "changes": [ { - "note": "Fix a bug when a custom `Geth` tracer didn't return stack entries for `DELEGATECALL`", + "note": "Fix a bug where a custom `Geth` tracer didn't return stack entries for `DELEGATECALL`", "pr": 1521 }, { -- cgit v1.2.3 From 8528660f5021ffc1d42c21b7e09687f5036a1416 Mon Sep 17 00:00:00 2001 From: Fabio B Date: Thu, 17 Jan 2019 15:06:35 +0100 Subject: Update packages/sol-tracing-utils/CHANGELOG.json Co-Authored-By: LogvinovLeon --- packages/sol-tracing-utils/CHANGELOG.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sol-tracing-utils/CHANGELOG.json b/packages/sol-tracing-utils/CHANGELOG.json index 66c065018..4a855d031 100644 --- a/packages/sol-tracing-utils/CHANGELOG.json +++ b/packages/sol-tracing-utils/CHANGELOG.json @@ -7,7 +7,7 @@ "pr": 1521 }, { - "note": "Fix a bug when `TraceCollectionSubprovider` was hanging on the fake `Geth` snapshot transaction", + "note": "Fix a bug where `TraceCollectionSubprovider` was hanging on the fake `Geth` snapshot transaction", "pr": 1521 }, { -- cgit v1.2.3 From 92ec4f57722eb738637431e3038dc589ca3708de Mon Sep 17 00:00:00 2001 From: Fabio B Date: Thu, 17 Jan 2019 15:06:42 +0100 Subject: Update packages/sol-tracing-utils/src/trace_collection_subprovider.ts Co-Authored-By: LogvinovLeon --- packages/sol-tracing-utils/src/trace_collection_subprovider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sol-tracing-utils/src/trace_collection_subprovider.ts b/packages/sol-tracing-utils/src/trace_collection_subprovider.ts index d34707a13..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 { - if (!txData.isFakeTransaction && !(txData.from === txData.to)) { + 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(); -- cgit v1.2.3 From 8b69d918a968237571729725d3238529c0b7a7b2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 17 Jan 2019 15:38:09 +0100 Subject: Fix linter --- packages/sol-tracing-utils/src/types.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/sol-tracing-utils/src/types.ts b/packages/sol-tracing-utils/src/types.ts index 3cab6f7f1..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; -- cgit v1.2.3