p2p: adaptive connections services flags
Changes PlannedPublic
Actions

Authored by PiRK on Aug 6 2025, 11:20.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Summary

By relocating the peer desirable services flags into the peer manager, we
allow the connections acceptance process to handle post-IBD potential
stalling scenarios.

The peer manager will be able to dynamically adjust the services flags
based on the node's proximity to the tip (back and forth). Allowing the node
to recover from the following post-IBD scenario:
Suppose the node has successfully synced the chain, but later experienced
dropped connections and remained inactive for a duration longer than the limited
peers threshold (the timeframe within which limited peers can provide blocks). In
such cases, upon reconnecting to the network, the node might only establish
connections with limited peers, filling up all available outbound slots. Resulting
in an inability to synchronize the chain (because limited peers will not provide
blocks older than the NODE_NETWORK_LIMITED_MIN_BLOCKS threshold).

This is a backport of core#28170

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 34058
Build 67585: Build Diff	lint-circular-dependencies · build-without-wallet · build-fuzzer · build-diff · build-debug · build-clang-tidy · build-clang
Build 67584: arc lint + arc unit

Event Timeline

PiRK created this revision.Aug 6 2025, 11:20

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 6 2025, 11:20

PiRK requested review of this revision.Aug 6 2025, 11:20

teamcity edited the summary of this revision. (Show Details)Aug 6 2025, 11:20

PiRK planned changes to this revision.Aug 6 2025, 11:21

remove unused makefile

PiRK added a child revision: D18452: test: p2p: check disconnect due to lack of desirable service flags.Aug 6 2025, 11:26

Harbormaster completed remote builds in B34053: Diff 55110.Aug 6 2025, 11:31

Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:


