rlp codec support optional fields (#613)

Instead of patching BlockHeader or BlockBody codec
each time it get additional optional fields,
this PR make the rlp codec automatically handle
optional fields. Thus rlp codec overloading of
EthBlock, BlockHeader, and BlockBody can be removed.

doc/rlp.md

@@ -126,6 +126,25 @@ In rare circumstances, you may need to serialize the same field type
 differently depending on the enclosing object type. You can use the
 `rlpCustomSerialization` pragma to achieve this.
 
+### Optional fields
+
+Both `Option[T]` of `std/options` and `Opt[T]` of `stew/results` are supported.
+But the decoder and encoder assume optional fields are always added at the end of the RLP object.
+You can never set a field to `None` unless all following fields are also `None`.
+
+```nim
+## Example
+
+type
+  RlpObject = object
+    size: int
+    color: Option[int]
+    width: Opt[int]
+```
+
+If `color` is `none`, `width` should also `none`. If `color` is `some`, `width` can be both.
+If `color` is `none`, but `width` is some, it will raise assertion error.
+
 ### Contributing / Testing
 
 To test the correctness of any modifications to the library, please execute

eth/common/eth_types_rlp.nim

@@ -113,13 +113,6 @@ proc append*(w: var RlpWriter, tx: Transaction) =
   of TxEip1559:
     w.appendTxEip1559(tx)
 
-proc append*(w: var RlpWriter, withdrawal: Withdrawal) =
-  w.startList(4)
-  w.append(withdrawal.index)
-  w.append(withdrawal.validatorIndex)
-  w.append(withdrawal.address)
-  w.append(withdrawal.amount)
-
 template read[T](rlp: var Rlp, val: var T)=
   val = rlp.read(type val)
 
@@ -314,126 +307,11 @@ proc read*(rlp: var Rlp, T: type HashOrNum): T =
 proc append*(rlpWriter: var RlpWriter, t: Time) {.inline.} =
   rlpWriter.append(t.toUnix())
 
-proc append*(w: var RlpWriter, h: BlockHeader) =
-  var len = 15
-  if h.fee.isSome: inc len
-  if h.withdrawalsRoot.isSome:
-    doAssert(h.fee.isSome, "baseFee expected")
-    inc len
-  if h.excessDataGas.isSome:
-    doAssert(h.fee.isSome, "baseFee expected")
-    doAssert(h.withdrawalsRoot.isSome, "withdrawalsRoot expected")
-    inc len
-  w.startList(len)
-  for k, v in fieldPairs(h):
-    when v isnot Option:
-      w.append(v)
-  if h.fee.isSome:
-    w.append(h.fee.get())
-  if h.withdrawalsRoot.isSome:
-    w.append(h.withdrawalsRoot.get())
-  if h.excessDataGas.isSome:
-    w.append(h.excessDataGas.get())
-
-proc read*(rlp: var Rlp, T: type BlockHeader): T =
-  let len = rlp.listLen
-
-  if len notin {15, 16, 17, 18}:
-    raise newException(UnsupportedRlpError,
-      "BlockHeader elems should be 15, 16, 17, or 18 got " & $len)
-
-  rlp.tryEnterList()
-  for k, v in fieldPairs(result):
-    when v isnot Option:
-      v = rlp.read(type v)
-
-  if len >= 16:
-    # EIP-1559
-    result.baseFee = rlp.read(UInt256)
-  if len >= 17:
-    # EIP-4895
-    result.withdrawalsRoot = some rlp.read(Hash256)
-  if len >= 18:
-    # EIP-4844
-    result.excessDataGas = some rlp.read(UInt256)
-
 proc rlpHash*[T](v: T): Hash256 =
   keccakHash(rlp.encode(v))
 
 func blockHash*(h: BlockHeader): KeccakHash {.inline.} = rlpHash(h)
 
-proc append*(w: var RlpWriter, b: BlockBody) =
-  w.startList 2 + b.withdrawals.isSome.ord
-  w.append(b.transactions)
-  w.append(b.uncles)
-  if b.withdrawals.isSome:
-    w.append(b.withdrawals.unsafeGet)
-
-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])
-
-    const
-      # If in the future Withdrawal have optional fields
-      # we should put it into consideration
-      wdFieldsCount = rlpFieldsCount(Withdrawal)
-
-    result.withdrawals =
-      if rlp.hasData and
-         rlp.isList and
-         rlp.listLen == wdFieldsCount:
-        some(rlp.read(seq[Withdrawal]))
-      else:
-        none[seq[Withdrawal]]()
-  else:
-    let len = rlp.listLen
-
-    if len notin {2, 3}:
-      raise newException(UnsupportedRlpError,
-        "BlockBody elems should be 2 or 3, got " & $len)
-
-    rlp.tryEnterList()
-
-    result.transactions = rlp.read(seq[Transaction])
-    result.uncles       = rlp.read(seq[BlockHeader])
-
-    # EIP-4895
-    result.withdrawals =
-      if len >= 3:
-        some(rlp.read(seq[Withdrawal]))
-      else:
-        none[seq[Withdrawal]]()
-
-proc read*(rlp: var Rlp, T: type BlockBody): T =
-  rlp.readRecordType(BlockBody, true)
-
-proc read*(rlp: var Rlp, T: type EthBlock): T =
-  let len = rlp.listLen
-  if len notin {3, 4}:
-    raise newException(UnsupportedRlpError,
-      "EthBlock elems should be 3 or 4, got " & $len)
-
-  rlp.tryEnterList()
-  result.header = rlp.read(BlockHeader)
-  result.txs    = rlp.read(seq[Transaction])
-  result.uncles = rlp.read(seq[BlockHeader])
-
-  # EIP-4895
-  result.withdrawals =
-    if len >= 4:
-      some(rlp.read(seq[Withdrawal]))
-    else:
-      none[seq[Withdrawal]]()
-
-proc append*(w: var RlpWriter, b: EthBlock) =
-  w.startList 3 + b.withdrawals.isSome.ord
-  w.append(b.header)
-  w.append(b.txs)
-  w.append(b.uncles)
-  if b.withdrawals.isSome:
-    w.append(b.withdrawals.unsafeGet)
-
 proc append*(rlpWriter: var RlpWriter, id: NetworkId) =
   rlpWriter.append(id.uint)
 

