Peer status peer scorer #7480
Peer status peer scorer #7480
Conversation
…o p2p-peer-scoring
Codecov Report
@@ Coverage Diff @@
## master #7480 +/- ##
==========================================
- Coverage 61.99% 61.96% -0.04%
==========================================
Files 424 423 -1
Lines 29798 30081 +283
==========================================
+ Hits 18473 18639 +166
- Misses 8424 8507 +83
- Partials 2901 2935 +34 |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
farazdagi
added
Ready For Review
| @@ -58,6 +58,9 @@ func (s *BadResponsesScorer) Score(pid peer.ID) float64 { | |||
|
|
|||
| // score is a lock-free version of Score. | |||
| func (s *BadResponsesScorer) score(pid peer.ID) float64 { | |||
| if s.isBadPeer(pid) { | |||
| return BadPeerScore | |||
There was a problem hiding this comment.
in that case, you only return -1 . Isn't that a very low negative score for a 'bad' peer ? Shouldn't it be more in absolute terms
There was a problem hiding this comment.
All the scores will be within [0..1] range (signed). Then you can change scorer's weight (in aggregate service we have weights), and turn that into anything you want (for example bad responses scorer can have a magnifying coefficient x10, and turn that into -10).
| return false | ||
| } | ||
| // Mark peer as bad, if the latest error is one of the terminal ones. | ||
| terminalErrs := []error{ |
There was a problem hiding this comment.
maybe store this out of the function and in the general file. Rather than initializing it in the function
There was a problem hiding this comment.
Can I address this in the next PR (I will extend that field to include other errors there anyway)?
beacon-chain/sync/rpc_status.go
Outdated
| s.p2p.Peers().Scorers().PeerStatusScorer().SetPeerStatus(id, msg, err) | ||
| if s.p2p.Peers().IsBad(id) { | ||
| s.disconnectBadPeer(s.ctx, id) | ||
| return s.p2p.Peers().Scorers().ValidationError(id) |
There was a problem hiding this comment.
no need to return validation error here, we can just allow the function to exit with the error below
What type of PR is this?
What does this PR do? Why is it needed?
PeerStatusScorer(scores peer on chain status - fork digest and head slot). This scorer can be used as base for more p2p scorers.ErrWrongForkDigestVersionpeer is marked as bad)Which issues(s) does this PR fix?
Part of #6622
Other notes for review