diff --git a/storage/discovery.nim b/storage/discovery.nim index 70e8d1b2..66da8c32 100644 --- a/storage/discovery.nim +++ b/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 diff --git a/storage/nat.nim b/storage/nat.nim index b1729713..07c56b2d 100644 --- a/storage/nat.nim +++ b/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 diff --git a/storage/storage.nim b/storage/storage.nim index 15165fcc..9374bcb8 100644 --- a/storage/storage.nim +++ b/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() diff --git a/tests/storage/helpers/nodeutils.nim b/tests/storage/helpers/nodeutils.nim index fd9e658f..108635a0 100644 --- a/tests/storage/helpers/nodeutils.nim +++ b/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()