Delete leftPad when capturing the stack before and after a frame exec…

…ution (hyperledger#6102) * Delete leftPad when capturing the stack before and after the execution * Still use leftPad when displaying the stack in the output (ex. for debug_traceTransaction) * Fix integration test * Use StringBuilder to left pad the hex representation of a 32 bytes Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
jflo · Nov 9, 2023 · 9710a9a · 9710a9a
1 parent 701cbb0
commit 9710a9a
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 24 deletions.
diff --git a/...i/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLog.java b/...i/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/results/StructLog.java
@@ -24,6 +24,7 @@
 import com.fasterxml.jackson.annotation.JsonGetter;
 import com.fasterxml.jackson.annotation.JsonPropertyOrder;
 import org.apache.tuweni.bytes.Bytes;
+import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.units.bigints.UInt256;
 
 @JsonPropertyOrder({"pc", "op", "gas", "gasCost", "depth", "stack", "memory", "storage"})
@@ -38,6 +39,7 @@ public class StructLog {
   private final String[] stack;
   private final Object storage;
   private final String reason;
+  static final String bytes32ZeroString = Bytes32.ZERO.toUnprefixedHexString();
 
   public StructLog(final TraceFrame traceFrame) {
     depth = traceFrame.getDepth() + 1;
@@ -53,12 +55,25 @@ public StructLog(final TraceFrame traceFrame) {
     stack =
         traceFrame
             .getStack()
-            .map(a -> Arrays.stream(a).map(Bytes::toUnprefixedHexString).toArray(String[]::new))
+            .map(
+                a ->
+                    Arrays.stream(a)
+                        .map(Bytes::toUnprefixedHexString)
+                        .map(this::stringLeftPadTo64)
+                        .toArray(String[]::new))
             .orElse(null);
+
     storage = traceFrame.getStorage().map(StructLog::formatStorage).orElse(null);
     reason = traceFrame.getRevertReason().map(Bytes::toShortHexString).orElse(null);
   }
 
+  private String stringLeftPadTo64(final String unPaddedHexString) {
+    StringBuilder sb = new StringBuilder(64);
+    sb.append(bytes32ZeroString, 0, 64 - unPaddedHexString.length());
+    sb.append(unPaddedHexString);
+    return sb.toString();
+  }
+
   private static Map<String, String> formatStorage(final Map<UInt256, UInt256> storage) {
     final Map<String, String> formattedStorage = new TreeMap<>();
     storage.forEach(

diff --git a/...tegration-test/java/org/hyperledger/besu/ethereum/vm/TraceTransactionIntegrationTest.java b/...tegration-test/java/org/hyperledger/besu/ethereum/vm/TraceTransactionIntegrationTest.java
@@ -206,8 +206,7 @@ public void shouldTraceContractCreation() {
     assertThat(frame.getGasCost()).isEqualTo(OptionalLong.of(3L));
     assertThat(frame.getOpcode()).isEqualTo("PUSH1");
     assertThat(frame.getPc()).isEqualTo(2);
-    assertStackContainsExactly(
-        frame, "0000000000000000000000000000000000000000000000000000000000000080");
+    assertStackContainsExactly(frame, "0x80");
     assertMemoryContainsExactly(frame);
     assertStorageContainsExactly(frame);
 
@@ -217,10 +216,7 @@ public void shouldTraceContractCreation() {
     assertThat(frame.getGasCost()).isEqualTo(OptionalLong.of(12L));
     assertThat(frame.getOpcode()).isEqualTo("MSTORE");
     assertThat(frame.getPc()).isEqualTo(4);
-    assertStackContainsExactly(
-        frame,
-        "0000000000000000000000000000000000000000000000000000000000000080",
-        "0000000000000000000000000000000000000000000000000000000000000040");
+    assertStackContainsExactly(frame, "80", "40");
     assertMemoryContainsExactly(
         frame,
         "0x0000000000000000000000000000000000000000000000000000000000000000",
@@ -251,8 +247,8 @@ public void shouldTraceContractCreation() {
   private void assertStackContainsExactly(
       final TraceFrame frame, final String... stackEntriesAsHex) {
     assertThat(frame.getStack()).isPresent();
-    final Bytes32[] stackEntries =
-        Stream.of(stackEntriesAsHex).map(Bytes32::fromHexString).toArray(Bytes32[]::new);
+    final Bytes[] stackEntries =
+        Stream.of(stackEntriesAsHex).map(Bytes::fromHexString).toArray(Bytes[]::new);
     assertThat(frame.getStack().get()).containsExactly(stackEntries);
   }
 

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/debug/TraceFrame.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/debug/TraceFrame.java
@@ -28,7 +28,6 @@
 
 import com.google.common.base.MoreObjects;
 import org.apache.tuweni.bytes.Bytes;
-import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.units.bigints.UInt256;
 
 public class TraceFrame {
@@ -44,7 +43,7 @@ public class TraceFrame {
   private final Wei value;
   private final Bytes inputData;
   private final Bytes outputData;
-  private final Optional<Bytes32[]> stack;
+  private final Optional<Bytes[]> stack;
   private final Optional<Bytes[]> memory;
   private final Optional<Map<UInt256, UInt256>> storage;
 
@@ -53,7 +52,7 @@ public class TraceFrame {
   private final Optional<Map<Address, Wei>> maybeRefunds;
   private final Optional<Code> maybeCode;
   private final int stackItemsProduced;
-  private final Optional<Bytes32[]> stackPostExecution;
+  private final Optional<Bytes[]> stackPostExecution;
 
   private long gasRemainingPostExecution;
   private final boolean virtualOperation;
@@ -73,15 +72,15 @@ public TraceFrame(
       final Wei value,
       final Bytes inputData,
       final Bytes outputData,
-      final Optional<Bytes32[]> stack,
+      final Optional<Bytes[]> stack,
       final Optional<Bytes[]> memory,
       final Optional<Map<UInt256, UInt256>> storage,
       final WorldUpdater worldUpdater,
       final Optional<Bytes> revertReason,
       final Optional<Map<Address, Wei>> maybeRefunds,
       final Optional<Code> maybeCode,
       final int stackItemsProduced,
-      final Optional<Bytes32[]> stackPostExecution,
+      final Optional<Bytes[]> stackPostExecution,
       final boolean virtualOperation,
       final Optional<MemoryEntry> maybeUpdatedMemory,
       final Optional<StorageEntry> maybeUpdatedStorage) {
@@ -159,7 +158,7 @@ public Bytes getOutputData() {
     return outputData;
   }
 
-  public Optional<Bytes32[]> getStack() {
+  public Optional<Bytes[]> getStack() {
     return stack;
   }
 
@@ -206,7 +205,7 @@ public int getStackItemsProduced() {
     return stackItemsProduced;
   }
 
-  public Optional<Bytes32[]> getStackPostExecution() {
+  public Optional<Bytes[]> getStackPostExecution() {
     return stackPostExecution;
   }
 

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
@@ -14,8 +14,6 @@
  */
 package org.hyperledger.besu.ethereum.vm;
 
-import static org.apache.tuweni.bytes.Bytes32.leftPad;
-
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.debug.TraceFrame;
@@ -36,7 +34,6 @@
 import java.util.TreeMap;
 
 import org.apache.tuweni.bytes.Bytes;
-import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.units.bigints.UInt256;
 
 public class DebugOperationTracer implements OperationTracer {
@@ -45,7 +42,7 @@ public class DebugOperationTracer implements OperationTracer {
   private List<TraceFrame> traceFrames = new ArrayList<>();
   private TraceFrame lastFrame;
 
-  private Optional<Bytes32[]> preExecutionStack;
+  private Optional<Bytes[]> preExecutionStack;
   private long gasRemaining;
   private Bytes inputData;
   private int pc;
@@ -73,7 +70,7 @@ public void tracePostExecution(final MessageFrame frame, final OperationResult o
     final WorldUpdater worldUpdater = frame.getWorldUpdater();
     final Bytes outputData = frame.getOutputData();
     final Optional<Bytes[]> memory = captureMemory(frame);
-    final Optional<Bytes32[]> stackPostExecution = captureStack(frame);
+    final Optional<Bytes[]> stackPostExecution = captureStack(frame);
 
     if (lastFrame != null) {
       lastFrame.setGasRemainingPostExecution(gasRemaining);
@@ -218,15 +215,15 @@ private Optional<Bytes[]> captureMemory(final MessageFrame frame) {
     return Optional.of(memoryContents);
   }
 
-  private Optional<Bytes32[]> captureStack(final MessageFrame frame) {
+  private Optional<Bytes[]> captureStack(final MessageFrame frame) {
     if (!options.isStackEnabled()) {
       return Optional.empty();
     }
 
-    final Bytes32[] stackContents = new Bytes32[frame.stackSize()];
+    final Bytes[] stackContents = new Bytes[frame.stackSize()];
     for (int i = 0; i < stackContents.length; i++) {
       // Record stack contents in reverse
-      stackContents[i] = leftPad(frame.getStackItem(stackContents.length - i - 1));
+      stackContents[i] = frame.getStackItem(stackContents.length - i - 1);
     }
     return Optional.of(stackContents);
   }