From 92bdd2ed9fc2f12dcdd1fc46d785b386e004b0aa Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Tue, 28 Apr 2026 10:16:52 +0700 Subject: [PATCH] fix(consensus): harvest replayed export signatures --- .../consensus/ConsensusExtensions_test.cpp | 59 ++++++++++ .../app/consensus/ConsensusExtensions.cpp | 103 ++++++++++++------ src/xrpld/app/consensus/ConsensusExtensions.h | 14 ++- src/xrpld/consensus/Consensus.h | 31 ++++-- 4 files changed, 166 insertions(+), 41 deletions(-) diff --git a/src/test/consensus/ConsensusExtensions_test.cpp b/src/test/consensus/ConsensusExtensions_test.cpp index a052f3018..e1575ceae 100644 --- a/src/test/consensus/ConsensusExtensions_test.cpp +++ b/src/test/consensus/ConsensusExtensions_test.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -48,6 +49,17 @@ makeNode(std::uint8_t id) return node; } +std::string +makeExportSigBlob(uint256 const& txHash, PublicKey const& publicKey) +{ + std::string blob; + blob.append(reinterpret_cast(txHash.data()), uint256::size()); + blob.append( + reinterpret_cast(publicKey.data()), publicKey.size()); + blob.push_back('\x30'); + return blob; +} + struct FakeTxSet { using ID = uint256; @@ -492,6 +504,52 @@ class ConsensusExtensions_test : public beast::unit_test::suite BEAST_EXPECT(ce.exportSigCollector().signatureCount(tx) == 0); } + void + testReplayedProposalHarvestsExportSigs() + { + testcase("Replayed proposal harvests export signatures"); + + using namespace jtx; + Env env{ + *this, envconfig(validator, ""), supported_amendments(), nullptr}; + auto const& valKeys = env.app().getValidatorKeys(); + BEAST_EXPECT(valKeys.keys); + if (!valKeys.keys) + return; + + ConsensusExtensions ce{env.app(), env.journal}; + ce.setExportEnabledThisRound(true); + ce.cacheUNLReport(); + + auto const activeView = ce.activeValidatorView(); + BEAST_EXPECT(activeView->sourceLedgerHash); + if (!activeView->sourceLedgerHash) + return; + + auto const senderPK = valKeys.keys->publicKey; + BEAST_EXPECT(ce.isActiveValidator(senderPK, *activeView)); + if (!ce.isActiveValidator(senderPK, *activeView)) + return; + + auto const tx = makeHash("replayed-export-sig-tx"); + auto const blob = makeExportSigBlob(tx, senderPK); + ExtendedPosition position{makeHash("replayed-position")}; + position.exportSignaturesHash = + proposalExportSignaturesHash(std::vector{blob}); + + ce.onTrustedPeerProposal( + calcNodeID(senderPK), + senderPK, + position, + 0, + NetClock::time_point{}, + *activeView->sourceLedgerHash, + Slice{}, + std::vector{blob}); + + BEAST_EXPECT(ce.exportSigCollector().hasUnverifiedSignatures()); + } + public: void run() override @@ -502,6 +560,7 @@ public: testExportSigGateFetchesAdvertisedPeerSets(); testExportSigGateSkipsWhenExportDisabled(); testExportDisabledRoundClearsCollector(); + testReplayedProposalHarvestsExportSigs(); } }; diff --git a/src/xrpld/app/consensus/ConsensusExtensions.cpp b/src/xrpld/app/consensus/ConsensusExtensions.cpp index 2754370a3..ad30d314b 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.cpp +++ b/src/xrpld/app/consensus/ConsensusExtensions.cpp @@ -1804,7 +1804,8 @@ ConsensusExtensions::onTrustedPeerProposal( std::uint32_t proposeSeq, NetClock::time_point closeTime, uint256 const& prevLedger, - Slice const& signature) + Slice const& signature, + std::vector const& exportSignatures) { harvestRngData( nodeId, @@ -1814,6 +1815,17 @@ ConsensusExtensions::onTrustedPeerProposal( closeTime, prevLedger, signature); + + // Stored future/wrong-ledger proposals are replayed through this path, not + // through PeerImp's original protobuf packet. Re-harvest signed export + // blobs here so they are evaluated against the active view for this parent. + if (!exportSignatures.empty() && position.exportSignaturesHash && + proposalExportSignaturesHash(exportSignatures) == + *position.exportSignaturesHash) + { + harvestExportSignatures( + publicKey, prevLedger, exportSignatures, "stored proposal"); + } } void @@ -1875,26 +1887,27 @@ ConsensusExtensions::logPosition( << " myReveal=" << (pos.myReveal ? "yes" : "no"); } -//@@start peer-harvest-export-sigs -void -ConsensusExtensions::onTrustedPeerMessage( - ::protocol::TMProposeSet const& wireMsg) +std::size_t +ConsensusExtensions::harvestExportSignatures( + PublicKey const& senderPK, + uint256 const& proposalPrevLedger, + std::vector const& exportSignatures, + char const* source) { if (!exportEnabled()) - return; + return 0; - if (wireMsg.exportsignatures_size() == 0) - return; + if (exportSignatures.empty()) + return 0; // Cap the number of export sig entries per proposal to bound DoS // surface. Honest validators attach at most maxPendingExports sigs. - if (wireMsg.exportsignatures_size() > ExportLimits::maxPendingExports) + if (exportSignatures.size() > ExportLimits::maxPendingExports) { JLOG(j_.warn()) << "Export: rejecting proposal with " - << wireMsg.exportsignatures_size() - << " export sigs (max " + << exportSignatures.size() << " export sigs (max " << +ExportLimits::maxPendingExports << ")"; - return; + return 0; } // Bind export sig pubkeys to the proposal sender. Validators only @@ -1905,38 +1918,24 @@ ConsensusExtensions::onTrustedPeerMessage( // // Two-pass: validate all blobs first, then commit — ensures no partial // state if a later blob fails the sender binding check. - auto const senderSlice = makeSlice(wireMsg.nodepubkey()); - if (!publicKeyType(senderSlice)) - return; - PublicKey const senderPK{senderSlice}; - auto const validatorView = activeValidatorView(); // Proposal ingress is outside the consensus mutex, so take a snapshot of // the shared active view and reject trusted-but-inactive signers here. if (!isActiveValidator(senderPK, *validatorView)) - return; + return 0; // The active view is pinned to one parent ledger. Do not let a proposal // for another parent feed signatures into this round's export collector; // build, merge, and apply all count against this same parent-ledger view. if (validatorView->sourceLedgerHash) { - if (wireMsg.previousledger().size() != uint256::size()) - return; - - uint256 proposalPrevLedger; - std::memcpy( - proposalPrevLedger.data(), - wireMsg.previousledger().data(), - uint256::size()); if (proposalPrevLedger != *validatorView->sourceLedgerHash) - return; + return 0; } // Pass 1: validate all blobs. - for (int i = 0; i < wireMsg.exportsignatures_size(); ++i) + for (auto const& blob : exportSignatures) { - auto const& blob = wireMsg.exportsignatures(i); if (blob.size() < 65) continue; @@ -1949,7 +1948,7 @@ ConsensusExtensions::onTrustedPeerMessage( JLOG(j_.warn()) << "Export: rejecting sigs from proposal — embedded pubkey " "does not match sender"; - return; + return 0; } } @@ -1959,10 +1958,10 @@ ConsensusExtensions::onTrustedPeerMessage( // still gated by the candidate tx hash before sidecar publication. auto const exportTxns = buildOpenLedgerExportTxnLookup(app_); auto const currentSeq = currentClosedLedgerSeq(app_); + std::size_t stored = 0; - for (int i = 0; i < wireMsg.exportsignatures_size(); ++i) + for (auto const& blob : exportSignatures) { - auto const& blob = wireMsg.exportsignatures(i); // Each entry: txnHash (32) + validator pubkey (33) + sig (var) if (blob.size() < 65) continue; @@ -1995,6 +1994,7 @@ ConsensusExtensions::onTrustedPeerMessage( Buffer sigBuf(sigSlice.data(), sigSlice.size()); exportSigCollector_.addUnverifiedSignature( txHash, senderPK, sigBuf, currentSeq); + ++stored; continue; } @@ -2005,7 +2005,46 @@ ConsensusExtensions::onTrustedPeerMessage( Buffer sigBuf(sigSlice.data(), sigSlice.size()); exportSigCollector_.addVerifiedSignature( txHash, senderPK, sigBuf, currentSeq); + ++stored; } + + if (stored > 0) + { + JLOG(j_.debug()) << "Export: harvested " << stored << " sigs from " + << source; + } + return stored; +} + +//@@start peer-harvest-export-sigs +void +ConsensusExtensions::onTrustedPeerMessage( + ::protocol::TMProposeSet const& wireMsg) +{ + if (wireMsg.exportsignatures_size() == 0) + return; + + auto const senderSlice = makeSlice(wireMsg.nodepubkey()); + if (!publicKeyType(senderSlice)) + return; + PublicKey const senderPK{senderSlice}; + + if (wireMsg.previousledger().size() != uint256::size()) + return; + + uint256 proposalPrevLedger; + std::memcpy( + proposalPrevLedger.data(), + wireMsg.previousledger().data(), + uint256::size()); + + std::vector exportSignatures; + exportSignatures.reserve(wireMsg.exportsignatures_size()); + for (int i = 0; i < wireMsg.exportsignatures_size(); ++i) + exportSignatures.push_back(wireMsg.exportsignatures(i)); + + harvestExportSignatures( + senderPK, proposalPrevLedger, exportSignatures, "wire proposal"); } //@@end peer-harvest-export-sigs diff --git a/src/xrpld/app/consensus/ConsensusExtensions.h b/src/xrpld/app/consensus/ConsensusExtensions.h index ceac24f0a..60306f1c3 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.h +++ b/src/xrpld/app/consensus/ConsensusExtensions.h @@ -16,6 +16,8 @@ #include #include #include +#include +#include namespace ripple { @@ -353,7 +355,17 @@ public: std::uint32_t proposeSeq, NetClock::time_point closeTime, uint256 const& prevLedger, - Slice const& signature); + Slice const& signature, + std::vector const& exportSignatures = {}); + + /** Harvest proposal-carried export signatures after the proposal payload is + known to be signed by `publicKey`. */ + std::size_t + harvestExportSignatures( + PublicKey const& publicKey, + uint256 const& prevLedger, + std::vector const& exportSignatures, + char const* source); /** Signal that the accept/build path finished successfully. Called from doAccept (frozen state, no consensus mutex). */ diff --git a/src/xrpld/consensus/Consensus.h b/src/xrpld/consensus/Consensus.h index 9e54fa461..c18445885 100644 --- a/src/xrpld/consensus/Consensus.h +++ b/src/xrpld/consensus/Consensus.h @@ -890,14 +890,29 @@ Consensus::peerProposalInternal( if constexpr (requires(Adaptor& a) { a.ce(); }) { auto& ce = adaptor_.ce(); - ce.onTrustedPeerProposal( - peerID, - newPeerPos.publicKey(), - newPeerProp.position(), - newPeerProp.proposeSeq(), - newPeerProp.closeTime(), - newPeerProp.prevLedger(), - newPeerPos.signature()); + if constexpr (requires { newPeerPos.exportSignatures(); }) + { + ce.onTrustedPeerProposal( + peerID, + newPeerPos.publicKey(), + newPeerProp.position(), + newPeerProp.proposeSeq(), + newPeerProp.closeTime(), + newPeerProp.prevLedger(), + newPeerPos.signature(), + newPeerPos.exportSignatures()); + } + else + { + ce.onTrustedPeerProposal( + peerID, + newPeerPos.publicKey(), + newPeerProp.position(), + newPeerProp.proposeSeq(), + newPeerProp.closeTime(), + newPeerProp.prevLedger(), + newPeerPos.signature()); + } if (ce.extensionsBusy()) ce.fetchSidecarsIfNeeded(newPeerProp.position());