properly annotate protobuf fields as required when needed

nimble.lock

@@ -84,8 +84,8 @@
       }
     },
     "protobuf_serialization": {
-      "version": "0.4.0",
-      "vcsRevision": "38d24eb3bd93e605fb88199da71d36b1ec0ad60d",
+      "version": "0.5.1",
+      "vcsRevision": "d9aa950b9d9e8bfc8a201740042b5e8ea5880875",
       "url": "https://github.com/status-im/nim-protobuf-serialization",
       "downloadMethod": "git",
       "dependencies": [
@@ -96,7 +96,7 @@
         "unittest2"
       ],
       "checksums": {
-        "sha1": "5a7a80fb8cca29e41899ce9540b74e49c874f8fd"
+        "sha1": "c02d1124931612041a25c6c8ed59a6120849c85d"
       }
     },
     "json_serialization": {

nix/deps.nix

@@ -54,8 +54,8 @@
 
   protobuf_serialization = pkgs.fetchgit {
     url = "https://github.com/status-im/nim-protobuf-serialization";
-    rev = "38d24eb3bd93e605fb88199da71d36b1ec0ad60d";
-    sha256 = "0jr0a41b4r444si6xfa7bclw8mjsk6id10lrdvbxzp99750zspb9";
+    rev = "d9aa950b9d9e8bfc8a201740042b5e8ea5880875";
+    sha256 = "11hrqpq7dpdqfn71izmq7ysrdnh8gry0qvrgqdspcz2k2lifzz0c";
     fetchSubmodules = true;
   };
 

sds.nimble

@@ -10,7 +10,7 @@ srcDir = "sds"
 # Dependencies
 requires "nim >= 2.2.4"
 requires "chronos >= 4.0.4"
-requires "protobuf_serialization >= 0.4.0"
+requires "protobuf_serialization >= 0.5.0"
 requires "chronicles"
 requires "stew"
 requires "stint"

sds/protobuf.nim

@@ -7,12 +7,16 @@
 ## conversion bridges the two. The mirror string-ish fields are `seq[byte]`
 ## (not `pstring`) so message/channel/sender ids stay opaque bytes — no UTF-8
 ## validation — and the distinct `SdsParticipantID` needs no special support.
+##
+## The mirrors are `proto2` so mandatory fields can be marked `{.required.}`:
+## decoding a message that omits one fails (matching the previous codec), which
+## proto3 cannot express. Genuinely optional fields are `Opt[seq[byte]]`.
 
 {.push raises: [].}
 
 import endians
-import results
 import protobuf_serialization
+import protobuf_serialization/pkg/results
 import ./types/[sds_message_id, history_entry, sds_message, reliability_error]
 import ./bloom
 
@@ -21,30 +25,30 @@ import ./bloom
 # ---------------------------------------------------------------------------
 
 type
-  HistoryEntryPB* {.proto3.} = object
-    messageId* {.fieldNumber: 1.}: seq[byte]
-    retrievalHint* {.fieldNumber: 2.}: seq[byte]
-    senderId* {.fieldNumber: 3.}: seq[byte]
+  HistoryEntryPB* {.proto2.} = object
+    messageId* {.fieldNumber: 1, required.}: seq[byte]
+    retrievalHint* {.fieldNumber: 2.}: Opt[seq[byte]]
+    senderId* {.fieldNumber: 3.}: Opt[seq[byte]]
 
-  SdsMessagePB* {.proto3.} = object
-    messageId* {.fieldNumber: 1.}: seq[byte]
-    lamportTimestamp* {.fieldNumber: 2, pint.}: int64
+  SdsMessagePB* {.proto2.} = object
+    messageId* {.fieldNumber: 1, required.}: seq[byte]
+    lamportTimestamp* {.fieldNumber: 2, pint, required.}: int64
     causalHistory* {.fieldNumber: 3.}: seq[HistoryEntryPB]
-    channelId* {.fieldNumber: 4.}: seq[byte]
-    content* {.fieldNumber: 5.}: seq[byte]
-    bloomFilter* {.fieldNumber: 6.}: seq[byte]
-    senderId* {.fieldNumber: 7.}: seq[byte]
+    channelId* {.fieldNumber: 4, required.}: seq[byte]
+    content* {.fieldNumber: 5, required.}: seq[byte]
+    bloomFilter* {.fieldNumber: 6.}: Opt[seq[byte]]
+    senderId* {.fieldNumber: 7.}: Opt[seq[byte]]
     repairRequest* {.fieldNumber: 13.}: seq[HistoryEntryPB]
 
-  BloomFilterPB {.proto3.} = object
-    data {.fieldNumber: 1.}: seq[byte]
-    capacity {.fieldNumber: 2, pint.}: uint64
-    errorRate {.fieldNumber: 3, pint.}: uint64
-    kHashes {.fieldNumber: 4, pint.}: uint64
-    mBits {.fieldNumber: 5, pint.}: uint64
+  BloomFilterPB {.proto2.} = object
+    data {.fieldNumber: 1, required.}: seq[byte]
+    capacity {.fieldNumber: 2, pint, required.}: uint64
+    errorRate {.fieldNumber: 3, pint, required.}: uint64
+    kHashes {.fieldNumber: 4, pint, required.}: uint64
+    mBits {.fieldNumber: 5, pint, required.}: uint64
 
 # ---------------------------------------------------------------------------
-# string <-> bytes (opaque, no UTF-8 validation)
+# string <-> bytes (opaque, no UTF-8 validation) and optional-bytes helpers
 # ---------------------------------------------------------------------------
 
 func toBytes(s: string): seq[byte] =
@@ -59,6 +63,12 @@ func toStr(b: seq[byte]): string =
     copyMem(addr s[0], unsafeAddr b[0], b.len)
   return s
 
+func optBytes(b: seq[byte]): Opt[seq[byte]] =
+  ## Present only when non-empty, so empty optionals stay off the wire.
+  if b.len > 0:
+    return Opt.some(b)
+  return Opt.none(seq[byte])
+
 # ---------------------------------------------------------------------------
 # Domain <-> wire conversion
 # ---------------------------------------------------------------------------
@@ -66,15 +76,15 @@ func toStr(b: seq[byte]): string =
 func toPB*(e: HistoryEntry): HistoryEntryPB =
   return HistoryEntryPB(
     messageId: e.messageId.toBytes,
-    retrievalHint: e.retrievalHint,
-    senderId: e.senderId.string.toBytes,
+    retrievalHint: optBytes(e.retrievalHint),
+    senderId: optBytes(e.senderId.string.toBytes),
   )
 
 func fromPB*(e: HistoryEntryPB): HistoryEntry =
   return HistoryEntry(
     messageId: e.messageId.toStr,
-    retrievalHint: e.retrievalHint,
-    senderId: e.senderId.toStr.SdsParticipantID,
+    retrievalHint: e.retrievalHint.valueOr(@[]),
+    senderId: e.senderId.valueOr(@[]).toStr.SdsParticipantID,
   )
 
 func toPB*(m: SdsMessage): SdsMessagePB =
@@ -83,8 +93,8 @@ func toPB*(m: SdsMessage): SdsMessagePB =
     lamportTimestamp: m.lamportTimestamp,
     channelId: m.channelId.toBytes,
     content: m.content,
-    bloomFilter: m.bloomFilter,
-    senderId: m.senderId.string.toBytes,
+    bloomFilter: optBytes(m.bloomFilter),
+    senderId: optBytes(m.senderId.string.toBytes),
   )
   for e in m.causalHistory:
     pb.causalHistory.add(e.toPB)
