fixes

Author: tinniaru3005

libp2p/protocols/service_discovery/registrar.nim

@@ -379,8 +379,14 @@ proc updateExistingAd*(
     registrar.cacheTimestamps[existing.toAdvertisementKey()] = now
     return false
   elif ad.data.seqNo > existing.data.seqNo:
+    let oldKey = existing.toAdvertisementKey()
+    for _, sads in registrar.cache.mpairs:
+      for j in 0 ..< sads.len:
+        if sads[j].toAdvertisementKey() == oldKey:
+          sads[j] = ad
+
     registrar.ipTree.removeAd(existing)
-    registrar.cacheTimestamps.del(existing.toAdvertisementKey())
+    registrar.cacheTimestamps.del(oldKey)
     ads[idx] = ad
     registrar.cacheTimestamps[ad.toAdvertisementKey()] = now
     registrar.ipTree.insertAd(ad)
@@ -398,11 +404,15 @@ proc insertNewAd*(
   ## Insert a brand-new advertisement into the cache.
   ## Evicts the globally oldest entry first if the cache is at capacity.
   ## Returns true (a new insertion always warrants a metrics update).
-  if disco.registrar.cacheTimestamps.len.uint64 >= disco.discoConfig.advertCacheCap:
+  let key = ad.toAdvertisementKey()
+  let isNewKey = key notin disco.registrar.cacheTimestamps
+  if isNewKey and
+      disco.registrar.cacheTimestamps.len.uint64 >= disco.discoConfig.advertCacheCap:
     evictOldestAd(disco, serviceId, ads)
   ads.add(ad)
-  disco.registrar.cacheTimestamps[ad.toAdvertisementKey()] = now
-  disco.registrar.ipTree.insertAd(ad)
+  disco.registrar.cacheTimestamps[key] = now
+  if isNewKey:
+    disco.registrar.ipTree.insertAd(ad)
   return true
 
 proc acceptAdvertisement*(

tests/libp2p/service_discovery/test_registrar.nim

@@ -1336,6 +1336,32 @@ suite "Service Discovery Registrar - updateExistingAd":
     check currentAd.toAdvertisementKey() in registrar.cacheTimestamps
     check staleAd.toAdvertisementKey() notin registrar.cacheTimestamps
 
+  test "higher seqNo replaces stale copies in every other service that cached it":
+    let registrar = Registrar.new()
+    let serviceId1 = makeServiceId(1)
+    let serviceId2 = makeServiceId(2)
+    let privateKey = PrivateKey.random(rng()).get()
+    let oldAd = makeAdvertisement(privateKey = privateKey, seqNo = 1)
+    let newAd = makeAdvertisement(privateKey = privateKey, seqNo = 2)
+
+    registrar.cache[serviceId1] = @[oldAd]
+    registrar.cache[serviceId2] = @[oldAd]
+    registrar.cacheTimestamps[oldAd.toAdvertisementKey()] = initMoment(1000)
+    registrar.ipTree.insertAd(oldAd)
+
+    var ads1 = registrar.cache[serviceId1]
+    let changed = registrar.updateExistingAd(ads1, 0, newAd, initMoment(2000))
+    registrar.cache[serviceId1] = ads1
+
+    check changed
+    check registrar.cache[serviceId1].len == 1
+    check registrar.cache[serviceId1][0].data.seqNo == 2
+    # serviceId2 was never passed into updateExistingAd directly.
+    check registrar.cache[serviceId2].len == 1
+    check registrar.cache[serviceId2][0].data.seqNo == 2
+    check oldAd.toAdvertisementKey() notin registrar.cacheTimestamps
+    check newAd.toAdvertisementKey() in registrar.cacheTimestamps
+
 suite "Service Discovery Registrar - insertNewAd":
   test "inserts ad into cache, IP tree, and timestamps, returns true":
     let disco =
@@ -1354,6 +1380,27 @@ suite "Service Discovery Registrar - insertNewAd":
     check disco.registrar.cacheTimestamps[ad.toAdvertisementKey()] == now
     check disco.registrar.ipTree.root.counter > 0
 
+  test "same ad accepted for three services counts the IP tree only once":
+    let disco =
+      setupServiceDiscoveryNode(discoConfig = ServiceDiscoveryConfig.new(fReturn = 3))
+    let serviceId1 = makeServiceId(1)
+    let serviceId2 = makeServiceId(2)
+    let serviceId3 = makeServiceId(3)
+    let ad = makeAdvertisement(addrs = @[makeMultiAddress("10.0.0.1")])
+    let now = initMoment(1000)
+
+    var ads1: seq[Advertisement] = @[]
+    var ads2: seq[Advertisement] = @[]
+    var ads3: seq[Advertisement] = @[]
+    discard disco.insertNewAd(serviceId1, ads1, ad, now)
+    discard disco.insertNewAd(serviceId2, ads2, ad, now)
+    discard disco.insertNewAd(serviceId3, ads3, ad, now)
+    disco.registrar.cache[serviceId1] = ads1
+    disco.registrar.cache[serviceId2] = ads2
+    disco.registrar.cache[serviceId3] = ads3
+
+    check disco.registrar.ipTree.root.counter == 1
+
   test "inserts ad without eviction when cache is under capacity":
     let disco = setupServiceDiscoveryNode(
       discoConfig = ServiceDiscoveryConfig.new(fReturn = 3, advertExpiry = 900.secs)