eth: simplify peer protection — compute protected set upfront

Compute the protected peer set once in dropRandomPeer via
protectedPeers(), then include protection as a condition in
selectDoNotDrop alongside trusted/static/recent checks. This
eliminates the separate filterProtectedPeers post-pass and the
awkward "all protected → skip" branch.

Rename filterProtectedPeers to protectedPeers, returning
map[*p2p.Peer]bool instead of filtering a slice. The map is
checked directly in selectDoNotDrop via protected[p].
This commit is contained in:
Csaba Kiraly 2026-04-10 11:50:11 +02:00
parent db611822db
commit 1c518be79f
2 changed files with 53 additions and 88 deletions

View file

@ -159,27 +159,23 @@ func (cm *dropper) dropRandomPeer() bool {
} }
numDialed := len(peers) - numInbound numDialed := len(peers) - numInbound
// Compute the set of inclusion-protected peers before filtering.
protected := cm.protectedPeers(peers)
selectDoNotDrop := func(p *p2p.Peer) bool { selectDoNotDrop := func(p *p2p.Peer) bool {
// Avoid dropping trusted and static peers, or recent peers.
// Only drop peers if their respective category (dialed/inbound)
// is close to limit capacity.
return p.Trusted() || p.StaticDialed() || return p.Trusted() || p.StaticDialed() ||
p.Lifetime() < mclock.AbsTime(doNotDropBefore) || p.Lifetime() < mclock.AbsTime(doNotDropBefore) ||
(p.DynDialed() && cm.maxDialPeers-numDialed > peerDropThreshold) || (p.DynDialed() && cm.maxDialPeers-numDialed > peerDropThreshold) ||
(p.Inbound() && cm.maxInboundPeers-numInbound > peerDropThreshold) (p.Inbound() && cm.maxInboundPeers-numInbound > peerDropThreshold) ||
protected[p]
} }
droppable := slices.DeleteFunc(peers, selectDoNotDrop) droppable := slices.DeleteFunc(peers, selectDoNotDrop)
if len(droppable) == 0 { if len(droppable) == 0 {
return false if len(protected) > 0 {
}
// Protect peers with the highest inclusion stats.
if cm.peerStatsFunc != nil {
droppable = cm.filterProtectedPeers(droppable)
if len(droppable) == 0 {
dropSkipped.Mark(1) dropSkipped.Mark(1)
return false
} }
return false
} }
p := droppable[mrand.Intn(len(droppable))] p := droppable[mrand.Intn(len(droppable))]
log.Debug("Dropping random peer", "inbound", p.Inbound(), log.Debug("Dropping random peer", "inbound", p.Inbound(),
@ -193,34 +189,35 @@ func (cm *dropper) dropRandomPeer() bool {
return true return true
} }
// filterProtectedPeers removes peers from the droppable list that are // protectedPeers computes the set of peers that should not be dropped based
// protected by any of the protection categories. Each category independently // on inclusion stats. Each protection category independently selects its
// selects the top-N peers per inbound/dialed pool by score; the union of all // top-N peers per inbound/dialed pool; the union is returned.
// selections is protected. func (cm *dropper) protectedPeers(peers []*p2p.Peer) map[*p2p.Peer]bool {
func (cm *dropper) filterProtectedPeers(droppable []*p2p.Peer) []*p2p.Peer { if cm.peerStatsFunc == nil {
return nil
}
stats := cm.peerStatsFunc() stats := cm.peerStatsFunc()
if len(stats) == 0 { if len(stats) == 0 {
return droppable return nil
} }
type peerWithStats struct { type peerWithStats struct {
peer *p2p.Peer peer *p2p.Peer
s PeerInclusionStats s PeerInclusionStats
} }
var inbound, dialed []peerWithStats var inbound, dialed []peerWithStats
for _, p := range droppable { for _, p := range peers {
id := p.ID().String() entry := peerWithStats{p, stats[p.ID().String()]}
entry := peerWithStats{p, stats[id]}
if p.Inbound() { if p.Inbound() {
inbound = append(inbound, entry) inbound = append(inbound, entry)
} else { } else {
dialed = append(dialed, entry) dialed = append(dialed, entry)
} }
} }
protectedSet := make(map[*p2p.Peer]struct{}) result := make(map[*p2p.Peer]bool)
protectTopN := func(entries []peerWithStats, cat protectionCategory) { protectTopN := func(entries []peerWithStats, cat protectionCategory) {
n := int(float64(len(entries)) * cat.frac) n := int(float64(len(entries)) * cat.frac)
if n == 0 || len(entries) == 0 { if n == 0 {
return return
} }
sort.Slice(entries, func(i, j int) bool { sort.Slice(entries, func(i, j int) bool {
@ -228,7 +225,7 @@ func (cm *dropper) filterProtectedPeers(droppable []*p2p.Peer) []*p2p.Peer {
}) })
for i := 0; i < n && i < len(entries); i++ { for i := 0; i < n && i < len(entries); i++ {
if cat.score(entries[i].s) > 0 { if cat.score(entries[i].s) > 0 {
protectedSet[entries[i].peer] = struct{}{} result[entries[i].peer] = true
} }
} }
} }
@ -237,21 +234,11 @@ func (cm *dropper) filterProtectedPeers(droppable []*p2p.Peer) []*p2p.Peer {
copy(inCopy, inbound) copy(inCopy, inbound)
dialCopy := make([]peerWithStats, len(dialed)) dialCopy := make([]peerWithStats, len(dialed))
copy(dialCopy, dialed) copy(dialCopy, dialed)
protectTopN(inCopy, cat) protectTopN(inCopy, cat)
protectTopN(dialCopy, cat) protectTopN(dialCopy, cat)
} }
if len(protectedSet) == 0 { if len(result) > 0 {
return droppable log.Debug("Protecting high-value peers from drop", "protected", len(result))
}
log.Debug("Protecting high-value peers from drop",
"protected", len(protectedSet), "droppable", len(droppable))
result := make([]*p2p.Peer, 0, len(droppable))
for _, p := range droppable {
if _, ok := protectedSet[p]; !ok {
result = append(result, p)
}
} }
return result return result
} }

