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
This commit is contained in:
Kittywhiskers Van Gogh 2024-04-09 17:43:08 +00:00
parent bfd33cd2b4
commit 35be4e2ebe
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
4 changed files with 12 additions and 18 deletions

View File

@ -43,7 +43,7 @@ LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterminis
}()},
isman{[&]() -> llmq::CInstantSendManager* const {
assert(llmq::quorumInstantSendManager == nullptr);
llmq::quorumInstantSendManager = std::make_unique<llmq::CInstantSendManager>(*llmq::chainLocksHandler, chainstate, connman, *llmq::quorumManager, *sigman, *shareman, sporkman, mempool, mn_sync, unit_tests, wipe);
llmq::quorumInstantSendManager = std::make_unique<llmq::CInstantSendManager>(*llmq::chainLocksHandler, chainstate, connman, *llmq::quorumManager, *sigman, *shareman, sporkman, mempool, mn_sync, peerman, unit_tests, wipe);
return llmq::quorumInstantSendManager.get();
}()},
ehfSignalsHandler{std::make_unique<llmq::CEHFSignalsHandler>(chainstate, mnhfman, *sigman, *shareman, mempool, *llmq::quorumManager, sporkman, peerman)}

View File

@ -747,15 +747,9 @@ void CInstantSendManager::HandleNewInstantSendLockRecoveredSig(const llmq::CReco
pendingInstantSendLocks.emplace(hash, std::make_pair(-1, islock));
}
PeerMsgRet CInstantSendManager::ProcessMessage(const CNode& pfrom, gsl::not_null<PeerManager*> peerman, std::string_view msg_type, CDataStream& vRecv)
PeerMsgRet CInstantSendManager::ProcessMessage(const CNode& pfrom, std::string_view msg_type, CDataStream& vRecv)
{
if (IsInstantSendEnabled() && msg_type == NetMsgType::ISDLOCK) {
if (m_peerman == nullptr) {
m_peerman = peerman;
}
// we should never use one CInstantSendManager with different PeerManager
assert(m_peerman == peerman);
const auto islock = std::make_shared<CInstantSendLock>();
vRecv >> *islock;
return ProcessMessageInstantSendLock(pfrom, islock);
@ -957,7 +951,7 @@ std::unordered_set<uint256, StaticSaltedHasher> CInstantSendManager::ProcessPend
for (const auto& nodeId : batchVerifier.badSources) {
// Let's not be too harsh, as the peer might simply be unlucky and might have sent us an old lock which
// does not validate anymore due to changed quorums
m_peerman.load()->Misbehaving(nodeId, 20);
m_peerman->Misbehaving(nodeId, 20);
}
}
for (const auto& p : pend) {
@ -1051,11 +1045,11 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
CInv inv(MSG_ISDLOCK, hash);
if (tx != nullptr) {
m_peerman.load()->RelayInvFiltered(inv, *tx, ISDLOCK_PROTO_VERSION);
m_peerman->RelayInvFiltered(inv, *tx, ISDLOCK_PROTO_VERSION);
} else {
// we don't have the TX yet, so we only filter based on txid. Later when that TX arrives, we will re-announce
// with the TX taken into account.
m_peerman.load()->RelayInvFiltered(inv, islock->txid, ISDLOCK_PROTO_VERSION);
m_peerman->RelayInvFiltered(inv, islock->txid, ISDLOCK_PROTO_VERSION);
}
ResolveBlockConflicts(hash, *islock);
@ -1068,7 +1062,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has
// bump mempool counter to make sure newly locked txes are picked up by getblocktemplate
mempool.AddTransactionsUpdated(1);
} else {
AskNodesForLockedTx(islock->txid, connman, *m_peerman.load());
AskNodesForLockedTx(islock->txid, connman, *m_peerman);
}
}
@ -1344,7 +1338,7 @@ void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, con
for (const auto& p : toDelete) {
RemoveConflictedTx(*p.second);
}
AskNodesForLockedTx(islock.txid, connman, *m_peerman.load());
AskNodesForLockedTx(islock.txid, connman, *m_peerman);
}
}

View File

@ -206,7 +206,7 @@ private:
CSporkManager& spork_manager;
CTxMemPool& mempool;
const CMasternodeSync& m_mn_sync;
std::atomic<PeerManager*> m_peerman{nullptr};
const std::unique_ptr<PeerManager>& m_peerman;
std::atomic<bool> fUpgradedDB{false};
@ -257,10 +257,10 @@ public:
explicit CInstantSendManager(CChainLocksHandler& _clhandler, CChainState& chainstate, CConnman& _connman,
CQuorumManager& _qman, CSigningManager& _sigman, CSigSharesManager& _shareman,
CSporkManager& sporkman, CTxMemPool& _mempool, const CMasternodeSync& mn_sync,
bool unitTests, bool fWipe) :
const std::unique_ptr<PeerManager>& peerman, bool unitTests, bool fWipe) :
db(unitTests, fWipe),
clhandler(_clhandler), m_chainstate(chainstate), connman(_connman), qman(_qman), sigman(_sigman),
shareman(_shareman), spork_manager(sporkman), mempool(_mempool), m_mn_sync(mn_sync)
shareman(_shareman), spork_manager(sporkman), mempool(_mempool), m_mn_sync(mn_sync), m_peerman(peerman)
{
workInterrupt.reset();
}
@ -313,7 +313,7 @@ public:
void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override LOCKS_EXCLUDED(cs_inputReqests, cs_creating);
PeerMsgRet ProcessMessage(const CNode& pfrom, gsl::not_null<PeerManager*> peerman, std::string_view msg_type, CDataStream& vRecv);
PeerMsgRet ProcessMessage(const CNode& pfrom, std::string_view msg_type, CDataStream& vRecv);
void TransactionAddedToMempool(const CTransactionRef& tx) LOCKS_EXCLUDED(cs_pendingLocks);
void TransactionRemovedFromMempool(const CTransactionRef& tx);

View File

@ -4748,7 +4748,7 @@ void PeerManagerImpl::ProcessMessage(
m_llmq_ctx->shareman->ProcessMessage(pfrom, m_sporkman, msg_type, vRecv);
ProcessPeerMsgRet(m_llmq_ctx->sigman->ProcessMessage(pfrom, this, msg_type, vRecv), pfrom);
ProcessPeerMsgRet(m_llmq_ctx->clhandler->ProcessMessage(pfrom, msg_type, vRecv), pfrom);
ProcessPeerMsgRet(m_llmq_ctx->isman->ProcessMessage(pfrom, this, msg_type, vRecv), pfrom);
ProcessPeerMsgRet(m_llmq_ctx->isman->ProcessMessage(pfrom, msg_type, vRecv), pfrom);
return;
}