Skip to content

Commit 8b6cd42

Browse files
committed
Merge bitcoin/bitcoin#24165: p2p: extend inbound eviction protection by network to CJDNS peers
b7be28c test: add combined CJDNS/I2P/localhost/onion eviction protection tests (Jon Atack) 0a1bb84 test: add tests for inbound eviction protection of CJDNS peers (Jon Atack) 0c00c0c test: fix off-by-one logic in an eviction protection test (Jon Atack) f7b8094 p2p: extend inbound eviction protection by network to CJDNS peers (Jon Atack) Pull request description: Extend inbound eviction protection for peers connected over CJDNS, as is the case for peers connected via onion, localhost, and I2P since #21261 and #20197. CJDNS peers seem to have better min ping latency than onion and I2P peers but still higher than that of unencrypted IPv4/6 peers and can be disadvantaged under our eviction criteria. They are also very few in number, which is a further reason to protect them, as the goal of this logic is to favorise the diversity of our peer connections. CJDNS support was added in #23077 for the upcoming v23 release. ACKs for top commit: laanwj: Concept and code review ACK b7be28c w0xlt: tACK b7be28c Tree-SHA512: 89ebdd217602e16ae14b9bd0d5a25fc09f9b2384c951f820bc0f5a6d8452bbc9042065db817d5d5296c0ad22988491a83fc5b9a611e660c40ebd4f03448c4061
2 parents 267917f + b7be28c commit 8b6cd42

File tree

3 files changed

+156
-21
lines changed

3 files changed

+156
-21
lines changed

src/net.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -928,17 +928,17 @@ void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& evicti
928928
// Protect the half of the remaining nodes which have been connected the longest.
929929
// This replicates the non-eviction implicit behavior, and precludes attacks that start later.
930930
// To favorise the diversity of our peer connections, reserve up to half of these protected
931-
// spots for Tor/onion, localhost and I2P peers, even if they're not longest uptime overall.
932-
// This helps protect these higher-latency peers that tend to be otherwise
931+
// spots for Tor/onion, localhost, I2P, and CJDNS peers, even if they're not longest uptime
932+
// overall. This helps protect these higher-latency peers that tend to be otherwise
933933
// disadvantaged under our eviction criteria.
934934
const size_t initial_size = eviction_candidates.size();
935935
const size_t total_protect_size{initial_size / 2};
936936

937-
// Disadvantaged networks to protect: I2P, localhost, Tor/onion. In case of equal counts, earlier
938-
// array members have first opportunity to recover unused slots from the previous iteration.
937+
// Disadvantaged networks to protect. In the case of equal counts, earlier array members
938+
// have the first opportunity to recover unused slots from the previous iteration.
939939
struct Net { bool is_local; Network id; size_t count; };
940-
std::array<Net, 3> networks{
941-
{{false, NET_I2P, 0}, {/* localhost */ true, NET_MAX, 0}, {false, NET_ONION, 0}}};
940+
std::array<Net, 4> networks{
941+
{{false, NET_CJDNS, 0}, {false, NET_I2P, 0}, {/*localhost=*/true, NET_MAX, 0}, {false, NET_ONION, 0}}};
942942

