Merge pull request #5429 from NomicFoundation/refactor/vm-tracer-cleanup

Clean up the VMTracer and trace events interface in TS

packages/hardhat-core/src/internal/hardhat-network/provider/provider.ts

@@ -12,12 +12,9 @@ import type {
 import type {
   EdrContext,
   Provider as EdrProviderT,
-  ExecutionResult,
   RawTrace,
   Response,
   SubscriptionEvent,
-  TracingMessage,
-  TracingStep,
 } from "@nomicfoundation/edr";
 import { Common } from "@nomicfoundation/ethereumjs-common";
 import chalk from "chalk";
@@ -152,12 +149,6 @@ export function getNodeConfig(
   };
 }
 
-export interface RawTraceCallbacks {
-  onStep?: (messageTrace: TracingStep) => Promise<void>;
-  onBeforeMessage?: (messageTrace: TracingMessage) => Promise<void>;
-  onAfterMessage?: (messageTrace: ExecutionResult) => Promise<void>;
-}
-
 class EdrProviderEventAdapter extends EventEmitter {}
 
 type CallOverrideCallback = (
@@ -176,6 +167,9 @@ export class EdrProviderWrapper
   // temporarily added to make smock work with HH+EDR
   private _callOverrideCallback?: CallOverrideCallback;
 
+  /** Used for internal stack trace tests. */
+  private _vmTracer?: VMTracer;
+
   private constructor(
     private readonly _provider: EdrProviderT,
     // we add this for backwards-compatibility with plugins like solidity-coverage
@@ -184,7 +178,6 @@ export class EdrProviderWrapper
     },
     private readonly _eventAdapter: EdrProviderEventAdapter,
     private readonly _vmTraceDecoder: VmTraceDecoder,
-    private readonly _rawTraceCallbacks: RawTraceCallbacks,
     // The common configuration for EthereumJS VM is not used by EDR, but tests expect it as part of the provider.
     private readonly _common: Common,
     tracingConfig?: TracingConfig
@@ -199,7 +192,6 @@ export class EdrProviderWrapper
   public static async create(
     config: HardhatNetworkProviderConfig,
     loggerConfig: LoggerConfig,
-    rawTraceCallbacks: RawTraceCallbacks,
     tracingConfig?: TracingConfig
   ): Promise<EdrProviderWrapper> {
     const { Provider } = requireNapiRsModule(
@@ -325,7 +317,6 @@ export class EdrProviderWrapper
       minimalEthereumJsNode,
       eventAdapter,
       vmTraceDecoder,
-      rawTraceCallbacks,
       common,
       tracingConfig
     );
@@ -371,9 +362,7 @@ export class EdrProviderWrapper
     const needsTraces =
       this._node._vm.evm.events.eventNames().length > 0 ||
       this._node._vm.events.eventNames().length > 0 ||
-      this._rawTraceCallbacks.onStep !== undefined ||
-      this._rawTraceCallbacks.onAfterMessage !== undefined ||
-      this._rawTraceCallbacks.onBeforeMessage !== undefined;
+      this._vmTracer !== undefined;
 
     if (needsTraces) {
       const rawTraces = responseObject.traces;
@@ -394,9 +383,8 @@ export class EdrProviderWrapper
                 edrTracingStepToMinimalInterpreterStep(traceItem)
               );
             }
-            if (this._rawTraceCallbacks.onStep !== undefined) {
-              await this._rawTraceCallbacks.onStep(traceItem);
-            }
+
+            this._vmTracer?.addStep(traceItem);
           }
           // afterMessage event
           else if ("executionResult" in traceItem) {
@@ -406,11 +394,8 @@ export class EdrProviderWrapper
                 edrTracingMessageResultToMinimalEVMResult(traceItem)
               );
             }
-            if (this._rawTraceCallbacks.onAfterMessage !== undefined) {
-              await this._rawTraceCallbacks.onAfterMessage(
-                traceItem.executionResult
-              );
-            }
+
+            this._vmTracer?.addAfterMessage(traceItem.executionResult);
           }
           // beforeMessage event
           else {
@@ -420,9 +405,8 @@ export class EdrProviderWrapper
                 edrTracingMessageToMinimalMessage(traceItem)
               );
             }
