fix(export): close replay witness review gaps

Thread the replay parent through ApplyOptions so export replay can rebuild the historical validator view without relying on LedgerMaster visibility for freshly replayed parents.

Harden ttEXPORT_SIGNATURES canonicality before witness indexing, validate witness signer accounts against their pubkeys, reject ambiguous absent NetworkID on low-ID source chains, and lock the consensus phase accessor.

Add focused regression coverage and clarify that destination SignerList/quorum compatibility is an operator/client contract.
This commit is contained in:
Nicholas Dudfield
2026-06-27 10:53:34 +07:00
parent 8ddb0a1134
commit efcd2a0ab7
12 changed files with 123 additions and 18 deletions

View File

@@ -332,6 +332,34 @@ public:
}
}
void
testRejectsNonCanonicalWitnessSigner()
{
testcase("signature witness validates signer account");
auto const src = randomKeyPair(KeyType::secp256k1);
auto const dst = randomKeyPair(KeyType::secp256k1);
auto const signer = randomKeyPair(KeyType::secp256k1);
auto const wrongAccount = randomKeyPair(KeyType::secp256k1);
auto const innerTx = makeExportedPayment(
calcAccountID(src.first), calcAccountID(dst.first));
auto const exportTxHash = makeHash("bad-witness-signer");
ExportResultBuilder::SignatureSnapshot signatures;
signatures.emplace(
signer.first,
ExportResultBuilder::signExportedTxn(
innerTx, signer.first, signer.second));
auto witness = ExportResultBuilder::buildSignatureWitness(
exportTxHash, signatures, 654);
auto signers = witness.getFieldArray(sfSigners);
signers[0].setAccountID(sfAccount, calcAccountID(wrongAccount.first));
witness.setFieldArray(sfSigners, signers);
BEAST_EXPECT(!ExportResultBuilder::signaturesFromWitness(witness));
}
void
run() override
{
@@ -341,6 +369,7 @@ public:
testCapsSignerArray();
testAssemblesWitnessReferenceMetadata();
testSignatureWitnessRoundTrip();
testRejectsNonCanonicalWitnessSigner();
}
};

View File

