EIP-7756 Trace updates #8006

shemnon · 2024-12-09T17:08:53Z

PR description

Update tracing for EIP-7756 changes, mostly swapping hex format and numbers for opcode and gas members.

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

Checked out our contribution guidelines?
Considered documentation and added the doc-change-required label to this PR if updates are required.
Considered the changelog and included an update if required.
For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

unit tests: ./gradlew build
acceptance tests: ./gradlew acceptanceTest
integration tests: ./gradlew integrationTest
reference tests: ./gradlew ethereum:referenceTests:referenceTests

...ain/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/processor/TransactionTracer.java

github-actions · 2025-02-02T02:01:21Z

This pr is stale because it has been open for 30 days with no activity.

github-actions · 2025-03-21T02:05:53Z

This pr is stale because it has been open for 30 days with no activity.

ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java

lu-pinto · 2025-03-24T10:27:36Z

evm/src/main/java/org/hyperledger/besu/evm/tracing/StandardJsonTracer.java

+    if (eip3155strict) {
+      sb.append("\"op\":").append(opcode).append(",");
+    } else {
+      sb.append("\"op\":\"").append(fastHexByte(opcode)).append("\",");


Correct me if I'm wrong but fastHexByte is the same as "0x" + Integer.toHexString(opcode) why not use JDK features?

fastHexByte only works on a single byte, and only does hex, and always keeps the leading zero, producing exactly two characters. The Integer alternative may do up to 4 character and does not keep the leading zero.

shemnon · 2025-03-31T21:31:02Z

@lu-pinto Ready for review. The defaults are to do 3155 tracing data, but I am needing the new EOF elements for fuzzing in the main branch.

lu-pinto · 2025-04-01T16:00:18Z

ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java

@@ -199,6 +199,13 @@ void setBytes(final String optionValue) {
      scope = INHERIT)
  final Boolean showJsonAlloc = false;

+  @Option(
+      names = {"--eip-3155", "--trace.noeip-3155"},


is --trace.noeip-3155 really required or intencional? Isn't it --trace.eip-3155 instead?
The negative version --no-trace.noeip-3155 looks kinda weird

The negation is --noeip-3155 - will update code. There are wierd things going on with picocli and negation, but the end result is that --eip-3155 and --noeip3155 have the expected impact.

lu-pinto · 2025-04-01T16:01:45Z

@lu-pinto Ready for review. The defaults are to do 3155 tracing data, but I am needing the new EOF elements for fuzzing in the main branch.

Yeah no probs with that, current version looks alright to me. Just made a small remark regarding the 2nd name of the flag

Update tracing for EIP-7756 changes, mostly swapping hex format and numbers for opcode and gas members. Signed-off-by: Danno Ferrin <danno@numisight.com>

EIP-3155 and EIP-7756 call for "stateRoot" where we use "postHash". Signed-off-by: Danno Ferrin <danno@numisight.com>

Signed-off-by: Danno Ferrin <danno@numisight.com>

Change the defaults so EIP-7756 trace variations have to be explicitly enabled for now. Signed-off-by: Danno Ferrin <danno@numisight.com>

Signed-off-by: Danno Ferrin <danno@numisight.com>

lu-pinto · 2025-04-07T16:17:56Z

@lu-pinto Ready for review. The defaults are to do 3155 tracing data, but I am needing the new EOF elements for fuzzing in the main branch.

Yeah no probs with that, current version looks alright to me. Just made a small remark regarding the 2nd name of the flag

I see you updated it to:

   @Option(
      names = {"--noeip-3155", "--trace.noeip-3155"},
      description = "Produce a trace with types strictly compatible with EIP-3155.",

But it doesn't feel very intuitive to me, I think you should have --eip-3155 and --trace.eip-3155 because that's what the variable actually means from the description there. From the documentation https://picocli.info/#_negatable_options it seems that it's expected to work that way. Is that a bug in picocli or something? Is it because you are not using the primitive boolean that the negation don't work as expected?

shemnon · 2025-04-07T21:48:53Z

Whether it is a bug (as most people think) or a design choice (what PicoCLI calls it) "flag" type PicoCLI options need to have the value when not specified be the default value, and when specified be the parsed value. Because the default is true we need to specify the negation as the PicoCLI value. And once Fusaka ships I intend to flip the default, so keeping the API "--trace.eip-3155" to enable backwards compatibility mode is the goal.

Try changing the option names yourself and watch the tests break. PicoCLI is bizarre in this sense.

You can see it works itself out in the help text.

Usage: evm state-test [-hV] [--[no]intrinsic-gas] [--json] [--json-alloc] [--
                      [no]memory] [--[no]eip-3155] [--[no]time] [--trace.[no]
                      stack] [--trace.[no]returndata] [--trace.[no]storage]
                      [--data-index=<dataIndex>] [--fork=<fork>]
                      [--fork-index=<forkIndex>] [--gas-index=<gasIndex>]
                      [--test-name=<testName>] [--value-index=<valueIndex>]
                      [<stateTestFiles>...]
Execute an Ethereum State Test.
      [<stateTestFiles>...]
      --data-index=<dataIndex>
                            Limit execution to one data variable.
      --fork=<fork>         Force the state tests to run on a specific fork.
      --fork-index=<forkIndex>
                            Limit execution to one fork.
      --gas-index=<gasIndex>
                            Limit execution to one gas variable.
  -h, --help                Show this help message and exit.
      --[no]intrinsic-gas   Calculate and charge intrinsic and tx content gas.
                              Default is not to charge.
      --json, --trace       Trace each opcode as a json object.
      --json-alloc          Output the final allocations after a run.
      --[no]memory, --trace.[no]memory
                            Show the full memory output in tracing for each op.
                              Default is not to show memory.
      --[no]eip-3155, --trace.[no]eip-3155
                            Produce a trace with types strictly compatible with
                              EIP-3155.
      --[no]time            Don't include time data in summary output.
      --test-name=<testName>
                            Limit execution to one named test.
      --trace.[no]stack     Show the operand stack in tracing for each op.
                              Default is to show stack.
      --trace.[no]returndata
                            Show the return data in tracing for each op when
                              present. Default is to show return data.
      --trace.[no]storage   Show the updated storage slots for the current
                              account. Default is to not show updated storage.
  -V, --version             Print version information and exit.
      --value-index=<valueIndex>
                            Limit execution to one value variable.

shemnon · 2025-04-07T21:52:41Z

I would like to re-iterate this PR is needed for fuzz testing EOF in Fusaka. All the tests pass and the options are all exercised. To the best of my ability to tweak it, it appears the PicoCLI documentation is leading to an incorrect understanding of what their code actually does.

shemnon · 2025-04-08T16:29:56Z

Thanks!

shemnon commented Dec 9, 2024

View reviewed changes

...ain/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/processor/TransactionTracer.java Show resolved Hide resolved

shemnon mentioned this pull request Dec 15, 2024

Non-osaka EOF changes holiman/goevmlab#179

Merged

shemnon force-pushed the eip-7756/gas-as-numbers branch from 1470a95 to 338e491 Compare December 15, 2024 04:02

shemnon marked this pull request as draft December 15, 2024 04:07

github-actions bot added the Stale label Feb 2, 2025

shemnon force-pushed the eip-7756/gas-as-numbers branch from 4a767bc to d656bdd Compare February 2, 2025 07:11

github-actions bot removed the Stale label Feb 3, 2025

shemnon force-pushed the eip-7756/gas-as-numbers branch from d656bdd to 21d844a Compare February 18, 2025 15:50

github-actions bot added the Stale label Mar 21, 2025

lu-pinto reviewed Mar 24, 2025

View reviewed changes

ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EvmToolCommand.java Outdated Show resolved Hide resolved

lu-pinto reviewed Mar 24, 2025

View reviewed changes

lu-pinto removed the Stale label Mar 24, 2025

macfarla added the Fusaka label Mar 25, 2025

shemnon force-pushed the eip-7756/gas-as-numbers branch from 21d844a to a5e734c Compare March 31, 2025 16:18

shemnon marked this pull request as ready for review March 31, 2025 21:28

lu-pinto reviewed Apr 1, 2025

View reviewed changes

shemnon added 9 commits April 1, 2025 12:55

EIP-7756 Trace updates

4ceb900

Update tracing for EIP-7756 changes, mostly swapping hex format and numbers for opcode and gas members. Signed-off-by: Danno Ferrin <danno@numisight.com>

postHash -> stateRoot

9b7583c

EIP-3155 and EIP-7756 call for "stateRoot" where we use "postHash". Signed-off-by: Danno Ferrin <danno@numisight.com>

functionDepth = 1 for base level EOF

e2cd9f0

Signed-off-by: Danno Ferrin <danno@numisight.com>

spotless

bf28fc0

Signed-off-by: Danno Ferrin <danno@numisight.com>

update test values

c9a9814

Signed-off-by: Danno Ferrin <danno@numisight.com>

nonce and balance in summary.

c11ff41

Signed-off-by: Danno Ferrin <danno@numisight.com>

rebase error

8cb6418

Signed-off-by: Danno Ferrin <danno@numisight.com>

change defaults

2560d41

Change the defaults so EIP-7756 trace variations have to be explicitly enabled for now. Signed-off-by: Danno Ferrin <danno@numisight.com>

missed a negation

5251020

Signed-off-by: Danno Ferrin <danno@numisight.com>

shemnon force-pushed the eip-7756/gas-as-numbers branch from 000f804 to 5251020 Compare April 1, 2025 18:55

Merge branch 'main' into eip-7756/gas-as-numbers

859f1ac

lu-pinto approved these changes Apr 8, 2025

View reviewed changes

Merge branch 'main' into eip-7756/gas-as-numbers

fd70882

lu-pinto enabled auto-merge (squash) April 8, 2025 14:46

Merge branch 'main' into eip-7756/gas-as-numbers

a5609e6

lu-pinto merged commit 31bf5b4 into hyperledger:main Apr 8, 2025
43 checks passed

jflo moved this to Todo in Osaka May 1, 2025

jflo added this to Osaka May 1, 2025

jflo moved this from Todo to In Progress in Osaka May 1, 2025

jflo moved this from In Progress to Done in Osaka May 8, 2025

macfarla added the Osaka Osaka fork related - part of Fusaka label May 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

EIP-7756 Trace updates #8006

EIP-7756 Trace updates #8006

Uh oh!

shemnon commented Dec 9, 2024

Uh oh!

Uh oh!

github-actions bot commented Feb 2, 2025

Uh oh!

github-actions bot commented Mar 21, 2025

Uh oh!

Uh oh!

lu-pinto Mar 24, 2025 •

edited

Loading

Uh oh!

shemnon Apr 1, 2025

Uh oh!

shemnon commented Mar 31, 2025

Uh oh!

lu-pinto Apr 1, 2025

Uh oh!

shemnon Apr 1, 2025

Uh oh!

lu-pinto commented Apr 1, 2025

Uh oh!

lu-pinto commented Apr 7, 2025

Uh oh!

shemnon commented Apr 7, 2025

Uh oh!

shemnon commented Apr 7, 2025

Uh oh!

Uh oh!

shemnon commented Apr 8, 2025

Uh oh!

Uh oh!

EIP-7756 Trace updates #8006

EIP-7756 Trace updates #8006

Uh oh!

Conversation

shemnon commented Dec 9, 2024

PR description

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

Locally, you can run these tests to catch failures early:

Uh oh!

Uh oh!

github-actions bot commented Feb 2, 2025

Uh oh!

github-actions bot commented Mar 21, 2025

Uh oh!

Uh oh!

lu-pinto Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shemnon Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

shemnon commented Mar 31, 2025

Uh oh!

lu-pinto Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

shemnon Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

lu-pinto commented Apr 1, 2025

Uh oh!

lu-pinto commented Apr 7, 2025

Uh oh!

shemnon commented Apr 7, 2025

Uh oh!

shemnon commented Apr 7, 2025

Uh oh!

Uh oh!

shemnon commented Apr 8, 2025

Uh oh!

Uh oh!

lu-pinto Mar 24, 2025 •

edited

Loading