feat: add backwards compatibility for Response::traces - #1301
Conversation
GitHub Actions
Error
🦋 Changeset detectedLatest commit: a0d5e90 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR reintroduces backwards compatibility for the Response::traces getter to support Hardhat 2's EthereumJS VM interface. The implementation converts the new CallTraceArena format back to the flat trace format that Hardhat 2 expects.
Changes:
- Adds
Response::traces()method that convertsCallTraceArenato flat trace format compatible with Hardhat 2 - Removes
edr_tracingdependency from multiple crates - Deletes
nested_tracer.rsand related code that are no longer needed - Re-enables verbose tracing tests for Hardhat 2 compatibility
Reviewed changes
Copilot reviewed 22 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/edr_napi/src/trace.rs |
Implements conversion from CallTraceArena to flat trace format with Hardhat 2 compatibility quirks |
crates/edr_napi/src/provider/response.rs |
Adds traces() method to Response |
crates/edr_provider/src/observability.rs |
Updates tracing configuration to capture full stack snapshots |
patches/hardhat@2.28.4.patch |
Updates Hardhat 2 integration to use new trace format |
crates/edr_napi/test/provider.ts |
Re-enables verbose tracing tests |
| Multiple Cargo.toml files | Removes edr_tracing dependency |
crates/edr_solidity/src/nested_tracer.rs |
Deleted - no longer needed |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let tracing_config = if config.verbose_raw_tracing { | ||
| TracingInspectorConfig::all() | ||
| } else { | ||
| TracingInspectorConfig::default_parity().set_steps(true) | ||
| TracingInspectorConfig::default_parity() | ||
| .set_steps(true) | ||
| .set_stack_snapshots(StackSnapshotType::Full) |
There was a problem hiding this comment.
The StackSnapshotType::Full configuration is set regardless of verbose mode. This means that the full stack will always be captured in the tracing inspector, even when verbose tracing is disabled.
However, the actual filtering to return only the top of the stack happens in the conversion function raw_trace_from_call_trace_arena based on the verbose parameter. This creates an inconsistency where:
- The inspector always captures full stack (performance cost)
- The conversion then throws away most of the data when
verbose=false
Consider setting StackSnapshotType based on verbose_raw_tracing to avoid unnecessary data collection and improve performance.
There was a problem hiding this comment.
It's fixed in the stacked PRs rather than here: #1536 drops StackSnapshotType::Full from the non-verbose config and captures only the top of the stack per step.
87edb00 to
041399f
Compare
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Failure
GitHub Actions
Failure
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1301 +/- ##
==========================================
+ Coverage 79.68% 80.10% +0.42%
==========================================
Files 446 445 -1
Lines 76660 76479 -181
Branches 76660 76479 -181
==========================================
+ Hits 61087 61267 +180
+ Misses 13458 13088 -370
- Partials 2115 2124 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Failure
popescuoctavian
left a comment
popescuoctavian
left a comment
There was a problem hiding this comment.
There is significant performance regression seen in the benchmarks -- up to 2.45x.
I believe this is caused by the inclusion of the full stack snapshot. However, I don't see an immediate solution here, with the existing revm-inspectors tracing code.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Failure
GitHub Actions
Inactive
d203b12 to
369dc66
Compare
GitHub Actions
Inactive
GitHub Actions
Inactive
The verbose-mode trace tests send raw eth_sendTransaction requests without a gas field, so they inherited defaultTransactionGasLimit. Under the latest L1 hardfork the EIP-7825 per-transaction gas cap rejects that, producing no trace arena and an empty traces() result. Set an explicit sub-cap gas limit, matching the existing isStaticCall test.
Regenerate the hardhat patch against 2.28.6: all code hunks apply unchanged; hardhat#7918 only shifted the provider source maps, which are spliced from the old patch and drift by a few mapping lines after the changed region (cosmetic, dev-only). Set observability.includeCallTraces: All at provider creation. Without it Response.traces() returns no arenas and the re-enabled vm trace-event bridge silently emits nothing (the solidity-coverage 0%-coverage failure mode). Add a hardhat-tests spec asserting step/beforeMessage/afterMessage fire on a transaction: the only executable coverage of the bridge.
9acaaac to
a8ef4d2
Compare
GitHub Actions
Inactive
GitHub Actions
Failure
GitHub Actions
Failure
GitHub Actions
Failure
|
I locally ran benchmarks comparing the state before the old
There is one more optimisation worth to avoid regressions in HH3; i.e. to never collect stack trace tops. |
Also ignores `verbose` option when generating raw traces, since the `ObservalityConfig` already has the `record_memory` and `record_stack` options to control the level of detail in the traces.
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Error
|
@popescuoctavian could you please review these changes that I made:
|
GitHub Actions
Inactive
GitHub Actions
Failure
GitHub Actions
Inactive
popescuoctavian
left a comment
popescuoctavian
left a comment
There was a problem hiding this comment.
The changes look good to me, thanks @Wodann!
I've noticed some benchmark regressions: https://github.com/NomicFoundation/edr/actions/runs/28920891714/job/85797979674?pr=1301. Are these expected?
GitHub Actions
Inactive
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
GitHub Actions
Failure
GitHub Actions
Inactive
@popescuoctavian I performed profiling to analyse the HH2 performance regression. Here's the outcome:
CI (run 28920891714) shows this branch at Where the time goes (seaport: 5.85s → 7.50s, +1.65s, +28%)EVM/provider execution itself is unchanged — the entire delta is in producing, delivering, holding, and destroying per-response trace arenas:
The unifying driver is the first two rows, and the memory data explains them: peak RSS goes 445 MB → 1.5 GB (3.3×). Every For scale: the pre-fix build ( Methodology
I attempted a fix for the GC & teardown time of the |
|
Off-loading deallocation to a background thread resulted in better performance, but not on par with
|
Resolves #1288.
This reintroduces the
Response::tracesgetter function to maintain support for Hardhat 2's EthereumJS VM interface.I tested this using the help of Claude:
Hardhat Plugin Compatibility Report — EDR Tracing Back-Compat
Date: 2026-02-20
EDR branch:
feat/tracing-back-compatHardhat branch:
hh2/tracing-unificationOverview
This report validates that the new EDR trace format (unified tracing architecture)
is backward-compatible with the existing Hardhat v2 plugin ecosystem. Testing was
performed at three levels:
Critical Finding:
includeCallTracesDefaultDuring testing, solidity-coverage's test suite revealed that
Response::traces()returned empty arrays because
include_call_tracesdefaults toIncludeTraces::Nonein the new architecture. This was fixed in Hardhat commit
c46cf5bby settingincludeCallTraces: IncludeTraces.Allin the provider's observability config.Plugin Test Suite Results
hardhat-gas-reporter v2.3.0
Trace-format failures: 0. All gas measurement and reporting functionality works.
solidity-coverage v0.8.17
Trace-format failures after fix: 0.
hardhat-tracer v3.4.0
Trace-format failures: 0. All trace-exercising tests pass (CALLs, STATICCALLs,
DELEGATECALLs, opcodes,
debug_traceTransaction).Third-Party Project Results (with linked plugins)
Seaport (best result — full validation)
OpenZeppelin Contracts v5.5.0
NexusMutual
Additional projects tested (stock plugins, local Hardhat+EDR)
Non-Trace Issues Found
fail. This is a hardfork behavioral change, not a trace issue.
causes compilation failure on the full OZ codebase. Not trace-related.
Risk Assessment
Risk of trace format changes breaking existing Hardhat plugins: LOW
With the
includeCallTraces: IncludeTraces.Allfix in place, the backward-compatibilitylayer in EDR (
Response::traces()) combined with the Hardhat adapter successfullypreserves the expected trace format for all tested plugins: