fix: avoid panic when using smock

.changeset/nasty-teachers-brush.md

@@ -0,0 +1,5 @@
+---
+"@nomicfoundation/edr": patch
+---
+
+Fixed `smock.fake` causing a panic

.github/workflows/edr-npm-release.yml

@@ -185,7 +185,7 @@ jobs:
         run: ls -R .
         shell: bash
       - name: Test bindings
-        run: pnpm testNoBuild
+        run: pnpm testArtifact
   test-linux-x64-gnu-binding:
     name: Test bindings on Linux-x64-gnu - node@${{ matrix.node }}
     needs:
@@ -219,7 +219,9 @@ jobs:
         run: ls -R .
         shell: bash
       - name: Test bindings
-        run: docker run --rm -v $(pwd):/build -w /build/crates/edr_napi node:${{ matrix.node }} bash -c "wget -qO- 'https://unpkg.com/@pnpm/self-installer' | node; pnpm testNoBuild"
+        # Setting CI=1 is important to make PNPM install non-interactive
+        # https://github.com/pnpm/pnpm/issues/6615#issuecomment-1656945689
+        run: docker run --rm  -e CI=1 -v $(pwd):/build -w /build/crates/edr_napi node:${{ matrix.node }} bash -c "wget -qO- 'https://unpkg.com/@pnpm/self-installer' | node; pnpm testArtifact"
   test-linux-x64-musl-binding:
     name: Test bindings on x86_64-unknown-linux-musl - node@${{ matrix.node }}
     needs:
@@ -255,7 +257,7 @@ jobs:
         run: ls -R .
         shell: bash
       - name: Test bindings
-        run: docker run --rm -v $(pwd):/build -w /build/crates/edr_napi node:${{ matrix.node }}-alpine sh -c "wget -qO- 'https://unpkg.com/@pnpm/self-installer' | node; pnpm testNoBuild"
+        run: docker run --rm  -e CI=1 -v $(pwd):/build -w /build/crates/edr_napi node:${{ matrix.node }}-alpine sh -c "wget -qO- 'https://unpkg.com/@pnpm/self-installer' | node; pnpm testArtifact"
   test-linux-aarch64-gnu-binding:
     name: Test bindings on aarch64-unknown-linux-gnu - node@${{ matrix.node }}
     needs:
@@ -294,11 +296,11 @@ jobs:
         uses: addnab/docker-run-action@v3
         with:
           image: node:${{ matrix.node }}
-          options: "--platform linux/arm64 -v ${{ github.workspace }}:/build -w /build/crates/edr_napi"
+          options: "--platform linux/arm64 -v ${{ github.workspace }}:/build -w /build/crates/edr_napi -e CI=1"
           run: |
             wget -qO- 'https://unpkg.com/@pnpm/self-installer' | node
             set -e
-            pnpm testNoBuild
+            pnpm testArtifact
             ls -la
   test-linux-aarch64-musl-binding:
     name: Test bindings on aarch64-unknown-linux-musl - node@lts
@@ -332,11 +334,11 @@ jobs:
         uses: addnab/docker-run-action@v3
         with:
           image: node:lts-alpine
-          options: "--platform linux/arm64 -v ${{ github.workspace }}:/build -w /build/crates/edr_napi"
+          options: "--platform linux/arm64 -v ${{ github.workspace }}:/build -w /build/crates/edr_napi -e CI=1"
           run: |
             wget -qO- 'https://unpkg.com/@pnpm/self-installer' | node
             set -e
-            pnpm testNoBuild
+            pnpm testArtifact
   check_commit:
     name: Check commit
     runs-on: ubuntu-22.04

crates/edr_napi/.gitignore

@@ -96,3 +96,7 @@ typings/
 
 # DynamoDB Local files
 .dynamodb/
+
+# Hardhat output
+artifacts
+cache

crates/edr_napi/hardhat.config.js

@@ -0,0 +1,9 @@
+require("@nomiclabs/hardhat-ethers")
+
+/** @type import('hardhat/config').HardhatUserConfig */
+module.exports = {
+  paths: {
+    sources: './test/contracts',
+  },
+  solidity: "0.8.24",
+};

crates/edr_napi/package.json

@@ -27,13 +27,16 @@
   },
   "license": "MIT",
   "devDependencies": {
+    "@defi-wonderland/smock": "^2.4.0",
     "@napi-rs/cli": "^2.18.1",
+    "@nomiclabs/hardhat-ethers": "^2.2.3",
     "@types/chai": "^4.2.0",
     "@types/chai-as-promised": "^7.1.8",
     "@types/mocha": ">=9.1.0",
     "@types/node": "^18.0.0",
     "chai": "^4.3.6",
     "chai-as-promised": "^7.1.1",
+    "hardhat": "^2.22.3",
     "mocha": "^10.0.0",
     "ts-node": "^10.8.0",
     "typescript": "~4.5.2"
@@ -50,9 +53,10 @@
     "prepublishOnly": "napi prepublish -t npm --skip-gh-release",
     "universal": "napi universal",
     "version": "napi version",
-    "pretest": "pnpm build",
+    "pretest": "pnpm build && npx hardhat compile",
     "test": "pnpm tsc && mocha --recursive \"test/**/*.ts\"",
-    "testNoBuild": "pnpm tsc && mocha --recursive \"test/**/*.ts\"",
+    "pretestArtifact": "pnpm install && pnpm tsc && npx hardhat compile",
+    "testArtifact": "mocha --recursive \"test/**/*.ts\" --timeout 60000",
     "clean": "rm -rf @nomicfoundation/edr.node"
   }
 }

