From 4fb91ea9f5f90924ebbbcc04575315f554be8a64 Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Wed, 24 Jun 2026 18:58:24 +0700 Subject: [PATCH] refactor(consensus): expose threshold policy seams --- .../consensus/ConsensusExtensions_test.cpp | 66 ++++++++++++++++- src/test/consensus/NegativeUNL_test.cpp | 15 ++++ .../app/consensus/ConsensusExtensions.cpp | 61 ++++++++++----- src/xrpld/app/consensus/ConsensusExtensions.h | 20 +++++ src/xrpld/app/misc/NegativeUNLVote.cpp | 3 +- src/xrpld/app/misc/NegativeUNLVote.h | 14 +++- src/xrpld/app/tx/detail/Export.cpp | 3 +- src/xrpld/consensus/ConsensusExtensionsTick.h | 74 +++++++++++++++++-- src/xrpld/consensus/ConsensusParms.h | 23 ++++++ 9 files changed, 248 insertions(+), 31 deletions(-) diff --git a/src/test/consensus/ConsensusExtensions_test.cpp b/src/test/consensus/ConsensusExtensions_test.cpp index c1c40d8b7..20796e7fe 100644 --- a/src/test/consensus/ConsensusExtensions_test.cpp +++ b/src/test/consensus/ConsensusExtensions_test.cpp @@ -788,6 +788,19 @@ class ConsensusExtensions_test : public beast::unit_test::suite { testcase("Sidecar peer alignment helper"); + BEAST_EXPECT(detail::sidecarLocalContribution(true) == 1); + BEAST_EXPECT(detail::sidecarLocalContribution(false) == 0); + BEAST_EXPECT(detail::sidecarLocalContribution(true, true) == 1); + BEAST_EXPECT(detail::sidecarLocalContribution(false, true) == 0); + BEAST_EXPECT(detail::sidecarAlignedParticipants(2, true) == 3); + BEAST_EXPECT(detail::sidecarAlignedParticipants(2, false) == 2); + BEAST_EXPECT(detail::sidecarQuorumAligned(2, true, 3)); + BEAST_EXPECT(!detail::sidecarQuorumAligned(2, true, 4)); + BEAST_EXPECT(detail::sidecarFullObservation(2, 2)); + BEAST_EXPECT(!detail::sidecarFullObservation(2, 3)); + BEAST_EXPECT(detail::exportSigSetQuorumAligned(4, 4)); + BEAST_EXPECT(!detail::exportSigSetQuorumAligned(3, 4)); + ExportTickHarness harness; auto const localHash = makeHash("sidecar-local"); auto const conflictHash = makeHash("sidecar-conflict"); @@ -814,7 +827,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite fetched.push_back(*hash); }); - BEAST_EXPECT(state.localPublished); + BEAST_EXPECT(state.localCounts); BEAST_EXPECT(state.conflict); BEAST_EXPECT(state.aligned == 1); BEAST_EXPECT(state.alignedParticipants() == 2); @@ -835,7 +848,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite exportHashOf, allMembers, [](auto const&) {}); - BEAST_EXPECT(!unpublishedState.localPublished); + BEAST_EXPECT(!unpublishedState.localCounts); BEAST_EXPECT(unpublishedState.alignedParticipants() == 0); BEAST_EXPECT(!unpublishedState.quorumAligned(1)); BEAST_EXPECT(unpublishedState.fullObservation()); @@ -880,7 +893,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite exportHashOf, activeOnly, [](auto const&) {}); - BEAST_EXPECT(!nonActiveLocal.localPublished); + BEAST_EXPECT(!nonActiveLocal.localCounts); BEAST_EXPECT(nonActiveLocal.alignedParticipants() == 1); // node 1 only //@@end test-sidecar-active-view-filter } @@ -1112,6 +1125,52 @@ class ConsensusExtensions_test : public beast::unit_test::suite } } + void + testThresholdPolicyHelpers() + { + testcase("consensus-extension threshold policy helpers"); + + // Empty views fail closed at one participant. The raw formulas stay + // mathematical; sidecar gates use the safe wrappers. + BEAST_EXPECT(calculateQuorumThreshold(0) == 0); + BEAST_EXPECT(safeQuorumThreshold(0) == 1); + BEAST_EXPECT(safeQuorumThreshold(6) == calculateQuorumThreshold(6)); + BEAST_EXPECT(calculateParticipantThreshold(0) == 1); + BEAST_EXPECT(safeParticipantThreshold(0) == 1); + BEAST_EXPECT( + safeParticipantThreshold(10) == calculateParticipantThreshold(10)); + + // Gate threshold uses effective view for the 80% quorum and original + // view for the Tier-2 floor, then takes the lower enabled bar. + BEAST_EXPECT( + ConsensusExtensions::entropyGateThresholdForView(0, 0) == 1); + BEAST_EXPECT( + ConsensusExtensions::entropyGateThresholdForView(6, 6) == 4); + BEAST_EXPECT( + ConsensusExtensions::entropyGateThresholdForView(8, 10) == 7); + BEAST_EXPECT( + ConsensusExtensions::entropyGateThresholdForView(6, 10) == 5); + + // Tier labels require a ledger-anchored UNLReport view. With one, the + // ladder is validator_quorum first, then participant_aligned, then + // fallback. + BEAST_EXPECT( + ConsensusExtensions::selectEntropyTierForView(false, 99, 8, 10) == + entropyTierConsensusFallback); + BEAST_EXPECT( + ConsensusExtensions::selectEntropyTierForView(true, 7, 8, 10) == + entropyTierValidatorQuorum); + BEAST_EXPECT( + ConsensusExtensions::selectEntropyTierForView(true, 6, 8, 10) == + entropyTierConsensusFallback); + BEAST_EXPECT( + ConsensusExtensions::selectEntropyTierForView(true, 5, 8, 8) == + entropyTierParticipantAligned); + BEAST_EXPECT( + ConsensusExtensions::selectEntropyTierForView(true, 4, 8, 8) == + entropyTierConsensusFallback); + } + void testRuntimeConfigPolicyAccessors() { @@ -3291,6 +3350,7 @@ public: testActiveValidatorViewAppliesNegativeUNL(); testActiveValidatorViewNullSourceAndExpectedProposers(); testParticipantThreshold(); + testThresholdPolicyHelpers(); testRuntimeConfigPolicyAccessors(); testDecoratePositionGeneratesCommitment(); testOnPreBuildInjectsZeroEntropyFallback(); diff --git a/src/test/consensus/NegativeUNL_test.cpp b/src/test/consensus/NegativeUNL_test.cpp index 4aecfaac3..2f865c183 100644 --- a/src/test/consensus/NegativeUNL_test.cpp +++ b/src/test/consensus/NegativeUNL_test.cpp @@ -1613,6 +1613,20 @@ class NegativeUNLVoteOffline_test : public beast::unit_test::suite class NegativeUNLVoteMaxListed_test : public beast::unit_test::suite { + void + testMaxListedPolicy() + { + testcase("Max listed policy"); + + BEAST_EXPECT(NegativeUNLVote::maxNegativeUNLListed(0) == 0); + BEAST_EXPECT(NegativeUNLVote::maxNegativeUNLListed(1) == 1); + BEAST_EXPECT(NegativeUNLVote::maxNegativeUNLListed(4) == 1); + BEAST_EXPECT(NegativeUNLVote::maxNegativeUNLListed(5) == 2); + BEAST_EXPECT(NegativeUNLVote::maxNegativeUNLListed(8) == 2); + BEAST_EXPECT(NegativeUNLVote::maxNegativeUNLListed(9) == 3); + BEAST_EXPECT(NegativeUNLVote::maxNegativeUNLListed(32) == 8); + } + void testDoVoting() { @@ -1640,6 +1654,7 @@ class NegativeUNLVoteMaxListed_test : public beast::unit_test::suite void run() override { + testMaxListedPolicy(); testDoVoting(); } }; diff --git a/src/xrpld/app/consensus/ConsensusExtensions.cpp b/src/xrpld/app/consensus/ConsensusExtensions.cpp index ebf97c8d7..658e32ae2 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.cpp +++ b/src/xrpld/app/consensus/ConsensusExtensions.cpp @@ -215,23 +215,19 @@ ConsensusExtensions::quorumThreshold() const // Use the shared validator view so Tier 3 RNG and Export use the same // denominator. auto const base = activeValidatorView()->size(); - if (base == 0) - return 1; // safety: need at least one commit - return calculateQuorumThreshold(base); + return safeQuorumThreshold(base); } std::size_t ConsensusExtensions::exportSigQuorumThreshold() const { auto const base = activeValidatorView()->size(); - if (base == 0) - return 1; // Export sidecar hashes are signed through ExtendedPosition even when RNG // is disabled, so a quorum-aligned exportSigSetHash is deterministic // enough for Export-only mode. Unanimity would let one active validator // veto an otherwise converged export round. - return calculateQuorumThreshold(base); + return safeQuorumThreshold(base); } std::size_t @@ -248,9 +244,7 @@ ConsensusExtensions::tier2Threshold() const // Regressing this to size() is a consensus fork under nUNL and is pinned by // ConsensusExtensions_test::testTier2ThresholdAnchorsToOriginalView. auto const base = activeValidatorView()->originalViewSize; - if (base == 0) - return 1; // safety: need at least one aligned participant - return calculateParticipantThreshold(base); + return safeParticipantThreshold(base); } std::size_t @@ -267,7 +261,39 @@ ConsensusExtensions::entropyGateThreshold() const // allowed only when the round view is anchored by UNLReport; the // trusted-fallback view is local configuration and selectEntropy() maps it // to consensus_fallback. - return std::min(quorumThreshold(), tier2Threshold()); + auto const view = activeValidatorView(); + return entropyGateThresholdForView(view->size(), view->originalViewSize); +} + +std::size_t +ConsensusExtensions::entropyGateThresholdForView( + std::size_t effectiveViewSize, + std::size_t originalViewSize) +{ + auto const quorum = safeQuorumThreshold(effectiveViewSize); + auto const tier2 = safeParticipantThreshold(originalViewSize); + return std::min(quorum, tier2); +} + +EntropyTier +ConsensusExtensions::selectEntropyTierForView( + bool fromUNLReport, + std::size_t participantCount, + std::size_t effectiveViewSize, + std::size_t originalViewSize) +{ + if (!fromUNLReport) + return entropyTierConsensusFallback; + + auto const quorum = safeQuorumThreshold(effectiveViewSize); + if (participantCount >= quorum) + return entropyTierValidatorQuorum; + + auto const tier2 = safeParticipantThreshold(originalViewSize); + if (participantCount >= tier2) + return entropyTierParticipantAligned; + + return entropyTierConsensusFallback; } void @@ -349,9 +375,7 @@ bool ConsensusExtensions::hasQuorumOfCommits() const { auto const validatorView = activeValidatorView(); - auto const threshold = validatorView->size() == 0 - ? std::size_t{1} - : calculateQuorumThreshold(validatorView->size()); + auto const threshold = safeQuorumThreshold(validatorView->size()); auto const proofedCommitCount = std::count_if( pendingCommits_.begin(), pendingCommits_.end(), @@ -534,10 +558,13 @@ ConsensusExtensions::selectEntropy( // floor over the original view (~0.6*n; see calculateParticipantThreshold). // Below tier2Threshold too few aligned participants contributed to trust // the result — fall back. - if (count >= quorumThreshold()) - return {digest, entropyTierValidatorQuorum, count}; - if (count >= tier2Threshold()) - return {digest, entropyTierParticipantAligned, count}; + auto const tier = selectEntropyTierForView( + validatorView->fromUNLReport, + count, + validatorView->size(), + validatorView->originalViewSize); + if (tier != entropyTierConsensusFallback) + return {digest, static_cast(tier), count}; return fallback(); //@@end entropy-selector-tier-ladder } diff --git a/src/xrpld/app/consensus/ConsensusExtensions.h b/src/xrpld/app/consensus/ConsensusExtensions.h index 65f5dea7d..891d01e90 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.h +++ b/src/xrpld/app/consensus/ConsensusExtensions.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -174,6 +175,25 @@ public: std::size_t entropyGateThreshold() const; + /// Pure threshold helper used by the live gate and optional drift tests. + /// `effectiveViewSize` is post-nUNL; `originalViewSize` is the pre-nUNL + /// UNLReport active count. + static std::size_t + entropyGateThresholdForView( + std::size_t effectiveViewSize, + std::size_t originalViewSize); + + /// Pure selector helper for the tier label only. The digest/count bytes are + /// still derived from the agreed entropySetMap_ in selectEntropy(); this + /// isolates the policy that says which EntropyTier a given agreed count + /// earns under a ledger-anchored validator view. + static EntropyTier + selectEntropyTierForView( + bool fromUNLReport, + std::size_t participantCount, + std::size_t effectiveViewSize, + std::size_t originalViewSize); + void setExpectedProposers(hash_set proposers); diff --git a/src/xrpld/app/misc/NegativeUNLVote.cpp b/src/xrpld/app/misc/NegativeUNLVote.cpp index ba12fc781..d875f6e5a 100644 --- a/src/xrpld/app/misc/NegativeUNLVote.cpp +++ b/src/xrpld/app/misc/NegativeUNLVote.cpp @@ -368,8 +368,7 @@ NegativeUNLVote::findAllCandidates( { // Compute if need to find more validators to disable auto const canAdd = [&]() -> bool { - auto const maxNegativeListed = static_cast( - std::ceil(unl.size() * negativeUNLMaxListed)); + auto const maxNegativeListed = maxNegativeUNLListed(unl.size()); std::size_t negativeListed = 0; for (auto const& n : unl) { diff --git a/src/xrpld/app/misc/NegativeUNLVote.h b/src/xrpld/app/misc/NegativeUNLVote.h index cb60d7307..6bf9f47b2 100644 --- a/src/xrpld/app/misc/NegativeUNLVote.h +++ b/src/xrpld/app/misc/NegativeUNLVote.h @@ -74,7 +74,19 @@ public: /** * We only want to put 25% of the UNL on the NegativeUNL. */ - static constexpr float negativeUNLMaxListed = 0.25; + static constexpr size_t negativeUNLMaxListedDenominator = 4; + + /** Maximum number of validators allowed on the NegativeUNL for a given + * UNL size, rounded up. Kept as integer policy arithmetic so consensus + * tests compare against the production helper, not a copied + * floating-point expression. + */ + static constexpr size_t + maxNegativeUNLListed(size_t unlSize) + { + return (unlSize + negativeUNLMaxListedDenominator - 1) / + negativeUNLMaxListedDenominator; + } /** * A flag indicating whether a UNLModify Tx is to disable or to re-enable diff --git a/src/xrpld/app/tx/detail/Export.cpp b/src/xrpld/app/tx/detail/Export.cpp index 59e463d0b..1389aa2d8 100644 --- a/src/xrpld/app/tx/detail/Export.cpp +++ b/src/xrpld/app/tx/detail/Export.cpp @@ -173,8 +173,7 @@ Export::doApply() if (!ctx_.app.config().standalone()) { //@@start export-doapply-agreed-signature-snapshot - std::size_t const threshold = - unlSize == 0 ? 1 : calculateQuorumThreshold(unlSize); + std::size_t const threshold = safeQuorumThreshold(unlSize); if (!validatorView->fromUNLReport) { diff --git a/src/xrpld/consensus/ConsensusExtensionsTick.h b/src/xrpld/consensus/ConsensusExtensionsTick.h index a32ff335d..8aae1f349 100644 --- a/src/xrpld/consensus/ConsensusExtensionsTick.h +++ b/src/xrpld/consensus/ConsensusExtensionsTick.h @@ -10,9 +10,70 @@ namespace ripple { namespace detail { +inline std::size_t +sidecarLocalContribution(bool localCounts) +{ + return localCounts ? 1 : 0; +} + +inline std::size_t +sidecarLocalContribution(bool localIsMember, bool localPublished) +{ + return sidecarLocalContribution(localIsMember && localPublished); +} + +inline std::size_t +sidecarAlignedParticipants(std::size_t aligned, bool localCounts) +{ + return aligned + sidecarLocalContribution(localCounts); +} + +inline std::size_t +sidecarAlignedParticipants( + std::size_t aligned, + bool localIsMember, + bool localPublished) +{ + return aligned + sidecarLocalContribution(localIsMember, localPublished); +} + +inline bool +sidecarQuorumAligned( + std::size_t aligned, + bool localCounts, + std::size_t threshold) +{ + return sidecarAlignedParticipants(aligned, localCounts) >= threshold; +} + +inline bool +sidecarQuorumAligned( + std::size_t aligned, + bool localIsMember, + bool localPublished, + std::size_t threshold) +{ + return sidecarAlignedParticipants(aligned, localIsMember, localPublished) >= + threshold; +} + +inline bool +sidecarFullObservation(std::size_t peersSeen, std::size_t txConverged) +{ + return peersSeen == txConverged; +} + +inline bool +exportSigSetQuorumAligned( + std::size_t alignedParticipants, + std::size_t quorumThreshold) +{ + return alignedParticipants >= quorumThreshold; +} + struct SidecarPeerAlignment { - bool localPublished = false; + bool localCounts = false; bool conflict = false; std::size_t aligned = 0; std::size_t peersSeen = 0; @@ -21,19 +82,19 @@ struct SidecarPeerAlignment std::size_t alignedParticipants() const { - return aligned + (localPublished ? 1 : 0); + return sidecarAlignedParticipants(aligned, localCounts); } bool quorumAligned(std::size_t quorum) const { - return alignedParticipants() >= quorum; + return sidecarQuorumAligned(aligned, localCounts, quorum); } bool fullObservation() const { - return peersSeen == txConverged; + return sidecarFullObservation(peersSeen, txConverged); } }; @@ -66,7 +127,7 @@ inspectTxConvergedSidecarPeers( // margin (2t - N) below the Byzantine floor f, breaking equivocation // uniqueness. Mirror buildEntropySet/hasQuorumOfCommits' containsNode // filter, and only count our own +1 when this node is itself active. - state.localPublished = localIsMember; + state.localCounts = localIsMember; for (auto const& [nodeId, peerPos] : peerPositions) { if (!isMember(nodeId)) @@ -1104,7 +1165,8 @@ extensionsTick(Ext& ext, Ctx const& ctx) auto exportState = inspectExportPeers(ctx.getPosition(), true); auto const exportQuorum = ext.exportSigQuorumThreshold(); auto quorumAligned = [&] { - return exportState.quorumAligned(exportQuorum); + return detail::exportSigSetQuorumAligned( + exportState.alignedParticipants(), exportQuorum); }; //@@start export-sigset-alignment-check if (exportState.conflict && !quorumAligned()) diff --git a/src/xrpld/consensus/ConsensusParms.h b/src/xrpld/consensus/ConsensusParms.h index cf0ec751d..caa5b809b 100644 --- a/src/xrpld/consensus/ConsensusParms.h +++ b/src/xrpld/consensus/ConsensusParms.h @@ -229,6 +229,18 @@ calculateQuorumThreshold(std::size_t count) return (count * 80 + 99) / 100; } +/** Safe quorum helper for consensus-extension gates. + + The raw quorum formula returns zero for an empty view. Consensus-extension + sidecar gates use a fail-closed floor of one participant instead: an empty + local view must not make a sidecar quorum vacuously true. +*/ +inline std::size_t +safeQuorumThreshold(std::size_t count) +{ + return count == 0 ? 1 : calculateQuorumThreshold(count); +} + /** Calculate the Tier 2 (participant_aligned) alignment floor. Tier 2 sub-quorum entropy aligns a cohort at this lower bar. The floor is @@ -265,6 +277,17 @@ calculateParticipantThreshold(std::size_t count) return (count + byzantine) / 2 + 1; } +/** Safe Tier-2 helper for consensus-extension gates. + + Like safeQuorumThreshold(), this floors empty-view sidecar gates at one + participant so they fail closed instead of succeeding vacuously. +*/ +inline std::size_t +safeParticipantThreshold(std::size_t count) +{ + return count == 0 ? 1 : calculateParticipantThreshold(count); +} + inline std::pair> getNeededWeight( ConsensusParms const& p,