Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand All @@ -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<String, String> formatStorage(final Map<UInt256, UInt256> storage) {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Comment thread
sagarkhandagre998 marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")));
Comment thread
sagarkhandagre998 marked this conversation as resolved.

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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public void tracePostExecution(final MessageFrame frame, final OperationResult o
&& currentOperation instanceof AbstractCreateOperation
? forceCaptureMem(frame)
: Optional.empty());
final Optional<Bytes> returnData = captureReturnData(frame);
final Optional<Bytes[]> stackPostExecution = captureStack(frame);

if (!traceFrames.isEmpty()) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -234,6 +236,16 @@ private void addNewTraceFrame(
traceFrames.add(traceFrame);
}

private Optional<Bytes> 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<Map<UInt256, UInt256>> captureStorage(final MessageFrame frame) {
if (!options.traceStorage()) {
return Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
sagarkhandagre998 marked this conversation as resolved.
.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 =
Expand Down
25 changes: 25 additions & 0 deletions evm/src/main/java/org/hyperledger/besu/evm/tracing/TraceFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public class TraceFrame {

private final Optional<SoftFailureReason> softFailureReason;
private final OptionalLong gasAvailableForChildCall;
private final Optional<Bytes> returnData;

/** Private constructor - only accessible through Builder */
private TraceFrame(final Builder builder) {
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -158,6 +160,7 @@ public static class Builder {
private Optional<Bytes> precompileOutputData = Optional.empty();
private Optional<SoftFailureReason> softFailureReason = Optional.empty();
private OptionalLong gasAvailableForChildCall = OptionalLong.empty();
private Optional<Bytes> returnData = Optional.empty();

/** Default constructor */
public Builder() {}
Expand Down Expand Up @@ -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;
Comment thread
sagarkhandagre998 marked this conversation as resolved.
Comment thread
sagarkhandagre998 marked this conversation as resolved.
}

/**
Expand Down Expand Up @@ -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<Bytes> returnData) {
this.returnData = returnData;
return this;
}

/**
* Builds the TraceFrame instance.
*
Expand Down Expand Up @@ -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<Bytes> getReturnData() {
return returnData;
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
Expand All @@ -886,6 +910,7 @@ public String toString() {
.add("stack", stack)
.add("memory", memory)
.add("storage", storage)
.add("returnData", returnData)
.toString();
}
}
Loading