fix: prevent crash when returning large JSON responses

.changeset/two-starfishes-obey.md

@@ -0,0 +1,5 @@
+---
+"@nomicfoundation/edr": patch
+---
+
+Fixed crash when returning large JSON responses

crates/edr_napi/index.d.ts

@@ -418,7 +418,8 @@ export class Provider {
   setVerboseTracing(verboseTracing: boolean): void
 }
 export class Response {
-  get json(): string
+  /** Returns the response data as a JSON string or a JSON object. */
+  get data(): string | any
   get solidityTrace(): RawTrace | null
   get traces(): Array<RawTrace>
 }

crates/edr_napi/package.json

@@ -38,6 +38,7 @@
     "@types/node": "^18.0.0",
     "chai": "^4.3.6",
     "chai-as-promised": "^7.1.1",
+    "json-stream-stringify": "^3.1.4",
     "mocha": "^10.0.0",
     "ts-node": "^10.8.0",
     "typescript": "~4.5.2"
@@ -55,8 +56,8 @@
     "universal": "napi universal",
     "version": "napi version",
     "pretest": "pnpm build",
-    "test": "pnpm tsc && mocha --recursive \"test/**/*.ts\"",
-    "testNoBuild": "pnpm tsc && mocha --recursive \"test/**/*.ts\"",
+    "test": "pnpm tsc && node --max-old-space-size=8192 node_modules/mocha/bin/_mocha --recursive \"test/**/*.ts\"",
+    "testNoBuild": "pnpm tsc && node --max-old-space-size=8192 node_modules/mocha/bin/_mocha --recursive \"test/**/*.ts\"",
     "clean": "rm -rf @nomicfoundation/edr.node"
   }
 }

crates/edr_napi/src/provider.rs

@@ -4,7 +4,7 @@ use std::sync::Arc;
 
 use edr_provider::{time::CurrentTime, InvalidRequestReason};
 use edr_rpc_eth::jsonrpc;
-use napi::{tokio::runtime, Env, JsFunction, JsObject, Status};
+use napi::{tokio::runtime, Either, Env, JsFunction, JsObject, Status};
 use napi_derive::napi;
 
 use self::config::ProviderConfig;
@@ -120,9 +120,9 @@ impl Provider {
                             format!("Invalid JSON `{json_request}` due to: {error}"),
                         )
                     })
