fix(libp2p,kademlia): fixes discrepency - kademlia and libp2p peers list

[istae]

Added extra checks in libp2p after connect calls to kademlia. This prevents peers that disconnect in the background while the stream handle is ongoing, from remining in the kademlia list

---

This change is 

[anatollupacescu]

should this be in singular (found)?

[mrekucci]

Except the typo

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @acud, @aloknerurkar, and @esadakar)

[acud]

Reviewed 1 of 2 files at r1.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @aloknerurkar, @esadakar, and @janos)

---

pkg/p2p/libp2p/libp2p.go, line 267 at r1 (raw file):

		overlay := i.BzzAddress.Overlay

		if s.notifier == nil {

i think this can be removed. notifier is never nil while the node is running

---

pkg/p2p/libp2p/libp2p.go, line 363 at r1 (raw file):

		s.metrics.HandledStreamCount.Inc()
		if !s.peers.Exists(overlay) {
			s.logger.Debugf("libp2p: inbound peer %s does not exist, disconnecting", overlay)

isn't the prefix here supposed to be stream handler?

---

pkg/p2p/libp2p/libp2p.go, line 368 at r1 (raw file):

		}

		s.logger.Debugf("successfully connected to peer %s%s (inbound)", i.BzzAddress.ShortString(), i.LightString())

i would add the same prefix to these messages here too and double check all log lines printed in this function have it too

---

pkg/p2p/libp2p/libp2p.go, line 602 at r1 (raw file):

				_ = s.Disconnect(overlay)
				s.protocolsmu.RUnlock()
				return nil, fmt.Errorf("connectOut: protocol %w", err)

consider: adding information about which protocol failed. the error is wrapped but the protocol in the message doesn't really say anything. more information will be availble from tn

---

pkg/p2p/libp2p/libp2p.go, line 609 at r1 (raw file):

	if !s.peers.Exists(overlay) {
		s.logger.Debugf("libp2p Connect: peer %s does not exist, disconnecting", overlay)

it is not necessary to log and return an error. you can decorate the returned error with more information when necessary and the calling context will log is as necessary

[istae]

Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @acud, @aloknerurkar, @anatollupacescu, @esadakar, and @janos)

---

pkg/p2p/libp2p/libp2p.go, line 363 at r1 (raw file):

Previously, acud (acud) wrote…

isn't the prefix here supposed to be stream handler?

Done.

---

pkg/p2p/libp2p/libp2p.go, line 368 at r1 (raw file):

Previously, acud (acud) wrote…

i would add the same prefix to these messages here too and double check all log lines printed in this function have it too

Done.

---

pkg/p2p/libp2p/libp2p.go, line 602 at r1 (raw file):

Previously, acud (acud) wrote…

consider: adding information about which protocol failed. the error is wrapped but the protocol in the message doesn't really say anything. more information will be availble from tn

Done.

---

pkg/p2p/libp2p/libp2p.go, line 609 at r1 (raw file):

s.logger.Debugf("libp2p Connect: peer %s does not exist

---

pkg/p2p/libp2p/libp2p.go, line 609 at r1 (raw file):

Previously, acud (acud) wrote…

it is not necessary to log and return an error. you can decorate the returned error with more information when necessary and the calling context will log is as necessary

Done.

---

pkg/p2p/libp2p/libp2p.go, line 650 at r1 (raw file):

Previously, anatollupacescu (Anatolie Lupacescu) wrote…

should this be in singular (found)?

Done.

[istae]

Reviewable status: 1 of 2 files reviewed, 7 unresolved discussions (waiting on @acud, @aloknerurkar, @anatollupacescu, @janos, and @mrekucci)

---

pkg/p2p/libp2p/libp2p.go, line 267 at r1 (raw file):

Previously, acud (acud) wrote…

i think this can be removed. notifier is never nil while the node is running

Done.

[acud]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aloknerurkar, @anatollupacescu, @esadakar, and @janos)