RevertReason changed to BytesValue

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/TransactionReceipt.java

@@ -18,7 +18,6 @@
 import tech.pegasys.pantheon.ethereum.rlp.RLPOutput;
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 
-import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
@@ -46,7 +45,7 @@ public class TransactionReceipt {
   private final LogsBloomFilter bloomFilter;
   private final int status;
   private final TransactionReceiptType transactionReceiptType;
-  private final Optional<String> revertReason;
+  private final Optional<BytesValue> revertReason;
 
   /**
    * Creates an instance of a state root-encoded transaction receipt.
@@ -60,7 +59,7 @@ public TransactionReceipt(
       final Hash stateRoot,
       final long cumulativeGasUsed,
       final List<Log> logs,
-      final Optional<String> revertReason) {
+      final Optional<BytesValue> revertReason) {
     this(
         stateRoot,
         NONEXISTENT,
@@ -75,7 +74,7 @@ private TransactionReceipt(
       final long cumulativeGasUsed,
       final List<Log> logs,
       final LogsBloomFilter bloomFilter,
-      final Optional<String> revertReason) {
+      final Optional<BytesValue> revertReason) {
     this(stateRoot, NONEXISTENT, cumulativeGasUsed, logs, bloomFilter, revertReason);
   }
 
@@ -91,7 +90,7 @@ public TransactionReceipt(
       final int status,
       final long cumulativeGasUsed,
       final List<Log> logs,
-      final Optional<String> revertReason) {
+      final Optional<BytesValue> revertReason) {
     this(null, status, cumulativeGasUsed, logs, LogsBloomFilter.compute(logs), revertReason);
   }
 
@@ -100,7 +99,7 @@ private TransactionReceipt(
       final long cumulativeGasUsed,
       final List<Log> logs,
       final LogsBloomFilter bloomFilter,
-      final Optional<String> revertReason) {
+      final Optional<BytesValue> revertReason) {
     this(null, status, cumulativeGasUsed, logs, bloomFilter, revertReason);
   }
 
@@ -110,7 +109,7 @@ private TransactionReceipt(
       final long cumulativeGasUsed,
       final List<Log> logs,
       final LogsBloomFilter bloomFilter,
-      final Optional<String> revertReason) {
+      final Optional<BytesValue> revertReason) {
     this.stateRoot = stateRoot;
     this.cumulativeGasUsed = cumulativeGasUsed;
     this.status = status;
@@ -148,7 +147,7 @@ private void writeTo(final RLPOutput out, final boolean withRevertReason) {
     out.writeBytesValue(bloomFilter.getBytes());
     out.writeList(logs, Log::writeTo);
     if (withRevertReason && revertReason.isPresent()) {
-      out.writeBytesValue(BytesValue.wrap(revertReason.get().getBytes(StandardCharsets.UTF_8)));
+      out.writeBytesValue(revertReason.get());
     }
     out.endList();
   }
@@ -182,15 +181,14 @@ public static TransactionReceipt readFrom(
       // TODO consider validating that the logs and bloom filter match.
       final LogsBloomFilter bloomFilter = LogsBloomFilter.readFrom(input);
       final List<Log> logs = input.readList(Log::readFrom);
-      final Optional<String> revertReason;
+      final Optional<BytesValue> revertReason;
       if (input.isEndOfCurrentList()) {
         revertReason = Optional.empty();
       } else {
         if (!revertReasonAllowed) {
           throw new RLPException("Unexpected value at end of TransactionReceipt");
         }
-        final byte[] bytes = input.readBytesValue().getArrayUnsafe();
-        revertReason = Optional.of(new String(bytes, StandardCharsets.UTF_8));
+        revertReason = Optional.of(input.readBytesValue());
       }
 
       // Status code-encoded transaction receipts have a single
@@ -256,7 +254,7 @@ public TransactionReceiptType getTransactionReceiptType() {
     return transactionReceiptType;
   }
 
-  public Optional<String> getRevertReason() {
+  public Optional<BytesValue> getRevertReason() {
     return revertReason;
   }
 

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/debug/TraceFrame.java

@@ -15,6 +15,7 @@
 import tech.pegasys.pantheon.ethereum.core.Gas;
 import tech.pegasys.pantheon.ethereum.vm.ExceptionalHaltReason;
 import tech.pegasys.pantheon.util.bytes.Bytes32;
+import tech.pegasys.pantheon.util.bytes.BytesValue;
 import tech.pegasys.pantheon.util.uint.UInt256;
 
 import java.util.EnumSet;
@@ -34,7 +35,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;
+  private final BytesValue revertReason;
 
   public TraceFrame(
       final int pc,
@@ -46,7 +47,7 @@ public TraceFrame(
       final Optional<Bytes32[]> stack,
       final Optional<Bytes32[]> memory,
       final Optional<Map<UInt256, UInt256>> storage,
-      final String revertReason) {
+      final BytesValue revertReason) {
     this.pc = pc;
     this.opcode = opcode;
     this.gasRemaining = gasRemaining;
@@ -118,7 +119,7 @@ public Optional<Map<UInt256, UInt256>> getStorage() {
     return storage;
   }
 
-  public String getRevertReason() {
+  public BytesValue getRevertReason() {
     return revertReason;
   }
 

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetTransactionProcessor.java

@@ -64,7 +64,7 @@ public static class Result implements TransactionProcessor.Result {
     private final BytesValue output;
 
     private final ValidationResult<TransactionInvalidReason> validationResult;
-    private final Optional<String> revertReason;
+    private final Optional<BytesValue> revertReason;
 
     public static Result invalid(
         final ValidationResult<TransactionInvalidReason> validationResult) {
@@ -80,7 +80,7 @@ public static Result invalid(
     public static Result failed(
         final long gasRemaining,
         final ValidationResult<TransactionInvalidReason> validationResult,
-        final Optional<String> revertReason) {
+        final Optional<BytesValue> revertReason) {
       return new Result(
           Status.FAILED,
           LogSeries.empty(),
@@ -105,7 +105,7 @@ public static Result successful(
         final long gasRemaining,
         final BytesValue output,
         final ValidationResult<TransactionInvalidReason> validationResult,
-        final Optional<String> revertReason) {
+        final Optional<BytesValue> revertReason) {
       this.status = status;
       this.logs = logs;
       this.gasRemaining = gasRemaining;
@@ -140,7 +140,7 @@ public ValidationResult<TransactionInvalidReason> getValidationResult() {
     }
 
     @Override
-    public Optional<String> getRevertReason() {
+    public Optional<BytesValue> getRevertReason() {
       return revertReason;
     }
   }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/TransactionProcessor.java

@@ -103,7 +103,7 @@ default boolean isSuccessful() {
      *
      * @return the revert reason.
      */
-    Optional<String> getRevertReason();
+    Optional<BytesValue> getRevertReason();
   }
 
   /**

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/privacy/PrivateTransactionProcessor.java

@@ -72,7 +72,7 @@ public static class Result implements TransactionProcessor.Result {
     private final BytesValue output;
 
     private final ValidationResult<TransactionInvalidReason> validationResult;
-    private final Optional<String> revertReason;
+    private final Optional<BytesValue> revertReason;
 
     public static Result invalid(
         final ValidationResult<TransactionInvalidReason> validationResult) {
@@ -88,7 +88,7 @@ public static Result invalid(
     public static Result failed(
         final long gasRemaining,
         final ValidationResult<TransactionInvalidReason> validationResult,
-        final Optional<String> revertReason) {
+        final Optional<BytesValue> revertReason) {
       return new Result(
           Status.FAILED,
           LogSeries.empty(),
@@ -113,7 +113,7 @@ public static Result successful(
         final long gasRemaining,
         final BytesValue output,
         final ValidationResult<TransactionInvalidReason> validationResult,
-        final Optional<String> revertReason) {
+        final Optional<BytesValue> revertReason) {
       this.status = status;
       this.logs = logs;
       this.gasRemaining = gasRemaining;
@@ -148,7 +148,7 @@ public ValidationResult<TransactionInvalidReason> getValidationResult() {
     }
 
     @Override
-    public Optional<String> getRevertReason() {
+    public Optional<BytesValue> getRevertReason() {
       return revertReason;
     }
   }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/MessageFrame.java

@@ -219,7 +219,7 @@ public enum Type {
   private final Deque<MessageFrame> messageFrameStack;
   private final Address miningBeneficiary;
   private final Boolean isPersistingState;
-  private Optional<String> revertReason;
+  private Optional<BytesValue> revertReason;
 
   // Miscellaneous fields.
   private final EnumSet<ExceptionalHaltReason> exceptionalHaltReasons =
@@ -254,7 +254,7 @@ private MessageFrame(
       final Address miningBeneficiary,
       final BlockHashLookup blockHashLookup,
       final Boolean isPersistingState,
-      final Optional<String> revertReason,
+      final Optional<BytesValue> revertReason,
       final int maxStackSize) {
     this.type = type;
     this.blockchain = blockchain;
@@ -511,11 +511,11 @@ public UInt256 memoryWordSize() {
    *
    * @return the revertReason string
    */
