Skip to content

fix(ethereum): handle trace_filter traces missing result.output via c…#6535

Open
ibrahim1023 wants to merge 3 commits intographprotocol:masterfrom
ibrahim1023:fix-6489-trace-output-compat
Open

fix(ethereum): handle trace_filter traces missing result.output via c…#6535
ibrahim1023 wants to merge 3 commits intographprotocol:masterfrom
ibrahim1023:fix-6489-trace-output-compat

Conversation

@ibrahim1023
Copy link
Copy Markdown

Summary

Fixes #6489

This PR fixes a regression where trace_filter responses fail to deserialize when a trace result is missing the output field (for example, some Sonic traces), causing repeated retries and trace ingestion failure.

Root Cause

Graph Node now uses Alloy trace types for trace_filter responses. Some providers return traces where result.gasUsed is present but result.output is omitted. Alloy deserialization treats that as an invalid TraceOutput variant and fails the entire response.

What Changed

  • Kept the normal typed Alloy trace_filter request path unchanged for healthy responses.
  • Added targeted detection for the known deserialization error:
    • data did not match any variant of untagged enum TraceOutput
  • Added a compatibility fallback path only for that error:
    • re-issue trace_filter as raw JSON
    • patch traces with missing result.output by injecting "output": "0x" when result.gasUsed exists
    • deserialize patched JSON back into Vec<LocalizedTransactionTrace>
  • Added/updated regression coverage in ethereum_adapter tests for:
    • reproducing the original missing-output deserialization error
    • verifying the compatibility patch injects result.output

Why This Is Safe

  • The fallback is narrow and only activates for one known error signature.
  • Normal successful responses stay on the existing typed path.
  • Patch logic is minimal and scoped to trace entries where result already indicates execution output context (gasUsed) but output is omitted.
  • Existing parsing/trigger logic remains unchanged.

Validation

Executed locally:

  • cargo test -p graph-chain-ethereum missing_output_trace_repro -- --nocapture
  • cargo test -p graph-chain-ethereum

Result: all tests passed (50 passed, 0 failed for graph-chain-ethereum).

Copy link
Copy Markdown
Member

@incrypto32 incrypto32 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR

A few thoughts:

  1. RpcError::DeserError already has the failing response text on it (see alloy's try_deserialize_ok: https://github.com/alloy-rs/alloy/blob/main/crates/json-rpc/src/result.rs#L65-L68). So you can match the variant and reuse text rather than firing a second RPC. That also gets you off to_string().contains(...), which is wider than what you actually want to catch.

  2. patch_missing_trace_output probably belongs in graph/src/components/ethereum/json_patch.rs next to patch_receipts. PatchingHttp already uses that module for the same kind of thing (alloy being stricter than rust-web3 was), and putting it there means you can unit test it without spinning up a provider mock.

  3. On tests, a few json!() cases for the patch fn would be nice (missing output, already set, create-style result, no result at all), plus one that runs it on the Sonic fixture from the issue and checks Vec<LocalizedTransactionTrace> actually deserializes. The mock test is fine, just a bit heavy for what it ends up asserting.

  4. I opened alloy-rs/alloy#3931 (issue alloy-rs/alloy#3930) for the upstream fix, same shape they used in alloy#1102 for TraceResults.output. Once that ships and we bump alloy this whole thing goes away, so a // TODO: remove after alloy #3931 would help future-us find it.

@ibrahim1023
Copy link
Copy Markdown
Author

Thanks for the detailed review, this is all addressed now.

  1. I switched the fallback trigger to match RpcError::DeserError { text, err } and check the specific TraceOutput deserialization failure.
  2. Instead of issuing a second RPC, the compat path now reuses the text payload from the deserialization error.
  3. I moved the patcher into graph/src/components/ethereum/json_patch.rs and wired the adapter to use that shared helper.
  4. Added focused unit tests for the patch function (missing output, already set, create-style result, missing result) plus a fixture test that patches and then deserializes into Vec<LocalizedTransactionTrace>.
  5. Added a TODO in the compat path to remove this once fix(rpc-types-trace): default missing/null CallOutput.output to empty bytes alloy-rs/alloy#3931 is released and adopted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Graphnode can no longer parse traces with results missing output field

2 participants