diff --git a/src/herder/HerderUtils.cpp b/src/herder/HerderUtils.cpp index 2640399f5a..1190b56f86 100644 --- a/src/herder/HerderUtils.cpp +++ b/src/herder/HerderUtils.cpp @@ -4,7 +4,6 @@ #include "herder/HerderUtils.h" #include "scp/Slot.h" -#include "util/ProtocolVersion.h" #include "xdr/Stellar-ledger.h" #include #include @@ -41,18 +40,4 @@ getStellarValues(SCPStatement const& statement) return result; } - -bool -shouldDropPeerPredicate(Peer::pointer peer, uint32_t protocolVersion) -{ - bool upgraded = - protocolVersionStartsFrom(protocolVersion, SOROBAN_PROTOCOL_VERSION); - auto ovVersion = peer->getRemoteOverlayVersion(); - if (upgraded && ovVersion && - ovVersion.value() < Peer::FIRST_VERSION_REQUIRED_FOR_PROTOCOL_20) - { - return true; - } - return false; -} } diff --git a/src/herder/HerderUtils.h b/src/herder/HerderUtils.h index 5e14a62a8e..182b2f0142 100644 --- a/src/herder/HerderUtils.h +++ b/src/herder/HerderUtils.h @@ -4,7 +4,6 @@ // under the Apache License, Version 2.0. See the COPYING file at the root // of this distribution or at http://www.apache.org/licenses/LICENSE-2.0 -#include "overlay/Peer.h" #include "xdr/Stellar-types.h" #include @@ -17,5 +16,4 @@ struct StellarValue; std::vector getTxSetHashes(SCPEnvelope const& envelope); std::vector getStellarValues(SCPStatement const& envelope); -bool shouldDropPeerPredicate(Peer::pointer peer, uint32_t protocolVersion); } diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index 60bce0bb25..759bcadd85 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -13,7 +13,6 @@ #include "database/Database.h" #include "herder/Herder.h" #include "herder/HerderPersistence.h" -#include "herder/HerderUtils.h" #include "herder/LedgerCloseData.h" #include "herder/TxSetFrame.h" #include "herder/Upgrades.h" @@ -978,8 +977,6 @@ LedgerManagerImpl::closeLedger(LedgerCloseData const& ledgerData) if (protocolVersionStartsFrom(maybeNewVersion, SOROBAN_PROTOCOL_VERSION)) { updateNetworkConfig(ltx); - mApp.getOverlayManager().dropPeersIf( - shouldDropPeerPredicate, maybeNewVersion, "version too old"); } ledgerClosed(ltx, ledgerCloseMeta, initialLedgerVers); diff --git a/src/overlay/OverlayManager.h b/src/overlay/OverlayManager.h index a03f09aea0..ae01421c15 100644 --- a/src/overlay/OverlayManager.h +++ b/src/overlay/OverlayManager.h @@ -112,8 +112,6 @@ class OverlayManager // Return a list of random peers from the set of authenticated peers. virtual std::vector getRandomAuthenticatedPeers() = 0; - virtual std::vector - getAuthenticatedPeers(bool randomize) = 0; // Return a list of random peers from the set of inbound authenticated // peers. @@ -207,12 +205,7 @@ class OverlayManager virtual void recordMessageMetric(StellarMessage const& stellarMsg, Peer::pointer peer) = 0; - virtual AdjustedFlowControlConfig getFlowControlBytesConfig() const = 0; - - virtual void - dropPeersIf(std::function predicate, - uint32_t version, std::string const& reason) = 0; virtual ~OverlayManager() { } diff --git a/src/overlay/OverlayManagerImpl.cpp b/src/overlay/OverlayManagerImpl.cpp index 9d0d47d524..63eba70a3d 100644 --- a/src/overlay/OverlayManagerImpl.cpp +++ b/src/overlay/OverlayManagerImpl.cpp @@ -366,37 +366,6 @@ OverlayManagerImpl::getFlowControlBytesConfig() const cfg.FLOW_CONTROL_SEND_MORE_BATCH_SIZE_BYTES}; } -void -OverlayManagerImpl::dropPeersIf( - std::function predicate, uint32_t version, - std::string const& reason) -{ - auto maybeDrop = [](auto peers, - std::function predicate, - uint32_t version, std::string const& reason) { - std::vector peersToDrop; - for (auto it = peers.begin(); it != peers.end(); ++it) - { - if (predicate(*it, version)) - { - peersToDrop.emplace_back(*it); - } - } - - for (auto& peer : peersToDrop) - { - // Drop will cleanup peer lists and remove peer references from - // overlay manager - peer->drop(reason, Peer::DropDirection::WE_DROPPED_REMOTE, - Peer::DropMode::IGNORE_WRITE_QUEUE); - } - }; - - maybeDrop(getPendingPeers(), predicate, version, reason); - maybeDrop(getAuthenticatedPeers(/* randomize */ false), predicate, version, - reason); -} - void OverlayManagerImpl::connectTo(PeerBareAddress const& address) { @@ -1071,22 +1040,13 @@ OverlayManager::isFloodMessage(StellarMessage const& msg) } std::vector OverlayManagerImpl::getRandomAuthenticatedPeers() -{ - return getAuthenticatedPeers(true); -} - -std::vector -OverlayManagerImpl::getAuthenticatedPeers(bool randomize) { std::vector result; result.reserve(mInboundPeers.mAuthenticated.size() + mOutboundPeers.mAuthenticated.size()); extractPeersFromMap(mInboundPeers.mAuthenticated, result); extractPeersFromMap(mOutboundPeers.mAuthenticated, result); - if (randomize) - { - shufflePeerList(result); - } + shufflePeerList(result); return result; } diff --git a/src/overlay/OverlayManagerImpl.h b/src/overlay/OverlayManagerImpl.h index fcddad728a..830e9bf46a 100644 --- a/src/overlay/OverlayManagerImpl.h +++ b/src/overlay/OverlayManagerImpl.h @@ -143,8 +143,6 @@ class OverlayManagerImpl : public OverlayManager Peer::pointer getConnectedPeer(PeerBareAddress const& address) override; std::vector getRandomAuthenticatedPeers() override; - std::vector getAuthenticatedPeers(bool randomize) override; - std::vector getRandomInboundAuthenticatedPeers() override; std::vector getRandomOutboundAuthenticatedPeers() override; @@ -207,9 +205,6 @@ class OverlayManagerImpl : public OverlayManager void shufflePeerList(std::vector& peerList); AdjustedFlowControlConfig getFlowControlBytesConfig() const override; - void dropPeersIf(std::function predicate, - uint32_t version, std::string const& reason) override; - // Returns `true` iff the overlay can accept the outbound peer at `address`. // Logs whenever a peer cannot be accepted. bool canAcceptOutboundPeer(PeerBareAddress const& address) const; diff --git a/src/overlay/Peer.cpp b/src/overlay/Peer.cpp index d388fc2d2c..143ba1dbfd 100644 --- a/src/overlay/Peer.cpp +++ b/src/overlay/Peer.cpp @@ -28,7 +28,6 @@ #include "util/ProtocolVersion.h" #include "util/finally.h" -#include "herder/HerderUtils.h" #include "medida/meter.h" #include "medida/timer.h" #include "xdrpp/marshal.h" @@ -63,7 +62,7 @@ Peer::Peer(Application& app, PeerRole role) , mRecurringTimer(app) , mState(role == WE_CALLED_REMOTE ? CONNECTING : CONNECTED) , mRemoteOverlayMinVersion(0) - , mRemoteOverlayVersion(std::nullopt) + , mRemoteOverlayVersion(0) , mCreationTime(app.getClock().now()) , mDelayedExecutionTimer(app) , mTxAdverts(std::make_shared(app)) @@ -291,10 +290,7 @@ Peer::getJsonInfo(bool compact) const res["elapsed"] = (int)getLifeTime().count(); res["latency"] = (int)getPing().count(); res["ver"] = getRemoteVersion(); - if (getRemoteOverlayVersion()) - { - res["olver"] = getRemoteOverlayVersion().value(); - } + res["olver"] = (int)getRemoteOverlayVersion(); if (mFlowControl) { res["flow_control"] = mFlowControl->getFlowControlJsonInfo(compact); @@ -1408,19 +1404,14 @@ Peer::recvHello(Hello const& elo) dropMode = Peer::DropMode::FLUSH_WRITE_QUEUE; } - bool rejectBasedOnLedgerVersion = shouldDropPeerPredicate( - shared_from_this(), mAppConnector.getLedgerManager() - .getLastClosedLedgerHeader() - .header.ledgerVersion); - if (mRemoteOverlayMinVersion > mRemoteOverlayVersion.value() || - mRemoteOverlayVersion.value() < + if (mRemoteOverlayMinVersion > mRemoteOverlayVersion || + mRemoteOverlayVersion < mAppConnector.getConfig().OVERLAY_PROTOCOL_MIN_VERSION || mRemoteOverlayMinVersion > - mAppConnector.getConfig().OVERLAY_PROTOCOL_VERSION || - rejectBasedOnLedgerVersion) + mAppConnector.getConfig().OVERLAY_PROTOCOL_VERSION) { CLOG_DEBUG(Overlay, "Protocol = [{},{}] expected: [{},{}]", - mRemoteOverlayMinVersion, mRemoteOverlayVersion.value(), + mRemoteOverlayMinVersion, mRemoteOverlayVersion, mAppConnector.getConfig().OVERLAY_PROTOCOL_MIN_VERSION, mAppConnector.getConfig().OVERLAY_PROTOCOL_VERSION); sendErrorAndDrop(ERR_CONF, "wrong protocol version", dropMode); diff --git a/src/overlay/Peer.h b/src/overlay/Peer.h index 1857b002e8..fecb597b85 100644 --- a/src/overlay/Peer.h +++ b/src/overlay/Peer.h @@ -220,7 +220,7 @@ class Peer : public std::enable_shared_from_this, std::string mRemoteVersion; uint32_t mRemoteOverlayMinVersion; - std::optional mRemoteOverlayVersion; + uint32_t mRemoteOverlayVersion; PeerBareAddress mAddress; VirtualClock::time_point mCreationTime; @@ -356,7 +356,7 @@ class Peer : public std::enable_shared_from_this, return mRemoteOverlayMinVersion; } - std::optional + uint32_t getRemoteOverlayVersion() const { return mRemoteOverlayVersion; diff --git a/src/overlay/test/OverlayTests.cpp b/src/overlay/test/OverlayTests.cpp index 6456876ba0..5c04bfea15 100644 --- a/src/overlay/test/OverlayTests.cpp +++ b/src/overlay/test/OverlayTests.cpp @@ -135,58 +135,6 @@ TEST_CASE("loopback peer send auth before hello", "[overlay][connections]") testutil::shutdownWorkScheduler(*app1); } -TEST_CASE("overlay version peer drop post-protocol-20") -{ - auto cfg1 = getTestConfig(0); - auto cfg2 = getTestConfig(1); - - cfg1.OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_REQUIRED_FOR_PROTOCOL_20 - 1; - cfg1.OVERLAY_PROTOCOL_MIN_VERSION = cfg1.OVERLAY_PROTOCOL_VERSION - 1; - cfg2.OVERLAY_PROTOCOL_MIN_VERSION = cfg1.OVERLAY_PROTOCOL_MIN_VERSION; - cfg1.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; - cfg2.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; - - VirtualClock clock; - auto app1 = createTestApplication(clock, cfg1); - auto app2 = createTestApplication(clock, cfg2); - - auto result = LedgerUpgrade{LEDGER_UPGRADE_VERSION}; - result.newLedgerVersion() = static_cast(SOROBAN_PROTOCOL_VERSION); - LoopbackPeerConnection conn(*app1, *app2); - - std::string expectedDropReason = "version too old"; - SECTION("fully authenticated") - { - testutil::crankSome(clock); - REQUIRE(conn.getInitiator()->isAuthenticated()); - REQUIRE(conn.getAcceptor()->isAuthenticated()); - } - SECTION("pending peers") - { - REQUIRE(!conn.getInitiator()->isAuthenticated()); - REQUIRE(!conn.getAcceptor()->isAuthenticated()); - expectedDropReason = "wrong protocol version"; - } - - txtest::executeUpgrade(*app1, result); - txtest::executeUpgrade(*app2, result); - - testutil::crankSome(clock); - REQUIRE(!conn.getInitiator()->isConnected()); - REQUIRE(!conn.getAcceptor()->isConnected()); - REQUIRE(conn.getAcceptor()->getDropReason() == expectedDropReason); - - // Post-upgrade, try to re-connect and get rejected due to old protocol - LoopbackPeerConnection conn2(*app1, *app2); - testutil::crankSome(clock); - REQUIRE(!conn2.getInitiator()->isConnected()); - REQUIRE(!conn2.getAcceptor()->isConnected()); - REQUIRE(conn2.getAcceptor()->getDropReason() == "wrong protocol version"); -} - TEST_CASE("flow control byte capacity", "[overlay][flowcontrol]") { VirtualClock clock; @@ -329,17 +277,7 @@ TEST_CASE("flow control byte capacity", "[overlay][flowcontrol]") } SECTION("mixed versions") { - cfg1.OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_SUPPORTING_FLOW_CONTROL_IN_BYTES; - cfg2.OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_REQUIRED_FOR_PROTOCOL_20 - 1; - cfg1.OVERLAY_PROTOCOL_MIN_VERSION = cfg1.OVERLAY_PROTOCOL_VERSION; - cfg2.OVERLAY_PROTOCOL_MIN_VERSION = cfg1.OVERLAY_PROTOCOL_VERSION; - // Mixed versions do not work in v20, as older clients are banned - cfg1.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; - cfg2.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; + cfg1.OVERLAY_PROTOCOL_VERSION = cfg1.OVERLAY_PROTOCOL_MIN_VERSION; cfg1.PEER_FLOOD_READING_CAPACITY_BYTES = 2 * getTxSize(tx1) + Herder::FLOW_CONTROL_BYTES_EXTRA_BUFFER; @@ -352,17 +290,8 @@ TEST_CASE("flow control byte capacity", "[overlay][flowcontrol]") } SECTION("older versions") { - cfg1.OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_SUPPORTING_FLOW_CONTROL_IN_BYTES; - cfg2.OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_SUPPORTING_FLOW_CONTROL_IN_BYTES; - cfg1.OVERLAY_PROTOCOL_MIN_VERSION = cfg1.OVERLAY_PROTOCOL_VERSION; - cfg2.OVERLAY_PROTOCOL_MIN_VERSION = cfg2.OVERLAY_PROTOCOL_VERSION; - // Mixed versions do not work in v20, as older clients are banned - cfg1.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; - cfg2.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; + cfg1.OVERLAY_PROTOCOL_VERSION = cfg1.OVERLAY_PROTOCOL_MIN_VERSION; + cfg2.OVERLAY_PROTOCOL_VERSION = cfg2.OVERLAY_PROTOCOL_MIN_VERSION; cfg1.PEER_FLOOD_READING_CAPACITY_BYTES = 2 * getTxSize(tx1) + Herder::FLOW_CONTROL_BYTES_EXTRA_BUFFER; cfg1.FLOW_CONTROL_SEND_MORE_BATCH_SIZE_BYTES = getTxSize(tx1); @@ -614,8 +543,7 @@ runWithBothFlowControlModes(std::vector& cfgs, { for (auto& cfg : cfgs) { - cfg.OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_SUPPORTING_FLOW_CONTROL_IN_BYTES - 1; + cfg.ENABLE_FLOW_CONTROL_BYTES = false; } f(false); } @@ -712,21 +640,6 @@ TEST_CASE("loopback peer flow control activation", "[overlay][flowcontrol]") // Successfully enabled flow control runTest({cfg1, cfg2}, false); } - SECTION("mix versions") - { - cfg1.OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_SUPPORTING_FLOW_CONTROL_IN_BYTES; - cfg1.OVERLAY_PROTOCOL_MIN_VERSION = - cfg1.OVERLAY_PROTOCOL_VERSION; - cfg2.OVERLAY_PROTOCOL_MIN_VERSION = - cfg1.OVERLAY_PROTOCOL_VERSION; - - cfg1.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; - cfg2.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; - runTest({cfg1, cfg2}, false); - } SECTION("bad peer") { runTest({cfg1, cfg2}, true); @@ -2352,21 +2265,6 @@ TEST_CASE("overlay flow control", "[overlay][flowcontrol]") { setupSimulation(); } - SECTION("one peer does not support flow control in bytes") - { - configs[2].OVERLAY_PROTOCOL_VERSION = - Peer::FIRST_VERSION_SUPPORTING_FLOW_CONTROL_IN_BYTES - 1; - - for (auto& cfg : configs) - { - cfg.OVERLAY_PROTOCOL_MIN_VERSION = - configs[2].OVERLAY_PROTOCOL_VERSION - 1; - cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = - static_cast(SOROBAN_PROTOCOL_VERSION) - 1; - } - - setupSimulation(); - } SECTION("one peer disables") { configs[2].ENABLE_FLOW_CONTROL_BYTES = false;