Add standalone check-quorum-intersection command

Closes #4062 This change adds a command `check-quorum-intersection` that takes a JSON file produced by the `quorum` endpoint and checks whether it enjoys a quorum intersection. Most of the change centers around reading JSON in, but this change also required refactoring `QuorumIntersectionChecker` to use a map from `NodeID` to `SCPQuorumSetPtr` instead of a map from `NodeID` to `NodeInfo`, as the intersection checking only relies on the `mQuorumSet` field of a `QuorumMap` and the JSON input does not contain everything in `NodeInfo`.
stellar · Dec 22, 2023 · a93dfe6 · a93dfe6 · latobarita · Dec 22, 2023
1 parent 3c95d00
commit a93dfe6
Show file tree

Hide file tree

Showing 16 changed files with 480 additions and 36 deletions.
diff --git a/docs/software/commands.md b/docs/software/commands.md
@@ -29,6 +29,15 @@ Command options can only by placed after command.
   Option **--trusted-checkpoint-hashes <FILE-NAME>** checks the destination
   ledger hash against the provided reference list of trusted hashes. See the
   command verify-checkpoints for details.
+* **check-quorum-intersection <FILE-NAME>** checks that a given network
+  specified as a JSON file enjoys a quorum intersection. The JSON file must
+  match the output format of the `quorum` HTTP endpoint with the `transitive`
+  and `fullkeys` flags set to `true`. Unlike many other commands, omitting
+  `--conf` specifies that a configuration file should not be used (that is,
+  `--conf` does not default to `stellar-core.cfg`). `check-quorum-intersection`
+  uses the config file only to produce human readable node names in its output,
+  so the option can be safely omitted if human readable node names are not
+  necessary.
 * **convert-id <ID>**: Will output the passed ID in all known forms and then
   exit. Useful for determining the public key that corresponds to a given
   private key. For example:

diff --git a/src/crypto/KeyUtils.h b/src/crypto/KeyUtils.h
@@ -75,6 +75,12 @@ toShortString(T const& key)
 
 std::size_t getKeyVersionSize(strKey::StrKeyVersionByte keyVersion);
 
+// An exception representing an invalid string key representation
+struct InvalidStrKey : public std::invalid_argument
+{
+    using std::invalid_argument::invalid_argument;
+};
+
 template <typename T>
 T
 fromStrKey(std::string const& s)
@@ -84,7 +90,7 @@ fromStrKey(std::string const& s)
     std::vector<uint8_t> k;
     if (!strKey::fromStrKey(s, verByte, k))
     {
-        throw std::invalid_argument("bad " + KeyFunctions<T>::getKeyTypeName());
+        throw InvalidStrKey("bad " + KeyFunctions<T>::getKeyTypeName());
     }
 
     strKey::StrKeyVersionByte ver =
@@ -96,7 +102,7 @@ fromStrKey(std::string const& s)
     if (fixedSizeKeyValid || !KeyFunctions<T>::getKeyVersionIsSupported(ver) ||
         s.size() != strKey::getStrKeySize(k.size()))
     {
-        throw std::invalid_argument("bad " + KeyFunctions<T>::getKeyTypeName());
+        throw InvalidStrKey("bad " + KeyFunctions<T>::getKeyTypeName());
     }
 
     key.type(KeyFunctions<T>::toKeyType(ver));

diff --git a/src/herder/QuorumIntersectionChecker.h b/src/herder/QuorumIntersectionChecker.h
@@ -7,6 +7,7 @@
 #include "herder/QuorumTracker.h"
 #include <atomic>
 #include <memory>
