Skip to content

Commit c16a6d1

Browse files
authored
Update stellar-quorum-analyzer to master and handle NoQuorum status (#5342)
# Description ### What Bump `stellar-quorum-analyzer` from `f67e9e2f` to `b422b366` (master) and adapt the Quorum Intersection Checker V2 integration: - Add `QuorumCheckerStatus::NO_QUORUM` (103), propagated through the subprocess exit code, `result.json`, adaptor state, a new `result-no-quorum` metric, and the CLI. It counts as a complete run but never advances `mLastGoodLedger`, so the network does not report as enjoying quorum intersection. - Remove the now-dead `set_rust_global_memory_limit_to_unlimited()` and its stale abort-based-memory comments. - Update the two null-qset tests to expect `NO_QUORUM`; add a no-quorum test. ### Why The new analyzer distinguishes an FBAS with *no quorum at all* (a degenerate / potential-halt state) from `UNSAT` ("enjoys quorum intersection"), which the old version wrongly conflated. It also replaced the global-allocator hard memory limit (process abort on exceed) with a soft, estimate-based per-solver limit, making `set_rust_global_memory_limit_to_unlimited()` a no-op. Tested: full suite passes (5,678,585 assertions / 698 cases), incl. the `[quorumintersection]` suite. # Checklist - [x] Reviewed the contributing document - [x] Rebased on top of master (no merge commits) - [x] Ran `clang-format` (v20.1.8, the repo-pinned version; via `git-clang-format` on the diff) - [x] Compiles - [x] Ran all tests - [ ] If change impacts performance, include supporting evidence — n/a
2 parents 04479c3 + 7e04e37 commit c16a6d1

10 files changed

Lines changed: 174 additions & 56 deletions

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/herder/RustQuorumCheckerAdaptor.cpp

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -159,13 +159,17 @@ fromQuorumCheckerStatusJson(Json::Value const& value)
159159
}
160160

161161
auto statusInt = value.asUInt();
162-
if (statusInt > static_cast<unsigned>(QuorumCheckerStatus::UNKNOWN))
163-
{
162+
switch (statusInt)
163+
{
164+
case static_cast<unsigned>(QuorumCheckerStatus::UNSAT):
165+
case static_cast<unsigned>(QuorumCheckerStatus::SAT):
166+
case static_cast<unsigned>(QuorumCheckerStatus::UNKNOWN):
167+
case static_cast<unsigned>(QuorumCheckerStatus::NO_QUORUM):
168+
return static_cast<QuorumCheckerStatus>(statusInt);
169+
default:
164170
throw RustQuorumCheckerError("Invalid status value: " +
165171
std::to_string(statusInt));
166172
}
167-
168-
return static_cast<QuorumCheckerStatus>(statusInt);
169173
}
170174

