fix rlp.readRecordType of BlockBody and friends

including:
- fix rlp.append of BlockHeader
- add tests for rlp.readRecordType of BlockBody
- add tests for rlp.append of BlockHeader
- add tests for EthBlock roundtrip

eth/common/eth_types_rlp.nim

@@ -317,8 +317,16 @@ proc append*(rlpWriter: var RlpWriter, t: Time) {.inline.} =
 proc append*(w: var RlpWriter, h: BlockHeader) =
   var len = 15
   if h.fee.isSome: inc len
-  if h.withdrawalsRoot.isSome: inc len
-  if h.excessDataGas.isSome: inc len
+  if h.withdrawalsRoot.isSome:
+    if h.fee.isNone:
+      raise newException(RlpError, "baseFee expected")
+    inc len
+  if h.excessDataGas.isSome:
+    if h.fee.isNone:
+      raise newException(RlpError, "baseFee expected")
+    if h.withdrawalsRoot.isNone:
+      raise newException(RlpError, "withdrawalsRoot expected")
+    inc len
   w.startList(len)
   for k, v in fieldPairs(h):
     when v isnot Option:
@@ -364,17 +372,20 @@ proc append*(w: var RlpWriter, b: BlockBody) =
   if b.withdrawals.isSome:
     w.append(b.withdrawals.unsafeGet)
 
-# Is there a better way of doing this? We have tests that call
-# rlp.readRecordType(BlockBody, false), so I overrode
-# `readRecordType` as well as `read`. --Adam
 proc readRecordType*(rlp: var Rlp, T: type BlockBody, wrappedInList: bool): BlockBody =
   if not wrappedInList:
     result.transactions = rlp.read(seq[Transaction])
     result.uncles = rlp.read(seq[BlockHeader])
-    # Is this the right thing to do here? I don't really
-    # understand what wrappedInList is used for. --Adam
+
+    const
+      # If in the future Withdrawal have optional fields
+      # we should put it into consideration
+      wdFieldsCount = rlpFieldsCount(Withdrawal)
+
     result.withdrawals =
-      if rlp.hasData:
+      if rlp.hasData and
+         rlp.isList and
+         rlp.listLen == wdFieldsCount:
         some(rlp.read(seq[Withdrawal]))
       else:
         none[seq[Withdrawal]]()

tests/rlp/all_tests.nim

@@ -2,4 +2,5 @@ import
   ./test_api_usage,
   ./test_common,
   ./test_json_suite,
-  ./test_object_serialization
+  ./test_object_serialization,
+  ./test_rlp_codec

tests/rlp/test_rlp_codec.nim

@@ -0,0 +1,117 @@
+{.used.}
+
+import
+  std/[os, strutils],
+  stew/[io2, results],
+  unittest2,
+  ../../eth/[rlp, common]
+
+type
+  # trick the rlp decoder
+  # so we can separate the body and header
+  EthHeader = object
+    header: BlockHeader
+
+proc importBlock(blocksRlp: openArray[byte]): bool =
+  var
+    # the encoded rlp can contains one or more blocks
+    rlp = rlpFromBytes(blocksRlp)
+
+  while rlp.hasData:
+    let
+      header = rlp.read(EthHeader).header
+      body = rlp.readRecordType(BlockBody, false)
+
+  true
+
+proc runTest(importFile: string): bool =
+  let res = io2.readAllBytes(importFile)
+  if res.isErr:
+    echo "failed to import", importFile
+    return
+
+  importBlock(res.get)
+
+suite "Partial EthBlock read using rlp.read and rlp.readRecordType":
+  for filename in walkDirRec("tests/rlp/rlps"):
+    if not filename.endsWith(".rlp"):
+      continue
+    test filename:
+      check runTest(filename)
+
+func `==`(a, b: ChainId): bool =
+  a.uint == b.uint
+  
+template roundTrip(blk: EthBlock) =
+  let bytes = rlp.encode(blk)
+  let blk2 = rlp.decode(bytes, EthBlock)
+  check blk2 == blk
+
+template roundTrip(h: BlockHeader) =
+  let bytes = rlp.encode(h)
+  let h2 = rlp.decode(bytes, BlockHeader)
+  check h2 == h
+
+suite "BlockHeader roundtrip test":
+  test "Empty header":
+    let h = BlockHeader()
+    roundTrip(h)
+
+  test "Header with gas":
+    let h = BlockHeader(gasLimit: 10.GasInt, gasUsed: 11.GasInt)
+    roundTrip(h)
+    
+  test "Header + some(baseFee)":
+    let h = BlockHeader(fee: some(1.u256))
+    roundTrip(h)
+  
+  test "Header + none(baseFee) + some(withdrawalsRoot)":
+    let h = BlockHeader(withdrawalsRoot: some(Hash256()))
+    expect RlpError:
+      roundTrip(h)
+  
+  test "Header + none(baseFee) + some(withdrawalsRoot) + some(excessDataGas)":
+    let h = BlockHeader(
+      withdrawalsRoot: some(Hash256()), 
+      excessDataGas: some(1.u256)
+    )
+    expect RlpError:
+      roundTrip(h)
+  
+  test "Header + none(baseFee) + none(withdrawalsRoot) + some(excessDataGas)":
+    let h = BlockHeader(      
+      excessDataGas: some(1.u256)
+    )
+    expect RlpError:
+      roundTrip(h)
+  
+  test "Header + some(baseFee) + none(withdrawalsRoot) + some(excessDataGas)":
+    let h = BlockHeader(
+      fee: some(2.u256),
+      excessDataGas: some(1.u256)
+    )
+    expect RlpError:
+      roundTrip(h)
+      
+  test "Header + some(baseFee) + some(withdrawalsRoot)":
+    let h = BlockHeader(
+      fee: some(2.u256),
+      withdrawalsRoot: some(Hash256())
+    )
+    roundTrip(h)
+  
+  test "Header + some(baseFee) + some(withdrawalsRoot) + some(excessDataGas)":
+    let h = BlockHeader(
+      fee: some(2.u256),
+      withdrawalsRoot: some(Hash256()),
+      excessDataGas: some(1.u256)
+    )
+    
+suite "EthBlock roundtrip test":
+  test "Empty EthBlock":
+    let blk = EthBlock()
+    roundTrip(blk)
+
+  test "EthBlock with withdrawals":
+    let blk = EthBlock(withdrawals: some(@[Withdrawal()]))
+    roundTrip(blk)