eth/rlp.nim

@@ -3,8 +3,8 @@
 ## https://ethereum.github.io/yellowpaper/paper.pdf
 
 import
-  std/[macros, strutils],
+  std/[strutils, options],
-  stew/byteutils,
+  stew/[byteutils, shims/macros, results],
   ./rlp/[writer, object_serialization],
   ./rlp/priv/defs
 
@@ -365,6 +365,7 @@ proc readImpl(rlp: var Rlp, T: type[object|tuple],
               wrappedInList = wrapObjsInList): T =
   mixin enumerateRlpFields, read
 
+  var payloadEnd = rlp.bytes.len
   if wrappedInList:
     if not rlp.isList:
       raise newException(RlpTypeMismatch,
@@ -373,15 +374,39 @@ proc readImpl(rlp: var Rlp, T: type[object|tuple],
       payloadOffset = rlp.payloadOffset()
 
     # there's an exception-raising side effect in there *sigh*
-    discard rlp.payloadBytesCount()
+    payloadEnd = rlp.position + payloadOffset + rlp.payloadBytesCount()
 
     rlp.position += payloadOffset
 
-  template op(field) =
+  template getUnderlyingType[T](_: Option[T]): untyped = T
-    when hasCustomPragma(field, rlpCustomSerialization):
+  template getUnderlyingType[T](_: Opt[T]): untyped = T
-      field = rlp.read(result, type(field))
+
+  template op(RecordType, fieldName, field) =
+    type FieldType {.used.} = type field
+    when hasCustomPragmaFixed(RecordType, fieldName, rlpCustomSerialization):
+      field = rlp.read(result, FieldType)
+    elif field is Option:
+      # this works for optional fields at the end of an object/tuple
+      # if the optional field is followed by a mandatory field,
+      # custom serialization for a field or for the parent object
+      # will be better
+      type UT = getUnderlyingType(field)
+      if rlp.position < payloadEnd:
+        field = some(rlp.read(UT))
       else:
-      field = rlp.read(type(field))
+        field = none(UT)
+    elif field is Opt:
+      # this works for optional fields at the end of an object/tuple
+      # if the optional field is followed by a mandatory field,
+      # custom serialization for a field or for the parent object
+      # will be better
+      type UT = getUnderlyingType(field)
+      if rlp.position < payloadEnd:
+        field = Opt.some(rlp.read(UT))
+      else:
+        field = Opt.none(UT)
+    else:
+      field = rlp.read(FieldType)
 
   enumerateRlpFields(result, op)
 

eth/rlp/object_serialization.nim

@@ -1,4 +1,5 @@
-import std/macros
+import
+  stew/shims/macros
 
 template rlpIgnore* {.pragma.}
   ## Specifies that a certain field should be ignored for the purposes
@@ -10,16 +11,17 @@ template rlpCustomSerialization* {.pragma.}
   ## a reference to the object holding the field.
 
 template enumerateRlpFields*[T](x: T, op: untyped) =
-  for f in fields(x):
+  type RecordType = type x
-    when not hasCustomPragma(f, rlpIgnore):
+  for fieldName, field in fieldPairs(x):
-      op(f)
+    when not hasCustomPragmaFixed(RecordType, fieldName, rlpIgnore):
+      op(RecordType, fieldName, field)
 
 proc rlpFieldsCount*(T: type): int =
   mixin enumerateRlpFields
 
   proc helper: int =
     var dummy: T
-    template countFields(x) = inc result
+    template countFields(RT, n, x) = inc result
     enumerateRlpFields(dummy, countFields)
 
   const res = helper()
@@ -32,7 +34,8 @@ macro rlpFields*(T: typedesc, fields: varargs[untyped]): untyped =
     op = genSym(nskParam, "op")
 
   for field in fields:
-    body.add quote do: `op`(`ins`.`field`)
+    let fieldName = $field
+    body.add quote do: `op`(`T`, `fieldName`, `ins`.`field`)
 
   result = quote do:
     template enumerateRlpFields*(`ins`: `T`, `op`: untyped) {.inject.} =

eth/rlp/writer.nim

@@ -1,5 +1,6 @@
 import
-  std/macros,
+  std/options,
+  stew/[shims/macros, results],
   ./object_serialization, ./priv/defs
 
 type
@@ -181,15 +182,131 @@ proc appendImpl[T](self: var RlpWriter, listOrBlob: openArray[T]) =
     for i in 0 ..< listOrBlob.len:
       self.append listOrBlob[i]
 
+proc hasOptionalFields(T: type): bool =
+  mixin enumerateRlpFields
+
+  proc helper: bool =
+    var dummy: T
+    template detectOptionalField(RT, n, x) =
+      when x is Option or x is Opt:
+        return true
+    enumerateRlpFields(dummy, detectOptionalField)
+    false
+
+  const res = helper()
+  return res
+
+proc optionalFieldsNum(x: openArray[bool]): int =
+  # count optional fields backward
+  for i in countdown(x.len-1, 0):
+    if x[i]: inc result
+    else: break
+
+proc checkedOptionalFields(T: type, FC: static[int]): int =
+  mixin enumerateRlpFields
+
+  var
+    i = 0
+    dummy: T
+    res: array[FC, bool]
+
+  template op(RT, fN, f) =
+    res[i] = f is Option or f is Opt
+    inc i
+
+  enumerateRlpFields(dummy, op)
+
+  # ignoring first optional fields
+  optionalFieldsNum(res) - 1
+
+proc genPrevFields(obj: NimNode, fd: openArray[FieldDescription], hi, lo: int): NimNode =
+  result = newStmtList()
+  for i in countdown(hi, lo):
+    let fieldName = fd[i].name
+    let msg = fieldName.strVal & " expected"
+    result.add quote do:
+      doAssert(`obj`.`fieldName`.isSome, `msg`)
+
+macro genOptionalFieldsValidation(obj: untyped, T: type, num: static[int]): untyped =
+  let
+    Tresolved = getType(T)[1]
+    fd = recordFields(Tresolved.getImpl)
+    loidx = fd.len-num
+
+  result = newStmtList()
+  for i in countdown(fd.high, loidx):
+    let fieldName = fd[i].name
+    let prevFields = genPrevFields(obj, fd, i-1, loidx-1)
+    result.add quote do:
+      if `obj`.`fieldName`.isSome:
+        `prevFields`
+
+  # generate something like
+  when false:
+    if obj.excessDataGas.isSome:
+      doAssert(obj.withdrawalsRoot.isSome, "withdrawalsRoot expected")
+      doAssert(obj.fee.isSome, "fee expected")
+    if obj.withdrawalsRoot.isSome:
+      doAssert(obj.fee.isSome, "fee expected")
+
+macro countFieldsRuntimeImpl(obj: untyped, T: type, num: static[int]): untyped =
+  let
+    Tresolved = getType(T)[1]
+    fd = recordFields(Tresolved.getImpl)
+    res = ident("result")
+    mlen = fd.len - num
+
+  result = newStmtList()
+  result.add quote do:
+    `res` = `mlen`
+
+  for i in countdown(fd.high, fd.len-num):
+    let fieldName = fd[i].name
+    result.add quote do:
+      `res` += `obj`.`fieldName`.isSome.ord
+
+proc countFieldsRuntime(obj: object|tuple): int =
+  # count mandatory fields and non empty optional fields
+  type ObjType = type obj
+
+  const
+    fieldsCount = ObjType.rlpFieldsCount
+    # include first optional fields
+    cof = checkedOptionalFields(ObjType, fieldsCount) + 1
+
+  countFieldsRuntimeImpl(obj, ObjType, cof)
+
 proc appendRecordType*(self: var RlpWriter, obj: object|tuple, wrapInList = wrapObjsInList) =
   mixin enumerateRlpFields, append
 
-  if wrapInList:
+  type ObjType = type obj
-    self.startList(static obj.type.rlpFieldsCount)
 
-  template op(field) =
+  const
-    when hasCustomPragma(field, rlpCustomSerialization):
+    hasOptional = hasOptionalFields(ObjType)
+    fieldsCount = ObjType.rlpFieldsCount
+
+  when hasOptional:
+    const
+      cof = checkedOptionalFields(ObjType, fieldsCount)
+    when cof > 0:
+      genOptionalFieldsValidation(obj, ObjType, cof)
+
+  if wrapInList:
+    when hasOptional:
+      self.startList(obj.countFieldsRuntime)
+    else:
+      self.startList(fieldsCount)
+
+  template op(RecordType, fieldName, field) =
+    when hasCustomPragmaFixed(RecordType, fieldName, rlpCustomSerialization):
       append(self, obj, field)
+    elif (field is Option or field is Opt) and hasOptional:
+      # this works for optional fields at the end of an object/tuple
+      # if the optional field is followed by a mandatory field,
+      # custom serialization for a field or for the parent object
+      # will be better
+      if field.isSome:
+        append(self, field.unsafeGet)    
     else:
       append(self, field)
 

tests/rlp/test_rlp_codec.nim

@@ -6,21 +6,16 @@ import
   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
+    let blk = rlp.read(EthBlock)
-      header = rlp.read(EthHeader).header
+    if blk.withdrawals.isSome:
-      body = rlp.readRecordType(BlockBody, false)
+      # all of these blocks are pre shanghai blocks
+      return false
 
   true
 
@@ -32,7 +27,7 @@ proc runTest(importFile: string): bool =
 
   importBlock(res.get)
 
-suite "Partial EthBlock read using rlp.read and rlp.readRecordType":
+suite "Decode multiple EthBlock from bytes":
   for filename in walkDirRec("tests/rlp/rlps"):
     if not filename.endsWith(".rlp"):
       continue
@@ -42,15 +37,11 @@ suite "Partial EthBlock read using rlp.read and rlp.readRecordType":
 func `==`(a, b: ChainId): bool =
   a.uint == b.uint
 
-template roundTrip(blk: EthBlock) =
+template roundTrip(x) =
-  let bytes = rlp.encode(blk)
+  type TT = type(x)
-  let blk2 = rlp.decode(bytes, EthBlock)
+  let bytes = rlp.encode(x)
-  check blk2 == blk
+  let xx = rlp.decode(bytes, TT)
-
+  check xx == x
-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":
@@ -106,12 +97,131 @@ suite "BlockHeader roundtrip test":
       withdrawalsRoot: some(Hash256()),
       excessDataGas: some(1.u256)
     )
+    roundTrip(h)
 
-suite "EthBlock roundtrip test":
+template roundTrip2(a1, a2, body: untyped) =
-  test "Empty EthBlock":
+  type TT = type(a1)
-    let blk = EthBlock()
+  when type(a2) isnot TT:
+    {.error: "mismatch type".}
+  var bytes = rlp.encode(a1)
+  bytes.add rlp.encode(a2)
+  var r = rlpFromBytes(bytes)
+  let
+    b1 {.inject.} = r.read(TT)
+    b2 {.inject.} = r.read(TT)
+  check b1 == a1
+  check b2 == a2
+  body
+
+
+template genTest(TT) =
+  const TTS = astToStr(TT)
+  suite TTS & " roundtrip test":
+    test "Empty " & TTS:
+      let blk = TT()
       roundTrip(blk)
 
-  test "EthBlock with withdrawals":
+    test TTS & " with withdrawals":
-    let blk = EthBlock(withdrawals: some(@[Withdrawal()]))
+      let blk = TT(withdrawals: some(@[Withdrawal()]))
       roundTrip(blk)
+
+    test "2 " & TTS & " none(Withdrawal)+none(Withdrawal)":
+      let blk = TT()
+      roundTrip2(blk, blk):
+        check b1.withdrawals.isNone
+        check b2.withdrawals.isNone
+
+    test "2 " & TTS & " none(Withdrawal)+some(Withdrawal)":
+      let blk1 = TT()
+      let blk2 = TT(withdrawals: some(@[Withdrawal()]))
+      roundTrip2(blk1, blk2):
+        check b1.withdrawals.isNone
+        check b2.withdrawals.isSome
+
+    test "2 " & TTS & " some(Withdrawal)+none(Withdrawal)":
+      let blk1 = TT()
+      let blk2 = TT(withdrawals: some(@[Withdrawal()]))
+      roundTrip2(blk2, blk1):
+        check b1.withdrawals.isSome
+        check b2.withdrawals.isNone
+
+    test "2 " & TTS & " some(Withdrawal)+some(Withdrawal)":
+      let blk = TT(withdrawals: some(@[Withdrawal()]))
+      roundTrip2(blk, blk):
+        check b1.withdrawals.isSome
+        check b2.withdrawals.isSome
+
+genTest(EthBlock)
+genTest(BlockBody)
+
+type
+  BlockHeaderOpt* = object
+    parentHash*:      Hash256
+    ommersHash*:      Hash256
+    coinbase*:        EthAddress
+    stateRoot*:       Hash256
+    txRoot*:          Hash256
+    receiptRoot*:     Hash256
+    bloom*:           BloomFilter
+    difficulty*:      DifficultyInt
+    blockNumber*:     BlockNumber
+    gasLimit*:        GasInt
+    gasUsed*:         GasInt
+    timestamp*:       EthTime
+    extraData*:       Blob
+    mixDigest*:       Hash256
+    nonce*:           BlockNonce
+    fee*:             Opt[UInt256]
+    withdrawalsRoot*: Opt[Hash256]
+    excessDataGas*:   Opt[UInt256]
+
+  BlockBodyOpt* = object
+    transactions*:  seq[Transaction]
+    uncles*:        seq[BlockHeaderOpt]
+    withdrawals*:   Opt[seq[Withdrawal]]
+
+  EthBlockOpt* = object
+    header*     : BlockHeader
+    txs*        : seq[Transaction]
+    uncles*     : seq[BlockHeaderOpt]
+    withdrawals*: Opt[seq[Withdrawal]]
+
+template genTestOpt(TT) =
+  const TTS = astToStr(TT)
+  suite TTS & " roundtrip test":
+    test "Empty " & TTS:
+      let blk = TT()
+      roundTrip(blk)
+
+    test TTS & " with withdrawals":
+      let blk = TT(withdrawals: Opt.some(@[Withdrawal()]))
+      roundTrip(blk)
+
+    test "2 " & TTS & " none(Withdrawal)+none(Withdrawal)":
+      let blk = TT()
+      roundTrip2(blk, blk):
+        check b1.withdrawals.isNone
+        check b2.withdrawals.isNone
+
+    test "2 " & TTS & " none(Withdrawal)+some(Withdrawal)":
+      let blk1 = TT()
+      let blk2 = TT(withdrawals: Opt.some(@[Withdrawal()]))
+      roundTrip2(blk1, blk2):
+        check b1.withdrawals.isNone
+        check b2.withdrawals.isSome
+
+    test "2 " & TTS & " some(Withdrawal)+none(Withdrawal)":
+      let blk1 = TT()
+      let blk2 = TT(withdrawals: Opt.some(@[Withdrawal()]))
+      roundTrip2(blk2, blk1):
+        check b1.withdrawals.isSome
+        check b2.withdrawals.isNone
+
+    test "2 " & TTS & " some(Withdrawal)+some(Withdrawal)":
+      let blk = TT(withdrawals: Opt.some(@[Withdrawal()]))
+      roundTrip2(blk, blk):
+        check b1.withdrawals.isSome
+        check b2.withdrawals.isSome
+
+genTestOpt(BlockBodyOpt)
+genTestOpt(EthBlockOpt)