From ce045cb0840f5d719bad6e6b18659fd7f797655d Mon Sep 17 00:00:00 2001 From: Brett Boston Date: Mon, 17 Jun 2024 16:50:38 -0700 Subject: [PATCH] Make future removal easier --- src/overlay/Peer.cpp | 11 ++++---- src/overlay/Peer.h | 3 +++ src/overlay/test/OverlayTests.cpp | 44 +++++++++++++++++++------------ 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/overlay/Peer.cpp b/src/overlay/Peer.cpp index e303ea7e87..80b8686b4f 100644 --- a/src/overlay/Peer.cpp +++ b/src/overlay/Peer.cpp @@ -392,12 +392,10 @@ Peer::sendAuth() #ifdef BUILD_TESTS if (!mAppConnector.getOverlayManager() .isFlowControlBytesDisabledForTesting()) +#endif { msg.auth().flags = AUTH_MSG_FLAG_FLOW_CONTROL_BYTES_REQUESTED; } -#else - msg.auth().flags = AUTH_MSG_FLAG_FLOW_CONTROL_BYTES_REQUESTED; -#endif auto msgPtr = std::make_shared(msg); sendMessage(msgPtr); } @@ -1751,9 +1749,12 @@ Peer::recvAuth(StellarMessage const& msg) return; } - // NOTE: Once min overlay version is 35 we can remove this check + // NOTE: Once min overlay version is + // MANDATORY_FLOW_CONTROL_BYTES_MIN_OVERLAY_VERSION we can remove this check bool bothWantBytes = - msg.auth().flags == AUTH_MSG_FLAG_FLOW_CONTROL_BYTES_REQUESTED; + msg.auth().flags == AUTH_MSG_FLAG_FLOW_CONTROL_BYTES_REQUESTED || + getRemoteOverlayVersion() >= + MANDATORY_FLOW_CONTROL_BYTES_MIN_OVERLAY_VERSION; #ifdef BUILD_TESTS if (mAppConnector.getOverlayManager() diff --git a/src/overlay/Peer.h b/src/overlay/Peer.h index a95e516af8..a46e7cb024 100644 --- a/src/overlay/Peer.h +++ b/src/overlay/Peer.h @@ -27,6 +27,9 @@ static size_t const MAX_SOROBAN_BYTE_ALLOWANCE = static size_t const MAX_CLASSIC_BYTE_ALLOWANCE = MAX_TX_SET_ALLOWANCE / 2; // 5 MB +// Overlay version at which we started mandating flow control bytes +uint32_t constexpr MANDATORY_FLOW_CONTROL_BYTES_MIN_OVERLAY_VERSION = 35; + static_assert(MAX_TX_SET_ALLOWANCE >= MAX_SOROBAN_BYTE_ALLOWANCE + MAX_CLASSIC_BYTE_ALLOWANCE); diff --git a/src/overlay/test/OverlayTests.cpp b/src/overlay/test/OverlayTests.cpp index 6ae6a58780..cd2e07fa1a 100644 --- a/src/overlay/test/OverlayTests.cpp +++ b/src/overlay/test/OverlayTests.cpp @@ -529,16 +529,23 @@ TEST_CASE("flow control byte capacity", "[overlay][flowcontrol]") } } +// Run a test where flow control is enabled on both nodes, and then run the same +// test with flow controlled disabled on one of the nodes. `cfgMaybeDisabled` +// is the config that will be used for the node that may have flow control +// disabled. `f` is the test function that will be run. void -runWithBothFlowControlModes(std::function f) +runWithBothFlowControlModes(Config& cfgMaybeDisabled, + std::function f) { SECTION("bytes enabled on all") { - f(true); + f(cfgMaybeDisabled, true); } SECTION("bytes disabled on some") { - f(false); + cfgMaybeDisabled.OVERLAY_PROTOCOL_VERSION = + MANDATORY_FLOW_CONTROL_BYTES_MIN_OVERLAY_VERSION - 1; + f(cfgMaybeDisabled, false); } } @@ -546,12 +553,13 @@ TEST_CASE("loopback peer flow control activation", "[overlay][flowcontrol]") { VirtualClock clock; std::vector cfgs = {getTestConfig(0), getTestConfig(1)}; - auto cfg1 = cfgs[0]; + auto cfg1Base = cfgs[0]; auto cfg2 = cfgs[1]; - REQUIRE(cfg1.PEER_FLOOD_READING_CAPACITY != - cfg1.PEER_FLOOD_READING_CAPACITY_BYTES); + REQUIRE(cfg1Base.PEER_FLOOD_READING_CAPACITY != + cfg1Base.PEER_FLOOD_READING_CAPACITY_BYTES); - auto runTest = [&](bool sendIllegalSendMore, bool fcBytesEnabledOnAll) { + auto runTest = [&](bool sendIllegalSendMore, Config cfg1, + bool fcBytesEnabledOnAll) { auto app1 = createTestApplication(clock, cfg1); auto app2 = createTestApplication(clock, cfg2); @@ -623,26 +631,26 @@ TEST_CASE("loopback peer flow control activation", "[overlay][flowcontrol]") testutil::shutdownWorkScheduler(*app1); }; - auto test = [&](bool fcBytesEnabledOnAll) { + auto test = [&](Config cfg1, bool fcBytesEnabledOnAll) { SECTION("basic") { // Successfully enabled flow control - runTest(false, fcBytesEnabledOnAll); + runTest(false, cfg1, fcBytesEnabledOnAll); } SECTION("bad peer") { - runTest(true, fcBytesEnabledOnAll); + runTest(true, cfg1, fcBytesEnabledOnAll); } }; - runWithBothFlowControlModes(test); + runWithBothFlowControlModes(cfg1Base, test); } TEST_CASE("drop peers that dont respect capacity", "[overlay][flowcontrol]") { VirtualClock clock; std::vector cfgs = {getTestConfig(0), getTestConfig(1)}; - auto cfg1 = cfgs[0]; + auto cfg1Base = cfgs[0]; auto cfg2 = cfgs[1]; // tx is invalid, but it doesn't matter @@ -652,7 +660,7 @@ TEST_CASE("drop peers that dont respect capacity", "[overlay][flowcontrol]") getOperationGreaterThanMinMaxSizeBytes()); uint32 txSize = static_cast(xdr::xdr_argpack_size(msg)); - auto test = [&](bool fcBytesEnabledOnAll) { + auto test = [&](Config cfg1, bool fcBytesEnabledOnAll) { if (fcBytesEnabledOnAll) { cfg1.PEER_FLOOD_READING_CAPACITY_BYTES = @@ -709,21 +717,21 @@ TEST_CASE("drop peers that dont respect capacity", "[overlay][flowcontrol]") testutil::shutdownWorkScheduler(*app1); }; - runWithBothFlowControlModes(test); + runWithBothFlowControlModes(cfg1Base, test); } TEST_CASE("drop idle flow-controlled peers", "[overlay][flowcontrol]") { VirtualClock clock; std::vector cfgs = {getTestConfig(0), getTestConfig(1)}; - auto cfg1 = cfgs[0]; + auto cfg1Base = cfgs[0]; auto cfg2 = cfgs[1]; StellarMessage msg; msg.type(TRANSACTION); uint32 txSize = static_cast(xdr::xdr_argpack_size(msg)); - auto test = [&](bool fcBytesEnabledOnAll) { + auto test = [&](Config cfg1, bool fcBytesEnabledOnAll) { if (fcBytesEnabledOnAll) { cfg1.PEER_FLOOD_READING_CAPACITY_BYTES = txSize; @@ -786,7 +794,7 @@ TEST_CASE("drop idle flow-controlled peers", "[overlay][flowcontrol]") testutil::shutdownWorkScheduler(*app1); }; - runWithBothFlowControlModes(test); + runWithBothFlowControlModes(cfg1Base, test); } TEST_CASE("drop peers that overflow capacity", "[overlay][flowcontrol]") @@ -2221,6 +2229,8 @@ TEST_CASE("overlay flow control", "[overlay][flowcontrol]") Herder::FLOW_CONTROL_BYTES_EXTRA_BUFFER; cfg.FLOW_CONTROL_SEND_MORE_BATCH_SIZE_BYTES = 100; cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 1000; + cfg.OVERLAY_PROTOCOL_VERSION = + MANDATORY_FLOW_CONTROL_BYTES_MIN_OVERLAY_VERSION - 1; configs.push_back(cfg); }