From 021d618e6fb3aac08a1666c6941596eeeb1358e7 Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Fri, 27 Sep 2024 12:04:37 +0200 Subject: [PATCH 1/9] chore: remove useless `registerEvent` --- libp2p/protocols/rendezvous.nim | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index 06155bad24..eeed84e743 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -312,8 +312,6 @@ type salt: string defaultDT: Moment registerDeletionLoop: Future[void] - #registerEvent: AsyncEvent # TODO: to raise during the heartbeat - # + make the heartbeat sleep duration "smarter" sema: AsyncSemaphore peers: seq[PeerId] cookiesSaved: Table[PeerId, Table[string, seq[byte]]] @@ -402,7 +400,6 @@ proc save( ) ) rdv.namespaces[nsSalted].add(rdv.registered.high) - # rdv.registerEvent.fire() except KeyError: doAssert false, "Should have key" @@ -708,7 +705,6 @@ proc new*( salt: string.fromBytes(generateBytes(rng[], 8)), registered: initOffsettedSeq[RegisteredData](1), defaultDT: Moment.now() - 1.days, - #registerEvent: newAsyncEvent(), sema: newAsyncSemaphore(SemaphoreDefaultSize), minDuration: minDuration, maxDuration: maxDuration, From cdbbabf2e260391cdb9b13cac77b2a381b8dda5e Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Fri, 27 Sep 2024 12:05:12 +0200 Subject: [PATCH 2/9] chore: change `advertise` from method to proc --- libp2p/protocols/rendezvous.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index eeed84e743..938293aafd 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -530,9 +530,9 @@ proc advertise*( await allFutures(futs) -method advertise*( +proc advertise*( rdv: RendezVous, ns: string, ttl: Duration = rdv.minDuration -) {.async, base.} = +) {.async.} = await rdv.advertise(ns, ttl, rdv.peers) proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = From 157279b0d92cab4ee000a72b3ed781bdfe9d97a2 Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Fri, 27 Sep 2024 12:05:50 +0200 Subject: [PATCH 3/9] docs: add documentation to public rendezvous procedure --- libp2p/protocols/rendezvous.nim | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index 938293aafd..ece1a684f8 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -508,6 +508,11 @@ proc advertisePeer(rdv: RendezVous, peer: PeerId, msg: seq[byte]) {.async.} = proc advertise*( rdv: RendezVous, ns: string, ttl: Duration, peers: seq[PeerId] ) {.async.} = + ## The advertise async procedure sends advertisements for a namespace + ## to a sequence of peers. It encodes and sends a signed peer record + ## along with a time-to-live value, validating the inputs. Advertisements + ## are sent concurrently to all specified peers. + ## if ns.len notin 1 .. 255: raise newException(RendezVousError, "Invalid namespace") @@ -536,6 +541,10 @@ proc advertise*( await rdv.advertise(ns, ttl, rdv.peers) proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = + ## This procedure returns all the peers already registered on the + ## given namespace. This function is synchronous, no remote + ## request is done. + ## let nsSalted = ns & rdv.salt n = Moment.now() @@ -552,6 +561,11 @@ proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = proc request*( rdv: RendezVous, ns: string, l: int = DiscoverLimit.int, peers: seq[PeerId] ): Future[seq[PeerRecord]] {.async.} = + ## This async procedure request discovers peer records for a given namespace + ## by dialing peers, sending requests, and processing responses. It handles + ## response validation, cookie updates, and limits the number of peer records + ## retrieved based on the provided limit. + ## var s: Table[PeerId, (PeerRecord, Register)] limit: uint64 @@ -643,6 +657,11 @@ proc unsubscribeLocally*(rdv: RendezVous, ns: string) = return proc unsubscribe*(rdv: RendezVous, ns: string, peerIds: seq[PeerId]) {.async.} = + ## The async unsubscribe procedure removes peers from a namespace by + ## sending an "Unregister" message to each peer. It validates the + ## namespace, dials the peers. The operation is bounded by a timeout + ## for unsubscribing from all peers. + ## if ns.len notin 1 .. 255: raise newException(RendezVousError, "Invalid namespace") From a6c5343998d3ad1deef934e2ae4fc4331b4f53fd Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Fri, 27 Sep 2024 12:12:06 +0200 Subject: [PATCH 4/9] docs: make comments clearer --- libp2p/protocols/rendezvous.nim | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index ece1a684f8..3c515868e6 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -510,8 +510,8 @@ proc advertise*( ) {.async.} = ## The advertise async procedure sends advertisements for a namespace ## to a sequence of peers. It encodes and sends a signed peer record - ## along with a time-to-live value, validating the inputs. Advertisements - ## are sent concurrently to all specified peers. + ## along with a time-to-live value. Advertisements are sent + ## concurrently to all specified peers. ## if ns.len notin 1 .. 255: raise newException(RendezVousError, "Invalid namespace") @@ -542,8 +542,7 @@ proc advertise*( proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = ## This procedure returns all the peers already registered on the - ## given namespace. This function is synchronous, no remote - ## request is done. + ## given namespace. This function is synchronous ## let nsSalted = ns & rdv.salt @@ -561,10 +560,9 @@ proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = proc request*( rdv: RendezVous, ns: string, l: int = DiscoverLimit.int, peers: seq[PeerId] ): Future[seq[PeerRecord]] {.async.} = - ## This async procedure request discovers peer records for a given namespace - ## by dialing peers, sending requests, and processing responses. It handles - ## response validation, cookie updates, and limits the number of peer records - ## retrieved based on the provided limit. + ## This async procedure request discovers and returns peers for + ## a given namespace by sending requests and processing responses. + ## It limits the number of peer records retrieved based on the provided limit. ## var s: Table[PeerId, (PeerRecord, Register)] @@ -658,9 +656,8 @@ proc unsubscribeLocally*(rdv: RendezVous, ns: string) = proc unsubscribe*(rdv: RendezVous, ns: string, peerIds: seq[PeerId]) {.async.} = ## The async unsubscribe procedure removes peers from a namespace by - ## sending an "Unregister" message to each peer. It validates the - ## namespace, dials the peers. The operation is bounded by a timeout - ## for unsubscribing from all peers. + ## sending an "Unregister" message to each connected peer. The operation + ## is bounded by a timeout for unsubscribing from all peers. ## if ns.len notin 1 .. 255: raise newException(RendezVousError, "Invalid namespace") From db2699b3f1ba3c32200059d9bd26bc52b37b50ef Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Tue, 1 Oct 2024 12:17:08 +0200 Subject: [PATCH 5/9] docs: change advertisements to registration --- libp2p/protocols/rendezvous.nim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index 3c515868e6..0c1758c85b 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -506,11 +506,11 @@ proc advertisePeer(rdv: RendezVous, peer: PeerId, msg: seq[byte]) {.async.} = discard await advertiseWrap().withTimeout(5.seconds) proc advertise*( - rdv: RendezVous, ns: string, ttl: Duration, peers: seq[PeerId] + rdv: RendezVous, ns: string, ttl: Duration = rdv.minDuration, peers: seq[PeerId] ) {.async.} = - ## The advertise async procedure sends advertisements for a namespace + ## The advertise async procedure sends a registration for a namespace ## to a sequence of peers. It encodes and sends a signed peer record - ## along with a time-to-live value. Advertisements are sent + ## along with a time-to-live value. The registrations are sent ## concurrently to all specified peers. ## if ns.len notin 1 .. 255: From 43d1ce2ef1700129badc14f8717542a0f701beff Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Thu, 3 Oct 2024 11:28:14 +0200 Subject: [PATCH 6/9] fix: address comments --- libp2p/protocols/rendezvous.nim | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index 0c1758c85b..35c7d8712d 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -542,7 +542,7 @@ proc advertise*( proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = ## This procedure returns all the peers already registered on the - ## given namespace. This function is synchronous + ## given namespace. ## let nsSalted = ns & rdv.salt @@ -558,22 +558,22 @@ proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = @[] proc request*( - rdv: RendezVous, ns: string, l: int = DiscoverLimit.int, peers: seq[PeerId] + rdv: RendezVous, ns: string, limit: int = DiscoverLimit.int, peers: seq[PeerId] ): Future[seq[PeerRecord]] {.async.} = - ## This async procedure request discovers and returns peers for - ## a given namespace by sending requests and processing responses. - ## It limits the number of peer records retrieved based on the provided limit. + ## This async procedure discovers and returns peers for a given namespace + ## by sending requests and processing responses. It limits the number of + ## peer records retrieved based on the provided limit. ## var s: Table[PeerId, (PeerRecord, Register)] limit: uint64 d = Discover(ns: ns) - if l <= 0 or l > DiscoverLimit.int: + if limit <= 0 or limit > DiscoverLimit.int: raise newException(RendezVousError, "Invalid limit") if ns.len notin 0 .. 255: raise newException(RendezVousError, "Invalid namespace") - limit = l.uint64 + limit = limit.uint64 proc requestPeer(peer: PeerId) {.async.} = let conn = await rdv.switch.dial(peer, RendezVousCodec) defer: @@ -641,9 +641,9 @@ proc request*( return toSeq(s.values()).mapIt(it[0]) proc request*( - rdv: RendezVous, ns: string, l: int = DiscoverLimit.int + rdv: RendezVous, ns: string, limit: int = DiscoverLimit.int ): Future[seq[PeerRecord]] {.async.} = - await rdv.request(ns, l, rdv.peers) + await rdv.request(ns, limit, rdv.peers) proc unsubscribeLocally*(rdv: RendezVous, ns: string) = let nsSalted = ns & rdv.salt From ea7d6c8b3adb44c847c44f9d85d3a23f7e8ca081 Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Thu, 3 Oct 2024 12:00:58 +0200 Subject: [PATCH 7/9] fix: limit --- libp2p/protocols/rendezvous.nim | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index 35c7d8712d..35f1416727 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -558,7 +558,7 @@ proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = @[] proc request*( - rdv: RendezVous, ns: string, limit: int = DiscoverLimit.int, peers: seq[PeerId] + rdv: RendezVous, ns: string, limit: uint64 = DiscoverLimit, peers: seq[PeerId] ): Future[seq[PeerRecord]] {.async.} = ## This async procedure discovers and returns peers for a given namespace ## by sending requests and processing responses. It limits the number of @@ -566,14 +566,12 @@ proc request*( ## var s: Table[PeerId, (PeerRecord, Register)] - limit: uint64 d = Discover(ns: ns) - if limit <= 0 or limit > DiscoverLimit.int: + if limit > DiscoverLimit.int: raise newException(RendezVousError, "Invalid limit") if ns.len notin 0 .. 255: raise newException(RendezVousError, "Invalid namespace") - limit = limit.uint64 proc requestPeer(peer: PeerId) {.async.} = let conn = await rdv.switch.dial(peer, RendezVousCodec) defer: @@ -641,7 +639,7 @@ proc request*( return toSeq(s.values()).mapIt(it[0]) proc request*( - rdv: RendezVous, ns: string, limit: int = DiscoverLimit.int + rdv: RendezVous, ns: string, limit: uint64 = DiscoverLimit ): Future[seq[PeerRecord]] {.async.} = await rdv.request(ns, limit, rdv.peers) From 4f3df474cdd0a17c3b2ae3b2dff37adc84eea702 Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Thu, 3 Oct 2024 16:46:49 +0200 Subject: [PATCH 8/9] fix: remove bad cast on DiscoveryLimit --- libp2p/protocols/rendezvous.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index 35f1416727..e4c2d84eab 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -568,7 +568,7 @@ proc request*( s: Table[PeerId, (PeerRecord, Register)] d = Discover(ns: ns) - if limit > DiscoverLimit.int: + if limit > DiscoverLimit: raise newException(RendezVousError, "Invalid limit") if ns.len notin 0 .. 255: raise newException(RendezVousError, "Invalid namespace") From f6d1530877de88faff69c0d5a245d5692f3b236e Mon Sep 17 00:00:00 2001 From: Ludovic Chenut Date: Thu, 3 Oct 2024 17:15:13 +0200 Subject: [PATCH 9/9] fix: replace limit by peerLimit --- libp2p/protocols/rendezvous.nim | 3 ++- tests/testrendezvous.nim | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libp2p/protocols/rendezvous.nim b/libp2p/protocols/rendezvous.nim index e4c2d84eab..089f3a5f36 100644 --- a/libp2p/protocols/rendezvous.nim +++ b/libp2p/protocols/rendezvous.nim @@ -558,7 +558,7 @@ proc requestLocally*(rdv: RendezVous, ns: string): seq[PeerRecord] = @[] proc request*( - rdv: RendezVous, ns: string, limit: uint64 = DiscoverLimit, peers: seq[PeerId] + rdv: RendezVous, ns: string, peerLimit: uint64 = DiscoverLimit, peers: seq[PeerId] ): Future[seq[PeerRecord]] {.async.} = ## This async procedure discovers and returns peers for a given namespace ## by sending requests and processing responses. It limits the number of @@ -566,6 +566,7 @@ proc request*( ## var s: Table[PeerId, (PeerRecord, Register)] + limit = peerLimit d = Discover(ns: ns) if limit > DiscoverLimit: diff --git a/tests/testrendezvous.nim b/tests/testrendezvous.nim index e0c4c25776..d42ac9cc7b 100644 --- a/tests/testrendezvous.nim +++ b/tests/testrendezvous.nim @@ -130,8 +130,6 @@ suite "RendezVous": switch = createSwitch(rdv) expect RendezVousError: discard await rdv.request("A".repeat(300)) - expect RendezVousError: - discard await rdv.request("A", -1) expect RendezVousError: discard await rdv.request("A", 3000) expect RendezVousError: