Skip to content

Commit 4666dc0

Browse files
committed
Update stellar-quorum-analyzer to master and handle NoQuorum status
Bump the stellar-quorum-analyzer dependency from f67e9e2f to b422b366 (current master) and adapt stellar-core to its two behavioral changes. The analyzer now reports a distinct NoQuorum status when the FBAS contains no quorum at all (a degenerate / potential-halt configuration) instead of conflating it with UNSAT ("enjoys quorum intersection"). Surface this as a new QuorumCheckerStatus::NO_QUORUM (103) and propagate it through the subprocess exit code, result.json, the adaptor state and metrics (new result-no-quorum counter), and the CLI. A NoQuorum result is treated as a complete run but never advances mLastGoodLedger, so the network correctly does not report as enjoying quorum intersection. The analyzer also replaced its global-allocator hard memory limit (which aborted the process on exceed) with a soft, estimate-based per-solver limit. Remove the now-dead set_rust_global_memory_limit_to_unlimited() and update the stale comments describing the abort-based design. Update the null-qset quorum intersection tests, which previously asserted these halted networks vacuously enjoy quorum intersection, to expect NO_QUORUM, and add a dedicated no-quorum test case.
1 parent d6f2546 commit 4666dc0

9 files changed

Lines changed: 162 additions & 52 deletions

File tree

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: 32 additions & 7 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.
@@ -579,17 +598,23 @@ runQuorumIntersectionCheckAsync(
579598

580599
if (ecode == static_cast<int>(QuorumCheckerStatus::UNSAT) ||
581600
ecode == static_cast<int>(QuorumCheckerStatus::SAT) ||
582-
ecode == static_cast<int>(QuorumCheckerStatus::UNKNOWN))
601+
ecode == static_cast<int>(QuorumCheckerStatus::UNKNOWN) ||
602+
ecode == static_cast<int>(QuorumCheckerStatus::NO_QUORUM))
583603
{
584604
try
585605
{
586606
auto res = parseResultsJson(qicResultJson);
587607
hStateSP->mStatus = fromQuorumCheckerStatusJson(res["status"]);
588608
QuorumCheckerMetrics metrics(res["metrics"]);
589609
metrics.flush(hStateSP->mMetrics);
590-
// only update the following info if we had a complete run
610+
// only update the following info if we had a complete run.
611+
// NO_QUORUM is a complete result (the checker definitively
612+
// determined the FBAS has no quorum) so we record it too, but
613+
// unlike UNSAT it is not a "good" result and must not advance
614+
// mLastGoodLedger.
591615
if (ecode == static_cast<int>(QuorumCheckerStatus::UNSAT) ||
592-
ecode == static_cast<int>(QuorumCheckerStatus::SAT))
616+
ecode == static_cast<int>(QuorumCheckerStatus::SAT) ||
617+
ecode == static_cast<int>(QuorumCheckerStatus::NO_QUORUM))
593618
{
594619
hStateSP->mNumNodes = numNodes;
595620
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/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

src/rust/src/bridge.rs

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ pub(crate) mod rust_bridge {
181181
UNSAT = 0,
182182
SAT = 101,
183183
UNKNOWN = 102,
184+
NO_QUORUM = 103,
184185
}
185186

186187
struct QuorumSplit {
@@ -363,25 +364,27 @@ pub(crate) mod rust_bridge {
363364
// - `QuorumCheckerStatus::UNKNOWN`. Solver could not finish, possibly
364365
// due to reaching some internal limits (e.g. num conflicts) not
365366
// including resource limits (see "Resource limits and errors")
367+
// - `QuorumCheckerStatus::NO_QUORUM` if the FBAS contains no quorum at
368+
// all (a degenerate / potential-halt configuration). The
369+
// disjoint-quorum question is vacuously unsatisfiable here, but this
370+
// must NOT be conflated with `UNSAT` ("a quorum exists and all
371+
// quorums intersect"): a network with no quorum does not enjoy
372+
// quorum intersection.
366373
//
367374
// Resource limits and errors:
368375
//
369376
// The quorum checker accepts two limits (passed via `resource_limit`),
370-
// time (ms) and memory (bytes). The time limit is enforced internally
371-
// via code logic, once exceeds, returns a solver error. The memory
372-
// limit is enforced by a global memory allocator, and if exceeded, will
373-
// abort the program. In other words, memory limit is a hard, system
374-
// enforced limit.
377+
// time (ms) and memory (bytes). Both are enforced internally via code
378+
// logic; once exceeded, the solver returns an error. Memory is not
379+
// measured directly but conservatively *estimated* from the solver's
380+
// clause/variable counts, so the limit is a soft, per-solver limit
381+
// rather than a hard, process-global one.
375382
//
376383
// Errors:
377-
// - if resource limits (not including memory) have been exceeded.
378-
// - any other solver error In either sucess or error case, the
384+
// - if resource limits (time or estimated memory) have been exceeded.
385+
// - any other solver error. In either success or error case, the
379386
// `resource_usage` will be updated with the actual resource
380387
// consumption.
381-
//
382-
// Aborts:
383-
// - if the memory limit has been exceeded
384-
// Abort is non-recoverable (it cannot be caught by catch_unwind)
385388
fn network_enjoys_quorum_intersection(
386389
nodes: &Vec<CxxBuf>,
387390
quorum_set: &Vec<CxxBuf>,
@@ -390,13 +393,6 @@ pub(crate) mod rust_bridge {
390393
resource_usage: &mut QuorumCheckerResource,
391394
) -> Result<QuorumCheckerStatus>;
392395

393-
// The QI checker actually manages the memory limit using a global
394-
// allocator, which winds up controlling _all_ memory allocation by
395-
// rust code in the process. So we want to ensure that limit is unlimited
396-
// when the process starts up -- the QI check call will limit it later,
397-
// if and only if it's running as a QI-checking subprocess.
398-
fn set_rust_global_memory_limit_to_unlimited();
399-
400396
// Soroban fuzzing support - always declared but only functional with --features fuzz.
401397
// Panics on internal errors (which libfuzzer will catch as crashes).
402398
fn run_soroban_fuzz_target(name: &str, data: &[u8]) -> FuzzResultCode;

0 commit comments

Comments
 (0)