From 1d4f7c7a4312c431bf73a90457e9521aa98e1d3e Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Sun, 11 Jul 2021 09:59:42 +0200 Subject: [PATCH 1/3] avoid borrow, breaks logging (#604) --- libp2p/crypto/secp.nim | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/libp2p/crypto/secp.nim b/libp2p/crypto/secp.nim index 70f8e9b45..97c5ddb49 100644 --- a/libp2p/crypto/secp.nim +++ b/libp2p/crypto/secp.nim @@ -201,14 +201,18 @@ proc verify*[T: byte|char](sig: SkSignature, msg: openarray[T], let h = sha256.digest(msg) verify(secp256k1.SkSignature(sig), SkMessage(h.data), secp256k1.SkPublicKey(key)) -func clear*(key: var SkPrivateKey) {.borrow.} +func clear*(key: var SkPrivateKey) = clear(secp256k1.SkSecretKey(key)) -proc `$`*(key: SkPrivateKey): string {.borrow.} -proc `$`*(key: SkPublicKey): string {.borrow.} -proc `$`*(key: SkSignature): string {.borrow.} -proc `$`*(key: SkKeyPair): string {.borrow.} +func `$`*(key: SkPrivateKey): string = $secp256k1.SkSecretKey(key) +func `$`*(key: SkPublicKey): string = $secp256k1.SkPublicKey(key) +func `$`*(key: SkSignature): string = $secp256k1.SkSignature(key) +func `$`*(key: SkKeyPair): string = $secp256k1.SkKeyPair(key) -proc `==`*(a, b: SkPrivateKey): bool {.borrow.} -proc `==`*(a, b: SkPublicKey): bool {.borrow.} -proc `==`*(a, b: SkSignature): bool {.borrow.} -proc `==`*(a, b: SkKeyPair): bool {.borrow.} \ No newline at end of file +func `==`*(a, b: SkPrivateKey): bool = + secp256k1.SkSecretKey(a) == secp256k1.SkSecretKey(b) +func `==`*(a, b: SkPublicKey): bool = + secp256k1.SkPublicKey(a) == secp256k1.SkPublicKey(b) +func `==`*(a, b: SkSignature): bool = + secp256k1.SkSignature(a) == secp256k1.SkSignature(b) +func `==`*(a, b: SkKeyPair): bool = + secp256k1.SkKeyPair(a) == secp256k1.SkKeyPair(b) \ No newline at end of file From cb94baf9c4081a78a889957c93baa16aff3511de Mon Sep 17 00:00:00 2001 From: Tanguy Cizain Date: Mon, 19 Jul 2021 12:51:27 +0200 Subject: [PATCH 2/3] Fix transport's switch start (#609) * fix switch start * don't fail eagerly on transport start * handlecancel * raise exc --- libp2p/switch.nim | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/libp2p/switch.nim b/libp2p/switch.nim index 66bea4ef8..c4a653113 100644 --- a/libp2p/switch.nim +++ b/libp2p/switch.nim @@ -203,25 +203,6 @@ proc accept(s: Switch, transport: Transport) {.async.} = # noraises await conn.close() return -proc start*(s: Switch): Future[seq[Future[void]]] {.async, gcsafe.} = - trace "starting switch for peer", peerInfo = s.peerInfo - var startFuts: seq[Future[void]] - for t in s.transports: # for each transport - for i, a in s.peerInfo.addrs: - if t.handles(a): # check if it handles the multiaddr - var server = t.start(a) - s.peerInfo.addrs[i] = t.ma # update peer's address - s.acceptFuts.add(s.accept(t)) - startFuts.add(server) - - proc peerIdentifiedHandler(peerInfo: PeerInfo, event: PeerEvent) {.async.} = - s.peerStore.replace(peerInfo) - - s.connManager.addPeerEventHandler(peerIdentifiedHandler, PeerEventKind.Identified) - - debug "Started libp2p node", peer = s.peerInfo - return startFuts # listen for incoming connections - proc stop*(s: Switch) {.async.} = trace "Stopping switch" @@ -250,6 +231,33 @@ proc stop*(s: Switch) {.async.} = trace "Switch stopped" +proc start*(s: Switch): Future[seq[Future[void]]] {.async, gcsafe.} = + trace "starting switch for peer", peerInfo = s.peerInfo + var startFuts: seq[Future[void]] + for t in s.transports: # for each transport + for i, a in s.peerInfo.addrs: + if t.handles(a): # check if it handles the multiaddr + let transpStart = t.start(a) + startFuts.add(transpStart) + try: + await transpStart + s.peerInfo.addrs[i] = t.ma # update peer's address + s.acceptFuts.add(s.accept(t)) + except CancelledError as exc: + await s.stop() + raise exc + except CatchableError as exc: + debug "Failed to start one transport", address = $a, err = exc.msg + continue + + proc peerIdentifiedHandler(peerInfo: PeerInfo, event: PeerEvent) {.async.} = + s.peerStore.replace(peerInfo) + + s.connManager.addPeerEventHandler(peerIdentifiedHandler, PeerEventKind.Identified) + + debug "Started libp2p node", peer = s.peerInfo + return startFuts # listen for incoming connections + proc newSwitch*(peerInfo: PeerInfo, transports: seq[Transport], identity: Identify, From 8ea90037e5fc1009cc9c24903ae27da3aa691a47 Mon Sep 17 00:00:00 2001 From: Tanguy Cizain Date: Wed, 1 Sep 2021 08:41:11 +0200 Subject: [PATCH 3/3] Fix gossipsub incoming graft backoff (#616) * Fix gossipsub incoming graft backoff * Improve debug messages * clamp to 24h --- libp2p/protocols/pubsub/gossipsub/behavior.nim | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/libp2p/protocols/pubsub/gossipsub/behavior.nim b/libp2p/protocols/pubsub/gossipsub/behavior.nim index c9d8081e4..83be2af35 100644 --- a/libp2p/protocols/pubsub/gossipsub/behavior.nim +++ b/libp2p/protocols/pubsub/gossipsub/behavior.nim @@ -89,7 +89,7 @@ proc handleGraft*(g: GossipSub, # It is an error to GRAFT on a explicit peer if peer.peerId in g.parameters.directPeers: # receiving a graft from a direct peer should yield a more prominent warning (protocol violation) - warn "attempt to graft an explicit peer, peering agreements should be reciprocal", + warn "an explicit peer attempted to graft us, peering agreements should be reciprocal", peer, topic # and such an attempt should be logged and rejected with a PRUNE prunes.add(ControlPrune( @@ -105,10 +105,14 @@ proc handleGraft*(g: GossipSub, continue + # Check backingOff + # Ignore BackoffSlackTime here, since this only for outbound activity + # and subtract a second time to avoid race conditions + # (peers may wait to graft us as the exact instant they're allowed to) if g.backingOff .getOrDefault(topic) - .getOrDefault(peer.peerId) > Moment.now(): - debug "attempt to graft a backingOff peer", peer, topic + .getOrDefault(peer.peerId) - (BackoffSlackTime * 2).seconds > Moment.now(): + debug "a backingOff peer attempted to graft us", peer, topic # and such an attempt should be logged and rejected with a PRUNE prunes.add(ControlPrune( topicID: topic, @@ -162,13 +166,11 @@ proc handlePrune*(g: GossipSub, peer: PubSubPeer, prunes: seq[ControlPrune]) {.r # add peer backoff if prune.backoff > 0: let - # avoid overflows and follow params - # worst case if the remote thinks we are wrong we get penalized - # but we won't end up with ghost peers + # avoid overflows and clamp to reasonable value backoffSeconds = clamp( prune.backoff + BackoffSlackTime, 0'u64, - g.parameters.pruneBackoff.seconds.uint64 + BackoffSlackTime + 1.days.seconds.uint64 ) backoff = Moment.fromNow(backoffSeconds.int64.seconds) current = g.backingOff.getOrDefault(topic).getOrDefault(peer.peerId)