Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add Optimism bindings in N-API #664

Merged
merged 15 commits into from
Sep 17, 2024
Merged

feat: add Optimism bindings in N-API #664

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
09d7311
refactor: move prepublish script
Wodann Sep 6, 2024
4f8e763
feat: add Optimism N-API package
Wodann Sep 6, 2024
2499926
fix: edr_napi (test) target
Wodann Sep 10, 2024
fb093cf
ci: prevent RUSTFLAGS from overriding .cargo/config.toml
Wodann Sep 12, 2024
74f55e8
refactor: revert to single NPM package
Wodann Sep 13, 2024
80c3dc7
fix: remove Optimism bindings
Wodann Sep 13, 2024
65a545e
ci: add environment variables for cargo hack
Wodann Sep 13, 2024
e750ef0
fix: pnpm testNoBuild
Wodann Sep 13, 2024
7dd2b47
misc: apply review suggestions
Wodann Sep 13, 2024
fa4f9e4
ci: increase timeout for issue 543
Wodann Sep 13, 2024
e6c1908
ci: increase timeout for macOS
Wodann Sep 16, 2024
1bb2508
ci: increase timeout for macOS
Wodann Sep 16, 2024
f335282
ci: increase timeout for macOS
Wodann Sep 16, 2024
35219e5
ci: increase timeout for macOS
Wodann Sep 17, 2024
92e9191
ci: decrease timeout for macOS
Wodann Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/edr-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ on:
workflow_dispatch:

env:
RUSTFLAGS: -Dwarnings
Copy link
Member Author

@Wodann Wodann Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was overriding the .cargo/config.toml file's rustflags.

This change should not have any adverse side-effects, as we also manually pass this argument to cargo clippy which should fail if there are any warnings.

For reference:

cargo clippy --workspace --all-targets --all-features -- -D warnings

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanatory comment! I'm wondering if removing it has effect on the check-edr and test-edr-rs? E.g. if there are some unused imports with some feature combination, would the CI not fail now? It's not a merge blocker for me if it doesn't, just want to make sure we consider it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Given that we use cargo hack in check-edr, we would lose some functionality - I think. I've re-added the environment variables for that specific command here.

Clippy catches all warnings that cargo check|build|test would catch, as it is a superset of the check command - under the hood. Since we run cargo clippy with --all-targets, the test target is also run.

This means that it's possible for the test-edr-rs step to succeed with warnings, but the step running cargo clippy would fail the CI.

To validate this, I just added two simple functions (dead code) to lib.rs:

fn foo() {
    println!("foo");
}

#[cfg(test)]
fn bar() {
    prinltn!("bar");
}

Running cargo check --all-targets gives the following warnings:

warning: function `foo` is never used
  --> crates/edr_eth/src/lib.rs:62:4
   |
