From 3e8e37bb39a379f4a530abbcaaa9c8bd2652fb32 Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Thu, 11 Jun 2026 12:00:56 +0200 Subject: [PATCH] properly annotate protobuf fields as required when needed --- nimble.lock | 6 ++-- nix/deps.nix | 4 +-- sds.nimble | 2 +- sds/protobuf.nim | 78 +++++++++++++++++++++++++----------------------- 4 files changed, 47 insertions(+), 43 deletions(-) diff --git a/nimble.lock b/nimble.lock index 4812d8f..3520d20 100644 --- a/nimble.lock +++ b/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": { diff --git a/nix/deps.nix b/nix/deps.nix index aaefc05..f728089 100644 --- a/nix/deps.nix +++ b/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; }; diff --git a/sds.nimble b/sds.nimble index e35e056..78bddea 100644 --- a/sds.nimble +++ b/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" diff --git a/sds/protobuf.nim b/sds/protobuf.nim index d9b2897..ddbc305 100644 --- a/sds/protobuf.nim +++ b/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)