Skip to content

Commit 35be4e2

Browse files
kwvgkwvg
committed
llmq: pass PeerManager to llmq::CInstantSendManager constructor
Required to avoid crashes when calling RelayInvFiltered in situations where the PeerManager* atomic hasn't been set (possible when ProcessMessage isn't called, leaving the value unset, while a separate thread traverses the ProcessPendingInstantSendLocks > ProcessPendingInstantSendLocks[1] > ProcessInstantSendLock > RelayInvFiltered call chain). [1] - There is a function with the exact same name but with multiple arguments
1 parent bfd33cd commit 35be4e2

File tree

4 files changed

+12
-18
lines changed

4 files changed

+12
-18
lines changed

‎src/llmq/context.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterminis
4343
}()},
4444
isman{[&]() -> llmq::CInstantSendManager* const {
4545
assert(llmq::quorumInstantSendManager == nullptr);
46-
llmq::quorumInstantSendManager = std::make_unique<llmq::CInstantSendManager>(*llmq::chainLocksHandler, chainstate, connman, *llmq::quorumManager, *sigman, *shareman, sporkman, mempool, mn_sync, unit_tests, wipe);
46+
llmq::quorumInstantSendManager = std::make_unique<llmq::CInstantSendManager>(*llmq::chainLocksHandler, chainstate, connman, *llmq::quorumManager, *sigman, *shareman, sporkman, mempool, mn_sync, peerman, unit_tests, wipe);
4747
return llmq::quorumInstantSendManager.get();
4848
}()},
4949
ehfSignalsHandler{std::make_unique<llmq::CEHFSignalsHandler>(chainstate, mnhfman, *sigman, *shareman, mempool, *llmq::quorumManager, sporkman, peerman)}

‎src/llmq/instantsend.cpp‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -747,15 +747,9 @@ void CInstantSendManager::HandleNewInstantSendLockRecoveredSig(const llmq::CReco
747747
pendingInstantSendLocks.emplace(hash, std::make_pair(-1, islock));
748748
}
749749

750-
PeerMsgRet CInstantSendManager::ProcessMessage(const CNode& pfrom, gsl::not_null<PeerManager*> peerman, std::string_view msg_type, CDataStream& vRecv)
750+
PeerMsgRet CInstantSendManager::ProcessMessage(const CNode& pfrom, std::string_view msg_type, CDataStream& vRecv)
751751
{
752752
if (IsInstantSendEnabled() && msg_type == NetMsgType::ISDLOCK) {
753-
if (m_peerman == nullptr) {
754-
m_peerman = peerman;
755-
}
756-
// we should never use one CInstantSendManager with different PeerManager
757-
assert(m_peerman == peerman);
758-
759753
const auto islock = std::make_shared<CInstantSendLock>();
760754
vRecv >> *islock;
761755
return ProcessMessageInstantSendLock(pfrom, islock);
@@ -957,7 +951,7 @@ std::unordered_set<uint256, StaticSaltedHasher> CInstantSendManager::ProcessPend
957951
for (const auto& nodeId : batchVerifier.badSources) {
958952
// Let's not be too harsh, as the peer might simply be unlucky and might have sent us an old lock which
959953
// does not validate anymore due to changed quorums
960-
m_peerman.load()->Misbehaving(nodeId, 20);
954+
m_peerman->Misbehaving(nodeId, 20);
961955
}
962956
}
963957
for (const auto& p : pend) {
@@ -1051,11 +1045,11 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
10511045

10521046
CInv inv(MSG_ISDLOCK, hash);
10531047
if (tx != nullptr) {
1054-
m_peerman.load()->RelayInvFiltered(inv, *tx, ISDLOCK_PROTO_VERSION);
1048+
m_peerman->RelayInvFiltered(inv, *tx, ISDLOCK_PROTO_VERSION);
10551049
} else {
10561050
// we don't have the TX yet, so we only filter based on txid. Later when that TX arrives, we will re-announce
10571051
// with the TX taken into account.
1058-
m_peerman.load()->RelayInvFiltered(inv, islock->txid, ISDLOCK_PROTO_VERSION);
1052+
m_peerman->RelayInvFiltered(inv, islock->txid, ISDLOCK_PROTO_VERSION);
10591053
}
10601054

10611055
ResolveBlockConflicts(hash, *islock);
@@ -1068,7 +1062,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
10681062
// bump mempool counter to make sure newly locked txes are picked up by getblocktemplate
10691063
mempool.AddTransactionsUpdated(1);
10701064
} else {
1071-
AskNodesForLockedTx(islock->txid, connman, *m_peerman.load());
1065+
AskNodesForLockedTx(islock->txid, connman, *m_peerman);
10721066
}
10731067
}
10741068

