Page MenuHomePhabricator

Protect onion+localhost peers in ProtectEvictionCandidatesByRatio()
ClosedPublic

Authored by PiRK on Feb 3 2022, 15:22.

Details

Summary

https://github.com/bitcoin/bitcoin/pull/20197/commits/8b1e156143740a5548dc7b601d40fb141e6aae1c

Add m_inbound_onion to AttemptToEvictConnection() and an m_is_onion struct member to NodeEvictionCandidate and tests.

We'll use these in the peer eviction logic to protect inbound onion peers
in addition to the existing protection of localhost peers.

https://github.com/bitcoin/bitcoin/pull/20197/commits/caa21f586f951d626a67f391050c3644f1057f57

Now that we have a reliable way to detect inbound onion peers, this commit
updates our existing eviction protection of 1/4 localhost peers to instead
protect up to 1/4 onion peers (connected via our tor control service), sorted by
longest uptime. Any remaining slots of the 1/4 are then allocated to protect
localhost peers, or 2 localhost peers if no slots remain and 2 or more onion
peers are protected, sorted by longest uptime.

The goal is to avoid penalizing onion peers, due to their higher min ping times
relative to IPv4 and IPv6 peers, and improve our diversity of peer connections.

Thank you to Gregory Maxwell, Suhas Daftuar, Vasil Dimov and Pieter Wuille
for valuable review feedback that shaped the direction.

https://github.com/bitcoin/bitcoin/pull/20197/commits/0cca08a8ee33b4e05ff586ae4fd914f5ea860cea

Add unit test coverage for our onion peer eviction protectioni

Note: the commits are squashed to have the feature and corresponding test at the same time

This concludes backport of core#20197

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Feb 3 2022, 15:22
PiRK retitled this revision from Add m_inbound_onion to AttemptToEvictConnection() to Protect onion+localhost peers in ProtectEvictionCandidatesByRatio() .Feb 3 2022, 15:26
PiRK edited the summary of this revision. (Show Details)
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/net.cpp
998 ↗(On Diff #32175)

Any reason you changed the text ?

This revision is now accepted and ready to land.Feb 4 2022, 09:59
src/net.cpp
998 ↗(On Diff #32175)

This was intentional. The original word "favorise" seemed very french.