crates/edr_napi/src/call_override.rs

@@ -2,10 +2,11 @@ use std::sync::mpsc::channel;
 
 use edr_eth::{Address, Bytes};
 use napi::{
-    bindgen_prelude::Buffer,
+    bindgen_prelude::{Buffer, Promise},
     threadsafe_function::{
         ErrorStrategy, ThreadSafeCallContext, ThreadsafeFunction, ThreadsafeFunctionCallMode,
     },
+    tokio::runtime,
     Env, JsFunction, Status,
 };
 use napi_derive::napi;
@@ -41,10 +42,15 @@ struct CallOverrideCall {
 #[derive(Clone)]
 pub struct CallOverrideCallback {
     call_override_callback_fn: ThreadsafeFunction<CallOverrideCall, ErrorStrategy::Fatal>,
+    runtime: runtime::Handle,
 }
 
 impl CallOverrideCallback {
-    pub fn new(env: &Env, call_override_callback: JsFunction) -> napi::Result<Self> {
+    pub fn new(
+        env: &Env,
+        call_override_callback: JsFunction,
+        runtime: runtime::Handle,
+    ) -> napi::Result<Self> {
         let mut call_override_callback_fn = call_override_callback.create_threadsafe_function(
             0,
             |ctx: ThreadSafeCallContext<CallOverrideCall>| {
@@ -68,6 +74,7 @@ impl CallOverrideCallback {
 
         Ok(Self {
             call_override_callback_fn,
+            runtime,
         })
     }
 
@@ -78,21 +85,24 @@ impl CallOverrideCallback {
     ) -> Option<edr_provider::CallOverrideResult> {
         let (sender, receiver) = channel();
 
+        let runtime = self.runtime.clone();
         let status = self.call_override_callback_fn.call_with_return_value(
             CallOverrideCall {
                 contract_address,
                 data,
             },
             ThreadsafeFunctionCallMode::Blocking,
-            move |result: Option<CallOverrideResult>| {
-                let result = result.try_cast();
-
-                sender.send(result).map_err(|_error| {
-                    napi::Error::new(
-                        Status::GenericFailure,
-                        "Failed to send result from call_override_callback",
-                    )
-                })
+            move |result: Promise<Option<CallOverrideResult>>| {
+                runtime.spawn(async move {
+                    let result = result.await?.try_cast();
+                    sender.send(result).map_err(|_error| {
+                        napi::Error::new(
+                            Status::GenericFailure,
+                            "Failed to send result from call_override_callback",
+                        )
+                    })
+                });
+                Ok(())
             },
         );
 

crates/edr_napi/src/provider.rs

@@ -20,6 +20,7 @@ use crate::{
 #[napi]
 pub struct Provider {
     provider: Arc<edr_provider::Provider<LoggerError>>,
+    runtime: runtime::Handle,
     #[cfg(feature = "scenarios")]
     scenario_file: Option<napi::tokio::sync::Mutex<napi::tokio::fs::File>>,
 }
@@ -53,7 +54,7 @@ impl Provider {
                 ))?;
 
             let result = edr_provider::Provider::new(
-                runtime,
+                runtime.clone(),
                 logger,
                 subscriber_callback,
                 config,
@@ -64,6 +65,7 @@ impl Provider {
                 |provider| {
                     Ok(Provider {
                         provider: Arc::new(provider),
+                        runtime,
                         #[cfg(feature = "scenarios")]
                         scenario_file,
                     })
@@ -185,7 +187,8 @@ impl Provider {
     ) -> napi::Result<()> {
         let provider = self.provider.clone();
 
-        let call_override_callback = CallOverrideCallback::new(&env, call_override_callback)?;
+        let call_override_callback =
+            CallOverrideCallback::new(&env, call_override_callback, self.runtime.clone())?;
         let call_override_callback =
             Arc::new(move |address, data| call_override_callback.call_override(address, data));
 

crates/edr_napi/test/contracts/FooBar.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.24;
+
+contract Foo {
+  function f() public {}
+}
+
+contract Bar {
+  function callFoo(Foo foo) public {
+    foo.f();
+  }
+}

crates/edr_napi/test/provider.ts

@@ -1,5 +1,7 @@
+import { smock } from "@defi-wonderland/smock";
 import chai, { assert } from "chai";
 import chaiAsPromised from "chai-as-promised";
+
 import {
   ContractAndFunctionName,
   EdrContext,
@@ -101,4 +103,27 @@ describe("Provider", () => {
 
     await assert.isFulfilled(provider);
   });
+
+  // Ported from https://github.com/fvictorio/edr-smock-issue
+  it("set call override callback for smock", async function () {
+    // Hardhat doesn't work with ESM modules, so we have to require it here.
+    // The pretest hook runs compile.
+    const { ethers } = require("hardhat");
+
+    const provider = await Provider.withConfig(
+      context,
+      providerConfig,
+      loggerConfig,
+      (_event: SubscriptionEvent) => {}
+    );
+
+    // Test that we handle promise result appropriately in Rust
+    provider.setCallOverrideCallback(async (contractAddress, data) => undefined);
+
+    const mockedFoo = await smock.fake('Foo');
+    const Bar = await ethers.getContractFactory('Bar');
+    const bar = await Bar.deploy();
+
+    await bar.callFoo(mockedFoo.address);
+  });
 });