edr icon indicating copy to clipboard operation
edr copied to clipboard

feat: add Optimism as separate N-API package

Open Wodann opened this issue 5 months ago • 2 comments

This PR restructures the edr_napi_core and edr_napi to be able to share code with the new edr_napi_optimism package.

I ran into one problem that I was able to solve. Namely, when a crate that generates NAPI bindings is added as a dependency to another crate that generates NAPI bindings, the test target of the dependant crate will throw linker errors. This is a known issue. I was only able to work around it by adding the recommended rustflags to both the workspace- and project-level .cargo/config.toml. If I only added it to the project, the cargo test --all-features --all-targets --workspace command would still throw the linker errors.

There is currently still a bug in the edr_napi_optimism JS tests. In the background, the hyper Client is sporadically trying to execute a future using its executor. By default, this executor uses tokio::spawn but since the edr_optimism NPM package doesn't have a tokio runtime, this panics.

thread '' panicked at /home/vscode/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/common/exec.rs:49:21: there is no reactor running, must be called from the context of a Tokio 1.x runtime note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

The solution is to specify a custom executor, but reqwest always uses the Tokio executor. An issue exists to add this executor, but a previous PR was rejected because it was deemed unnecessary.

For now, my proposal would be to merge the changes to the project layout and move forward with the release of the GenericChainSpec, as edr_optimism won't be exposed anyway.

A future issue can then:

  1. fork reqwest
  2. add setting of the executor
  3. set the executor with the tokio::Runtime of our edr NPM package's runtime

Could you please pay special attention to these two aspects when reviewing?

Wodann avatar Sep 10 '24 22:09 Wodann