From 0680887de20141b4eb65b3297eb3356e524e007a Mon Sep 17 00:00:00 2001 From: Wodann Date: Thu, 19 Oct 2023 20:04:00 -0500 Subject: [PATCH] fix: static state change error (#4499) Co-authored-by: Franco Victorio --- .../hardhat-network/provider/miner/edr.ts | 2 +- .../provider/utils/convertToEdr.ts | 40 ++++++++++++++----- .../provider/vm/block-builder/edr.ts | 2 +- .../hardhat-network/provider/vm/edr.ts | 10 ++--- .../hardhat-network/provider/vm/ethereumjs.ts | 17 ++++++-- .../hardhat-network/provider/vm/exit.ts | 17 ++++++-- .../hardhat-network/stack-traces/vm-tracer.ts | 21 ++++------ 7 files changed, 71 insertions(+), 38 deletions(-) diff --git a/packages/hardhat-core/src/internal/hardhat-network/provider/miner/edr.ts b/packages/hardhat-core/src/internal/hardhat-network/provider/miner/edr.ts index 3b9d2f32ed..fc47097e68 100644 --- a/packages/hardhat-core/src/internal/hardhat-network/provider/miner/edr.ts +++ b/packages/hardhat-core/src/internal/hardhat-network/provider/miner/edr.ts @@ -111,7 +111,7 @@ export class EdrMiner implements BlockMinerAdapter { }); } } else if ("executionResult" in traceItem) { - await vmTracer.addAfterMessage(traceItem); + await vmTracer.addAfterMessage(traceItem.executionResult); } else { await vmTracer.addBeforeMessage(traceItem); } diff --git a/packages/hardhat-core/src/internal/hardhat-network/provider/utils/convertToEdr.ts b/packages/hardhat-core/src/internal/hardhat-network/provider/utils/convertToEdr.ts index 8e29a7f1d4..d4c3194bf0 100644 --- a/packages/hardhat-core/src/internal/hardhat-network/provider/utils/convertToEdr.ts +++ b/packages/hardhat-core/src/internal/hardhat-network/provider/utils/convertToEdr.ts @@ -543,7 +543,8 @@ export function edrResultToEthereumjsEvmResult( } export function ethereumjsEvmResultToEdrResult( - result: EVMResult + result: EVMResult, + overrideExceptionalHalt: boolean = false ): ExecutionResult { const gasUsed = result.execResult.executionGasUsed; @@ -589,16 +590,35 @@ export function ethereumjsEvmResultToEdrResult( }, }; } else { - const vmError = Exit.fromEthereumJSEvmError( - result.execResult.exceptionError - ); - - return { - result: { - reason: vmError.getEdrExceptionalHalt(), + if (overrideExceptionalHalt) { + const overridenResult: any = { gasUsed, - }, - }; + }; + + // Throw an error if reason is accessed + Object.defineProperty(overridenResult, "reason", { + get: () => { + throw new Error( + "Cannot access reason of an exceptional halt in EthereumJS mode" + ); + }, + }); + + return { + result: overridenResult, + }; + } else { + const vmError = Exit.fromEthereumJSEvmError( + result.execResult.exceptionError + ); + + return { + result: { + reason: vmError.getEdrExceptionalHalt(), + gasUsed, + }, + }; + } } } diff --git a/packages/hardhat-core/src/internal/hardhat-network/provider/vm/block-builder/edr.ts b/packages/hardhat-core/src/internal/hardhat-network/provider/vm/block-builder/edr.ts index ae9a9ed6b2..ae70f077c2 100644 --- a/packages/hardhat-core/src/internal/hardhat-network/provider/vm/block-builder/edr.ts +++ b/packages/hardhat-core/src/internal/hardhat-network/provider/vm/block-builder/edr.ts @@ -88,7 +88,7 @@ export class EdrBlockBuilder implements BlockBuilderAdapter { if ("pc" in traceItem) { await this._vmTracer.addStep(traceItem); } else if ("executionResult" in traceItem) { - await this._vmTracer.addAfterMessage(traceItem); + await this._vmTracer.addAfterMessage(traceItem.executionResult); } else { await this._vmTracer.addBeforeMessage(traceItem); } diff --git a/packages/hardhat-core/src/internal/hardhat-network/provider/vm/edr.ts b/packages/hardhat-core/src/internal/hardhat-network/provider/vm/edr.ts index d351d8ca36..ddcc5bd5cf 100644 --- a/packages/hardhat-core/src/internal/hardhat-network/provider/vm/edr.ts +++ b/packages/hardhat-core/src/internal/hardhat-network/provider/vm/edr.ts @@ -256,13 +256,11 @@ export class EdrAdapter implements VMAdapter { const trace = edrResult.trace!; for (const traceItem of trace) { if ("pc" in traceItem) { - // TODO: these "as any" shouldn't be necessary, we had - // to add them after merging the changes in main - await this._vmTracer.addStep(traceItem as any); + await this._vmTracer.addStep(traceItem); } else if ("executionResult" in traceItem) { - await this._vmTracer.addAfterMessage(traceItem as any); + await this._vmTracer.addAfterMessage(traceItem.executionResult); } else { - await this._vmTracer.addBeforeMessage(traceItem as any); + await this._vmTracer.addBeforeMessage(traceItem); } } @@ -504,7 +502,7 @@ export class EdrAdapter implements VMAdapter { if ("pc" in traceItem) { await this._vmTracer.addStep(traceItem); } else if ("executionResult" in traceItem) { - await this._vmTracer.addAfterMessage(traceItem); + await this._vmTracer.addAfterMessage(traceItem.executionResult); } else { await this._vmTracer.addBeforeMessage(traceItem); } diff --git a/packages/hardhat-core/src/internal/hardhat-network/provider/vm/ethereumjs.ts b/packages/hardhat-core/src/internal/hardhat-network/provider/vm/ethereumjs.ts index a146338714..4053cf6aeb 100644 --- a/packages/hardhat-core/src/internal/hardhat-network/provider/vm/ethereumjs.ts +++ b/packages/hardhat-core/src/internal/hardhat-network/provider/vm/ethereumjs.ts @@ -6,6 +6,7 @@ import { InterpreterStep, Message, } from "@nomicfoundation/ethereumjs-evm"; +import { ERROR } from "@nomicfoundation/ethereumjs-evm/dist/exceptions"; import { DefaultStateManager, StateManager, @@ -767,11 +768,21 @@ export class EthereumJSAdapter implements VMAdapter { next: any ): Promise { try { - const executionResult = ethereumjsEvmResultToEdrResult(result); + const hasExceptionalHalt = + result.execResult.exceptionError !== undefined && + result.execResult.exceptionError.error !== ERROR.REVERT; - await this._vmTracer.addAfterMessage({ + const executionResult = ethereumjsEvmResultToEdrResult( + result, + hasExceptionalHalt + ); + + await this._vmTracer.addAfterMessage( executionResult, - }); + hasExceptionalHalt + ? Exit.fromEthereumJSEvmError(result.execResult.exceptionError) + : undefined + ); for (const listener of this._afterMessageListeners) { await listener(result); diff --git a/packages/hardhat-core/src/internal/hardhat-network/provider/vm/exit.ts b/packages/hardhat-core/src/internal/hardhat-network/provider/vm/exit.ts index ba1f405b5a..d9487bba98 100644 --- a/packages/hardhat-core/src/internal/hardhat-network/provider/vm/exit.ts +++ b/packages/hardhat-core/src/internal/hardhat-network/provider/vm/exit.ts @@ -11,6 +11,7 @@ export enum ExitCode { STACK_UNDERFLOW, CODESIZE_EXCEEDS_MAXIMUM, CREATE_COLLISION, + STATIC_STATE_CHANGE, } export class Exit { @@ -20,8 +21,9 @@ export class Exit { case SuccessReason.Return: case SuccessReason.SelfDestruct: return new Exit(ExitCode.SUCCESS); - // TODO: Should we throw an error if default is hit? } + + const _exhaustiveCheck: never = reason; } public static fromEdrExceptionalHalt(halt: ExceptionalHalt): Exit { @@ -45,9 +47,8 @@ export class Exit { return new Exit(ExitCode.CODESIZE_EXCEEDS_MAXIMUM); default: { - // TODO temporary, should be removed in production // eslint-disable-next-line @nomicfoundation/hardhat-internal-rules/only-hardhat-error - throw new Error(`Unmatched edr exceptional halt: ${halt}`); + throw new Error(`Unmatched EDR exceptional halt: ${halt}`); } } } @@ -85,6 +86,10 @@ export class Exit { return new Exit(ExitCode.CREATE_COLLISION); } + if (evmError.error === ERROR.STATIC_STATE_CHANGE) { + return new Exit(ExitCode.STATIC_STATE_CHANGE); + } + // TODO temporary, should be removed in production // eslint-disable-next-line @nomicfoundation/hardhat-internal-rules/only-hardhat-error throw new Error(`Unmatched evm error: ${evmError.error}`); @@ -114,6 +119,8 @@ export class Exit { return "Codesize exceeds maximum"; case ExitCode.CREATE_COLLISION: return "Create collision"; + case ExitCode.STATIC_STATE_CHANGE: + return "Static state change"; } const _exhaustiveCheck: never = this.kind; @@ -137,6 +144,8 @@ export class Exit { return new EvmError(ERROR.CODESIZE_EXCEEDS_MAXIMUM); case ExitCode.CREATE_COLLISION: return new EvmError(ERROR.CREATE_COLLISION); + case ExitCode.STATIC_STATE_CHANGE: + return new EvmError(ERROR.STATIC_STATE_CHANGE); } const _exhaustiveCheck: never = this.kind; @@ -155,7 +164,7 @@ export class Exit { default: // eslint-disable-next-line @nomicfoundation/hardhat-internal-rules/only-hardhat-error - throw new Error(`Unmatched edr exceptional halt: ${this.kind}`); + throw new Error(`Unmatched exit code: ${this.kind}`); } } } diff --git a/packages/hardhat-core/src/internal/hardhat-network/stack-traces/vm-tracer.ts b/packages/hardhat-core/src/internal/hardhat-network/stack-traces/vm-tracer.ts index 0b68f4534d..12a542ea3c 100644 --- a/packages/hardhat-core/src/internal/hardhat-network/stack-traces/vm-tracer.ts +++ b/packages/hardhat-core/src/internal/hardhat-network/stack-traces/vm-tracer.ts @@ -1,8 +1,8 @@ import type { Common } from "@nomicfoundation/ethereumjs-common"; import { CreateOutput, + ExecutionResult, TracingMessage, - TracingMessageResult, TracingStep, } from "@ignored/edr"; @@ -29,9 +29,7 @@ const DUMMY_RETURN_DATA = Buffer.from([]); const DUMMY_GAS_USED = 0n; export class VMTracer { - public tracingMessages: TracingMessage[] = []; public tracingSteps: TracingStep[] = []; - public tracingMessageResults: TracingMessageResult[] = []; private _messageTraces: MessageTrace[] = []; private _lastError: Error | undefined; @@ -67,13 +65,9 @@ export class VMTracer { if (message.depth === 0) { this._messageTraces = []; - this.tracingMessages = []; this.tracingSteps = []; - this.tracingMessageResults = []; } - this.tracingMessages.push(message); - if (message.to === undefined) { const createTrace: CreateMessageTrace = { code: message.data, @@ -184,18 +178,16 @@ export class VMTracer { } } - public async addAfterMessage(result: TracingMessageResult) { + public async addAfterMessage(result: ExecutionResult, haltOverride?: Exit) { if (!this._shouldKeepTracing()) { return; } - this.tracingMessageResults.push(result); - try { const trace = this._messageTraces[this._messageTraces.length - 1]; - trace.gasUsed = result.executionResult.result.gasUsed; + trace.gasUsed = result.result.gasUsed; - const executionResult = result.executionResult.result; + const executionResult = result.result; if (isSuccessResult(executionResult)) { trace.exit = Exit.fromEdrSuccessReason(executionResult.reason); trace.returnData = executionResult.output.returnValue; @@ -206,10 +198,13 @@ export class VMTracer { ).address; } } else if (isHaltResult(executionResult)) { - trace.exit = Exit.fromEdrExceptionalHalt(executionResult.reason); + trace.exit = + haltOverride ?? Exit.fromEdrExceptionalHalt(executionResult.reason); + trace.returnData = Buffer.from([]); } else { trace.exit = new Exit(ExitCode.REVERT); + trace.returnData = executionResult.output; }