Skip to content

Commit

Permalink
llmq: pass PeerManager to llmq::CInstantSendManager constructor
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kwvg committed Apr 23, 2024
1 parent bfd33cd commit 35be4e2
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/llmq/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
18 changes: 6 additions & 12 deletions src/llmq/instantsend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/llmq/instantsend.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ class CInstantSendManager : public CRecoveredSigsListener
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};

Expand Down Expand Up @@ -257,10 +257,10 @@ class CInstantSendManager : public CRecoveredSigsListener
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();
}
Expand Down Expand Up @@ -313,7 +313,7 @@ class CInstantSendManager : public CRecoveredSigsListener

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);
Expand Down
2 changes: 1 addition & 1 deletion src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 35be4e2

Please sign in to comment.