RevertReason changed to BytesValue (#1746)

The revert reason returned from the EVM has been changed to a bytes value to
better represent the specification (of it being an ABI encoded data block).

This has meant that the TransactionReceipt returned from
ethGetTransactionReceipt now provides a hex-encoded string (rather than UTF-8).

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/condition/eth/ExpectSuccessfulEthGetTransactionReceiptWithReason.java

@@ -19,6 +19,10 @@
 import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node;
 import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.eth.EthGetTransactionReceiptWithRevertReason;
 
+import java.nio.charset.StandardCharsets;
+
+import org.web3j.utils.Numeric;
+
 public class ExpectSuccessfulEthGetTransactionReceiptWithReason implements Condition {
 
   private final EthGetTransactionReceiptWithRevertReason transaction;
@@ -39,8 +43,12 @@ public void verify(final Node node) {
   private boolean revertReasonMatches(final Node node, final String expectedRevertReason) {
     return node.execute(transaction)
         .filter(
-            transactionReceipt ->
-                transactionReceipt.getRevertReason().contains(expectedRevertReason))
+            transactionReceipt -> {
+              final byte[] bytes =
+                  Numeric.hexStringToByteArray(transactionReceipt.getRevertReason());
+              final String utf8Encoded = new String(bytes, StandardCharsets.UTF_8);
+              return utf8Encoded.contains(expectedRevertReason);
+            })
         .isPresent();
   }
 }

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/condition/eth/ExpectSuccessfulEthGetTransactionReceiptWithoutReason.java

@@ -37,7 +37,7 @@ public void verify(final Node node) {
   private boolean revertReasonIsEmpty(final Node node) {
     return node.execute(transaction)
         .map(TransactionReceiptWithRevertReason::getRevertReason)
-        .filter(String::isEmpty)
+        .filter(str -> str.equals("0x"))
         .isPresent();
   }
 }

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 Optional<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 Optional<BytesValue> revertReason) {
     this.pc = pc;
     this.opcode = opcode;
     this.gasRemaining = gasRemaining;
@@ -79,7 +80,7 @@ public TraceFrame(
         stack,
         memory,
         storage,
-        null);
+        Optional.empty());
   }
 
   public int getPc() {
@@ -118,7 +119,7 @@ public Optional<Map<UInt256, UInt256>> getStorage() {
     return storage;
   }
 
-  public String getRevertReason() {
+  public Optional<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/DebugOperationTracer.java

@@ -68,7 +68,7 @@ public void traceExecution(
               stack,
               memory,
               storage,
-              frame.getRevertReason().orElse(null)));
+              frame.getRevertReason()));
     }
   }
 

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);