Standardize opcode tracer behavior for debug_traceBlockByNumber and debug_traceTransaction#762
Standardize opcode tracer behavior for debug_traceBlockByNumber and debug_traceTransaction#762MysticRyuujin wants to merge 8 commits intoethereum:mainfrom
Conversation
src/schemas/opcode-tracer.yaml
Outdated
| minimum: 0 | ||
| gasCost: | ||
| title: gas cost | ||
| description: The gas cost charged for executing this opcode. |
There was a problem hiding this comment.
This is not well specified. Does it mean the amount of gas to be burned by successful execution of the instruction?
There was a problem hiding this comment.
I updated the description to reflect the intent, though I'm not 100% sure if this wording is correct? I think it is.
There was a problem hiding this comment.
In geth, OnOpcode is called before the opcode executes, receiving the pre-computed cost as a parameter. gasCost is that value — the amount that will be deducted from the remaining gas assuming successful execution, including dynamic costs such as memory expansion and cold/warm storage access charges (EIP-2929). So yeah, I think the updated description is accurate.
There was a problem hiding this comment.
I think the main issue is in CALL instructions. I think gasCost should exclude the amount of gas passed down to the callee and only report the amount being burned as the instruction cost. This way the sum of all gasCost entries should give you the final gas used amount. cc @MariusVanDerWijden.
There was a problem hiding this comment.
Idea about sum being equal to the final used amount looks interesting, but want to highlight a few points:
- For formula to hold CALL
gasCostwon't include any "execution" gas, as it will already be accounted for in "inner" opcodes. Only base call gas, memory expansion, value transfer, and account creation fees. This may be a bit counterintuitive. - From a brief check none of the top clients have such exact implementation, so this will require a change from all of them. Also most clients take full forwarded gas as
gasCost(without deducting amount returned by the callee). May be wrong here so.
+1 from Erigon |
…nd debug_traceTransaction Made-with: Cursor
Made-with: Cursor
The reexec field is an implementation detail of geth's state reconstruction and does not belong in the standardized trace configuration.
Update the replace directive to use the fix-legacy-struct-log-json branch commit (c787778ed44a) from the MysticRyuujin fork, which fixes legacy structLog JSON encoding in the struct logger tracer.
MysticRyuujin
force-pushed
the
opcode-tracer-spec
branch
from
b2f7439 to
4805dab
Compare
macfarla
left a comment
macfarla
left a comment
There was a problem hiding this comment.
the execution timeout is non-trivial to implement in Besu. would prefer this to be a nice to have rather than a must-have. everything else is fine.
| each StructLog entry. Ignored when tracer is set. | ||
| Default: false. | ||
| type: boolean | ||
| limit: |
There was a problem hiding this comment.
medium effort to implement this in Besu since we don't have this currently
| $ref: '#/components/schemas/TraceConfig' | ||
| errors: | ||
| - code: 4444 | ||
| message: Pruned history unavailable |
There was a problem hiding this comment.
probably also an error for "block not found" and parent not found
…pdate go-ethereum - Add debug_traceBlockByHash method spec (mirrors debug_traceBlockByNumber but takes a block hash; clients MUST error when block not found) - Add timeout error-on-timeout behavior to TraceConfig and method descriptions - Update OpcodeBlockTransactionTrace description to reference both block trace methods - Add DebugTraceBlockByHash test generator (trace-block-with-transactions, trace-genesis, trace-block-not-found) - Update tools/go.mod replace to MysticRyuujin fork commit 957e1ed413ed (includes gofmt fix for formatMemoryWord) - Regenerate tests/debug_traceBlockByHash/ via make fill Made-with: Cursor
Clients that do not support execution timeouts (e.g. streaming JSON-RPC implementations) MAY ignore the timeout field. When a timeout is supported and reached, the client MUST still return an error with no partial results. Made-with: Cursor
This is a breaking change in the opcode (structLog) tracer. Several fields will have a slight formatting difference to conform to the newly established spec at: ethereum/execution-apis#762. The differences include: - `memory`: words will have the 0x prefix. Also last word of memory will be padded to 32-bytes. - `storage`: keys and values will have the 0x prefix. --------- Co-authored-by: Sina M <1591639+s1na@users.noreply.github.com>
|
go-ethereum PR was merged but there is no release, go.mod needs to be updated |
| description: >- | ||
| The contents of EVM memory at this step, divided into 32-byte chunks. | ||
| Each element is a 0x-prefixed, zero-padded 32-byte hex word representing | ||
| a consecutive 32-byte memory slot starting at offset (index * 32). |
There was a problem hiding this comment.
When stack, storage and memory is enabled, we've seen block traces on mainnet which were bigger than 47GB and that was even without padding. I think we should try to reduce the trace size as much as possible and not do any padding
There was a problem hiding this comment.
Isn't the padding in practice only for the last word, which may be partial? The overhead seems low to me.
| The contents of EVM memory at this step, divided into 32-byte chunks. | ||
| Each element is a 0x-prefixed, zero-padded 32-byte hex word representing | ||
| a consecutive 32-byte memory slot starting at offset (index * 32). | ||
| This field is absent (not null) when enableMemory is false. |
There was a problem hiding this comment.
Similar to storage we should only populate this for op codes that touch memory to reduce the trace size
There was a problem hiding this comment.
Looking through the Besu codebase the following op codes touch memory:
MLOAD, KECCAK256, LOG0–LOG4, RETURN, REVERT, MSIZE, MSTORE, MSTORE8, CALLDATACOPY, CODECOPY, EXTCODECOPY, RETURNDATACOPY, CALL, CALLCODE, DELEGATECALL, STATICCALL, CREATE, CREATE2, MCOPY
There was a problem hiding this comment.
We discussed a similar idea which didn't end up happening because streaming traces alleviated some of the issue.
It is the same in spirit as to what you're suggesting with the difference that we focus on opcodes that change the memory (and anytime you enter a new scope) and at that point output the full memory.
There was a problem hiding this comment.
I just run tests on debug_traceBlockByNumber with memory tracing enabled, on mainnet, and saw a block that generated 643 GiB of json output :
0x17AEF7D 658854.28MB
this is just to show case that we need to optimize memory tracing, and to support @daniellehrner's idea.
There was a problem hiding this comment.
Maybe it's not worth to standardize this part then.
There was a problem hiding this comment.
I think it makes even more sense to standardize it to be sure that debug_trace* calls generate reasonable amount of data. So basically, we need to reduce the impact of memory tracing on the json output size.
There was a problem hiding this comment.
I'd like to point out that: 1) it is disabled by default and won't bloat the trace, 2) there are few use-cases that require memory.
That said, I fully support optimizing the impact of memory, either in this or a future PR. I'd propose a more granular flag than enableMemory for it. An enum which can support a range of values like none, full, onChange. Because there are multiple ways to optimise it, you can even go as far as only outputting the diff compared to last change.
There is a way to do this in a backwards compatible way too. If we pick full as the default value, then when enableMemory is toggled it behaves the same way it does today.
10 participants
Note: PR is in draft to gather feedback, and so I can continue testing go-ethereum via rpctestgen
Summary
This PR adds OpenRPC method and schema definitions for:
debug_traceTransactiondebug_traceBlockByNumberdebug_traceBlockByHashScope is intentionally limited to the default opcode (struct) logger output, while still allowing named tracers through
TraceConfig.tracer.It is also based on the investigation done by ethPandaOps on trace-comparisons
Goal
The goal of this PR is to standardize the existing live
debug_trace*opcode tracer contract as one canonical cross-client output:Non-goals
This PR does not:
EIP-3155conformanceIf the ecosystem wants a cleaner or more opinionated trace format than the current live RPC methods provide, that should be a separate effort.
Preserve vs normalize
Preserved from current live RPC practice
gas,failed,returnValue,structLogsopas a human-readable opcode name stringgasandgasCostas JSON integersStandardized by this PR
0x-prefixeduint256quantities0x-prefixedbytes320x-prefixedbytes32errormust be absent when no error occurredreturnDatamust be absent when disabled and valid hex when enabledstructLogs: []{ txHash, result }entriesSLOADandSSTORERelationship to
EIP-3155/EIP-7756This PR borrows the useful parts of the EIP work, but it does not adopt those specs literally.
The main intentional differences are:
output/gasUsed/passgas/gasCostopas a string namestateRoot,time,fork,memSize, and named tracer output schemas out of scopeKey field-level differences
EIP-3155/EIP-7756output,gasUsed,passreturnValue,gas,failedopopNamegas/gasCostmemorybytes32[]stateRoot,time,fork,memSizeStorage semantics
This PR standardizes storage behavior as part of the canonical output:
0x-prefixedbytes32SLOADandSSTOREnullor{}, at all other opcodesLikely client changes
Based on the current tests, the existing PR investigation, and the ethPandaOps comparison work, the most likely alignment work is:
Geth0xprefix to memory chunks and storage keys/values in opcode tracesErigontxHashpairing match the specBesuNetherminderror/storagefields where required and align storage timing/outputRethreturnDatagating and verify storage timing/encoding behaviorThis table is a concrete starting point for client review, not a claim of fully validated implementation diffs across every client version.
Tests
This PR adds focused tests for:
debug_traceTransactionstructLogs: []debug_traceBlockByNumber{ txHash, result }responsesreturnDatagating behaviordebug_traceBlockByHashMost large trace fixtures are
SpecOnly, meaning they validate the standardized wire contract rather than replaying a recorded byte-for-byte dump from one client.That is intentional: the purpose of this PR is to standardize one canonical response shape and semantics for the default opcode tracer, while avoiding overfitting the tests to incidental formatting outside the specified contract.
Recorded fixtures are still used where the expected behavior is small and unambiguous, such as:
Coverage still incomplete
Current coverage still does not fully prove:
refundpresence when non-zeroThe proposal here is still to define one canonical target; this section simply identifies the areas where additional fixtures or client feedback may still be useful.
Current validation status
make build: passgo test ./...intools/: passmake fill: fail (need go-ethereum to conform to the spec as written here)- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -