Fix sequence number overflow handling

src/seqno_generator.nim

@@ -4,12 +4,14 @@ import libp2p/peerid
 
 type SeqNo* = object
   counter: uint32
+  epoch: uint32 # Increments when counter wraps around
 
 proc initSeqNo*(peerId: PeerId): SeqNo =
   var seqNo: SeqNo
   let peerIdHash = sha256_hash(peerId.data)
   for i in 0 .. 3:
     seqNo.counter = seqNo.counter or (uint32(peerIdHash[i]) shl (8 * (3 - i)))
+  seqNo.epoch = 0 # Start with epoch 0
   return seqNo
 
 proc generateSeqNo*(seqNo: var SeqNo, messageBytes: seq[byte]) =
@@ -24,8 +26,45 @@ proc generateSeqNo*(seqNo: var SeqNo, messageBytes: seq[byte]) =
   seqNo.counter = (seqNo.counter + cnt) mod high(uint32)
 
 proc incSeqNo*(seqNo: var SeqNo) =
-  seqNo.counter = (seqNo.counter + 1) mod high(uint32)
-    # ToDo: Manage sequence no. overflow in a way that it does not affect re-assembly
+  let oldCounter = seqNo.counter
+  seqNo.counter = seqNo.counter + 1
+
+  # Detect wraparound and increment epoch
+  if seqNo.counter < oldCounter: # Overflow occurred
+    seqNo.epoch = seqNo.epoch + 1
 
 proc getSeqNo*(seqNo: SeqNo): uint32 =
   return seqNo.counter
+
+proc getEpoch*(seqNo: SeqNo): uint32 =
+  return seqNo.epoch
+
+proc getFullSeqNo*(seqNo: SeqNo): (uint32, uint32) =
+  ## Returns (counter, epoch) tuple for complete sequence identification
+  return (seqNo.counter, seqNo.epoch)
+
+proc compareSeqNo*(a, b: SeqNo): int =
+  ## Compare two sequence numbers accounting for wraparound
+  ## Returns: -1 if a < b, 0 if a == b, 1 if a > b
+  if a.epoch != b.epoch:
+    if a.epoch < b.epoch:
+      return -1
+    else:
+      return 1
+
+  # Same epoch, compare counters normally
+  if a.counter < b.counter:
+    return -1
+  elif a.counter > b.counter:
+    return 1
+  else:
+    return 0
+
+proc isNextSeqNo*(current, next: SeqNo): bool =
+  ## Check if 'next' is the immediate successor of 'current'
+  if current.epoch == next.epoch:
+    return next.counter == current.counter + 1
+  elif next.epoch == current.epoch + 1:
+    return current.counter == high(uint32) and next.counter == 0
+  else:
+    return false

tests/test_seqno_generator.nim

@@ -85,14 +85,33 @@ suite "Sequence Number Generator":
       PeerId.init("16Uiu2HAmFkwLVsVh6gGPmSm9R3X4scJ5thVdKfWYeJsKeVrbcgVC").get()
     var seqNo = initSeqNo(peerId)
     seqNo.counter = high(uint32) - 1
+    let initialEpoch = seqNo.getEpoch()
+
     if seqNo.counter != high(uint32) - 1:
       error "Sequence number must be max value",
         counter = seqNo.counter, maxval = high(uint32) - 1
       fail()
 
+    # Increment to max value
+    incSeqNo(seqNo)
+    if seqNo.counter != high(uint32):
+      error "Sequence number must be max value", counter = seqNo.counter
+      fail()
+
+    if seqNo.getEpoch() != initialEpoch:
+      error "Epoch should not change before overflow",
+        epoch = seqNo.getEpoch(), expected = initialEpoch
+      fail()
+
+    # Now wrap around
     incSeqNo(seqNo)
     if seqNo.counter != 0:
