Better tracing alignment

CHANGELOG.md

@@ -48,6 +48,7 @@
 - Fix the way an advertised host configured with `--p2p-host` is treated when communicating with the originator of a PING packet [#6225](https://github.com/hyperledger/besu/pull/6225)
 - Fix `poa-block-txs-selection-max-time` option that was inadvertently reset to its default after being configured [#6444](https://github.com/hyperledger/besu/pull/6444)
 - Fix for tx incorrectly discarded when there is a timeout during block creation [#6563](https://github.com/hyperledger/besu/pull/6563)
+- Fix traces so that call gas costing in traces matches other clients traces [#6525](https://github.com/hyperledger/besu/pull/6525)
 
 ### Download Links
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AbstractTraceByHash.java

@@ -78,19 +78,18 @@ private TransactionTrace getTransactionTrace(final Block block, final Hash trans
     return Tracer.processTracing(
             blockchainQueries,
             Optional.of(block.getHeader()),
-            mutableWorldState -> {
-              return blockTracerSupplier
-                  .get()
-                  .trace(
-                      mutableWorldState,
-                      block,
-                      new DebugOperationTracer(new TraceOptions(false, false, true)))
-                  .map(BlockTrace::getTransactionTraces)
-                  .orElse(Collections.emptyList())
-                  .stream()
-                  .filter(trxTrace -> trxTrace.getTransaction().getHash().equals(transactionHash))
-                  .findFirst();
-            })
+            mutableWorldState ->
+                blockTracerSupplier
+                    .get()
+                    .trace(
+                        mutableWorldState,
+                        block,
+                        new DebugOperationTracer(new TraceOptions(false, false, true), false))
+                    .map(BlockTrace::getTransactionTraces)
+                    .orElse(Collections.emptyList())
+                    .stream()
+                    .filter(trxTrace -> trxTrace.getTransaction().getHash().equals(transactionHash))
+                    .findFirst())
         .orElseThrow();
   }
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AbstractTraceCall.java

@@ -36,11 +36,20 @@
 public abstract class AbstractTraceCall extends AbstractTraceByBlock {
   private static final Logger LOG = LoggerFactory.getLogger(AbstractTraceCall.class);
 
-  public AbstractTraceCall(
+  /**
+   * A flag to indicate if call operations should trace just the operation cost (false, Geth style,
+   * debug_ series RPCs) or the operation cost and all gas granted to the child call (true, Parity
+   * style, trace_ series RPCs)
+   */
+  private final boolean recordChildCallGas;
+
+  protected AbstractTraceCall(
       final BlockchainQueries blockchainQueries,
       final ProtocolSchedule protocolSchedule,
-      final TransactionSimulator transactionSimulator) {
+      final TransactionSimulator transactionSimulator,
+      final boolean recordChildCallGas) {
     super(blockchainQueries, protocolSchedule, transactionSimulator);
+    this.recordChildCallGas = recordChildCallGas;
   }
 
   @Override
@@ -65,7 +74,7 @@ protected Object resultByBlockNumber(
       return new JsonRpcErrorResponse(requestContext.getRequest().getId(), BLOCK_NOT_FOUND);
     }
 
-    final DebugOperationTracer tracer = new DebugOperationTracer(traceOptions);
+    final DebugOperationTracer tracer = new DebugOperationTracer(traceOptions, recordChildCallGas);
     return transactionSimulator
         .process(
             callParams,

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugAccountAt.java

@@ -98,7 +98,7 @@ protected Object resultByBlockHash(
                       .trace(
                           mutableWorldState,
                           blockHash,
-                          new DebugOperationTracer(new TraceOptions(false, true, true)))
+                          new DebugOperationTracer(new TraceOptions(false, true, true), false))
                       .map(BlockTrace::getTransactionTraces)
                       .orElse(Collections.emptyList())
                       .stream()

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugTraceBlock.java

@@ -87,7 +87,10 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
                   mutableWorldState ->
                       blockTracerSupplier
                           .get()
-                          .trace(mutableWorldState, block, new DebugOperationTracer(traceOptions))
+                          .trace(
+                              mutableWorldState,
+                              block,
+                              new DebugOperationTracer(traceOptions, true))
                           .map(BlockTrace::getTransactionTraces)
                           .map(DebugTraceTransactionResult::of))
               .orElse(null);

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugTraceBlockByHash.java

@@ -64,7 +64,10 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
                 mutableWorldState ->
                     blockTracerSupplier
                         .get()
-                        .trace(mutableWorldState, blockHash, new DebugOperationTracer(traceOptions))
+                        .trace(
+                            mutableWorldState,
+                            blockHash,
+                            new DebugOperationTracer(traceOptions, true))
                         .map(BlockTrace::getTransactionTraces)
                         .map(DebugTraceTransactionResult::of))
             .orElse(null);

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugTraceBlockByNumber.java

@@ -66,13 +66,15 @@ protected Object resultByBlockNumber(
                 Tracer.processTracing(
                     blockchainQueriesSupplier.get(),
                     hash,
-                    mutableWorldState -> {
-                      return blockTracerSupplier
-                          .get()
-                          .trace(mutableWorldState, hash, new DebugOperationTracer(traceOptions))
-                          .map(BlockTrace::getTransactionTraces)
-                          .map(DebugTraceTransactionResult::of);
-                    }))
+                    mutableWorldState ->
+                        blockTracerSupplier
+                            .get()
+                            .trace(
+                                mutableWorldState,
+                                hash,
+                                new DebugOperationTracer(traceOptions, true))
+                            .map(BlockTrace::getTransactionTraces)
+                            .map(DebugTraceTransactionResult::of)))
         .orElse(null);
   }
 }

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugTraceCall.java