-                    .map(|json_response| Response {
+                    .map(|json| Response {
                         solidity_trace: None,
-                        json: json_response,
+                        data: Either::A(json),
                         traces: Vec::new(),
                     });
             }
@@ -168,10 +168,25 @@ impl Provider {
         let response = jsonrpc::ResponseData::from(response.map(|response| response.result));
 
         serde_json::to_string(&response)
-            .map_err(|e| napi::Error::new(Status::GenericFailure, e.to_string()))
-            .map(|json_response| Response {
+            .and_then(|json| {
+                // We experimentally determined that 500_000_000 was the maximum string length
+                // that can be returned without causing the error:
+                //
+                // > Failed to convert rust `String` into napi `string`
+                //
+                // To be safe, we're limiting string lengths to half of that.
+                const MAX_STRING_LENGTH: usize = 250_000_000;
+
+                if json.len() <= MAX_STRING_LENGTH {
+                    Ok(Either::A(json))
+                } else {
+                    serde_json::to_value(response).map(Either::B)
+                }
+            })
+            .map_err(|error| napi::Error::new(Status::GenericFailure, error.to_string()))
+            .map(|data| Response {
                 solidity_trace,
-                json: json_response,
+                data,
                 traces: traces.into_iter().map(Arc::new).collect(),
             })
     }
@@ -209,7 +224,10 @@ impl Provider {
 
 #[napi]
 pub struct Response {
-    json: String,
+    // N-API is known to be slow when marshalling `serde_json::Value`s, so we try to return a
+    // `String`. If the object is too large to be represented as a `String`, we return a `Buffer`
+    // instead.
+    data: Either<String, serde_json::Value>,
     /// When a transaction fails to execute, the provider returns a trace of the
     /// transaction.
     solidity_trace: Option<Arc<edr_evm::trace::Trace>>,
@@ -219,9 +237,10 @@ pub struct Response {
 
 #[napi]
 impl Response {
+    /// Returns the response data as a JSON string or a JSON object.
     #[napi(getter)]
-    pub fn json(&self) -> String {
-        self.json.clone()
+    pub fn data(&self) -> Either<String, serde_json::Value> {
+        self.data.clone()
     }
 
     #[napi(getter)]

crates/edr_napi/test/helpers.ts

@@ -1,5 +1,22 @@
 import { TracingMessage, TracingMessageResult, TracingStep } from "..";
 
+function getEnv(key: string): string | undefined {
+  const variable = process.env[key];
+  if (variable === undefined || variable === "") {
+    return undefined;
+  }
+
+  const trimmed = variable.trim();
+
+  return trimmed.length === 0 ? undefined : trimmed;
+}
+
+export const ALCHEMY_URL = getEnv("ALCHEMY_URL");
+
+export function isCI(): boolean {
+  return getEnv("CI") === "true";
+}
+
 /**
  * Given a trace, return only its steps.
  */

crates/edr_napi/test/issues.ts

@@ -0,0 +1,109 @@
+import chai, { assert } from "chai";
+import { JsonStreamStringify } from "json-stream-stringify";
+
+import {
+  ContractAndFunctionName,
+  EdrContext,
+  MineOrdering,
+  Provider,
+  SpecId,
+  SubscriptionEvent,
+} from "..";
+import { ALCHEMY_URL, isCI } from "./helpers";
+
+describe("Provider", () => {
+  const context = new EdrContext();
+  const providerConfig = {
+    allowBlocksWithSameTimestamp: false,
+    allowUnlimitedContractSize: true,
+    bailOnCallFailure: false,
+    bailOnTransactionFailure: false,
+    blockGasLimit: 300_000_000n,
+    chainId: 1n,
+    chains: [],
+    coinbase: Buffer.from("0000000000000000000000000000000000000000", "hex"),
+    enableRip7212: false,
+    genesisAccounts: [
+      {
+        secretKey:
+          "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80",
+        balance: 1000n * 10n ** 18n,
+      },
+    ],
+    hardfork: SpecId.Latest,
+    initialBlobGas: {
+      gasUsed: 0n,
+      excessGas: 0n,
+    },
+    initialParentBeaconBlockRoot: Buffer.from(
+      "0000000000000000000000000000000000000000000000000000000000000000",
+      "hex",
+    ),
+    minGasPrice: 0n,
+    mining: {
+      autoMine: true,
+      memPool: {
+        order: MineOrdering.Priority,
+      },
+    },
+    networkId: 123n,
+  };
+
+  const loggerConfig = {
+    enable: false,
+    decodeConsoleLogInputsCallback: (inputs: Buffer[]): string[] => {
+      return [];
+    },
+    getContractAndFunctionNameCallback: (
+      _code: Buffer,
+      _calldata?: Buffer,
+    ): ContractAndFunctionName => {
+      return {
+        contractName: "",
+      };
+    },
+    printLineCallback: (message: string, replace: boolean) => {},
+  };
+
+  it("issue 543", async function () {
+    if (ALCHEMY_URL === undefined || !isCI()) {
+      this.skip();
+    }
+
+    // This test is slow because the debug_traceTransaction is performed on a large transaction.
+    this.timeout(240_000);
+
+    const provider = await Provider.withConfig(
+      context,
+      {
+        fork: {
+          jsonRpcUrl: ALCHEMY_URL,
+        },
+        initialBaseFeePerGas: 0n,
+        ...providerConfig,
+      },
+      loggerConfig,
+      (_event: SubscriptionEvent) => {},
+    );
+
+    const debugTraceTransaction = `{
+        "jsonrpc": "2.0",
+        "method": "debug_traceTransaction",
+        "params": ["0x7e460f200343e5ab6653a8857cc5ef798e3f5bea6a517b156f90c77ef311a57c"],
+        "id": 1
+      }`;
+
+    const response = await provider.handleRequest(debugTraceTransaction);
+
+    let responseData;
+
+    if (typeof response.data === "string") {
+      responseData = JSON.parse(response.data);
+    } else {
+      responseData = response.data;
+    }
+
+    // Validate that we can query the response data without crashing.
+    const _json = new JsonStreamStringify(responseData);
+  });
+});

crates/edr_napi/test/provider.ts

@@ -9,23 +9,10 @@ import {
   SpecId,
   SubscriptionEvent,
 } from "..";
-import { collectMessages, collectSteps } from "./helpers";
+import { collectMessages, collectSteps, ALCHEMY_URL } from "./helpers";
 
 chai.use(chaiAsPromised);
 
-function getEnv(key: string): string | undefined {
-  const variable = process.env[key];
-  if (variable === undefined || variable === "") {
-    return undefined;
-  }
-
-  const trimmed = variable.trim();
-
-  return trimmed.length === 0 ? undefined : trimmed;
-}
-
-const ALCHEMY_URL = getEnv("ALCHEMY_URL");
-
 describe("Provider", () => {
   const context = new EdrContext();
   const providerConfig = {
@@ -350,7 +337,15 @@ describe("Provider", () => {
         }),
       );
 
-      const txHash = JSON.parse(sendTxResponse.json).result;
+      let responseData;
+
+      if (typeof sendTxResponse.data === "string") {
+        responseData = JSON.parse(sendTxResponse.data);
+      } else {
+        responseData = sendTxResponse.data;
+      }
+
+      const txHash = responseData.result;
 
       const traceTransactionResponse = await provider.handleRequest(
         JSON.stringify({

crates/tools/js/benchmark/index.js

@@ -90,10 +90,9 @@ async function report(benchmarkResultPath) {
 async function verify(benchmarkResultPath) {
   let success = true;
   const benchmarkResult = require(benchmarkResultPath);
-  const snapshotResult = require(path.join(
-    getScenariosDir(),
-    SCENARIO_SNAPSHOT_NAME
-  ));
+  const snapshotResult = require(
+    path.join(getScenariosDir(), SCENARIO_SNAPSHOT_NAME),
+  );
 
   for (let scenarioName in snapshotResult) {
     // TODO https://github.com/NomicFoundation/edr/issues/365
@@ -107,7 +106,7 @@ async function verify(benchmarkResultPath) {
       if (ratio > NEPTUNE_MAX_MIN_FAILURES) {
         console.error(
           `Snapshot failure for ${scenarioName} with max/min failure ratio`,
-          ratio
+          ratio,
         );
         success = false;
       }
@@ -129,16 +128,16 @@ async function verify(benchmarkResultPath) {
       if (shouldFail.size > 0) {
         console.error(
           `Scenario ${scenarioName} should fail at indexes ${Array.from(
-            shouldFail
-          ).sort()}`
+            shouldFail,
+          ).sort()}`,
         );
       }
 
       if (shouldNotFail.size > 0) {
         console.error(
           `Scenario ${scenarioName} should not fail at indexes ${Array.from(
-            shouldNotFail
-          ).sort()}`
+            shouldNotFail,
+          ).sort()}`,
         );
       }
     }
@@ -207,7 +206,7 @@ async function benchmarkAllScenarios(outPath, useAnvil) {
   console.error(
     `Total time ${
       Math.round(100 * (totalTime / 1000)) / 100
-    } seconds with ${totalFailures} failures.`
+    } seconds with ${totalFailures} failures.`,
   );
 
   console.error(`Benchmark results written to ${outPath}`);
@@ -275,7 +274,7 @@ async function benchmarkScenario(scenarioFileName, useAnvil) {
   console.error(
     `${name} finished in ${
       Math.round(100 * (timeMs / 1000)) / 100
-    } seconds with ${failures.length} failures.`
+    } seconds with ${failures.length} failures.`,
   );
 
   const result = {
@@ -331,11 +330,11 @@ function preprocessConfig(config) {
   config = removeNull(config);
 
   config.providerConfig.initialDate = new Date(
-    config.providerConfig.initialDate.secsSinceEpoch * 1000
+    config.providerConfig.initialDate.secsSinceEpoch * 1000,
   );
 
   config.providerConfig.hardfork = normalizeHardfork(
-    config.providerConfig.hardfork
+    config.providerConfig.hardfork,
   );
 
   // "accounts" in EDR are "genesisAccounts" in Hardhat
@@ -345,7 +344,7 @@ function preprocessConfig(config) {
   config.providerConfig.genesisAccounts = config.providerConfig.accounts.map(
     ({ balance, secretKey }) => {
       return { balance, privateKey: secretKey };
-    }
+    },
   );
   delete config.providerConfig.accounts;
 
@@ -380,6 +379,7 @@ function preprocessConfig(config) {
   }
 
   config.providerConfig.minGasPrice = BigInt(config.providerConfig.minGasPrice);
+  config.providerConfig.enableRip7212 = false;
 
   return config;
 }

package.json

@@ -16,6 +16,9 @@
   "pnpm": {
     "overrides": {
       "hardhat>@nomicfoundation/edr": "workspace:*"
+    },
+    "patchedDependencies": {
+      "[email protected]": "patches/[email protected]"
     }
   },
   "engines": {