Skip to content

Commit 226295c

Browse files
Wodanngithub-actions[bot]fvictorioXanewokagostbiro
authored
chore: merge main into feat/multichain (#615)
* edr-0.4.1 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * chore: run prettier (#535) * test: avoid unconditional reverts in stack traces tests (#537) * fix: allow missing nonce in remote blocks (#539) * fix: allow missing nonce in remote blocks * misc: add changeset * ci: upgrade pnpm/action-setup (#541) * ci: enable some debug logs for hardhat tests (#538) * Sync with recent Hardhat changes and deduplicate the Hardhat dep (#544) * chore: Explicitly depend on semver and fs-extra in hardhat-tests Appeases ESLint. These are de facto pulled by other dependencies but it's implicit. * refactor: Unify ts-node and use the one currently used by Hardhat Deduping the hardhat package makes it easier to patch it while working on the stack trace porting feature branch. * chore: Use the newest Hardhat 2.22.6 This will make patching the changes easier to review as we will sync with the upstream as it has some changes already related to the stack traces that we port. * feat: upgrade revm dependencies (#546) * feat: upgrade revm dependencies * Create wild-phones-drum.md * edr-0.4.2 (#540) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * ci: remove path checks for benchmarks (#550) * ci: remove path restrictions for EDR jobs (#551) * ci: check that index.d.ts is up to date (#553) * feat: add support for RIP-7212 (#552) * feat: add support for RIP-7212 * misc: add changelog * fix: updated index.d.ts * fix: revert rename of InvalidFEOpcode * fix: set enableRip7212 in ProviderConfig * test: validate that disabling RIP-7212 works * refactor: use runtime variable instead of const * Path Hardhat dev dep to work with latest EDR changes EDR uses Hardhat as a dev dependency to run some javascript tests. The way this works is that the tests are run using Hardhat, but we use pnpm to override the EDR dependency in Hardhat with the local one. This works fine as long as there are no breaking changes in EDR. When there are, we have a circular dependency problem: we can't publish a new version of EDR until the tests pass, but for the tests to pass we need a version of Hardhat that works with the new version of EDR. A temporary workaround for this is to use `pnpm patch` to temporarily modify the Hardhat code in a way that works with the breaking change. In this case, this just means adding the new field when constructing the provider. --------- Co-authored-by: Franco Victorio <[email protected]> * edr-0.5.0 (#555) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * build: upgrade revm to v12 (#557) * build: upgrade revm to v12 * test: remove invalid test * misc: add changeset * fix: use remote chain id for pre-fork simulation (#567) * fix: use remote chain id for pre-fork simulation * Fix error message * chore: Bump hardhat to 2.22.7 (#571) * chore: Bump hardhat to 2.22.7 * fixup: Don't enable RIP-7212 in our tests * Adapt for NomicFoundation/hardhat#5411 * fix: prevent crash when returning large JSON responses (#569) * ci: update collaborator check in the benchmarks workflow (#574) * edr-0.5.1 (#559) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix: add json alias property in provider response (#582) * fix: add json alias property in provider response * Create empty-bobcats-refuse.md * edr-0.5.2 (#583) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * breaking change: rename response.json to response.data (#584) * breaking change: rename response.json to response.data * Create sour-donkeys-draw.md * Update sour-donkeys-draw.md --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Franco Victorio <[email protected]> Co-authored-by: Igor Matuszewski <[email protected]> Co-authored-by: Agost Biro <[email protected]> Co-authored-by: Piotr Galar <[email protected]>
1 parent a409e55 commit 226295c

File tree

27 files changed

+4601
-2166
lines changed

27 files changed

+4601
-2166
lines changed

.changeset/sour-donkeys-draw.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@nomicfoundation/edr": minor
3+
---
4+
5+
Renamed `json` field in response to `data` and changed its type to `string | object` (specified as `string | any` due to an napi-rs limitation).

.github/workflows/edr-benchmark.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
environment: github-action-benchmark
2525
runs-on: self-hosted
2626
# Only run for trusted collaborators since third-parties could run malicious code on the self-hosted benchmark runner.
27-
if: github.ref == 'refs/heads/main' || (github.event.pull_request.author_association == 'OWNER' || github.event.pull_request.author_association == 'MEMBER' || github.event.pull_request.author_association == 'COLLABORATOR')
27+
if: github.ref == 'refs/heads/main' || github.repository == github.event.pull_request.head.repo.full_name
2828
steps:
2929
- uses: actions/checkout@v3
3030

crates/edr_evm/src/blockchain.rs

+8
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,14 @@ where
125125
/// Retrieves the instances chain ID.
126126
fn chain_id(&self) -> u64;
127127

128+
/// Retrieves the chain ID of the block at the provided number.
129+
/// The chain ID can be different in fork mode pre- and post-fork block
130+
/// number.
131+
fn chain_id_at_block_number(&self, _block_number: u64) -> Result<u64, Self::BlockchainError> {
132+
// Chain id only depends on the block number in fork mode
133+
Ok(self.chain_id())
134+
}
135+
128136
/// Retrieves the last block in the blockchain.
129137
fn last_block(
130138
&self,

crates/edr_evm/src/blockchain/forked.rs

+12
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,18 @@ where
365365
self.chain_id
366366
}
367367

368+
fn chain_id_at_block_number(&self, block_number: u64) -> Result<u64, Self::BlockchainError> {
369+
if block_number > self.last_block_number() {
370+
return Err(BlockchainError::UnknownBlockNumber);
371+
}
372+
373+
if block_number <= self.fork_block_number {
374+
Ok(self.remote_chain_id())
375+
} else {
376+
Ok(self.chain_id())
377+
}
378+
}
379+
368380
#[cfg_attr(feature = "tracing", tracing::instrument(skip_all))]
369381
fn last_block(
370382
&self,

crates/edr_napi/CHANGELOG.md

+14
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
11
# @nomicfoundation/edr
22

3+
## 0.5.2
4+
5+
### Patch Changes
6+
7+
- 66ca796: fix: added `json` alias property in provider response to fix breaking change in v0.5.1
8+
9+
## 0.5.1
10+
11+
### Patch Changes
12+
13+
- 0bba027: Fixed a bug in fork mode where the locally overridden chain id was used instead of the remote chain id when simulating transactions in pre-fork blocks.
14+
- e02eb22: Fixed a bug in the JSON-RPC where we previously allowed a null value for storage keys of an access list
15+
- 8ae31b9: Fixed crash when returning large JSON responses
16+
317
## 0.5.0
418

519
### Minor Changes

crates/edr_napi/index.d.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,8 @@ export class Provider {
418418
setVerboseTracing(verboseTracing: boolean): void
419419
}
420420
export class Response {
421-
get json(): string
421+
/** Returns the response data as a JSON string or a JSON object. */
422+
get data(): string | any
422423
get solidityTrace(): RawTrace | null
423424
get traces(): Array<RawTrace>
424425
}

crates/edr_napi/package.json

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@nomicfoundation/edr",
3-
"version": "0.5.0",
3+
"version": "0.5.2",
44
"main": "index.js",
55
"types": "index.d.ts",
66
"files": [
@@ -38,6 +38,7 @@
3838
"@types/node": "^18.0.0",
3939
"chai": "^4.3.6",
4040
"chai-as-promised": "^7.1.1",
41+
"json-stream-stringify": "^3.1.4",
4142
"mocha": "^10.0.0",
4243
"ts-node": "^10.8.0",
4344
"typescript": "~4.5.2"
@@ -55,8 +56,8 @@
5556
"universal": "napi universal",
5657
"version": "napi version",
5758
"pretest": "pnpm build",
58-
"test": "pnpm tsc && mocha --recursive \"test/**/*.ts\"",
59-
"testNoBuild": "pnpm tsc && mocha --recursive \"test/**/*.ts\"",
59+
"test": "pnpm tsc && node --max-old-space-size=8192 node_modules/mocha/bin/_mocha --recursive \"test/**/*.ts\"",
60+
"testNoBuild": "pnpm tsc && node --max-old-space-size=8192 node_modules/mocha/bin/_mocha --recursive \"test/**/*.ts\"",
6061
"clean": "rm -rf @nomicfoundation/edr.node"
6162
}
6263
}

crates/edr_napi/src/provider.rs

+28-9
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::sync::Arc;
55
use edr_eth::chain_spec::L1ChainSpec;
66
use edr_provider::{time::CurrentTime, InvalidRequestReason};
77
use edr_rpc_eth::jsonrpc;
8-
use napi::{tokio::runtime, Env, JsFunction, JsObject, Status};
8+
use napi::{tokio::runtime, Either, Env, JsFunction, JsObject, Status};
99
use napi_derive::napi;
1010

1111
use self::config::ProviderConfig;
@@ -121,9 +121,9 @@ impl Provider {
121121
format!("Invalid JSON `{json_request}` due to: {error}"),
122122
)
123123
})
124-
.map(|json_response| Response {
124+
.map(|json| Response {
125125
solidity_trace: None,
126-
json: json_response,
126+
data: Either::A(json),
127127
traces: Vec::new(),
128128
});
129129
}
@@ -169,10 +169,25 @@ impl Provider {
169169
let response = jsonrpc::ResponseData::from(response.map(|response| response.result));
170170

171171
serde_json::to_string(&response)
172-
.map_err(|e| napi::Error::new(Status::GenericFailure, e.to_string()))
173-
.map(|json_response| Response {
172+
.and_then(|json| {
173+
// We experimentally determined that 500_000_000 was the maximum string length
174+
// that can be returned without causing the error:
175+
//
176+
// > Failed to convert rust `String` into napi `string`
177+
//
178+
// To be safe, we're limiting string lengths to half of that.
179+
const MAX_STRING_LENGTH: usize = 250_000_000;
180+
181+
if json.len() <= MAX_STRING_LENGTH {
182+
Ok(Either::A(json))
183+
} else {
184+
serde_json::to_value(response).map(Either::B)
185+
}
186+
})
187+
.map_err(|error| napi::Error::new(Status::GenericFailure, error.to_string()))
188+
.map(|data| Response {
174189
solidity_trace,
175-
json: json_response,
190+
data,
176191
traces: traces.into_iter().map(Arc::new).collect(),
177192
})
178193
}
@@ -210,7 +225,10 @@ impl Provider {
210225

211226
#[napi]
212227
pub struct Response {
213-
json: String,
228+
// N-API is known to be slow when marshalling `serde_json::Value`s, so we try to return a
229+
// `String`. If the object is too large to be represented as a `String`, we return a `Buffer`
230+
// instead.
231+
data: Either<String, serde_json::Value>,
214232
/// When a transaction fails to execute, the provider returns a trace of the
215233
/// transaction.
216234
solidity_trace: Option<Arc<edr_evm::trace::Trace<L1ChainSpec>>>,
@@ -220,9 +238,10 @@ pub struct Response {
220238

221239
#[napi]
222240
impl Response {
241+
/// Returns the response data as a JSON string or a JSON object.
223242
#[napi(getter)]
224-
pub fn json(&self) -> String {
225-
self.json.clone()
243+
pub fn data(&self) -> Either<String, serde_json::Value> {
244+
self.data.clone()
226245
}
227246

228247
#[napi(getter)]

crates/edr_napi/test/helpers.ts

+17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,22 @@
11
import { TracingMessage, TracingMessageResult, TracingStep } from "..";
22

3+
function getEnv(key: string): string | undefined {
4+
const variable = process.env[key];
5+
if (variable === undefined || variable === "") {
6+
return undefined;
7+
}
8+
9+
const trimmed = variable.trim();
10+
11+
return trimmed.length === 0 ? undefined : trimmed;
12+
}
13+
14+
export const ALCHEMY_URL = getEnv("ALCHEMY_URL");
15+
16+
export function isCI(): boolean {
17+
return getEnv("CI") === "true";
18+
}
19+
320
/**
421
* Given a trace, return only its steps.
522
*/

crates/edr_napi/test/issues.ts

+109
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import chai, { assert } from "chai";
2+
import { JsonStreamStringify } from "json-stream-stringify";
3+
4+
import {
5+
ContractAndFunctionName,
6+
EdrContext,
7+
MineOrdering,
8+
Provider,
9+
SpecId,
10+
SubscriptionEvent,
11+
} from "..";
12+
import { ALCHEMY_URL, isCI } from "./helpers";
13+
14+
describe("Provider", () => {
15+
const context = new EdrContext();
16+
const providerConfig = {
17+
allowBlocksWithSameTimestamp: false,
18+
allowUnlimitedContractSize: true,
19+
bailOnCallFailure: false,
20+
bailOnTransactionFailure: false,
21+
blockGasLimit: 300_000_000n,
22+
chainId: 1n,
23+
chains: [],
24+
coinbase: Buffer.from("0000000000000000000000000000000000000000", "hex"),
25+
enableRip7212: false,
26+
genesisAccounts: [
27+
{
28+
secretKey:
29+
"0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80",
30+
balance: 1000n * 10n ** 18n,
31+
},
32+
],
33+
hardfork: SpecId.Latest,
34+
initialBlobGas: {
35+
gasUsed: 0n,
36+
excessGas: 0n,
37+
},
38+
initialParentBeaconBlockRoot: Buffer.from(
39+
"0000000000000000000000000000000000000000000000000000000000000000",
40+
"hex",
41+
),
42+
minGasPrice: 0n,
43+
mining: {
44+
autoMine: true,
45+
memPool: {
46+
order: MineOrdering.Priority,
47+
},
48+
},
49+
networkId: 123n,
50+
};
51+
52+
const loggerConfig = {
53+
enable: false,
54+
decodeConsoleLogInputsCallback: (inputs: Buffer[]): string[] => {
55+
return [];
56+
},
57+
getContractAndFunctionNameCallback: (
58+
_code: Buffer,
59+
_calldata?: Buffer,
60+
): ContractAndFunctionName => {
61+
return {
62+
contractName: "",
63+
};
64+
},
65+
printLineCallback: (message: string, replace: boolean) => {},
66+
};
67+
68+
it("issue 543", async function () {
69+
if (ALCHEMY_URL === undefined || !isCI()) {
70+
this.skip();
71+
}
72+
73+
// This test is slow because the debug_traceTransaction is performed on a large transaction.
74+
this.timeout(240_000);
75+
76+
const provider = await Provider.withConfig(
77+
context,
78+
{
79+
fork: {
80+
jsonRpcUrl: ALCHEMY_URL,
81+
},
82+
initialBaseFeePerGas: 0n,
83+
...providerConfig,
84+
},
85+
loggerConfig,
86+
(_event: SubscriptionEvent) => {},
87+
);
88+
89+
const debugTraceTransaction = `{
90+
"jsonrpc": "2.0",
91+
"method": "debug_traceTransaction",
92+
"params": ["0x7e460f200343e5ab6653a8857cc5ef798e3f5bea6a517b156f90c77ef311a57c"],
93+
"id": 1
94+
}`;
95+
96+
const response = await provider.handleRequest(debugTraceTransaction);
97+
98+
let responseData;
99+
100+
if (typeof response.data === "string") {
101+
responseData = JSON.parse(response.data);
102+
} else {
103+
responseData = response.data;
104+
}
105+
106+
// Validate that we can query the response data without crashing.
107+
const _json = new JsonStreamStringify(responseData);
108+
});
109+
});

crates/edr_napi/test/provider.ts

+10-15
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,10 @@ import {
99
SpecId,
1010
SubscriptionEvent,
1111
} from "..";
12-
import { collectMessages, collectSteps } from "./helpers";
12+
import { collectMessages, collectSteps, ALCHEMY_URL } from "./helpers";
1313

1414
chai.use(chaiAsPromised);
1515

16-
function getEnv(key: string): string | undefined {
17-
const variable = process.env[key];
18-
if (variable === undefined || variable === "") {
19-
return undefined;
20-
}
21-
22-
const trimmed = variable.trim();
23-
24-
return trimmed.length === 0 ? undefined : trimmed;
25-
}
26-
27-
const ALCHEMY_URL = getEnv("ALCHEMY_URL");
28-
2916
describe("Provider", () => {
3017
const context = new EdrContext();
3118
const providerConfig = {
@@ -350,7 +337,15 @@ describe("Provider", () => {
350337
}),
351338
);
352339

353-
const txHash = JSON.parse(sendTxResponse.json).result;
340+
let responseData;
341+
342+
if (typeof sendTxResponse.data === "string") {
343+
responseData = JSON.parse(sendTxResponse.data);
344+
} else {
345+
responseData = sendTxResponse.data;
346+
}
347+
348+
const txHash = responseData.result;
354349

355350
const traceTransactionResponse = await provider.handleRequest(
356351
JSON.stringify({

0 commit comments

Comments
 (0)