62 | fn foo() {
   |    ^^^
   |
   = note: `#[warn(dead_code)]` on by default

warning: `edr_eth` (lib) generated 1 warning
warning: function `bar` is never used
  --> crates/edr_eth/src/lib.rs:67:4
   |
67 | fn bar() {
   |    ^^^

warning: `edr_eth` (lib test) generated 2 warnings (1 duplicate)

These become errors when running cargo clippy --all-targets -- -D warnings (failing one at a time):

error: function `foo` is never used
  --> crates/edr_eth/src/lib.rs:62:4
   |
62 | fn foo() {
   |    ^^^
   |
   = note: `-D dead-code` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(dead_code)]`

error: could not compile `edr_eth` (lib) due to 1 previous error

error: function `bar` is never used
  --> crates/edr_eth/src/lib.rs:63:4
   |
63 | fn bar() {
   |    ^^^
   |
   = note: `-D dead-code` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(dead_code)]`

RUSTDOCFLAGS: -Dwarnings

concurrency:
Expand All @@ -37,6 +36,8 @@ jobs:

- name: Cargo hack
uses: actions-rs/cargo@v1
env:
RUSTFLAGS: -Dwarnings
with:
command: hack
args: check --feature-powerset --no-dev-deps
Expand Down
15 changes: 12 additions & 3 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 3 additions & 7 deletions crates/edr_napi/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@ edition = "2021"
crate-type = ["cdylib"]

[dependencies]
ansi_term = { version = "0.12.1", default-features = false }
derive-where = { version = "1.2.7", default-features = false }
edr_eth = { path = "../edr_eth" }
edr_evm = { path = "../edr_evm" }
edr_generic = { path = "../edr_generic" }
edr_napi_core = { path = "../edr_napi_core" }
edr_optimism = { path = "../edr_optimism", optional = true }
edr_provider = { path = "../edr_provider" }
edr_rpc_client = { path = "../edr_rpc_client" }
itertools = { version = "0.12.0", default-features = false }
mimalloc = { version = "0.1.39", default-features = false, features = ["local_dynamic_tls"] }
# when napi is pinned, be sure to pin napi-derive to the same version
# The `async` feature ensures that a tokio runtime is available
Expand All @@ -24,7 +23,6 @@ napi-derive = "2.16.0"
rand = { version = "0.8.4", optional = true }
serde = { version = "1.0.209", features = ["derive"] }
serde_json = { version = "1.0.127" }
thiserror = { version = "1.0.37", default-features = false }
tracing = { version = "0.1.37", default-features = false, features = ["std"] }
tracing-flame = { version = "0.2.0", default-features = false, features = ["smallvec"] }
tracing-subscriber = { version = "0.3.18", default-features = false, features = ["ansi", "env-filter", "fmt", "parking_lot", "smallvec", "std"] }
Expand All @@ -45,11 +43,9 @@ openssl-sys = { version = "0.9.93", features = ["vendored"] }
napi-build = "2.0.1"

[features]
tracing = ["edr_evm/tracing", "edr_napi_core/tracing", "edr_provider/tracing"]
optimism = ["dep:edr_optimism"]
scenarios = ["rand"]

[profile.release]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cargo was reporting that this setting was ignored. Only the workspace Cargo.toml's profile are respected.

lto = true
tracing = ["edr_evm/tracing", "edr_napi_core/tracing", "edr_provider/tracing"]

[lints]
workspace = true
148 changes: 74 additions & 74 deletions crates/edr_napi/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,72 @@ export interface CallOverrideResult {
result: Buffer
shouldRevert: boolean
}
export const GENERIC_CHAIN_TYPE: string
export function genericChainProviderFactory(): ProviderFactory
export const L1_CHAIN_TYPE: string
export function l1ProviderFactory(): ProviderFactory
/** Identifier for the Ethereum spec. */
export const enum SpecId {
/** Frontier */
Frontier = 0,
/** Frontier Thawing */
FrontierThawing = 1,
/** Homestead */
Homestead = 2,
/** DAO Fork */
DaoFork = 3,
/** Tangerine */
Tangerine = 4,
/** Spurious Dragon */
SpuriousDragon = 5,
/** Byzantium */
Byzantium = 6,
/** Constantinople */
Constantinople = 7,
/** Petersburg */
Petersburg = 8,
/** Istanbul */
Istanbul = 9,
/** Muir Glacier */
MuirGlacier = 10,
/** Berlin */
Berlin = 11,
/** London */
London = 12,
/** Arrow Glacier */
ArrowGlacier = 13,
/** Gray Glacier */
GrayGlacier = 14,
/** Merge */
Merge = 15,
/** Shanghai */
Shanghai = 16,
/** Cancun */
Cancun = 17,
/** Latest */
Latest = 18
}
export const FRONTIER: string
export const FRONTIER_THAWING: string
export const HOMESTEAD: string
export const DAO_FORK: string
export const TANGERINE: string
export const SPURIOUS_DRAGON: string
export const BYZANTIUM: string
export const CONSTANTINOPLE: string
export const PETERSBURG: string
export const ISTANBUL: string
export const MUIR_GLACIER: string
export const BERLIN: string
export const LONDON: string
export const ARROW_GLACIER: string
export const GRAY_GLACIER: string
export const MERGE: string
export const SHANGHAI: string
export const CANCUN: string
export const PRAGUE: string
export const PRAGUE_EOF: string
export const LATEST: string
/** Configuration for a chain */
export interface ChainConfig {
/** The chain ID */
Expand Down Expand Up @@ -195,72 +261,6 @@ export interface DebugTraceLogItem {
/** Map of all stored values with keys and values encoded as hex strings. */
storage?: Record<string, string>
}
export const GENERIC_CHAIN_TYPE: string
export function genericChainProviderFactory(): ProviderFactory
export const L1_CHAIN_TYPE: string
export function l1ProviderFactory(): ProviderFactory
/** Identifier for the Ethereum spec. */
export const enum SpecId {
/** Frontier */
Frontier = 0,
/** Frontier Thawing */
FrontierThawing = 1,
/** Homestead */
Homestead = 2,
/** DAO Fork */
DaoFork = 3,
/** Tangerine */
Tangerine = 4,
/** Spurious Dragon */
SpuriousDragon = 5,
/** Byzantium */
Byzantium = 6,
/** Constantinople */
Constantinople = 7,
/** Petersburg */
Petersburg = 8,
/** Istanbul */
Istanbul = 9,
/** Muir Glacier */
MuirGlacier = 10,
/** Berlin */
Berlin = 11,
/** London */
London = 12,
/** Arrow Glacier */
ArrowGlacier = 13,
/** Gray Glacier */
GrayGlacier = 14,
/** Merge */
Merge = 15,
/** Shanghai */
Shanghai = 16,
/** Cancun */
Cancun = 17,
/** Latest */
Latest = 18
}
export const FRONTIER: string
export const FRONTIER_THAWING: string
export const HOMESTEAD: string
export const DAO_FORK: string
export const TANGERINE: string
export const SPURIOUS_DRAGON: string
export const BYZANTIUM: string
export const CONSTANTINOPLE: string
export const PETERSBURG: string
export const ISTANBUL: string
export const MUIR_GLACIER: string
export const BERLIN: string
export const LONDON: string
export const ARROW_GLACIER: string
export const GRAY_GLACIER: string
export const MERGE: string
export const SHANGHAI: string
export const CANCUN: string
export const PRAGUE: string
export const PRAGUE_EOF: string
export const LATEST: string
/** Ethereum execution log. */
export interface ExecutionLog {
address: Buffer
Expand Down Expand Up @@ -437,6 +437,14 @@ export class EdrContext {
registerProviderFactory(chainType: string, factory: ProviderFactory): Promise<void>
}
export class ProviderFactory { }
export class Response {
/**Returns the response data as a JSON string or a JSON object. */
get data(): string | any
/**Returns the Solidity trace of the transaction that failed to execute, if any. */
get solidityTrace(): RawTrace | null
/**Returns the raw traces of executed contracts. This maybe contain zero or more traces. */
get traces(): Array<RawTrace>
}
/** A JSON-RPC provider for Ethereum. */
export class Provider {
/**Handles a JSON-RPC request and returns a JSON-RPC response. */
Expand All @@ -450,14 +458,6 @@ export class Provider {
*/
setVerboseTracing(verboseTracing: boolean): Promise<void>
}
export class Response {
/**Returns the response data as a JSON string or a JSON object. */
get data(): string | any
/**Returns the Solidity trace of the transaction that failed to execute, if any. */
get solidityTrace(): RawTrace | null
/**Returns the raw traces of executed contracts. This maybe contain zero or more traces. */
get traces(): Array<RawTrace>
}
export class RawTrace {
trace(): Array<TracingMessage | TracingStep | TracingMessageResult>
}
8 changes: 4 additions & 4 deletions crates/edr_napi/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,8 @@ if (!nativeBinding) {
throw new Error(`Failed to load native binding`)
}

const { MineOrdering, EdrContext, GENERIC_CHAIN_TYPE, genericChainProviderFactory, L1_CHAIN_TYPE, l1ProviderFactory, SpecId, FRONTIER, FRONTIER_THAWING, HOMESTEAD, DAO_FORK, TANGERINE, SPURIOUS_DRAGON, BYZANTIUM, CONSTANTINOPLE, PETERSBURG, ISTANBUL, MUIR_GLACIER, BERLIN, LONDON, ARROW_GLACIER, GRAY_GLACIER, MERGE, SHANGHAI, CANCUN, PRAGUE, PRAGUE_EOF, LATEST, ProviderFactory, Provider, SuccessReason, ExceptionalHalt, Response, RawTrace } = nativeBinding
const { GENERIC_CHAIN_TYPE, genericChainProviderFactory, L1_CHAIN_TYPE, l1ProviderFactory, SpecId, FRONTIER, FRONTIER_THAWING, HOMESTEAD, DAO_FORK, TANGERINE, SPURIOUS_DRAGON, BYZANTIUM, CONSTANTINOPLE, PETERSBURG, ISTANBUL, MUIR_GLACIER, BERLIN, LONDON, ARROW_GLACIER, GRAY_GLACIER, MERGE, SHANGHAI, CANCUN, PRAGUE, PRAGUE_EOF, LATEST, MineOrdering, EdrContext, ProviderFactory, Response, Provider, SuccessReason, ExceptionalHalt, RawTrace } = nativeBinding

module.exports.MineOrdering = MineOrdering
module.exports.EdrContext = EdrContext
module.exports.GENERIC_CHAIN_TYPE = GENERIC_CHAIN_TYPE
module.exports.genericChainProviderFactory = genericChainProviderFactory
module.exports.L1_CHAIN_TYPE = L1_CHAIN_TYPE
Expand All @@ -340,9 +338,11 @@ module.exports.CANCUN = CANCUN
module.exports.PRAGUE = PRAGUE
module.exports.PRAGUE_EOF = PRAGUE_EOF
module.exports.LATEST = LATEST
module.exports.MineOrdering = MineOrdering
module.exports.EdrContext = EdrContext
module.exports.ProviderFactory = ProviderFactory
module.exports.Response = Response
module.exports.Provider = Provider
module.exports.SuccessReason = SuccessReason
module.exports.ExceptionalHalt = ExceptionalHalt
module.exports.Response = Response
module.exports.RawTrace = RawTrace
8 changes: 5 additions & 3 deletions crates/edr_napi/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"license": "MIT",
"devDependencies": {
"@napi-rs/cli": "^2.18.1",
"@nomicfoundation/ethereumjs-util": "^9.0.4",
"@types/chai": "^4.2.0",
"@types/chai-as-promised": "^7.1.8",
"@types/mocha": ">=9.1.0",
Expand All @@ -50,14 +51,15 @@
"artifacts": "napi artifacts",
"build": "napi build --platform --release",
"build:debug": "napi build --platform",
"build:optimism": "napi build --platform --release --features optimism",
"build:tracing": "napi build --platform --release --features tracing",
"build:scenarios": "napi build --platform --release --features scenarios",
"prepublishOnly": "bash scripts/prepublish.sh",
"prepublishOnly": "bash ../../scripts/prepublish.sh",
"universal": "napi universal",
"version": "napi version",
"pretest": "pnpm build",
"pretest": "pnpm build:optimism",
"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\"",
"testNoBuild": "pnpm tsc && node --max-old-space-size=8192 node_modules/mocha/bin/_mocha --recursive \"test/**/{,!(optimism)}.ts\"",
"clean": "rm -rf @nomicfoundation/edr.node"
}
}
7 changes: 7 additions & 0 deletions crates/edr_napi/src/chains.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// Types for the generic L1 Ethereum implementation.
pub mod generic;
/// Types for L1 Ethereum implementation.
pub mod l1;
/// Types for Optimism implementation.
#[cfg(feature = "optimism")]
pub mod optimism;
23 changes: 12 additions & 11 deletions crates/edr_napi/src/generic_chain.rs → crates/edr_napi/src/chains/generic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use std::sync::Arc;

use edr_generic::GenericChainSpec;
use edr_napi_core::{
logger::{self, Logger},
provider::{self, ProviderBuilder, SyncProviderFactory},
spec::SyncNapiSpec as _,
subscription,
};
use napi_derive::napi;

use crate::{
logger::{Logger, LoggerConfig},
provider::{self, factory::SyncProviderFactory, ProviderBuilder, ProviderFactory},
spec::SyncNapiSpec,
subscription::{SubscriptionCallback, SubscriptionConfig},
};
use crate::provider::ProviderFactory;

pub struct GenericChainProviderFactory;

Expand All @@ -17,19 +18,19 @@ impl SyncProviderFactory for GenericChainProviderFactory {
&self,
env: &napi::Env,
provider_config: edr_napi_core::provider::Config,
logger_config: LoggerConfig,
subscription_config: SubscriptionConfig,
logger_config: logger::Config,
subscription_config: subscription::Config,
) -> napi::Result<Box<dyn provider::Builder>> {
let logger = Logger::<GenericChainSpec>::new(env, logger_config)?;
let logger = Logger::<GenericChainSpec>::new(logger_config)?;

let provider_config =
edr_provider::ProviderConfig::<GenericChainSpec>::from(provider_config);

let subscription_callback =
SubscriptionCallback::new(env, subscription_config.subscription_callback)?;
subscription::Callback::new(env, subscription_config.subscription_callback)?;

Ok(Box::new(ProviderBuilder::new(
logger,
Box::new(logger),
provider_config,
subscription_callback,
)))
Expand Down
Loading
Loading