+#include <optional>
 
 namespace stellar
 {
@@ -16,14 +17,26 @@ class Config;
 class QuorumIntersectionChecker
 {
   public:
+    using QuorumSetMap =
+        stellar::UnorderedMap<stellar::NodeID, stellar::SCPQuorumSetPtr>;
+
+    static std::shared_ptr<QuorumIntersectionChecker>
+    create(QuorumTracker::QuorumMap const& qmap,
+           std::optional<stellar::Config> const& cfg,
+           std::atomic<bool>& interruptFlag, bool quiet = false);
+
     static std::shared_ptr<QuorumIntersectionChecker>
-    create(stellar::QuorumTracker::QuorumMap const& qmap,
-           stellar::Config const& cfg, std::atomic<bool>& interruptFlag,
-           bool quiet = false);
+    create(QuorumSetMap const& qmap, std::optional<stellar::Config> const& cfg,
+           std::atomic<bool>& interruptFlag, bool quiet = false);
+
+    static std::set<std::set<NodeID>>
+    getIntersectionCriticalGroups(QuorumTracker::QuorumMap const& qmap,
+                                  std::optional<stellar::Config> const& cfg,
+                                  std::atomic<bool>& interruptFlag);
 
     static std::set<std::set<NodeID>>
-    getIntersectionCriticalGroups(stellar::QuorumTracker::QuorumMap const& qmap,
-                                  stellar::Config const& cfg,
+    getIntersectionCriticalGroups(QuorumSetMap const& qmap,
+                                  std::optional<stellar::Config> const& cfg,
                                   std::atomic<bool>& interruptFlag);
 
     virtual ~QuorumIntersectionChecker(){};

diff --git a/src/herder/QuorumIntersectionCheckerImpl.cpp b/src/herder/QuorumIntersectionCheckerImpl.cpp
@@ -266,8 +266,9 @@ MinQuorumEnumerator::anyMinQuorumHasDisjointQuorum()
 ////////////////////////////////////////////////////////////////////////////////
 
 QuorumIntersectionCheckerImpl::QuorumIntersectionCheckerImpl(
-    QuorumTracker::QuorumMap const& qmap, Config const& cfg,
-    std::atomic<bool>& interruptFlag, bool quiet)
+    QuorumIntersectionChecker::QuorumSetMap const& qmap,
+    std::optional<Config> const& cfg, std::atomic<bool>& interruptFlag,
+    bool quiet)
     : mCfg(cfg)
     , mLogTrace(Logging::logTrace("SCP"))
     , mQuiet(quiet)
@@ -528,6 +529,22 @@ MinQuorumEnumerator::hasDisjointQuorum(BitSet const& nodes) const
     return !disj.empty();
 }
 
+// Render `id` as a short, human readable string. If `cfg` has a value, this
+// function uses `cfg` to render the string. Otherwise, it returns the first 5
+// hex values `id`.
+std::string
+toShortString(std::optional<Config> const& cfg, NodeID const& id)
+{
+    if (cfg)
+    {
+        return cfg->toShortString(id);
+    }
+    else
+    {
+        return KeyUtils::toShortString(id).substr(0, 5);
+    }
+}
+
 QBitSet
 QuorumIntersectionCheckerImpl::convertSCPQuorumSet(SCPQuorumSet const& sqs)
 {
@@ -563,7 +580,7 @@ QuorumIntersectionCheckerImpl::convertSCPQuorumSet(SCPQuorumSet const& sqs)
             // approximation. The tests referring to "null qsets" differentiate
             // these cases.
             CLOG_DEBUG(SCP, "Depending on node with missing QSet: {}",
-                       mCfg.toShortString(v));
+                       toShortString(mCfg, v));
         }
         else
         {
@@ -580,15 +597,16 @@ QuorumIntersectionCheckerImpl::convertSCPQuorumSet(SCPQuorumSet const& sqs)
 }
 
 void
