feat(debug): add enableReturnData param and returnData field by sagarkhandagre998 · Pull Request #10172 · besu-eth/besu

sagarkhandagre998 · 2026-04-03T03:45:32Z

PR description

Add enableReturnData param and returnData field

Part of aligning Besu's debug RPC endpoints with the execution-apis spec.

What changed:

TransactionTraceParams — new optional enableReturnData boolean param (default false)
DebugOperationTracer — captures frame.getReturnData() per opcode when enabled
TraceFrame — carries the captured return data buffer through the trace pipeline
StructLog — serializes returnData as a hex string; field is omitted from JSON when not enabled

Behaviour:

When enableReturnData: false (default) — no change to existing responses
When enableReturnData: true — each StructLog entry includes a returnData field containing the EVM return data buffer (populated after CALL/CREATE/etc.) as a 0x-prefixed hex string

Fixed Issue(s)

Refs #10115

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:

spotless: ./gradlew spotlessApply
unit tests: ./gradlew build
acceptance tests: ./gradlew acceptanceTest
integration tests: ./gradlew integrationTest
reference tests: ./gradlew ethereum:referenceTests:referenceTests
hive tests: Engine or other RPCs modified?

sagarkhandagre998 · 2026-04-04T04:10:50Z

@macfarla Kindly take a look at once.

parthdagia05 · 2026-04-10T12:29:55Z

This PR doesn't include any unit tests. Would be good to add at least: a test that returnData is captured or absent.
Just want to check when TraceFrame.from(lastTraceFrame) is used in DebugOperationTracer to backfill gasRemainingPostExecution, it copies all fields from the old frame into a new one. Is the new returnData field included in that copy?
@sagarkhandagre998

sagarkhandagre998 · 2026-04-10T13:35:28Z

This PR doesn't include any unit tests. Would be good to add at least: a test that returnData is captured or absent. Just want to check when TraceFrame.from(lastTraceFrame) is used in DebugOperationTracer to backfill gasRemainingPostExecution, it copies all fields from the old frame into a new one. Is the new returnData field included in that copy? @sagarkhandagre998

Thanks @parthdagia05 for the review.
I think we need tests by following the priority.

Test file	What's missing	Priority
`DebugOperationTracerTest`	`shouldRecordReturnDataWhenEnabled` + `shouldNotRecordReturnDataWhenDisabled`	High
`StructLogTest`	`returnData` field serialized/absent in JSON, value mapping	Medium
`TransactionTraceParamsTest`	`enableReturnData` deserialization + default false	Low

parthdagia05 · 2026-04-10T15:06:41Z

@sagarkhandagre998
A few things still missing in StructLog.java:

@JsonPropertyOrder needs returnData without it, Jackson serializes the field at an arbitrary position, breaking spec-expected ordering:

@JsonPropertyOrder({"pc", "op", "gas", "gasCost", "depth", "refund", "stack", "memory", "returnData", "storage"})
equals() and hashCode() need updating the new returnData field isn't included in either method, so two StructLog objects differing only by returnData would incorrectly be considered equal.

sagarkhandagre998 · 2026-04-11T12:49:48Z

@sagarkhandagre998 A few things still missing in StructLog.java:

@JsonPropertyOrder needs returnData without it, Jackson serializes the field at an arbitrary position, breaking spec-expected ordering:

@JsonPropertyOrder({"pc", "op", "gas", "gasCost", "depth", "refund", "stack", "memory", "returnData", "storage"}) equals() and hashCode() need updating the new returnData field isn't included in either method, so two StructLog objects differing only by returnData would incorrectly be considered equal.

equals() and hashCode() on StructLog are only used for test assertions, not in any production logic. The fields included — pc, op, gas, gasCost, depth, memory, stack, storage — represent the EVM execution state at the point of tracing. reason was deliberately left out of both methods for the same reason returnData is: they are conditional output fields populated only on specific opcodes, not part of the core state being compared. Adding returnData here would be inconsistent. The @JsonPropertyOrder fix is valid though — will fix that.

@jframe is this correct behavior ?

macfarla

some comments, please take a look

sagarkhandagre998 · 2026-04-22T06:13:10Z

@macfarla Kindly take a look

macfarla

prob worth a changelog entry

sagarkhandagre998 · 2026-04-29T16:33:20Z

@macfarla All Done , Please take a look.

macfarla

@sagarkhandagre998 spotless check is failing. before you ask for review, run gradlew build locally and ensure code builds and passes tests. reviewer's time is precious and limited.

macfarla · 2026-05-07T08:08:28Z

@sagarkhandagre998 can you merge latest main and resolve conflicts

sagarkhandagre998 · 2026-05-07T08:15:18Z

@sagarkhandagre998 can you merge latest main and resolve conflicts

@macfarla Done.

macfarla · 2026-05-08T02:37:45Z

@sagarkhandagre998 I'm going to run the checks but you'll also need to resolve conflicts in changelog

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

…nsures empty return buffer is omitted even when traceReturnData is enabled Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

…tests. Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

sagarkhandagre998 · 2026-05-08T03:48:42Z

@sagarkhandagre998 I'm going to run the checks but you'll also need to resolve conflicts in changelog