============================== 20 passed in 0.64s ==============================
[199/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/write_name_tests.cpp.o
[200/524] Automatic MOC for target test_bitcoin-qt
[201/524] Test Bitcoin utilities...
[202/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/db_tests.cpp.o
[203/524] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/daa_tests.cpp.o
[204/524] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/aserti32d_tests.cpp.o
[205/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/message_writer_tests.cpp.o
[206/524] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/eda_tests.cpp.o
[207/524] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/wallet/test/wallet_tests.cpp.o
[208/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/p2p_messaging_tests.cpp.o
[209/524] Linking CXX executable src/seeder/test/test-seeder
[210/524] seeder: testing db_tests
[211/524] Running utility command for check-seeder-db_tests
[212/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/util.cpp.o
[213/524] seeder: testing message_writer_tests
[214/524] Running utility command for check-seeder-message_writer_tests
[215/524] seeder: testing options_tests
[216/524] Running utility command for check-seeder-options_tests
[217/524] seeder: testing p2p_messaging_tests
[218/524] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/grasberg_tests.cpp.o
[219/524] Running utility command for check-seeder-p2p_messaging_tests
[220/524] seeder: testing parse_name_tests
[221/524] Running utility command for check-seeder-parse_name_tests
[222/524] seeder: testing write_name_tests
[223/524] Running utility command for check-seeder-write_name_tests
[224/524] Running seeder test suite
PASSED: seeder test suite
[225/524] Linking CXX executable src/pow/test/test-pow
[226/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/compattests.cpp.o
[227/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/bitcoinaddressvalidatortests.cpp.o
[228/524] pow: testing daa_tests
[229/524] Running utility command for check-pow-daa_tests
[230/524] pow: testing eda_tests
[231/524] Running utility command for check-pow-eda_tests
[232/524] pow: testing grasberg_tests
[233/524] Running utility command for check-pow-grasberg_tests
[234/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_bitcoin-qt_autogen/mocs_compilation.cpp.o
[235/524] pow: testing aserti32d_tests
[236/524] Running utility command for check-pow-aserti32d_tests
[237/524] Running pow test suite
PASSED: pow test suite
[238/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/guiutiltests.cpp.o
[239/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/uritests.cpp.o
[240/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/optiontests.cpp.o
[241/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o
[242/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/apptests.cpp.o
[243/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o
[244/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[245/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o
[246/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[247/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[248/524] Linking CXX executable src/qt/test/test_bitcoin-qt
[249/524] bitcoin-qt: testing test_bitcoin-qt
[250/524] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

fix clang-tidy

Harbormaster completed remote builds in B34054: Diff 55111.Aug 6 2025, 11:38

Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:


============================== 20 passed in 0.72s ==============================
[199/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/write_name_tests.cpp.o
[200/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/fixture.cpp.o
[201/524] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/aserti32d_tests.cpp.o
[202/524] Test Bitcoin utilities...
[203/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/db_tests.cpp.o
[204/524] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/daa_tests.cpp.o
[205/524] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/eda_tests.cpp.o
[206/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/message_writer_tests.cpp.o
[207/524] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/wallet/test/wallet_tests.cpp.o
[208/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/util.cpp.o
[209/524] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/grasberg_tests.cpp.o
[210/524] Linking CXX executable src/pow/test/test-pow
[211/524] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/p2p_messaging_tests.cpp.o
[212/524] pow: testing daa_tests
[213/524] Running utility command for check-pow-daa_tests
[214/524] pow: testing eda_tests
[215/524] Running utility command for check-pow-eda_tests
[216/524] pow: testing grasberg_tests
[217/524] Running utility command for check-pow-grasberg_tests
[218/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/bitcoinaddressvalidatortests.cpp.o
[219/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/compattests.cpp.o
[220/524] Linking CXX executable src/seeder/test/test-seeder
[221/524] seeder: testing db_tests
[222/524] seeder: testing message_writer_tests
[223/524] Running utility command for check-seeder-db_tests
[224/524] Running utility command for check-seeder-message_writer_tests
[225/524] seeder: testing options_tests
[226/524] seeder: testing p2p_messaging_tests
[227/524] Running utility command for check-seeder-options_tests
[228/524] Running utility command for check-seeder-p2p_messaging_tests
[229/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_bitcoin-qt_autogen/mocs_compilation.cpp.o
[230/524] seeder: testing parse_name_tests
[231/524] Running utility command for check-seeder-parse_name_tests
[232/524] seeder: testing write_name_tests
[233/524] Running utility command for check-seeder-write_name_tests
[234/524] Running seeder test suite
PASSED: seeder test suite
[235/524] pow: testing aserti32d_tests
[236/524] Running utility command for check-pow-aserti32d_tests
[237/524] Running pow test suite
PASSED: pow test suite
[238/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/uritests.cpp.o
[239/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/guiutiltests.cpp.o
[240/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/optiontests.cpp.o
[241/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/apptests.cpp.o
[242/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o
[243/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o
[244/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[245/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o
[246/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[247/524] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[248/524] Linking CXX executable src/qt/test/test_bitcoin-qt
[249/524] bitcoin-qt: testing test_bitcoin-qt
[250/524] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

Harbormaster completed remote builds in B34058: Diff 55115.Aug 6 2025, 12:04

This diff would benefit from being split, several refactors are happening that would be easy to review so we can focus on the behavior change

src/avalanche/processor.h
386	That is not correct. The avalanche processor is only interested in peers that have some flags set (avalanche flag, maybe others?)
src/net.cpp
1918	I don't think all_of is correct here. If it's useful for one then it's a good candidate, any_of is a better fit imo

This revision now requires changes to proceed.Aug 9 2025, 16:32

PiRK planned changes to this revision.Aug 26 2025, 15:54

PiRK added inline comments.Aug 27 2025, 09:57

src/net.cpp
1918	If i switch to any_of and I modify HasAllDesirableServiceFlags in processor.h to check for the NODE_AVALANCHE flag, it causes a bunch of failures in the eviction tests. So there is a change in behavior here if we want to properly check both the net_processing and the avalanche processor net_interface