@@ -43,7 +43,7 @@ public DebugTraceCall(
       final BlockchainQueries blockchainQueries,
       final ProtocolSchedule protocolSchedule,
       final TransactionSimulator transactionSimulator) {
-    super(blockchainQueries, protocolSchedule, transactionSimulator);
+    super(blockchainQueries, protocolSchedule, transactionSimulator, true);
   }
 
   @Override

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugTraceTransaction.java

@@ -73,7 +73,7 @@ private DebugTraceTransactionResult debugTraceTransactionResult(
       final TraceOptions traceOptions) {
     final Hash blockHash = transactionWithMetadata.getBlockHash().get();
 
-    final DebugOperationTracer execTracer = new DebugOperationTracer(traceOptions);
+    final DebugOperationTracer execTracer = new DebugOperationTracer(traceOptions, true);
 
     return Tracer.processTracing(
             blockchain,

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceBlock.java

@@ -116,7 +116,7 @@ protected ArrayNodeWrapper traceBlock(
                           "step",
                           "action");
               DebugOperationTracer debugOperationTracer =
-                  new DebugOperationTracer(new TraceOptions(false, false, true));
+                  new DebugOperationTracer(new TraceOptions(false, false, true), false);
               ExecuteTransactionStep executeTransactionStep =
                   new ExecuteTransactionStep(
                       chainUpdater,

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceCall.java

@@ -41,7 +41,7 @@ public TraceCall(
       final BlockchainQueries blockchainQueries,
       final ProtocolSchedule protocolSchedule,
       final TransactionSimulator transactionSimulator) {
-    super(blockchainQueries, protocolSchedule, transactionSimulator);
+    super(blockchainQueries, protocolSchedule, transactionSimulator, false);
   }
 
   @Override

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceCallMany.java

@@ -66,11 +66,7 @@ protected BlockParameter blockParameter(final JsonRpcRequestContext request) {
     final Optional<BlockParameter> maybeBlockParameter =
         request.getOptionalParameter(1, BlockParameter.class);
 
-    if (maybeBlockParameter.isPresent()) {
-      return maybeBlockParameter.get();
-    }
-
-    return BlockParameter.LATEST;
+    return maybeBlockParameter.orElse(BlockParameter.LATEST);
   }
 
   @Override
@@ -153,7 +149,8 @@ private JsonNode getSingleCallResult(
       final BlockHeader header,
       final WorldUpdater worldUpdater) {
     final Set<TraceTypeParameter.TraceType> traceTypes = traceTypeParameter.getTraceTypes();
-    final DebugOperationTracer tracer = new DebugOperationTracer(buildTraceOptions(traceTypes));
+    final DebugOperationTracer tracer =
+        new DebugOperationTracer(buildTraceOptions(traceTypes), false);
     final Optional<TransactionSimulatorResult> maybeSimulatorResult =
         transactionSimulator.processWithWorldUpdater(
             callParameter, buildTransactionValidationParams(), tracer, header, worldUpdater);

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceFilter.java

@@ -161,7 +161,7 @@ private JsonRpcResponse traceFilterWithPipeline(
                               "action");
 
                   DebugOperationTracer debugOperationTracer =
-                      new DebugOperationTracer(new TraceOptions(false, false, true));
+                      new DebugOperationTracer(new TraceOptions(false, false, true), false);
                   ExecuteTransactionStep executeTransactionStep =
                       new ExecuteTransactionStep(
                           chainUpdater,

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceRawTransaction.java

@@ -89,7 +89,8 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
     }
 
     final Set<TraceTypeParameter.TraceType> traceTypes = traceTypeParameter.getTraceTypes();
-    final DebugOperationTracer tracer = new DebugOperationTracer(buildTraceOptions(traceTypes));
+    final DebugOperationTracer tracer =
+        new DebugOperationTracer(buildTraceOptions(traceTypes), false);
     final BlockHeader headBlock = blockchainQueriesSupplier.get().headBlockHeader();
     return transactionSimulator
         .process(

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceReplayBlockTransactions.java

@@ -93,7 +93,7 @@ protected ArrayNode resultByBlockNumber(
     return getBlockchainQueries()
         .getBlockchain()
         .getBlockByNumber(blockNumber)
-        .map((block) -> traceBlock(block, traceTypeParameter))
+        .map(block -> traceBlock(block, traceTypeParameter))
         .orElse(null);
   }
 
@@ -127,7 +127,7 @@ private ArrayNode traceBlock(final Block block, final TraceTypeParameter traceTy
                           "step",
                           "action");
               final DebugOperationTracer debugOperationTracer =
-                  new DebugOperationTracer(new TraceOptions(false, false, true));
+                  new DebugOperationTracer(new TraceOptions(false, false, true), false);
               final ExecuteTransactionStep executeTransactionStep =
                   new ExecuteTransactionStep(
                       chainUpdater,

ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/jsonrpc/debug/trace-call/debug_traceCall_all.json

@@ -167,7 +167,7 @@
         "pc" : 21,
         "op" : "CALLCODE",
         "gas" : 16755865,
-        "gasCost" : 700,
+        "gasCost" : 16494066,
         "depth" : 1,
         "stack" : null,
         "memory" : null,

ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/jsonrpc/debug/trace-call/debug_traceCall_complete.json

@@ -164,7 +164,7 @@
         "pc" : 21,
         "op" : "CALLCODE",
         "gas" : 16755865,
-        "gasCost" : 700,
+        "gasCost" : 16494066,
         "depth" : 1,
         "stack" : [ "0000000000000000000000000000000000000000000000000000000000000020", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000020", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000030000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000ffac99" ],
         "memory" : [ "f000000000000000000000000000000000000000000000000000000000000001" ],

ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/jsonrpc/debug/trace-call/debug_traceCall_disableMemory.json

@@ -167,7 +167,7 @@
         "pc" : 21,
         "op" : "CALLCODE",
         "gas" : 16755865,
-        "gasCost" : 700,
+        "gasCost" : 16494066,
         "depth" : 1,
         "stack" : [ "0000000000000000000000000000000000000000000000000000000000000020", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000020", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000030000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000ffac99" ],
         "memory" : null,

ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/jsonrpc/debug/trace-call/debug_traceCall_disableStack.json

@@ -167,7 +167,7 @@
         "pc" : 21,
         "op" : "CALLCODE",
         "gas" : 16755865,
-        "gasCost" : 700,
+        "gasCost" : 16494066,
         "depth" : 1,
         "stack" : null,
         "memory" : [ "f000000000000000000000000000000000000000000000000000000000000001" ],

ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/jsonrpc/debug/trace-call/debug_traceCall_disableStorage.json

@@ -167,7 +167,7 @@
         "pc" : 21,
         "op" : "CALLCODE",
         "gas" : 16755865,
-        "gasCost" : 700,
+        "gasCost" : 16494066,
         "depth" : 1,
         "stack" : [ "0000000000000000000000000000000000000000000000000000000000000020", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000020", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000030000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000ffac99" ],
         "memory" : [ "f000000000000000000000000000000000000000000000000000000000000001" ],

ethereum/core/src/integration-test/java/org/hyperledger/besu/ethereum/vm/TraceTransactionIntegrationTest.java

@@ -119,7 +119,7 @@ public void shouldTraceSStoreOperation() {
 
     // Now call the transaction to execute the SSTORE.
     final DebugOperationTracer tracer =
-        new DebugOperationTracer(new TraceOptions(true, true, true));
+        new DebugOperationTracer(new TraceOptions(true, true, true), false);
     final Transaction executeTransaction =
         Transaction.builder()
             .type(TransactionType.FRONTIER)
@@ -166,7 +166,7 @@ public void shouldTraceSStoreOperation() {
   @Test
   public void shouldTraceContractCreation() {
     final DebugOperationTracer tracer =
-        new DebugOperationTracer(new TraceOptions(true, true, true));
+        new DebugOperationTracer(new TraceOptions(true, true, true), false);
     final Transaction transaction =
         Transaction.readFrom(
             new BytesValueRLPInput(Bytes.fromHexString(CONTRACT_CREATION_TX), false));

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

@@ -21,6 +21,7 @@
 import org.hyperledger.besu.evm.ModificationNotAllowedException;
 import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
 import org.hyperledger.besu.evm.frame.MessageFrame;
+import org.hyperledger.besu.evm.operation.AbstractCallOperation;
 import org.hyperledger.besu.evm.operation.Operation;
 import org.hyperledger.besu.evm.operation.Operation.OperationResult;
 import org.hyperledger.besu.evm.tracing.OperationTracer;
@@ -39,6 +40,14 @@
 public class DebugOperationTracer implements OperationTracer {
 
   private final TraceOptions options;
+
+  /**
+   * A flag to indicate if call operations should trace just the operation cost (false, Geth style,
+   * debug_ series RPCs) or the operation cost and all gas granted to the child call (true, Parity
+   * style, trace_ series RPCs)
+   */
+  private final boolean recordChildCallGas;
+
   private List<TraceFrame> traceFrames = new ArrayList<>();
   private TraceFrame lastFrame;
 
@@ -48,8 +57,16 @@ public class DebugOperationTracer implements OperationTracer {
   private int pc;
   private int depth;
 
-  public DebugOperationTracer(final TraceOptions options) {
+  /**
+   * Creates the operation tracer.
+   *
+   * @param options The options, as passed in through the RPC
+   * @param recordChildCallGas A flag on whether to produce geth style (true) or parity style
+   *     (false) gas amounts for call operations
+   */
+  public DebugOperationTracer(final TraceOptions options, final boolean recordChildCallGas) {
     this.options = options;
+    this.recordChildCallGas = recordChildCallGas;
   }
 
   @Override
@@ -78,14 +95,16 @@ public void tracePostExecution(final MessageFrame frame, final OperationResult o
     final Optional<Map<UInt256, UInt256>> storage = captureStorage(frame);
     final Optional<Map<Address, Wei>> maybeRefunds =
         frame.getRefunds().isEmpty() ? Optional.empty() : Optional.of(frame.getRefunds());
+    long thisGasCost = operationResult.getGasCost();
+    if (recordChildCallGas && currentOperation instanceof AbstractCallOperation) {
+      thisGasCost += frame.getMessageFrameStack().getFirst().getRemainingGas();
+    }
     lastFrame =
         new TraceFrame(
             pc,
             Optional.of(opcode),
             gasRemaining,
-            operationResult.getGasCost() == 0
-                ? OptionalLong.empty()
-                : OptionalLong.of(operationResult.getGasCost()),
+            thisGasCost == 0 ? OptionalLong.empty() : OptionalLong.of(thisGasCost),
             frame.getGasRefund(),
             depth,
             Optional.ofNullable(operationResult.getHaltReason()),