-            if (this._rawTraceCallbacks.onBeforeMessage !== undefined) {
-              await this._rawTraceCallbacks.onBeforeMessage(traceItem);
-            }
+
+            this._vmTracer?.addBeforeMessage(traceItem);
           }
         }
 
@@ -484,6 +468,15 @@ export class EdrProviderWrapper
     }
   }
 
+  /**
+   * Sets a `VMTracer` that observes EVM throughout requests.
+   *
+   * Used for internal stack traces integration tests.
+   */
+  public setVmTracer(vmTracer?: VMTracer) {
+    this._vmTracer = vmTracer;
+  }
+
   // temporarily added to make smock work with HH+EDR
   private _setCallOverrideCallback(callback: CallOverrideCallback) {
     this._callOverrideCallback = callback;
@@ -583,16 +576,16 @@ export class EdrProviderWrapper
   private async _rawTraceToSolidityStackTrace(
     rawTrace: RawTrace
   ): Promise<SolidityStackTrace | undefined> {
-    const vmTracer = new VMTracer(false);
+    const vmTracer = new VMTracer();
 
     const trace = rawTrace.trace();
     for (const traceItem of trace) {
       if ("pc" in traceItem) {
-        await vmTracer.addStep(traceItem);
+        vmTracer.addStep(traceItem);
       } else if ("executionResult" in traceItem) {
-        await vmTracer.addAfterMessage(traceItem.executionResult);
+        vmTracer.addAfterMessage(traceItem.executionResult);
       } else {
-        await vmTracer.addBeforeMessage(traceItem);
+        vmTracer.addBeforeMessage(traceItem);
       }
     }
 
@@ -634,7 +627,6 @@ export async function createHardhatNetworkProvider(
   return EdrProviderWrapper.create(
     hardhatNetworkProviderConfig,
     loggerConfig,
-    {},
     await makeTracingConfig(artifacts)
   );
 }

packages/hardhat-core/src/internal/hardhat-network/provider/vm/exit.ts

@@ -91,25 +91,4 @@ export class Exit {
 
     const _exhaustiveCheck: never = this.kind;
   }
-
-  public getEdrExceptionalHalt(): ExceptionalHalt {
-    const { ExceptionalHalt } = requireNapiRsModule(
-      "@nomicfoundation/edr"
-    ) as typeof import("@nomicfoundation/edr");
-
-    switch (this.kind) {
-      case ExitCode.OUT_OF_GAS:
-        return ExceptionalHalt.OutOfGas;
-      case ExitCode.INVALID_OPCODE:
-        return ExceptionalHalt.OpcodeNotFound;
-      case ExitCode.CODESIZE_EXCEEDS_MAXIMUM:
-        return ExceptionalHalt.CreateContractSizeLimit;
-      case ExitCode.CREATE_COLLISION:
-        return ExceptionalHalt.CreateCollision;
-
-      default:
-        // eslint-disable-next-line @nomicfoundation/hardhat-internal-rules/only-hardhat-error
-        throw new Error(`Unmatched exit code: ${this.kind}`);
-    }
-  }
 }

packages/hardhat-core/src/internal/hardhat-network/stack-traces/message-trace.ts

@@ -34,8 +34,6 @@ export interface PrecompileMessageTrace extends BaseMessageTrace {
 
 export interface BaseEvmMessageTrace extends BaseMessageTrace {
   code: Uint8Array;
-  value: bigint;
-  returnData: Uint8Array;
   steps: MessageTraceStep[];
   bytecode?: Bytecode;
   // The following is just an optimization: When processing this traces it's useful to know ahead of

packages/hardhat-core/src/internal/hardhat-network/stack-traces/vm-tracer.ts

@@ -26,14 +26,18 @@ import {
 const DUMMY_RETURN_DATA = Buffer.from([]);
 const DUMMY_GAS_USED = 0n;
 
+/**
+ * Consumes the incoming VM trace events, until an error occurs, to keep track
+ * of the last top level message trace/error.
+ */
 export class VMTracer {
   public tracingSteps: TracingStep[] = [];
 
   private _messageTraces: MessageTrace[] = [];
   private _lastError: Error | undefined;
   private _maxPrecompileNumber;
 
-  constructor(private readonly _throwErrors = true) {
+  constructor() {
     // TODO: temporarily hardcoded to remove the need of using ethereumjs' common and evm here
     this._maxPrecompileNumber = 10;
   }
@@ -46,15 +50,11 @@ export class VMTracer {
     return this._lastError;
   }
 
-  public clearLastError() {
-    this._lastError = undefined;
-  }
-
   private _shouldKeepTracing() {
-    return this._throwErrors || this._lastError === undefined;
+    return this._lastError === undefined;
   }
 
-  public async addBeforeMessage(message: TracingMessage) {
+  public addBeforeMessage(message: TracingMessage) {
     if (!this._shouldKeepTracing()) {
       return;
     }
@@ -143,15 +143,11 @@ export class VMTracer {
 
       this._messageTraces.push(trace);
     } catch (error) {
-      if (this._throwErrors) {
-        throw error;
-      } else {
-        this._lastError = error as Error;
-      }
+      this._lastError = error as Error;
     }
   }
 
-  public async addStep(step: TracingStep) {
+  public addStep(step: TracingStep) {
     if (!this._shouldKeepTracing()) {
       return;
     }
@@ -169,15 +165,11 @@ export class VMTracer {
 
       trace.steps.push({ pc: Number(step.pc) });
     } catch (error) {
-      if (this._throwErrors) {
-        throw error;
-      } else {
-        this._lastError = error as Error;
-      }
+      this._lastError = error as Error;
     }
   }
 
-  public async addAfterMessage(result: ExecutionResult, haltOverride?: Exit) {
+  public addAfterMessage(result: ExecutionResult) {
     if (!this._shouldKeepTracing()) {
       return;
     }
@@ -197,8 +189,7 @@ export class VMTracer {
           ).address;
         }
       } else if (isHaltResult(executionResult)) {
-        trace.exit =
-          haltOverride ?? Exit.fromEdrExceptionalHalt(executionResult.reason);
+        trace.exit = Exit.fromEdrExceptionalHalt(executionResult.reason);
 
         trace.returnData = Buffer.from([]);
       } else {
@@ -211,11 +202,7 @@ export class VMTracer {
         this._messageTraces.pop();
       }
     } catch (error) {
-      if (this._throwErrors) {
-        throw error;
-      } else {
-        this._lastError = error as Error;
-      }
+      this._lastError = error as Error;
     }
   }
 }

packages/hardhat-core/test/internal/hardhat-network/stack-traces/execution.ts

@@ -29,7 +29,7 @@ const senderAddress = bytesToHex(privateToAddress(toBuffer(senderPrivateKey)));
 export async function instantiateProvider(
   loggerConfig: LoggerConfig,
   tracingConfig: TracingConfig
-): Promise<[EdrProviderWrapper, VMTracer]> {
+): Promise<EdrProviderWrapper> {
   const config = {
     hardfork: "shanghai",
     chainId: 1,
@@ -55,26 +55,13 @@ export async function instantiateProvider(
     enableTransientStorage: false,
   };
 
-  const vmTracer = new VMTracer(false);
-
   const provider = await EdrProviderWrapper.create(
     config,
     loggerConfig,
-    {
-      onStep: async (step) => {
-        await vmTracer.addStep(step);
-      },
-      onAfterMessage: async (message) => {
-        await vmTracer.addAfterMessage(message);
-      },
-      onBeforeMessage: async (message) => {
-        await vmTracer.addBeforeMessage(message);
-      },
-    },
     tracingConfig
   );
 
-  return [provider, vmTracer];
+  return provider;
 }
 
 export function encodeConstructorParams(
@@ -116,9 +103,11 @@ export interface TxData {
 
 export async function traceTransaction(
   provider: EdrProviderWrapper,
-  vmTracer: VMTracer,
   txData: TxData
 ): Promise<MessageTrace> {
+  const vmTracer = new VMTracer();
+  provider.setVmTracer(vmTracer);
+
   try {
     await provider.request({
       method: "eth_sendTransaction",
@@ -142,6 +131,6 @@ export async function traceTransaction(
     }
     return trace;
   } finally {
-    vmTracer.clearLastError();
+    provider.setVmTracer(undefined);
   }
 }