Seperate discv5 protocol message encoding from packet encoding (#539)

And some additional clean-ups

eth/p2p/discoveryv5/encoding.nim

@@ -18,7 +18,7 @@ import
   nimcrypto/[bcmode, rijndael, sha2], stint, chronicles,
   stew/[results, byteutils], metrics,
   ".."/../[rlp, keys],
-  "."/[messages, node, enr, hkdf, sessions]
+  "."/[messages_encoding, node, enr, hkdf, sessions]
 
 from stew/objects import checkedEnumAssign
 
@@ -371,50 +371,6 @@ proc decodeHeader*(id: NodeId, iv, maskedHeader: openArray[byte]):
   ok((StaticHeader(authdataSize: authdataSize, flag: flag, nonce: nonce),
     staticHeader & authdata))
 
-proc decodeMessage*(body: openArray[byte]): DecodeResult[Message] =
-  ## Decodes to the specific `Message` type.
-  if body.len < 1:
-    return err("No message data")
-
-  var kind: MessageKind
-  if not checkedEnumAssign(kind, body[0]):
-    return err("Invalid message type")
-
-  var message = Message(kind: kind)
-  var rlp = rlpFromBytes(body.toOpenArray(1, body.high))
-  if rlp.enterList:
-    try:
-      message.reqId = rlp.read(RequestId)
-    except RlpError, ValueError:
-      return err("Invalid request-id")
-
-    proc decode[T](rlp: var Rlp, v: var T)
-        {.nimcall, raises:[RlpError, ValueError, Defect].} =
-      for k, v in v.fieldPairs:
-        v = rlp.read(typeof(v))
-
-    try:
-      case kind
-      of unused: return err("Invalid message type")
-      of ping: rlp.decode(message.ping)
-      of pong: rlp.decode(message.pong)
-      of findNode: rlp.decode(message.findNode)
-      of nodes: rlp.decode(message.nodes)
-      of talkReq: rlp.decode(message.talkReq)
-      of talkResp: rlp.decode(message.talkResp)
-      of regTopic, ticket, regConfirmation, topicQuery:
-        # We just pass the empty type of this message without attempting to
-        # decode, so that the protocol knows what was received.
-        # But we ignore the message as per specification as "the content and
-        # semantics of this message are not final".
-        discard
-    except RlpError, ValueError:
-      return err("Invalid message encoding")
-
-    ok(message)
-  else:
-    err("Invalid message encoding: no rlp list")
-
 proc decodeMessagePacket(c: var Codec, fromAddr: Address, nonce: AESGCMNonce,
     iv, header, ct: openArray[byte]): DecodeResult[Packet] =
   # We now know the exact size that the header should be
@@ -609,22 +565,3 @@ proc decodePacket*(c: var Codec, fromAddr: Address, input: openArray[byte]):
     return decodeHandshakePacket(c, fromAddr, staticHeader.nonce,
       input.toOpenArray(0, ivSize - 1), header,
       input.toOpenArray(ivSize + header.len, input.high))
-
-proc init*(T: type RequestId, rng: var HmacDrbgContext): T =
-  var reqId = RequestId(id: newSeq[byte](8)) # RequestId must be <= 8 bytes
-  rng.generate(reqId.id)
-  reqId
-
-proc numFields(T: typedesc): int =
-  for k, v in fieldPairs(default(T)): inc result
-
-proc encodeMessage*[T: SomeMessage](p: T, reqId: RequestId): seq[byte] =
-  result = newSeqOfCap[byte](64)
-  result.add(messageKind(T).ord)
-
-  const sz = numFields(T)
-  var writer = initRlpList(sz + 1)
-  writer.append(reqId)
-  for k, v in fieldPairs(p):
-    writer.append(v)
-  result.add(writer.finish())

eth/p2p/discoveryv5/messages.nim

@@ -7,23 +7,24 @@
 #
 ## Discovery v5 Protocol Messages as specified at
 ## https://github.com/ethereum/devp2p/blob/master/discv5/discv5-wire.md#protocol-messages
-## These messages get RLP encoded.
 ##
 
 {.push raises: [Defect].}
 
 import
   std/[hashes, net],
-  stew/arrayops,
-  ../../rlp, ./enr
+  ./enr
 
 type
   MessageKind* = enum
-    # TODO This is needed only to make Nim 1.2.6 happy
-    #      Without it, the `MessageKind` type cannot be used as
-    #      a discriminator in case objects.
+    # Note:
+    # This is needed only to keep the compiler happy. Without it, the
+    # `MessageKind` type cannot be used as a discriminator in case objects.
+    # If a message with this value is received however, it will fail at the
+    # decoding step.
     unused = 0x00
 
+    # The supported message types
     ping = 0x01
     pong = 0x02
     findNode = 0x03
@@ -103,42 +104,10 @@ template messageKind*(T: typedesc[SomeMessage]): MessageKind =
   elif T is TalkReqMessage: talkReq
   elif T is TalkRespMessage: talkResp
 
-proc read*(rlp: var Rlp, T: type RequestId): T
-    {.raises: [ValueError, RlpError, Defect].} =
-  mixin read
-  var reqId: RequestId
-  reqId.id = rlp.toBytes()
-  if reqId.id.len > 8:
-    raise newException(ValueError, "RequestId is > 8 bytes")
-  rlp.skipElem()
-
+func init*(T: type RequestId, rng: var HmacDrbgContext): T =
+  var reqId = RequestId(id: newSeq[byte](8)) # RequestId must be <= 8 bytes
+  rng.generate(reqId.id)
   reqId
 
-proc append*(writer: var RlpWriter, value: RequestId) =
-  writer.append(value.id)
-
-proc read*(rlp: var Rlp, T: type IpAddress): T
-    {.raises: [RlpError, Defect].} =
-  let ipBytes = rlp.toBytes()
-  rlp.skipElem()
-
-  if ipBytes.len == 4:
-    var ip: array[4, byte]
-    discard copyFrom(ip, ipBytes)
-    IpAddress(family: IPv4, address_v4: ip)
-  elif ipBytes.len == 16:
-    var ip: array[16, byte]
-    discard copyFrom(ip, ipBytes)
-    IpAddress(family: IPv6, address_v6: ip)
-  else:
-    raise newException(RlpTypeMismatch,
-      "Amount of bytes for IP address is different from 4 or 16")
-
-proc append*(writer: var RlpWriter, ip: IpAddress) =
-  case ip.family:
-  of IpAddressFamily.IPv4:
-    writer.append(ip.address_v4)
-  of IpAddressFamily.IPv6: writer.append(ip.address_v6)
-
-proc hash*(reqId: RequestId): Hash =
+func hash*(reqId: RequestId): Hash =
   hash(reqId.id)

eth/p2p/discoveryv5/messages_encoding.nim

@@ -0,0 +1,120 @@
+# nim-eth - Node Discovery Protocol v5
+# Copyright (c) 2020-2022 Status Research & Development GmbH
+# Licensed and distributed under either of
+#   * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT).
+#   * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0).
+# at your option. This file may not be copied, modified, or distributed except according to those terms.
+#
+## Discovery v5 Protocol Messages RLP Encoding
+
+{.push raises: [Defect].}
+
+import
+  std/net,
+  stew/[arrayops, results],
+  ../../rlp,
+  "."/[messages, enr]
+
+from stew/objects import checkedEnumAssign
+
+export messages, rlp, results
+
+func read*(rlp: var Rlp, T: type RequestId): T
+    {.raises: [ValueError, RlpError, Defect].} =
+  mixin read
+  var reqId: RequestId
+  reqId.id = rlp.toBytes()
+  if reqId.id.len > 8:
+    raise newException(ValueError, "RequestId is > 8 bytes")
+  rlp.skipElem()
+
+  reqId
+
+func append*(writer: var RlpWriter, value: RequestId) =
+  writer.append(value.id)
+
+func read*(rlp: var Rlp, T: type IpAddress): T
+    {.raises: [RlpError, Defect].} =
+  let ipBytes = rlp.toBytes()
+  rlp.skipElem()
+
+  if ipBytes.len == 4:
+    var ip: array[4, byte]
+    discard copyFrom(ip, ipBytes)
+    IpAddress(family: IPv4, address_v4: ip)
+  elif ipBytes.len == 16:
+    var ip: array[16, byte]
+    discard copyFrom(ip, ipBytes)
+    IpAddress(family: IPv6, address_v6: ip)
+  else:
+    raise newException(RlpTypeMismatch,
+      "Amount of bytes for IP address is different from 4 or 16")
+
+func append*(writer: var RlpWriter, ip: IpAddress) =
+  case ip.family:
+  of IpAddressFamily.IPv4:
+    writer.append(ip.address_v4)
+  of IpAddressFamily.IPv6:
+    writer.append(ip.address_v6)
+
+func numFields(T: typedesc): int =
+  for k, v in fieldPairs(default(T)): inc result
+
+func encodeMessage*[T: SomeMessage](p: T, reqId: RequestId): seq[byte] =
+  ## Encodes a message with provided `reqId`.
+  var bytes = newSeqOfCap[byte](64)
+  bytes.add(messageKind(T).ord)
+
+  const sz = numFields(T)
+  var writer = initRlpList(sz + 1)
+  writer.append(reqId)
+  for k, v in fieldPairs(p):
+    writer.append(v)
+
+  bytes.add(writer.finish())
+
+  bytes
+
+func decodeMessage*(body: openArray[byte]): Result[Message, cstring] =
+  ## Decodes to the specific `Message` type.
+  if body.len < 1:
+    return err("No message data")
+
+  var kind: MessageKind
+  if not checkedEnumAssign(kind, body[0]):
+    return err("Invalid message type")
+
+  var message = Message(kind: kind)
+  var rlp = rlpFromBytes(body.toOpenArray(1, body.high))
+  if rlp.enterList:
+    try:
+      message.reqId = rlp.read(RequestId)
+    except RlpError, ValueError:
+      return err("Invalid request-id")
+
+    func decode[T](rlp: var Rlp, v: var T)
+        {.nimcall, raises:[RlpError, ValueError, Defect].} =
+      for k, v in v.fieldPairs:
+        v = rlp.read(typeof(v))
+
+    try:
+      case kind
+      of unused: return err("Invalid message type")
+      of ping: rlp.decode(message.ping)
+      of pong: rlp.decode(message.pong)
+      of findNode: rlp.decode(message.findNode)
+      of nodes: rlp.decode(message.nodes)
+      of talkReq: rlp.decode(message.talkReq)
+      of talkResp: rlp.decode(message.talkResp)
+      of regTopic, ticket, regConfirmation, topicQuery:
+        # We just pass the empty type of this message without attempting to
+        # decode, so that the protocol knows what was received.
+        # But we ignore the message as per specification as "the content and
+        # semantics of this message are not final".
+        discard
+    except RlpError, ValueError:
+      return err("Invalid message encoding")
+
+    ok(message)
+  else:
+    err("Invalid message encoding: no rlp list")

eth/p2p/discoveryv5/protocol.nim

@@ -85,8 +85,8 @@ import
   stew/shims/net as stewNet, json_serialization/std/net,
   stew/[endians2, results], chronicles, chronos, stint, metrics,
   ".."/../[rlp, keys],
-  "."/[messages, encoding, node, routing_table, enr, random2, sessions, ip_vote,
-    nodes_verification]
+  "."/[messages_encoding, encoding, node, routing_table, enr, random2, sessions,
+    ip_vote, nodes_verification]
 
 export
   options, results, node, enr, encoding.maxDiscv5PacketSize

eth/utp/utp_discv5_protocol.nim

@@ -9,7 +9,7 @@
 import
   std/[hashes, sugar],
   chronos, chronicles,
-  ../p2p/discoveryv5/[protocol, messages, encoding],
+  ../p2p/discoveryv5/[protocol, messages_encoding, encoding],
   ./utp_router,
   ../keys
 

tests/fuzzing/discoveryv5/fuzz_decode_message.nim

@@ -1,6 +1,6 @@
 import
   testutils/fuzzing,
-  ../../../eth/rlp, ../../../eth/p2p/discoveryv5/[encoding, messages]
+  ../../../eth/p2p/discoveryv5/messages_encoding
 
 test:
   block:

tests/p2p/test_discoveryv5_encoding.nim

@@ -5,7 +5,7 @@ import
   unittest2,
   stint, stew/byteutils, stew/shims/net,
   ../../eth/keys,
-  ../../eth/p2p/discoveryv5/[messages, encoding, enr, node, sessions]
+  ../../eth/p2p/discoveryv5/[messages_encoding, encoding, enr, node, sessions]
 
 let rng = newRng()