-QuorumIntersectionCheckerImpl::buildGraph(QuorumTracker::QuorumMap const& qmap)
+QuorumIntersectionCheckerImpl::buildGraph(
+    QuorumIntersectionChecker::QuorumSetMap const& qmap)
 {
     mPubKeyBitNums.clear();
     mBitNumPubKeys.clear();
     mGraph.clear();
 
     for (auto const& pair : qmap)
     {
-        if (pair.second.mQuorumSet)
+        if (pair.second)
         {
             size_t n = mBitNumPubKeys.size();
             mPubKeyBitNums.insert(std::make_pair(pair.first, n));
@@ -597,19 +615,19 @@ QuorumIntersectionCheckerImpl::buildGraph(QuorumTracker::QuorumMap const& qmap)
         else
         {
             CLOG_DEBUG(SCP, "Node with missing QSet: {}",
-                       mCfg.toShortString(pair.first));
+                       toShortString(mCfg, pair.first));
         }
     }
 
     for (auto const& pair : qmap)
     {
-        if (pair.second.mQuorumSet)
+        if (pair.second)
         {
             auto i = mPubKeyBitNums.find(pair.first);
             releaseAssert(i != mPubKeyBitNums.end());
             auto nodeNum = i->second;
             releaseAssert(nodeNum == mGraph.size());
-            auto qb = convertSCPQuorumSet(*(pair.second.mQuorumSet));
+            auto qb = convertSCPQuorumSet(*(pair.second));
             qb.log();
             mGraph.emplace_back(qb);
         }
@@ -632,7 +650,7 @@ QuorumIntersectionCheckerImpl::buildSCCs()
 std::string
 QuorumIntersectionCheckerImpl::nodeName(size_t node) const
 {
-    return mCfg.toShortString(mBitNumPubKeys.at(node));
+    return toShortString(mCfg, mBitNumPubKeys.at(node));
 }
 
 bool
@@ -764,7 +782,7 @@ findCriticalityCandidates(SCPQuorumSet const& p,
 }
 
 std::string
-groupString(Config const& cfg, std::set<NodeID> const& group)
+groupString(std::optional<Config> const& cfg, std::set<NodeID> const& group)
 {
     std::ostringstream out;
     bool first = true;
@@ -776,18 +794,37 @@ groupString(Config const& cfg, std::set<NodeID> const& group)
             out << ", ";
         }
         first = false;
-        out << cfg.toShortString(k);
+        out << toShortString(cfg, k);
     }
     out << ']';
     return out.str();
 }