@@ -655,6 +655,33 @@ struct Export_test : public beast::unit_test::suite
BEAST_EXPECT(dirIsEmpty(*env.current(), emittedDirKey));
}
void
testExportRejectsAmbiguousLowNetworkID()
{
testcase("Export rejects absent NetworkID on low source networks");
using namespace jtx;
Account const alice{"alice"};
Account const carol{"carol"};
auto innerObj = buildExportedPayment(alice.id(), carol.id(), 2, 6);
auto const innerTx = makeSTTx(innerObj);
auto const j = beast::Journal{beast::Journal::getNullSink()};
// Low-ID networks do not encode sfNetworkID on ordinary transactions,
// so an absent destination ID is ambiguous with a self-target.
BEAST_EXPECT(
ExportLedgerOps::validateNetworkID(innerTx, 0, j) == temMALFORMED);
BEAST_EXPECT(
ExportLedgerOps::validateNetworkID(innerTx, 1024, j) ==
temMALFORMED);
// A high-ID source chain can still export to a legacy low-ID
// destination whose transactions omit sfNetworkID.
BEAST_EXPECT(
ExportLedgerOps::validateNetworkID(innerTx, 1025, j) == tesSUCCESS);
}
void
testXportEmissionLimit(FeatureBitset features)
{
@@ -868,7 +895,8 @@ struct Export_test : public beast::unit_test::suite
expectedSigs.emplace(valPK, originalSig);
auto const exportSignatureWitnesses =
makeExportSignatureWitnesses(txHash, expectedSigs, applySeq);
ApplyOptions const applyOptions{&exportSignatureWitnesses};
ApplyOptions const applyOptions{
&exportSignatureWitnesses, false, nullptr};
auto const expectedSignedTxHash =
ExportResultBuilder::assemble(
innerTx, expectedSigs, applySeq, txHash)
@@ -938,7 +966,8 @@ struct Export_test : public beast::unit_test::suite
auto const exportSignatureWitnesses =
makeExportSignatureWitnesses(txHash, signatures, applySeq);
ApplyOptions const liveOptions{&exportSignatureWitnesses};
ApplyOptions const liveOptions{
&exportSignatureWitnesses, false, nullptr};
{
auto next = std::make_shared<Ledger>(
*parent, env.app().timeKeeper().closeTime());
@@ -952,7 +981,8 @@ struct Export_test : public beast::unit_test::suite
// 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};
ApplyOptions const replayOptions{
&exportSignatureWitnesses, true, nullptr};
auto const expectedSignedTxHash =
ExportResultBuilder::assemble(innerTx, signatures, applySeq, txHash)
.signedTxHash;
@@ -1104,7 +1134,8 @@ struct Export_test : public beast::unit_test::suite
exportSignatureWitnesses =
makeExportSignatureWitnesses(txHash, signatures, applySeq);
}
ApplyOptions const applyOptions{&exportSignatureWitnesses};
ApplyOptions const applyOptions{
&exportSignatureWitnesses, false, nullptr};
auto next = std::make_shared<Ledger>(
*parent, env.app().timeKeeper().closeTime());
@@ -1645,6 +1676,7 @@ struct Export_test : public beast::unit_test::suite
testXportPayment(allWithExport);
testXportRejectsLocalNetworkID(allWithExport);
testXportRejectsUnconfiguredNetworkID(allWithExport);
testExportRejectsAmbiguousLowNetworkID();
testXportEmissionLimit(allWithExport);
// ttEXPORT transactor tests

View File

@@ -101,7 +101,9 @@ must follow the same deterministic contract as `ExportResultBuilder`: sort
signers canonically by AccountID, use an empty `SigningPubKey`, and cap the
target-chain `Signers` array at `STTx::maxMultiSigners()` before computing or
submitting the blob. The witness may contain extra source-side signatures that
are valid replay input but are not part of the target-chain blob.
are valid replay input but are not part of the target-chain blob. Source-chain
export does not prove the destination account's SignerList or quorum policy;
that compatibility is an operator/client contract for the chosen target chain.
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

View File

@@ -1010,6 +1010,7 @@ RCLConsensus::Adaptor::onModeChange(ConsensusMode before, ConsensusMode after)
ConsensusPhase
RCLConsensus::phase() const
{
std::lock_guard _{mutex_};
return consensus_->phase();
}

View File

@@ -68,7 +68,9 @@ collectExportSignatureWitness(
if (tx.getFieldU32(sfLedgerSequence) != ledgerSeq ||
tx.getAccountID(sfAccount) != AccountID{} ||
tx.getFieldU32(sfSequence) != 0 ||
tx.getFieldAmount(sfFee) != beast::zero)
tx.getFieldAmount(sfFee) != beast::zero ||
!tx.getSigningPubKey().empty() || !tx.getSignature().empty() ||
tx.isFieldPresent(sfPreviousTxnID))
{
JLOG(j.warn()) << "Export: ignoring non-canonical signature witness"
<< " witnessHash=" << tx.getTransactionID()
@@ -203,7 +205,7 @@ applyTransactions(
view.seq(),
j);
}
ApplyOptions const applyOptions{&exportSignatureWitnesses};
ApplyOptions const applyOptions{&exportSignatureWitnesses, false, nullptr};
//@@start rng-entropy-first-application
// CRITICAL: Apply consensus entropy pseudo-tx FIRST before any other
@@ -393,7 +395,8 @@ buildLedger(
accum.seq(),
j);
}
ApplyOptions const applyOptions{&exportSignatureWitnesses, true};
ApplyOptions const applyOptions{
&exportSignatureWitnesses, true, replayData.parent()};
for (auto& tx : replayData.orderedTxns())
applyTransaction(

View File

@@ -24,9 +24,12 @@
#include <xrpld/ledger/ApplyViewImpl.h>
#include <xrpl/beast/utility/Journal.h>
#include <memory>
namespace ripple {
class Application;
class Ledger;
class STTx;
class TxQ;
@@ -56,6 +59,11 @@ struct ApplyOptions
// historical manifest map; current ManifestCache state may have rotated
// since the ledger closed.
bool historicalLedgerReplay = false;
// LedgerReplay can build consecutive ledgers before the rebuilt parent is
// visible through LedgerMaster. Export apply needs the exact replay parent
// to rebuild the historical validator view.
std::shared_ptr<Ledger const> replayParentLedger;
};
/** Return true if the transaction can claim a fee (tec),

View File

@@ -38,7 +38,8 @@ ApplyContext::ApplyContext(
ApplyFlags flags,
beast::Journal journal_,
ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses,
bool historicalLedgerReplay)
bool historicalLedgerReplay,
std::shared_ptr<Ledger const> replayParentLedger)
: app(app_)
, tx(tx_)
, preclaimResult(preclaimResult_)
@@ -48,6 +49,7 @@ ApplyContext::ApplyContext(
, flags_(flags)
, exportSignatureWitnesses_(exportSignatureWitnesses)
, historicalLedgerReplay_(historicalLedgerReplay)
, replayParentLedger_(std::move(replayParentLedger))
{
view_.emplace(&base_, flags_);
}

View File

@@ -27,11 +27,14 @@
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/XRPAmount.h>
#include <memory>
#include <optional>
#include <utility>
namespace ripple {
class Ledger;
/** State information when applying a tx. */
class ApplyContext
{
@@ -46,7 +49,8 @@ public:
beast::Journal = beast::Journal{beast::Journal::getNullSink()},
ExportResultBuilder::SignatureWitnesses const*
exportSignatureWitnesses = nullptr,
bool historicalLedgerReplay = false);
bool historicalLedgerReplay = false,
std::shared_ptr<Ledger const> replayParentLedger = nullptr);
Application& app;
STTx const& tx;
@@ -152,6 +156,12 @@ public:
return historicalLedgerReplay_;
}
std::shared_ptr<Ledger const>
replayParentLedger() const
{
return replayParentLedger_;
}
ApplyFlags const&
flags()
{
@@ -174,6 +184,7 @@ private:
std::optional<ApplyViewImpl> view_;
ExportResultBuilder::SignatureWitnesses const* exportSignatureWitnesses_;
bool historicalLedgerReplay_;
std::shared_ptr<Ledger const> replayParentLedger_;
};
} // namespace ripple

