diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 254beec7f..fb1bb8dd2 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -439,6 +439,14 @@ RCLConsensus::Adaptor::onClose( // Bootstrap commit-reveal: generate entropy and include commitment // in our very first proposal so peers can collect it during consensus. + // + // This is gated on `proposing` — a node that just restarted enters + // as proposing=false (observing) and must watch at least one full + // round before consensus promotes it to proposing. During those + // observation rounds it cannot contribute to the RNG pipeline at + // all: no commitment, no reveal, no SHAMap entries. The surviving + // proposers will close those rounds with fewer commits (possibly + // falling back to ZERO entropy) until the rejoiner starts proposing. if (proposing && prevLedger->rules().enabled(featureConsensusEntropy)) { cacheActiveUNL(); diff --git a/src/ripple/consensus/Consensus.h b/src/ripple/consensus/Consensus.h index c260e83ea..67d80e987 100644 --- a/src/ripple/consensus/Consensus.h +++ b/src/ripple/consensus/Consensus.h @@ -863,6 +863,16 @@ Consensus::peerProposalInternal( // built our own local set for diffing. During ConvergingTx all // data arrives via proposal leaves — fetching a peer's commitSet // before we have our own just generates unnecessary traffic. + // + // IMPORTANT: SHAMap fetch/diff/merge is a safety net for the + // rare case where active proposers have slightly different + // commit/reveal sets due to dropped proposals. It does NOT + // help late-joining nodes: a node that restarts mid-round + // enters as proposing=false and cannot generate commitments + // (onClose gates on proposing). It must observe for at least + // one full round before consensus promotes it to proposing. + // The primary data transport is proposals themselves — the + // SHAMap sync is belt-and-suspenders, not the critical path. if constexpr (requires(Adaptor & a) { a.fetchRngSetIfNeeded(std::optional{}); }) @@ -1429,9 +1439,18 @@ Consensus::phaseEstablish() // Without this gate, execution falls through to the normal // consensus close logic and nodes inject partial/zero entropy // while others are still collecting — causing ledger mismatches. + // + // NOTE: Late-joining nodes (e.g. restarting after a crash) + // cannot help here. They enter the round as proposing=false + // and onClose() skips commitment generation for non-proposers. + // It takes at least one full round of observing before + // consensus promotes them to proposing. So waiting beyond + // a few seconds is pointless — use rngPIPELINE_TIMEOUT (3s) + // rather than ledgerMAX_CONSENSUS (10s) to avoid penalizing + // the recovery path. { bool timeout = - result_->roundTime.read() > parms.ledgerMAX_CONSENSUS; + result_->roundTime.read() > parms.rngPIPELINE_TIMEOUT; if (!timeout) return; // Wait for more commits // On timeout: fall through to normal close (zero entropy) @@ -1457,7 +1476,7 @@ Consensus::phaseEstablish() else if (estState_ == EstablishState::ConvergingReveal) { bool timeout = - result_->roundTime.read() > parms.ledgerMAX_CONSENSUS; + result_->roundTime.read() > parms.rngPIPELINE_TIMEOUT; bool ready = false; if ((haveConsensus() && adaptor_.hasMinimumReveals()) || timeout) diff --git a/src/ripple/consensus/ConsensusParms.h b/src/ripple/consensus/ConsensusParms.h index 542b3644b..868711d72 100644 --- a/src/ripple/consensus/ConsensusParms.h +++ b/src/ripple/consensus/ConsensusParms.h @@ -88,6 +88,23 @@ struct ConsensusParms */ std::chrono::milliseconds ledgerMAX_CONSENSUS = std::chrono::seconds{10}; + /** Maximum time to wait for RNG commit/reveal quorum before giving up. + * + * This is intentionally shorter than ledgerMAX_CONSENSUS because + * waiting longer won't help: a node that missed the start of the + * round (e.g. restarting after a crash) enters as proposing=false + * and cannot generate commitments until consensus promotes it to + * proposing — which takes at least one full round of observing. + * Waiting the full 10s just delays the inevitable ZERO-entropy + * fallback and slows recovery for the restarting node (it can't + * catch up until the survivors close a ledger). + * + * 3s is long enough for commits to propagate on any reasonable + * network, but short enough that a missing-node scenario recovers + * quickly via the ZERO-entropy fallback path. + */ + std::chrono::milliseconds rngPIPELINE_TIMEOUT = std::chrono::seconds{3}; + //! Minimum number of seconds to wait to ensure others have computed the LCL std::chrono::milliseconds ledgerMIN_CLOSE = std::chrono::seconds{2};