highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-11 21:20:39 +02:00
Code Issues Packages Projects Releases Wiki Activity

bugfix: give TxDownloadManager its own RNG

Browse Source 
TxDownloadManagerImpl retains a reference to PeerManagerImpl::m_rng,
which is non-thread-safe and guarded by g_msgproc_mutex.

BlockConnected runs on the validation background thread while holding
only m_tx_download_mutex. Reconsidering an orphan with multiple
announcers could therefore use m_rng concurrently with message
processing.

Regression introduced in 9cc7dc50bd
This commit is contained in:
Greg Sanders
2026-08-25 08:30:33 -04:00
parent 794a753958
commit 80eaa6cabf
6 changed files with 18 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/node/txdownloadman.h
View File

@@ -39,8 +39,6 @@ inline constexpr auto GETDATA_TX_INTERVAL{60s};
struct TxDownloadOptions {
/** Read-only reference to mempool. */
const CTxMemPool& m_mempool;
/** RNG provided by caller. */
FastRandomContext& m_rng;
/** Instantiate TxRequestTracker as deterministic (used for tests). */
bool m_deterministic_txrequest{false};
};

8
src/node/txdownloadman_impl.cpp
View File

@@ -101,7 +101,7 @@ void TxDownloadManagerImpl::BlockConnected(const std::shared_ptr<const CBlock>&
for (const auto& ptx : pblock->vtx) {
// Reconsider potential child transactions.
m_orphanage->AddChildrenToWorkSet(*ptx, m_opts.m_rng);
m_orphanage->AddChildrenToWorkSet(*ptx, m_rng);
RecentConfirmedTransactionsFilter().insert(ptx->GetHash().ToUint256());
if (ptx->HasWitness()) {
@@ -146,7 +146,7 @@ bool TxDownloadManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_rec
if (RecentConfirmedTransactionsFilter().contains(hash)) return true;
return RecentRejectsFilter().contains(hash) || std::visit([&](const auto& id) { return m_opts.m_mempool.exists(id); }, gtxid);
return RecentRejectsFilter().contains(hash) || std::visit([&](const auto& id) { return m_mempool.exists(id); }, gtxid);
}
void TxDownloadManagerImpl::ConnectedPeer(NodeId nodeid, const TxDownloadConnectionInfo& info)
@@ -330,7 +330,7 @@ void TxDownloadManagerImpl::MempoolAcceptedTx(const CTransactionRef& tx)
m_txrequest.ForgetTxHash(tx->GetHash().ToUint256());
m_txrequest.ForgetTxHash(tx->GetWitnessHash().ToUint256());
m_orphanage->AddChildrenToWorkSet(*tx, m_opts.m_rng);
m_orphanage->AddChildrenToWorkSet(*tx, m_rng);
// If it came from the orphanage, remove it. No-op if the tx is not in txorphanage.
m_orphanage->EraseTx(tx->GetWitnessHash());
}
@@ -380,7 +380,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
fRejectedParents = true;
break;
} else if (RecentRejectsReconsiderableFilter().contains(parent_txid.ToUint256()) &&
!m_opts.m_mempool.exists(parent_txid)) {
!m_mempool.exists(parent_txid)) {
// More than 1 parent in m_lazy_recent_rejects_reconsiderable: 1p1c will not be
// sufficient to accept this package, so just give up here.
if (rejected_parent_reconsiderable.has_value()) {

11
src/node/txdownloadman_impl.h
View File

@@ -13,13 +13,15 @@
#include <node/txorphanage.h>
#include <primitives/transaction.h>
#include <policy/packages.h>
#include <random.h>
#include <txrequest.h>
class CTxMemPool;
namespace node {
class TxDownloadManagerImpl {
public:
TxDownloadOptions m_opts;
const CTxMemPool& m_mempool;
FastRandomContext m_rng;
/** Manages unvalidated tx data (orphan transactions for which we are downloading ancestors). */
std::unique_ptr<TxOrphanage> m_orphanage;
@@ -128,7 +130,12 @@ public:
return *m_lazy_recent_confirmed_transactions;
}
TxDownloadManagerImpl(const TxDownloadOptions& options) : m_opts{options}, m_orphanage{MakeTxOrphanage()}, m_txrequest{options.m_deterministic_txrequest} {}
TxDownloadManagerImpl(const TxDownloadOptions& options)
: m_mempool{options.m_mempool},
m_rng{options.m_deterministic_txrequest},
m_orphanage{MakeTxOrphanage()},
m_txrequest{options.m_deterministic_txrequest}
{}
struct PeerInfo {
/** Information relevant to scheduling tx requests. */