From f0224cac1e5ce97b398c2387ac4e129af8cdbbb2 Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Sat, 27 Jun 2026 09:38:07 +0700 Subject: [PATCH] fix(export): replay witnesses without current manifests Historical LedgerReplay now treats ttEXPORT_SIGNATURES witnesses as validated-history membership input, while still verifying signatures and threshold. Live build and validation keep the current manifest-backed active-signer check. Harden witness handling by indexing only canonical same-ledger pseudos, dropping duplicate witnesses as ambiguous, and replacing pre-existing mismatched witnesses with the accepted sidecar witness during onPreBuild. Document the lean witness-vs-XPOP proof distinction and cover the manifest-rotation replay case with an Export regression test. --- src/test/app/Export_test.cpp | 84 +++++++++++++++++++ .../app/consensus/ConsensusExtensions.cpp | 20 ++--- .../consensus/ConsensusExtensionsDesign.md | 16 +++- src/xrpld/app/consensus/ExportIntent.md | 17 +++- src/xrpld/app/ledger/detail/BuildLedger.cpp | 59 +++++++++++-- src/xrpld/app/tx/applySteps.h | 6 ++ src/xrpld/app/tx/detail/ApplyContext.cpp | 4 +- src/xrpld/app/tx/detail/ApplyContext.h | 10 ++- src/xrpld/app/tx/detail/Export.cpp | 30 +++++-- src/xrpld/app/tx/detail/applySteps.cpp | 3 +- 10 files changed, 220 insertions(+), 29 deletions(-) diff --git a/src/test/app/Export_test.cpp b/src/test/app/Export_test.cpp index 1c0a04e80..c9b873ace 100644 --- a/src/test/app/Export_test.cpp +++ b/src/test/app/Export_test.cpp @@ -891,6 +891,89 @@ struct Export_test : public beast::unit_test::suite st->getFieldH256(sfTransactionHash) == expectedSignedTxHash); } + void + testExportHistoricalReplayIgnoresCurrentManifestMap(FeatureBitset features) + { + testcase( + "ttEXPORT historical replay does not require current manifest"); + + using namespace jtx; + + Env env{*this, exportTestConfig(), features}; + + Account const alice{"alice"}; + Account const carol{"carol"}; + + env.fund(XRP(10000), alice, carol); + env.close(); + + auto const& valKeys = env.app().getValidatorKeys(); + BEAST_EXPECT(valKeys.keys); + if (!valKeys.keys) + return; + + seedUNLReportLedger(env, {valKeys.keys->publicKey}); + forceNonStandalone(env.app()); + BEAST_EXPECT(!env.app().config().standalone()); + + auto const parent = env.app().getLedgerMaster().getClosedLedger(); + auto const applySeq = parent->seq() + 1; + auto const ticketSeq = std::uint32_t{1}; + auto innerObj = buildExportedPayment( + alice.id(), carol.id(), applySeq, applySeq + 5, ticketSeq); + auto const innerTx = makeSTTx(innerObj); + auto jt = makeExportJTx(env, alice, innerObj, applySeq + 5); + auto const exportTx = jt.stx; + BEAST_EXPECT(exportTx); + if (!exportTx) + return; + auto const txHash = exportTx->getTransactionID(); + + auto const oldSigningKey = randomKeyPair(KeyType::secp256k1); + auto const sig = ExportResultBuilder::signExportedTxn( + innerTx, oldSigningKey.first, oldSigningKey.second); + + ExportResultBuilder::SignatureSnapshot signatures; + signatures.emplace(oldSigningKey.first, sig); + auto const exportSignatureWitnesses = + makeExportSignatureWitnesses(txHash, signatures, applySeq); + + ApplyOptions const liveOptions{&exportSignatureWitnesses}; + { + auto next = std::make_shared( + *parent, env.app().timeKeeper().closeTime()); + OpenView accum(&*next); + auto const result = ripple::apply( + env.app(), accum, *exportTx, tapNONE, env.journal, liveOptions); + BEAST_EXPECT(result.ter == terRETRY_EXPORT); + BEAST_EXPECT(!result.applied); + } + + // Historical replay reconstructs an already-validated ledger. The + // persisted witness supplies the historical signing-key membership; + // current ManifestCache may no longer know a rotated signing key. + ApplyOptions const replayOptions{&exportSignatureWitnesses, true}; + auto const expectedSignedTxHash = + ExportResultBuilder::assemble(innerTx, signatures, applySeq, txHash) + .signedTxHash; + + auto replayed = std::make_shared( + *parent, env.app().timeKeeper().closeTime()); + OpenView accum(&*replayed); + auto const replayResult = ripple::apply( + env.app(), accum, *exportTx, tapNONE, env.journal, replayOptions); + BEAST_EXPECT(replayResult.ter == tesSUCCESS); + BEAST_EXPECT(replayResult.applied); + accum.apply(*replayed); + + auto const st = + replayed->read(keylet::shadowTicket(alice.id(), ticketSeq)); + BEAST_EXPECT(st); + if (st) + BEAST_EXPECT( + st->getFieldH256(sfTransactionHash) == expectedSignedTxHash); + } + void testExportNetworkRetryWithoutUNLReport(FeatureBitset features) { @@ -1568,6 +1651,7 @@ struct Export_test : public beast::unit_test::suite testExportTxnOpenLedger(allWithExport); testExportNetworkRetryWithoutQuorum(allWithExport); testExportNetworkApplyUsesAgreedSidecar(allWithExport); + testExportHistoricalReplayIgnoresCurrentManifestMap(allWithExport); testExportNetworkRetryWithoutUNLReport(allWithExport); testExportNetworkLastLedgerSequenceBoundary(allWithExport); testOpenLedgerExportLimit(allWithExport); diff --git a/src/xrpld/app/consensus/ConsensusExtensions.cpp b/src/xrpld/app/consensus/ConsensusExtensions.cpp index 661631963..4758d6aaa 100644 --- a/src/xrpld/app/consensus/ConsensusExtensions.cpp +++ b/src/xrpld/app/consensus/ConsensusExtensions.cpp @@ -2055,16 +2055,16 @@ ConsensusExtensions::onPreBuild( { auto const existingHash = existing->second->getTransactionID(); - if (existingHash != witnessHash) - { - JLOG(j_.error()) - << "Export: signature witness pseudo-tx mismatch" - << " exportTxHash=" << exportTxHash - << " witnessHash=" << witnessHash - << " existingHash=" << existingHash - << " action=keep-agreed-and-flag"; - } - continue; + if (existingHash == witnessHash) + continue; + + JLOG(j_.error()) + << "Export: signature witness pseudo-tx mismatch" + << " exportTxHash=" << exportTxHash + << " witnessHash=" << witnessHash + << " existingHash=" << existingHash + << " action=replace-with-agreed"; + retriableTxs.erase(existing); } // Export signatures change the shadow-ticket hash, so they diff --git a/src/xrpld/app/consensus/ConsensusExtensionsDesign.md b/src/xrpld/app/consensus/ConsensusExtensionsDesign.md index 5a2ce39f0..acadf3c8f 100644 --- a/src/xrpld/app/consensus/ConsensusExtensionsDesign.md +++ b/src/xrpld/app/consensus/ConsensusExtensionsDesign.md @@ -369,7 +369,14 @@ Closed-ledger apply consumes the pre-scanned `ttEXPORT_SIGNATURES` witness, not the live collector and not ephemeral sidecar state. `Export::doApply` rebuilds the active validator view from the parent ledger, verifies each witness signature against the `ttEXPORT` inner transaction, requires source-view quorum, -then canonically assembles the target-chain multisigned transaction. A node that +then canonically assembles the target-chain multisigned transaction. During live +build/validation, signing keys must still map through the node's current +manifest cache to active parent-ledger validators. Historical `LedgerReplay` is +different: it is reconstructing an already-validated ledger from its persisted +transaction stream, and the witness does not carry a historical manifest map. In +that mode, the witness supplies the historical membership material; apply still +checks signatures and threshold, but does not reject an old signing key merely +because today's manifest cache no longer maps it after rotation. A node that times out before accepting a root has no witness and retries/expires; a node that proceeds uses the same transaction-stream witness during live build and historical replay. The build-scoped witness map is only an index over that @@ -382,6 +389,13 @@ assemble the final foreign-chain blob from `ttEXPORT` plus the witness signatures, or can use a convenience RPC/helper that performs that pure read-time assembly. +This is intentionally leaner than XPOP. XPOP carries its own UNL and manifest +bundle so it can be independently verified as an external proof. Export witnesses +are not external proof bundles; they are inputs that made it into validated +ledger history. Making them self-contained would require embedding manifest +material or equivalent signing-key history in every witness, which is a separate +protocol/storage design. + Closed-ledger apply must not promote unverified proposal-carried signatures into current-round quorum material. It may verify and retain them for a future retry, where they can be published in a sidecar set and converged before use. diff --git a/src/xrpld/app/consensus/ExportIntent.md b/src/xrpld/app/consensus/ExportIntent.md index 7e94c2892..d9841d299 100644 --- a/src/xrpld/app/consensus/ExportIntent.md +++ b/src/xrpld/app/consensus/ExportIntent.md @@ -56,9 +56,13 @@ accepted sidecar set; standalone/dev helpers may produce the same witness from the local validator key. Ledger build pre-scans the ordered transaction stream into a build-local index, and `ttEXPORT` apply consumes that pre-scanned witness. The index is not an extra consensus input; it is only an efficient lookup over -the canonical transaction set. Apply re-verifies signer activity and signatures -against the parent-ledger active view (or the standalone validator key), and -derives the signed target-chain transaction hash from that replayable input. +the canonical transaction set. Live build/validation re-verifies signer +activity through the parent-ledger active view plus current manifest cache (or +the standalone validator key), verifies signatures, and derives the signed +target-chain transaction hash from that replayable input. Historical +`LedgerReplay` still verifies signatures and threshold, but it treats the +persisted witness as the historical membership source because current manifests +may no longer map old rotated signing keys. **INV-5 — Store the witness once.** The signature witness is canonical input; metadata is output. Metadata may carry @@ -93,3 +97,10 @@ Metadata stores `sfExportSignatureHash`, a direct reference to the witness pseudo, rather than duplicating the signature payload as an assembled `sfExportedTxn` blob. Clients assemble the final foreign-chain transaction from the original `ttEXPORT` inner transaction plus the witness signatures. + +This is not an XPOP-style self-contained proof. XPOP embeds its UNL and manifest +bundle because it is imported as external proof material. Export witnesses are +validated-history replay inputs. If we later want trustless historical +re-verification without relying on validated inclusion, the larger design is to +ledger-anchor validator signing-key history (for example via `UNLReport`) or to +embed manifest proof material; that is intentionally out of scope here. diff --git a/src/xrpld/app/ledger/detail/BuildLedger.cpp b/src/xrpld/app/ledger/detail/BuildLedger.cpp index 485d2d33f..26311d4ed 100644 --- a/src/xrpld/app/ledger/detail/BuildLedger.cpp +++ b/src/xrpld/app/ledger/detail/BuildLedger.cpp @@ -28,13 +28,17 @@ #include #include +#include + namespace ripple { namespace { void collectExportSignatureWitness( ExportResultBuilder::SignatureWitnesses& witnesses, + std::set& ambiguousWitnesses, STTx const& tx, + LedgerIndex ledgerSeq, beast::Journal j) { if (tx.getTxnType() != ttEXPORT_SIGNATURES) @@ -42,7 +46,37 @@ collectExportSignatureWitness( try { + if (!tx.isFieldPresent(sfTransactionHash) || + !tx.isFieldPresent(sfLedgerSequence) || + !tx.isFieldPresent(sfAccount) || !tx.isFieldPresent(sfSequence) || + !tx.isFieldPresent(sfFee)) + { + JLOG(j.warn()) << "Export: ignoring incomplete signature witness" + << " witnessHash=" << tx.getTransactionID(); + return; + } + auto const exportTxHash = tx.getFieldH256(sfTransactionHash); + if (ambiguousWitnesses.count(exportTxHash)) + { + JLOG(j.warn()) << "Export: ignoring ambiguous signature witness" + << " witnessHash=" << tx.getTransactionID() + << " exportTxHash=" << exportTxHash; + return; + } + + if (tx.getFieldU32(sfLedgerSequence) != ledgerSeq || + tx.getAccountID(sfAccount) != AccountID{} || + tx.getFieldU32(sfSequence) != 0 || + tx.getFieldAmount(sfFee) != beast::zero) + { + JLOG(j.warn()) << "Export: ignoring non-canonical signature witness" + << " witnessHash=" << tx.getTransactionID() + << " exportTxHash=" << exportTxHash + << " ledgerSeq=" << ledgerSeq; + return; + } + auto signatures = ExportResultBuilder::signaturesFromWitness(tx); if (!signatures) { @@ -53,15 +87,20 @@ collectExportSignatureWitness( } auto const witnessHash = tx.getTransactionID(); - auto const [_, inserted] = witnesses.emplace( + auto const [it, inserted] = witnesses.emplace( exportTxHash, ExportResultBuilder::SignatureWitness{ witnessHash, std::move(*signatures)}); if (!inserted) { + auto const existingHash = it->second.witnessHash; + ambiguousWitnesses.insert(exportTxHash); + witnesses.erase(it); JLOG(j.warn()) << "Export: duplicate signature witness" << " witnessHash=" << witnessHash - << " exportTxHash=" << exportTxHash; + << " existingHash=" << existingHash + << " exportTxHash=" << exportTxHash + << " action=drop-ambiguous"; } } catch (std::exception const& e) @@ -153,11 +192,16 @@ applyTransactions( std::size_t count = 0; ExportResultBuilder::SignatureWitnesses exportSignatureWitnesses; + std::set ambiguousExportSignatureWitnesses; for (auto const& entry : txns) { if (entry.second) collectExportSignatureWitness( - exportSignatureWitnesses, *entry.second, j); + exportSignatureWitnesses, + ambiguousExportSignatureWitnesses, + *entry.second, + view.seq(), + j); } ApplyOptions const applyOptions{&exportSignatureWitnesses}; @@ -338,13 +382,18 @@ buildLedger( j, [&](OpenView& accum, std::shared_ptr const& built) { ExportResultBuilder::SignatureWitnesses exportSignatureWitnesses; + std::set ambiguousExportSignatureWitnesses; for (auto const& tx : replayData.orderedTxns()) { if (tx.second) collectExportSignatureWitness( - exportSignatureWitnesses, *tx.second, j); + exportSignatureWitnesses, + ambiguousExportSignatureWitnesses, + *tx.second, + accum.seq(), + j); } - ApplyOptions const applyOptions{&exportSignatureWitnesses}; + ApplyOptions const applyOptions{&exportSignatureWitnesses, true}; for (auto& tx : replayData.orderedTxns()) applyTransaction( diff --git a/src/xrpld/app/tx/applySteps.h b/src/xrpld/app/tx/applySteps.h index 5c4a3275a..e9f97c23c 100644 --- a/src/xrpld/app/tx/applySteps.h +++ b/src/xrpld/app/tx/applySteps.h @@ -50,6 +50,12 @@ struct ApplyOptions // until another feature needs the same sibling-pseudo lookup. ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses = nullptr; + + // LedgerReplay rebuilds already-validated ledgers from persisted inputs. + // Export witnesses carry signing keys and signatures, but not the + // historical manifest map; current ManifestCache state may have rotated + // since the ledger closed. + bool historicalLedgerReplay = false; }; /** Return true if the transaction can claim a fee (tec), diff --git a/src/xrpld/app/tx/detail/ApplyContext.cpp b/src/xrpld/app/tx/detail/ApplyContext.cpp index 4b482e857..5de9cb456 100644 --- a/src/xrpld/app/tx/detail/ApplyContext.cpp +++ b/src/xrpld/app/tx/detail/ApplyContext.cpp @@ -37,7 +37,8 @@ ApplyContext::ApplyContext( XRPAmount baseFee_, ApplyFlags flags, beast::Journal journal_, - ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses) + ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses, + bool historicalLedgerReplay) : app(app_) , tx(tx_) , preclaimResult(preclaimResult_) @@ -46,6 +47,7 @@ ApplyContext::ApplyContext( , base_(base) , flags_(flags) , exportSignatureWitnesses_(exportSignatureWitnesses) + , historicalLedgerReplay_(historicalLedgerReplay) { view_.emplace(&base_, flags_); } diff --git a/src/xrpld/app/tx/detail/ApplyContext.h b/src/xrpld/app/tx/detail/ApplyContext.h index 74634b4f1..7ff33e463 100644 --- a/src/xrpld/app/tx/detail/ApplyContext.h +++ b/src/xrpld/app/tx/detail/ApplyContext.h @@ -45,7 +45,8 @@ public: ApplyFlags flags, beast::Journal = beast::Journal{beast::Journal::getNullSink()}, ExportResultBuilder::SignatureWitnesses const* - exportSignatureWitnesses = nullptr); + exportSignatureWitnesses = nullptr, + bool historicalLedgerReplay = false); Application& app; STTx const& tx; @@ -145,6 +146,12 @@ public: return it->second; } + bool + historicalLedgerReplay() const + { + return historicalLedgerReplay_; + } + ApplyFlags const& flags() { @@ -166,6 +173,7 @@ private: ApplyFlags flags_; std::optional view_; ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses_; + bool historicalLedgerReplay_; }; } // namespace ripple diff --git a/src/xrpld/app/tx/detail/Export.cpp b/src/xrpld/app/tx/detail/Export.cpp index b6d1a7e30..af57e8c2f 100644 --- a/src/xrpld/app/tx/detail/Export.cpp +++ b/src/xrpld/app/tx/detail/Export.cpp @@ -137,12 +137,16 @@ Export::doApply() auto const validatorView = consensusExtensions.makeActiveValidatorView(parentLedger); + bool const historicalReplay = ctx_.historicalLedgerReplay(); auto const isActiveSigner = [standalone, + historicalReplay, &valKeys, &consensusExtensions, validatorView](PublicKey const& key) { if (standalone) return valKeys.keys && key == valKeys.keys->publicKey; + if (historicalReplay) + return true; return consensusExtensions.isActiveValidator(key, *validatorView); }; // Closed-ledger export builds a local parent-ledger validator view, not the @@ -201,6 +205,13 @@ Export::doApply() // sidecars, standalone helpers, or replay all hand signatures to Export // through the same transaction-stream witness; apply re-checks the // witness against this parent ledger before creating ledger effects. + // + // Historical LedgerReplay is reconstructing an already-validated + // ledger. The witness does not carry historical manifests, and the + // current ManifestCache may no longer map old rotated signing keys to + // their validator masters. In that mode, the persisted witness supplies + // membership material; apply still verifies each signature against the + // export transaction and requires the parent-view threshold. if (auto witness = ctx_.exportSignatureWitness(txId)) { exportSignatureHash = witness->witnessHash; @@ -227,9 +238,10 @@ Export::doApply() //@@start export-doapply-retry-without-signature-quorum if (signatures.size() < threshold) { - auto const sigCount = - consensusExtensions.exportSigCollector().signatureCount( - txId, isActiveSigner); + auto const sigCount = historicalReplay + ? std::size_t{0} + : consensusExtensions.exportSigCollector().signatureCount( + txId, isActiveSigner); // LLS semantics for retriable exports: // // Transactor::preclaim rejects with tefMAX_LEDGER when @@ -250,8 +262,10 @@ Export::doApply() auto const lls = ctx_.tx.getFieldU32(sfLastLedgerSequence); if (currentSeq >= lls) { - ctx_.app.getConsensusExtensions().exportSigCollector().clear( - txId); + if (!historicalReplay) + ctx_.app.getConsensusExtensions() + .exportSigCollector() + .clear(txId); JLOG(j_.info()) << "Export: last ledger expired" << " txHash=" << txId << " ledgerSeq=" << currentSeq @@ -262,7 +276,8 @@ Export::doApply() } } - upgradeUnverifiedForNextRound(); + if (!historicalReplay) + upgradeUnverifiedForNextRound(); JLOG(j_.info()) << "Export: insufficient signatures" << " txHash=" << txId << " ledgerSeq=" << currentSeq @@ -303,7 +318,8 @@ Export::doApply() avi->setExportResultMetaData(std::move(assembled.metadata)); // Clean up the collector. - ctx_.app.getConsensusExtensions().exportSigCollector().clear(txId); + if (!historicalReplay) + ctx_.app.getConsensusExtensions().exportSigCollector().clear(txId); JLOG(j_.info()) << "Export: success" << " txHash=" << txId << " ledgerSeq=" << currentSeq diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index 211d56b46..d1def68d6 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -410,7 +410,8 @@ doApply( calculateBaseFee(view, preclaimResult.tx), preclaimResult.flags, preclaimResult.j, - options.exportSignatureWitnesses); + options.exportSignatureWitnesses, + options.historicalLedgerReplay); return invoke_apply(ctx); } catch (std::exception const& e)