From 95bfc258e9f9c5586eab296e7fe8e7add6c2dbf3 Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Thu, 17 Sep 2026 15:26:55 +0700 Subject: [PATCH] fix(consensus): pin export candidacy to the round parent --- .../consensus/ConsensusExtensions_test.cpp | 24 ++++++---- .../app/consensus/ConsensusExtensions.cpp | 48 ++++++++----------- src/xrpld/app/consensus/ConsensusExtensions.h | 6 +++ src/xrpld/app/consensus/ExportIntent.md | 10 ++++ 4 files changed, 52 insertions(+), 36 deletions(-) diff --git a/src/test/consensus/ConsensusExtensions_test.cpp b/src/test/consensus/ConsensusExtensions_test.cpp index f025bd191a..d25319eb44 100644 --- a/src/test/consensus/ConsensusExtensions_test.cpp +++ b/src/test/consensus/ConsensusExtensions_test.cpp @@ -2753,7 +2753,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite return; auto validated = std::make_shared( *parent, env.app().timeKeeper().closeTime()); - auto const deadline = validated->info().seq; + auto const deadline = validated->info().seq + 1; auto const origin = makeHash("export-sidecar-deadline-origin"); auto latch = std::make_shared(keylet::exportLatch(alice.id(), origin)); @@ -2761,7 +2761,7 @@ class ConsensusExtensions_test : public beast::unit_test::suite latch->setFieldU32(sfTicketSequence, 1); latch->setFieldH256(sfTransactionHash, origin); latch->setFieldH256(sfDigest, makeHash("export-sidecar-intent")); - latch->setFieldU32(sfLedgerSequence, deadline); + latch->setFieldU32(sfLedgerSequence, validated->info().seq); latch->setFieldH256( sfExportCommitteeHash, validated->info().parentHash); latch->setFieldU32(sfLastLedgerSequence, deadline); @@ -2779,11 +2779,12 @@ class ConsensusExtensions_test : public beast::unit_test::suite auto const currentValidated = env.app().getLedgerMaster().getValidatedLedger(); BEAST_EXPECT( - currentValidated && currentValidated->info().seq == deadline); - if (!currentValidated || currentValidated->info().seq != deadline) + currentValidated && currentValidated->info().seq == deadline - 1); + if (!currentValidated || currentValidated->info().seq != deadline - 1) return; ConsensusExtensions ce{env.app(), activeNoopJournal()}; + ce.onRoundStart(RCLCxLedger{validated}, {}); auto& collector = ce.postValidationExportSigCollector(); auto const signer = randomKeyPair(KeyType::secp256k1).first; std::uint8_t const signatureBytes[] = {1, 2, 3}; @@ -2811,14 +2812,20 @@ class ConsensusExtensions_test : public beast::unit_test::suite return count; }; - // The validated view remains D in both cases. Candidate D is the + // The validated view remains D-1 in both cases. Candidate D is the // inclusive final publication opportunity; candidate D+1 is expired. - ce.buildingLedgerSeq_ = deadline; BEAST_EXPECT(ce.hasEligiblePendingExports()); BEAST_EXPECT(ce.hasPendingExportSigs()); BEAST_EXPECT(leafCount(ce.buildExportSigSet(deadline)) == 1); - ce.buildingLedgerSeq_ = deadline + 1; + auto nextParent = std::make_shared( + *validated, env.app().timeKeeper().closeTime()); + nextParent->updateSkipList(); + nextParent->setAccepted( + nextParent->info().closeTime, + nextParent->info().closeTimeResolution, + true); + ce.onRoundStart(RCLCxLedger{nextParent}, {}); BEAST_EXPECT(!ce.hasEligiblePendingExports()); BEAST_EXPECT(!ce.hasPendingExportSigs()); BEAST_EXPECT(leafCount(ce.buildExportSigSet(deadline + 1)) == 0); @@ -3116,7 +3123,8 @@ class ConsensusExtensions_test : public beast::unit_test::suite expected->getTransactionID()); // Production admission and production alignment, not a manually - // accepted map: reproduce the publication-to-next-tick race at 3248. + // accepted map: validation may overtake a round between candidate + // publication and the next peer-alignment tick. ConsensusExtensions aligning{env.app(), activeNoopJournal()}; aligning.onRoundStart(RCLCxLedger{parent}, {}); aligning.setRngEnabledThisRound(false); diff --git a/src/xrpld/app/consensus/ConsensusExtensions.cpp b/src/xrpld/app/consensus/ConsensusExtensions.cpp index 7eb849e26d..8f792b9201 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.cpp +++ b/src/xrpld/app/consensus/ConsensusExtensions.cpp @@ -1869,6 +1869,18 @@ ConsensusExtensions::buildEntropySet(LedgerIndex seq) return hash; } +std::map> +ConsensusExtensions::pendingRoundExports(LedgerIndex candidateSeq) const +{ + // This is reconstruction eligibility, not permission to admit or release + // a share. A newer validated ledger may already have witnessed an origin + // that is still pending in the parent of our in-flight round. + if (!roundParentLedger_ || candidateSeq == 0 || + roundParentLedger_->info().seq != candidateSeq - 1) + return {}; + return pendingExportLatches(*roundParentLedger_, candidateSeq); +} + uint256 ConsensusExtensions::buildExportSigSet(LedgerIndex seq) { @@ -1876,10 +1888,7 @@ ConsensusExtensions::buildExportSigSet(LedgerIndex seq) std::make_shared(SHAMapType::SIDECAR, app_.getNodeFamily()); map->setUnbacked(); - auto const validated = app_.getLedgerMaster().getValidatedLedger(); - auto const live = validated - ? pendingExportLatches(*validated, seq) - : std::map>{}; + auto const live = pendingRoundExports(seq); auto const allSigs = postValidationExportSigCollector_.fullUnionSnapshot(); std::size_t entryCount = 0; @@ -1923,6 +1932,8 @@ ConsensusExtensions::buildExportSigSet(LedgerIndex seq) // longer advertised, fetched, served, or merged from peers. app_.getInboundTransactions().giveSet(hash, map, false); + // The moving validated cursor is diagnostic only at this boundary. + auto const validated = app_.getLedgerMaster().getValidatedLedger(); JLOG(j_.debug()) << "Export: built exportSigSet SHAMap" << " hash=" << hash << " seq=" << seq << " entries=" << entryCount @@ -1938,17 +1949,7 @@ ConsensusExtensions::buildExportSigSet(LedgerIndex seq) bool ConsensusExtensions::hasPendingExportSigs() const { - auto const validated = app_.getLedgerMaster().getValidatedLedger(); - if (!validated) - return false; - auto candidateSeq = buildingLedgerSeq_; - if (!candidateSeq) - { - if (validated->info().seq == std::numeric_limits::max()) - return false; - candidateSeq = validated->info().seq + 1; - } - auto const live = pendingExportLatches(*validated, *candidateSeq); + auto const live = pendingRoundExports(buildingLedgerSeq_.value_or(0)); auto const allSigs = postValidationExportSigCollector_.fullUnionSnapshot(); return std::any_of(allSigs.begin(), allSigs.end(), [&](auto const& entry) { return live.find(entry.first) != live.end(); @@ -1958,17 +1959,7 @@ ConsensusExtensions::hasPendingExportSigs() const bool ConsensusExtensions::hasEligiblePendingExports() const { - auto const validated = app_.getLedgerMaster().getValidatedLedger(); - if (!validated) - return false; - auto candidateSeq = buildingLedgerSeq_; - if (!candidateSeq) - { - if (validated->info().seq == std::numeric_limits::max()) - return false; - candidateSeq = validated->info().seq + 1; - } - return !pendingExportLatches(*validated, *candidateSeq).empty(); + return !pendingRoundExports(buildingLedgerSeq_.value_or(0)).empty(); } void @@ -2198,6 +2189,7 @@ ConsensusExtensions::clearRngStatePreservingExport() acceptedEntropySetHash_.reset(); buildingLedgerSeq_.reset(); roundPrevLedgerHash_ = uint256{}; + roundParentLedger_.reset(); observedParticipantsHash_.reset(); observedParticipantsCount_ = 0; observedParticipantsBitmapBin_.clear(); @@ -2566,8 +2558,7 @@ ConsensusExtensions::onPreBuild( if (app_.config().standalone() && hasPendingExportSigs()) buildExportSigSet(seq); - auto const parent = - app_.getLedgerMaster().getLedgerByHash(roundPrevLedgerHash_); + auto const parent = roundParentLedger_; auto const validated = app_.getLedgerMaster().getValidatedLedger(); // Rebuild only from this round's exact parent and accepted evidence. // Local validation can lag that parent or already have passed it. @@ -3034,6 +3025,7 @@ ConsensusExtensions::onRoundStart( clearRngState(); roundPrevLedgerHash_ = prevLedger.ledger_->info().hash; + roundParentLedger_ = prevLedger.ledger_; buildingLedgerSeq_ = prevLedger.ledger_->info().seq + 1; cacheUNLReport(prevLedger.ledger_); auto const validatorView = activeValidatorView(); diff --git a/src/xrpld/app/consensus/ConsensusExtensions.h b/src/xrpld/app/consensus/ConsensusExtensions.h index ec93c57600..83e52e239b 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.h +++ b/src/xrpld/app/consensus/ConsensusExtensions.h @@ -175,6 +175,9 @@ private: // Consensus parent ledger hash, pinned at round start. Input to the // Tier 1 consensus_fallback entropy digest. uint256 roundPrevLedgerHash_; + // Immutable execution parent for this round. Export candidacy must not + // follow the asynchronously advancing validated-ledger cursor. + std::shared_ptr roundParentLedger_; // Parent-ledger validator view used by RNG and Export quorum logic. ActiveValidatorViewPtr activeValidatorView_ = std::make_shared(); @@ -201,6 +204,9 @@ public: bool exportSigConvergenceFailed_{false}; private: + std::map> + pendingRoundExports(LedgerIndex candidateSeq) const; + void clearRngStatePreservingExport(); diff --git a/src/xrpld/app/consensus/ExportIntent.md b/src/xrpld/app/consensus/ExportIntent.md index 0111773890..2a63b018ea 100644 --- a/src/xrpld/app/consensus/ExportIntent.md +++ b/src/xrpld/app/consensus/ExportIntent.md @@ -144,6 +144,16 @@ This rebuild rule does not authorize share admission, signing or release from a merely closed parent. Those live paths retain the source-validation checks in INV-4 and INV-5; an origin ahead of local validation remains deferred there. +Round candidacy uses that same immutable parent: pending-origin observation, +the local-share predicate, and signature-set construction all select pending +latches from the round parent at the candidate ledger sequence. Advancing the +validated cursor must not hide an origin between publication and root alignment +merely because a newer validated descendant already witnessed it. New rounds +select their own parent and candidates; they do not inherit old candidacy. +Without an active round parent, no Export candidate is inferred from the latest +validated ledger. Verified contributions may still arrive during the bounded window; +pinning candidacy neither freezes the collector early nor extends the deadline. + *Anti-pattern:* assembling from the current collector at apply time or treating peer root support as remote payload availability.