diff --git a/CHANGELOG.md b/CHANGELOG.md index 95c3dcca40f..bf1cd289096 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Fix `engine_forkchoiceUpdatedV1` now returns `-38003 INVALID_PAYLOAD_ATTRIBUTES` for invalid payload attribute timestamps (zero or not greater than head). [#10353](https://github.com/besu-eth/besu/pull/10353) ### Additions and Improvements +- Add `enableReturnData` parameter to `debug_traceTransaction` and `debug_traceBlockByNumber`, and include `returnData` in `StructLog` when captured; the field is omitted when return data is empty or not captured. [#10172](https://github.com/besu-eth/besu/pull/10172) ## 26.5.0 diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParams.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParams.java index 3f5bb4ec6ac..f2ba9405812 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParams.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParams.java @@ -72,6 +72,13 @@ default boolean disableStack() { @JsonProperty(value = "limit") @Nullable Integer limit(); + @JsonProperty(value = "enableReturnData") + @Nullable Boolean enableReturnDataNullable(); + + default boolean enableReturnData() { + return Boolean.TRUE.equals(enableReturnDataNullable()); + } + @JsonProperty("tracer") @JsonInclude(JsonInclude.Include.NON_NULL) @Nullable String tracer(); @@ -135,6 +142,9 @@ default TraceOptions traceOptions() { if (limit() != null) { builder.limit(limit()); } + if (enableReturnDataNullable() != null) { + builder.traceReturnData(enableReturnData()); + } var opCodeTracerConfig = builder.traceOpcodes(opcodes()).build(); return new TraceOptions(tracerType, opCodeTracerConfig, tracerConfig(), stateOverrides()); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLog.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLog.java index 5bf18523ca5..d9d16cca7ec 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLog.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLog.java @@ -27,7 +27,18 @@ import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.units.bigints.UInt256; -@JsonPropertyOrder({"pc", "op", "gas", "gasCost", "depth", "refund", "stack", "memory", "storage"}) +@JsonPropertyOrder({ + "pc", + "op", + "gas", + "gasCost", + "depth", + "refund", + "stack", + "memory", + "returnData", + "storage" +}) public class StructLog { private static final char[] hexChars = "0123456789abcdef".toCharArray(); @@ -41,6 +52,7 @@ public class StructLog { private final String[] stack; private final Object storage; private final String reason; + private final String returnData; public StructLog(final TraceFrame traceFrame) { depth = traceFrame.getDepth() + 1; @@ -64,6 +76,7 @@ public StructLog(final TraceFrame traceFrame) { storage = traceFrame.getStorage().map(StructLog::formatStorage).orElse(null); reason = traceFrame.getRevertReason().map(bytes -> toCompactHex(bytes, true)).orElse(null); + returnData = traceFrame.getReturnData().map(Bytes::toHexString).orElse(null); } private static Map formatStorage(final Map storage) { @@ -145,6 +158,12 @@ public String reason() { return reason; } + @JsonGetter("returnData") + @JsonInclude(JsonInclude.Include.NON_NULL) + public String returnData() { + return returnData; + } + @Override public boolean equals(final Object o) { if (this == o) { diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParamsTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParamsTest.java index eaee3ce96c9..a6c4c2f16f1 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParamsTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/TransactionTraceParamsTest.java @@ -138,4 +138,36 @@ public void negativeLimitShouldFailDuringDeserialization() { assertThatThrownBy(() -> MAPPER.readValue("{\"limit\": -1}", TransactionTraceParams.class)) .hasMessageContaining("limit must be >= 0, got: -1"); } + + @Test + public void enableReturnDataTrueShouldSetTraceReturnData() throws Exception { + final TransactionTraceParams params = + MAPPER.readValue("{\"enableReturnData\": true}", TransactionTraceParams.class); + final OpCodeTracerConfig config = params.traceOptions().opCodeTracerConfig(); + + assertThat(config.traceReturnData()) + .describedAs("enableReturnData: true should set traceReturnData to true") + .isTrue(); + } + + @Test + public void enableReturnDataFalseShouldSetTraceReturnDataFalse() throws Exception { + final TransactionTraceParams params = + MAPPER.readValue("{\"enableReturnData\": false}", TransactionTraceParams.class); + final OpCodeTracerConfig config = params.traceOptions().opCodeTracerConfig(); + + assertThat(config.traceReturnData()) + .describedAs("enableReturnData: false should set traceReturnData to false") + .isFalse(); + } + + @Test + public void missingEnableReturnDataShouldDefaultToFalse() throws Exception { + final TransactionTraceParams params = MAPPER.readValue("{}", TransactionTraceParams.class); + final OpCodeTracerConfig config = params.traceOptions().opCodeTracerConfig(); + + assertThat(config.traceReturnData()) + .describedAs("traceReturnData should default to false when enableReturnData is absent") + .isFalse(); + } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLogTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLogTest.java index 07396fcdac6..3d7839774a7 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLogTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLogTest.java @@ -246,4 +246,50 @@ public void testToCompactHexWithLargeData() { String result = StructLog.toCompactHex(bytes, true); assertEquals("0x102030405060708090a", result, "Expected correct hex output for large data"); } + + // --- returnData tests --- + + @Test + public void returnDataShouldBePresentWhenCaptured() { + setupMinimalTraceFrameForReturnDataTests(); + when(traceFrame.getReturnData()).thenReturn(Optional.of(Bytes.fromHexString("0xdeadbeef"))); + + final StructLog log = new StructLog(traceFrame); + + assertThat(log.returnData()).isEqualTo("0xdeadbeef"); + } + + @Test + public void returnDataShouldBeNullWhenNotCaptured() { + setupMinimalTraceFrameForReturnDataTests(); + when(traceFrame.getReturnData()).thenReturn(Optional.empty()); + + final StructLog log = new StructLog(traceFrame); + + assertThat(log.returnData()).isNull(); + } + + @Test + public void returnDataShouldBeAbsentFromJsonWhenNotCaptured() throws Exception { + setupMinimalTraceFrameForReturnDataTests(); + when(traceFrame.getReturnData()).thenReturn(Optional.empty()); + + final StructLog log = new StructLog(traceFrame); + final String json = objectMapper.writeValueAsString(log); + + assertThat(json).doesNotContain("returnData"); + } + + private void setupMinimalTraceFrameForReturnDataTests() { + when(traceFrame.getDepth()).thenReturn(0); + when(traceFrame.getGasRemaining()).thenReturn(0L); + when(traceFrame.getGasCost()).thenReturn(OptionalLong.empty()); + when(traceFrame.getGasRefund()).thenReturn(0L); + when(traceFrame.getMemory()).thenReturn(Optional.empty()); + when(traceFrame.getOpcode()).thenReturn("PUSH1"); + when(traceFrame.getPc()).thenReturn(0); + when(traceFrame.getStack()).thenReturn(Optional.empty()); + when(traceFrame.getStorage()).thenReturn(Optional.empty()); + when(traceFrame.getRevertReason()).thenReturn(Optional.empty()); + } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracer.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracer.java index c35e8fc2a17..44aef097f17 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracer.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracer.java @@ -86,6 +86,7 @@ public void tracePostExecution(final MessageFrame frame, final OperationResult o && currentOperation instanceof AbstractCreateOperation ? forceCaptureMem(frame) : Optional.empty()); + final Optional returnData = captureReturnData(frame); final Optional stackPostExecution = captureStack(frame); if (!traceFrames.isEmpty()) { @@ -122,6 +123,7 @@ public void tracePostExecution(final MessageFrame frame, final OperationResult o .setValue(frame.getApparentValue()) .setInputData(inputData) .setOutputData(outputData) + .setReturnData(returnData) .setStack(preExecutionStack) .setMemory(memory) .setStorage(storage) @@ -234,6 +236,16 @@ private void addNewTraceFrame( traceFrames.add(traceFrame); } + private Optional captureReturnData(final MessageFrame frame) { + if (!options.traceReturnData()) { + return Optional.empty(); + } + final Bytes returnData = frame.getReturnData(); + return (returnData == null || returnData.isEmpty()) + ? Optional.empty() + : Optional.of(returnData); + } + private Optional> captureStorage(final MessageFrame frame) { if (!options.traceStorage()) { return Optional.empty(); diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracerTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracerTest.java index 024744abbbd..ceaf374a4d7 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracerTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracerTest.java @@ -222,6 +222,52 @@ void shouldNotRecordStorageWhenDisabled() { assertThat(traceFrame.getStorage()).isEmpty(); } + @Test + void shouldRecordReturnDataWhenEnabled() { + final MessageFrame frame = validMessageFrame(); + setupStorageForCapture(frame); + frame.setReturnData(Bytes.fromHexString("0xdeadbeef")); + final TraceFrame traceFrame = + traceFrame( + frame, + OpCodeTracerConfigBuilder.createFrom(OpCodeTracerConfig.DEFAULT) + .traceReturnData(true) + .build(), + false); + assertThat(traceFrame.getReturnData()).isPresent(); + assertThat(traceFrame.getReturnData().get()).isEqualTo(Bytes.fromHexString("0xdeadbeef")); + } + + @Test + void shouldRecordEmptyReturnDataWhenEnabled() { + final MessageFrame frame = validMessageFrame(); + setupStorageForCapture(frame); + frame.setReturnData(Bytes.EMPTY); + final TraceFrame traceFrame = + traceFrame( + frame, + OpCodeTracerConfigBuilder.createFrom(OpCodeTracerConfig.DEFAULT) + .traceReturnData(true) + .build(), + false); + assertThat(traceFrame.getReturnData()).isEmpty(); + } + + @Test + void shouldNotRecordReturnDataWhenDisabled() { + final MessageFrame frame = validMessageFrame(); + setupStorageForCapture(frame); + frame.setReturnData(Bytes.fromHexString("0xdeadbeef")); + final TraceFrame traceFrame = + traceFrame( + frame, + OpCodeTracerConfigBuilder.createFrom(OpCodeTracerConfig.DEFAULT) + .traceReturnData(false) + .build(), + false); + assertThat(traceFrame.getReturnData()).isEmpty(); + } + @Test void shouldNotAddGasWhenDisabled() { final TraceFrame traceFrame = diff --git a/evm/src/main/java/org/hyperledger/besu/evm/tracing/TraceFrame.java b/evm/src/main/java/org/hyperledger/besu/evm/tracing/TraceFrame.java index 2fe465520e1..8128912da49 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/tracing/TraceFrame.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/tracing/TraceFrame.java @@ -68,6 +68,7 @@ public class TraceFrame { private final Optional softFailureReason; private final OptionalLong gasAvailableForChildCall; + private final Optional returnData; /** Private constructor - only accessible through Builder */ private TraceFrame(final Builder builder) { @@ -103,6 +104,7 @@ private TraceFrame(final Builder builder) { this.precompileOutputData = builder.precompileOutputData; this.softFailureReason = builder.softFailureReason; this.gasAvailableForChildCall = builder.gasAvailableForChildCall; + this.returnData = builder.returnData; } /** @@ -158,6 +160,7 @@ public static class Builder { private Optional precompileOutputData = Optional.empty(); private Optional softFailureReason = Optional.empty(); private OptionalLong gasAvailableForChildCall = OptionalLong.empty(); + private Optional returnData = Optional.empty(); /** Default constructor */ public Builder() {} @@ -200,6 +203,7 @@ public Builder(final TraceFrame traceFrame) { this.precompileOutputData = traceFrame.precompileOutputData; this.softFailureReason = traceFrame.softFailureReason; this.gasAvailableForChildCall = traceFrame.gasAvailableForChildCall; + this.returnData = traceFrame.returnData; } /** @@ -575,6 +579,17 @@ public Builder setGasAvailableForChildCall(final OptionalLong gasAvailableForChi return this; } + /** + * Sets the return data buffer captured after the opcode executed. + * + * @param returnData the return data buffer contents + * @return this builder instance for method chaining + */ + public Builder setReturnData(final Optional returnData) { + this.returnData = returnData; + return this; + } + /** * Builds the TraceFrame instance. * @@ -874,6 +889,15 @@ public OptionalLong getGasAvailableForChildCall() { return gasAvailableForChildCall; } + /** + * Return data buffer at the time this opcode was traced. + * + * @return the return data buffer, or empty if not captured + */ + public Optional getReturnData() { + return returnData; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) @@ -886,6 +910,7 @@ public String toString() { .add("stack", stack) .add("memory", memory) .add("storage", storage) + .add("returnData", returnData) .toString(); } }