-  public Optional<String> getRevertReason() {
+  public Optional<BytesValue> getRevertReason() {
     return revertReason;
   }
 
-  public void setRevertReason(final String revertReason) {
+  public void setRevertReason(final BytesValue revertReason) {
     this.revertReason = Optional.ofNullable(revertReason);
   }
 
@@ -879,7 +879,7 @@ public static class Builder {
     private Address miningBeneficiary;
     private BlockHashLookup blockHashLookup;
     private Boolean isPersistingState = false;
-    private Optional<String> reason = Optional.empty();
+    private Optional<BytesValue> reason = Optional.empty();
 
     public Builder type(final Type type) {
       this.type = type;
@@ -997,7 +997,7 @@ public Builder isPersistingState(final Boolean isPersistingState) {
       return this;
     }
 
-    public Builder reason(final String reason) {
+    public Builder reason(final BytesValue reason) {
       this.reason = Optional.ofNullable(reason);
       return this;
     }

ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/vm/operations/RevertOperation.java

@@ -19,10 +19,7 @@
 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);
@@ -42,8 +39,7 @@ public void execute(final MessageFrame frame) {
     final UInt256 length = frame.popStackItem().asUInt256();
     BytesValue reason = frame.readMemory(from, length);
     frame.setOutputData(reason);
-    String reasonMessage = new String(reason.extractArray(), CHARSET);
-    frame.setRevertReason(reasonMessage);
+    frame.setRevertReason(reason);
     frame.setState(MessageFrame.State.REVERT);
   }
 }

ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/BlockDataGenerator.java

@@ -338,7 +338,7 @@ public TransactionReceipt receipt(final long cumulativeGasUsed) {
         hash(), cumulativeGasUsed, Arrays.asList(log(), log()), Optional.empty());
   }
 
-  public TransactionReceipt receipt(final String revertReason) {
+  public TransactionReceipt receipt(final BytesValue revertReason) {
     return new TransactionReceipt(
         hash(), positiveLong(), Arrays.asList(log(), log()), Optional.of(revertReason));
   }

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionReceiptTest.java

@@ -15,6 +15,7 @@
 import static org.junit.Assert.assertEquals;
 
 import tech.pegasys.pantheon.ethereum.rlp.RLP;
+import tech.pegasys.pantheon.util.bytes.BytesValue;
 
 import org.junit.Test;
 
@@ -32,7 +33,7 @@ public void toFromRlp() {
   @Test
   public void toFromRlpWithReason() {
     final BlockDataGenerator gen = new BlockDataGenerator();
-    final TransactionReceipt receipt = gen.receipt("RevertReason");
+    final TransactionReceipt receipt = gen.receipt(BytesValue.fromHexString("0x1122334455667788"));
     final TransactionReceipt copy =
         TransactionReceipt.readFrom(RLP.input(RLP.encode(receipt::writeToWithRevertReason)));
     assertEquals(receipt, copy);

ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/operations/RevertOperationTest.java

@@ -13,8 +13,6 @@
 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;
@@ -26,47 +24,33 @@
 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.Mock;
 import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
 
-@RunWith(Parameterized.class)
+@RunWith(MockitoJUnitRunner.class)
 public class RevertOperationTest {
 
-  private final String code;
-
-  private final MessageFrame messageFrame = mock(MessageFrame.class);
+  @Mock private MessageFrame messageFrame;
   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;
-  }
+  private final BytesValue revertReasonBytes =
+      BytesValue.fromHexString("726576657274206d657373616765");
 
   @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"));
+    when(messageFrame.readMemory(UInt256.ZERO, UInt256.of(0x0e))).thenReturn(revertReasonBytes);
   }
 
   @Test
   public void shouldReturnReason() {
-    assertTrue(code.contains("726576657274206d657373616765"));
-    ArgumentCaptor<String> arg = ArgumentCaptor.forClass(String.class);
+    ArgumentCaptor<BytesValue> arg = ArgumentCaptor.forClass(BytesValue.class);
     operation.execute(messageFrame);
     Mockito.verify(messageFrame).setRevertReason(arg.capture());
-    assertEquals("revert message", arg.getValue());
+    assertEquals(revertReasonBytes, arg.getValue());
   }
 }