feat: add backwards compatibility for Response::traces#1301
feat: add backwards compatibility for Response::traces#1301
Response::traces#1301Conversation
🦋 Changeset detectedLatest commit: 369dc66 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.
| .get(call_id) | ||
| .expect("child index should be valid"); | ||
|
|
||
| if matches!(step.op, OpCode::CREATE | OpCode::CREATE2) || !should_skip_call(trace) { |
There was a problem hiding this comment.
The should_skip_call function is called with the wrong trace parameter. On line 230, it's called with trace (the parent trace), but it should be called with node.trace (the child trace) to check whether the child call should be skipped.
The function checks if a call has no steps and immediately reverts, which is a property of the child trace, not the parent trace.
| if matches!(step.op, OpCode::CREATE | OpCode::CREATE2) || !should_skip_call(trace) { | |
| if matches!(step.op, OpCode::CREATE | OpCode::CREATE2) || !should_skip_call(&node.trace) { |
There was a problem hiding this comment.
I believe this is a correct observation from Copilot.
| 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.
87edb00 to
041399f
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1301 +/- ##
==========================================
+ Coverage 73.72% 73.94% +0.21%
==========================================
Files 445 444 -1
Lines 75876 75662 -214
Branches 75876 75662 -214
==========================================
+ Hits 55942 55945 +3
+ Misses 17882 17666 -216
+ Partials 2052 2051 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| .get(call_id) | ||
| .expect("child index should be valid"); | ||
|
|
||
| if matches!(step.op, OpCode::CREATE | OpCode::CREATE2) || !should_skip_call(trace) { |
There was a problem hiding this comment.
I believe this is a correct observation from Copilot.
| if trace.steps.is_empty() | ||
| && let Some(status) = trace.status | ||
| && matches!(status, return_revert!()) | ||
| { | ||
| true | ||
| } else { | ||
| false | ||
| } |
There was a problem hiding this comment.
This can be simplified to return the condition
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.
d203b12 to
369dc66
Compare
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: