refactor(rng): address review minors (dead-API rename + explicit-final dedup)

Two small follow-ups from the Codex review of the tier-2 implementation, both
on the consensus-extension internals:

- Rename shouldZeroEntropy() -> belowValidatorQuorum(). The selector replaced its
  callers, leaving it production-dead and MISNAMED: post-tier-2, "below the 80%
  validator quorum" is no longer "zero entropy" (a participant_aligned set is
  sub-quorum but non-zero). The new name + a doc comment make it a tier-3
  eligibility predicate only and warn against gating injection on it
  (selectEntropy() owns the tiering). Behavior unchanged.

- buildExplicitFinalProposalTxSet now dedups the entropy pseudo-tx by VALUE, not
  type. Its comment claimed it "mirrors onPreBuild", but onPreBuild went
  value-based: it verifies a present pseudo-tx is the EXACT txID it would have
  produced and logs a determinism-violation on mismatch. Explicit-final now does
  the same (skip-duplicate-verified on match, error log on mismatch), returning
  the base unchanged either way. Default-off experimental path, so low blast
  radius, but the two paths now agree.
This commit is contained in:
Nicholas Dudfield
2026-06-15 17:14:30 +07:00
parent 14ebe74a56
commit 156a8cbb85
3 changed files with 34 additions and 17 deletions

View File

@@ -1161,7 +1161,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite
auto const ledger = env.app().getLedgerMaster().getClosedLedger();
ce.onRoundStart(RCLCxLedger{ledger}, {});
ce.setRngEnabledThisRound(true);
BEAST_EXPECT(ce.shouldZeroEntropy());
BEAST_EXPECT(ce.belowValidatorQuorum());
CanonicalTXSet retriableTxs{makeHash("rng-zero-fallback-salt")};
auto const seq = ledger->seq() + 1;
@@ -1239,7 +1239,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite
auto const entropySetHash = ce.buildEntropySet(seq);
BEAST_EXPECT(ce.isSidecarSet(entropySetHash));
BEAST_EXPECT(!ce.shouldZeroEntropy());
BEAST_EXPECT(!ce.belowValidatorQuorum());
CanonicalTXSet retriableTxs{makeHash("entropy-set-prebuild-salt")};
ce.onPreBuild(retriableTxs, seq, txSetHash);
@@ -2809,7 +2809,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite
BEAST_EXPECT(ce.exportSigConvergenceFailed());
ce.setEntropyFailed();
BEAST_EXPECT(ce.shouldZeroEntropy());
BEAST_EXPECT(ce.belowValidatorQuorum());
ce.generateEntropySecret();
BEAST_EXPECT(!ce.hasAnyReveals());

View File

@@ -413,7 +413,7 @@ ConsensusExtensions::hasAnyReveals() const
}
bool
ConsensusExtensions::shouldZeroEntropy() const
ConsensusExtensions::belowValidatorQuorum() const
{
if (entropyFailed_ || !entropySetMap_)
return true;
@@ -452,9 +452,9 @@ ConsensusExtensions::selectEntropy(
// No agreed entropy set (round failed, or none was built) → fallback. We do
// NOT fall back merely for being below the 80% quorum the way
// shouldZeroEntropy() does: a sub-quorum-but-aligned set may still qualify
// for participant_aligned (tier 2). The tier ladder below decides from the
// agreed participant count.
// belowValidatorQuorum() does: a sub-quorum-but-aligned set may still
// qualify for participant_aligned (tier 2). The tier ladder below decides
// from the agreed participant count.
if (entropyFailed_ || !entropySetMap_)
return fallback();
@@ -591,29 +591,42 @@ ConsensusExtensions::buildExplicitFinalProposalTxSet(
});
auto const txID = tx.getTransactionID();
// Dedup by type (mirrors onPreBuild): there must never be two entropy
// pseudo-txs, and a fallback digest derived from a different base set
// hash would not match by exact txID.
bool alreadyPresent = false;
// Value-based dedup (mirrors onPreBuild): there must never be two entropy
// pseudo-txs. If one is already in the base set it must be the EXACT
// pseudo-tx we would have produced (injection is deterministic, so the same
// agreed inputs yield an identical txID); a present-but-different one is a
// determinism violation to surface, not silently accept. Either way return
// the base unchanged — explicit-final is best-effort and must not rewrite
// an already-committed set.
std::optional<uint256> presentID;
txns.map_->visitLeaves(
[&](boost::intrusive_ptr<SHAMapItem const> const& item) {
if (alreadyPresent)
if (presentID)
return;
try
{
SerialIter sit(item->slice());
STTx const parsed{sit};
if (parsed.getTxnType() == ttCONSENSUS_ENTROPY)
alreadyPresent = true;
presentID = parsed.getTransactionID();
}
catch (...)
{
}
});
if (alreadyPresent)
if (presentID)
{
JLOG(j_.debug()) << "RNGFINAL: entropy pseudo-tx already in base txSet"
<< " txHash=" << txID << " baseTxSet=" << txns.id();
if (*presentID == txID)
JLOG(j_.debug())
<< "RNGFINAL: entropy pseudo-tx already in base txSet"
<< " txHash=" << txID << " baseTxSet=" << txns.id()
<< " action=skip-duplicate-verified";
else
JLOG(j_.error())
<< "RNGFINAL: entropy pseudo-tx MISMATCH in base txSet"
<< " reason=determinism-violation action=keep-base"
<< " ourTxHash=" << txID << " presentTxHash=" << *presentID
<< " baseTxSet=" << txns.id();
return txns;
}

View File

@@ -189,8 +189,12 @@ public:
bool
hasAnyReveals() const;
/// True when the agreed entropy set would NOT reach validator_quorum
/// (tier 3): absent/failed, or fewer leaves than quorumThreshold(). A
/// tier-3 eligibility predicate only — it does NOT account for the tier-2
/// floor, so never gate injection on it (selectEntropy() does the tiering).
bool
shouldZeroEntropy() const;
belowValidatorQuorum() const;
/// Result of the shared deterministic entropy selector: the digest to
/// inject plus its tier/count labels. Both injection paths derive these