From f0550ca625bc242dfd2b8262517f3601f4598c3d Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Mon, 8 Jun 2026 14:49:16 +0700 Subject: [PATCH] refactor(export): extract signature upgrade policy --- src/test/app/ExportSignatureUpgrader_test.cpp | 154 ++++++++++++++++++ src/xrpld/app/tx/detail/Export.cpp | 38 ++--- .../app/tx/detail/ExportSignatureUpgrader.cpp | 53 ++++++ .../app/tx/detail/ExportSignatureUpgrader.h | 39 +++++ 4 files changed, 256 insertions(+), 28 deletions(-) create mode 100644 src/test/app/ExportSignatureUpgrader_test.cpp create mode 100644 src/xrpld/app/tx/detail/ExportSignatureUpgrader.cpp create mode 100644 src/xrpld/app/tx/detail/ExportSignatureUpgrader.h diff --git a/src/test/app/ExportSignatureUpgrader_test.cpp b/src/test/app/ExportSignatureUpgrader_test.cpp new file mode 100644 index 000000000..f2ec8ba76 --- /dev/null +++ b/src/test/app/ExportSignatureUpgrader_test.cpp @@ -0,0 +1,154 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace ripple { +namespace test { +namespace { + +uint256 +makeHash(char const* label) +{ + return sha512Half(Slice(label, std::strlen(label))); +} + +STTx +makeSTTx(STObject const& obj) +{ + Serializer s; + obj.add(s); + SerialIter sit{s.slice()}; + return STTx{std::ref(sit)}; +} + +STTx +makeExportedPayment(AccountID const& src, AccountID const& dst) +{ + STObject obj(sfExportedTxn); + obj.setFieldU16(sfTransactionType, ttPAYMENT); + obj.setFieldU32(sfFlags, tfFullyCanonicalSig); + obj.setFieldU32(sfSequence, 0); + obj.setFieldU32(sfTicketSequence, 1); + obj.setFieldU32(sfFirstLedgerSequence, 2); + obj.setFieldU32(sfLastLedgerSequence, 6); + obj.setFieldAmount(sfAmount, XRPAmount{1000000}); + obj.setFieldAmount(sfFee, XRPAmount{10}); + obj.setFieldVL(sfSigningPubKey, Blob{}); + obj.setAccountID(sfAccount, src); + obj.setAccountID(sfDestination, dst); + return makeSTTx(obj); +} + +Buffer +makeInvalidSignature() +{ + std::uint8_t bytes[] = {1, 2, 3, 4, 5}; + return Buffer(bytes, sizeof(bytes)); +} + +beast::Journal +nullJournal() +{ + return beast::Journal{beast::Journal::getNullSink()}; +} + +} // namespace + +class ExportSignatureUpgrader_test : public beast::unit_test::suite +{ +public: + void + testUpgradeFiltersAndRemovesInvalid() + { + testcase("upgrade filters and removes invalid signatures"); + + auto const validSigner = randomKeyPair(KeyType::secp256k1); + auto const invalidSigner = randomKeyPair(KeyType::secp256k1); + auto const inactiveSigner = randomKeyPair(KeyType::secp256k1); + auto const dst = randomKeyPair(KeyType::secp256k1); + auto const innerTx = makeExportedPayment( + calcAccountID(validSigner.first), calcAccountID(dst.first)); + auto const txHash = makeHash("export-upgrade"); + + auto validSig = ExportResultBuilder::signExportedTxn( + innerTx, validSigner.first, validSigner.second); + auto invalidSig = makeInvalidSignature(); + auto inactiveSig = ExportResultBuilder::signExportedTxn( + innerTx, inactiveSigner.first, inactiveSigner.second); + + ExportSigCollector collector; + collector.addUnverifiedSignature( + txHash, validSigner.first, validSig, 7); + collector.addUnverifiedSignature( + txHash, invalidSigner.first, invalidSig, 7); + collector.addUnverifiedSignature( + txHash, inactiveSigner.first, inactiveSig, 7); + + std::set active{ + validSigner.first, + invalidSigner.first, + }; + auto stats = ExportSignatureUpgrader::upgradeUnverifiedSignatures( + collector, + innerTx, + txHash, + 12, + [&active](PublicKey const& pk) { return active.count(pk) > 0; }, + nullJournal()); + + BEAST_EXPECT(stats.inspected == 3); + BEAST_EXPECT(stats.inactiveSkipped == 1); + BEAST_EXPECT(stats.upgraded == 1); + BEAST_EXPECT(stats.removedInvalid == 1); + + BEAST_EXPECT(collector.hasVerifiedSignature(txHash, validSigner.first)); + BEAST_EXPECT( + !collector.hasVerifiedSignature(txHash, invalidSigner.first)); + BEAST_EXPECT( + !collector.hasVerifiedSignature(txHash, inactiveSigner.first)); + + auto const unverified = collector.unverifiedSignatures(txHash); + BEAST_EXPECT(!unverified.contains(invalidSigner.first)); + BEAST_EXPECT(unverified.contains(inactiveSigner.first)); + BEAST_EXPECT(collector.signatureCount(txHash) == 1); + } + + void + run() override + { + testUpgradeFiltersAndRemovesInvalid(); + } +}; + +BEAST_DEFINE_TESTSUITE(ExportSignatureUpgrader, app, ripple); + +} // namespace test +} // namespace ripple diff --git a/src/xrpld/app/tx/detail/Export.cpp b/src/xrpld/app/tx/detail/Export.cpp index 3523c59e9..afea6ee9a 100644 --- a/src/xrpld/app/tx/detail/Export.cpp +++ b/src/xrpld/app/tx/detail/Export.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -153,34 +154,15 @@ Export::doApply() // Closed-ledger apply must not create new current-round quorum // material. These upgrades are retained for a retrying export, where // the sidecar alignment gate can publish and converge them first. - auto& collector = consensusExtensions.exportSigCollector(); - auto const unverified = collector.unverifiedSignatures(txId); - for (auto const& [valPK, sigBuf] : unverified) - { - // Upgrade only active-view signatures; inactive trusted signatures - // may stay cached, but they must not become quorum material. - if (!isActiveSigner(valPK)) - continue; - - auto const signerAcctID = calcAccountID(valPK); - auto const sigData = buildMultiSigningData(innerTx, signerAcctID); - if (verify( - valPK, - sigData.slice(), - Slice(sigBuf.data(), sigBuf.size()))) - { - collector.upgradeSignature(txId, valPK, sigBuf, currentSeq); - } - else - { - JLOG(j_.warn()) - << "Export: upgrade verify failed" - << " txHash=" << txId << " signer=" << calcNodeID(valPK) - << " ledgerSeq=" << currentSeq - << " action=remove-invalid-sig"; - collector.removeSignature(txId, valPK, sigBuf); - } - } + // Upgrade only active-view signatures; inactive trusted signatures may + // stay cached, but they must not become quorum material. + ExportSignatureUpgrader::upgradeUnverifiedSignatures( + consensusExtensions.exportSigCollector(), + innerTx, + txId, + currentSeq, + isActiveSigner, + j_); }; // Atomic quorum check + snapshot for network mode. diff --git a/src/xrpld/app/tx/detail/ExportSignatureUpgrader.cpp b/src/xrpld/app/tx/detail/ExportSignatureUpgrader.cpp new file mode 100644 index 000000000..a94d8dd2b --- /dev/null +++ b/src/xrpld/app/tx/detail/ExportSignatureUpgrader.cpp @@ -0,0 +1,53 @@ +#include +#include +#include +#include + +namespace ripple { +namespace ExportSignatureUpgrader { + +UpgradeStats +upgradeUnverifiedSignatures( + ExportSigCollector& collector, + STTx const& innerTx, + uint256 const& exportTxHash, + LedgerIndex currentSeq, + IsActiveSigner isActiveSigner, + beast::Journal j) +{ + UpgradeStats stats; + auto const unverified = collector.unverifiedSignatures(exportTxHash); + for (auto const& [valPK, sigBuf] : unverified) + { + ++stats.inspected; + + if (!isActiveSigner(valPK)) + { + ++stats.inactiveSkipped; + continue; + } + + auto const signerAcctID = calcAccountID(valPK); + auto const sigData = buildMultiSigningData(innerTx, signerAcctID); + if (verify(valPK, sigData.slice(), Slice(sigBuf.data(), sigBuf.size()))) + { + collector.upgradeSignature(exportTxHash, valPK, sigBuf, currentSeq); + ++stats.upgraded; + } + else + { + JLOG(j.warn()) << "Export: upgrade verify failed" + << " txHash=" << exportTxHash + << " signer=" << calcNodeID(valPK) + << " ledgerSeq=" << currentSeq + << " action=remove-invalid-sig"; + if (collector.removeSignature(exportTxHash, valPK, sigBuf)) + ++stats.removedInvalid; + } + } + + return stats; +} + +} // namespace ExportSignatureUpgrader +} // namespace ripple diff --git a/src/xrpld/app/tx/detail/ExportSignatureUpgrader.h b/src/xrpld/app/tx/detail/ExportSignatureUpgrader.h new file mode 100644 index 000000000..5d5099b3a --- /dev/null +++ b/src/xrpld/app/tx/detail/ExportSignatureUpgrader.h @@ -0,0 +1,39 @@ +#ifndef RIPPLE_TX_EXPORTSIGNATUREUPGRADER_H_INCLUDED +#define RIPPLE_TX_EXPORTSIGNATUREUPGRADER_H_INCLUDED + +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace ripple { +namespace ExportSignatureUpgrader { + +using IsActiveSigner = std::function; + +struct UpgradeStats +{ + std::size_t inspected = 0; + std::size_t inactiveSkipped = 0; + std::size_t upgraded = 0; + std::size_t removedInvalid = 0; +}; + +UpgradeStats +upgradeUnverifiedSignatures( + ExportSigCollector& collector, + STTx const& innerTx, + uint256 const& exportTxHash, + LedgerIndex currentSeq, + IsActiveSigner isActiveSigner, + beast::Journal j); + +} // namespace ExportSignatureUpgrader +} // namespace ripple + +#endif