diff --git a/src/test/overlay/enhanced_squelch_test.cpp b/src/test/overlay/enhanced_squelch_test.cpp index 8cd16122ca..5e57bb983f 100644 --- a/src/test/overlay/enhanced_squelch_test.cpp +++ b/src/test/overlay/enhanced_squelch_test.cpp @@ -210,8 +210,8 @@ vp_enhanced_squelch_enable=0 testSquelchTracking() { testcase("squelchTracking"); - Peer::id_t squelchedPeerID = 0; - Peer::id_t newPeerID = 1; + Peer::id_t const squelchedPeerID = 0; + Peer::id_t const newPeerID = 1; TestStopwatch stopwatch; EnhancedSquelchingTestSlots slots( env_.app().logs(), noop_handler, env_.app().config(), stopwatch); @@ -262,7 +262,7 @@ vp_enhanced_squelch_enable=0 Peer::id_t const peerID = 1; auto const validator = randomKeyPair(KeyType::ed25519).first; - uint256 message{0}; + uint256 const message{0}; slots.updateUntrustedValidatorSlot(message, validator, peerID); @@ -285,8 +285,8 @@ vp_enhanced_squelch_enable=0 { testcase("testUpdateValidatorSlot_squelchedValidator"); - Peer::id_t squelchedPeerID = 0; - Peer::id_t newPeerID = 1; + Peer::id_t const squelchedPeerID = 0; + Peer::id_t const newPeerID = 1; auto const validator = randomKeyPair(KeyType::ed25519).first; TestHandler::squelch_method const squelch_f = @@ -361,8 +361,7 @@ vp_enhanced_squelch_enable=0 // an untrusted slot was added for each validator BEAST_EXPECT( - slots.getSlots(false).size() == - env_.app().config().VP_REDUCE_RELAY_SQUELCH_MAX_SELECTED_PEERS); + slots.getSlots(false).size() == reduce_relay::MAX_UNTRUSTED_SLOTS); for (auto const& validator : validators) BEAST_EXPECTS( @@ -420,8 +419,7 @@ vp_enhanced_squelch_enable=0 }; BEAST_EXPECTS( - slots.getSlots(false).size() == - env_.app().config().VP_REDUCE_RELAY_SQUELCH_MAX_SELECTED_PEERS, + slots.getSlots(false).size() == reduce_relay::MAX_UNTRUSTED_SLOTS, "unexpected number of untrusted slots"); // advance the manual clock to after slot expiration @@ -665,44 +663,20 @@ vp_enhanced_squelch_enable=0 // insert some random validator key auto const validator = randomKeyPair(KeyType::ed25519).first; - auto const validator2 = randomKeyPair(KeyType::ed25519).first; Peer::id_t peerID = 0; - Peer::id_t peerID2 = - env_.app().config().VP_REDUCE_RELAY_SQUELCH_MAX_SELECTED_PEERS; - // a validator that sends only unique messages, but only from one peer - // must not be selected - for (int i = 0; i < reduce_relay::MAX_MESSAGE_THRESHOLD + 1; ++i) + for (int i = 0; i < reduce_relay::MAX_MESSAGE_THRESHOLD - 1; ++i) { BEAST_EXPECTS( !slots.updateConsideredValidator(validator, peerID), "validator was selected before reaching message threshold"); - BEAST_EXPECTS( - !slots.updateConsideredValidator(validator2, peerID), - "validator was selected before reaching message threshold"); - - stopwatch.advance( - reduce_relay::PEER_IDLED - std::chrono::seconds(1)); - } - // as long as the peer criteria is not met, the validator most not be - // selected - for (int i = 1; i < - env_.app().config().VP_REDUCE_RELAY_SQUELCH_MAX_SELECTED_PEERS - 1; - ++i) - { - BEAST_EXPECTS( - !slots.updateConsideredValidator(validator, i), - "validator was selected before reaching enough peers"); - BEAST_EXPECTS( - !slots.updateConsideredValidator(validator2, i), - "validator was selected before reaching enough peers"); stopwatch.advance( reduce_relay::PEER_IDLED - std::chrono::seconds(1)); } auto const consideredValidator = - slots.updateConsideredValidator(validator, peerID2); + slots.updateConsideredValidator(validator, peerID); BEAST_EXPECTS( consideredValidator && *consideredValidator == validator, "expected validator was not selected"); @@ -711,10 +685,6 @@ vp_enhanced_squelch_enable=0 BEAST_EXPECTS( !slots.getConsideredValidators().contains(validator), "selected validator was not removed from considered list"); - - BEAST_EXPECTS( - slots.getConsideredValidators().contains(validator2), - "unqualified validator was removed from considered list"); } void @@ -782,7 +752,7 @@ vp_enhanced_squelch_enable=0 { testcase("cleanConsideredValidators_deletePoorlyConnected"); auto const validator = randomKeyPair(KeyType::ed25519).first; - Peer::id_t peerID = 0; + Peer::id_t const peerID = 0; TestHandler handler{noop_handler}; // verify that squelchAll is called for poorly connected validator @@ -824,7 +794,7 @@ vp_enhanced_squelch_enable=0 // insert some random validator key auto const idleValidator = randomKeyPair(KeyType::ed25519).first; auto const validator = randomKeyPair(KeyType::ed25519).first; - Peer::id_t peerID = 0; + Peer::id_t const peerID = 0; TestHandler handler{noop_handler}; @@ -869,6 +839,75 @@ vp_enhanced_squelch_enable=0 "timely validator was removed"); } + void + testSquelchUntrustedValidator_consideredListCleared() + { + testcase("testSquelchUntrustedValidator"); + + auto const validator = randomKeyPair(KeyType::ed25519).first; + Peer::id_t const peerID = 0; + + TestHandler handler{noop_handler}; + // verify that squelchAll is called for idle validator + handler.squelchAll_f_ = [&](PublicKey const& actualKey, + std::uint32_t duration, + std::function callback) { + BEAST_EXPECTS( + actualKey == validator, "unexpected key passed to squelchAll"); + }; + + TestStopwatch stopwatch; + EnhancedSquelchingTestSlots slots( + env_.app().logs(), handler, env_.app().config(), stopwatch); + + // add the validator to the considered list + slots.updateUntrustedValidatorSlot( + sha512Half(validator), validator, peerID); + + BEAST_EXPECTS( + slots.getConsideredValidators().contains(validator), + "validator was not added to considered list"); + + slots.squelchUntrustedValidator(validator); + + BEAST_EXPECTS( + !slots.getConsideredValidators().contains(validator), + "validator was not removed from considered list"); + } + + void + testSquelchUntrustedValidator_slotEvicted() + { + testcase("testSquelchUntrustedValidator_slotEvicted"); + + TestHandler handler{noop_handler}; + TestStopwatch stopwatch; + EnhancedSquelchingTestSlots slots( + env_.app().logs(), handler, env_.app().config(), stopwatch); + + // assign a slot to the untrusted validator + auto const validators = fillUntrustedSlots(slots, 1); + + // verify that squelchAll is called for idle validator + handler.squelchAll_f_ = [&](PublicKey const& actualKey, + std::uint32_t duration, + std::function callback) { + BEAST_EXPECTS( + actualKey == validators[0], + "unexpected key passed to squelchAll"); + }; + + BEAST_EXPECTS( + slots.getSlots(false).contains(validators[0]), + "a slot was not assigned to a validator"); + + slots.squelchUntrustedValidator(validators[0]); + + BEAST_EXPECTS( + !slots.getSlots(false).contains(validators[0]), + "a slot was not evicted"); + } + private: /** A helper method to fill untrusted slots of a given Slots instance * with random validator messages*/ @@ -930,6 +969,8 @@ private: testCleanConsideredValidators_deleteSilent(); testCleanConsideredValidators_resetIdle(); testCleanConsideredValidators_deletePoorlyConnected(); + testSquelchUntrustedValidator_consideredListCleared(); + testSquelchUntrustedValidator_slotEvicted(); } }; diff --git a/src/xrpld/overlay/Slot.h b/src/xrpld/overlay/Slot.h index 0bce52699f..d0d387af06 100644 --- a/src/xrpld/overlay/Slot.h +++ b/src/xrpld/overlay/Slot.h @@ -416,6 +416,13 @@ public: typename Slot::ignored_squelch_callback callback, bool isTrusted); + /** Squelch untrusted validator for all peers, and if it has an assigned + * slot, release it. + * @param validatorKey Validator public key + */ + void + squelchUntrustedValidator(PublicKey const& validatorKey); + /** Check if peers stopped relaying messages * and if slots stopped receiving messages from the validator. */ diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index e1bcc726d5..c95f387124 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -1505,6 +1505,30 @@ OverlayImpl::updateUntrustedValidatorSlot( }); } +void +OverlayImpl::handleUntrustedSquelch(PublicKey const& validator) +{ + if (!strand_.running_in_this_thread()) + return post( + strand_, + std::bind(&OverlayImpl::handleUntrustedSquelch, this, validator)); + + auto count = 0; + // we can get the total number of peers with size(), however that would have + // to acquire another lock on peers. Instead, count the number of peers in + // the same loop, as we're already iterating all peers. + auto total = 0; + for_each([&](std::shared_ptr&& p) { + ++total; + if (p->expireAndIsSquelched(validator)) + ++count; + }); + + // if majority of peers squelched the validator + if (count >= total - 1) + slots_.squelchUntrustedValidator(validator); +} + void OverlayImpl::deletePeer(Peer::id_t id) { diff --git a/src/xrpld/overlay/detail/OverlayImpl.h b/src/xrpld/overlay/detail/OverlayImpl.h index 57f935c171..8daa4f0ffa 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.h +++ b/src/xrpld/overlay/detail/OverlayImpl.h @@ -435,6 +435,14 @@ public: PublicKey const& validator, Peer::id_t peer); + /** Handle a squelch message for an untrusted validator. The method + * squelches a given validator and removes it from untrusted slots and + * consideration list if majority of peers have squelched the validator. + * @param validator Validator's public key that was squelched + */ + void + handleUntrustedSquelch(PublicKey const& validator); + /** Called when the peer is deleted. If the peer was selected to be the * source of messages from the validator then squelched peers have to be * unsquelched. @@ -501,7 +509,7 @@ private: /** Handles validator list requests. Using a /vl/ URL, will retrieve the - latest valdiator list (or UNL) that this node has for that + latest validator list (or UNL) that this node has for that public key, if the node trusts that public key. @return true if the request was handled. diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 6c0610aa6f..624baadd2b 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -44,6 +44,8 @@ #include #include +#include "xrpld/overlay/Peer.h" + #include #include #include @@ -249,7 +251,7 @@ PeerImp::send(std::shared_ptr const& m) return; auto const validator = m->getValidatorKey(); - if (validator && squelchStore_.expireAndIsSquelched(*validator)) + if (validator && expireAndIsSquelched(*validator)) { overlay_.reportOutboundTraffic( TrafficCount::category::squelch_suppressed, @@ -572,6 +574,12 @@ PeerImp::hasRange(std::uint32_t uMin, std::uint32_t uMax) (uMax <= maxLedger_); } +bool +PeerImp::expireAndIsSquelched(PublicKey const& validator) +{ + return squelchStore_.expireAndIsSquelched(validator); +} + //------------------------------------------------------------------------------ void @@ -2726,15 +2734,27 @@ PeerImp::onMessage(std::shared_ptr const& m) return; } - std::uint32_t duration = - m->has_squelchduration() ? m->squelchduration() : 0; - if (!m->squelch()) - squelch_.removeSquelch(key); - else if (!squelch_.addSquelch(key, std::chrono::seconds{duration})) + auto duration = std::chrono::seconds{ + m->has_squelchduration() ? m->squelchduration() : 0}; + + if (m->squelch() && + (duration < reduce_relay::MIN_UNSQUELCH_EXPIRE || + duration > reduce_relay::MAX_UNSQUELCH_EXPIRE_PEERS)) + { fee_.update(Resource::feeInvalidData, "squelch duration"); + return; + } JLOG(p_journal_.debug()) - << "onMessage: TMSquelch " << slice << " " << id() << " " << duration; + << "onMessage: TMSquelch " << (!m->squelch() ? "un" : "") + << "squelch message; validator: " << slice << "peer: " << id() + << " duration: " << duration.count(); + + squelchStore_.handleSquelch(key, m->squelch(), duration); + + // if the squelch is for an untrusted validator + if (m->squelch() && !app_.validators().trusted(key)) + overlay_.handleUntrustedSquelch(key); } //-------------------------------------------------------------------------- diff --git a/src/xrpld/overlay/detail/PeerImp.h b/src/xrpld/overlay/detail/PeerImp.h index be6378ba9e..eab8d24802 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -440,6 +440,14 @@ public: return txReduceRelayEnabled_; } + /** Check if a given validator is squelched. If the validator is no longer + * squelched, clear the squelch entry. + * @param validator Validator's public key + * @return true if squelch exists and it is not expired. False otherwise. + */ + bool + expireAndIsSquelched(PublicKey const& validator); + private: void close(); diff --git a/src/xrpld/overlay/detail/Slot.cpp b/src/xrpld/overlay/detail/Slot.cpp index 6e4ac74d0b..9eeb1d01c8 100644 --- a/src/xrpld/overlay/detail/Slot.cpp +++ b/src/xrpld/overlay/detail/Slot.cpp @@ -539,6 +539,27 @@ Slots::updateConsideredValidator(PublicKey const& validator, Peer::id_t peer) return key; } +void +Slots::squelchUntrustedValidator(PublicKey const& validator) +{ + // to prevent the validator from being reinserted squelch the validator + // before removing the validator from consideration and slots + handler_.squelchAll( + validator, MAX_UNSQUELCH_EXPIRE_DEFAULT.count(), [&](Peer::id_t id) { + registerSquelchedValidator(validator, id); + }); + + // if we are considering the validator, remove it + auto itC = consideredValidators_.find(validator); + if (itC != consideredValidators_.end()) + consideredValidators_.erase(itC); + + // if we have assigned an untrusted slot for the validator, remove it + auto itS = untrustedSlots_.find(validator); + if (itS != untrustedSlots_.end()) + untrustedSlots_.erase(itS); +} + void Slots::deletePeer(Peer::id_t id, bool erase) { @@ -618,21 +639,7 @@ Slots::cleanConsideredValidators() for (auto it = consideredValidators_.begin(); it != consideredValidators_.end();) { - // this is a safety check for validators that have - // sent a lot of validations via limited number of peers - if (it->second.count > 2 * reduce_relay::MAX_MESSAGE_THRESHOLD && - it->second.peers.size() < maxSelectedPeers_) - { - JLOG(journal_.warn()) - << "cleanConsideredValidators: removing " - "validator " - << Slice(it->first) << " with insufficient peers"; - - keys.push_back(it->first); - it = consideredValidators_.erase(it); - } - else if ( - now - it->second.lastMessage > + if (now - it->second.lastMessage > reduce_relay::MAX_UNTRUSTED_VALIDATOR_IDLE) { keys.push_back(it->first);