search

celestiaorg / go-header

Go library with all the services needed to request, sync and store blockchain headers.
Apache License 2.0
17 stars 16 forks source link

refactoring(p2p/peerTracker): extend conditions for peers handling #165

Open vgonkivs opened 3 months ago

vgonkivs commented 3 months ago

Overview

This PR brings a bunch of improvements to the peer tracker:

  1. removed any limiting. After some time I realized that we don't have to limit our peer tracker by some min score. The current improvement allows us to get rid of gc at all. Peers will be removed only after disconnection.
  2. Moved peer score to the ConnManager. Libp2p provides us with a good mechanism for tracking peers' scores, so I don't see any reason why we shouldn't use it. Now, instead of keeping all peerStats inside the peer tracker, we will store only peers, and constructing peerStats will be on fly.
  3. Added additional events and conditions for tracked peers: a. EvtPeerIdentificationCompleted helps to ensure that peer is correct and supports all basic libp2p protocols+ during EvtPeerConnectednessChanged peer store hasn't got information about supported protocols of the incoming peer, so we can't check if the peer supports header-ex protocolID. b. EvtPeerProtocolsUpdated allows to detect when peer stop/start supporting header-ex protocol. c. Check if the incoming peer supports a header-ex protocol before storing it.
  4. Handled case when the session can be started with an empty peerQueue.
  5. During tests, I randomly caught {err: close called for the closed stream}. This error was received when we were trying to close the stream after the request had been finished(for quic transport). Stream.Close() does stream.CloseRead() + stream.CloseWrite() internally, so I decided to change Close() -> CloseRead(). PS. The issue has gone.

Checklist

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 67.92453% with 34 lines in your changes are missing coverage. Please review.

Project coverage is 62.11%. Comparing base (6d0f9a4) to head (4f48548). Report is 1 commits behind head on main.

Files Patch % Lines
p2p/peer_tracker.go 64.00% 19 Missing and 8 partials :warning:
p2p/exchange.go 62.50% 2 Missing and 1 partial :warning:
p2p/session.go 70.00% 2 Missing and 1 partial :warning:
p2p/helpers.go 0.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #165 +/- ## ========================================= + Coverage 0 62.11% +62.11% ========================================= Files 0 39 +39 Lines 0 3590 +3590 ========================================= + Hits 0 2230 +2230 - Misses 0 1182 +1182 - Partials 0 178 +178 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

vgonkivs commented 2 months ago

moving to draft until https://github.com/libp2p/go-libp2p/pull/2759 is integrated as it brings improvements to the IdentificationEvent that we rely on.