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>
This commit is contained in:
Bart
2026-09-15 13:14:12 +00:00
committed by GitHub
parent 1a4a40ebb8
commit e302e4eeed
4 changed files with 102 additions and 4 deletions

View File

@@ -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};

View File

@@ -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

View File

@@ -0,0 +1,83 @@
#include <test/jtx/Env.h>
#include <test/jtx/envconfig.h>
#include <xrpld/app/main/Application.h>
#include <xrpld/overlay/Overlay.h>
#include <xrpl/beast/unit_test/suite.h>
#include <xrpl/peerfinder/detail/Tuning.h>
#include <utility>
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

View File

@@ -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<MockStore> store;
allowEmptyStore(store);
NiceMock<MockChecker> checker;