-      error "Sequence number must wrap around", counter = seqNo.counter
+      error "Sequence number must wrap around to 0", counter = seqNo.counter
+      fail()
+
+    if seqNo.getEpoch() != initialEpoch + 1:
+      error "Epoch must increment on overflow",
+        epoch = seqNo.getEpoch(), expected = initialEpoch + 1
       fail()
 
   test "generate_seq_no_uses_entire_uint32_range":
@@ -110,3 +129,91 @@ suite "Sequence Number Generator":
       error "Sequence numbers must be uniformly distributed",
         uniqueValues = seenValues.len
       fail()
+
+  test "compare_seq_no_same_epoch":
+    let peerId =
+      PeerId.init("16Uiu2HAmFkwLVsVh6gGPmSm9R3X4scJ5thVdKfWYeJsKeVrbcgVC").get()
+    var seqNo1 = initSeqNo(peerId)
+    var seqNo2 = initSeqNo(peerId)
+
+    seqNo1.counter = 100
+    seqNo2.counter = 200
+
+    if compareSeqNo(seqNo1, seqNo2) != -1:
+      error "seqNo1 should be less than seqNo2"
+      fail()
+
+    if compareSeqNo(seqNo2, seqNo1) != 1:
+      error "seqNo2 should be greater than seqNo1"
+      fail()
+
+    seqNo2.counter = 100
+    if compareSeqNo(seqNo1, seqNo2) != 0:
+      error "Equal sequence numbers should compare as equal"
+      fail()
+
+  test "compare_seq_no_different_epochs":
+    let peerId =
+      PeerId.init("16Uiu2HAmFkwLVsVh6gGPmSm9R3X4scJ5thVdKfWYeJsKeVrbcgVC").get()
+    var seqNo1 = initSeqNo(peerId)
+    var seqNo2 = initSeqNo(peerId)
+
+    seqNo1.counter = high(uint32)
+    seqNo1.epoch = 0
+    seqNo2.counter = 0
+    seqNo2.epoch = 1
+
+    if compareSeqNo(seqNo1, seqNo2) != -1:
+      error "SeqNo from earlier epoch should be less than later epoch"
+      fail()
+
+  test "is_next_seq_no_normal_increment":
+    let peerId =
+      PeerId.init("16Uiu2HAmFkwLVsVh6gGPmSm9R3X4scJ5thVdKfWYeJsKeVrbcgVC").get()
+    var current = initSeqNo(peerId)
+    var next = initSeqNo(peerId)
+
+    current.counter = 100
+    current.epoch = 0
+    next.counter = 101
+    next.epoch = 0
+
+    if not isNextSeqNo(current, next):
+      error "Next sequence number should be recognized as successor"
+      fail()
+
+  test "is_next_seq_no_wraparound":
+    let peerId =
+      PeerId.init("16Uiu2HAmFkwLVsVh6gGPmSm9R3X4scJ5thVdKfWYeJsKeVrbcgVC").get()
+    var current = initSeqNo(peerId)
+    var next = initSeqNo(peerId)
+
+    current.counter = high(uint32)
+    current.epoch = 0
+    next.counter = 0
+    next.epoch = 1
+
+    if not isNextSeqNo(current, next):
+      error "Wraparound sequence number should be recognized as successor"
+      fail()
+
+  test "epoch_overflow_handling":
+    let peerId =
+      PeerId.init("16Uiu2HAmFkwLVsVh6gGPmSm9R3X4scJ5thVdKfWYeJsKeVrbcgVC").get()
+    var seqNo = initSeqNo(peerId)
+
+    # Simulate multiple wraparounds
+    seqNo.counter = high(uint32) - 2
+    seqNo.epoch = 5
+
+    for i in 0 .. 5:
+      incSeqNo(seqNo)
+
+    if seqNo.epoch != 6:
+      error "Epoch should increment after wraparound", epoch = seqNo.epoch, expected = 6
+      fail()
+
+    if seqNo.counter != 3:
+      error "Counter should be 3 after wraparound",
+        counter = seqNo.counter, expected = 3
+      fail()