@@ -1344,7 +1338,7 @@ void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, con
13441338
for (const auto& p : toDelete) {
13451339
RemoveConflictedTx(*p.second);
13461340
}
1347-
AskNodesForLockedTx(islock.txid, connman, *m_peerman.load());
1341+
AskNodesForLockedTx(islock.txid, connman, *m_peerman);
13481342
}
13491343
}
13501344

‎src/llmq/instantsend.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ class CInstantSendManager : public CRecoveredSigsListener
206206
CSporkManager& spork_manager;
207207
CTxMemPool& mempool;
208208
const CMasternodeSync& m_mn_sync;
209-
std::atomic<PeerManager*> m_peerman{nullptr};
209+
const std::unique_ptr<PeerManager>& m_peerman;
210210

211211
std::atomic<bool> fUpgradedDB{false};
212212

@@ -257,10 +257,10 @@ class CInstantSendManager : public CRecoveredSigsListener
257257
explicit CInstantSendManager(CChainLocksHandler& _clhandler, CChainState& chainstate, CConnman& _connman,
258258
CQuorumManager& _qman, CSigningManager& _sigman, CSigSharesManager& _shareman,
259259
CSporkManager& sporkman, CTxMemPool& _mempool, const CMasternodeSync& mn_sync,
260-
bool unitTests, bool fWipe) :
260+
const std::unique_ptr<PeerManager>& peerman, bool unitTests, bool fWipe) :
261261
db(unitTests, fWipe),
262262
clhandler(_clhandler), m_chainstate(chainstate), connman(_connman), qman(_qman), sigman(_sigman),
263-
shareman(_shareman), spork_manager(sporkman), mempool(_mempool), m_mn_sync(mn_sync)
263+
shareman(_shareman), spork_manager(sporkman), mempool(_mempool), m_mn_sync(mn_sync), m_peerman(peerman)
264264
{
265265
workInterrupt.reset();
266266
}
@@ -313,7 +313,7 @@ class CInstantSendManager : public CRecoveredSigsListener
313313

314314
void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override LOCKS_EXCLUDED(cs_inputReqests, cs_creating);
315315

316-
PeerMsgRet ProcessMessage(const CNode& pfrom, gsl::not_null<PeerManager*> peerman, std::string_view msg_type, CDataStream& vRecv);
316+
PeerMsgRet ProcessMessage(const CNode& pfrom, std::string_view msg_type, CDataStream& vRecv);
317317

318318
void TransactionAddedToMempool(const CTransactionRef& tx) LOCKS_EXCLUDED(cs_pendingLocks);
319319
void TransactionRemovedFromMempool(const CTransactionRef& tx);

‎src/net_processing.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4748,7 +4748,7 @@ void PeerManagerImpl::ProcessMessage(
47484748
m_llmq_ctx->shareman->ProcessMessage(pfrom, m_sporkman, msg_type, vRecv);
47494749
ProcessPeerMsgRet(m_llmq_ctx->sigman->ProcessMessage(pfrom, this, msg_type, vRecv), pfrom);
47504750
ProcessPeerMsgRet(m_llmq_ctx->clhandler->ProcessMessage(pfrom, msg_type, vRecv), pfrom);
4751-
ProcessPeerMsgRet(m_llmq_ctx->isman->ProcessMessage(pfrom, this, msg_type, vRecv), pfrom);
4751+
ProcessPeerMsgRet(m_llmq_ctx->isman->ProcessMessage(pfrom, msg_type, vRecv), pfrom);
47524752
return;
47534753
}
47544754

0 commit comments

Comments
 (0)