Use local record as source of truth for the spr

storage/discovery.nim

@@ -177,27 +177,10 @@ method removeProvider*(
     raiseAssert("Unexpected Exception in removeProvider")
 
 proc getSpr*(d: Discovery): ?SignedPeerRecord =
-  ## Combined TCP+UDP record for bootstrap use by connecting nodes.
-  without providerRecord =? d.providerRecord:
-    return none(SignedPeerRecord)
+  ## Returns the node's current Signed Peer Record as registered in the DHT.
+  some(d.protocol.getRecord())
 
-  without dhtRecord =? d.dhtRecord:
-    return none(SignedPeerRecord)
-
-  let tcpAddrs = providerRecord.data.addresses.mapIt(it.address)
-  let udpAddrs = dhtRecord.data.addresses.mapIt(it.address)
-
-  SignedPeerRecord
-    .init(d.key, PeerRecord.init(d.peerId, tcpAddrs & udpAddrs))
-    .expect("Should construct signed record").some
-
-proc updateSpr(d: Discovery) =
-  if not d.protocol.isNil:
-    let spr = d.getSpr()
-    if spr.isSome:
-      d.protocol.updateRecord(spr).expect("Should update SPR")
-
-proc updateRecords*(
+proc updateRecordsAndSpr*(
     d: Discovery, announceAddrs: openArray[MultiAddress], udpPort: Port
 ) =
   # UDP addresses are derived from TCP announce addresses by remapping protocol and port.
@@ -215,7 +198,11 @@ proc updateRecords*(
     .init(d.key, PeerRecord.init(d.peerId, udpAddrs))
     .expect("Should construct signed record").some
 
-  d.updateSpr()
+  if not d.protocol.isNil:
+    let spr = SignedPeerRecord
+      .init(d.key, PeerRecord.init(d.peerId, tcpAddrs & udpAddrs))
+      .expect("Should construct signed record").some
+    d.protocol.updateRecord(spr).expect("Should update SPR")
 
 proc updateAnnounceRecord*(d: Discovery, addrs: openArray[MultiAddress]) =
   # Updates announce addresses only, not the DHT routing record.
@@ -226,18 +213,14 @@ proc updateAnnounceRecord*(d: Discovery, addrs: openArray[MultiAddress]) =
     .init(d.key, PeerRecord.init(d.peerId, d.announceAddrs))
     .expect("Should construct signed record").some
 
-  d.updateSpr()
-
 proc updateDhtRecord*(
     d: Discovery, addrs: openArray[MultiAddress]
-) {.deprecated: "use updateRecords instead".} =
+) {.deprecated: "use updateRecordsAndSpr instead".} =
   info "Updating Dht record", addrs = addrs
   d.dhtRecord = SignedPeerRecord
     .init(d.key, PeerRecord.init(d.peerId, @addrs))
     .expect("Should construct signed record").some
 
-  d.updateSpr()
-
 proc start*(d: Discovery) {.async: (raises: []).} =
   try:
     d.protocol.open()
@@ -287,7 +270,9 @@ proc new*(
     key: key, peerId: PeerId.init(key).expect("Should construct PeerId"), store: store
   )
 
-  self.updateRecords(announceAddrs, udpPort = discoveryPort)
+  # Called even when announceAddrs is empty: newProtocol below requires
+  # providerRecord to be set, and it will be updated with real addresses in start().
+  self.updateRecordsAndSpr(announceAddrs, udpPort = discoveryPort)
 
   let discoveryConfig =
     DiscoveryConfig(tableIpLimits: tableIpLimits, bitsPerHop: DefaultBitsPerHop)
@@ -303,4 +288,7 @@ proc new*(
     config = discoveryConfig,
   )
 
+  # Protocol now exists: call again so the SPR is synced into the protocol's local record.
+  self.updateRecordsAndSpr(announceAddrs, udpPort = discoveryPort)
+
   self

storage/nat.nim

@@ -152,7 +152,7 @@ method handleNatStatus*(
       else:
         debug "AutoRelayService stopped"
 
-    discovery.updateRecords(@[dialBackAddr.get], udpPort = discoveryPort)
+    discovery.updateRecordsAndSpr(@[dialBackAddr.get], udpPort = discoveryPort)
     discovery.protocol.clientMode = false
   of NotReachable:
     var hasPortMapping = false
@@ -171,7 +171,7 @@ method handleNatStatus*(
 
       # We remove the announced records.
       # Eventually, it will we updated by the relay when it started
-      discovery.updateRecords(@[], udpPort = discoveryPort)
+      discovery.updateRecordsAndSpr(@[], udpPort = discoveryPort)
     else:
       debug "Node is not reachable trying port mapping now"
 
@@ -196,7 +196,7 @@ method handleNatStatus*(
         # The client mode will be updated on the next iteration of autonat.
         # Trying to check manually that the node is reachable is not trivial,
         # this is exactly what Autonat is for.
-        discovery.updateRecords(@[announceAddress], udpPort = udpPort)
+        discovery.updateRecordsAndSpr(@[announceAddress], udpPort = udpPort)
         hasPortMapping = true
       else:
         # In case of failure, close the port mapping in order to rerun discover

storage/storage.nim

@@ -107,13 +107,15 @@ proc start*(s: StorageServer) {.async.} =
     else:
       # Don't announce address and wait for AutoNat
       @[]
-  info "info ", addrs = $s.storageNode.switch.peerInfo.addrs[0]
+
   if not s.config.nat.hasExtIp:
     # Nodes with autonat start with client mode.
     # It will be updated if reachable.
     s.storageNode.discovery.protocol.clientMode = true
 
-  s.storageNode.discovery.updateRecords(announceAddrs, udpPort = s.config.discoveryPort)
+  s.storageNode.discovery.updateRecordsAndSpr(
+    announceAddrs, udpPort = s.config.discoveryPort
+  )
 
   await s.storageNode.start()
 

tests/storage/helpers/nodeutils.nim

@@ -224,7 +224,9 @@ proc generateNodes*(
 
         if config.enableBootstrap:
           waitFor switch.peerInfo.update()
-          blockDiscovery.updateRecords(switch.peerInfo.addrs, udpPort = bindPort.Port)
+          blockDiscovery.updateRecordsAndSpr(
+            switch.peerInfo.addrs, udpPort = bindPort.Port
+          )
           if blockDiscovery.getSpr().isSome:
             bootstrapNodes.add !blockDiscovery.getSpr()