+
+QuorumIntersectionChecker::QuorumSetMap
+toQuorumIntersectionMap(QuorumTracker::QuorumMap const& qmap)
+{
+    QuorumIntersectionChecker::QuorumSetMap ret;
+    for (auto const& elem : qmap)
+    {
+        ret[elem.first] = elem.second.mQuorumSet;
+    }
+    return ret;
+}
 }
 
 namespace stellar
 {
 std::shared_ptr<QuorumIntersectionChecker>
 QuorumIntersectionChecker::create(QuorumTracker::QuorumMap const& qmap,
-                                  Config const& cfg,
+                                  std::optional<Config> const& cfg,
+                                  std::atomic<bool>& interruptFlag, bool quiet)
+{
+    return create(toQuorumIntersectionMap(qmap), cfg, interruptFlag, quiet);
+}
+
+std::shared_ptr<QuorumIntersectionChecker>
+QuorumIntersectionChecker::create(QuorumSetMap const& qmap,
+                                  std::optional<Config> const& cfg,
                                   std::atomic<bool>& interruptFlag, bool quiet)
 {
     return std::make_shared<QuorumIntersectionCheckerImpl>(
@@ -796,7 +833,16 @@ QuorumIntersectionChecker::create(QuorumTracker::QuorumMap const& qmap,
 
 std::set<std::set<NodeID>>
 QuorumIntersectionChecker::getIntersectionCriticalGroups(
-    stellar::QuorumTracker::QuorumMap const& qmap, stellar::Config const& cfg,
+    QuorumTracker::QuorumMap const& qmap, std::optional<Config> const& cfg,
+    std::atomic<bool>& interruptFlag)
+{
+    return getIntersectionCriticalGroups(toQuorumIntersectionMap(qmap), cfg,
+                                         interruptFlag);
+}
+
+std::set<std::set<NodeID>>
+QuorumIntersectionChecker::getIntersectionCriticalGroups(
+    QuorumSetMap const& qmap, std::optional<Config> const& cfg,
     std::atomic<bool>& interruptFlag)
 {
     // We're going to search for "intersection-critical" groups, by considering
@@ -826,13 +872,13 @@ QuorumIntersectionChecker::getIntersectionCriticalGroups(
 
     std::set<std::set<NodeID>> candidates;
     std::set<std::set<NodeID>> critical;
-    QuorumTracker::QuorumMap test_qmap(qmap);
+    QuorumSetMap test_qmap(qmap);
 
     for (auto const& k : qmap)
     {
-        if (k.second.mQuorumSet)
+        if (k.second)
         {
-            findCriticalityCandidates(*(k.second.mQuorumSet), candidates, true);
+            findCriticalityCandidates(*(k.second), candidates, true);
         }
     }
 
@@ -859,8 +905,8 @@ QuorumIntersectionChecker::getIntersectionCriticalGroups(
         {
             for (auto const& d : qmap)
             {
-                if (group.find(d.first) == group.end() && d.second.mQuorumSet &&
-                    pointsToCandidate(*(d.second.mQuorumSet), candidate))
+                if (group.find(d.first) == group.end() && d.second &&
+                    pointsToCandidate(*(d.second), candidate))
                 {
                     pointsToGroup.insert(d.first);
                 }
@@ -879,7 +925,7 @@ QuorumIntersectionChecker::getIntersectionCriticalGroups(
         // Install the fickle qset in every member of the group.
         for (auto const& candidate : group)
         {
-            test_qmap[candidate] = QuorumTracker::NodeInfo{fickleQSet, 0};
+            test_qmap[candidate] = fickleQSet;
         }
 
         // Check to see if this modified config is vulnerable to splitting.

diff --git a/src/herder/QuorumIntersectionCheckerImpl.h b/src/herder/QuorumIntersectionCheckerImpl.h
@@ -358,6 +358,7 @@
 #include "xdr/Stellar-SCP.h"
 #include "xdr/Stellar-types.h"
 #include <functional>
+#include <optional>
 
 namespace
 {
@@ -441,13 +442,14 @@ class MinQuorumEnumerator
 };
 
 // Quorum intersection checking is done by establishing a root
-// QuorumIntersectionChecker on a given QuorumMap. The QuorumIntersectionChecker
-// builds a QGraph of the nodes, uses TarjanSCCCalculator to calculate its SCCs,
-// and then runs a MinQuorumEnumerator to recursively scan the powerset.
+// QuorumIntersectionChecker on a given QuorumSetMap. The
+// QuorumIntersectionChecker builds a QGraph of the nodes, uses
+// TarjanSCCCalculator to calculate its SCCs, and then runs a
+// MinQuorumEnumerator to recursively scan the powerset.
 class QuorumIntersectionCheckerImpl : public stellar::QuorumIntersectionChecker
 {
 
-    stellar::Config const& mCfg;
+    std::optional<stellar::Config> const mCfg;
 
     struct Stats
     {
@@ -506,7 +508,8 @@ class QuorumIntersectionCheckerImpl : public stellar::QuorumIntersectionChecker
     std::atomic<bool>& mInterruptFlag;
 
     QBitSet convertSCPQuorumSet(stellar::SCPQuorumSet const& sqs);
-    void buildGraph(stellar::QuorumTracker::QuorumMap const& qmap);
+    void
+    buildGraph(stellar::QuorumIntersectionChecker::QuorumSetMap const& qmap);
     void buildSCCs();
 
     bool containsQuorumSlice(BitSet const& bs, QBitSet const& qbs) const;
@@ -525,10 +528,10 @@ class QuorumIntersectionCheckerImpl : public stellar::QuorumIntersectionChecker
     friend class MinQuorumEnumerator;
 
   public:
-    QuorumIntersectionCheckerImpl(stellar::QuorumTracker::QuorumMap const& qmap,
-                                  stellar::Config const& cfg,
-                                  std::atomic<bool>& interruptFlag,
-                                  bool quiet = false);
+    QuorumIntersectionCheckerImpl(
+        stellar::QuorumIntersectionChecker::QuorumSetMap const& qmap,
+        std::optional<stellar::Config> const& cfg,
+        std::atomic<bool>& interruptFlag, bool quiet = false);
     bool networkEnjoysQuorumIntersection() const override;
 
     std::pair<std::vector<stellar::NodeID>, std::vector<stellar::NodeID>>