171175
void
@@ -252,6 +256,7 @@ QuorumCheckerMetrics::QuorumCheckerMetrics()
252256
, mAbortedRun(0)
253257
, mResultPotentialSplit(0)
254258
, mResultUnknown(0)
259+
, mResultNoQuorum(0)
255260
, mCumulativeTimeMs(0)
256261
, mCumulativeMemByte(0)
257262
{
@@ -293,6 +298,12 @@ QuorumCheckerMetrics::QuorumCheckerMetrics(Json::Value const& value)
293298
throw RustQuorumCheckerError(
294299
"Metrics missing or invalid 'result_unknown_count' field");
295300
}
301+
if (!value.isMember("result_no_quorum_count") ||
302+
!value["result_no_quorum_count"].isUInt())
303+
{
304+
throw RustQuorumCheckerError(
305+
"Metrics missing or invalid 'result_no_quorum_count' field");
306+
}
296307
if (!value.isMember("cumulative_time_ms") ||
297308
!value["cumulative_time_ms"].isUInt64())
298309
{
@@ -310,6 +321,7 @@ QuorumCheckerMetrics::QuorumCheckerMetrics(Json::Value const& value)
310321
mAbortedRun = value["aborted_run_count"].asUInt64();
311322
mResultPotentialSplit = value["result_potential_split_count"].asUInt64();
312323
mResultUnknown = value["result_unknown_count"].asUInt64();
324+
mResultNoQuorum = value["result_no_quorum_count"].asUInt64();
313325
mCumulativeTimeMs = value["cumulative_time_ms"].asUInt64();
314326
mCumulativeMemByte = value["cumulative_mem_byte"].asUInt64();
315327
}
@@ -323,6 +335,7 @@ QuorumCheckerMetrics::toJson()
323335
ret["aborted_run_count"] = Json::UInt64(mAbortedRun);
324336
ret["result_potential_split_count"] = Json::UInt64(mResultPotentialSplit);
325337
ret["result_unknown_count"] = Json::UInt64(mResultUnknown);
338+
ret["result_no_quorum_count"] = Json::UInt64(mResultNoQuorum);
326339
ret["cumulative_time_ms"] = Json::UInt64(mCumulativeTimeMs);
327340
ret["cumulative_mem_byte"] = Json::UInt64(mCumulativeMemByte);
328341
return ret;
@@ -337,6 +350,7 @@ QuorumCheckerMetrics::flush(MetricsRegistry& metrics)
337350
metrics.NewCounter({"scp", "qic", "result-potential-split"})
338351
.inc(mResultPotentialSplit);
339352
metrics.NewCounter({"scp", "qic", "result-unknown"}).inc(mResultUnknown);
353+
metrics.NewCounter({"scp", "qic", "result-no-quorum"}).inc(mResultNoQuorum);
340354
metrics.NewMeter({"scp", "qic", "cumulative-time-ms"}, "milli-second")
341355
.Mark(mCumulativeTimeMs);
342356
metrics.NewMeter({"scp", "qic", "cumulative-mem-byte"}, "byte")
@@ -346,6 +360,7 @@ QuorumCheckerMetrics::flush(MetricsRegistry& metrics)
346360
mAbortedRun = 0;
347361
mResultPotentialSplit = 0;
348362
mResultUnknown = 0;
363+
mResultNoQuorum = 0;
349364
mCumulativeTimeMs = 0;
350365
mCumulativeMemByte = 0;
351366
}
@@ -396,6 +411,10 @@ checkQuorumIntersectionInner(
396411
{
397412
metrics.mResultPotentialSplit += 1;
398413
}
414+
else if (status == QuorumCheckerStatus::NO_QUORUM)
415+
{
416+
metrics.mResultNoQuorum += 1;
417+
}
399418

400419
// Update time limit. Memory is a transient resource that gets reclaimed
401420
// back.
@@ -565,10 +584,12 @@ runQuorumIntersectionCheckAsync(
565584

566585
// Note: the ecode should match the return code from the command
567586
// line-running process which is just `QuorumCheckerStatus` as integer
568-
// on success. However, if the command fails due to abort (if exceeding
569-
// the memory limit), the ecode=1 will be returned because of the
570-
// simplification of collapsing all non-WIFEXITED exits to error code 1
571-
// (see `mapExitStatusToErrorCode` in ProcessManagerImpl.cpp).
587+
// on success. Exceeding the time or (estimated) memory limit is now a
588+
// recoverable solver error that surfaces as `UNKNOWN` (102), not a
589+
// process abort. If the command dies abnormally (crash/signal, i.e. a
590+
// non-WIFEXITED exit), ecode=1 is returned because of the
591+
// simplification of collapsing all such exits to error code 1 (see
592+
// `mapExitStatusToErrorCode` in ProcessManagerImpl.cpp).
572593
int ecode = ec.value();
573594
CLOG_DEBUG(SCP,
574595
"Processing quorum intersection check result: numNodes={}, "
@@ -579,17 +600,23 @@ runQuorumIntersectionCheckAsync(
579600

580601
if (ecode == static_cast<int>(QuorumCheckerStatus::UNSAT) ||
581602
ecode == static_cast<int>(QuorumCheckerStatus::SAT) ||
582-
ecode == static_cast<int>(QuorumCheckerStatus::UNKNOWN))
603+
ecode == static_cast<int>(QuorumCheckerStatus::UNKNOWN) ||
604+
ecode == static_cast<int>(QuorumCheckerStatus::NO_QUORUM))
583605
{
584606
try
585607
{
586608
auto res = parseResultsJson(qicResultJson);
587609
hStateSP->mStatus = fromQuorumCheckerStatusJson(res["status"]);
588610
QuorumCheckerMetrics metrics(res["metrics"]);
589611
metrics.flush(hStateSP->mMetrics);
590-
// only update the following info if we had a complete run
612+
// only update the following info if we had a complete run.
613+
// NO_QUORUM is a complete result (the checker definitively
614+
// determined the FBAS has no quorum) so we record it too, but
615+
// unlike UNSAT it is not a "good" result and must not advance
616+
// mLastGoodLedger.
591617
if (ecode == static_cast<int>(QuorumCheckerStatus::UNSAT) ||
592-
ecode == static_cast<int>(QuorumCheckerStatus::SAT))
618+
ecode == static_cast<int>(QuorumCheckerStatus::SAT) ||
619+
ecode == static_cast<int>(QuorumCheckerStatus::NO_QUORUM))
593620
{
594621
hStateSP->mNumNodes = numNodes;
595622
hStateSP->mLastCheckLedger = ledger;

src/herder/RustQuorumCheckerAdaptor.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ struct QuorumCheckerMetrics
3535
uint64_t mAbortedRun;
3636
uint64_t mResultPotentialSplit;
3737
uint64_t mResultUnknown;
38+
uint64_t mResultNoQuorum;
3839
uint64_t mCumulativeTimeMs;
3940
uint64_t mCumulativeMemByte;
4041
QuorumCheckerMetrics();
@@ -52,16 +53,20 @@ struct QuorumCheckerMetrics
5253
// - The time limit applies across all runs (all loops during criticality
5354
// analysis), and is enforced by the Rust implementation, which returns an
5455
// `Err` if exceeded.
55-
// - The memory limit is enforced by the glocal allocator, and once exceeds,
56-
// will abort the program immediately.
56+
// - The memory limit is enforced softly inside the solver: memory usage is
57+
// conservatively estimated from the solver's clause/variable counts, and the
58+
// solver returns an `Err` once the estimate exceeds the limit.
5759
//
58-
// Therefore it is **crucial** this routine runs in a separate process from the
59-
// main stellar-core!!
60+
// We still run this routine in a separate process from the main stellar-core so
61+
// that its (potentially large) resource usage stays isolated.
6062
//
6163
// Return values:
6264
// - `UNSAT` if the quorum intersection check finds no non-intersecting quorums
6365
// (good)
6466
// - `SAT` if the quorum intersection check finds quorum splits (bad!!)
67+
// - `NO_QUORUM` if the FBAS contains no quorum at all (a degenerate /
68+
// potential-halt configuration). This is distinct from `UNSAT`: a network
69+
// with no quorum does not enjoy quorum intersection.
6570
// - `UNKNOWN` if the quorum intersection check does not complete, likely due to
6671
// exceeding solver internal limits (e.g. no. conflicts). Note: if the quorum
6772
// intersection check completes, but the criticality analysis

src/herder/test/QuorumIntersectionTests.cpp

Lines changed: 90 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,24 @@ networkEnjoysQuorumIntersectionV2Wrapper(QuorumTracker::QuorumMap const& qmap,
112112
return state->mStatus == QuorumCheckerStatus::UNSAT;
113113
}
114114

115+
// Like networkEnjoysQuorumIntersectionV2Wrapper, but returns the raw checker
116+
// status so tests can distinguish UNSAT (enjoys intersection) from NO_QUORUM
117+
// (the FBAS admits no quorum at all -- a halted/degenerate configuration).
118+
QuorumCheckerStatus
119+
quorumCheckStatusV2Wrapper(QuorumTracker::QuorumMap const& qmap,
120+
Config const& cfg)
121+
{
122+
VirtualClock clock;
123+
Application::pointer app = createTestApplication(clock, cfg);
124+
auto state = std::make_shared<QuorumMapIntersectionState>(*app);
125+
state->mRecalculating = true;
126+
quorumIntersectionCheckerV2Wrapper(
127+
*app, state, qmap, app->getProcessManager(), clock, false,
128+
cfg.QUORUM_INTERSECTION_CHECKER_TIME_LIMIT_MS,
129+
cfg.QUORUM_INTERSECTION_CHECKER_MEMORY_LIMIT_BYTES);
130+
return state->mStatus;
131+
}
132+
115133
std::set<std::set<NodeID>>
116134
runIntersectionCriticalGroupsCheckV2(QuorumTracker::QuorumMap const& qmap,
117135
Config const& cfg)
@@ -237,6 +255,58 @@ TEST_CASE("quorum non intersection basic 4-node",
237255
REQUIRE(!networkEnjoysQuorumIntersectionV2Wrapper(qm, cfg));
238256
}
239257

258+
TEST_CASE("quorum no-quorum threshold exceeds degree",
259+
"[herder][quorumintersection]")
260+
{
261+
QuorumTracker::QuorumMap qm;
262+
263+
PublicKey pkA = SecretKey::pseudoRandomForTesting().getPublicKey();
264+
PublicKey pkB = SecretKey::pseudoRandomForTesting().getPublicKey();
265+
PublicKey pkC = SecretKey::pseudoRandomForTesting().getPublicKey();
266+
PublicKey pkD = SecretKey::pseudoRandomForTesting().getPublicKey();
267+
// A "ghost" validator that is referenced by others' quorum sets but is
268+
// itself absent from the quorum map, so it can never be available. With a
269+
// threshold of 3 over {self, peer, ghost}, no node can ever reach its
270+
// threshold, so the FBAS admits no quorum at all. This must be reported as
271+
// NO_QUORUM, and is distinct from UNSAT ("enjoys quorum intersection").
272+
PublicKey pkGhost = SecretKey::pseudoRandomForTesting().getPublicKey();
273+
274+
qm[pkA] = QuorumTracker::NodeInfo{
275+
make_shared<QS>(3, VK({pkA, pkB, pkGhost}), VQ{}), 0};
276+
qm[pkB] = QuorumTracker::NodeInfo{
277+
make_shared<QS>(3, VK({pkA, pkB, pkGhost}), VQ{}), 0};
278+
qm[pkC] = QuorumTracker::NodeInfo{
279+
make_shared<QS>(3, VK({pkC, pkD, pkGhost}), VQ{}), 0};
280+
qm[pkD] = QuorumTracker::NodeInfo{
281+
make_shared<QS>(3, VK({pkC, pkD, pkGhost}), VQ{}), 0};
282+
283+
Config cfg(getTestConfig());
284+
285+
VirtualClock clock;
286+
Application::pointer app = createTestApplication(clock, cfg);
287+
auto state = std::make_shared<QuorumMapIntersectionState>(*app);
288+
289+
auto& noQuorumCounter =
290+
state->mMetrics.NewCounter({"scp", "qic", "result-no-quorum"});
291+
auto noQuorumCountBefore = noQuorumCounter.count();
292+
293+
state->mRecalculating = true;
294+
state->mStatus = QuorumCheckerStatus::UNKNOWN;
295+
quorumIntersectionCheckerV2Wrapper(
296+
*app, state, qm, app->getProcessManager(), clock, false,
297+
cfg.QUORUM_INTERSECTION_CHECKER_TIME_LIMIT_MS,
298+
cfg.QUORUM_INTERSECTION_CHECKER_MEMORY_LIMIT_BYTES);
299+
300+
REQUIRE(state->mStatus == QuorumCheckerStatus::NO_QUORUM);
301+
REQUIRE(noQuorumCounter.count() == noQuorumCountBefore + 1);
302+
// NO_QUORUM is a complete result (records the checked ledger) but it is not
303+
// a "good" result, so it does not advance mLastGoodLedger and the network
304+
// does not enjoy quorum intersection.
305+
REQUIRE(state->mLastCheckLedger != 0);
306+
REQUIRE(state->mLastGoodLedger == 0);
307+
REQUIRE(!state->enjoysQuorunIntersection());
308+
}
309+
240310
TEST_CASE("quorum non intersection 6-node", "[herder][quorumintersection]")
241311
{
242312
QuorumTracker::QuorumMap qm;
@@ -956,8 +1026,13 @@ TEST_CASE("quorum intersection 6-org 1-node 4-null qsets",
9561026
// for org2..org5. We know org0..org1 have threshold 67% = 3-of-4 (4 being
9571027
// "self + 3 neighbours"); the current logic in the quorum intersection
9581028
// checker (see buildGraph and convertSCPQuorumSet) will treat this network
959-
// as _only_ having 2-nodes and will therefore declare it vacuously enjoying
960-
// quorum intersection due to being halted.
1029+
// as _only_ having 2-nodes and therefore as a halted network with no
1030+
// quorum.
1031+
//
1032+
// The V1 checker declares such a halted network as vacuously enjoying
1033+
// quorum intersection; the V2 checker instead reports NO_QUORUM (the FBAS
1034+
// admits no quorum at all), which is not conflated with UNSAT ("enjoys
1035+
// intersection").
9611036
//
9621037
// (At other points in the design, and possibly again in the future if we
9631038
// change our minds, we modeled this differently, treating the null-qset
@@ -991,7 +1066,10 @@ TEST_CASE("quorum intersection 6-org 1-node 4-null qsets",
9911066
REQUIRE(qic->networkEnjoysQuorumIntersection());
9921067
REQUIRE(qic->getMaxQuorumsFound() == 0);
9931068

994-
REQUIRE(networkEnjoysQuorumIntersectionV2Wrapper(qm, cfg));
1069+
// V2 reports the halted network as having no quorum, rather than vacuously
1070+
// enjoying quorum intersection like V1.
1071+
REQUIRE(quorumCheckStatusV2Wrapper(qm, cfg) ==
1072+
QuorumCheckerStatus::NO_QUORUM);
9951073
}
9961074

9971075
TEST_CASE("quorum intersection 4-org 1-node 4-null qsets",
@@ -1008,10 +1086,11 @@ TEST_CASE("quorum intersection 4-org 1-node 4-null qsets",
10081086
// +-> org3 <-+
10091087
//
10101088
// As with the case before, this represents (to the quorum intersection
1011-
// checker's eyes) a halted network which vacuously enjoys quorum
1012-
// intersection. But if we were using one of the other models for the
1013-
// meaning of a null qset, it might be different: split in the byzantine
1014-
// case, live and enjoying quorum intersection in the live-and-unknown case.
1089+
// checker's eyes) a halted network with no quorum. The V1 checker treats it
1090+
// as vacuously enjoying quorum intersection, while the V2 checker reports
1091+
// NO_QUORUM. But if we were using one of the other models for the meaning
1092+
// of a null qset, it might be different: split in the byzantine case, live
1093+
// and enjoying quorum intersection in the live-and-unknown case.
10151094

10161095
auto orgs = generateOrgs(4, {1});
10171096
auto qm = interconnectOrgsUnidir(orgs, {
@@ -1040,7 +1119,10 @@ TEST_CASE("quorum intersection 4-org 1-node 4-null qsets",
10401119
REQUIRE(qic->networkEnjoysQuorumIntersection());
10411120
REQUIRE(qic->getMaxQuorumsFound() == 0);
10421121

1043-
REQUIRE(networkEnjoysQuorumIntersectionV2Wrapper(qm, cfg));
1122+
// V2 reports the halted network as having no quorum, rather than vacuously
1123+
// enjoying quorum intersection like V1.
1124+
REQUIRE(quorumCheckStatusV2Wrapper(qm, cfg) ==
1125+
QuorumCheckerStatus::NO_QUORUM);
10441126
}
10451127

10461128
TEST_CASE("quorum intersection 6-org 3-node fully-connected",

src/main/CommandLine.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1386,6 +1386,12 @@ runCheckQuorumIntersection(CommandLineArgs const& args)
13861386
{
13871387
CLOG_WARNING(SCP, "Network does not enjoy quorum intersection");
13881388
}
1389+
else if (status == QuorumCheckerStatus::NO_QUORUM)
1390+
{
1391+
CLOG_WARNING(SCP, "Network has no quorum -- the configuration "
1392+
"admits no quorum at all, which does not "
1393+
"enjoy quorum intersection");
1394+
}
13891395
else
13901396
{
13911397
CLOG_WARNING(SCP, "UNKNOWN result -- quorum intersection "

src/main/Config.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,12 @@ Config::Config() : NODE_SEED(SecretKey::random())
307307
QUORUM_INTERSECTION_CHECKER = true;
308308
USE_QUORUM_INTERSECTION_CHECKER_V2 = false;
309309
QUORUM_INTERSECTION_CHECKER_TIME_LIMIT_MS = 5000; // 5 secs
310+
// NB: this budget is compared against a conservative over-estimate of the
311+
// solver's memory (charged per clause/variable), not measured allocator
312+
// usage. Under the current tier-1 network configuration (7 orgs) usage
313+
// stays well under this limit, but the solver's encoding grows
314+
// combinatorially with a vertex's degree, so if the number of tier-1
315+
// organizations ever increases we will have to revisit this limit.
310316
QUORUM_INTERSECTION_CHECKER_MEMORY_LIMIT_BYTES =
311317
100 * 1024 * 1024; // 100 MiB
312318

src/main/main.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,6 @@ main(int argc, char* const* argv)
411411
// that would call std::terminate
412412
std::set_terminate(printBacktraceAndAbort);
413413

414-
rust_bridge::set_rust_global_memory_limit_to_unlimited();
415-
416414
Logging::init();
417415
if (sodium_init() != 0)
418416
{

src/rust/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ optional = true
190190
[dependencies.stellar-quorum-analyzer]
191191
version = "0.1.0"
192192
git = "https://github.com/stellar/stellar-quorum-analyzer"
193-
rev = "f67e9e2f080c2cc1a332d894c42277c64845e257"
193+
rev = "b422b366ede667c030c78ffe18b2010194698dbd"
194194

195195
[features]
196196

0 commit comments

Comments
 (0)