Log: replace additional structure for json by typed fields#20340
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors execution/types.Log JSON encoding by replacing the generated gencodec shadow structs with typed hexutil fields directly on the struct, aiming to improve JSON marshal performance and reduce allocations across log-related RPC paths.
Changes:
- Replaced
types.LogJSON gencodec-generated approach with typedhexutilfields and customUnmarshalJSONvalidation. - Updated multiple RPC/execution call sites to populate log fields using
hexutil.Uint*wrappers (block number, tx index, log index, timestamps). - Removed generated JSON files for logs and added a new JSON benchmark for logs.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
txnprovider/shutter/eon_tracker.go |
Adjusts block number typing when building call options. |
txnprovider/shutter/encrypted_txn_submission.go |
Casts event raw block number to uint64 for submission struct. |
rpc/rpchelper/logtracer.go |
Switches log numeric fields to hexutil wrappers during trace capture. |
rpc/rpchelper/logsfilter.go |
Switches distributed log numeric fields to hexutil wrappers. |
rpc/jsonrpc/receipts/receipts_generator.go |
Updates derived log indices to hexutil.Uint. |
rpc/jsonrpc/receipts/bor_receipts_generator.go |
Updates Bor log indices to hexutil.Uint. |
rpc/jsonrpc/eth_simulation.go |
Wraps BlockTimestamp as hexutil.Uint64 in simulation results. |
rpc/jsonrpc/eth_receipts.go |
Wraps log timestamps as hexutil.Uint64 when building responses. |
rpc/jsonrpc/eth_mining_test.go |
Updates test expectations to use hexutil.Bytes for log data. |
rpc/jsonrpc/erigon_receipts.go |
Updates indices/timestamps/block numbers to hexutil wrappers in latest-logs path. |
rpc/jsonrpc/erigon_receipts_test.go |
Updates assertions for hexutil.Uint64 block numbers. |
polygon/bor/types/bor_receipt.go |
Updates derived receipt log fields to hexutil wrappers. |
execution/vm/instructions.go |
Updates LOG opcode-generated BlockNumber to hexutil.Uint64. |
execution/types/receipt.go |
Updates log-derived fields and log-index assertions to use hexutil numeric types. |
execution/types/log.go |
Core change: typed hexutil fields on Log, custom JSON unmarshal, removal of gencodec override struct. |
execution/types/gen_log_json.go |
Removes generated log JSON code. |
execution/types/gen_erigon_log_json.go |
Removes generated erigon-log JSON code. |
execution/types/encdec_test.go |
Adds JSON benchmark for logs and updates benchmark data generation. |
execution/state/intra_block_state.go |
Updates log tx/log indices and block number derivation to hexutil wrappers. |
execution/exec/txtask.go |
Updates receipt log indices and block number derivation to hexutil wrappers. |
execution/abi/bind/base_test.go |
Updates mock log block number literal to match new typed field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a2f6545 to
f990c40
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yperbasis
left a comment
There was a problem hiding this comment.
Major
Collapse the double-unmarshal in ErigonLog/RPCLog (execution/types/log.go:111,152). Each calls l.Log.UnmarshalJSON(input) and then json.Unmarshal(input, &dec) — parsing the full
input twice. A single flat struct with Timestamp/BlockTimestamp added avoids it. The old gencodec versions parsed once. The benchmark only covers marshal, so this regression isn't
currently visible.
Minor
- Benchmark only covers Marshal. Given this PR rewrites both paths, add BenchmarkLogJSONDecode/Unmarshal to verify unmarshal hasn't regressed (see point 1).
- erigon_receipts.go:354 — int(log.TxIndex) == len(body.Transactions) then body.Transactions[log.TxIndex]. The int() cast on one line and implicit hexutil.Uint → int conversion for
indexing on the next is slightly inconsistent; consider txi := int(log.TxIndex) once at the top, used for both. - RPCLog.BlockTimestamp type change (hexutil.Uint → hexutil.Uint64) is an incidental correctness improvement — the old code would truncate timestamps past 2^32 on 32-bit builds.
Worth a one-line PR note.
- Collapse double-unmarshal in ErigonLog/RPCLog UnmarshalJSON into a single flat struct parse (was calling l.Log.UnmarshalJSON then json.Unmarshal a second time) - Add BenchmarkLogJSONUnmarshal to cover the unmarshal path - Consolidate txi := int(log.TxIndex) in erigon_receipts.go to avoid mixed implicit/explicit int conversions on adjacent lines
The ~3x speedup comes from eliminating the gencodec shadow struct + appendCompact double-pass. The benchmark is in execution/types/encdec_test.go:BenchmarkLogJSON
Also -40% memory and -14% allocs across the board. All statistically significant (p=0.008).