943943
// Count and store the number of eviction candidates per network.
944944
for (Net& n : networks) {

src/net.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,6 +1316,8 @@ struct NodeEvictionCandidate
13161316
*
13171317
* - I2P peers
13181318
*
1319+
* - CJDNS peers
1320+
*
13191321
* This helps protect these privacy network peers, which tend to be otherwise
13201322
* disadvantaged under our eviction criteria for their higher min ping times
13211323
* relative to IPv4/IPv6 peers, and favorise the diversity of peer connections.

src/test/net_peer_eviction_tests.cpp

Lines changed: 148 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
9090
// Test protection of onion, localhost, and I2P peers...
9191

9292
// Expect 1/4 onion peers to be protected from eviction,
93-
// if no localhost or I2P peers.
93+
// if no localhost, I2P, or CJDNS peers.
9494
BOOST_CHECK(IsProtected(
9595
num_peers, [](NodeEvictionCandidate& c) {
9696
c.m_is_local = false;
@@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
101101
random_context));
102102

103103
// Expect 1/4 onion peers and 1/4 of the other peers to be protected,
104-
// sorted by longest uptime (lowest m_connected), if no localhost or I2P peers.
104+
// sorted by longest uptime (lowest m_connected), if no localhost, I2P or CJDNS peers.
105105
BOOST_CHECK(IsProtected(
106106
num_peers, [](NodeEvictionCandidate& c) {
107107
c.m_connected = std::chrono::seconds{c.id};
@@ -113,7 +113,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
113113
random_context));
114114

115115
// Expect 1/4 localhost peers to be protected from eviction,
116-
// if no onion or I2P peers.
116+
// if no onion, I2P, or CJDNS peers.
117117
BOOST_CHECK(IsProtected(
118118
num_peers, [](NodeEvictionCandidate& c) {
119119
c.m_is_local = (c.id == 1 || c.id == 9 || c.id == 11);
@@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
124124
random_context));
125125

126126
// Expect 1/4 localhost peers and 1/4 of the other peers to be protected,
127-
// sorted by longest uptime (lowest m_connected), if no onion or I2P peers.
127+
// sorted by longest uptime (lowest m_connected), if no onion, I2P, or CJDNS peers.
128128
BOOST_CHECK(IsProtected(
129129
num_peers, [](NodeEvictionCandidate& c) {
130130
c.m_connected = std::chrono::seconds{c.id};
@@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
136136
random_context));
137137

138138
// Expect 1/4 I2P peers to be protected from eviction,
139-
// if no onion or localhost peers.
139+
// if no onion, localhost, or CJDNS peers.
140140
BOOST_CHECK(IsProtected(
141141
num_peers, [](NodeEvictionCandidate& c) {
142142
c.m_is_local = false;
@@ -146,8 +146,8 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
146146
/*unprotected_peer_ids=*/{},
147147
random_context));
148148

149-
// Expect 1/4 I2P peers and 1/4 of the other peers to be protected,
150-
// sorted by longest uptime (lowest m_connected), if no onion or localhost peers.
149+
// Expect 1/4 I2P peers and 1/4 of the other peers to be protected, sorted
150+
// by longest uptime (lowest m_connected), if no onion, localhost, or CJDNS peers.
151151
BOOST_CHECK(IsProtected(
152152
num_peers, [](NodeEvictionCandidate& c) {
153153
c.m_connected = std::chrono::seconds{c.id};
@@ -158,6 +158,29 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
158158
/*unprotected_peer_ids=*/{3, 5, 6, 7, 8, 11},
159159
random_context));
160160

161+
// Expect 1/4 CJDNS peers to be protected from eviction,
162+
// if no onion, localhost, or I2P peers.
163+
BOOST_CHECK(IsProtected(
164+
num_peers, [](NodeEvictionCandidate& c) {
165+
c.m_is_local = false;
166+
c.m_network = (c.id == 2 || c.id == 7 || c.id == 10) ? NET_CJDNS : NET_IPV4;
167+
},
168+
/*protected_peer_ids=*/{2, 7, 10},
169+
/*unprotected_peer_ids=*/{},
170+
random_context));
171+
172+
// Expect 1/4 CJDNS peers and 1/4 of the other peers to be protected, sorted
173+
// by longest uptime (lowest m_connected), if no onion, localhost, or I2P peers.
174+
BOOST_CHECK(IsProtected(
175+
num_peers, [](NodeEvictionCandidate& c) {
176+
c.m_connected = std::chrono::seconds{c.id};
177+
c.m_is_local = false;
178+
c.m_network = (c.id == 4 || c.id > 8) ? NET_CJDNS : NET_IPV6;
179+
},
180+
/*protected_peer_ids=*/{0, 1, 2, 4, 9, 10},
181+
/*unprotected_peer_ids=*/{3, 5, 6, 7, 8, 11},
182+
random_context));
183+
161184
// Tests with 2 networks...
162185

163186
// Combined test: expect having 1 localhost and 1 onion peer out of 4 to
@@ -289,16 +312,16 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
289312
BOOST_CHECK(IsProtected(
290313
4, [](NodeEvictionCandidate& c) {
291314
c.m_connected = std::chrono::seconds{c.id};
292-
c.m_is_local = (c.id == 3);
293-
if (c.id == 4) {
315+
c.m_is_local = (c.id == 2);
316+
if (c.id == 3) {
294317
c.m_network = NET_I2P;
295-
} else if (c.id == 2) {
318+
} else if (c.id == 1) {
296319
c.m_network = NET_ONION;
297320
} else {
298321
c.m_network = NET_IPV6;
299322
}
300323
},
301-
/*protected_peer_ids=*/{0, 4},
324+
/*protected_peer_ids=*/{0, 3},
302325
/*unprotected_peer_ids=*/{1, 2},
303326
random_context));
304327

@@ -416,15 +439,15 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
416439
/*unprotected_peer_ids=*/{6, 7, 8, 9, 10, 11, 16, 19, 20, 21, 22, 23},
417440
random_context));
418441

419-
// Combined test: expect having 8 localhost, 4 I2P, and 3 onion peers out of
420-
// 24 to protect 2 of each (6 total), plus 6 others for 12/24 total, sorted
421-
// by longest uptime.
442+
// Combined test: expect having 8 localhost, 4 CJDNS, and 3 onion peers out
443+
// of 24 to protect 2 of each (6 total), plus 6 others for 12/24 total,
444+
// sorted by longest uptime.
422445
BOOST_CHECK(IsProtected(
423446
24, [](NodeEvictionCandidate& c) {
424447
c.m_connected = std::chrono::seconds{c.id};
425448
c.m_is_local = (c.id > 15);
426449
if (c.id > 10 && c.id < 15) {
427-
c.m_network = NET_I2P;
450+
c.m_network = NET_CJDNS;
428451
} else if (c.id > 6 && c.id < 10) {
429452
c.m_network = NET_ONION;
430453
} else {
@@ -434,6 +457,116 @@ BOOST_AUTO_TEST_CASE(peer_protection_test)
434457
/*protected_peer_ids=*/{0, 1, 2, 3, 4, 5, 7, 8, 11, 12, 16, 17},
435458
/*unprotected_peer_ids=*/{6, 9, 10, 13, 14, 15, 18, 19, 20, 21, 22, 23},
436459
random_context));
460+
461+
// Tests with 4 networks...
462+
463+
// Combined test: expect having 1 CJDNS, 1 I2P, 1 localhost and 1 onion peer
464+
// out of 5 to protect 1 CJDNS, 0 I2P, 0 localhost, 0 onion and 1 other peer
465+
// (2 total), sorted by longest uptime; stable sort breaks tie with array
466+
// order of CJDNS first.
467+
BOOST_CHECK(IsProtected(
468+
5, [](NodeEvictionCandidate& c) {
469+
c.m_connected = std::chrono::seconds{c.id};
470+
c.m_is_local = (c.id == 3);
471+
if (c.id == 4) {
472+
c.m_network = NET_CJDNS;
473+
} else if (c.id == 1) {
474+
c.m_network = NET_I2P;
475+
} else if (c.id == 2) {
476+
c.m_network = NET_ONION;
477+
} else {
478+
c.m_network = NET_IPV6;
479+
}
480+
},
481+
/* protected_peer_ids */ {0, 4},
482+
/* unprotected_peer_ids */ {1, 2, 3},
483+
random_context));
484+
485+
// Combined test: expect having 1 CJDNS, 1 I2P, 1 localhost and 1 onion peer
486+
// out of 7 to protect 1 CJDNS, 0, I2P, 0 localhost, 0 onion and 2 other
487+
// peers (3 total) sorted by longest uptime; stable sort breaks tie with
488+
// array order of CJDNS first.
489+
BOOST_CHECK(IsProtected(
490+
7, [](NodeEvictionCandidate& c) {
491+
c.m_connected = std::chrono::seconds{c.id};
492+
c.m_is_local = (c.id == 4);
493+
if (c.id == 6) {
494+
c.m_network = NET_CJDNS;
495+
} else if (c.id == 5) {
496+
c.m_network = NET_I2P;
497+
} else if (c.id == 3) {
498+
c.m_network = NET_ONION;
499+
} else {
500+
c.m_network = NET_IPV4;
501+
}
502+
},
503+
/*protected_peer_ids=*/{0, 1, 6},
504+
/*unprotected_peer_ids=*/{2, 3, 4, 5},
505+
random_context));
506+
507+
// Combined test: expect having 1 CJDNS, 1 I2P, 1 localhost and 1 onion peer
508+
// out of 8 to protect 1 CJDNS, 1 I2P, 0 localhost, 0 onion and 2 other
509+
// peers (4 total) sorted by longest uptime; stable sort breaks tie with
510+
// array order of CJDNS first.
511+
BOOST_CHECK(IsProtected(
512+
8, [](NodeEvictionCandidate& c) {
513+
c.m_connected = std::chrono::seconds{c.id};
514+
c.m_is_local = (c.id == 3);
515+
if (c.id == 5) {
516+
c.m_network = NET_CJDNS;
517+
} else if (c.id == 6) {
518+
c.m_network = NET_I2P;
519+
} else if (c.id == 3) {
520+
c.m_network = NET_ONION;
521+
} else {
522+
c.m_network = NET_IPV6;
523+
}
524+
},
525+
/*protected_peer_ids=*/{0, 1, 5, 6},
526+
/*unprotected_peer_ids=*/{2, 3, 4, 7},
527+
random_context));
528+
529+
// Combined test: expect having 2 CJDNS, 2 I2P, 4 localhost, and 2 onion
530+
// peers out of 16 to protect 1 CJDNS, 1 I2P, 1 localhost, 1 onion (4/16
531+
// total), plus 4 others for 8 total, sorted by longest uptime.
532+
BOOST_CHECK(IsProtected(
533+
16, [](NodeEvictionCandidate& c) {
534+
c.m_connected = std::chrono::seconds{c.id};
535+
c.m_is_local = (c.id > 5);
536+
if (c.id == 11 || c.id == 15) {
537+
c.m_network = NET_CJDNS;
538+
} else if (c.id == 10 || c.id == 14) {
539+
c.m_network = NET_I2P;
540+
} else if (c.id == 8 || c.id == 9) {
541+
c.m_network = NET_ONION;
542+
} else {
543+
c.m_network = NET_IPV4;
544+
}
545+
},
546+
/*protected_peer_ids=*/{0, 1, 2, 3, 6, 8, 10, 11},
547+
/*unprotected_peer_ids=*/{4, 5, 7, 9, 12, 13, 14, 15},
548+
random_context));
549+
550+
// Combined test: expect having 6 CJDNS, 1 I2P, 1 localhost, and 4 onion
551+
// peers out of 24 to protect 2 CJDNS, 1 I2P, 1 localhost, and 2 onions (6
552+
// total), plus 6 others for 12/24 total, sorted by longest uptime.
553+
BOOST_CHECK(IsProtected(
554+
24, [](NodeEvictionCandidate& c) {
555+
c.m_connected = std::chrono::seconds{c.id};
556+
c.m_is_local = (c.id == 13);
557+
if (c.id > 17) {
558+
c.m_network = NET_CJDNS;
559+
} else if (c.id == 17) {
560+
c.m_network = NET_I2P;
561+
} else if (c.id == 12 || c.id == 14 || c.id == 15 || c.id == 16) {
562+
c.m_network = NET_ONION;
563+
} else {
564+
c.m_network = NET_IPV6;
565+
}
566+
},
567+
/*protected_peer_ids=*/{0, 1, 2, 3, 4, 5, 12, 13, 14, 17, 18, 19},
568+
/*unprotected_peer_ids=*/{6, 7, 8, 9, 10, 11, 15, 16, 20, 21, 22, 23},
569+
random_context));
437570
}
438571

439572
// Returns true if any of the node ids in node_ids are selected for eviction.

0 commit comments

Comments
 (0)