From 57b451cab070f696ef72ba1a1d9a1e2604f991b3 Mon Sep 17 00:00:00 2001
From: Bronek Kozicki <brok@incorrekt.com>
Date: Mon, 2 Sep 2024 15:28:23 +0100
Subject: [PATCH] Disable quorum also when threshold is very large

---
 src/test/app/ValidatorList_test.cpp         |  7 ++--
 src/xrpld/app/misc/detail/ValidatorList.cpp | 40 ++++++++++++++++-----
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp
index 52365be506c..9a51c2ce0c7 100644
--- a/src/test/app/ValidatorList_test.cpp
+++ b/src/test/app/ValidatorList_test.cpp
@@ -2763,6 +2763,7 @@ class ValidatorList_test : public beast::unit_test::suite
         };
 
         // All test cases use 5 publishers.
+        constexpr auto quorumDisabled = std::numeric_limits<std::size_t>::max();
         {
             // List threshold = 5 (same as number of trusted publishers)
             ManifestCache pubManifests;
@@ -2811,7 +2812,7 @@ class ValidatorList_test : public beast::unit_test::suite
                 env.app().getOPs(),
                 env.app().overlay(),
                 env.app().getHashRouter());
-            BEAST_EXPECT(trustedKeys->quorum() == 1);
+            BEAST_EXPECT(trustedKeys->quorum() == quorumDisabled);
             BEAST_EXPECT(trustedKeys->getTrustedMasterKeys().size() == 1);
 
             hash_set<NodeID> removed;
@@ -2879,7 +2880,7 @@ class ValidatorList_test : public beast::unit_test::suite
                 env.app().getOPs(),
                 env.app().overlay(),
                 env.app().getHashRouter());
-            BEAST_EXPECT(trustedKeys->quorum() == 1);
+            BEAST_EXPECT(trustedKeys->quorum() == quorumDisabled);
             BEAST_EXPECT(trustedKeys->getTrustedMasterKeys().size() == 1);
 
             hash_set<NodeID> removed;
@@ -2896,8 +2897,6 @@ class ValidatorList_test : public beast::unit_test::suite
             BEAST_EXPECT(changes.added.empty());
             BEAST_EXPECT(changes.removed == removed);
         }
-
-        constexpr auto quorumDisabled = std::numeric_limits<std::size_t>::max();
         {
             // List threshold = 3 (default), 2 publishers are revoked
             ManifestCache pubManifests;
diff --git a/src/xrpld/app/misc/detail/ValidatorList.cpp b/src/xrpld/app/misc/detail/ValidatorList.cpp
index b961578fba2..d7c025ab4c9 100644
--- a/src/xrpld/app/misc/detail/ValidatorList.cpp
+++ b/src/xrpld/app/misc/detail/ValidatorList.cpp
@@ -1780,16 +1780,40 @@ ValidatorList::calculateQuorum(
         return *minimumQuorum_;
     }
 
-    // Do not use achievable quorum until lists from a sufficient number of
-    // configured publishers are available
-    std::size_t unavailable = 0;
-    for (auto const& list : publisherLists_)
+    if (!publisherLists_.empty())
     {
-        if (list.second.status != PublisherStatus::available)
-            unavailable += 1;
+        // Do not use achievable quorum until lists from a sufficient number of
+        // configured publishers are available
+        std::size_t unavailable = 0;
+        for (auto const& list : publisherLists_)
+        {
+            if (list.second.status != PublisherStatus::available)
+                unavailable += 1;
+        }
+        // There are two, subtly different, sides to list threshold
+        //
+        // 1. The minimum required intersection between lists listThreshold_
+        //    for a validator to be included in trustedMasterKeys_.
+        //    If this many (or more) publishers are unavailable, we are likely
+        //    to NOT include a validator which otherwise would have been used.
+        // 2. The minimum number of lists which, when UNAVAILABLE, will prevent
+        //    us from hitting the above threshold on ANY validator. This is
+        //    calculated as:
+        //      N - M + 1
+        //    where
+        //      N: number of lists i.e. publisherLists_.size()
+        //      M: minimum required intersection i.e. listThreshold_
+        //
+        // Ideally we want both to be equal, but it is only possible when we
+        // have an odd number of publisher lists; also the user might have
+        // configured a non-standard listThreshold_.
+        auto const errorThreshold = std::min(
+            listThreshold_,  //
+            publisherLists_.size() - listThreshold_ + 1);
+        assert(errorThreshold > 0);
+        if (unavailable >= errorThreshold)
+            return std::numeric_limits<std::size_t>::max();
     }
-    if (unavailable >= listThreshold_)
-        return std::numeric_limits<std::size_t>::max();
 
     // Use an 80% quorum to balance fork safety, liveness, and required UNL
     // overlap.