View file

@ -33,114 +33,92 @@ func makePeers(n int) []*p2p.Peer {
return peers return peers
} }
func TestFilterProtectedNoStats(t *testing.T) { func TestProtectedPeersNoStats(t *testing.T) {
// When the stats func returns nil/empty, all peers remain droppable.
cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30} cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30}
cm.peerStatsFunc = func() map[string]PeerInclusionStats { return nil } cm.peerStatsFunc = func() map[string]PeerInclusionStats { return nil }
peers := makePeers(10) peers := makePeers(10)
result := cm.filterProtectedPeers(peers) protected := cm.protectedPeers(peers)
if len(result) != len(peers) { if len(protected) != 0 {
t.Fatalf("expected all peers droppable with nil stats, got %d/%d", len(result), len(peers)) t.Fatalf("expected no protected peers with nil stats, got %d", len(protected))
} }
} }
func TestFilterProtectedEmptyStats(t *testing.T) { func TestProtectedPeersEmptyStats(t *testing.T) {
cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30} cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30}
cm.peerStatsFunc = func() map[string]PeerInclusionStats { cm.peerStatsFunc = func() map[string]PeerInclusionStats {
return map[string]PeerInclusionStats{} return map[string]PeerInclusionStats{}
} }
peers := makePeers(10) peers := makePeers(10)
result := cm.filterProtectedPeers(peers) protected := cm.protectedPeers(peers)
if len(result) != len(peers) { if len(protected) != 0 {
t.Fatalf("expected all peers droppable with empty stats, got %d/%d", len(result), len(peers)) t.Fatalf("expected no protected peers with empty stats, got %d", len(protected))
} }
} }
func TestFilterProtectedTopPeer(t *testing.T) { func TestProtectedPeersTopPeer(t *testing.T) {
// 20 peers, 10% of 20 = 2 protected per category. // 20 peers, 10% of 20 = 2 protected per category.
// NewPeer creates non-inbound peers, so all go to dialed bucket.
cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30} cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30}
peers := makePeers(20) peers := makePeers(20)
stats := make(map[string]PeerInclusionStats) stats := make(map[string]PeerInclusionStats)
// Peer 0: top by Finalized
stats[peers[0].ID().String()] = PeerInclusionStats{Finalized: 100} stats[peers[0].ID().String()] = PeerInclusionStats{Finalized: 100}
// Peer 1: top by RecentIncluded
stats[peers[1].ID().String()] = PeerInclusionStats{RecentIncluded: 5.0} stats[peers[1].ID().String()] = PeerInclusionStats{RecentIncluded: 5.0}
cm.peerStatsFunc = func() map[string]PeerInclusionStats { return stats } cm.peerStatsFunc = func() map[string]PeerInclusionStats { return stats }
result := cm.filterProtectedPeers(peers) protected := cm.protectedPeers(peers)
// 2 categories × 2 protected each = up to 4, but peers 0 and 1 are if len(protected) != 2 {
// different so both should be removed. Other peers have zero scores. t.Fatalf("expected 2 protected peers, got %d", len(protected))
protected := len(peers) - len(result)
if protected != 2 {
t.Fatalf("expected 2 protected peers, got %d", protected)
} }
// Verify peers 0 and 1 are not in result. if !protected[peers[0]] {
for _, p := range result { t.Fatal("peer 0 should be protected (top Finalized)")
id := p.ID().String() }
if id == peers[0].ID().String() || id == peers[1].ID().String() { if !protected[peers[1]] {
t.Fatalf("peer %s should be protected", id) t.Fatal("peer 1 should be protected (top RecentIncluded)")
}
} }
} }
func TestFilterProtectedZeroScore(t *testing.T) { func TestProtectedPeersZeroScore(t *testing.T) {
cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30} cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30}
peers := makePeers(10) peers := makePeers(10)
stats := make(map[string]PeerInclusionStats) stats := make(map[string]PeerInclusionStats)
// All peers have zero stats.
for _, p := range peers { for _, p := range peers {
stats[p.ID().String()] = PeerInclusionStats{} stats[p.ID().String()] = PeerInclusionStats{}
} }
cm.peerStatsFunc = func() map[string]PeerInclusionStats { return stats } cm.peerStatsFunc = func() map[string]PeerInclusionStats { return stats }
result := cm.filterProtectedPeers(peers) protected := cm.protectedPeers(peers)
if len(result) != len(peers) { if len(protected) != 0 {
t.Fatalf("expected no protection with zero scores, got %d protected", len(peers)-len(result)) t.Fatalf("expected no protection with zero scores, got %d", len(protected))
} }
} }
func TestFilterProtectedOverlap(t *testing.T) { func TestProtectedPeersOverlap(t *testing.T) {
// One peer is top in both categories — should only be removed once. // One peer is top in both categories — counted once.
cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30} cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30}
peers := makePeers(20) peers := makePeers(20)
stats := make(map[string]PeerInclusionStats) stats := make(map[string]PeerInclusionStats)
// Peer 0 is top in both.
stats[peers[0].ID().String()] = PeerInclusionStats{Finalized: 100, RecentIncluded: 5.0} stats[peers[0].ID().String()] = PeerInclusionStats{Finalized: 100, RecentIncluded: 5.0}
cm.peerStatsFunc = func() map[string]PeerInclusionStats { return stats } cm.peerStatsFunc = func() map[string]PeerInclusionStats { return stats }
result := cm.filterProtectedPeers(peers) protected := cm.protectedPeers(peers)
protected := len(peers) - len(result) if len(protected) != 1 {
if protected != 1 { t.Fatalf("expected 1 protected peer (overlap), got %d", len(protected))
t.Fatalf("expected 1 protected peer (overlap), got %d", protected)
} }
} }
func TestFilterProtectedAllProtected(t *testing.T) { func TestProtectedPeersNilFunc(t *testing.T) {
// 10 peers: 10% = 1 per category. Give all 10 peers high scores in
// one of the two categories so the union covers everyone.
cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30} cm := &dropper{maxDialPeers: 20, maxInboundPeers: 30}
// peerStatsFunc is nil (default).
peers := makePeers(10) peers := makePeers(10)
stats := make(map[string]PeerInclusionStats) protected := cm.protectedPeers(peers)
// Peer 0 has the highest Finalized → protected by total-finalized. if protected != nil {
stats[peers[0].ID().String()] = PeerInclusionStats{Finalized: 100} t.Fatalf("expected nil with nil stats func, got %v", protected)
// Peer 1 has the highest RecentIncluded → protected by recent-included.
stats[peers[1].ID().String()] = PeerInclusionStats{RecentIncluded: 5.0}
cm.peerStatsFunc = func() map[string]PeerInclusionStats { return stats }
result := cm.filterProtectedPeers(peers)
// 10% of 10 = 1 per category, 2 categories = 2 protected.
// 10 - 2 = 8 droppable.
if len(result) != 8 {
t.Fatalf("expected 8 droppable peers, got %d", len(result))
} }
} }