View File

@@ -125,8 +125,18 @@ Export::doApply()
auto& consensusExtensions = ctx_.app.getConsensusExtensions();
bool const standalone = ctx_.app.config().standalone();
auto const& valKeys = ctx_.app.getValidatorKeys();
auto const parentLedger =
ctx_.app.getLedgerMaster().getLedgerByHash(view().info().parentHash);
auto parentLedger = ctx_.replayParentLedger();
if (parentLedger && parentLedger->info().hash != view().info().parentHash)
{
JLOG(j_.warn()) << "Export: ignoring mismatched replay parent"
<< " txHash=" << txId << " ledgerSeq=" << currentSeq
<< " parentHash=" << view().info().parentHash
<< " replayParentHash=" << parentLedger->info().hash;
parentLedger.reset();
}
if (!parentLedger)
parentLedger = ctx_.app.getLedgerMaster().getLedgerByHash(
view().info().parentHash);
if (!standalone && !parentLedger)
{
JLOG(j_.warn()) << "Export: retrying without parent ledger"

View File

@@ -103,8 +103,8 @@ checkExportTxnLimit(ReadView const& view, beast::Journal j)
/// - Networks > 1024: sfNetworkID is REQUIRED and must match
///
/// So: if exported tx has sfNetworkID matching local → self-target.
/// if local NETWORK_ID is 0 (unconfigured) and tx has no
/// sfNetworkID → can't distinguish self from cross-chain, reject.
/// if local NETWORK_ID <= 1024 and tx has no sfNetworkID → can't
/// distinguish self from another low-ID chain, reject.
inline TER
validateNetworkID(
STTx const& stx,
@@ -120,10 +120,10 @@ validateNetworkID(
return temMALFORMED;
}
if (localNetworkID == 0 && !stx.isFieldPresent(sfNetworkID))
if (localNetworkID <= 1024 && !stx.isFieldPresent(sfNetworkID))
{
JLOG(j.warn()) << "ExportLedgerOps: rejected export with "
"unconfigured NETWORK_ID";
"ambiguous low NETWORK_ID";
return temMALFORMED;
}

View File

@@ -40,7 +40,8 @@ buildSigners(SignatureSnapshot const& signatures, bool capForTargetChain)
// XRPL validates the Signers array size before checking signer weights.
// Export quorum is decided earlier from the agreed sidecar snapshot; this
// cap only materializes a target-chain-valid canonical prefix.
// cap only materializes a protocol-size-valid canonical prefix. The
// destination account's SignerList/quorum remains an operator contract.
if (capForTargetChain)
{
auto const maxSigners = STTx::maxMultiSigners();
@@ -114,6 +115,7 @@ signaturesFromWitness(STTx const& witness)
for (auto const& signer : witness.getFieldArray(sfSigners))
{
if (signer.getFName() != sfSigner ||
!signer.isFieldPresent(sfAccount) ||
!signer.isFieldPresent(sfSigningPubKey) ||
!signer.isFieldPresent(sfTxnSignature))
return std::nullopt;
@@ -122,6 +124,10 @@ signaturesFromWitness(STTx const& witness)
if (!publicKeyType(makeSlice(pkBlob)))
return std::nullopt;
if (signer.getAccountID(sfAccount) !=
calcAccountID(PublicKey(makeSlice(pkBlob))))
return std::nullopt;
auto const sigBlob = signer.getFieldVL(sfTxnSignature);
if (sigBlob.empty() || sigBlob.size() > maxTxnSignatureBytes)
return std::nullopt;

View File

@@ -411,7 +411,8 @@ doApply(
preclaimResult.flags,
preclaimResult.j,
options.exportSignatureWitnesses,
options.historicalLedgerReplay);
options.historicalLedgerReplay,
options.replayParentLedger);
return invoke_apply(ctx);
}
catch (std::exception const& e)