@@ -105,8 +115,8 @@ func fromPB*(pb: SdsMessagePB): SdsMessage =
     causalHistory = causal,
     channelId = pb.channelId.toStr,
     content = pb.content,
-    bloomFilter = pb.bloomFilter,
-    senderId = pb.senderId.toStr.SdsParticipantID,
+    bloomFilter = pb.bloomFilter.valueOr(@[]),
+    senderId = pb.senderId.valueOr(@[]).toStr.SdsParticipantID,
     repairRequest = repair,
   )
 
@@ -121,17 +131,11 @@ proc serializeMessage*(msg: SdsMessage): Result[seq[byte], ReliabilityError] =
     return err(ReliabilityError.reSerializationError)
 
 proc deserializeMessage*(data: seq[byte]): Result[SdsMessage, ReliabilityError] =
-  ## proto3 has no required fields, so presence is validated by hand. Only the
-  ## identifiers are mandatory: `content`, `bloomFilter` and a zero
-  ## `lamportTimestamp` may legitimately be empty (e.g. periodic sync messages).
+  ## A missing `{.required.}` field (messageId, lamportTimestamp, channelId,
+  ## content, or any entry's messageId) makes `Protobuf.decode` raise, which is
+  ## surfaced here as a deserialization error.
   try:
-    let pb = Protobuf.decode(data, SdsMessagePB)
-    if pb.messageId.len == 0 or pb.channelId.len == 0:
-      return err(ReliabilityError.reDeserializationError)
-    for e in pb.causalHistory & pb.repairRequest:
-      if e.messageId.len == 0:
-        return err(ReliabilityError.reDeserializationError)
-    return ok(pb.fromPB)
+    return ok(Protobuf.decode(data, SdsMessagePB).fromPB)
   except CatchableError:
     return err(ReliabilityError.reDeserializationError)