fix: keep SDS causalHistory wire-compatible with pre-SDS-R nodes#86
Merged
Conversation
The declarative codec (nim-protobuf-serialization) encoded causalHistory (field 3) as a repeated embedded HistoryEntry. Pre-SDS-R nodes (v0.2.x / v0.3.x, and the release/v0.3 line status-go ships today) read field 3 as a repeated string of message IDs, so they misparse the embedded submessage bytes as an ID string, corrupt dependency tracking, and silently never deliver the message (buffered as an unmet dependency). Make the SDS-R metadata additive instead, matching release/v0.3: - field 3 stays a repeated string/bytes of message IDs (read by every SDS version); - the per-entry senderId/retrievalHint move to a new additive field 8 (repeated HistoryEntry keyed by message ID), which older nodes skip; - field 7 (message senderId) and field 13 (repairRequest) are unchanged. New nodes join field 3 + field 8 by message ID; messages from pre-SDS-R nodes carry no field 8 and decode to bare-ID entries. Adds test_wire_compat.nim (old-decode-of-new, new round-trip incl. senderId, new-decode-of-legacy). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The macos-latest runner rotated its Xcode images (16.2 is gone; only 26.x is available), so the pinned 'xcode-version: 16.2' failed with "Could not find Xcode version that satisfied version spec". Track 'latest-stable' so the iOS build follows the runner instead of a version that gets removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ivansete-status
requested review from
NagyZoltanPeter,
darshankabariya,
fcecin and
stubbsta
July 8, 2026 22:21
The retrieval-hint provider (the Go binding's C.CBytes) allocates the returned hint with malloc and hands us ownership. onRetrievalHint freed it with Nim's deallocShared, corrupting Nim's shared-heap allocator and crashing with a SIGSEGV in deallocBigChunk/avltree.del once wrapOutgoingMessage starts invoking the provider per causal-history entry. Free it with libc free instead. Reproduced end-to-end via the C ABI on the release/v0.3 line (malloc-based provider + 500 wraps): buggy build segfaults (exit 139), fixed build completes with ~4900 provider calls. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On the v0.4 line,
causalHistory(protobuf field 3) is encoded as a repeated embeddedHistoryEntrysubmessage. Pre-SDS-R nodes — v0.2.x, and therelease/v0.3line that status-go ships today — read field 3 as a repeated string of message IDs. Same field number and wire type, so decoding doesn't error; it silently misparses: the old node reads the submessage framing bytes as the ID string, so its causal-history/dependency IDs are garbage that never match real messages. The message is buffered as an unmet dependency and never delivered (0MESSAGES_NEW).This is the same regression already fixed on
release/v0.3(nim-sdsv0.3.1); this PR ports the fix to the v0.4 / declarative-codec line so v0.4 releases can interoperate with the v0.2.x/v0.3.x nodes live on the network.Fix (additive, matches release/v0.3)
senderId,retrievalHint) → new additive field 8 (repeated HistoryEntry, keyed by message ID), which older nodes skip as an unknown fieldsenderId) and field 13 (repairRequest) unchangedNew nodes join field 3 + field 8 by message ID; messages from pre-SDS-R nodes carry no field 8 and decode to bare-ID entries.
Field alignment with
release/v0.3: both lines now carry the causal-history IDs in field 3 and the rich metadata in field 8 (sameHistoryEntrysub-shape{1: id, 2: hint, 3: senderId}), so v0.3.x ↔ v0.4.x interoperate. (Note: nim-sdsv0.3.1currently carries hints in field 7; a companionv0.3.2moves them to field 8 to match this PR.)Verification
test_reliabilityincl. SDS-R repair: 67/67)test_wire_compat.nim: old-decoder-reads-new (the exact break), new round-trip incl.senderId, and new-decodes-legacy — all pass.🤖 Generated with Claude Code