Skip to content

eth/tracers/logger: fix legacy structLog JSON encoding#34093

Merged
s1na merged 5 commits into
ethereum:masterfrom
MysticRyuujin:eth/tracers/fix-legacy-struct-log-json
Mar 31, 2026
Merged

eth/tracers/logger: fix legacy structLog JSON encoding#34093
s1na merged 5 commits into
ethereum:masterfrom
MysticRyuujin:eth/tracers/fix-legacy-struct-log-json

Conversation

@MysticRyuujin
Copy link
Copy Markdown
Contributor

@MysticRyuujin MysticRyuujin commented Mar 26, 2026

Release notes

This is a breaking change for tracing APIs (debug_trace*), when using specifically the structlog (opcode) tracer. The following fields will have slightly different formatting in the response. The reason is for geth to conform to the newly established client-wide spec: ethereum/execution-apis#762.

  • memory words will have the 0x prefix. A word will be padded to 32-bytes.
  • storage: keys and values will have the 0x prefix.

Summary

This PR fixes several correctness issues in the legacy structLog JSON format produced by StructLogger (the default tracer behind debug_traceTransaction), and removes the non-spec reexec field from the tracer config.

Fix legacy structLog JSON encoding

  • Error field: changed from string to *string so the "error" key is truly omitted when there is no error, rather than being serialised as "".
  • Memory word padding: each 32-byte memory chunk is now zero-padded before hex-encoding, producing the correct 64-character output. Previously, a partial word (e.g. a 2-byte chunk) would be emitted as a short string like "0xaabb" instead of "0xaabb000000000000000000000000000000000000000000000000000000000000".
  • Storage / ReturnData encoding: replaced bare fmt.Sprintf("%x", …) with hexutil.Encode / common.Hash.Hex() to produce consistent 0x-prefixed output, matching the format expected by clients and conformance tests.

Remove reexec from tracer config

reexec was a geth-specific, hash-scheme-only knob that let callers cap how many ancestor blocks the node would re-execute when the requested state was not on disk. It was never part of the execution API spec and was silently ignored by path-scheme nodes.

  • Removed Reexec *uint64 from TraceConfig and StdTraceConfig — the field is no longer accepted over JSON-RPC.
  • Dropped the parameter from the Backend interface (StateAtBlock, StateAtTransaction) and propagated the removal down through EthAPIBackend, state_accessor, and api_debug.
  • Non-archive hash-scheme behaviour is preserved via a package-internal reexecLimit = 128 constant in state_accessor.

Tests

  • TestStructLogLegacyJSONSpecFormatting — unit tests directly exercising the legacy JSON shape for memory padding, storage encoding, and error omission.
  • TestTraceTransactionRefundAndStorageSnapshots — integration test verifying that SLOAD/SSTORE storage snapshots and refund counters are correctly captured in a traced transaction.
  • TestTraceTransactionFailureReturnValues — integration test verifying that a reverted transaction preserves its return data while a hard-failure (e.g. INVALID) clears it.

Reference

execution-apis

Fix several issues in the legacy structLog JSON format produced by the
struct logger tracer:

- Error field: change from string to *string so that error is truly
  omitted (rather than serialised as "") when there is no error.
- Memory words: zero-pad each 32-byte chunk before hex-encoding so the
  output always matches the expected 64-character format instead of
  emitting a truncated hex string for partial words.
- Storage/ReturnData: use hexutil.Encode / common.Hash.Hex() to produce
  consistent 0x-prefixed output rather than bare %x formatting.

Add tests that cover the legacy JSON shape directly (TestStructLogLegacyJSONSpecFormatting),
and two integration-level tracer tests verifying that refund counters and
storage snapshots are captured correctly and that hard-failure vs revert
return values are handled as specified.
@MysticRyuujin MysticRyuujin requested a review from s1na as a code owner March 26, 2026 15:34
@s1na s1na self-assigned this Mar 26, 2026
reexec was a geth-specific, hash-scheme-only implementation detail that
let callers cap how many ancestor blocks the node would re-execute to
reconstruct missing historical state. It was never part of the execution
API spec, and path-scheme nodes ignored it entirely.

Remove Reexec from TraceConfig and StdTraceConfig so it is no longer
accepted over JSON-RPC. Drop the parameter from the Backend interface
(StateAtBlock, StateAtTransaction) and all the way down through
EthAPIBackend and state_accessor. A local constant reexecLimit = 128
replaces the old defaultTraceReexec inside hashState, preserving the
existing non-archive hash-scheme behaviour.
@MysticRyuujin
Copy link
Copy Markdown
Contributor Author

ci test failed but not because of my PR 😆

Comment thread eth/tracers/logger/logger.go
@s1na
Copy link
Copy Markdown
Contributor

s1na commented Mar 27, 2026

Generally looks good to me. I'd wait for merging until after the coming release. Given that it's breaking API output we should probably release it as part of a major version.

MysticRyuujin and others added 2 commits March 27, 2026 13:07
Co-authored-by: Sina M <1591639+s1na@users.noreply.github.com>
Comment thread eth/tracers/logger/logger.go Outdated
Use a plain string field with omitzero for the legacy structLog error field so the JSON shape stays unchanged without pointer indirection.
Copy link
Copy Markdown
Contributor

@s1na s1na left a comment

Choose a reason for hiding this comment

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

LGTM

@s1na s1na added this to the 1.17.3 milestone Mar 31, 2026
@s1na s1na merged commit 92b4cb2 into ethereum:master Mar 31, 2026
11 of 13 checks passed
@MysticRyuujin MysticRyuujin deleted the eth/tracers/fix-legacy-struct-log-json branch April 2, 2026 02:48
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.

3 participants