extends enhanced squelching to squelch a validator squelched by majority of peers

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<void(Peer::id_t)> 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<void(Peer::id_t)> 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();
     }
 };
 

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.
      */

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<PeerImp>&& 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)
 {

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/<hex-encoded public key> 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.

src/xrpld/overlay/detail/PeerImp.cpp

@@ -44,6 +44,8 @@
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/beast/core/ostream.hpp>
 
+#include "xrpld/overlay/Peer.h"
+
 #include <algorithm>
 #include <chrono>
 #include <memory>
@@ -249,7 +251,7 @@ PeerImp::send(std::shared_ptr<Message> 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<protocol::TMSquelch> 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);
 }
 
 //--------------------------------------------------------------------------

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();

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);