feat: p2p disconnect with reason

[aloknerurkar]

This change is 

[acud]

@aloknerurkar could we also have the signature change on the Blocklist method on the Disconnecter interface? (in the p2p.go file)

[aloknerurkar]

@acud The sonar cloud check is complaining about code duplication in the accounting_test.go file. These don't seem as straightforward. Not sure if we should address this.

[acud]

nice! i have a few comments

Reviewed 7 of 11 files at r1, 9 of 9 files at r3, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @aloknerurkar and @metacertain)

---

pkg/debugapi/peer.go, line 39 at r3 (raw file):

	if err := s.topologyDriver.Connected(r.Context(), p2p.Peer{Address: bzzAddr.Overlay}, true); err != nil {
		_ = s.p2p.Disconnect(bzzAddr.Overlay, "failed to process connection notification")

failed to notify topology?

---

pkg/p2p/libp2p/libp2p.go, line 353 at r3 (raw file):

		s.logger.Debugf("stream handler: could not close stream %s: %v", overlay, err)
		s.logger.Errorf("stream handler: unable to handshake with peer %v", overlay)
		_ = s.Disconnect(overlay, "unable to handshake")

here maybe it would be good to disambiguate this error message from the line above, so maybe could not fully close stream on handshake

---

pkg/p2p/libp2p/libp2p.go, line 516 at r3 (raw file):

						logger.Errorf("unable to blocklist peer %v", peerID)
					}
					logger.Tracef("handler(%s): blocklisted %s reason %s", p.Name, overlay.String(), bpe.Error())

wherever we have "reason" maybe it would be nice to add a colon like, reason: %v

---

pkg/p2p/libp2p/libp2p.go, line 559 at r3 (raw file):

func (s *Service) Blocklist(overlay swarm.Address, duration time.Duration, reason string) error {
	s.logger.Tracef("libp2p blocklist: peer %s for %v reason %s", overlay.String(), duration, reason)

here too

---

pkg/p2p/libp2p/libp2p.go, line 562 at r3 (raw file):

	if err := s.blocklist.Add(overlay, duration); err != nil {
		s.metrics.BlocklistedPeerErrCount.Inc()
		_ = s.Disconnect(overlay, "blocklisting peer")

here it would be good to disambiguate the reason from the one below

---

pkg/p2p/libp2p/libp2p.go, line 655 at r3 (raw file):

	if exists := s.peers.addIfNotExists(stream.Conn(), overlay, i.FullNode); exists {
		if err := handshakeStream.FullClose(); err != nil {
			_ = s.Disconnect(overlay, "failed closing handshake")

here too

---

pkg/p2p/libp2p/libp2p.go, line 705 at r3 (raw file):

	s.metrics.DisconnectCount.Inc()

	s.logger.Debugf("libp2p disconnect: disconnecting peer %s reason %s", overlay, reason)

here also, would be good to have a colon

[aloknerurkar]

Dismissed @acud from 7 discussions.
Reviewable status: 11 of 13 files reviewed, all discussions resolved (waiting on @acud and @metacertain)

---

pkg/debugapi/peer.go, line 39 at r3 (raw file):

Previously, acud (acud) wrote…

failed to notify topology?

Done.

---

pkg/p2p/libp2p/libp2p.go, line 353 at r3 (raw file):

Previously, acud (acud) wrote…

here maybe it would be good to disambiguate this error message from the line above, so maybe could not fully close stream on handshake

Done.

---

pkg/p2p/libp2p/libp2p.go, line 516 at r3 (raw file):

Previously, acud (acud) wrote…

wherever we have "reason" maybe it would be nice to add a colon like, reason: %v

This is actually duplicate. I added this before we added the reason to the function call. Now we log it inside the Blocklist function call.

---

pkg/p2p/libp2p/libp2p.go, line 559 at r3 (raw file):

Previously, acud (acud) wrote…

here too

Done.

---

pkg/p2p/libp2p/libp2p.go, line 562 at r3 (raw file):

Previously, acud (acud) wrote…

here it would be good to disambiguate the reason from the one below

Done.

---

pkg/p2p/libp2p/libp2p.go, line 655 at r3 (raw file):

Previously, acud (acud) wrote…

here too

Done.

---

pkg/p2p/libp2p/libp2p.go, line 705 at r3 (raw file):

Previously, acud (acud) wrote…

here also, would be good to have a colon

Done.

[acud]

thank you 🙏

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @metacertain)

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[metacertain]

Great!