From 1cdf850e4a68a837a7580c39b91093ae2531fbdd Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Fri, 26 Jun 2026 19:35:07 +0700 Subject: [PATCH] fix(consensus): guard keyless proposal authoring --- .../consensus/ConsensusExtensions_test.cpp | 46 +++++++++++++++++++ .../app/consensus/ConsensusExtensions.cpp | 27 ++++++++++- src/xrpld/app/consensus/RCLConsensus.cpp | 17 +++++++ 3 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/test/consensus/ConsensusExtensions_test.cpp b/src/test/consensus/ConsensusExtensions_test.cpp index f84592440..0b0f047b9 100644 --- a/src/test/consensus/ConsensusExtensions_test.cpp +++ b/src/test/consensus/ConsensusExtensions_test.cpp @@ -3160,6 +3160,51 @@ class ConsensusExtensions_test : public beast::unit_test::suite BEAST_EXPECT(ce.exportSigCollector().signatureCount(tx) == 0); } + void + testValidatorKeylessAuthoringNoops() + { + testcase("validator-keyless extension authoring no-ops"); + + using namespace jtx; + Env env{ + *this, + envconfig(), + supported_amendments() | featureConsensusEntropy | featureExport, + nullptr}; + auto const& valKeys = env.app().getValidatorKeys(); + BEAST_EXPECT(!valKeys.keys); + + ConsensusExtensions ce{env.app(), activeNoopJournal()}; + auto const ledger = env.app().getLedgerMaster().getClosedLedger(); + + ExtendedPosition position{makeHash("keyless-authoring")}; + ce.decoratePosition(position, ledger, true); + BEAST_EXPECT(!position.myCommitment); + BEAST_EXPECT(ce.pendingCommitCount() == 0); + + ce.generateEntropySecret(); + ce.selfSeedReveal(); + BEAST_EXPECT(ce.pendingRevealCount() == 0); + + protocol::TMProposeSet prop; + RCLCxPeerPos::Proposal proposal{ + ledger->info().hash, + 0, + position, + NetClock::time_point{}, + NetClock::time_point{}, + beast::zero}; + ce.setExportEnabledThisRound(true); + ce.attachExportSignatures(prop, proposal); + BEAST_EXPECT(prop.exportsignatures_size() == 0); + + position.myCommitment = makeHash("keyless-commitment"); + position.myReveal = makeHash("keyless-reveal"); + ce.decorateMessage(prop, proposal, position, Buffer{}); + BEAST_EXPECT(ce.pendingRevealCount() == 0); + BEAST_EXPECT(ce.pendingCommitCount() == 0); + } + void testReplayedProposalHarvestsExportSigs() { @@ -3468,6 +3513,7 @@ public: testExportSigGateSkipsWhenExportDisabled(); testParticipantDiagnosticsOnlyWhenExtensionEnabled(); testExportDisabledRoundClearsCollector(); + testValidatorKeylessAuthoringNoops(); testReplayedProposalHarvestsExportSigs(); testWireProposalHarvestsExportSigs(); testPublicHookNoopAndFailureBranches(); diff --git a/src/xrpld/app/consensus/ConsensusExtensions.cpp b/src/xrpld/app/consensus/ConsensusExtensions.cpp index 22245e053..5764d6454 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.cpp +++ b/src/xrpld/app/consensus/ConsensusExtensions.cpp @@ -1009,6 +1009,9 @@ void ConsensusExtensions::selfSeedReveal() { auto const& valKeys = app_.getValidatorKeys(); + if (!valKeys.keys || valKeys.nodeID == beast::zero) + return; + if (myEntropySecret_ != uint256{}) { pendingReveals_[valKeys.nodeID] = myEntropySecret_; @@ -2446,11 +2449,22 @@ ConsensusExtensions::decoratePosition( return; } + auto const& valKeys = app_.getValidatorKeys(); + if (!valKeys.keys || valKeys.nodeID == beast::zero) + { + // Only validators author RNG sidecars. Observers still consume peer + // sidecars and diagnostics, but never seed local commit/reveal state. + JLOG(j_.debug()) << "RNG: decoratePosition skipped" + << " reason=no-validator-key" + << " prevLedgerSeq=" << prevLedger->info().seq + << " prevLedger=" << prevLedger->info().hash; + return; + } + setMode(ConsensusMode::proposing); cacheUNLReport(prevLedger); generateEntropySecret(); - auto const& valKeys = app_.getValidatorKeys(); pos.myCommitment = sha512Half( getEntropySecret(), valKeys.keys->publicKey, @@ -2505,6 +2519,15 @@ ConsensusExtensions::attachExportSignatures( if (!openLedger || !openLedger->rules().enabled(featureExport)) return; + if (!valKeys.keys || valKeys.nodeID == beast::zero) + { + // Export signatures are validator attestations. Non-validator nodes may + // relay proposals, but must not advertise locally authored signatures. + JLOG(j_.debug()) << "Export: skipping proposal signatures" + << " reason=no-validator-key"; + return; + } + auto const& valPK = valKeys.keys->publicKey; auto const& valSK = valKeys.keys->secretKey; // A locally configured validator may be trusted but not active for this @@ -2597,6 +2620,8 @@ ConsensusExtensions::decorateMessage( Buffer const& proposalSig) { auto const& valKeys = app_.getValidatorKeys(); + if (!valKeys.keys || valKeys.nodeID == beast::zero) + return; // Self-seed our own reveal so we count toward reveal quorum // (harvestRngData only sees peer proposals, not our own). diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 10f257dcb..7a59df436 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -249,6 +249,15 @@ RCLConsensus::Adaptor::share(RCLCxTx const& tx) void RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) { + if (!validatorKeys_.keys) + { + // Proposal packets are signed consensus messages. Observing nodes can + // follow consensus, but cannot safely author proposals without a + // configured validator key. + JLOG(j_.warn()) << "Skipping proposal without validator keys"; + return; + } + JLOG(j_.trace()) << (proposal.isBowOut() ? "We bow out: " : "We propose: ") << ripple::to_string(proposal.prevLedger()) << " -> " << ripple::to_string(proposal.position()); @@ -892,6 +901,14 @@ RCLConsensus::Adaptor::validate( RCLTxSet const& txns, bool proposing) { + if (!validatorKeys_.keys) + { + // preStartRound normally prevents this path. Keep validate() itself + // fail-closed so future call sites cannot dereference an observer key. + JLOG(j_.warn()) << "Skipping validation without validator keys"; + return; + } + using namespace std::chrono_literals; auto validationTime = app_.timeKeeper().closeTime();