mirror of
https://github.com/logos-messaging/logos-delivery.git
synced 2026-07-22 20:50:22 +00:00
fix: clamp by range width, not absolute floor, to avoid clock-skew false clamp
Adversarial review found the previous clamp compared the incoming range's lower bound to a fresh now()-derived selfLowerBound. Because the responder samples now() later than the initiator, that floor is always slightly higher, so the clamp fired even between EQUAL-window peers (store<->store), skipping a sub-second sliver at the bottom of the window and risking permanent store-fleet inconsistency for a message landing in it. Compare range WIDTH to our syncRange instead: width is independent of each peer's now() (both bounds subtract the same now+jitter), so equal windows never clamp and only a genuinely wider-window peer is clamped, erring toward reconciling slightly extra rather than dropping. Added an equal-window near-floor guard test (suite 38/38). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
94551413b1
commit
9812f99010
@ -155,17 +155,21 @@ proc preProcessPayload(self: SyncReconciliation, payload: RangesData): Opt[Range
|
||||
elif rangeType == RangeType.ItemSet:
|
||||
payload.itemSets.delete(0)
|
||||
else:
|
||||
# This range straddles our window's lower bound: mark the portion below
|
||||
# our window as skip so a peer with a wider window does not reconcile
|
||||
# (and offer us) messages older than we retain. Only triggers on a
|
||||
# window mismatch; equal windows never straddle.
|
||||
# If the remote is reconciling a wider window than ours, clamp the
|
||||
# bottom of this range to our window's width so a wider-window peer
|
||||
# (e.g. a store node) does not offer us history older than we retain.
|
||||
# We compare range WIDTH to our syncRange rather than absolute floors:
|
||||
# width is independent of each peer's now(), so equal-window syncs
|
||||
# (e.g. store<->store) never clamp, avoiding clock-skew false positives.
|
||||
let lowerBound = payload.ranges[i][0].a.time
|
||||
if lowerBound < selfLowerBound:
|
||||
let clampId = SyncID(time: selfLowerBound, hash: EmptyFingerprint)
|
||||
let myRange = self.syncRange.nanos
|
||||
if upperBound - lowerBound > myRange:
|
||||
let clampTime = upperBound - myRange
|
||||
let clampId = SyncID(time: clampTime, hash: EmptyFingerprint)
|
||||
let belowSkip = (payload.ranges[i][0].a .. clampId, RangeType.Skip)
|
||||
|
||||
if rangeType == RangeType.ItemSet:
|
||||
payload.itemSets[0].elements.keepItIf(it.time >= selfLowerBound)
|
||||
payload.itemSets[0].elements.keepItIf(it.time >= clampTime)
|
||||
|
||||
payload.ranges[i][0].a = clampId
|
||||
payload.ranges.insert(belowSkip, i)
|
||||
|
||||
@ -198,6 +198,32 @@ suite "Waku Sync: reconciliation":
|
||||
# the old, below-window message is clamped out, never offered
|
||||
remoteNeeds.contains((serverPeerInfo.peerId, oldHash)) == false
|
||||
|
||||
asyncTest "equal-window peers reconcile a message near the window floor":
|
||||
## Guard for the clamp: two peers with the SAME sync range must not clamp
|
||||
## each other (the clamp compares range width to our window, not absolute
|
||||
## floors, so peer clock skew never triggers it), so a message near the
|
||||
## bottom of the shared window is still reconciled.
|
||||
const equalRange = 1.hours
|
||||
|
||||
server = await newTestWakuRecon(
|
||||
serverSwitch, @[], @[], equalRange, idsChannel, localWants, remoteNeeds
|
||||
)
|
||||
client = await newTestWakuRecon(
|
||||
clientSwitch, @[], @[], equalRange, idsChannel, localWants, remoteNeeds
|
||||
)
|
||||
|
||||
let
|
||||
nearFloorTs = now() - 3_300_000_000_000 # 55 min ago: in-window, near floor
|
||||
msg = fakeWakuMessage(ts = nearFloorTs, contentTopic = DefaultContentTopic)
|
||||
hash = computeMessageHash(DefaultPubsubTopic, msg)
|
||||
|
||||
client.messageIngress(hash, DefaultPubsubTopic, msg)
|
||||
|
||||
let res = await client.storeSynchronization(Opt.some(serverPeerInfo))
|
||||
assert res.isOk(), res.error
|
||||
|
||||
check remoteNeeds.contains((serverPeerInfo.peerId, hash)) == true
|
||||
|
||||
asyncTest "sync 2 nodes different hashes":
|
||||
server = await newTestWakuRecon(
|
||||
serverSwitch, @[], @[], DefaultSyncRange, idsChannel, localWants, remoteNeeds
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user