From 034010716e9bb828382a4f970c65ced0e944c451 Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Tue, 27 Jan 2026 18:03:55 +0700 Subject: [PATCH] feat: add signature verification cache for export signatures Add cryptographic verification of export signatures as they arrive: - stashTxnData() caches serialized txn for verification - verifyAndAddSignature() verifies against cached data, rejects invalid - isSignatureVerified() / verifySignature() for Transactor fallback - Cleanup methods updated to clear verification cache Also removes leftover debug std::cerr from OpenView, STObject, and tests. --- .../app/misc/ExportSignatureCollector.h | 62 ++++++ .../misc/impl/ExportSignatureCollector.cpp | 206 ++++++++++++++++++ src/ripple/app/tx/impl/ExportSign.cpp | 7 +- src/ripple/ledger/impl/OpenView.cpp | 29 +-- src/ripple/overlay/impl/PeerImp.cpp | 4 +- src/ripple/protocol/impl/STObject.cpp | 10 - src/test/app/Export_test.cpp | 53 ----- 7 files changed, 278 insertions(+), 93 deletions(-) diff --git a/src/ripple/app/misc/ExportSignatureCollector.h b/src/ripple/app/misc/ExportSignatureCollector.h index fb971155a..9e38654b4 100644 --- a/src/ripple/app/misc/ExportSignatureCollector.h +++ b/src/ripple/app/misc/ExportSignatureCollector.h @@ -26,11 +26,13 @@ #include #include #include +#include #include #include #include #include +#include #include namespace ripple { @@ -167,6 +169,59 @@ public: void cleanupStale(LedgerIndex currentSeq, LedgerIndex maxAge = 256); + // --- Signature verification cache --- + + /** Stash the serialized transaction data for signature verification. + + Called by signPendingExports() when signing, so we have the txn + data available to verify signatures from other validators. + + @param txnHash The hash of the exported transaction + @param txnData Serialized STTx for building verification data + */ + void + stashTxnData(uint256 const& txnHash, Serializer txnData); + + /** Verify and add a signature. + + Verifies the signature against the cached txn data before adding. + If txn data isn't cached yet (race), adds without verification and + marks as unverified. + + @param txnHash The hash of the exported transaction + @param validator The public key of the signing validator + @param signer The STObject containing the signature (sfSigner) + @param currentSeq Current ledger sequence + @return true if signature was added (regardless of verification status) + */ + bool + verifyAndAddSignature( + uint256 const& txnHash, + PublicKey const& validator, + STObject signer, + LedgerIndex currentSeq); + + /** Check if a signature has been cryptographically verified. + + @param txnHash The hash of the exported transaction + @param validator The public key of the validator + @return true if signature exists AND has been verified + */ + bool + isSignatureVerified(uint256 const& txnHash, PublicKey const& validator) + const; + + /** Verify a previously-added signature that wasn't verified on add. + + Called by Transactor as fallback when isSignatureVerified returns false. + + @param txnHash The hash of the exported transaction + @param validator The public key of the validator + @return true if verification succeeded (or already verified) + */ + bool + verifySignature(uint256 const& txnHash, PublicKey const& validator); + private: // Map> std::map> signatures_; @@ -174,6 +229,13 @@ private: // Track when each export was first seen (for timeout) std::map firstSeenLedger_; + // Signature verification cache + // Serialized STTx for building multisig verification data + std::map exportedTxnData_; + + // Which signatures have been cryptographically verified + std::map> verified_; + mutable std::mutex mutex_; beast::Journal j_; diff --git a/src/ripple/app/misc/impl/ExportSignatureCollector.cpp b/src/ripple/app/misc/impl/ExportSignatureCollector.cpp index c3bfec4c9..566e47b7f 100644 --- a/src/ripple/app/misc/impl/ExportSignatureCollector.cpp +++ b/src/ripple/app/misc/impl/ExportSignatureCollector.cpp @@ -22,7 +22,10 @@ #include #include #include +#include #include +#include +#include namespace ripple { @@ -213,6 +216,8 @@ ExportSignatureCollector::clearForTxn(uint256 const& txnHash) auto sigCount = signatures_.erase(txnHash); auto seqCount = firstSeenLedger_.erase(txnHash); + exportedTxnData_.erase(txnHash); + verified_.erase(txnHash); if (sigCount > 0 || seqCount > 0) { @@ -246,6 +251,8 @@ ExportSignatureCollector::cleanupStale( signatures_.erase(txnHash); firstSeenLedger_.erase(txnHash); + exportedTxnData_.erase(txnHash); + verified_.erase(txnHash); } if (!toRemove.empty()) @@ -255,4 +262,203 @@ ExportSignatureCollector::cleanupStale( } } +void +ExportSignatureCollector::stashTxnData( + uint256 const& txnHash, + Serializer txnData) +{ + std::lock_guard lock(mutex_); + + // Only stash if we don't already have it + if (exportedTxnData_.find(txnHash) == exportedTxnData_.end()) + { + exportedTxnData_.emplace(txnHash, std::move(txnData)); + JLOG(j_.trace()) << "Export: stashed txn data for " << txnHash; + } +} + +bool +ExportSignatureCollector::verifyAndAddSignature( + uint256 const& txnHash, + PublicKey const& validator, + STObject signer, + LedgerIndex currentSeq) +{ + std::lock_guard lock(mutex_); + + // Track first-seen time for cleanup + if (firstSeenLedger_.find(txnHash) == firstSeenLedger_.end()) + { + firstSeenLedger_[txnHash] = currentSeq; + JLOG(j_.debug()) << "Export: first signature for " << txnHash + << " at ledger " << currentSeq; + } + + // Check if we already have this signature + auto& signerMap = signatures_[txnHash]; + if (signerMap.find(validator) != signerMap.end()) + { + JLOG(j_.trace()) << "Export: already have signature from " + << toBase58(TokenType::NodePublic, validator) + << " for " << txnHash; + return true; // Already have it + } + + // Try to verify if we have the txn data + bool verified = false; + auto txnIt = exportedTxnData_.find(txnHash); + if (txnIt != exportedTxnData_.end()) + { + try + { + // Parse the stashed transaction + SerialIter sit(txnIt->second.slice()); + auto stpTrans = std::make_shared(std::ref(sit)); + + // Get signer account from the signer object + auto signingAcc = signer.getAccountID(sfAccount); + auto sigPubKey = signer.getFieldVL(sfSigningPubKey); + auto signature = signer.getFieldVL(sfTxnSignature); + + // Build the multisig data and verify + Serializer sigData = buildMultiSigningData(*stpTrans, signingAcc); + verified = ripple::verify( + PublicKey(makeSlice(sigPubKey)), + sigData.slice(), + makeSlice(signature), + true); + + if (!verified) + { + JLOG(j_.warn()) + << "Export: signature verification FAILED for " << txnHash + << " from " << toBase58(TokenType::NodePublic, validator); + return false; // Don't add invalid signature + } + + JLOG(j_.trace()) + << "Export: signature verified for " << txnHash << " from " + << toBase58(TokenType::NodePublic, validator); + } + catch (std::exception const& e) + { + JLOG(j_.warn()) << "Export: signature verification exception for " + << txnHash << ": " << e.what(); + return false; // Don't add if we can't verify + } + } + else + { + // No txn data yet - add unverified (will verify later or in Transactor) + JLOG(j_.trace()) << "Export: adding unverified signature for " + << txnHash << " (no txn data yet)"; + } + + // Add the signature + signerMap.emplace(validator, std::move(signer)); + + if (verified) + { + verified_[txnHash].insert(validator); + } + + JLOG(j_.trace()) << "Export: added signature from " + << toBase58(TokenType::NodePublic, validator) << " for " + << txnHash << " (total: " << signerMap.size() + << ", verified=" << verified << ")"; + + return true; +} + +bool +ExportSignatureCollector::isSignatureVerified( + uint256 const& txnHash, + PublicKey const& validator) const +{ + std::lock_guard lock(mutex_); + + auto it = verified_.find(txnHash); + if (it == verified_.end()) + return false; + + return it->second.find(validator) != it->second.end(); +} + +bool +ExportSignatureCollector::verifySignature( + uint256 const& txnHash, + PublicKey const& validator) +{ + std::lock_guard lock(mutex_); + + // Already verified? + auto verIt = verified_.find(txnHash); + if (verIt != verified_.end() && + verIt->second.find(validator) != verIt->second.end()) + { + return true; + } + + // Get the signature + auto sigIt = signatures_.find(txnHash); + if (sigIt == signatures_.end()) + return false; + + auto signerIt = sigIt->second.find(validator); + if (signerIt == sigIt->second.end()) + return false; + + // Get the txn data + auto txnIt = exportedTxnData_.find(txnHash); + if (txnIt == exportedTxnData_.end()) + { + JLOG(j_.warn()) << "Export: cannot verify signature - no txn data for " + << txnHash; + return false; + } + + try + { + // Parse the stashed transaction + SerialIter sit(txnIt->second.slice()); + auto stpTrans = std::make_shared(std::ref(sit)); + + // Get signer info + auto const& signer = signerIt->second; + auto signingAcc = signer.getAccountID(sfAccount); + auto sigPubKey = signer.getFieldVL(sfSigningPubKey); + auto signature = signer.getFieldVL(sfTxnSignature); + + // Build the multisig data and verify + Serializer sigData = buildMultiSigningData(*stpTrans, signingAcc); + bool verified = ripple::verify( + PublicKey(makeSlice(sigPubKey)), + sigData.slice(), + makeSlice(signature), + true); + + if (verified) + { + verified_[txnHash].insert(validator); + JLOG(j_.trace()) + << "Export: late-verified signature for " << txnHash << " from " + << toBase58(TokenType::NodePublic, validator); + } + else + { + JLOG(j_.warn()) + << "Export: late signature verification FAILED for " << txnHash + << " from " << toBase58(TokenType::NodePublic, validator); + } + + return verified; + } + catch (std::exception const& e) + { + JLOG(j_.warn()) << "Export: late verification exception for " << txnHash + << ": " << e.what(); + return false; + } +} + } // namespace ripple diff --git a/src/ripple/app/tx/impl/ExportSign.cpp b/src/ripple/app/tx/impl/ExportSign.cpp index 8745c0b1e..9b5dc3f3c 100644 --- a/src/ripple/app/tx/impl/ExportSign.cpp +++ b/src/ripple/app/tx/impl/ExportSign.cpp @@ -374,11 +374,16 @@ signPendingExports( auto txnHash = stpTrans->getTransactionID(); + // Get the collector and stash txn data for signature verification. + // This must happen before checking for cached signature so that + // peer signatures can be verified against this txn data. + auto& collector = app.getExportSignatureCollector(); + collector.stashTxnData(txnHash, *s); + // Check if we already have our signature cached in the collector. // This enables continuous broadcasting: we sign once, then keep // re-broadcasting our cached signature every ledger until the // export is finalized (ltEXPORTED_TXN deleted). - auto& collector = app.getExportSignatureCollector(); auto cachedSig = collector.getSignatureFrom(txnHash, pkSigning); if (cachedSig) diff --git a/src/ripple/ledger/impl/OpenView.cpp b/src/ripple/ledger/impl/OpenView.cpp index 2d5741ed6..fd03d7665 100644 --- a/src/ripple/ledger/impl/OpenView.cpp +++ b/src/ripple/ledger/impl/OpenView.cpp @@ -19,17 +19,6 @@ #include #include -#include -#include - -// Debug macro for export investigation -#define DBG_DEREF(msg) \ - do \ - { \ - std::cerr << "[OpenView::dereference t=" << std::this_thread::get_id() \ - << "] " << msg << std::endl; \ - std::cerr.flush(); \ - } while (0) namespace ripple { @@ -70,32 +59,16 @@ public: value_type dereference() const override { - DBG_DEREF("ENTER key=" << iter_->first); value_type result; { - DBG_DEREF( - "creating SerialIter, txn size=" << iter_->second.txn->size()); SerialIter sit(iter_->second.txn->slice()); - DBG_DEREF("creating STTx..."); - try - { - result.first = std::make_shared(sit); - DBG_DEREF("STTx created, type=" << result.first->getTxnType()); - } - catch (std::exception& e) - { - DBG_DEREF("STTx EXCEPTION: " << e.what()); - throw; - } + result.first = std::make_shared(sit); } if (metadata_) { - DBG_DEREF("creating metadata"); SerialIter sit(iter_->second.meta->slice()); result.second = std::make_shared(sit, sfMetadata); - DBG_DEREF("metadata created"); } - DBG_DEREF("EXIT"); return result; } }; diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 9ed5f0d6e..61acd6221 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -3219,7 +3219,9 @@ PeerImp::checkValidation( << " from validator " << toBase58(TokenType::NodePublic, validatorPK); - app_.getExportSignatureCollector().addSignature( + // Verify and add - will verify against cached txn data if + // available, otherwise adds unverified (verified later) + app_.getExportSignatureCollector().verifyAndAddSignature( txnHash, validatorPK, std::move(signer), currentSeq); } catch (std::exception const& e) diff --git a/src/ripple/protocol/impl/STObject.cpp b/src/ripple/protocol/impl/STObject.cpp index 6074153de..91d9f087c 100644 --- a/src/ripple/protocol/impl/STObject.cpp +++ b/src/ripple/protocol/impl/STObject.cpp @@ -251,17 +251,7 @@ STObject::set(SerialIter& sit, int depth) }); if (dup != sf.cend()) - { - // Debug: dump all field names to identify the duplicate - std::cerr << "[STObject::set] DUPLICATE FIELD DETECTED!" << std::endl; - std::cerr << "[STObject::set] Duplicate field name: " - << (*dup)->getFName().getName() << std::endl; - std::cerr << "[STObject::set] All fields in object:" << std::endl; - for (auto const& f : sf) - std::cerr << " - " << f->getFName().getName() << std::endl; - std::cerr.flush(); Throw("Duplicate field detected"); - } return reachedEndOfObject; } diff --git a/src/test/app/Export_test.cpp b/src/test/app/Export_test.cpp index 9a1dd5149..edcb15ed4 100644 --- a/src/test/app/Export_test.cpp +++ b/src/test/app/Export_test.cpp @@ -528,59 +528,6 @@ struct Export_test : public beast::unit_test::suite { auto const exportedDirKey = keylet::exportedDir(); bool dirEmpty = dirIsEmpty(*env.current(), exportedDirKey); - - // Debug: if not empty, inspect what's in there - if (!dirEmpty && expectCleanup) - { - std::cerr << "=== DEBUG: exportedDir not empty ===" - << std::endl; - std::shared_ptr sleDirNode{}; - unsigned int uDirEntry{0}; - uint256 dirEntry{beast::zero}; - if (cdirFirst( - *env.current(), - exportedDirKey.key, - sleDirNode, - uDirEntry, - dirEntry)) - { - do - { - auto sleItem = - env.current()->read(Keylet{ltCHILD, dirEntry}); - if (sleItem) - { - std::cerr << " Entry: " << dirEntry << std::endl; - std::cerr << " LedgerSeq: " - << sleItem->getFieldU32(sfLedgerSequence) - << std::endl; - std::cerr - << " TxnHash: " - << sleItem->getFieldH256(sfTransactionHash) - << std::endl; - if (sleItem->isFieldPresent(sfSigners)) - { - auto const& signers = - sleItem->getFieldArray(sfSigners); - std::cerr - << " Signers count: " << signers.size() - << std::endl; - } - else - { - std::cerr << " Signers: NONE" << std::endl; - } - } - } while (cdirNext( - *env.current(), - exportedDirKey.key, - sleDirNode, - uDirEntry, - dirEntry)); - } - std::cerr << "=== END DEBUG ===" << std::endl; - } - BEAST_EXPECT(dirEmpty == expectCleanup); }