From f18d4e1d1c245bce3f461387e282af5dc415a07e Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 15 May 2021 03:17:16 +0300 Subject: [PATCH] instantsend: Remove islocks for rejected/removed txes (#4155) * instantsend: Resolve block conflicts first and take care of mempool ones later * refactor: Rename RemoveChainLockConflictingLock -> RemoveConflictingLock * instantsend: Handle transaction removal from mempool (for all reasons besides inclusion in blocks) * instantsend: Remove old islocks with no known txes from db (once) * refactor: Replace magic number with CURRENT_VERSION * fix: Do not remove islocks for (yet) valid orphans * Apply suggestions from code review Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com> Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com> --- src/dsnotificationinterface.cpp | 5 +++ src/dsnotificationinterface.h | 1 + src/llmq/quorums_instantsend.cpp | 71 ++++++++++++++++++++++++++++++-- src/llmq/quorums_instantsend.h | 9 +++- src/net_processing.cpp | 2 + src/txmempool.cpp | 3 ++ src/validation.cpp | 2 +- 7 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index 3fae7d1fa8..da6cf77717 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -83,6 +83,11 @@ void CDSNotificationInterface::TransactionAddedToMempool(const CTransactionRef& CCoinJoin::TransactionAddedToMempool(ptx); } +void CDSNotificationInterface::TransactionRemovedFromMempool(const CTransactionRef& ptx) +{ + llmq::quorumInstantSendManager->TransactionRemovedFromMempool(ptx); +} + void CDSNotificationInterface::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex, const std::vector& vtxConflicted) { // TODO: Tempoarily ensure that mempool removals are notified before diff --git a/src/dsnotificationinterface.h b/src/dsnotificationinterface.h index 42a965ce91..a9d04f3a0a 100644 --- a/src/dsnotificationinterface.h +++ b/src/dsnotificationinterface.h @@ -23,6 +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 BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex, const std::vector& vtxConflicted) override; void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected) override; void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff) override; diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index e3ca26af53..1d38360d7b 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -31,6 +31,10 @@ static const std::string DB_MINED_BY_HEIGHT_AND_HASH = "is_m"; static const std::string DB_ARCHIVED_BY_HEIGHT_AND_HASH = "is_a1"; static const std::string DB_ARCHIVED_BY_HASH = "is_a2"; +static const std::string DB_VERSION = "is_v"; + +const int CInstantSendDb::CURRENT_VERSION; + CInstantSendManager* quorumInstantSendManager; uint256 CInstantSendLock::GetRequestId() const @@ -43,6 +47,46 @@ uint256 CInstantSendLock::GetRequestId() const //////////////// +CInstantSendDb::CInstantSendDb(CDBWrapper& _db) : db(_db) +{ + Upgrade(); +} + +void CInstantSendDb::Upgrade() +{ + int v{0}; + if (!db.Read(DB_VERSION, v) || v < CInstantSendDb::CURRENT_VERSION) { + CDBBatch batch(db); + CInstantSendLock islock; + CTransactionRef tx; + uint256 hashBlock; + + auto it = std::unique_ptr(db.NewIterator()); + auto firstKey = std::make_tuple(DB_ISLOCK_BY_HASH, uint256()); + it->Seek(firstKey); + decltype(firstKey) curKey; + + while (it->Valid()) { + if (!it->GetKey(curKey) || std::get<0>(curKey) != DB_ISLOCK_BY_HASH) { + break; + } + if (it->GetValue(islock)) { + if (!GetTransaction(islock.txid, tx, Params().GetConsensus(), hashBlock)) { + // Drop locks for unknown txes + batch.Erase(std::make_tuple(DB_HASH_BY_TXID, islock.txid)); + for (auto& in : islock.inputs) { + batch.Erase(std::make_tuple(DB_HASH_BY_OUTPOINT, in)); + } + batch.Erase(curKey); + } + } + it->Next(); + } + batch.Write(DB_VERSION, CInstantSendDb::CURRENT_VERSION); + db.WriteBatch(batch); + } +} + void CInstantSendDb::WriteNewInstantSendLock(const uint256& hash, const CInstantSendLock& islock) { CDBBatch batch(db); @@ -982,8 +1026,8 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has g_connman->RelayInvFiltered(inv, islock->txid, LLMQS_PROTO_VERSION); } - RemoveMempoolConflictsForLock(hash, *islock); ResolveBlockConflicts(hash, *islock); + RemoveMempoolConflictsForLock(hash, *islock); if (tx != nullptr) { LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- notify about an in-time lock for tx %s\n", __func__, tx->GetHash().ToString()); @@ -1023,6 +1067,23 @@ void CInstantSendManager::TransactionAddedToMempool(const CTransactionRef& tx) } } +void CInstantSendManager::TransactionRemovedFromMempool(const CTransactionRef& tx) +{ + if (tx->vin.empty()) { + return; + } + + LOCK(cs); + CInstantSendLockPtr islock = db.GetInstantSendLockByTxid(tx->GetHash()); + + if (islock == nullptr) { + return; + } + + LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- transaction %s was removed from mempool\n", __func__, tx->GetHash().ToString()); + RemoveConflictingLock(::SerializeHash(*islock), *islock); +} + void CInstantSendManager::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex, const std::vector& vtxConflicted) { if (!IsInstantSendEnabled()) { @@ -1291,7 +1352,9 @@ void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const // when large parts of the masternode network are controlled by an attacker. In this case we must still find consensus // and its better to sacrifice individual ISLOCKs then to sacrifice whole ChainLocks. if (hasChainLockedConflict) { - RemoveChainLockConflictingLock(islockHash, islock); + LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: at least one conflicted TX already got a ChainLock\n", __func__, + islock.txid.ToString(), islockHash.ToString()); + RemoveConflictingLock(islockHash, islock); return; } @@ -1330,9 +1393,9 @@ void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const } } -void CInstantSendManager::RemoveChainLockConflictingLock(const uint256& islockHash, const llmq::CInstantSendLock& islock) +void CInstantSendManager::RemoveConflictingLock(const uint256& islockHash, const llmq::CInstantSendLock& islock) { - LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: at least one conflicted TX already got a ChainLock. Removing ISLOCK and its chained children.\n", __func__, + LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: Removing ISLOCK and its chained children\n", __func__, islock.txid.ToString(), islockHash.ToString()); int tipHeight; { diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index f547631fd3..b684960381 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -43,6 +43,8 @@ typedef std::shared_ptr CInstantSendLockPtr; class CInstantSendDb { private: + static const int CURRENT_VERSION = 1; + CDBWrapper& db; mutable unordered_lru_cache islockCache; @@ -52,8 +54,10 @@ private: void WriteInstantSendLockMined(CDBBatch& batch, const uint256& hash, int nHeight); void RemoveInstantSendLockMined(CDBBatch& batch, const uint256& hash, int nHeight); + void Upgrade(); + public: - explicit CInstantSendDb(CDBWrapper& _db) : db(_db) {} + explicit CInstantSendDb(CDBWrapper& _db); void WriteNewInstantSendLock(const uint256& hash, const CInstantSendLock& islock); void RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, CInstantSendLockPtr islock, bool keep_cache = true); @@ -146,6 +150,7 @@ public: void ProcessInstantSendLock(NodeId from, const uint256& hash, const CInstantSendLockPtr& islock); void TransactionAddedToMempool(const CTransactionRef& tx); + void TransactionRemovedFromMempool(const CTransactionRef& tx); void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex, const std::vector& vtxConflicted); void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected); @@ -161,7 +166,7 @@ public: void RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock); void ResolveBlockConflicts(const uint256& islockHash, const CInstantSendLock& islock); - void RemoveChainLockConflictingLock(const uint256& islockHash, const CInstantSendLock& islock); + void RemoveConflictingLock(const uint256& islockHash, const CInstantSendLock& islock); static void AskNodesForLockedTx(const uint256& txid); void ProcessPendingRetryLockTxs(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 9a34de1f34..ca99c24d47 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2938,6 +2938,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr // We will continue to reject this tx since it has rejected // parents so avoid re-requesting it from other peers. recentRejects->insert(tx.GetHash()); + llmq::quorumInstantSendManager->TransactionRemovedFromMempool(ptx); } } else { if (!state.CorruptionPossible()) { @@ -2965,6 +2966,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrintf("Not relaying invalid transaction %s from whitelisted peer=%d (%s)\n", tx.GetHash().ToString(), pfrom->GetId(), FormatStateMessage(state)); } } + llmq::quorumInstantSendManager->TransactionRemovedFromMempool(ptx); } int nDoS = 0; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index f7bfa69302..ffe4d56499 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -622,6 +622,9 @@ 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); diff --git a/src/validation.cpp b/src/validation.cpp index 6b1c87566e..66963fc444 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2379,7 +2379,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl continue; } if (llmq::chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash())) { - llmq::quorumInstantSendManager->RemoveChainLockConflictingLock(::SerializeHash(*conflictLock), *conflictLock); + llmq::quorumInstantSendManager->RemoveConflictingLock(::SerializeHash(*conflictLock), *conflictLock); assert(llmq::quorumInstantSendManager->GetConflictingLock(*tx) == nullptr); } else { // The node which relayed this should switch to correct chain.