diff --git a/storage/discovery.nim b/storage/discovery.nim index 919ff237..87c88262 100644 --- a/storage/discovery.nim +++ b/storage/discovery.nim @@ -180,7 +180,7 @@ proc getSpr*(d: Discovery): SignedPeerRecord = ## Returns the node's current Signed Peer Record as registered in the DHT. d.protocol.getRecord() -proc updateRecordsAndSpr*( +proc announceDirectAddrs*( d: Discovery, announceAddrs: openArray[MultiAddress], udpPort: Port ) = # UDP addresses are derived from TCP announce addresses by remapping protocol and port. @@ -204,15 +204,18 @@ proc updateRecordsAndSpr*( .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. - # Relay addresses should not pollute DHT routing. +proc announceRelayAddrs*(d: Discovery, addrs: openArray[MultiAddress]) = + ## Updates only announce addresses + ## When using relay, the DHT routing record is not updated to not pollute the DHT. d.announceAddrs = @addrs info "Updating announce record", addrs = d.announceAddrs d.providerRecord = SignedPeerRecord .init(d.key, PeerRecord.init(d.peerId, d.announceAddrs)) .expect("Should construct signed record").some + if not d.protocol.isNil: + d.protocol.updateRecord(d.providerRecord).expect("Should update SPR") + proc start*(d: Discovery) {.async: (raises: []).} = try: d.protocol.open() @@ -264,7 +267,7 @@ proc new*( # 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) + self.announceDirectAddrs(announceAddrs, udpPort = discoveryPort) let discoveryConfig = DiscoveryConfig(tableIpLimits: tableIpLimits, bitsPerHop: DefaultBitsPerHop) diff --git a/storage/nat.nim b/storage/nat.nim index e9769011..5a12b675 100644 --- a/storage/nat.nim +++ b/storage/nat.nim @@ -137,7 +137,7 @@ proc announcePeerInfoAddrs*(discovery: Discovery, peerInfo: PeerInfo, udpPort: P let addrs = peerInfo.addrs.filterIt(not it.isCircuitRelayMA()) if addrs.len == 0 or addrs == discovery.announceAddrs: return - discovery.updateRecordsAndSpr(addrs, udpPort = udpPort) + discovery.announceDirectAddrs(addrs, udpPort = udpPort) proc setupPeerInfoObserver*( switch: Switch, @@ -193,7 +193,7 @@ method handleNatStatus*( if m.tcpMappingId.isSome and m.udpMappingId.isSome: m.close() - discovery.updateRecordsAndSpr(@[], udpPort = discoveryPort) + discovery.announceDirectAddrs(@[], udpPort = discoveryPort) elif m.tcpMappingId.isSome and m.udpMappingId.isSome: warn "Not Reachable with active port mapping. The port mapping will be deleted and relay will start." @@ -203,7 +203,7 @@ method handleNatStatus*( # We remove the announced records. # Eventually, it will we updated by the relay when it started - discovery.updateRecordsAndSpr(@[], udpPort = discoveryPort) + discovery.announceDirectAddrs(@[], udpPort = discoveryPort) elif autoRelayService.isRunning: # The mapping was already tried and did not make the node reachable. # If the relay is running, there is nothing to do. @@ -233,7 +233,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.updateRecordsAndSpr(@[announceAddress], udpPort = udpPort) + discovery.announceDirectAddrs(@[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 18a5270f..e2479bdd 100644 --- a/storage/storage.nim +++ b/storage/storage.nim @@ -113,7 +113,7 @@ proc start*(s: StorageServer) {.async.} = ip = some(s.config.nat.extIp), port = none(Port) ) ] - s.storageNode.discovery.updateRecordsAndSpr( + s.storageNode.discovery.announceDirectAddrs( announceAddresses, udpPort = s.config.discoveryPort ) else: @@ -468,7 +468,7 @@ proc new*( onReservation = proc(addresses: seq[MultiAddress]) {.gcsafe, raises: [].} = info "Relay reservation updated", addresses # relay addresses are for download traffic only, not DHT routing - discovery.updateAnnounceRecord(addresses), + discovery.announceRelayAddrs(addresses), rng = random.Rng.instance(), ) diff --git a/tests/storage/helpers/nodeutils.nim b/tests/storage/helpers/nodeutils.nim index f34daa21..97d43add 100644 --- a/tests/storage/helpers/nodeutils.nim +++ b/tests/storage/helpers/nodeutils.nim @@ -224,7 +224,7 @@ proc generateNodes*( if config.enableBootstrap: waitFor switch.peerInfo.update() - blockDiscovery.updateRecordsAndSpr( + blockDiscovery.announceDirectAddrs( switch.peerInfo.addrs, udpPort = bindPort.Port ) bootstrapNodes.add blockDiscovery.getSpr() diff --git a/tests/storage/testdiscovery.nim b/tests/storage/testdiscovery.nim index a32d4401..8178319b 100644 --- a/tests/storage/testdiscovery.nim +++ b/tests/storage/testdiscovery.nim @@ -23,18 +23,18 @@ suite "Discovery - SPR record logic": key = PrivateKey.random(Rng.instance()).get() disc = Discovery.new(key, announceAddrs = @[]) - test "updateRecordsAndSpr sets the SPR with both TCP and UDP addresses": - disc.updateRecordsAndSpr(@[directAddr], udpPort) + test "announceDirectAddrs sets the SPR with both TCP and UDP addresses": + disc.announceDirectAddrs(@[directAddr], udpPort) let spr = disc.getSpr() let addrs = spr.data.addresses.mapIt($it.address) check addrs.anyIt(it.contains("/tcp/")) check addrs.anyIt(it.contains("/udp/")) - test "updateAnnounceRecord does not update the SPR": - disc.updateRecordsAndSpr(@[directAddr], udpPort) - let sprBefore = disc.getSpr() + test "announceRelayAddrs updates the SPR with the announce addresses": + disc.announceDirectAddrs(@[directAddr], udpPort) - disc.updateAnnounceRecord(@[relayAddr]) + disc.announceRelayAddrs(@[relayAddr]) - check disc.getSpr() == sprBefore + let addrs = disc.getSpr().data.addresses.mapIt($it.address) + check addrs == @[$relayAddr]