[ETCM-921] properly decode SignedTransaction in BlockBody + scaladoc

Author: strauss-m

rlp/src/main/scala/io/iohk/ethereum/rlp/package.scala

@@ -44,7 +44,22 @@ package object rlp {
     override def toString: String = s"RLPValue(${Hex.toHexString(bytes)})"
   }
 
-  case class PrefixedRLPEncodable(prefix: Byte, prefixedRLPEncodeable: RLPEncodeable) extends RLPEncodeable
+  /** Modelise a RLPEncodable that should be binary prefixed by a raw byte.
+    *
+    * When converting this RLPEncodable to byte, the resulting value will be:
+    * prefix || prefixedRLPEncodable.toByte
+    * where || is the binary concatenation symbol.
+    *
+    * To be able to read back the data, use TypedTransaction.TypedTransactionsRLPAggregator
+    *
+    * This is for example used for typed transaction and typed receipt.
+    *
+    * @param prefix the raw byte
+    * @param prefixedRLPEncodeable the RLPEncodable to prefix with
+    */
+  case class PrefixedRLPEncodable(prefix: Byte, prefixedRLPEncodeable: RLPEncodeable) extends RLPEncodeable {
+    require(prefix <= 0x07f, "prefix should be lower than 0x7f")
+  }
 
   trait RLPEncoder[T] {
     def encode(obj: T): RLPEncodeable

src/main/scala/io/iohk/ethereum/domain/Block.scala

@@ -43,12 +43,13 @@ object Block {
 
   implicit class BlockDec(val bytes: Array[Byte]) extends AnyVal {
     import io.iohk.ethereum.network.p2p.messages.BaseETH6XMessages.SignedTransactions._
+    import io.iohk.ethereum.network.p2p.messages.BaseETH6XMessages.TypedTransaction._
     def toBlock: Block = rawDecode(bytes) match {
       case RLPList(header: RLPList, stx: RLPList, uncles: RLPList) =>
         Block(
           header.toBlockHeader,
           BlockBody(
-            stx.items.map(_.toSignedTransaction),
+            stx.items.toTypedRLPEncodables.map(_.toSignedTransaction),
             uncles.items.map(_.toBlockHeader)
           )
         )

src/main/scala/io/iohk/ethereum/domain/BlockBody.scala

@@ -23,6 +23,8 @@ object BlockBody {
 
   val empty: BlockBody = BlockBody(Seq.empty, Seq.empty)
 
+  import io.iohk.ethereum.network.p2p.messages.BaseETH6XMessages.TypedTransaction._
+
   def blockBodyToRlpEncodable(
       blockBody: BlockBody,
       signedTxToRlpEncodable: SignedTransaction => RLPEncodeable,
@@ -57,7 +59,7 @@ object BlockBody {
     rlpEncodeable match {
       case RLPList((transactions: RLPList), (uncles: RLPList)) =>
         BlockBody(
-          transactions.items.map(rlpEncodableToSignedTransaction),
+          transactions.items.toTypedRLPEncodables.map(rlpEncodableToSignedTransaction),
           uncles.items.map(rlpEncodableToBlockHeader)
         )
       case _ => throw new RuntimeException("Cannot decode BlockBody")

src/main/scala/io/iohk/ethereum/network/p2p/messages/BaseETH6XMessages.scala

@@ -117,13 +117,17 @@ object BaseETH6XMessages {
 
     implicit class NewBlockDec(val bytes: Array[Byte]) extends AnyVal {
       import SignedTransactions._
+      import TypedTransaction._
 
       def toNewBlock: NewBlock = rawDecode(bytes) match {
         case RLPList(RLPList(blockHeader, transactionList: RLPList, uncleNodesList: RLPList), totalDifficulty) =>
           NewBlock(
             Block(
               blockHeader.toBlockHeader,
-              BlockBody(transactionList.items.map(_.toSignedTransaction), uncleNodesList.items.map(_.toBlockHeader))
+              BlockBody(
+                transactionList.items.toTypedRLPEncodables.map(_.toSignedTransaction),
+                uncleNodesList.items.map(_.toBlockHeader)
+              )
             ),
             totalDifficulty
           )
@@ -154,6 +158,36 @@ object BaseETH6XMessages {
     override def code: Int = Codes.NewBlockCode
   }
 
+  object TypedTransaction {
+    implicit class TypedTransactionsRLPAggregator(val encodables: Seq[RLPEncodeable]) extends AnyVal {
+
+      /** Convert a Seq of RLPEncodable containing TypedTransaction informations into a Seq of
+        * Prefixed RLPEncodable.
+        *
+        * PrefixedRLPEncodable(prefix, prefixedRLPEncodable) generates binary data
+        * as prefix || RLPEncodable(prefixedRLPEncodable).
+        *
+        * As prefix is a byte value lower than 0x7f, it is read back as RLPValue(prefix),
+        * thus PrefixedRLPEncodable is binary equivalent to RLPValue(prefix), RLPEncodable
+        *
+        * The method aggregates back the typed transaction prefix with the following heuristic:
+        * - a RLPValue(byte) with byte < 07f + the following RLPEncodable are associated as a PrefixedRLPEncodable
+        * - all other RLPEncodable are kept unchanged
+        *
+        * This is the responsibility of the RLPDecoder to insert this meaning into its RLPList, when appropriate.
+        *
+        * @return a Seq of TypedTransaction enriched RLPEncodable
+        */
+      def toTypedRLPEncodables: Seq[RLPEncodeable] =
+        encodables match {
+          case Seq() => Seq()
+          case Seq(RLPValue(v), rlpList: RLPList, tail @ _*) if v.length == 1 && v.head < 0x7f =>
+            PrefixedRLPEncodable(v.head, rlpList) +: tail.toTypedRLPEncodables
+          case Seq(head, tail @ _*) => head +: tail.toTypedRLPEncodables
+        }
+    }
+  }
+
   object SignedTransactions {
 
     implicit class SignedTransactionEnc(val signedTx: SignedTransaction) extends RLPSerializable {
@@ -207,23 +241,25 @@ object BaseETH6XMessages {
 
     implicit class SignedTransactionsDec(val bytes: Array[Byte]) extends AnyVal {
 
-      def mergeTransactionType(encodables: Seq[RLPEncodeable]): Seq[RLPEncodeable] =
-        encodables match {
-          case Seq() => Seq()
-          case Seq(RLPValue(v), rlpList: RLPList, tail @ _*) if v.length == 1 =>
-            PrefixedRLPEncodable(v.head, rlpList) +: mergeTransactionType(tail)
-          case Seq(head, tail @ _*) => head +: mergeTransactionType(tail)
-        }
-      def toSignedTransactions: SignedTransactions =
-        rawDecode(bytes) match {
-          case rlpList: RLPList => SignedTransactions(mergeTransactionType(rlpList.items).map(_.toSignedTransaction))
-          case _                => throw new RuntimeException("Cannot decode SignedTransactions")
-        }
+      import TypedTransaction._
+
+      def toSignedTransactions: SignedTransactions = rawDecode(bytes) match {
+        case rlpList: RLPList => SignedTransactions(rlpList.items.toTypedRLPEncodables.map(_.toSignedTransaction))
+        case _                => throw new RuntimeException("Cannot decode SignedTransactions")
+      }
     }
 
     implicit class SignedTransactionRlpEncodableDec(val rlpEncodeable: RLPEncodeable) extends AnyVal {
 
       // scalastyle:off method.length
+
+      /** A signed transaction is either a RLPList representing a Legacy SignedTransaction
+        * or a PrefixedRLPEncodable(transactionType, RLPList of typed transaction envelope)
+        *
+        * @see TypedTransaction.TypedTransactionsRLPAggregator
+        *
+        * @return a SignedTransaction
+        */
       def toSignedTransaction: SignedTransaction = rlpEncodeable match {
         case PrefixedRLPEncodable(
               Transaction.Type01,

src/main/scala/io/iohk/ethereum/network/p2p/messages/ETC64.scala

@@ -105,6 +105,7 @@ object ETC64 {
 
     implicit class NewBlockDec(val bytes: Array[Byte]) extends AnyVal {
       import io.iohk.ethereum.network.p2p.messages.BaseETH6XMessages.SignedTransactions._
+      import io.iohk.ethereum.network.p2p.messages.BaseETH6XMessages.TypedTransaction._
 
       def toNewBlock: NewBlock = rawDecode(bytes) match {
         case RLPList(
@@ -115,7 +116,10 @@ object ETC64 {
           NewBlock(
             Block(
               blockHeader.toBlockHeader,
-              BlockBody(transactionList.items.map(_.toSignedTransaction), uncleNodesList.items.map(_.toBlockHeader))
+              BlockBody(
+                transactionList.items.toTypedRLPEncodables.map(_.toSignedTransaction),
+                uncleNodesList.items.map(_.toBlockHeader)
+              )
             ),
             ChainWeight(lastCheckpointNumber, totalDifficulty)
           )

src/test/scala/io/iohk/ethereum/domain/TransactionSpec.scala

@@ -43,31 +43,12 @@ class TransactionSpec
     forAll(signedTxSeqGen(2, secureRandom, None)) { (originalSignedTransactionSeq: Seq[SignedTransaction]) =>
       // encode it
       import SignedTransactions.SignedTransactionsEnc
-
-      // SignedTransactionsEnc is the Sequence version of SignedTransactionEnc
-      // the SignedTransactionsEnc.toBytes calls
-      // -> SignedTransactionsEnc.toRLPEncodable which maps over
-      // -> SignedTransactionEnc.toRLPEncodable (single signed transaction encoder)
-      // without going through the SignedTransactionEnc.toByte, which is the actual part where the 01 prefix is inserted
       val encodedSignedTransactionSeq: Array[Byte] = SignedTransactions(originalSignedTransactionSeq).toBytes
 
       // decode it
       import SignedTransactions.SignedTransactionsDec
-      // likewise, the SignedTransactionsDec.toSignedTransactions maps over
-      // -> SignedTransactionRlpEncodableDec.toSignedTransaction (single signed transaction)
-      // while ignoring the SignedTransactionDec(Array[Byte]), which is responsible to parse the prefix if available
       val SignedTransactions(decodedSignedTransactionSeq) = encodedSignedTransactionSeq.toSignedTransactions
 
-      // The test is working because both encoding and decoding are skipping the prefix part,
-      // and the rlp encoded transaction is different enough to recognize a LegacyTransaction from a TX1
-
-      // I see two problems:
-      // - the encoding is not compatible with other clients
-      // - this is not working for receipt, where legacy and tx1 receipt payload are the same. As such we can't
-      // distinguish which one it is without the prefix
-
-      // The root cause seems to be that the prefix stuff is done on a byte[] level,
-      // whereas RLPList are working on RLPEncodable
       decodedSignedTransactionSeq shouldEqual originalSignedTransactionSeq
     }
   }