From e302e4eeed9841676608db87f3389c99ee64ab24 Mon Sep 17 00:00:00 2001 From: Bart Date: Tue, 15 Sep 2026 13:14:12 +0000 Subject: [PATCH] fix: Set the peer limit total when per-direction limits are configured (#8220) Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com> Co-authored-by: Vito Tumas <5780819+Tapanito@users.noreply.github.com> --- include/xrpl/peerfinder/Config.h | 3 +- src/libxrpl/peerfinder/Config.cpp | 8 +- src/test/overlay/overlay_limit_test.cpp | 83 +++++++++++++++++++++ src/tests/libxrpl/peerfinder/PeerFinder.cpp | 12 ++- 4 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 src/test/overlay/overlay_limit_test.cpp diff --git a/include/xrpl/peerfinder/Config.h b/include/xrpl/peerfinder/Config.h index 3326ae8a97..fa2d2d78e8 100644 --- a/include/xrpl/peerfinder/Config.h +++ b/include/xrpl/peerfinder/Config.h @@ -26,7 +26,8 @@ struct Config /** * The largest number of public peer slots to allow. * This includes both inbound and outbound, but does not include - * fixed peers. + * fixed peers. A configuration built by `makeConfig` always holds + * `maxPeers == inPeers + outPeers`. */ std::size_t maxPeers{tuning::kDefaultMaxPeers}; diff --git a/src/libxrpl/peerfinder/Config.cpp b/src/libxrpl/peerfinder/Config.cpp index 60ac0ca547..2e0f793a3a 100644 --- a/src/libxrpl/peerfinder/Config.cpp +++ b/src/libxrpl/peerfinder/Config.cpp @@ -107,8 +107,12 @@ Config::makeConfig( else { config.outPeers = *limits.outPeers; - config.inPeers = *limits.inPeers; - config.maxPeers = 0; + + // Inbound slots only exist if we accept incoming connections, and + // `maxPeers` is the total across both directions. The legacy branch + // above upholds the same two invariants. + config.inPeers = config.wantIncoming ? *limits.inPeers : 0; + config.maxPeers = config.inPeers + config.outPeers; } // This will cause servers configured as validators to request that diff --git a/src/test/overlay/overlay_limit_test.cpp b/src/test/overlay/overlay_limit_test.cpp new file mode 100644 index 0000000000..11bbdc2377 --- /dev/null +++ b/src/test/overlay/overlay_limit_test.cpp @@ -0,0 +1,83 @@ +#include +#include + +#include +#include + +#include +#include + +#include + +namespace xrpl::test { + +using namespace jtx; + +/** + * Tests for `Overlay::limit()`, the configured peer allowance reported once + * `OverlayImpl::start()` applies the computed `peer_finder::Config`. + * + * `ApplicationImp::fdRequired()` runs before `OverlayImpl::start()` does, so it + * always sees the peer finder manager's default-constructed configuration and + * never this value; `Overlay::limit()` instead surfaces through the PeerFinder + * property stream and other post-startup callers. + * + * `jtx::Env` runs standalone, and `ServerHandler` strips the `peer` protocol + * from every configured port under `config.standalone()`, so the peer port + * declared here is never bound and incoming connections are disabled + * throughout; every limit in this suite is an outbound-only allowance. The + * inbound cases live alongside `peer_finder::Config::makeConfig`, which takes + * the port as a parameter. + */ +class OverlayLimit_test : public beast::unit_test::Suite +{ + void + testLegacyPeersMax() + { + testcase("Legacy peers_max is reported"); + + auto config = jtx::envconfig(); + config->peersMax = 40; + + Env env(*this, std::move(config)); + BEAST_EXPECT(env.app().getOverlay().limit() == 40); + } + + void + testPerDirectionPeerLimits() + { + testcase("Per-direction peer limits are reported"); + + // With incoming connections disabled the 50 inbound slots are dropped + // and only the outbound allowance remains, so neither zero (the value + // `maxPeers` used to hold in this branch of makeConfig) nor 70 (the + // unconditional sum of both directions) is correct. + auto config = jtx::envconfig(); + config->peersInMax = 50; + config->peersOutMax = 20; + + Env env(*this, std::move(config)); + BEAST_EXPECT(env.app().getOverlay().limit() == 20); + } + + void + testDefaultConfig() + { + testcase("A default configuration reports the default limit"); + + Env env(*this); + BEAST_EXPECT(env.app().getOverlay().limit() == peer_finder::tuning::kDefaultMaxPeers); + } + + void + run() override + { + testLegacyPeersMax(); + testPerDirectionPeerLimits(); + testDefaultConfig(); + } +}; + +BEAST_DEFINE_TESTSUITE(OverlayLimit, overlay, xrpl); + +} // namespace xrpl::test diff --git a/src/tests/libxrpl/peerfinder/PeerFinder.cpp b/src/tests/libxrpl/peerfinder/PeerFinder.cpp index 3a52bbb5aa..2d98d4385f 100644 --- a/src/tests/libxrpl/peerfinder/PeerFinder.cpp +++ b/src/tests/libxrpl/peerfinder/PeerFinder.cpp @@ -1214,6 +1214,9 @@ TEST(PeerFinderConfig, applies_legacy_and_explicit_peer_limits) .expectedOut = 10, .expectedIn = 0, .expectedIpLimit = 1}, + // A port of zero disables incoming connections, so the configured + // inbound limit is dropped and the per-IP inbound limit collapses to + // one, exactly as in the legacy private case above. {.name = "new in 100/out 10, private", .maxPeers = {}, .maxIn = 100, @@ -1221,7 +1224,7 @@ TEST(PeerFinderConfig, applies_legacy_and_explicit_peer_limits) .port = 0, .expectedOut = 10, .expectedIn = 0, - .expectedIpLimit = 6}}; + .expectedIpLimit = 1}}; for (auto const& testCase : cases) { @@ -1239,6 +1242,13 @@ TEST(PeerFinderConfig, applies_legacy_and_explicit_peer_limits) EXPECT_EQ(counts.inMax(), testCase.expectedIn); EXPECT_EQ(config.ipLimit, testCase.expectedIpLimit); + // The configuration itself carries the same per-direction allowances + // that the slot counts derive, and `maxPeers` is their total. Callers + // such as `Overlay::limit` read `maxPeers` directly. + EXPECT_EQ(config.outPeers, testCase.expectedOut); + EXPECT_EQ(config.inPeers, testCase.expectedIn); + EXPECT_EQ(config.maxPeers, config.inPeers + config.outPeers); + NiceMock store; allowEmptyStore(store); NiceMock checker;