More trace fixes

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## 1.4 RC 
+
+### Additions and Improvements 
+
+- New`trace_replayBlockTransactions` JSON-RPC API
+
+This can be enabled using the `--rpc-http-api TRACE` CLI flag.  There are some philosophical differences between Besu and other implementations that are outlined in the `[trace_rpc_apis.md](./docs/trace_rpc_apis.md)` documentation.
+
 ## 1.4 Beta 3 
 
 ### Additions and Improvements 

docs/trace_rpc_apis.md

@@ -0,0 +1,39 @@
+# Trace RPC API Notes
+
+This document outlines major differences for `trace_replayBlockTransactions` 
+compared to other implementations.
+
+## `stateDiff` 
+
+No major differences were observed in the `stateDiff` field.
+
+## `trace`
+
+Besu reports `gasUsed` after applying the effects of gas refunds.  Future
+implementations of Besu might track gas refunds separately.
+
+## `vmTrace`
+
+### Returned Memory from Calls
+
+In the `vmTrace` `ope.ex.mem` fields Besu only reports actual data returned
+from a `RETURN` opcode.  Other implementations return the contents of the 
+reserved output space for the call operations.  Note two major differences:
+
+1. Besu reports `null` when a call operation ends because of a `STOP`,  `HALT`, 
+   `REVERT`, running out of instructions, or any exceptional halts.
+2. When a `RETURN` operation returns data of a different length than the space
+   reserved by the call only the data passed to the `RETURN` operation is 
+   reported.  Other implementations will include pre-existing memory data or 
+   trim the returned data.
+
+### Precompiled Contracts Calls
+
+Besu reports only the actual cost of the precompiled contract call in the 
+`cost` field. 
+
+### Out of Gas 
+
+Besu reports the operation that causes out fo gas exceptions, including 
+calculated gas cost.  The operation is not executed so no `ex` values are 
+reported.

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/tracing/flat/FlatTrace.java

@@ -187,6 +187,10 @@ public Builder type(final String type) {
       return this;
     }
 
