instantsend: Use NotifyEntryRemoved signal instead of calling CInstantSendManager::TransactionRemovedFromMempool from CTxMemPool::removeUnchecked directly (#4160)

* instantsend: Use `NotifyEntryRemoved` signal instead of calling `CInstantSendManager::TransactionRemovedFromMempool` from `CTxMemPool::removeUnchecked` directly

Fixes potential mempool.cs vs cs_main (in RemoveConflictingLock) deadlock

* Apply suggestions from code review

yay, typso!

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
This commit is contained in:
UdjinM6 2021-05-19 02:02:31 +03:00 committed by GitHub
parent a02f8302d9
commit cd7cd85fe7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 21 additions and 20 deletions

View File

@ -82,7 +82,7 @@ void CDSNotificationInterface::TransactionAddedToMempool(const CTransactionRef&
CCoinJoin::TransactionAddedToMempool(ptx);
}
void CDSNotificationInterface::TransactionRemovedFromMempool(const CTransactionRef& ptx)
void CDSNotificationInterface::TransactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason)
{
llmq::quorumInstantSendManager->TransactionRemovedFromMempool(ptx);
}

View File

@ -23,7 +23,7 @@ protected:
void SynchronousUpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override;
void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override;
void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override;
void TransactionRemovedFromMempool(const CTransactionRef& ptx) override;
void TransactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason) override;
void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex, const std::vector<CTransactionRef>& vtxConflicted) override;
void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexDisconnected) override;
void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff) override;

View File

@ -623,9 +623,6 @@ bool CTxMemPool::removeSpentIndex(const uint256 txhash)
void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
{
NotifyEntryRemoved(it->GetSharedTx(), reason);
if (reason != MemPoolRemovalReason::BLOCK) {
llmq::quorumInstantSendManager->TransactionRemovedFromMempool(it->GetSharedTx());
}
const uint256 hash = it->GetTx().GetHash();
for (const CTxIn& txin : it->GetTx().vin)
mapNextTx.erase(txin.prevout);

View File

@ -25,7 +25,7 @@ struct MainSignalsInstance {
boost::signals2::signal<void (const CTransactionRef &, int64_t)> TransactionAddedToMempool;
boost::signals2::signal<void (const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex, const std::vector<CTransactionRef>&)> BlockConnected;
boost::signals2::signal<void (const std::shared_ptr<const CBlock> &, const CBlockIndex* pindexDisconnected)> BlockDisconnected;
boost::signals2::signal<void (const CTransactionRef &)> TransactionRemovedFromMempool;
boost::signals2::signal<void (const CTransactionRef &, MemPoolRemovalReason)> TransactionRemovedFromMempool;
boost::signals2::signal<void (const CBlockLocator &)> SetBestChain;
boost::signals2::signal<void (int64_t nBestBlockTime, CConnman* connman)> Broadcast;
boost::signals2::signal<void (const CBlock&, const CValidationState&)> BlockChecked;
@ -92,7 +92,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) {
g_signals.m_internals->BlockDisconnected.connect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1, _2));
g_signals.m_internals->NotifyTransactionLock.connect(boost::bind(&CValidationInterface::NotifyTransactionLock, pwalletIn, _1, _2));
g_signals.m_internals->NotifyChainLock.connect(boost::bind(&CValidationInterface::NotifyChainLock, pwalletIn, _1, _2));
g_signals.m_internals->TransactionRemovedFromMempool.connect(boost::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, _1));
g_signals.m_internals->TransactionRemovedFromMempool.connect(boost::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, _1, _2));
g_signals.m_internals->SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
g_signals.m_internals->Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2));
g_signals.m_internals->BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
@ -113,7 +113,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
g_signals.m_internals->TransactionAddedToMempool.disconnect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1, _2));
g_signals.m_internals->BlockConnected.disconnect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3));
g_signals.m_internals->BlockDisconnected.disconnect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1, _2));
g_signals.m_internals->TransactionRemovedFromMempool.disconnect(boost::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, _1));
g_signals.m_internals->TransactionRemovedFromMempool.disconnect(boost::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, _1, _2));
g_signals.m_internals->UpdatedBlockTip.disconnect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1, _2, _3));
g_signals.m_internals->SynchronousUpdatedBlockTip.disconnect(boost::bind(&CValidationInterface::SynchronousUpdatedBlockTip, pwalletIn, _1, _2, _3));
g_signals.m_internals->NewPoWValidBlock.disconnect(boost::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, _1, _2));
@ -166,9 +166,9 @@ void SyncWithValidationInterfaceQueue() {
}
void CMainSignals::MempoolEntryRemoved(CTransactionRef ptx, MemPoolRemovalReason reason) {
if (reason != MemPoolRemovalReason::BLOCK && reason != MemPoolRemovalReason::CONFLICT) {
m_internals->m_schedulerClient.AddToProcessQueue([ptx, this] {
m_internals->TransactionRemovedFromMempool(ptx);
if (reason != MemPoolRemovalReason::BLOCK) {
m_internals->m_schedulerClient.AddToProcessQueue([ptx, reason, this] {
m_internals->TransactionRemovedFromMempool(ptx, reason);
});
}
}

View File

@ -112,7 +112,7 @@ protected:
*
* Called on a background thread.
*/
virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx) {}
virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {}
/**
* Notifies listeners of a block being connected.
* Provides a vector of transactions evicted from the mempool as a result.

View File

@ -1476,11 +1476,13 @@ void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx, int64_t nAcc
}
}
void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) {
LOCK(cs_wallet);
auto it = mapWallet.find(ptx->GetHash());
if (it != mapWallet.end()) {
it->second.fInMempool = false;
void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
if (reason != MemPoolRemovalReason::CONFLICT) {
LOCK(cs_wallet);
auto it = mapWallet.find(ptx->GetHash());
if (it != mapWallet.end()) {
it->second.fInMempool = false;
}
}
}
@ -1496,11 +1498,13 @@ void CWallet::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const
for (const CTransactionRef& ptx : vtxConflicted) {
SyncTransaction(ptx);
TransactionRemovedFromMempool(ptx);
// UNKNOWN because it's a manual removal, not using mempool logic
TransactionRemovedFromMempool(ptx, MemPoolRemovalReason::UNKNOWN);
}
for (size_t i = 0; i < pblock->vtx.size(); i++) {
SyncTransaction(pblock->vtx[i], pindex, i);
TransactionRemovedFromMempool(pblock->vtx[i]);
// UNKNOWN because it's a manual removal, not using mempool logic
TransactionRemovedFromMempool(pblock->vtx[i], MemPoolRemovalReason::UNKNOWN);
}
m_last_block_processed = pindex;

View File

@ -1033,7 +1033,7 @@ public:
bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update);
CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, const WalletRescanReserver& reserver, bool fUpdate = false);
void TransactionRemovedFromMempool(const CTransactionRef &ptx) override;
void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override;
void ReacceptWalletTransactions();
void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override;
// ResendWalletTransactionsBefore may only be called if fBroadcastTransactions!