@macfarla we can run the checks

…h#10172) * implements test by priority Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com> * add returnData field in the correct order - EIP-3155 Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com> * applying a targeted fix in captureReturnData and adding a test that ensures empty return buffer is omitted even when traceReturnData is enabled Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com> --------- Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: abhay-dev2901 <abhaytp1998@gmail.com>

sagarkhandagre998 force-pushed the issue-10115 branch from 34157f0 to eccea00 Compare April 3, 2026 03:48

sagarkhandagre998 mentioned this pull request Apr 3, 2026

debug RPC endpoints standardization #10115

Closed

sagarkhandagre998 commented Apr 10, 2026

View reviewed changes

Comment thread evm/src/main/java/org/hyperledger/besu/evm/tracing/TraceFrame.java

sagarkhandagre998 force-pushed the issue-10115 branch from 50a99e3 to b5de795 Compare April 10, 2026 14:22

sagarkhandagre998 force-pushed the issue-10115 branch 2 times, most recently from 03b7e20 to 32b2fa6 Compare April 11, 2026 13:15

macfarla reviewed Apr 21, 2026

View reviewed changes

Comment thread ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracer.java

Comment thread ...rg/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParamsTest.java

macfarla assigned sagarkhandagre998 Apr 21, 2026

macfarla marked this pull request as draft April 21, 2026 21:29

sagarkhandagre998 force-pushed the issue-10115 branch 3 times, most recently from b7e5253 to f286618 Compare April 22, 2026 06:12

sagarkhandagre998 requested a review from macfarla April 22, 2026 06:13

sagarkhandagre998 marked this pull request as ready for review April 22, 2026 06:13

macfarla assigned macfarla and unassigned sagarkhandagre998 Apr 27, 2026

macfarla reviewed Apr 28, 2026

View reviewed changes

macfarla assigned sagarkhandagre998 and unassigned macfarla Apr 28, 2026

sagarkhandagre998 force-pushed the issue-10115 branch 2 times, most recently from 6a0baf0 to f4c7489 Compare April 29, 2026 16:28

sagarkhandagre998 requested a review from macfarla April 29, 2026 16:33

macfarla previously requested changes Apr 30, 2026

View reviewed changes

sagarkhandagre998 requested a review from macfarla May 7, 2026 05:17

macfarla assigned macfarla and unassigned sagarkhandagre998 May 7, 2026

macfarla assigned sagarkhandagre998 and unassigned macfarla May 7, 2026

sagarkhandagre998 force-pushed the issue-10115 branch from 41cc7d3 to a216246 Compare May 7, 2026 08:14

macfarla self-assigned this May 8, 2026

sagarkhandagre998 added 10 commits May 8, 2026 08:56

resolving conflicts

1a334af

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

apply formatting

e043603

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

implements test by priority

701e86e

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

add returnData field in the correct order - EIP-3155

3efcc57

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

applying a targeted fix in captureReturnData and adding a test that e…

8e4ca0b

…nsures empty return buffer is omitted even when traceReturnData is enabled Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

Keep DEFAULT + traceReturnData(...) only for the returnData-specific …

3bdfb44

…tests. Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

adding returnData in toString()

f76fb33

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

reduce duplication in test

88dc885

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

fix

9040824

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

apply spotless

626a531

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

sagarkhandagre998 force-pushed the issue-10115 branch from a216246 to 1fd88ab Compare May 8, 2026 03:29

sagarkhandagre998 added 2 commits May 8, 2026 09:14

adding change log

4760074

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

apply suggested changes

9bddf72

Signed-off-by: Sagar Khandagre <sagar.khandagre998@gmail.com>

sagarkhandagre998 force-pushed the issue-10115 branch from 7f90075 to 9bddf72 Compare May 8, 2026 03:45

macfarla added the doc-change-required Indicates an issue or PR that requires doc to be updated label May 8, 2026

Merge branch 'main' into issue-10115

b191d40

macfarla approved these changes May 8, 2026

View reviewed changes

macfarla merged commit fbbd79e into besu-eth:main May 8, 2026
34 checks passed

Conversation

sagarkhandagre998 commented Apr 3, 2026 • edited by macfarla Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

sagarkhandagre998 commented Apr 4, 2026

Uh oh!

parthdagia05 commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sagarkhandagre998 commented Apr 10, 2026

Uh oh!

parthdagia05 commented Apr 10, 2026

Uh oh!

sagarkhandagre998 commented Apr 11, 2026

Uh oh!

macfarla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sagarkhandagre998 commented Apr 22, 2026

Uh oh!

macfarla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sagarkhandagre998 commented Apr 29, 2026

Uh oh!

macfarla left a comment

Choose a reason for hiding this comment

Uh oh!

macfarla commented May 7, 2026

Uh oh!

sagarkhandagre998 commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

macfarla commented May 8, 2026

Uh oh!

sagarkhandagre998 commented May 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sagarkhandagre998 commented Apr 3, 2026 •

edited by macfarla

Loading

parthdagia05 commented Apr 10, 2026 •

edited

Loading

sagarkhandagre998 commented May 7, 2026 •

edited

Loading