+    public String getType() {
+      return type;
+    }
+
     public Builder error(final Optional<String> error) {
       this.error = error;
       return this;

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/tracing/flat/FlatTraceGenerator.java

@@ -18,7 +18,6 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.tracing.Trace;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.tracing.TracingUtils;
-import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.tracing.flat.FlatTrace.Context;
 import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.core.Gas;
 import org.hyperledger.besu.ethereum.core.Transaction;
@@ -30,6 +29,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Deque;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
 import java.util.Optional;
@@ -98,9 +98,12 @@ public static Stream<Trace> generateFromTransactionTrace(
     // declare the first transactionTrace context as the previous transactionTrace context
     long cumulativeGasCost = 0;
 
-    int traceFrameIndex = 0;
-    final List<TraceFrame> traceFrames = transactionTrace.getTraceFrames();
-    for (final TraceFrame traceFrame : traceFrames) {
+    final Iterator<TraceFrame> iter = transactionTrace.getTraceFrames().iterator();
+    Optional<TraceFrame> nextTraceFrame =
+        iter.hasNext() ? Optional.of(iter.next()) : Optional.empty();
+    while (nextTraceFrame.isPresent()) {
+      final TraceFrame traceFrame = nextTraceFrame.get();
+      nextTraceFrame = iter.hasNext() ? Optional.of(iter.next()) : Optional.empty();
       cumulativeGasCost +=
           traceFrame.getGasCost().orElse(Gas.ZERO).toLong()
               + traceFrame.getPrecompiledGasCost().orElse(Gas.ZERO).toLong();
@@ -113,12 +116,10 @@ public static Stream<Trace> generateFromTransactionTrace(
             handleCall(
                 transactionTrace,
                 traceFrame,
-                smartContractAddress,
+                nextTraceFrame,
                 flatTraces,
                 cumulativeGasCost,
                 tracesContexts,
-                traceFrameIndex,
-                traceFrames,
                 opcodeString.toLowerCase(Locale.US));
       } else if ("RETURN".equals(opcodeString) || "STOP".equals(opcodeString)) {
         if (currentContext != null) {
@@ -136,19 +137,12 @@ public static Stream<Trace> generateFromTransactionTrace(
                 flatTraces,
                 tracesContexts,
                 cumulativeGasCost,
-                traceFrameIndex,
-                traceFrames);
+                nextTraceFrame);
       } else if ("REVERT".equals(opcodeString)) {
         currentContext = handleRevert(tracesContexts, currentContext);
       } else if (!traceFrame.getExceptionalHaltReasons().isEmpty()) {
-        currentContext
-            .getBuilder()
-            .error(
-                traceFrame.getExceptionalHaltReasons().stream()
-                    .map(ExceptionalHaltReason::getDescription)
-                    .reduce((a, b) -> a + ", " + b));
+        currentContext = handleHalt(tracesContexts, currentContext, traceFrame);
       }
-      traceFrameIndex++;
     }
 
     return flatTraces.stream().map(FlatTrace.Builder::build);
@@ -157,21 +151,18 @@ public static Stream<Trace> generateFromTransactionTrace(
   private static FlatTrace.Context handleCall(
       final TransactionTrace transactionTrace,
       final TraceFrame traceFrame,
-      final Optional<String> smartContractAddress,
+      final Optional<TraceFrame> nextTraceFrame,
       final List<FlatTrace.Builder> flatTraces,
       final long cumulativeGasCost,
       final Deque<FlatTrace.Context> tracesContexts,
-      final int traceFrameIndex,
-      final List<TraceFrame> traceFrames,
       final String opcodeString) {
-    final TraceFrame nextTraceFrame = traceFrames.get(traceFrameIndex + 1);
     final Bytes32[] stack = traceFrame.getStack().orElseThrow();
     final Address contractCallAddress = toAddress(stack[stack.length - 2]);
     final FlatTrace.Context lastContext = tracesContexts.peekLast();
     final String callingAddress = calculateCallingAddress(lastContext);
 
-    if (contractCallAddress.numberOfLeadingZeroBytes() >= 19) {
-      // don't log calls to precompiles
+    if (traceFrame.getDepth() >= nextTraceFrame.map(TraceFrame::getDepth).orElse(0)) {
+      // don't log calls to calls that don't execute, such as insufficient value and precompiles
       return tracesContexts.peekLast();
     }
 
@@ -181,13 +172,11 @@ private static FlatTrace.Context handleCall(
             .resultBuilder(Result.builder());
     final Action.Builder subTraceActionBuilder =
         Action.builder()
-            .from(smartContractAddress.orElse(callingAddress))
+            .from(callingAddress)
             .to(contractCallAddress.toString())
             .input(
-                Optional.ofNullable(nextTraceFrame.getInputData())
-                    .map(Bytes::toHexString)
-                    .orElse(null))
-            .gas(nextTraceFrame.getGasRemaining().toHexString())
+                nextTraceFrame.map(TraceFrame::getInputData).map(Bytes::toHexString).orElse(null))
+            .gas(nextTraceFrame.map(TraceFrame::getGasRemaining).orElse(Gas.ZERO).toHexString())
             .callType(opcodeString.toLowerCase(Locale.US))
             .value(Quantity.create(transactionTrace.getTransaction().getValue()));
 
@@ -218,7 +207,7 @@ private static FlatTrace.Context handleReturn(
     // set value for contract creation TXes, CREATE, and CREATE2
     if (actionBuilder.getCallType() == null && traceFrame.getMaybeCode().isPresent()) {
       actionBuilder.init(traceFrame.getMaybeCode().get().getBytes().toHexString());
-      resultBuilder.code(outputData.toHexString()).address(traceFrame.getRecipient().toHexString());
+      resultBuilder.code(outputData.toHexString());
       if (currentContext.isCreateOp()) {
         // this is from a CREATE/CREATE2, so add code deposit cost.
         currentContext.incGasUsed(outputData.size() * 200L);
@@ -275,9 +264,7 @@ private static FlatTrace.Context handleCreateOperation(
       final List<FlatTrace.Builder> flatTraces,
       final Deque<FlatTrace.Context> tracesContexts,
       final long cumulativeGasCost,
-      final int traceFrameIndex,
-      final List<TraceFrame> traceFrames) {
-    final TraceFrame nextTraceFrame = traceFrames.get(traceFrameIndex + 1);
+      final Optional<TraceFrame> nextTraceFrame) {
     final FlatTrace.Context lastContext = tracesContexts.peekLast();
     final String callingAddress = calculateCallingAddress(lastContext);
 
@@ -289,21 +276,49 @@ private static FlatTrace.Context handleCreateOperation(
     final Action.Builder subTraceActionBuilder =
         Action.builder()
             .from(smartContractAddress.orElse(callingAddress))
-            .gas(nextTraceFrame.getGasRemaining().toHexString())
-            .value(Quantity.create(nextTraceFrame.getValue()));
+            .gas(nextTraceFrame.map(TraceFrame::getGasRemaining).orElse(Gas.ZERO).toHexString())
+            .value(Quantity.create(nextTraceFrame.map(TraceFrame::getValue).orElse(Wei.ZERO)));
 
     final FlatTrace.Context currentContext =
         new FlatTrace.Context(subTraceBuilder.actionBuilder(subTraceActionBuilder));
+    currentContext
+        .getBuilder()
+        .getResultBuilder()
+        .address(nextTraceFrame.map(TraceFrame::getRecipient).orElse(Address.ZERO).toHexString());
     currentContext.setCreateOp(true);
     currentContext.decGasUsed(cumulativeGasCost);
     tracesContexts.addLast(currentContext);
     flatTraces.add(currentContext.getBuilder());
     return currentContext;
   }
 
-  private static Context handleRevert(
-      final Deque<Context> tracesContexts, final FlatTrace.Context currentContext) {
-    currentContext.getBuilder().error(Optional.of("Reverted"));
+  private static FlatTrace.Context handleHalt(
+      final Deque<FlatTrace.Context> tracesContexts,
+      final FlatTrace.Context currentContext,
+      final TraceFrame traceFrame) {
+    final FlatTrace.Builder traceFrameBuilder = currentContext.getBuilder();
+    traceFrameBuilder.error(
+        traceFrame.getExceptionalHaltReasons().stream()
+            .map(ExceptionalHaltReason::getDescription)
+            .reduce((a, b) -> a + ", " + b));
+    if (tracesContexts.size() > 1) {
+      traceFrameBuilder.getActionBuilder().value("0x0");
+    }
+    tracesContexts.removeLast();
+    final FlatTrace.Context nextContext = tracesContexts.peekLast();
+    if (nextContext != null) {
+      nextContext.getBuilder().incSubTraces();
+    }
+    return nextContext;
+  }
+
+  private static FlatTrace.Context handleRevert(
+      final Deque<FlatTrace.Context> tracesContexts, final FlatTrace.Context currentContext) {
+    final FlatTrace.Builder traceFrameBuilder = currentContext.getBuilder();
+    traceFrameBuilder.error(Optional.of("Reverted"));
+    if (tracesContexts.size() > 1) {
+      traceFrameBuilder.getActionBuilder().value("0x0");
+    }
     tracesContexts.removeLast();
     final FlatTrace.Context nextContext = tracesContexts.peekLast();
     if (nextContext != null) {
@@ -313,16 +328,25 @@ private static Context handleRevert(
   }
 
   private static String calculateCallingAddress(final FlatTrace.Context lastContext) {
-    if (lastContext.getBuilder().getActionBuilder().getCallType() == null) {
-      return ZERO_ADDRESS_STRING;
+    final FlatTrace.Builder lastContextBuilder = lastContext.getBuilder();
+    final Action.Builder lastActionBuilder = lastContextBuilder.getActionBuilder();
+    if (lastActionBuilder.getCallType() == null) {
+      if ("create".equals(lastContextBuilder.getType())) {
+        return lastContextBuilder.getResultBuilder().getAddress();
+      } else {
+        return ZERO_ADDRESS_STRING;
+      }
     }
-    switch (lastContext.getBuilder().getActionBuilder().getCallType()) {
+    switch (lastActionBuilder.getCallType()) {
       case "call":
       case "staticcall":
-        return lastContext.getBuilder().getActionBuilder().getTo();
+        return lastActionBuilder.getTo();
       case "delegatecall":
       case "callcode":
-        return lastContext.getBuilder().getActionBuilder().getFrom();
+        return lastActionBuilder.getFrom();
+      case "create":
+      case "create2":
+        return lastContextBuilder.getResultBuilder().getAddress();
       default:
         return ZERO_ADDRESS_STRING;
     }

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/tracing/flat/Result.java

@@ -89,6 +89,10 @@ public Builder address(final String address) {
       return this;
     }
 
+    public String getAddress() {
+      return address;
+    }
+
     public static Builder of(final Result result) {
       final Builder builder = new Builder();
       if (result != null) {