From 78cc3ae1c52e9bd49363e75cf72db783b77a02dd Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:08:01 +0200 Subject: [PATCH] clang-tidy --- .cspell.config.yaml | 1 - src/test/overlay/base_squelch_test.cpp | 340 +++++++++++---------- src/test/overlay/enhanced_squelch_test.cpp | 223 +++++++------- src/test/overlay/squelch_store_test.cpp | 36 +-- src/xrpld/overlay/Slot.h | 40 +-- src/xrpld/overlay/SquelchStore.h | 3 +- src/xrpld/overlay/detail/OverlayImpl.cpp | 2 +- src/xrpld/overlay/detail/OverlayImpl.h | 1 - src/xrpld/overlay/detail/PeerImp.h | 2 +- src/xrpld/overlay/detail/Slot.cpp | 79 +++-- src/xrpld/overlay/detail/SquelchStore.cpp | 20 +- 11 files changed, 367 insertions(+), 380 deletions(-) diff --git a/.cspell.config.yaml b/.cspell.config.yaml index de765b4293..e220cd0249 100644 --- a/.cspell.config.yaml +++ b/.cspell.config.yaml @@ -341,7 +341,6 @@ words: - unsquelched - unsquelching - unvalidated - - unviable - unveto - unvetoed - upvotes diff --git a/src/test/overlay/base_squelch_test.cpp b/src/test/overlay/base_squelch_test.cpp index 278522b6d2..61fbd8f46c 100644 --- a/src/test/overlay/base_squelch_test.cpp +++ b/src/test/overlay/base_squelch_test.cpp @@ -1,24 +1,8 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright 2020 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - #include +#include +#include +#include #include #include #include @@ -26,22 +10,48 @@ #include #include +#include #include -#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include +#include + #include +#include #include +#include +#include +#include +#include #include +#include #include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include namespace xrpl::test { using namespace std::chrono; template -class extended_manual_clock : public beast::ManualClock +class ExtendedManualClock : public beast::ManualClock { public: using typename beast::ManualClock::duration; @@ -64,11 +74,11 @@ using PeerWPtr = std::weak_ptr; using SquelchCB = std::function; using UnsquelchCB = std::function; using LinkIterCB = std::function; -using TestStopwatch = extended_manual_clock; +using TestStopwatch = ExtendedManualClock; -static constexpr std::uint32_t MAX_PEERS = 10; -static constexpr std::uint32_t MAX_VALIDATORS = 10; -static constexpr std::uint32_t MAX_MESSAGES = 200000; +static constexpr std::uint32_t maxPeers = 10; +static constexpr std::uint32_t maxValidators = 10; +static constexpr std::uint32_t maxMessages = 200000; /** * Simulate two entities - peer directly connected to the server @@ -77,14 +87,12 @@ static constexpr std::uint32_t MAX_MESSAGES = 200000; class PeerPartial : public Peer { public: - PeerPartial() : nodePublicKey_(derivePublicKey(KeyType::Ed25519, randomSecretKey())) + PeerPartial() : nodePublicKey(derivePublicKey(KeyType::Ed25519, randomSecretKey())) { } - PublicKey nodePublicKey_; - virtual ~PeerPartial() - { - } + PublicKey nodePublicKey; + ~PeerPartial() override = default; virtual void onMessage(MessageSPtr const& m, SquelchCB f) = 0; virtual void @@ -100,7 +108,7 @@ public: send(std::shared_ptr const& m) override { } - beast::IP::Endpoint + [[nodiscard]] beast::IP::Endpoint getRemoteAddress() const override { return {}; @@ -109,43 +117,43 @@ public: charge(Resource::Charge const& fee, std::string const& context = {}) override { } - bool + [[nodiscard]] bool cluster() const override { return false; } - bool + [[nodiscard]] bool isHighLatency() const override { return false; } - int + [[nodiscard]] int getScore(bool) const override { return 0; } - PublicKey const& + [[nodiscard]] PublicKey const& getNodePublic() const override { - return nodePublicKey_; + return nodePublicKey; } json::Value json() override { return {}; } - std::string const& + [[nodiscard]] std::string const& fingerprint() const override { static std::string const kFingerprint{}; return kFingerprint; } - bool + [[nodiscard]] bool supportsFeature(ProtocolFeature f) const override { return false; } - std::optional + [[nodiscard]] std::optional publisherListSequence(PublicKey const&) const override { return {}; @@ -154,13 +162,13 @@ public: setPublisherListSequence(PublicKey const&, std::size_t const) override { } - uint256 const& + [[nodiscard]] uint256 const& getClosedLedgerHash() const override { - static uint256 hash{}; + static uint256 const hash{}; return hash; } - bool + [[nodiscard]] bool hasLedger(uint256 const& hash, std::uint32_t seq) const override { return false; @@ -169,7 +177,7 @@ public: ledgerRange(std::uint32_t& minSeq, std::uint32_t& maxSeq) const override { } - bool + [[nodiscard]] bool hasTxSet(uint256 const& hash) const override { return false; @@ -183,12 +191,12 @@ public: { return false; } - bool + [[nodiscard]] bool compressionEnabled() const override { return false; } - bool + [[nodiscard]] bool txReduceRelayEnabled() const override { return false; @@ -248,11 +256,8 @@ class Link using Latency = std::pair; public: - Link( - Validator& validator, - PeerSPtr peer, - Latency const& latency = {milliseconds(5), milliseconds(15)}) - : validator_(validator), peer_(peer), latency_(latency), up_(true) + Link(Validator& validator, PeerSPtr peer, Latency latency = {milliseconds(5), milliseconds(15)}) + : validator_(validator), peer_(peer), latency_(std::move(latency)) { auto sp = peer_.lock(); assert(sp); @@ -297,7 +302,7 @@ private: Validator& validator_; PeerWPtr peer_; Latency latency_; - bool up_; + bool up_{true}; }; /** @@ -308,12 +313,11 @@ class Validator using Links = std::unordered_map; public: - Validator() : pkey_(std::get<0>(randomKeyPair(KeyType::Ed25519))) + Validator() : pkey_(std::get<0>(randomKeyPair(KeyType::Ed25519))), id_(sid_++) { protocol::TMValidation v; v.set_validation("validation"); message_ = std::make_shared(v, protocol::mtVALIDATION, pkey_); - id_ = sid_++; } Validator(Validator const&) = default; Validator(Validator&&) = default; @@ -362,7 +366,7 @@ public: } void - for_links(std::vector peers, LinkIterCB f) + forLinks(std::vector peers, LinkIterCB f) { for (auto id : peers) { @@ -372,12 +376,10 @@ public: } void - for_links(LinkIterCB f, bool simulateSlow = false) + forLinks(LinkIterCB f, bool simulateSlow = false) { std::vector v; - std::transform(links_.begin(), links_.end(), std::back_inserter(v), [](auto& kv) { - return kv.second; - }); + std::ranges::transform(links_, std::back_inserter(v), [](auto& kv) { return kv.second; }); std::random_device d; std::mt19937 g(d()); std::shuffle(v.begin(), v.end(), g); @@ -394,7 +396,7 @@ public: void send(std::vector peers, SquelchCB f) { - for_links(peers, [&](Link& link, MessageSPtr m) { link.send(m, f); }); + forLinks(peers, [&](Link& link, MessageSPtr m) { link.send(m, f); }); } /** @@ -403,7 +405,7 @@ public: void send(SquelchCB f) { - for_links([&](Link& link, MessageSPtr m) { link.send(m, f); }); + forLinks([&](Link& link, MessageSPtr m) { link.send(m, f); }); } MessageSPtr @@ -413,7 +415,7 @@ public: } std::uint16_t - id() + id() const { return id_; } @@ -467,12 +469,11 @@ class PeerSim : public PeerPartial, public std::enable_shared_from_this { public: PeerSim(Overlay& overlay, beast::Journal journal) - : overlay_(overlay), squelchStore_(journal, overlay_.clock()) + : id_(sid_++), overlay_(overlay), squelchStore_(journal, overlay_.clock()) { - id_ = sid_++; } - ~PeerSim() = default; + ~PeerSim() override = default; Peer::id_t id() const override @@ -503,11 +504,11 @@ public: /** * Remote Peer (Directly connected Peer) */ - virtual void + void onMessage(protocol::TMSquelch const& squelch) override { auto validator = squelch.validatorpubkey(); - PublicKey key(Slice(validator.data(), validator.size())); + PublicKey const key(Slice(validator.data(), validator.size())); squelchStore_.handleSquelch( key, squelch.squelch(), std::chrono::seconds{squelch.squelchduration()}); } @@ -529,7 +530,7 @@ public: { } - ~OverlaySim() = default; + ~OverlaySim() override = default; void clear() @@ -544,10 +545,12 @@ public: { auto const& it = slots_.getSlots().find(validator); if (it != slots_.getSlots().end()) + { return std::count_if( it->second.getPeers().begin(), it->second.getPeers().end(), [&](auto const& it) { return (it.second.state == state); }); + } return 0; } @@ -578,7 +581,7 @@ public: addPeer(bool useCache = true) { PeerSPtr peer{}; - Peer::id_t id; + Peer::id_t id = 0; if (peersCache_.empty() || !useCache) { peer = std::make_shared(*this, logs_.journal("Squelch")); @@ -626,8 +629,7 @@ public: for (auto& [id, _] : peers_) { (void)_; - if (id > maxId) - maxId = id; + maxId = std::max(id, maxId); } deletePeer(maxId, false); @@ -654,8 +656,10 @@ public: std::set r; for (auto const& [id, info] : it->second.getPeers()) + { if (info.state == reduce_relay::PeerState::Selected) r.insert(id); + } return r; } @@ -664,14 +668,14 @@ public: isSelected(PublicKey const& validator, Peer::id_t peer) { auto selected = getSelected(validator); - return selected.find(peer) != selected.end(); + return selected.contains(peer); } Peer::id_t getSelectedPeer(PublicKey const& validator) { auto selected = getSelected(validator); - assert(selected.size()); + assert(!selected.empty()); return *selected.begin(); } @@ -684,7 +688,7 @@ public: auto r = std::unordered_map(); for (auto const& [id, info] : it->second.getPeers()) - r.emplace(std::make_pair(id, info)); + r.emplace(id, info); return r; } @@ -741,8 +745,8 @@ public: void init() { - validators_.resize(MAX_VALIDATORS); - for (int p = 0; p < MAX_PEERS; p++) + validators_.resize(maxValidators); + for (int p = 0; p < maxPeers; p++) { auto peer = overlay_.addPeer(); for (auto& v : validators_) @@ -786,7 +790,7 @@ public: void purgePeers() { - while (overlay_.getNumPeers() > MAX_PEERS) + while (overlay_.getNumPeers() > maxPeers) deleteLastPeer(); } @@ -806,13 +810,16 @@ public: void enableLink(std::uint16_t validatorId, Peer::id_t peer, bool enable) { - auto it = std::find_if( - validators_.begin(), validators_.end(), [&](auto& v) { return v.id() == validatorId; }); + auto it = std::ranges::find_if(validators_, [&](auto& v) { return v.id() == validatorId; }); assert(it != validators_.end()); if (enable) + { it->linkUp(peer); + } else + { it->linkDown(peer); + } } void @@ -826,21 +833,21 @@ public: squelch.set_squelch(false); for (auto& v : validators_) { - PublicKey key = v; + PublicKey const key = v; squelch.clear_validatorpubkey(); squelch.set_validatorpubkey(key.data(), key.size()); - v.for_links({peer}, [&](Link& l, MessageSPtr) { + v.forLinks({peer}, [&](Link& l, MessageSPtr) { std::dynamic_pointer_cast(l.getPeer())->send(squelch); }); } } - void - for_rand(std::uint32_t min, std::uint32_t max, std::function f) + static void + forRand(std::uint32_t min, std::uint32_t max, std::function f) { auto size = max - min; std::vector s(size); - std::iota(s.begin(), s.end(), min); + std::ranges::iota(s, min); std::random_device d; std::mt19937 g(d()); std::shuffle(s.begin(), s.end(), g); @@ -851,8 +858,8 @@ public: void propagate( LinkIterCB link, - std::uint16_t nValidators = MAX_VALIDATORS, - std::uint32_t nMessages = MAX_MESSAGES, + std::uint16_t nValidators = maxValidators, + std::uint32_t nMessages = maxMessages, bool purge = true) { if (purge) @@ -864,7 +871,7 @@ public: for (int m = 0; m < nMessages; ++m) { overlay_.clock().randAdvance(milliseconds(1800), milliseconds(2200)); - for_rand(0, nValidators, [&](std::uint32_t v) { validators_[v].for_links(link); }); + forRand(0, nValidators, [&](std::uint32_t v) { validators_[v].forLinks(link); }); } } @@ -929,14 +936,14 @@ protected: /** * Send squelch (if duration is set) or unsquelch (if duration not set) */ - Peer::id_t + static Peer::id_t sendSquelch( PublicKey const& validator, PeerWPtr const& peerPtr, std::optional duration) { protocol::TMSquelch squelch; - bool res = duration ? true : false; + bool const res = static_cast(duration); squelch.set_squelch(res); squelch.set_validatorpubkey(validator.data(), validator.size()); if (res) @@ -955,15 +962,15 @@ protected: // time in any quantity struct Event { - State state_ = State::Off; - std::uint32_t cnt_ = 0; - std::uint32_t handledCnt_ = 0; + State state = State::Off; + std::uint32_t cnt = 0; + std::uint32_t handledCnt = 0; bool isSelected_ = false; - Peer::id_t peer_; - std::uint16_t validator_; - std::optional key_; - TestStopwatch::time_point time_; - bool handled_ = false; + Peer::id_t peer_{}; + std::uint16_t validator{}; + std::optional key; + TestStopwatch::time_point time; + bool handled = false; }; /** @@ -999,12 +1006,14 @@ protected: for (auto s : selected) str << s << " "; if (log) + { std::cout << (double)std::chrono::duration_cast( now.time_since_epoch()) .count() / 1000. << " random, squelched, validator: " << validator.id() << " peers: " << str.str() << std::endl; + } auto countingState = network_.overlay().isCountingState(validator); BEAST_EXPECT( countingState == false && @@ -1013,15 +1022,15 @@ protected: // Trigger Link Down or Peer Disconnect event // Only one Link Down at a time - if (events[EventType::LinkDown].state_ == State::Off) + if (events[EventType::LinkDown].state == State::Off) { auto update = [&](EventType event) { - events[event].cnt_++; - events[event].validator_ = validator.id(); - events[event].key_ = validator; + events[event].cnt++; + events[event].validator = validator.id(); + events[event].key = validator; events[event].peer_ = link.peerId(); - events[event].state_ = State::On; - events[event].time_ = now; + events[event].state = State::On; + events[event].time = now; if (event == EventType::LinkDown) { network_.enableLink(validator.id(), link.peerId(), false); @@ -1029,7 +1038,11 @@ protected: network_.overlay().isSelected(validator, link.peerId()); } else - events[event].isSelected_ = network_.isSelected(link.peerId()); + { + { + events[event].isSelected_ = network_.isSelected(link.peerId()); + } + } }; auto r = randInt(0, 1000); if (r == (int)EventType::LinkDown || r == (int)EventType::PeerDisconnected) @@ -1038,28 +1051,28 @@ protected: } } - if (events[EventType::PeerDisconnected].state_ == State::On) + if (events[EventType::PeerDisconnected].state == State::On) { auto& event = events[EventType::PeerDisconnected]; - bool allCounting = network_.allCounting(event.peer_); + bool const allCounting = network_.allCounting(event.peer_); network_.overlay().deletePeer( event.peer_, [&](PublicKey const& v, PeerWPtr const& peerPtr) { if (event.isSelected_) sendSquelch(v, peerPtr, {}); - event.handled_ = true; + event.handled = true; }); // Should only be unsquelched if the peer is in Selected state // If in Selected state it's possible unsquelching didn't // take place because there is no peers in Squelched state in // any of the slots where the peer is in Selected state // (allCounting is true) - bool handled = (event.isSelected_ == false && !event.handled_) || - (event.isSelected_ == true && (event.handled_ || allCounting)); + bool const handled = (!event.isSelected_ && !event.handled) || + (event.isSelected_ && (event.handled || allCounting)); BEAST_EXPECT(handled); - event.state_ = State::Off; + event.state = State::Off; event.isSelected_ = false; - event.handledCnt_ += handled; - event.handled_ = false; + event.handledCnt += handled; + event.handled = false; network_.onDisconnectPeer(event.peer_); } @@ -1076,10 +1089,10 @@ protected: // 4) peer is in Slot's peers_ (if not then it is deleted // by Slots::deleteIdlePeers()) bool mustHandle = false; - if (event.state_ == State::On && BEAST_EXPECT(event.key_)) + if (event.state == State::On && BEAST_EXPECT(event.key)) { - event.isSelected_ = network_.overlay().isSelected(*event.key_, event.peer_); - auto peers = network_.overlay().getPeers(*event.key_); + event.isSelected_ = network_.overlay().isSelected(*event.key, event.peer_); + auto peers = network_.overlay().getPeers(*event.key); auto d = std::chrono::duration_cast( now.time_since_epoch()) @@ -1090,46 +1103,48 @@ protected: mustHandle = event.isSelected_ && d > milliseconds(reduce_relay::kIdled).count() && - network_.overlay().inState( - *event.key_, reduce_relay::PeerState::Squelched) > 0 && - peers.find(event.peer_) != peers.end(); + network_.overlay().inState(*event.key, reduce_relay::PeerState::Squelched) > + 0 && + peers.contains(event.peer_); } network_.overlay().deleteIdlePeers([&](PublicKey const& v, PeerWPtr const& ptr) { - event.handled_ = true; - if (mustHandle && v == event.key_) + event.handled = true; + if (mustHandle && v == event.key) { - event.state_ = State::WaitReset; + event.state = State::WaitReset; sendSquelch(validator, ptr, {}); } }); - bool handled = (event.handled_ && event.state_ == State::WaitReset) || - (!event.handled_ && !mustHandle); + bool const handled = (event.handled && event.state == State::WaitReset) || + (!event.handled && !mustHandle); BEAST_EXPECT(handled); } - if (event.state_ == State::WaitReset || - (event.state_ == State::On && - (now - event.time_ > (reduce_relay::kIdled + seconds(2))))) + if (event.state == State::WaitReset || + (event.state == State::On && + (now - event.time > (reduce_relay::kIdled + seconds(2))))) { - bool handled = event.state_ == State::WaitReset || !event.handled_; + bool const handled = event.state == State::WaitReset || !event.handled; BEAST_EXPECT(handled); - event.state_ = State::Off; + event.state = State::Off; event.isSelected_ = false; - event.handledCnt_ += handled; - event.handled_ = false; - network_.enableLink(event.validator_, event.peer_, true); + event.handledCnt += handled; + event.handled = false; + network_.enableLink(event.validator, event.peer_, true); } }); auto& down = events[EventType::LinkDown]; auto& disconnected = events[EventType::PeerDisconnected]; // It's possible the last Down Link event is not handled - BEAST_EXPECT(down.handledCnt_ >= down.cnt_ - 1); + BEAST_EXPECT(down.handledCnt >= down.cnt - 1); // All Peer Disconnect events must be handled - BEAST_EXPECT(disconnected.cnt_ == disconnected.handledCnt_); + BEAST_EXPECT(disconnected.cnt == disconnected.handledCnt); if (log) - std::cout << "link down count: " << down.cnt_ << "/" << down.handledCnt_ - << " peer disconnect count: " << disconnected.cnt_ << "/" - << disconnected.handledCnt_; + { + std::cout << "link down count: " << down.cnt << "/" << down.handledCnt + << " peer disconnect count: " << disconnected.cnt << "/" + << disconnected.handledCnt; + } } bool @@ -1203,7 +1218,7 @@ protected: { BEAST_EXPECT( squelched == - MAX_PEERS - env_.app().config().vpReduceRelaySquelchMaxSelectedPeers); + maxPeers - env_.app().config().vpReduceRelaySquelchMaxSelectedPeers); squelchEvents++; } }, @@ -1272,8 +1287,7 @@ protected: network_.overlay().deletePeer( id, [&](PublicKey const& key, PeerWPtr const& peer) { unsquelched++; }); BEAST_EXPECT( - unsquelched == - MAX_PEERS - env_.app().config().vpReduceRelaySquelchMaxSelectedPeers); + unsquelched == maxPeers - env_.app().config().vpReduceRelaySquelchMaxSelectedPeers); BEAST_EXPECT(checkCounting(network_.validator(0), true)); }); } @@ -1294,8 +1308,7 @@ protected: [&](PublicKey const& key, PeerWPtr const& peer) { unsquelched++; }); BEAST_EXPECT( - unsquelched == - MAX_PEERS - env_.app().config().vpReduceRelaySquelchMaxSelectedPeers); + unsquelched == maxPeers - env_.app().config().vpReduceRelaySquelchMaxSelectedPeers); BEAST_EXPECT(checkCounting(network_.validator(0), true)); }); } @@ -1310,7 +1323,7 @@ protected: network_.overlay().clock().advance(seconds(601)); BEAST_EXPECT(propagateAndSquelch(log, true)); auto peers = network_.overlay().getPeers(network_.validator(0)); - auto it = std::find_if(peers.begin(), peers.end(), [&](auto it) { + auto it = std::ranges::find_if(peers, [&](auto it) { return it.second.state == reduce_relay::PeerState::Squelched; }); assert(it != peers.end()); @@ -1328,7 +1341,7 @@ protected: doTest("Test Config - squelch enabled (legacy)", log, [&](bool log) { Config c; - std::string toLoad(R"rippleConfig( + std::string const toLoad(R"rippleConfig( [reduce_relay] vp_enable=1 )rippleConfig"); @@ -1361,7 +1374,7 @@ vp_enable=0 doTest("Test Config - squelch enabled", log, [&](bool log) { Config c; - std::string toLoad(R"rippleConfig( + std::string const toLoad(R"rippleConfig( [reduce_relay] vp_base_squelch_enable=1 )rippleConfig"); @@ -1373,7 +1386,7 @@ vp_base_squelch_enable=1 doTest("Test Config - squelch disabled", log, [&](bool log) { Config c; - std::string toLoad(R"rippleConfig( + std::string const toLoad(R"rippleConfig( [reduce_relay] vp_base_squelch_enable=0 )rippleConfig"); @@ -1385,7 +1398,7 @@ vp_base_squelch_enable=0 doTest("Test Config - legacy and new", log, [&](bool log) { Config c; - std::string toLoad(R"rippleConfig( + std::string const toLoad(R"rippleConfig( [reduce_relay] vp_base_squelch_enable=0 vp_enable=0 @@ -1495,7 +1508,7 @@ vp_base_squelch_max_selected_peers=2 std::int16_t nMessages = 5; for (int i = 0; i < nMessages; i++) { - uint256 key(i); + uint256 const key(i); network_.overlay().updateSlotAndSquelch( key, network_.validator(0), 0, [&](PublicKey const&, PeerWPtr, std::uint32_t) { }); @@ -1505,7 +1518,7 @@ vp_base_squelch_max_selected_peers=2 // hence '-1'. BEAST_EXPECT(peers[0].count == (nMessages - 1)); // add duplicate - uint256 key(nMessages - 1); + uint256 const key(nMessages - 1); network_.overlay().updateSlotAndSquelch( key, network_.validator(0), 0, [&](PublicKey const&, PeerWPtr, std::uint32_t) {}); // confirm the same number of messages @@ -1523,14 +1536,13 @@ vp_base_squelch_max_selected_peers=2 struct Handler : public reduce_relay::SquelchHandler { - Handler() : maxDuration_(0) + Handler() { } void squelch(PublicKey const&, Peer::id_t, std::uint32_t duration) const override { - if (duration > maxDuration_) - maxDuration_ = duration; + maxDuration_ = std::max(duration, maxDuration_); } void @@ -1542,7 +1554,7 @@ vp_base_squelch_max_selected_peers=2 unsquelch(PublicKey const&, Peer::id_t) const override { } - mutable int maxDuration_; + mutable int maxDuration_{0}; }; void @@ -1566,7 +1578,7 @@ vp_base_squelch_max_selected_peers=2 { // make unique message hash to make the // slot's internal hash router accept the message - std::uint64_t mid = m * 1000 + peer; + std::uint64_t mid = (m * 1000) + peer; uint256 const message{mid}; slots.updateSlotAndSquelch(message, validator, peer, true); } @@ -1598,18 +1610,22 @@ vp_base_squelch_max_selected_peers=2 handler.maxDuration_ <= kMaxUnsquelchExpirePeers.count()); using namespace beast::unit_test::detail; if (handler.maxDuration_ <= kMaxUnsquelchExpireDefault.count()) + { log << makeReason("warning: squelch duration is low", __FILE__, __LINE__) << std::endl << std::flush; + } // more than 400 is still less than kMaxUnsquelchExpirePeers run(400); BEAST_EXPECT( handler.maxDuration_ >= kMinUnsquelchExpire.count() && handler.maxDuration_ <= kMaxUnsquelchExpirePeers.count()); if (handler.maxDuration_ <= kMaxUnsquelchExpireDefault.count()) + { log << makeReason("warning: squelch duration is low", __FILE__, __LINE__) << std::endl << std::flush; + } }); } @@ -1631,7 +1647,7 @@ vp_base_squelch_max_selected_peers=2 env_.app().config().compression = c.compression; }; auto handshake = [&](int outboundEnable, int inboundEnable) { - beast::IP::Address addr = boost::asio::ip::make_address("172.1.1.100"); + beast::IP::Address const addr = boost::asio::ip::make_address("172.1.1.100"); setEnv(outboundEnable); auto request = makeRequest( @@ -1640,25 +1656,25 @@ vp_base_squelch_max_selected_peers=2 false, env_.app().config().txReduceRelayEnable, env_.app().config().vpReduceRelayBaseSquelchEnable); - http_request_type http_request; - http_request.version(request.version()); - http_request.base() = request.base(); + http_request_type httpRequest; + httpRequest.version(request.version()); + httpRequest.base() = request.base(); // feature enabled on the peer's connection only if both sides // are enabled auto const peerEnabled = inboundEnable && outboundEnable; // inbound is enabled if the request's header has the feature // enabled and the peer's configuration is enabled auto const inboundEnabled = - peerFeatureEnabled(http_request, kFeatureVprr, inboundEnable); + peerFeatureEnabled(httpRequest, kFeatureVprr, inboundEnable); BEAST_EXPECT(!(peerEnabled ^ inboundEnabled)); setEnv(inboundEnable); - auto http_resp = - makeResponse(true, http_request, addr, addr, uint256{1}, 1, {1, 0}, env_.app()); + auto httpResp = + makeResponse(true, httpRequest, addr, addr, uint256{1}, 1, {1, 0}, env_.app()); // outbound is enabled if the response's header has the feature // enabled and the peer's configuration is enabled auto const outboundEnabled = - peerFeatureEnabled(http_resp, kFeatureVprr, outboundEnable); + peerFeatureEnabled(httpResp, kFeatureVprr, outboundEnable); BEAST_EXPECT(!(peerEnabled ^ outboundEnabled)); }; handshake(1, 1); @@ -1685,7 +1701,7 @@ public: void run() override { - bool log = false; + bool const log = false; testConfig(log); testInitialRound(log); testPeerUnsquelchedTooSoon(log); @@ -1712,7 +1728,7 @@ class base_squelch_simulate_test : public base_squelch_test void run() override { - bool log = false; + bool const log = false; testRandom(log); } }; diff --git a/src/test/overlay/enhanced_squelch_test.cpp b/src/test/overlay/enhanced_squelch_test.cpp index c96050d22c..fb38faaf1c 100644 --- a/src/test/overlay/enhanced_squelch_test.cpp +++ b/src/test/overlay/enhanced_squelch_test.cpp @@ -1,29 +1,17 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2025 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - #include +#include #include #include #include -#include +#include +#include +#include +#include +#include +#include +#include #include #include @@ -31,6 +19,8 @@ #include #include #include +#include +#include #include namespace xrpl::test { @@ -47,19 +37,18 @@ public: squelchAll_method squelchAll_f_; unsquelch_method unsquelch_f_; - TestHandler( - squelch_method const& squelch_f, - squelchAll_method const& squelchAll_f, - unsquelch_method const& unsquelch_f) - : squelch_f_(squelch_f), squelchAll_f_(squelchAll_f), unsquelch_f_(unsquelch_f) + TestHandler(squelch_method squelchF, squelchAll_method squelchAllF, unsquelch_method unsquelchF) + : squelch_f_(std::move(squelchF)) + , squelchAll_f_(std::move(squelchAllF)) + , unsquelch_f_(std::move(unsquelchF)) { } TestHandler(TestHandler& copy) + : squelch_f_(copy.squelch_f_) + , squelchAll_f_(copy.squelchAll_f_) + , unsquelch_f_(copy.unsquelch_f_) { - squelch_f_ = copy.squelch_f_; - squelchAll_f_ = copy.squelchAll_f_; - unsquelch_f_ = copy.unsquelch_f_; } void @@ -141,31 +130,31 @@ public: class enhanced_squelch_test : public beast::unit_test::Suite { public: - TestHandler::squelch_method noop_squelch = [&](PublicKey const&, Peer::id_t, std::uint32_t) { + TestHandler::squelch_method noopSquelch = [&](PublicKey const&, Peer::id_t, std::uint32_t) { BEAST_EXPECTS(false, "unexpected call to squelch handler"); }; - TestHandler::squelchAll_method noop_squelchAll = + TestHandler::squelchAll_method noopSquelchAll = [&](PublicKey const&, std::uint32_t, std::function) { BEAST_EXPECTS(false, "unexpected call to squelchAll handler"); }; - TestHandler::unsquelch_method noop_unsquelch = [&](PublicKey const&, Peer::id_t) { + TestHandler::unsquelch_method noopUnsquelch = [&](PublicKey const&, Peer::id_t) { BEAST_EXPECTS(false, "unexpected call to unsquelch handler"); }; // noop_handler is passed as a place holder Handler to slots - TestHandler noop_handler = { - noop_squelch, - noop_squelchAll, - noop_unsquelch, + TestHandler noopHandler = { + noopSquelch, + noopSquelchAll, + noopUnsquelch, }; - jtx::Env env_; + jtx::Env env; - enhanced_squelch_test() : env_(*this) + enhanced_squelch_test() : env(*this) { - env_.app().config().vpReduceRelayEnhancedSquelchEnable = true; + env.app().config().vpReduceRelayEnhancedSquelchEnable = true; } void @@ -208,7 +197,7 @@ vp_enhanced_squelch_enable=0 Peer::id_t const squelchedPeerID = 0; Peer::id_t const newPeerID = 1; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), noop_handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), noopHandler, env.app().config(), stopwatch); auto const publicKey = randomKeyPair(KeyType::Ed25519).first; // a new key should not be squelched @@ -237,11 +226,11 @@ vp_enhanced_squelch_enable=0 } void - testUpdateValidatorSlot_newValidator() + testUpdateValidatorSlotNewValidator() { testcase("updateValidatorSlot_newValidator"); TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), noop_handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), noopHandler, env.app().config(), stopwatch); Peer::id_t const peerID = 1; auto const validator = randomKeyPair(KeyType::Ed25519).first; @@ -250,10 +239,10 @@ vp_enhanced_squelch_enable=0 slots.updateUntrustedValidatorSlot(message, validator, peerID); // adding untrusted slot does not effect trusted slots - BEAST_EXPECTS(slots.getSlots(true).size() == 0, "trusted slots changed"); + BEAST_EXPECTS(slots.getSlots(true).empty(), "trusted slots changed"); // we expect that the validator was not added to untrusted slots - BEAST_EXPECTS(slots.getSlots(false).size() == 0, "untrusted slot changed"); + BEAST_EXPECTS(slots.getSlots(false).empty(), "untrusted slot changed"); // we expect that the validator was added to th consideration list BEAST_EXPECTS( @@ -262,7 +251,7 @@ vp_enhanced_squelch_enable=0 } void - testUpdateValidatorSlot_squelchedValidator() + testUpdateValidatorSlotSquelchedValidator() { testcase("testUpdateValidatorSlot_squelchedValidator"); @@ -270,17 +259,17 @@ vp_enhanced_squelch_enable=0 Peer::id_t const newPeerID = 1; auto const validator = randomKeyPair(KeyType::Ed25519).first; - TestHandler::squelch_method const squelch_f = + TestHandler::squelch_method const squelchF = [&](PublicKey const& key, Peer::id_t id, std::uint32_t duration) { BEAST_EXPECTS(key == validator, "squelch called for unknown validator key"); BEAST_EXPECTS(id == newPeerID, "squelch called for the wrong peer"); }; - TestHandler handler{squelch_f, noop_squelchAll, noop_unsquelch}; + TestHandler handler{squelchF, noopSquelchAll, noopUnsquelch}; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), handler, env.app().config(), stopwatch); slots.squelchValidator(validator, squelchedPeerID); @@ -302,29 +291,33 @@ vp_enhanced_squelch_enable=0 } void - testUpdateValidatorSlot_slotsFull() + testUpdateValidatorSlotSlotsFull() { testcase("updateValidatorSlot_slotsFull"); Peer::id_t const peerID = 1; // while there are open untrusted slots, no calls should be made to // squelch any validators - TestHandler handler{noop_handler}; + TestHandler handler{noopHandler}; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), handler, env.app().config(), stopwatch); // saturate validator slots auto const validators = fillUntrustedSlots(slots); // adding untrusted slot does not effect trusted slots - BEAST_EXPECTS(slots.getSlots(true).size() == 0, "trusted slots changed"); + BEAST_EXPECTS(slots.getSlots(true).empty(), "trusted slots changed"); // simulate additional messages from already selected validators for (auto const& validator : validators) + { for (int i = 0; i < reduce_relay::kMaxMessageThreshold; ++i) + { slots.updateUntrustedValidatorSlot( sha512Half(validator) + static_cast(i), validator, peerID); + } + } // an untrusted slot was added for each validator BEAST_EXPECT(slots.getSlots(false).size() == reduce_relay::kMaxUntrustedSlots); @@ -352,13 +345,13 @@ vp_enhanced_squelch_enable=0 } void - testDeleteIdlePeers_deleteIdleSlots() + testDeleteIdlePeersDeleteIdleSlots() { testcase("deleteIdlePeers"); - TestHandler handler{noop_handler}; + TestHandler handler{noopHandler}; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), handler, env.app().config(), stopwatch); auto keys = fillUntrustedSlots(slots); // verify that squelchAll is called for each idled slot validator @@ -385,20 +378,20 @@ vp_enhanced_squelch_enable=0 slots.deleteIdlePeers(); - BEAST_EXPECTS(slots.getSlots(false).size() == 0, "unexpected number of untrusted slots"); + BEAST_EXPECTS(slots.getSlots(false).empty(), "unexpected number of untrusted slots"); BEAST_EXPECTS(keys.empty(), "not all validators were squelched"); } void - testDeleteIdlePeers_deleteIdleUntrustedPeer() + testDeleteIdlePeersDeleteIdleUntrustedPeer() { testcase("deleteIdleUntrustedPeer"); Peer::id_t const peerID = 1; Peer::id_t const peerID2 = 2; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), noop_handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), noopHandler, env.app().config(), stopwatch); // fill one untrusted validator slot auto const validator = fillUntrustedSlots(slots, 1)[0]; @@ -426,21 +419,22 @@ vp_enhanced_squelch_enable=0 * updateSlotAndSquelch */ void - testUpdateSlotAndSquelch_untrustedValidator() + testUpdateSlotAndSquelchUntrustedValidator() { testcase("updateUntrsutedValidatorSlot"); - TestHandler handler{noop_handler}; + TestHandler handler{noopHandler}; handler.squelch_f_ = [](PublicKey const&, Peer::id_t, std::uint32_t) {}; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), handler, env.app().config(), stopwatch); // peers that will be source of validator messages std::vector peers = {}; // prepare n+1 peers, we expect the n+1st peer will be squelched - for (int i = 0; i < env_.app().config().vpReduceRelaySquelchMaxSelectedPeers + 1; ++i) + peers.reserve(env_.app().config().vpReduceRelaySquelchMaxSelectedPeers + 1); + for (int i = 0; i < env.app().config().vpReduceRelaySquelchMaxSelectedPeers + 1; ++i) peers.push_back(i); auto const validator = fillUntrustedSlots(slots, 1)[0]; @@ -462,6 +456,7 @@ vp_enhanced_squelch_enable=0 // simulate new, unique validator messages sent by peers for (auto const& peer : peers) + { for (int i = 0; i < reduce_relay::kMaxMessageThreshold + 1; ++i) { auto const now = stopwatch.now(); @@ -473,10 +468,11 @@ vp_enhanced_squelch_enable=0 stopwatch.advance(std::chrono::milliseconds{10}); } + } auto const slotPeers = getUntrustedSlotPeers(validator, slots); BEAST_EXPECTS( - slotPeers.size() == env_.app().config().vpReduceRelaySquelchMaxSelectedPeers + 1, + slotPeers.size() == env.app().config().vpReduceRelaySquelchMaxSelectedPeers + 1, "untrusted validator slot is missing"); int selected = 0; @@ -498,16 +494,16 @@ vp_enhanced_squelch_enable=0 BEAST_EXPECTS(squelched == 1, "expected one squelched peer"); BEAST_EXPECTS( - selected == env_.app().config().vpReduceRelaySquelchMaxSelectedPeers, + selected == env.app().config().vpReduceRelaySquelchMaxSelectedPeers, "wrong number of peers selected"); } void - testUpdateConsideredValidator_new() + testUpdateConsideredValidatorNew() { testcase("testUpdateConsideredValidator_new"); TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), noop_handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), noopHandler, env.app().config(), stopwatch); // insert some random validator key auto const validator = randomKeyPair(KeyType::Ed25519).first; @@ -542,15 +538,15 @@ vp_enhanced_squelch_enable=0 } void - testUpdateConsideredValidator_idle() + testUpdateConsideredValidatorIdle() { testcase("testUpdateConsideredValidator_idle"); TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), noop_handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), noopHandler, env.app().config(), stopwatch); // insert some random validator key auto const validator = randomKeyPair(KeyType::Ed25519).first; - Peer::id_t peerID = 0; + Peer::id_t const peerID = 0; BEAST_EXPECTS( !slots.updateConsideredValidator(validator, peerID), @@ -581,16 +577,16 @@ vp_enhanced_squelch_enable=0 } void - testUpdateConsideredValidator_selectQualifying() + testUpdateConsideredValidatorSelectQualifying() { testcase("testUpdateConsideredValidator_selectQualifying"); TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), noop_handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), noopHandler, env.app().config(), stopwatch); // insert some random validator key auto const validator = randomKeyPair(KeyType::Ed25519).first; - Peer::id_t peerID = 0; + Peer::id_t const peerID = 0; for (int i = 0; i < reduce_relay::kMaxMessageThreshold - 1; ++i) { @@ -613,27 +609,31 @@ vp_enhanced_squelch_enable=0 } void - testCleanConsideredValidators_resetIdle() + testCleanConsideredValidatorsResetIdle() { testcase("testCleanConsideredValidators_resetIdle"); auto const validator = randomKeyPair(KeyType::Ed25519).first; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), noop_handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), noopHandler, env.app().config(), stopwatch); // send enough messages for a slot to meet peer requirements - for (int i = 0; i < env_.app().config().vpReduceRelaySquelchMaxSelectedPeers; ++i) + for (int i = 0; i < env.app().config().vpReduceRelaySquelchMaxSelectedPeers; ++i) + { slots.updateUntrustedValidatorSlot( sha512Half(validator) + static_cast(i), validator, i); + } // send enough messages from some peer to be one message away from // meeting the selection criteria for (int i = 0; i < reduce_relay::kMaxMessageThreshold - - (env_.app().config().vpReduceRelaySquelchMaxSelectedPeers + 1); + (env.app().config().vpReduceRelaySquelchMaxSelectedPeers + 1); ++i) + { slots.updateUntrustedValidatorSlot( sha512Half(validator) + static_cast(i), validator, 0); + } BEAST_EXPECTS( slots.getConsideredValidators().at(validator).count == @@ -642,7 +642,7 @@ vp_enhanced_squelch_enable=0 BEAST_EXPECTS( slots.getConsideredValidators().at(validator).peers.size() == - env_.app().config().vpReduceRelaySquelchMaxSelectedPeers, + env.app().config().vpReduceRelaySquelchMaxSelectedPeers, "considered validator information is in an invalid state"); stopwatch.advance(reduce_relay::kIdled + std::chrono::seconds{1}); @@ -654,7 +654,7 @@ vp_enhanced_squelch_enable=0 sha512Half(validator) + static_cast(1), validator, 0); // we expect that the validator was not selected - BEAST_EXPECTS(slots.getSlots(false).size() == 0, "untrusted slot was created"); + BEAST_EXPECTS(slots.getSlots(false).empty(), "untrusted slot was created"); BEAST_EXPECTS( slots.getConsideredValidators().at(validator).count == 1, @@ -666,12 +666,12 @@ vp_enhanced_squelch_enable=0 } void - testCleanConsideredValidators_deletePoorlyConnected() + testCleanConsideredValidatorsDeletePoorlyConnected() { testcase("cleanConsideredValidators_deletePoorlyConnected"); auto const validator = randomKeyPair(KeyType::Ed25519).first; Peer::id_t const peerID = 0; - TestHandler handler{noop_handler}; + TestHandler handler{noopHandler}; // verify that squelchAll is called for poorly connected validator handler.squelchAll_f_ = [&](PublicKey const& actualKey, @@ -683,12 +683,14 @@ vp_enhanced_squelch_enable=0 TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), handler, env.app().config(), stopwatch); // send enough messages from a single peer - for (int i = 0; i < 2 * reduce_relay::kMaxMessageThreshold + 1; ++i) + for (int i = 0; i < (2 * reduce_relay::kMaxMessageThreshold) + 1; ++i) + { slots.updateUntrustedValidatorSlot( sha512Half(validator) + static_cast(i), validator, peerID); + } stopwatch.advance(reduce_relay::kIdled + std::chrono::seconds{1}); @@ -697,12 +699,11 @@ vp_enhanced_squelch_enable=0 slots.deleteIdlePeers(); BEAST_EXPECTS( - slots.getConsideredValidators().size() == 0, - "poorly connected validator was not deleted"); + slots.getConsideredValidators().empty(), "poorly connected validator was not deleted"); } void - testCleanConsideredValidators_deleteSilent() + testCleanConsideredValidatorsDeleteSilent() { testcase("cleanConsideredValidators_deleteSilent"); // insert some random validator key @@ -710,7 +711,7 @@ vp_enhanced_squelch_enable=0 auto const validator = randomKeyPair(KeyType::Ed25519).first; Peer::id_t const peerID = 0; - TestHandler handler{noop_handler}; + TestHandler handler{noopHandler}; // verify that squelchAll is called for idle validator handler.squelchAll_f_ = [&](PublicKey const& actualKey, @@ -722,7 +723,7 @@ vp_enhanced_squelch_enable=0 TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), handler, env.app().config(), stopwatch); BEAST_EXPECTS( !slots.updateConsideredValidator(idleValidator, peerID), @@ -748,14 +749,14 @@ vp_enhanced_squelch_enable=0 } void - testSquelchUntrustedValidator_consideredListCleared() + testSquelchUntrustedValidatorConsideredListCleared() { testcase("testSquelchUntrustedValidator"); auto const validator = randomKeyPair(KeyType::Ed25519).first; Peer::id_t const peerID = 0; - TestHandler handler{noop_handler}; + TestHandler handler{noopHandler}; // verify that squelchAll is called for idle validator handler.squelchAll_f_ = [&](PublicKey const& actualKey, std::uint32_t duration, @@ -764,7 +765,7 @@ vp_enhanced_squelch_enable=0 }; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), handler, env.app().config(), stopwatch); // add the validator to the considered list slots.updateUntrustedValidatorSlot(sha512Half(validator), validator, peerID); @@ -781,13 +782,13 @@ vp_enhanced_squelch_enable=0 } void - testSquelchUntrustedValidator_slotEvicted() + testSquelchUntrustedValidatorSlotEvicted() { testcase("testSquelchUntrustedValidator_slotEvicted"); - TestHandler handler{noop_handler}; + TestHandler handler{noopHandler}; TestStopwatch stopwatch; - EnhancedSquelchingTestSlots slots(env_.app(), handler, env_.app().config(), stopwatch); + EnhancedSquelchingTestSlots slots(env.app(), handler, env.app().config(), stopwatch); // assign a slot to the untrusted validator auto const validators = fillUntrustedSlots(slots, 1); @@ -823,17 +824,21 @@ private: { auto const validator = randomKeyPair(KeyType::Ed25519).first; keys.push_back(validator); - for (int j = 0; j < env_.app().config().vpReduceRelaySquelchMaxSelectedPeers; ++j) + for (int j = 0; j < env.app().config().vpReduceRelaySquelchMaxSelectedPeers; ++j) + { // send enough messages so that a validator slot is selected for (int k = 0; k < reduce_relay::kMaxMessageThreshold; ++k) + { slots.updateUntrustedValidatorSlot( sha512Half(validator) + static_cast(k), validator, j); + } + } } return keys; } - std::unordered_map + static std::unordered_map getUntrustedSlotPeers(PublicKey const& validator, EnhancedSquelchingTestSlots const& slots) { auto const& it = slots.getSlots(false).find(validator); @@ -843,7 +848,7 @@ private: auto r = std::unordered_map(); for (auto const& [id, info] : it->second.getPeers()) - r.emplace(std::make_pair(id, info)); + r.emplace(id, info); return r; } @@ -853,20 +858,20 @@ private: { testConfig(); testSquelchTracking(); - testUpdateValidatorSlot_newValidator(); - testUpdateValidatorSlot_slotsFull(); - testUpdateValidatorSlot_squelchedValidator(); - testDeleteIdlePeers_deleteIdleSlots(); - testDeleteIdlePeers_deleteIdleUntrustedPeer(); - testUpdateSlotAndSquelch_untrustedValidator(); - testUpdateConsideredValidator_new(); - testUpdateConsideredValidator_idle(); - testUpdateConsideredValidator_selectQualifying(); - testCleanConsideredValidators_deleteSilent(); - testCleanConsideredValidators_resetIdle(); - testCleanConsideredValidators_deletePoorlyConnected(); - testSquelchUntrustedValidator_consideredListCleared(); - testSquelchUntrustedValidator_slotEvicted(); + testUpdateValidatorSlotNewValidator(); + testUpdateValidatorSlotSlotsFull(); + testUpdateValidatorSlotSquelchedValidator(); + testDeleteIdlePeersDeleteIdleSlots(); + testDeleteIdlePeersDeleteIdleUntrustedPeer(); + testUpdateSlotAndSquelchUntrustedValidator(); + testUpdateConsideredValidatorNew(); + testUpdateConsideredValidatorIdle(); + testUpdateConsideredValidatorSelectQualifying(); + testCleanConsideredValidatorsDeleteSilent(); + testCleanConsideredValidatorsResetIdle(); + testCleanConsideredValidatorsDeletePoorlyConnected(); + testSquelchUntrustedValidatorConsideredListCleared(); + testSquelchUntrustedValidatorSlotEvicted(); } }; diff --git a/src/test/overlay/squelch_store_test.cpp b/src/test/overlay/squelch_store_test.cpp index 031e07a35d..a579871eef 100644 --- a/src/test/overlay/squelch_store_test.cpp +++ b/src/test/overlay/squelch_store_test.cpp @@ -1,29 +1,15 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2025 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - #include #include #include -#include +#include +#include +#include +#include +#include #include +#include #include @@ -49,9 +35,9 @@ class squelch_store_test : public beast::unit_test::Suite using seconds = std::chrono::seconds; public: - jtx::Env env_; + jtx::Env env; - squelch_store_test() : env_(*this) + squelch_store_test() : env(*this) { } @@ -61,7 +47,7 @@ public: testcase("SquelchStore handleSquelch"); TestStopwatch clock; - auto store = TestSquelchStore(env_.journal, clock); + auto store = TestSquelchStore(env.journal, clock); auto const validator = randomKeyPair(KeyType::Ed25519).first; @@ -94,7 +80,7 @@ public: { testcase("SquelchStore IsSquelched"); TestStopwatch clock; - auto store = TestSquelchStore(env_.journal, clock); + auto store = TestSquelchStore(env.journal, clock); auto const validator = randomKeyPair(KeyType::Ed25519).first; auto const duration = reduce_relay::kMinUnsquelchExpire + seconds{1}; @@ -113,7 +99,7 @@ public: { testcase("SquelchStore testClearExpiredSquelches"); TestStopwatch clock; - auto store = TestSquelchStore(env_.journal, clock); + auto store = TestSquelchStore(env.journal, clock); auto const validator = randomKeyPair(KeyType::Ed25519).first; auto const duration = reduce_relay::kMinUnsquelchExpire + seconds{1}; diff --git a/src/xrpld/overlay/Slot.h b/src/xrpld/overlay/Slot.h index ddc2913b3f..ec533cf615 100644 --- a/src/xrpld/overlay/Slot.h +++ b/src/xrpld/overlay/Slot.h @@ -2,11 +2,11 @@ #include #include -#include #include #include -#include +#include +#include #include #include #include @@ -14,9 +14,16 @@ #include #include +#include +#include +#include +#include #include #include #include +#include +#include +#include namespace xrpl::reduce_relay { @@ -75,9 +82,7 @@ to_string(SlotState state) class SquelchHandler { public: - virtual ~SquelchHandler() - { - } + virtual ~SquelchHandler() = default; /** * Squelch handler for a single peer * @param validator Public key of the source validator @@ -142,11 +147,11 @@ public: */ struct PeerInfo { - PeerState state; // peer's state - std::size_t count; // message count - time_point expire; // squelch expiration time - time_point lastMessage; // time last message received - std::size_t timesSelected; // number of times the peer was selected + PeerState state; // peer's state + std::size_t count{}; // message count + time_point expire; // squelch expiration time + time_point lastMessage; // time last message received + std::size_t timesSelected{}; // number of times the peer was selected }; /** @@ -183,9 +188,7 @@ private: uint16_t maxSelectedPeers, bool isTrusted, clock_type& clock) - : reachedThreshold_(0) - , lastSelected_(clock.now()) - , state_(SlotState::Counting) + : lastSelected_(clock.now()) , handler_(handler) , journal_(journal) , maxSelectedPeers_(maxSelectedPeers) @@ -333,14 +336,14 @@ private: std::unordered_set considered_; // A counter of peers that have reached the max message threshold. - std::uint16_t reachedThreshold_; + std::uint16_t reachedThreshold_{0}; // The timestamp of the last peer selection, used to determine when the // current selection has become stale. time_point lastSelected_; // The current state of the slot. - SlotState state_; + SlotState state_{SlotState::Counting}; // A reference to an external handler that executes squelch/unsquelch // operations on peers. @@ -532,7 +535,7 @@ public: uint256 const& key, PublicKey const& validator, Peer::id_t id, - typename Slot::ignored_squelch_callback report); + Slot::ignored_squelch_callback report); /** * Calls Slot::update of Slot associated with the validator, with a @@ -586,7 +589,7 @@ public: uint256 const& key, PublicKey const& validator, Peer::id_t id, - typename Slot::ignored_squelch_callback report, + Slot::ignored_squelch_callback report, bool isTrusted); /** @@ -611,8 +614,7 @@ public: squelchUntrustedValidator(PublicKey const& validatorKey); /** - * @brief Performs cleanup of idle peers, stale slots, and unviable - * validator candidates. + * @brief Performs cleanup of idle peers, stale slots, and inviable validator candidates. * * @details This function is responsible for the health of the entire slot * system. It executes three distinct cleanup phases: diff --git a/src/xrpld/overlay/SquelchStore.h b/src/xrpld/overlay/SquelchStore.h index a4a646dc97..8f0fe057c7 100644 --- a/src/xrpld/overlay/SquelchStore.h +++ b/src/xrpld/overlay/SquelchStore.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include #include @@ -23,7 +22,7 @@ namespace xrpl::reduce_relay { class SquelchStore { using clock_type = beast::AbstractClock; - using time_point = typename clock_type::time_point; + using time_point = clock_type::time_point; public: explicit SquelchStore(beast::Journal journal, clock_type& clock) diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index ff1e488467..af72006228 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -1543,7 +1543,7 @@ OverlayImpl::handleUntrustedSquelch(PublicKey const& validator) { if (!strand_.running_in_this_thread()) { - post(strand_, std::bind(&OverlayImpl::handleUntrustedSquelch, this, validator)); + post(strand_, [this, validator] { handleUntrustedSquelch(validator); }); return; } diff --git a/src/xrpld/overlay/detail/OverlayImpl.h b/src/xrpld/overlay/detail/OverlayImpl.h index 18ed14abdc..5de7603d9f 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.h +++ b/src/xrpld/overlay/detail/OverlayImpl.h @@ -14,7 +14,6 @@ #include #include -#include #include #include #include diff --git a/src/xrpld/overlay/detail/PeerImp.h b/src/xrpld/overlay/detail/PeerImp.h index 2776c0f071..ef3ef6da68 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -15,8 +15,8 @@ #include #include #include -#include #include +#include #include #include #include diff --git a/src/xrpld/overlay/detail/Slot.cpp b/src/xrpld/overlay/detail/Slot.cpp index 7deb208f28..d2a78519d4 100644 --- a/src/xrpld/overlay/detail/Slot.cpp +++ b/src/xrpld/overlay/detail/Slot.cpp @@ -1,22 +1,3 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2025 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - #include #include @@ -24,18 +5,23 @@ #include #include -#include +#include #include -#include #include #include +#include #include +#include +#include #include #include +#include #include #include #include +#include +#include #include namespace xrpl::reduce_relay { @@ -73,14 +59,13 @@ Slot::update(PublicKey const& validator, Peer::id_t id, ignored_squelch_callback { JLOG(journal_.trace()) << "update: adding new slot" << formatLogMessage(validator, id); peers_.emplace( - std::make_pair( - id, - PeerInfo{ - .state = PeerState::Counting, - .count = 0, - .expire = now, - .lastMessage = now, - .timesSelected = 0})); + id, + PeerInfo{ + .state = PeerState::Counting, + .count = 0, + .expire = now, + .lastMessage = now, + .timesSelected = 0}); initCounting(); return; } @@ -129,7 +114,7 @@ Slot::update(PublicKey const& validator, Peer::id_t id, ignored_squelch_callback // idled peers. std::unordered_set selected; std::stringstream str; - while (selected.size() != maxSelectedPeers_ && considered_.size() != 0) + while (selected.size() != maxSelectedPeers_ && !considered_.empty()) { auto const i = considered_.size() == 1 ? 0 : randInt(considered_.size() - 1); auto const it = std::next(considered_.begin(), i); @@ -176,7 +161,7 @@ Slot::update(PublicKey const& validator, Peer::id_t id, ignored_squelch_callback { v.count = 0; - if (selected.find(k) != selected.end()) + if (selected.contains(k)) { v.state = PeerState::Selected; ++v.timesSelected; @@ -187,7 +172,7 @@ Slot::update(PublicKey const& validator, Peer::id_t id, ignored_squelch_callback if (journal_.trace()) str << k << " "; v.state = PeerState::Squelched; - std::chrono::seconds duration = + std::chrono::seconds const duration = getSquelchDuration(peers_.size() - maxSelectedPeers_); v.expire = now + duration; handler_.squelch(validator, k, duration.count()); @@ -230,7 +215,7 @@ Slot::deletePeer(PublicKey const& validator, Peer::id_t id, bool erase) JLOG(journal_.debug()) << "deletePeer: unsquelching selected peer " << formatLogMessage(validator, id) << " peer_state: " << to_string(it->second.state) - << " considered: " << (considered_.find(id) != considered_.end()) + << " considered: " << (considered_.contains(id)) << " erase: " << erase; for (auto& [k, v] : peers_) @@ -320,8 +305,10 @@ bool Slots::reduceRelayReady() { if (!reduceRelayReady_) + { reduceRelayReady_ = std::chrono::duration_cast( clock_.now().time_since_epoch()) > reduce_relay::kWaitOnBootup; + } return reduceRelayReady_; } @@ -352,7 +339,7 @@ Slots::expireAndIsPeerSquelched(PublicKey const& validatorKey, Peer::id_t peerID return false; // if a peer is found the squelch for it has not expired - return it->second.find(peerID) != it->second.end(); + return it->second.contains(peerID); } bool @@ -372,7 +359,7 @@ Slots::updateSlotAndSquelch( uint256 const& key, PublicKey const& validator, Peer::id_t id, - typename Slot::ignored_squelch_callback report, + Slot::ignored_squelch_callback report, bool isTrusted) { if (expireAndIsPeerMessageCached(key, id)) @@ -414,11 +401,11 @@ Slots::updateUntrustedValidatorSlot( uint256 const& key, PublicKey const& validator, Peer::id_t id, - typename Slot::ignored_squelch_callback report) + Slot::ignored_squelch_callback report) { // We received a message from an already selected validator // we can ignore this message - if (untrustedSlots_.find(validator) != untrustedSlots_.end()) + if (untrustedSlots_.contains(validator)) return; // Did we receive a message from an already squelched validator? @@ -567,15 +554,21 @@ Slots::deleteIdlePeers() // if an untrusted validator slot idled - peers stopped // sending messages for this validator squelch it if (!it->second.isTrusted_) + { handler_.squelchAll( it->first, reduce_relay::kMaxUnsquelchExpireDefault.count(), [&](Peer::id_t id) { registerSquelchedValidator(it->first, id); }); + } it = slots.erase(it); } else - ++it; + { + { + ++it; + } + } } }; @@ -586,10 +579,12 @@ Slots::deleteIdlePeers() // there might be some good validators in this set that "lapsed". // However, since these are untrusted validators we're not concerned for (auto const& validator : cleanConsideredValidators()) + { handler_.squelchAll( validator, reduce_relay::kMaxUnsquelchExpireDefault.count(), [&](Peer::id_t id) { registerSquelchedValidator(validator, id); }); + } } std::vector @@ -614,10 +609,14 @@ Slots::cleanConsideredValidators() ++it; } else - ++it; + { + { + ++it; + } + } } - if (keys.size() > 0) + if (!keys.empty()) { JLOG(journal_.info()) << "cleanConsideredValidators: removed considered validators " << ss.str(); diff --git a/src/xrpld/overlay/detail/SquelchStore.cpp b/src/xrpld/overlay/detail/SquelchStore.cpp index 9f756ba047..485aeaf8c9 100644 --- a/src/xrpld/overlay/detail/SquelchStore.cpp +++ b/src/xrpld/overlay/detail/SquelchStore.cpp @@ -1,27 +1,9 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2025 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - #include #include #include +#include #include #include