fix(service-disco): apply advertExpiry cap after retry-time subtraction, not before

libp2p/protocols/service_discovery/registrar.nim

@@ -184,8 +184,6 @@ proc waitingTime*(
       if waitDuration < prevWaitDuration - elapsedDuration:
         waitDuration = prevWaitDuration - elapsedDuration
 
-  waitDuration = min(discoConfig.advertExpiry, waitDuration)
-
   return waitDuration
 
 proc updateLowerBounds*(
@@ -488,6 +486,8 @@ proc registration*(disco: ServiceDiscovery, peerId: PeerId, inMsg: Message): Mes
 
     return msg
 
+  tWait = min(max(disco.discoConfig.advertExpiry, ZeroDuration), tWait)
+
   disco.registrar.updateLowerBounds(serviceId, ad, tWait, now)
 
   var ticket = Ticket(

tests/libp2p/service_discovery/test_registrar.nim

@@ -227,13 +227,10 @@ suite "Service Discovery Registrar - Waiting Time Calculation":
     check w > 500.seconds
 
 suite "Service Discovery Registrar - advertExpiry cap":
-  # The wait time must never exceed advertExpiry: a wait longer than the
-  # advert's own lifetime is pointless and is clamped at the end of waitingTime.
-
-  test "waitingTime clamps formula-driven wait to advertExpiry":
+  test "waitingTime does not cap a formula-driven wait at advertExpiry":
     let registrar = Registrar.new()
     # cache at capacity ⇒ occupancy = 100.0; with safetyParam = 1.0 the
-    # uncapped w = 100 * 100.0 * 1.0 = 10000s ≫ advertExpiry (100s).
+    # w = 100 * 100.0 * 1.0 = 10000s ≫ advertExpiry (100s).
     let discoConfig =
       ServiceDiscoveryConfig.new(advertExpiry = 100.secs, safetyParam = 1.0)
     let serviceId = makeServiceId()
@@ -245,9 +242,9 @@ suite "Service Discovery Registrar - advertExpiry cap":
 
     let w = registrar.waitingTime(discoConfig, ad, 1000, serviceId, now)
 
-    check w == discoConfig.advertExpiry
+    check w == 10000.secs
 
-  test "waitingTime clamps lower-bound wait to advertExpiry":
+  test "waitingTime does not cap a lower-bound-driven wait at advertExpiry":
     let registrar = Registrar.new()
     let discoConfig = ServiceDiscoveryConfig.new() # advertExpiry = 900s (default)
     let serviceId = makeServiceId()
@@ -260,12 +257,12 @@ suite "Service Discovery Registrar - advertExpiry cap":
 
     let w = registrar.waitingTime(discoConfig, ad, 1000, serviceId, now)
 
-    check w == discoConfig.advertExpiry
+    check w == 100000.secs
 
-  test "waitingTime cap leaves a legitimately small wait untouched":
+  test "waitingTime returns a legitimately small wait untouched":
     let registrar = Registrar.new()
-    # Empty cache, safetyParam = 0.5 ⇒ uncapped w = 10000 * 0.5 = 5000s,
-    # comfortably below advertExpiry (10000s) so the clamp is a no-op.
+    # Empty cache, safetyParam = 0.5 ⇒ w = 10000 * 0.5 = 5000s, comfortably
+    # below advertExpiry (10000s) either way.
     let discoConfig =
       ServiceDiscoveryConfig.new(advertExpiry = 10000.secs, safetyParam = 0.5)
     let serviceId = makeServiceId()
@@ -277,6 +274,93 @@ suite "Service Discovery Registrar - advertExpiry cap":
     check w < discoConfig.advertExpiry
     check w == 5000.secs
 
+  test "registration caps the offered tWaitFor at advertExpiry":
+    let advertExpiry = 100.secs
+    let conf = ServiceDiscoveryConfig.new(
+      advertExpiry = advertExpiry, safetyParam = 1.0, advertCacheCap = 10
+    )
+    let disco = setupServiceDiscoveryNode(discoConfig = conf)
+    let serviceName = "service"
+    let serviceId = serviceName.hashServiceId()
+    let advertiserKey = PrivateKey.random(rng()).get()
+    let advertiserId = PeerId.init(advertiserKey).get()
+    let adBytes = makeAdvertisement(serviceName, advertiserKey).encode().get()
+
+    # Saturate the cache: occupancy pins at 100.0, so w = 100*100*1.0 = 10000s.
+    for i in 0 ..< 10:
+      disco.registrar.cacheTimestamps[(peerId: randomPeerId(), seqNo: uint64(i))] =
+        Moment.now()
+
+    let inMsg = kadprotobuf.Message(
+      msgType: kadprotobuf.MessageType.register,
+      key: serviceId,
+      register: Opt.some(
+        kadprotobuf.RegisterMessage(
+          advertisement: adBytes,
+          status: Opt.none(kadprotobuf.RegistrationStatus),
+          ticket: Opt.none(Ticket),
+        )
+      ),
+    )
+    let reply = disco.registration(advertiserId, inMsg).register.get()
+
+    check reply.status.get() == kadprotobuf.RegistrationStatus.Wait
+    check reply.ticket.get().tWaitFor.get() == advertExpiry
+
+  test "sustained overload keeps offering advertExpiry-length waits across retries":
+    # Regression test: previously the advertExpiry cap was applied inside
+    # waitingTime() *before* elapsed retry time was subtracted, so an
+    # advertiser retrying after waiting exactly advertExpiry seconds would
+    # see tWait collapse to ~0 (= Confirmed) regardless of whether the
+    # registrar was still overloaded. The cap must apply *after* the
+    # elapsed-time subtraction, so a still-overloaded registrar keeps
+    # offering a full advertExpiry-length wait on every retry.
+    let advertExpiry = 100.secs
+    let registrationWindow = 10.secs
+    let conf = ServiceDiscoveryConfig.new(
+      advertExpiry = advertExpiry,
+      safetyParam = 1.0,
+      advertCacheCap = 10,
+      registrationWindow = registrationWindow,
+    )
+    let disco = setupServiceDiscoveryNode(discoConfig = conf)
+    let serviceName = "service"
+    let serviceId = serviceName.hashServiceId()
+    let advertiserKey = PrivateKey.random(rng()).get()
+    let advertiserId = PeerId.init(advertiserKey).get()
+    let adBytes = makeAdvertisement(serviceName, advertiserKey).encode().get()
+
+    for i in 0 ..< 10:
+      disco.registrar.cacheTimestamps[(peerId: randomPeerId(), seqNo: uint64(i))] =
+        Moment.now()
+
+    let firstAttemptTime =
+      Moment.init((Moment.now() - advertExpiry).epochSeconds, Second)
+    var retryTicket = Ticket(
+      advertisement: adBytes,
+      tInit: firstAttemptTime,
+      tMod: firstAttemptTime,
+      tWaitFor: advertExpiry,
+      signature: Opt.none(seq[byte]),
+    )
+    check retryTicket.sign(disco.switch.peerInfo.privateKey).isOk()
+
+    let inMsg = kadprotobuf.Message(
+      msgType: kadprotobuf.MessageType.register,
+      key: serviceId,
+      register: Opt.some(
+        kadprotobuf.RegisterMessage(
+          advertisement: adBytes,
+          status: Opt.none(kadprotobuf.RegistrationStatus),
+          ticket: Opt.some(retryTicket),
+        )
+      ),
+    )
+    let reply = disco.registration(advertiserId, inMsg).register.get()
+
+    check reply.status.get() == kadprotobuf.RegistrationStatus.Wait
+    check reply.ticket.get().tWaitFor.get() == advertExpiry
+
 suite "Service Discovery Registrar - Lower Bound Enforcement":
   test "waitingTime enforces service lower bound when exists":
     let registrar = Registrar.new()