[PAN-2499] debug trace transaction (#1258)

* move subclass * Update Transaction.java * fix 500 error on tx not found Returns a JSON RPC error instead of failing due to NPE. * Handle reason on revert operation Implement reason string on revert operation as in the following EIP : https://github.com/ethereum/EIPs/blob/master/EIPS/eip-140.md * Update MessageFrame.java * Update RevertOperationTest.java * fix PR discussion * fix PR * Update DebugTraceTransaction.java
PegaSysEng · Apr 11, 2019 · cda51a9 · cda51a9
1 parent 5a43c61
commit cda51a9
Show file tree

Hide file tree

Showing 11 changed files with 213 additions and 24 deletions.
diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Transaction.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Transaction.java
@@ -63,7 +63,7 @@ public class Transaction {
   // Note that this hash does not include the transaction signature so it does not
   // fully identify the transaction (use the result of the {@code hash()} for that).
   // It is only used to compute said signature and recover the sender from it.
-  protected volatile Bytes32 hashNoSignature;
+  private volatile Bytes32 hashNoSignature;
 
   // Caches the transaction sender.
   protected volatile Address sender;

diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/debug/TraceFrame.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/debug/TraceFrame.java
@@ -34,6 +34,7 @@ public class TraceFrame {
   private final Optional<Bytes32[]> stack;
   private final Optional<Bytes32[]> memory;
   private final Optional<Map<UInt256, UInt256>> storage;
+  private final String revertReason;
 
   public TraceFrame(
       final int pc,
@@ -44,7 +45,8 @@ public TraceFrame(
       final EnumSet<ExceptionalHaltReason> exceptionalHaltReasons,
       final Optional<Bytes32[]> stack,
       final Optional<Bytes32[]> memory,
-      final Optional<Map<UInt256, UInt256>> storage) {
+      final Optional<Map<UInt256, UInt256>> storage,
+      final String revertReason) {
     this.pc = pc;
     this.opcode = opcode;
     this.gasRemaining = gasRemaining;
@@ -54,6 +56,30 @@ public TraceFrame(
     this.stack = stack;
     this.memory = memory;
     this.storage = storage;
+    this.revertReason = revertReason;
+  }
+
+  public TraceFrame(
+      final int pc,
+      final String opcode,
+      final Gas gasRemaining,
+      final Optional<Gas> gasCost,
+      final int depth,
+      final EnumSet<ExceptionalHaltReason> exceptionalHaltReasons,
+      final Optional<Bytes32[]> stack,
+      final Optional<Bytes32[]> memory,
+      final Optional<Map<UInt256, UInt256>> storage) {
+    this(
+        pc,
+        opcode,
+        gasRemaining,
+        gasCost,
+        depth,
+        exceptionalHaltReasons,
+        stack,
+        memory,
+        storage,
+        null);
   }
 
   public int getPc() {
@@ -92,6 +118,10 @@ public Optional<Map<UInt256, UInt256>> getStorage() {
     return storage;
   }
 
+  public String getRevertReason() {
+    return revertReason;
+  }
+
   @Override
   public String toString() {
     return MoreObjects.toStringHelper(this)

diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/DebugOperationTracer.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/DebugOperationTracer.java
@@ -67,7 +67,8 @@ public void traceExecution(
               exceptionalHaltReasons,
               stack,
               memory,
-              storage));
+              storage,
+              frame.getRevertReason().orElse(null)));
     }
   }
 

diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/MessageFrame.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/MessageFrame.java
@@ -32,6 +32,7 @@
 import java.util.Deque;
 import java.util.EnumSet;
 import java.util.HashSet;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
 
@@ -215,6 +216,7 @@ public enum Type {
   private final Deque<MessageFrame> messageFrameStack;
   private final Address miningBeneficiary;
   private final Boolean isPersistingState;
+  private Optional<String> revertReason;
 
   // Miscellaneous fields.
   private final EnumSet<ExceptionalHaltReason> exceptionalHaltReasons =
@@ -247,7 +249,8 @@ private MessageFrame(
       final Consumer<MessageFrame> completer,
       final Address miningBeneficiary,
       final BlockHashLookup blockHashLookup,
-      final Boolean isPersistingState) {
+      final Boolean isPersistingState,
+      final Optional<String> revertReason) {
     this.type = type;
     this.blockchain = blockchain;
     this.messageFrameStack = messageFrameStack;
@@ -278,6 +281,7 @@ private MessageFrame(
     this.completer = completer;
     this.miningBeneficiary = miningBeneficiary;
     this.isPersistingState = isPersistingState;
+    this.revertReason = revertReason;
   }
 
   /**
@@ -495,6 +499,19 @@ public UInt256 memoryWordSize() {
     return memory.getActiveWords();
   }
 
+  /**
+   * Returns the revertReason as string
+   *
+   * @return the revertReason string
+   */
+  public Optional<String> getRevertReason() {
+    return revertReason;
+  }
+
+  public void setRevertReason(final String revertReason) {
+    this.revertReason = Optional.ofNullable(revertReason);
+  }
+
   /**
    * Read bytes in memory.
    *
@@ -845,6 +862,7 @@ public static class Builder {
     private Address miningBeneficiary;
     private BlockHashLookup blockHashLookup;
     private Boolean isPersistingState = false;
+    private Optional<String> reason = Optional.empty();
 
     public Builder type(final Type type) {
       this.type = type;
@@ -951,6 +969,11 @@ public Builder isPersistingState(final Boolean isPersistingState) {
       return this;
     }
 
+    public Builder reason(final String reason) {
+      this.reason = Optional.ofNullable(reason);
+      return this;
+    }
+
     private void validate() {
       checkState(type != null, "Missing message frame type");
       checkState(blockchain != null, "Missing message frame blockchain");
@@ -998,7 +1021,8 @@ public MessageFrame build() {
           completer,
           miningBeneficiary,
           blockHashLookup,
-          isPersistingState);
+          isPersistingState,
+          reason);
     }
   }
 }
diff --git a/...reum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/operations/RevertOperation.java b/...reum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/operations/RevertOperation.java
@@ -16,9 +16,13 @@
 import tech.pegasys.pantheon.ethereum.vm.AbstractOperation;
 import tech.pegasys.pantheon.ethereum.vm.GasCalculator;
 import tech.pegasys.pantheon.ethereum.vm.MessageFrame;
+import tech.pegasys.pantheon.util.bytes.BytesValue;
 import tech.pegasys.pantheon.util.uint.UInt256;
 
+import java.nio.charset.Charset;
+
 public class RevertOperation extends AbstractOperation {
+  private static final Charset CHARSET = Charset.forName("UTF-8");
 
   public RevertOperation(final GasCalculator gasCalculator) {
     super(0xFD, "REVERT", 2, 0, false, 1, gasCalculator);
@@ -36,8 +40,10 @@ public Gas cost(final MessageFrame frame) {
   public void execute(final MessageFrame frame) {
     final UInt256 from = frame.popStackItem().asUInt256();
     final UInt256 length = frame.popStackItem().asUInt256();
-
-    frame.setOutputData(frame.readMemory(from, length));
+    BytesValue reason = frame.readMemory(from, length);
+    frame.setOutputData(reason);
+    String reasonMessage = new String(reason.extractArray(), CHARSET);
+    frame.setRevertReason(reasonMessage);
     frame.setState(MessageFrame.State.REVERT);
   }
 }
diff --git a/.../core/src/test/java/tech/pegasys/pantheon/ethereum/vm/operations/RevertOperationTest.java b/.../core/src/test/java/tech/pegasys/pantheon/ethereum/vm/operations/RevertOperationTest.java
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2018 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package tech.pegasys.pantheon.ethereum.vm.operations;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import tech.pegasys.pantheon.ethereum.mainnet.ConstantinopleGasCalculator;
+import tech.pegasys.pantheon.ethereum.vm.MessageFrame;
+import tech.pegasys.pantheon.util.bytes.Bytes32;
+import tech.pegasys.pantheon.util.bytes.BytesValue;
+import tech.pegasys.pantheon.util.uint.UInt256;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+
+@RunWith(Parameterized.class)
+public class RevertOperationTest {
+
+  private final String code;
+
+  private final MessageFrame messageFrame = mock(MessageFrame.class);
+  private final RevertOperation operation = new RevertOperation(new ConstantinopleGasCalculator());
+
+  @Parameters(name = "sender: {0}, salt: {1}, code: {2}")
+  public static Object[][] params() {
+    return new Object[][] {
+      {
+        "0x6c726576657274656420646174616000557f726576657274206d657373616765000000000000000000000000000000000000600052600e6000fd",
+      }
+    };
+  }
+
+  public RevertOperationTest(final String code) {
+    this.code = code;
+  }
+
+  @Before
+  public void setUp() {
+    when(messageFrame.popStackItem())
+        .thenReturn(Bytes32.fromHexString("0x00"))
+        .thenReturn(Bytes32.fromHexString("0x0e"));
+    when(messageFrame.readMemory(UInt256.ZERO, UInt256.of(0x0e)))
+        .thenReturn(BytesValue.fromHexString("726576657274206d657373616765"));
+  }
+
+  @Test
+  public void shouldReturnReason() {
+    assertTrue(code.contains("726576657274206d657373616765"));
+    ArgumentCaptor<String> arg = ArgumentCaptor.forClass(String.class);
+    operation.execute(messageFrame);
+    Mockito.verify(messageFrame).setRevertReason(arg.capture());
+    assertEquals("revert message", arg.getValue());
+  }
+}
diff --git a/...n/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/DebugTraceTransaction.java b/...n/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/DebugTraceTransaction.java
@@ -54,19 +54,31 @@ public JsonRpcResponse response(final JsonRpcRequest request) {
         parameters.optional(request.getParams(), 1, TransactionTraceParams.class);
     final Optional<TransactionWithMetadata> transactionWithMetadata =
         blockchain.transactionByHash(hash);
-    final Hash blockHash = transactionWithMetadata.get().getBlockHash();
+    if (transactionWithMetadata.isPresent()) {
+      DebugTraceTransactionResult debugTraceTransactionResult =
+          debugTraceTransactionResult(hash, transactionWithMetadata.get(), transactionTraceParams);
+
+      return new JsonRpcSuccessResponse(request.getId(), debugTraceTransactionResult);
+    } else {
+      return new JsonRpcSuccessResponse(request.getId(), null);
+    }
+  }
+
+  private DebugTraceTransactionResult debugTraceTransactionResult(
+      final Hash hash,
+      final TransactionWithMetadata transactionWithMetadata,
+      final Optional<TransactionTraceParams> transactionTraceParams) {
+    final Hash blockHash = transactionWithMetadata.getBlockHash();
     final TraceOptions traceOptions =
         transactionTraceParams
             .map(TransactionTraceParams::traceOptions)
             .orElse(TraceOptions.DEFAULT);
 
     final DebugOperationTracer execTracer = new DebugOperationTracer(traceOptions);
 
-    final DebugTraceTransactionResult result =
-        transactionTracer
-            .traceTransaction(blockHash, hash, execTracer)
-            .map(DebugTraceTransactionResult::new)
-            .orElse(null);
-    return new JsonRpcSuccessResponse(request.getId(), result);
+    return transactionTracer
+        .traceTransaction(blockHash, hash, execTracer)
+        .map(DebugTraceTransactionResult::new)
+        .orElse(null);
   }
 }
diff --git a/.../src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java b/.../src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java
@@ -45,7 +45,6 @@ public enum JsonRpcError {
   INCORRECT_NONCE(-32006, "Incorrect nonce"),
   TX_SENDER_NOT_AUTHORIZED(-32007, "Sender account not authorized to send transactions"),
   CHAIN_HEAD_WORLD_STATE_NOT_AVAILABLE(-32008, "Initial sync is still in progress"),
-
   // Miner failures
   COINBASE_NOT_SET(-32010, "Coinbase not set. Unable to start mining without a coinbase"),
   NO_HASHES_PER_SECOND(-32011, "No hashes being generated by the current node"),

diff --git a/...nrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/results/StructLog.java b/...nrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/results/StructLog.java
@@ -36,6 +36,7 @@ public class StructLog {
   private final int pc;
   private final String[] stack;
   private final Object storage;
+  private final String reason;
 
   public StructLog(final TraceFrame traceFrame) {
     depth = traceFrame.getDepth() + 1;
@@ -54,6 +55,7 @@ public StructLog(final TraceFrame traceFrame) {
             .map(a -> Arrays.stream(a).map(Bytes32s::unprefixedHexString).toArray(String[]::new))
             .orElse(null);
     storage = traceFrame.getStorage().map(StructLog::formatStorage).orElse(null);
+    reason = traceFrame.getRevertReason();
   }
 
   private static Map<String, String> formatStorage(final Map<UInt256, UInt256> storage) {
@@ -104,6 +106,11 @@ public Object storage() {
     return storage;
   }
 
+  @JsonGetter("reason")
+  public String reason() {
+    return reason;
+  }
+
   @Override
   public boolean equals(final Object o) {
     if (this == o) {

diff --git a/...va/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/DebugTraceTransactionTest.java b/...va/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/DebugTraceTransactionTest.java
@@ -13,6 +13,7 @@
 package tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
@@ -85,7 +86,8 @@ public void shouldTraceTheTransactionUsingTheTransactionTracer() {
             EnumSet.noneOf(ExceptionalHaltReason.class),
             Optional.empty(),
             Optional.empty(),
-            Optional.empty());
+            Optional.empty(),
+            "revert message");
     final List<TraceFrame> traceFrames = Collections.singletonList(traceFrame);
     final TransactionTrace transactionTrace =
         new TransactionTrace(transaction, result, traceFrames);
@@ -108,4 +110,41 @@ public void shouldTraceTheTransactionUsingTheTransactionTracer() {
     final List<StructLog> expectedStructLogs = Collections.singletonList(new StructLog(traceFrame));
     assertEquals(expectedStructLogs, transactionResult.getStructLogs());
   }
+
+  @Test
+  public void shouldNotTraceTheTransactionIfNotFound() {
+    final Map<String, Boolean> map = new HashMap<>();
+    map.put("disableStorage", true);
+    final Object[] params = new Object[] {transactionHash, map};
+    final JsonRpcRequest request = new JsonRpcRequest("2.0", "debug_traceTransaction", params);
+    final Result result = mock(Result.class);
+
+    final TraceFrame traceFrame =
+        new TraceFrame(
+            12,
+            "NONE",
+            Gas.of(45),
+            Optional.of(Gas.of(56)),
+            2,
+            EnumSet.noneOf(ExceptionalHaltReason.class),
+            Optional.empty(),
+            Optional.empty(),
+            Optional.empty(),
+            "revert message");
+    final List<TraceFrame> traceFrames = Collections.singletonList(traceFrame);
+    final TransactionTrace transactionTrace =
+        new TransactionTrace(transaction, result, traceFrames);
+    when(transaction.getGasLimit()).thenReturn(100L);
+    when(result.getGasRemaining()).thenReturn(27L);
+    when(result.getOutput()).thenReturn(BytesValue.fromHexString("1234"));
+    when(blockHeader.getNumber()).thenReturn(12L);
+    when(blockchain.headBlockNumber()).thenReturn(12L);
+    when(blockchain.transactionByHash(transactionHash)).thenReturn(Optional.empty());
+    when(transactionTracer.traceTransaction(eq(blockHash), eq(transactionHash), any()))
+        .thenReturn(Optional.of(transactionTrace));
+    final JsonRpcSuccessResponse response =
+        (JsonRpcSuccessResponse) debugTraceTransaction.response(request);
+
+    assertNull(response.getResult());
+  }
 }