fix roundtrip issues in TransactionRLPEncoderTest and standardize enc…

…oder/decoder classes

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/Transaction.java

@@ -87,7 +87,7 @@ public static Builder builder() {
   }
 
   public static Transaction readFrom(final RLPInput rlpInput) {
-    return TransactionRLPDecoder.decodeTransaction(rlpInput);
+    return TransactionRLPDecoder.decode(rlpInput);
   }
 
   /**

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPDecoder.java

@@ -33,20 +33,19 @@
 
 import org.apache.tuweni.bytes.Bytes;
 
-@FunctionalInterface
-public interface TransactionRLPDecoder {
+public class TransactionRLPDecoder {
 
-  static Transaction decodeTransaction(final RLPInput rlpInput) {
+  public static Transaction decode(final RLPInput rlpInput) {
     final Bytes typedTransactionBytes = rlpInput.raw();
     final int firstByte = typedTransactionBytes.get(0) & 0xff;
     final TransactionType transactionType = TransactionType.of(firstByte);
     if (transactionType.equals(TransactionType.FRONTIER)) {
-      return frontierDecoder().decode(rlpInput);
+      return decodeFrontier(rlpInput);
     } else {
       rlpInput.skipNext(); // throw away the type byte
       if (transactionType.equals(TransactionType.EIP1559)) {
         ExperimentalEIPs.eip1559MustBeEnabled();
-        return eip1559Decoder().decode(rlpInput);
+        return decodeEIP1559(rlpInput);
       } else {
         throw new IllegalStateException(
             String.format(
@@ -56,78 +55,70 @@ static Transaction decodeTransaction(final RLPInput rlpInput) {
     }
   }
 
-  Transaction decode(RLPInput input);
+  static Transaction decodeFrontier(final RLPInput input) {
+    input.enterList();
+    final Transaction.Builder builder =
+        Transaction.builder()
+            .nonce(input.readLongScalar())
+            .gasPrice(Wei.of(input.readUInt256Scalar()))
+            .gasLimit(input.readLongScalar())
+            .to(input.readBytes(v -> v.size() == 0 ? null : Address.wrap(v)))
+            .value(Wei.of(input.readUInt256Scalar()))
+            .payload(input.readBytes());
 
-  static TransactionRLPDecoder frontierDecoder() {
-    return input -> {
-      input.enterList();
-      final Transaction.Builder builder =
-          Transaction.builder()
-              .nonce(input.readLongScalar())
-              .gasPrice(Wei.of(input.readUInt256Scalar()))
-              .gasLimit(input.readLongScalar())
-              .to(input.readBytes(v -> v.size() == 0 ? null : Address.wrap(v)))
-              .value(Wei.of(input.readUInt256Scalar()))
-              .payload(input.readBytes());
-
-      final BigInteger v = input.readBigIntegerScalar();
-      final byte recId;
-      Optional<BigInteger> chainId = Optional.empty();
-      if (v.equals(REPLAY_UNPROTECTED_V_BASE) || v.equals(REPLAY_UNPROTECTED_V_BASE_PLUS_1)) {
-        recId = v.subtract(REPLAY_UNPROTECTED_V_BASE).byteValueExact();
-      } else if (v.compareTo(REPLAY_PROTECTED_V_MIN) > 0) {
-        chainId = Optional.of(v.subtract(REPLAY_PROTECTED_V_BASE).divide(TWO));
-        recId =
-            v.subtract(TWO.multiply(chainId.get()).add(REPLAY_PROTECTED_V_BASE)).byteValueExact();
-      } else {
-        throw new RuntimeException(
-            String.format("An unsupported encoded `v` value of %s was found", v));
-      }
-      final BigInteger r = input.readUInt256Scalar().toBytes().toUnsignedBigInteger();
-      final BigInteger s = input.readUInt256Scalar().toBytes().toUnsignedBigInteger();
-      final SECP256K1.Signature signature = SECP256K1.Signature.create(r, s, recId);
+    final BigInteger v = input.readBigIntegerScalar();
+    final byte recId;
+    Optional<BigInteger> chainId = Optional.empty();
+    if (v.equals(REPLAY_UNPROTECTED_V_BASE) || v.equals(REPLAY_UNPROTECTED_V_BASE_PLUS_1)) {
+      recId = v.subtract(REPLAY_UNPROTECTED_V_BASE).byteValueExact();
+    } else if (v.compareTo(REPLAY_PROTECTED_V_MIN) > 0) {
+      chainId = Optional.of(v.subtract(REPLAY_PROTECTED_V_BASE).divide(TWO));
+      recId = v.subtract(TWO.multiply(chainId.get()).add(REPLAY_PROTECTED_V_BASE)).byteValueExact();
+    } else {
+      throw new RuntimeException(
+          String.format("An unsupported encoded `v` value of %s was found", v));
+    }
+    final BigInteger r = input.readUInt256Scalar().toBytes().toUnsignedBigInteger();
+    final BigInteger s = input.readUInt256Scalar().toBytes().toUnsignedBigInteger();
+    final SECP256K1.Signature signature = SECP256K1.Signature.create(r, s, recId);
 
-      input.leaveList();
+    input.leaveList();
 
-      chainId.ifPresent(builder::chainId);
-      return builder.signature(signature).build();
-    };
+    chainId.ifPresent(builder::chainId);
+    return builder.signature(signature).build();
   }
 
-  static TransactionRLPDecoder eip1559Decoder() {
-    return input -> {
-      input.enterList();
+  static Transaction decodeEIP1559(final RLPInput input) {
+    input.enterList();
 
-      final Transaction.Builder builder =
-          Transaction.builder()
-              .nonce(input.readLongScalar())
-              .gasPrice(Wei.of(input.readUInt256Scalar()))
-              .gasLimit(input.readLongScalar())
-              .to(input.readBytes(v -> v.size() == 0 ? null : Address.wrap(v)))
-              .value(Wei.of(input.readUInt256Scalar()))
-              .payload(input.readBytes())
-              .gasPremium(Wei.of(input.readBytes().toBigInteger()))
-              .feeCap(Wei.of(input.readBytes().toBigInteger()));
+    final Transaction.Builder builder =
+        Transaction.builder()
+            .nonce(input.readLongScalar())
+            .gasPrice(Wei.of(input.readUInt256Scalar()))
+            .gasLimit(input.readLongScalar())
+            .to(input.readBytes(v -> v.size() == 0 ? null : Address.wrap(v)))
+            .value(Wei.of(input.readUInt256Scalar()))
+            .payload(input.readBytes())
+            .gasPremium(Wei.of(input.readBytes().toBigInteger()))
+            .feeCap(Wei.of(input.readBytes().toBigInteger()));
 
-      final BigInteger v = input.readBytes().toBigInteger();
-      final BigInteger r = input.readUInt256Scalar().toBytes().toUnsignedBigInteger();
-      final BigInteger s = input.readUInt256Scalar().toBytes().toUnsignedBigInteger();
-      final byte recId;
-      Optional<BigInteger> chainId = Optional.empty();
-      if (v.equals(REPLAY_UNPROTECTED_V_BASE) || v.equals(REPLAY_UNPROTECTED_V_BASE_PLUS_1)) {
-        recId = v.subtract(REPLAY_UNPROTECTED_V_BASE).byteValueExact();
-      } else if (v.compareTo(REPLAY_PROTECTED_V_MIN) > 0) {
-        chainId = Optional.of(v.subtract(REPLAY_PROTECTED_V_BASE).divide(TWO));
-        recId =
-            v.subtract(TWO.multiply(chainId.get()).add(REPLAY_PROTECTED_V_BASE)).byteValueExact();
-      } else {
-        throw new RuntimeException(
-            String.format("An unsupported encoded `v` value of %s was found", v));
-      }
-      final SECP256K1.Signature signature = SECP256K1.Signature.create(r, s, recId);
-      input.leaveList();
-      chainId.ifPresent(builder::chainId);
-      return builder.signature(signature).build();
-    };
+    final BigInteger v = input.readBytes().toBigInteger();
+    final BigInteger r = input.readUInt256Scalar().toBytes().toUnsignedBigInteger();
+    final BigInteger s = input.readUInt256Scalar().toBytes().toUnsignedBigInteger();
+    final byte recId;
+    Optional<BigInteger> chainId = Optional.empty();
+    if (v.equals(REPLAY_UNPROTECTED_V_BASE) || v.equals(REPLAY_UNPROTECTED_V_BASE_PLUS_1)) {
+      recId = v.subtract(REPLAY_UNPROTECTED_V_BASE).byteValueExact();
+    } else if (v.compareTo(REPLAY_PROTECTED_V_MIN) > 0) {
+      chainId = Optional.of(v.subtract(REPLAY_PROTECTED_V_BASE).divide(TWO));
+      recId = v.subtract(TWO.multiply(chainId.get()).add(REPLAY_PROTECTED_V_BASE)).byteValueExact();
+    } else {
+      throw new RuntimeException(
+          String.format("An unsupported encoded `v` value of %s was found", v));
+    }
+    final SECP256K1.Signature signature = SECP256K1.Signature.create(r, s, recId);
+    input.leaveList();
+    chainId.ifPresent(builder::chainId);
+    return builder.signature(signature).build();
   }
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPEncoder.java

@@ -30,11 +30,11 @@
 public class TransactionRLPEncoder {
 
   private static final ImmutableMap<TransactionType, Encoder> TYPED_TRANSACTION_ENCODERS =
-      ImmutableMap.of(TransactionType.EIP1559, eip1559Encoder());
+      ImmutableMap.of(TransactionType.EIP1559, TransactionRLPEncoder::encodeEIP1559);
 
   public static void encode(final Transaction transaction, final RLPOutput output) {
     if (transaction.getType().equals(TransactionType.FRONTIER)) {
-      frontierEncoder().encode(transaction, output);
+      encodeFrontier(transaction, output);
     } else {
       final BytesValueRLPOutput bytesValueRLPOutput = new BytesValueRLPOutput();
       Optional.ofNullable(TYPED_TRANSACTION_ENCODERS.get(transaction.getType()))
@@ -48,41 +48,37 @@ public static void encode(final Transaction transaction, final RLPOutput output)
     }
   }
 
-  static Encoder frontierEncoder() {
-    return (transaction, out) -> {
-      out.startList();
-      out.writeLongScalar(transaction.getNonce());
-      out.writeUInt256Scalar(transaction.getGasPrice());
-      out.writeLongScalar(transaction.getGasLimit());
-      out.writeBytes(transaction.getTo().map(Bytes::copy).orElse(Bytes.EMPTY));
-      out.writeUInt256Scalar(transaction.getValue());
-      out.writeBytes(transaction.getPayload());
-      writeSignature(transaction, out);
-      out.endList();
-    };
+  static void encodeFrontier(final Transaction transaction, final RLPOutput out) {
+    out.startList();
+    out.writeLongScalar(transaction.getNonce());
+    out.writeUInt256Scalar(transaction.getGasPrice());
+    out.writeLongScalar(transaction.getGasLimit());
+    out.writeBytes(transaction.getTo().map(Bytes::copy).orElse(Bytes.EMPTY));
+    out.writeUInt256Scalar(transaction.getValue());
+    out.writeBytes(transaction.getPayload());
+    writeSignature(transaction, out);
+    out.endList();
   }
 
-  static Encoder eip1559Encoder() {
-    return (transaction, out) -> {
-      if (!ExperimentalEIPs.eip1559Enabled
-          || !TransactionType.EIP1559.equals(transaction.getType())) {
-        throw new RuntimeException("Invalid transaction format");
-      }
+  static void encodeEIP1559(final Transaction transaction, final RLPOutput out) {
+    if (!ExperimentalEIPs.eip1559Enabled
+        || !TransactionType.EIP1559.equals(transaction.getType())) {
+      throw new RuntimeException("Invalid transaction format");
+    }
 
-      out.startList();
-      out.writeLongScalar(transaction.getNonce());
-      out.writeNull();
-      out.writeLongScalar(transaction.getGasLimit());
-      out.writeBytes(transaction.getTo().map(Bytes::copy).orElse(Bytes.EMPTY));
-      out.writeUInt256Scalar(transaction.getValue());
-      out.writeBytes(transaction.getPayload());
-      out.writeUInt256Scalar(
-          transaction.getGasPremium().map(Quantity::getValue).map(Wei::ofNumber).orElseThrow());
-      out.writeUInt256Scalar(
-          transaction.getFeeCap().map(Quantity::getValue).map(Wei::ofNumber).orElseThrow());
-      writeSignature(transaction, out);
-      out.endList();
-    };
+    out.startList();
+    out.writeLongScalar(transaction.getNonce());
+    out.writeNull();
+    out.writeLongScalar(transaction.getGasLimit());
+    out.writeBytes(transaction.getTo().map(Bytes::copy).orElse(Bytes.EMPTY));
+    out.writeUInt256Scalar(transaction.getValue());
+    out.writeBytes(transaction.getPayload());
+    out.writeUInt256Scalar(
+        transaction.getGasPremium().map(Quantity::getValue).map(Wei::ofNumber).orElseThrow());
+    out.writeUInt256Scalar(
+        transaction.getFeeCap().map(Quantity::getValue).map(Wei::ofNumber).orElseThrow());
+    writeSignature(transaction, out);
+    out.endList();
   }
 
   private static void writeSignature(final Transaction transaction, final RLPOutput out) {

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/TransactionRLPDecoderTest.java

@@ -35,7 +35,7 @@ public class TransactionRLPDecoderTest {
   @Test
   public void decodeFrontierNominalCase() {
     final Transaction transaction =
-        TransactionRLPDecoder.decodeTransaction(RLP.input(Bytes.fromHexString(FRONTIER_TX_RLP)));
+        TransactionRLPDecoder.decode(RLP.input(Bytes.fromHexString(FRONTIER_TX_RLP)));
     assertThat(transaction).isNotNull();
     assertThat(transaction.getGasPrice()).isEqualByComparingTo(Wei.of(50L));
     assertThat(transaction.getGasPremium()).isEmpty();
@@ -46,7 +46,7 @@ public void decodeFrontierNominalCase() {
   public void decodeEIP1559NominalCase() {
     ExperimentalEIPs.eip1559Enabled = true;
     final Transaction transaction =
-        TransactionRLPDecoder.decodeTransaction(RLP.input(Bytes.fromHexString(EIP1559_TX_RLP)));
+        TransactionRLPDecoder.decode(RLP.input(Bytes.fromHexString(EIP1559_TX_RLP)));
     assertThat(transaction).isNotNull();
     assertThat(transaction.getGasPremium()).hasValue(Wei.of(527L));
     assertThat(transaction.getFeeCap()).hasValue(Wei.of(369L));
@@ -57,9 +57,7 @@ public void decodeEIP1559NominalCase() {
   public void decodeEIP1559FailureWhenNotEnabled() {
     ExperimentalEIPs.eip1559Enabled = false;
     assertThatThrownBy(
-            () ->
-                TransactionRLPDecoder.decodeTransaction(
-                    RLP.input(Bytes.fromHexString(EIP1559_TX_RLP))))
+            () -> TransactionRLPDecoder.decode(RLP.input(Bytes.fromHexString(EIP1559_TX_RLP))))
         .isInstanceOf(RuntimeException.class)
         .hasMessageContaining("EIP-1559 feature flag");
     ExperimentalEIPs.eip1559Enabled = ExperimentalEIPs.EIP1559_ENABLED_DEFAULT_VALUE;