From a6d484719cd431f714756d698b87617c0056bb1d Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 18 Oct 2019 02:21:14 +0300 Subject: [PATCH 01/15] refactor: Add `const` qualifier to various InstantSend related methods --- src/llmq/quorums_instantsend.cpp | 34 +++++++++++++------------- src/llmq/quorums_instantsend.h | 42 ++++++++++++++++---------------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 15cf51b80c1d..a5d6a6dd8b83 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -177,12 +177,12 @@ void CInstantSendDb::RemoveArchivedInstantSendLocks(int nUntilHeight) db.WriteBatch(batch); } -bool CInstantSendDb::HasArchivedInstantSendLock(const uint256& islockHash) +bool CInstantSendDb::HasArchivedInstantSendLock(const uint256& islockHash) const { return db.Exists(std::make_tuple(std::string(DB_ARCHIVED_BY_HASH), islockHash)); } -size_t CInstantSendDb::GetInstantSendLockCount() +size_t CInstantSendDb::GetInstantSendLockCount() const { auto it = std::unique_ptr(db.NewIterator()); auto firstKey = std::make_tuple(std::string(DB_ISLOCK_BY_HASH), uint256()); @@ -204,7 +204,7 @@ size_t CInstantSendDb::GetInstantSendLockCount() return cnt; } -CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash) +CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash) const { CInstantSendLockPtr ret; if (islockCache.get(hash, ret)) { @@ -220,7 +220,7 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash return ret; } -uint256 CInstantSendDb::GetInstantSendLockHashByTxid(const uint256& txid) +uint256 CInstantSendDb::GetInstantSendLockHashByTxid(const uint256& txid) const { uint256 islockHash; @@ -240,7 +240,7 @@ uint256 CInstantSendDb::GetInstantSendLockHashByTxid(const uint256& txid) return islockHash; } -CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByTxid(const uint256& txid) +CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByTxid(const uint256& txid) const { uint256 islockHash = GetInstantSendLockHashByTxid(txid); if (islockHash.IsNull()) { @@ -249,7 +249,7 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByTxid(const uint256& txid return GetInstantSendLockByHash(islockHash); } -CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& outpoint) +CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& outpoint) const { uint256 islockHash; bool found = outpointCache.get(outpoint, islockHash); @@ -268,7 +268,7 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& o return GetInstantSendLockByHash(islockHash); } -std::vector CInstantSendDb::GetInstantSendLocksByParent(const uint256& parent) +std::vector CInstantSendDb::GetInstantSendLocksByParent(const uint256& parent) const { auto it = std::unique_ptr(db.NewIterator()); auto firstKey = std::make_tuple(std::string(DB_HASH_BY_OUTPOINT), COutPoint(parent, 0)); @@ -477,7 +477,7 @@ bool CInstantSendManager::ProcessTx(const CTransaction& tx, bool allowReSigning, return true; } -bool CInstantSendManager::CheckCanLock(const CTransaction& tx, bool printDebug, const Consensus::Params& params) +bool CInstantSendManager::CheckCanLock(const CTransaction& tx, bool printDebug, const Consensus::Params& params) const { if (tx.vin.empty()) { // can't lock TXs without inputs (e.g. quorum commitments) @@ -494,7 +494,7 @@ bool CInstantSendManager::CheckCanLock(const CTransaction& tx, bool printDebug, return true; } -bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, CAmount* retValue, const Consensus::Params& params) +bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, CAmount* retValue, const Consensus::Params& params) const { int nInstantSendConfirmationsRequired = params.nInstantSendConfirmationsRequired; @@ -1432,7 +1432,7 @@ bool CInstantSendManager::ProcessPendingRetryLockTxs() return retryCount != 0; } -bool CInstantSendManager::AlreadyHave(const CInv& inv) +bool CInstantSendManager::AlreadyHave(const CInv& inv) const { if (!IsInstantSendEnabled()) { return true; @@ -1442,7 +1442,7 @@ bool CInstantSendManager::AlreadyHave(const CInv& inv) return db.GetInstantSendLockByHash(inv.hash) != nullptr || pendingInstantSendLocks.count(inv.hash) != 0 || db.HasArchivedInstantSendLock(inv.hash); } -bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, llmq::CInstantSendLock& ret) +bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, llmq::CInstantSendLock& ret) const { if (!IsInstantSendEnabled()) { return false; @@ -1457,7 +1457,7 @@ bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, llmq::CI return true; } -CInstantSendLockPtr CInstantSendManager::GetInstantSendLockByTxid(const uint256& txid) +CInstantSendLockPtr CInstantSendManager::GetInstantSendLockByTxid(const uint256& txid) const { if (!IsInstantSendEnabled()) { return nullptr; @@ -1467,7 +1467,7 @@ CInstantSendLockPtr CInstantSendManager::GetInstantSendLockByTxid(const uint256& return db.GetInstantSendLockByTxid(txid); } -bool CInstantSendManager::GetInstantSendLockHashByTxid(const uint256& txid, uint256& ret) +bool CInstantSendManager::GetInstantSendLockHashByTxid(const uint256& txid, uint256& ret) const { if (!IsInstantSendEnabled()) { return false; @@ -1478,7 +1478,7 @@ bool CInstantSendManager::GetInstantSendLockHashByTxid(const uint256& txid, uint return !ret.IsNull(); } -bool CInstantSendManager::IsLocked(const uint256& txHash) +bool CInstantSendManager::IsLocked(const uint256& txHash) const { if (!IsInstantSendEnabled()) { return false; @@ -1488,12 +1488,12 @@ bool CInstantSendManager::IsLocked(const uint256& txHash) return db.GetInstantSendLockByTxid(txHash) != nullptr; } -bool CInstantSendManager::IsConflicted(const CTransaction& tx) +bool CInstantSendManager::IsConflicted(const CTransaction& tx) const { return GetConflictingLock(tx) != nullptr; } -CInstantSendLockPtr CInstantSendManager::GetConflictingLock(const CTransaction& tx) +CInstantSendLockPtr CInstantSendManager::GetConflictingLock(const CTransaction& tx) const { if (!IsInstantSendEnabled()) { return nullptr; @@ -1513,7 +1513,7 @@ CInstantSendLockPtr CInstantSendManager::GetConflictingLock(const CTransaction& return nullptr; } -size_t CInstantSendManager::GetInstantSendLockCount() +size_t CInstantSendManager::GetInstantSendLockCount() const { return db.GetInstantSendLockCount(); } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 8e5bbcc7caf9..0553b5d73745 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -45,9 +45,9 @@ class CInstantSendDb private: CDBWrapper& db; - unordered_lru_cache islockCache; - unordered_lru_cache txidCache; - unordered_lru_cache outpointCache; + mutable unordered_lru_cache islockCache; + mutable unordered_lru_cache txidCache; + mutable unordered_lru_cache outpointCache; public: explicit CInstantSendDb(CDBWrapper& _db) : db(_db) {} @@ -60,22 +60,22 @@ class CInstantSendDb static void WriteInstantSendLockArchived(CDBBatch& batch, const uint256& hash, int nHeight); std::unordered_map RemoveConfirmedInstantSendLocks(int nUntilHeight); void RemoveArchivedInstantSendLocks(int nUntilHeight); - bool HasArchivedInstantSendLock(const uint256& islockHash); - size_t GetInstantSendLockCount(); + bool HasArchivedInstantSendLock(const uint256& islockHash) const; + size_t GetInstantSendLockCount() const; - CInstantSendLockPtr GetInstantSendLockByHash(const uint256& hash); - uint256 GetInstantSendLockHashByTxid(const uint256& txid); - CInstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid); - CInstantSendLockPtr GetInstantSendLockByInput(const COutPoint& outpoint); + CInstantSendLockPtr GetInstantSendLockByHash(const uint256& hash) const; + uint256 GetInstantSendLockHashByTxid(const uint256& txid) const; + CInstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid) const; + CInstantSendLockPtr GetInstantSendLockByInput(const COutPoint& outpoint) const; - std::vector GetInstantSendLocksByParent(const uint256& parent); + std::vector GetInstantSendLocksByParent(const uint256& parent) const; std::vector RemoveChainedInstantSendLocks(const uint256& islockHash, const uint256& txid, int nHeight); }; class CInstantSendManager : public CRecoveredSigsListener { private: - CCriticalSection cs; + mutable CCriticalSection cs; CInstantSendDb db; std::thread workThread; @@ -121,11 +121,11 @@ class CInstantSendManager : public CRecoveredSigsListener public: bool ProcessTx(const CTransaction& tx, bool allowReSigning, const Consensus::Params& params); - bool CheckCanLock(const CTransaction& tx, bool printDebug, const Consensus::Params& params); - bool CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, CAmount* retValue, const Consensus::Params& params); - bool IsLocked(const uint256& txHash); - bool IsConflicted(const CTransaction& tx); - CInstantSendLockPtr GetConflictingLock(const CTransaction& tx); + bool CheckCanLock(const CTransaction& tx, bool printDebug, const Consensus::Params& params) const; + bool CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, CAmount* retValue, const Consensus::Params& params) const; + bool IsLocked(const uint256& txHash) const; + bool IsConflicted(const CTransaction& tx) const; + CInstantSendLockPtr GetConflictingLock(const CTransaction& tx) const; virtual void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig); void HandleNewInputLockRecoveredSig(const CRecoveredSig& recoveredSig, const uint256& txid); @@ -161,12 +161,12 @@ class CInstantSendManager : public CRecoveredSigsListener static void AskNodesForLockedTx(const uint256& txid); bool ProcessPendingRetryLockTxs(); - bool AlreadyHave(const CInv& inv); - bool GetInstantSendLockByHash(const uint256& hash, CInstantSendLock& ret); - CInstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid); - bool GetInstantSendLockHashByTxid(const uint256& txid, uint256& ret); + bool AlreadyHave(const CInv& inv) const; + bool GetInstantSendLockByHash(const uint256& hash, CInstantSendLock& ret) const; + CInstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid) const; + bool GetInstantSendLockHashByTxid(const uint256& txid, uint256& ret) const; - size_t GetInstantSendLockCount(); + size_t GetInstantSendLockCount() const; void WorkThreadMain(); }; From 10e5d07653c76c89c0fdb64269d940c207d4d30d Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 24 Jan 2021 02:27:25 +0300 Subject: [PATCH 02/15] instantsend: bump IS cache size --- src/llmq/quorums_instantsend.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 0553b5d73745..ffa154e59579 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -45,9 +45,9 @@ class CInstantSendDb private: CDBWrapper& db; - mutable unordered_lru_cache islockCache; - mutable unordered_lru_cache txidCache; - mutable unordered_lru_cache outpointCache; + mutable unordered_lru_cache islockCache; + mutable unordered_lru_cache txidCache; + mutable unordered_lru_cache outpointCache; public: explicit CInstantSendDb(CDBWrapper& _db) : db(_db) {} From daa2deb9922a68057faca9b932252e1938dae943 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 24 Jan 2021 15:46:34 +0300 Subject: [PATCH 03/15] instantsend: keep islocks cache when removing confirmed islocks from db unordered_lru_cache should truncate it automagically --- src/llmq/quorums_instantsend.cpp | 26 ++++++++++++++------------ src/llmq/quorums_instantsend.h | 4 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index a5d6a6dd8b83..1675df3a8151 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -65,10 +65,10 @@ void CInstantSendDb::WriteNewInstantSendLock(const uint256& hash, const CInstant } } -void CInstantSendDb::RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, CInstantSendLockPtr islock) +void CInstantSendDb::RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, CInstantSendLockPtr islock, bool keep_cache) { if (!islock) { - islock = GetInstantSendLockByHash(hash); + islock = GetInstantSendLockByHash(hash, false); if (!islock) { return; } @@ -80,10 +80,12 @@ void CInstantSendDb::RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, batch.Erase(std::make_tuple(std::string(DB_HASH_BY_OUTPOINT), in)); } - islockCache.erase(hash); - txidCache.erase(islock->txid); - for (auto& in : islock->inputs) { - outpointCache.erase(in); + if (!keep_cache) { + islockCache.erase(hash); + txidCache.erase(islock->txid); + for (auto& in : islock->inputs) { + outpointCache.erase(in); + } } } @@ -129,7 +131,7 @@ std::unordered_map CInstantSendDb::RemoveConfirmed } auto& islockHash = std::get<2>(curKey); - auto islock = GetInstantSendLockByHash(islockHash); + auto islock = GetInstantSendLockByHash(islockHash, false); if (islock) { RemoveInstantSendLock(batch, islockHash, islock); ret.emplace(islockHash, islock); @@ -204,10 +206,10 @@ size_t CInstantSendDb::GetInstantSendLockCount() const return cnt; } -CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash) const +CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash, bool use_cache) const { CInstantSendLockPtr ret; - if (islockCache.get(hash, ret)) { + if (use_cache && islockCache.get(hash, ret)) { return ret; } @@ -311,12 +313,12 @@ std::vector CInstantSendDb::RemoveChainedInstantSendLocks(const uint256 stack.pop_back(); for (auto& childIslockHash : children) { - auto childIsLock = GetInstantSendLockByHash(childIslockHash); + auto childIsLock = GetInstantSendLockByHash(childIslockHash, false); if (!childIsLock) { continue; } - RemoveInstantSendLock(batch, childIslockHash, childIsLock); + RemoveInstantSendLock(batch, childIslockHash, childIsLock, false); WriteInstantSendLockArchived(batch, childIslockHash, nHeight); result.emplace_back(childIslockHash); @@ -326,7 +328,7 @@ std::vector CInstantSendDb::RemoveChainedInstantSendLocks(const uint256 } } - RemoveInstantSendLock(batch, islockHash, nullptr); + RemoveInstantSendLock(batch, islockHash, nullptr, false); WriteInstantSendLockArchived(batch, islockHash, nHeight); result.emplace_back(islockHash); diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index ffa154e59579..94270d2a56c1 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -53,7 +53,7 @@ class CInstantSendDb explicit CInstantSendDb(CDBWrapper& _db) : db(_db) {} void WriteNewInstantSendLock(const uint256& hash, const CInstantSendLock& islock); - void RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, CInstantSendLockPtr islock); + void RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, CInstantSendLockPtr islock, bool keep_cache = true); void WriteInstantSendLockMined(const uint256& hash, int nHeight); void RemoveInstantSendLockMined(const uint256& hash, int nHeight); @@ -63,7 +63,7 @@ class CInstantSendDb bool HasArchivedInstantSendLock(const uint256& islockHash) const; size_t GetInstantSendLockCount() const; - CInstantSendLockPtr GetInstantSendLockByHash(const uint256& hash) const; + CInstantSendLockPtr GetInstantSendLockByHash(const uint256& hash, bool use_cache = true) const; uint256 GetInstantSendLockHashByTxid(const uint256& txid) const; CInstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid) const; CInstantSendLockPtr GetInstantSendLockByInput(const COutPoint& outpoint) const; From 122b7bee7cca1316559eaeddc5d985f5b83644a4 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 26 Jan 2021 13:46:31 +0300 Subject: [PATCH 04/15] refactor: Drop ProcessNewTransaction --- src/llmq/quorums_instantsend.cpp | 91 +++++++++++++++----------------- src/llmq/quorums_instantsend.h | 1 - 2 files changed, 44 insertions(+), 48 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 1675df3a8151..521aa09a11c8 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -965,62 +965,29 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has } } -void CInstantSendManager::ProcessNewTransaction(const CTransactionRef& tx, const CBlockIndex* pindex, bool allowReSigning) -{ - if (!IsInstantSendEnabled()) { - return; - } - - if (tx->IsCoinBase() || tx->vin.empty()) { - // coinbase and TXs with no inputs can't be locked - return; - } - - uint256 islockHash; - { - LOCK(cs); - islockHash = db.GetInstantSendLockHashByTxid(tx->GetHash()); - - // update DB about when an IS lock was mined - if (!islockHash.IsNull() && pindex) { - db.WriteInstantSendLockMined(islockHash, pindex->nHeight); - } - } - - if (!masternodeSync.IsBlockchainSynced()) { - return; - } - - bool chainlocked = pindex && chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash()); - if (islockHash.IsNull() && !chainlocked) { - ProcessTx(*tx, allowReSigning, Params().GetConsensus()); - } - - LOCK(cs); - if (!chainlocked && islockHash.IsNull()) { - // TX is not locked, so make sure it is tracked - AddNonLockedTx(tx, pindex); - } else { - // TX is locked, so make sure we don't track it anymore - RemoveNonLockedTx(tx->GetHash(), true); - } -} - void CInstantSendManager::TransactionAddedToMempool(const CTransactionRef& tx) { - if (!IsInstantSendEnabled()) { + if (!IsInstantSendEnabled() || !masternodeSync.IsBlockchainSynced() || tx->vin.empty()) { return; } - CInstantSendLockPtr islock{nullptr}; + CInstantSendLockPtr islock; { LOCK(cs); islock = db.GetInstantSendLockByTxid(tx->GetHash()); } - ProcessNewTransaction(tx, nullptr, false); - - if (islock != nullptr) { + if (islock == nullptr) { + ProcessTx(*tx, false, Params().GetConsensus()); + // TX is not locked, so make sure it is tracked + LOCK(cs); + AddNonLockedTx(tx, nullptr); + } else { + { + // TX is locked, so make sure we don't track it anymore + LOCK(cs); + RemoveNonLockedTx(tx->GetHash(), true); + } // If the islock was received before the TX, we know we were not able to send // the notification at that time, we need to do it now. LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- notify about an earlier received lock for tx %s\n", __func__, tx->GetHash().ToString()); @@ -1042,7 +1009,37 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr& pb } for (const auto& tx : pblock->vtx) { - ProcessNewTransaction(tx, pindex, true); + if (tx->IsCoinBase() || tx->vin.empty()) { + // coinbase and TXs with no inputs can't be locked + continue; + } + + uint256 islockHash; + { + LOCK(cs); + islockHash = db.GetInstantSendLockHashByTxid(tx->GetHash()); + + // update DB about when an IS lock was mined + if (!islockHash.IsNull()) { + db.WriteInstantSendLockMined(islockHash, pindex->nHeight); + } + } + + if (!masternodeSync.IsBlockchainSynced()) { + continue; + } + + bool non_locked = islockHash.IsNull() && !chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash()); + if (non_locked) { + ProcessTx(*tx, true, Params().GetConsensus()); + // TX is not locked, so make sure it is tracked + LOCK(cs); + AddNonLockedTx(tx, pindex); + } else { + // TX is locked, so make sure we don't track it anymore + LOCK(cs); + RemoveNonLockedTx(tx->GetHash(), true); + } } } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 94270d2a56c1..4f44b8f2e595 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -140,7 +140,6 @@ class CInstantSendManager : public CRecoveredSigsListener std::unordered_set ProcessPendingInstantSendLocks(int signOffset, const std::unordered_map, StaticSaltedHasher>& pend, bool ban); void ProcessInstantSendLock(NodeId from, const uint256& hash, const CInstantSendLockPtr& islock); - void ProcessNewTransaction(const CTransactionRef& tx, const CBlockIndex* pindex, bool allowReSigning); void TransactionAddedToMempool(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); From b500659f5a0e1e50872a25b7aa5e5e46d17bfdb7 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 26 Jan 2021 16:37:32 +0300 Subject: [PATCH 05/15] instantsend: Introduce (and use) KnownInstantSendLock Check islock hash against both current and archived islocks --- src/llmq/quorums_instantsend.cpp | 15 ++++++--------- src/llmq/quorums_instantsend.h | 3 ++- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 521aa09a11c8..9d6cd72447c9 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -179,9 +179,9 @@ void CInstantSendDb::RemoveArchivedInstantSendLocks(int nUntilHeight) db.WriteBatch(batch); } -bool CInstantSendDb::HasArchivedInstantSendLock(const uint256& islockHash) const +bool CInstantSendDb::KnownInstantSendLock(const uint256& islockHash) const { - return db.Exists(std::make_tuple(std::string(DB_ARCHIVED_BY_HASH), islockHash)); + return GetInstantSendLockByHash(islockHash) != nullptr || db.Exists(std::make_tuple(std::string(DB_ARCHIVED_BY_HASH), islockHash)); } size_t CInstantSendDb::GetInstantSendLockCount() const @@ -695,10 +695,7 @@ void CInstantSendManager::ProcessMessageInstantSendLock(CNode* pfrom, const llmq } LOCK(cs); - if (db.GetInstantSendLockByHash(hash) != nullptr) { - return; - } - if (pendingInstantSendLocks.count(hash)) { + if (pendingInstantSendLocks.count(hash) || db.KnownInstantSendLock(hash)) { return; } @@ -916,7 +913,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has txToCreatingInstantSendLocks.erase(islock->txid); CInstantSendLockPtr otherIsLock; - if (db.GetInstantSendLockByHash(hash)) { + if (pendingInstantSendLocks.count(hash) || db.KnownInstantSendLock(hash)) { return; } otherIsLock = db.GetInstantSendLockByTxid(islock->txid); @@ -1438,7 +1435,7 @@ bool CInstantSendManager::AlreadyHave(const CInv& inv) const } LOCK(cs); - return db.GetInstantSendLockByHash(inv.hash) != nullptr || pendingInstantSendLocks.count(inv.hash) != 0 || db.HasArchivedInstantSendLock(inv.hash); + return pendingInstantSendLocks.count(inv.hash) != 0 || db.KnownInstantSendLock(inv.hash); } bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, llmq::CInstantSendLock& ret) const @@ -1484,7 +1481,7 @@ bool CInstantSendManager::IsLocked(const uint256& txHash) const } LOCK(cs); - return db.GetInstantSendLockByTxid(txHash) != nullptr; + return db.KnownInstantSendLock(db.GetInstantSendLockHashByTxid(txHash)); } bool CInstantSendManager::IsConflicted(const CTransaction& tx) const diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 4f44b8f2e595..7671cbbb7b4e 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -60,7 +60,8 @@ class CInstantSendDb static void WriteInstantSendLockArchived(CDBBatch& batch, const uint256& hash, int nHeight); std::unordered_map RemoveConfirmedInstantSendLocks(int nUntilHeight); void RemoveArchivedInstantSendLocks(int nUntilHeight); - bool HasArchivedInstantSendLock(const uint256& islockHash) const; + + bool KnownInstantSendLock(const uint256& islockHash) const; size_t GetInstantSendLockCount() const; CInstantSendLockPtr GetInstantSendLockByHash(const uint256& hash, bool use_cache = true) const; From fc0874cf228dfaadac1a965a79eb8af1a6bd4d54 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 26 Jan 2021 21:39:30 +0300 Subject: [PATCH 06/15] instantsend: Optimize ProcessInstantSendLock, check for known islocks first Two reasons: 1. GetTransaction is a potentially much heavy one 2. Doesn't make much sense to look for a tx if we have a known islock already --- src/llmq/quorums_instantsend.cpp | 50 +++++++++++++++++--------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 9d6cd72447c9..649980c9467f 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -882,27 +882,6 @@ std::unordered_set CInstantSendManager::ProcessPendingInstantSendLocks( void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& hash, const CInstantSendLockPtr& islock) { - CTransactionRef tx; - uint256 hashBlock; - const CBlockIndex* pindexMined = nullptr; - // we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally - if (GetTransaction(islock->txid, tx, Params().GetConsensus(), hashBlock)) { - if (!hashBlock.IsNull()) { - { - LOCK(cs_main); - pindexMined = LookupBlockIndex(hashBlock); - } - - // Let's see if the TX that was locked by this islock is already mined in a ChainLocked block. If yes, - // we can simply ignore the islock, as the ChainLock implies locking of all TXs in that chain - if (llmq::chainLocksHandler->HasChainLock(pindexMined->nHeight, pindexMined->GetBlockHash())) { - LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txlock=%s, islock=%s: dropping islock as it already got a ChainLock in block %s, peer=%d\n", __func__, - islock->txid.ToString(), hash.ToString(), hashBlock.ToString(), from); - return; - } - } - } - { LOCK(cs); @@ -930,9 +909,6 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has } db.WriteNewInstantSendLock(hash, *islock); - if (pindexMined) { - db.WriteInstantSendLockMined(hash, pindexMined->nHeight); - } // This will also add children TXs to pendingRetryTxs RemoveNonLockedTx(islock->txid, true); @@ -942,6 +918,32 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has TruncateRecoveredSigsForInputs(*islock); } + CTransactionRef tx; + uint256 hashBlock; + // we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally + if (GetTransaction(islock->txid, tx, Params().GetConsensus(), hashBlock)) { + if (!hashBlock.IsNull()) { + const CBlockIndex* pindexMined = nullptr; + { + LOCK(cs_main); + pindexMined = LookupBlockIndex(hashBlock); + } + + { + LOCK(cs); + db.WriteInstantSendLockMined(hash, pindexMined->nHeight); + } + + // Let's see if the TX that was locked by this islock is already mined in a ChainLocked block. If yes, + // we can simply ignore the islock, as the ChainLock implies locking of all TXs in that chain + if (llmq::chainLocksHandler->HasChainLock(pindexMined->nHeight, pindexMined->GetBlockHash())) { + LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txlock=%s, islock=%s: dropping islock as it already got a ChainLock in block %s, peer=%d\n", __func__, + islock->txid.ToString(), hash.ToString(), hashBlock.ToString(), from); + return; + } + } + } + CInv inv(MSG_ISLOCK, hash); if (tx != nullptr) { g_connman->RelayInvFiltered(inv, *tx, LLMQS_PROTO_VERSION); From 310fcc90dd7fbcced00d4ea9ff26b1fc8da84add Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 26 Jan 2021 22:04:41 +0300 Subject: [PATCH 07/15] instantsend: Batched write/erase for connected/disconnected blocks --- src/llmq/quorums_instantsend.cpp | 79 +++++++++++++++++++++----------- src/llmq/quorums_instantsend.h | 7 ++- 2 files changed, 59 insertions(+), 27 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 649980c9467f..3da1dd43a98b 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -96,12 +96,19 @@ static std::tuple BuildInversedISLockKey(const s void CInstantSendDb::WriteInstantSendLockMined(const uint256& hash, int nHeight) { - db.Write(BuildInversedISLockKey(DB_MINED_BY_HEIGHT_AND_HASH, nHeight, hash), true); + CDBBatch batch(db); + WriteInstantSendLockMined(batch, hash, nHeight); + db.WriteBatch(batch); } -void CInstantSendDb::RemoveInstantSendLockMined(const uint256& hash, int nHeight) +void CInstantSendDb::WriteInstantSendLockMined(CDBBatch& batch, const uint256& hash, int nHeight) { - db.Erase(BuildInversedISLockKey(DB_MINED_BY_HEIGHT_AND_HASH, nHeight, hash)); + batch.Write(BuildInversedISLockKey(DB_MINED_BY_HEIGHT_AND_HASH, nHeight, hash), true); +} + +void CInstantSendDb::RemoveInstantSendLockMined(CDBBatch& batch, const uint256& hash, int nHeight) +{ + batch.Erase(BuildInversedISLockKey(DB_MINED_BY_HEIGHT_AND_HASH, nHeight, hash)); } void CInstantSendDb::WriteInstantSendLockArchived(CDBBatch& batch, const uint256& hash, int nHeight) @@ -179,6 +186,39 @@ void CInstantSendDb::RemoveArchivedInstantSendLocks(int nUntilHeight) db.WriteBatch(batch); } +void CInstantSendDb::WriteBlockInstantSendLocks(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected) +{ + CDBBatch batch(db); + for (const auto& tx : pblock->vtx) { + if (tx->IsCoinBase() || tx->vin.empty()) { + // coinbase and TXs with no inputs can't be locked + continue; + } + uint256 islockHash = GetInstantSendLockHashByTxid(tx->GetHash()); + // update DB about when an IS lock was mined + if (!islockHash.IsNull()) { + WriteInstantSendLockMined(batch, islockHash, pindexConnected->nHeight); + } + } + db.WriteBatch(batch); +} + +void CInstantSendDb::RemoveBlockInstantSendLocks(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected) +{ + CDBBatch batch(db); + for (const auto& tx : pblock->vtx) { + if (tx->IsCoinBase() || tx->vin.empty()) { + // coinbase and TXs with no inputs can't be locked + continue; + } + uint256 islockHash = GetInstantSendLockHashByTxid(tx->GetHash()); + if (!islockHash.IsNull()) { + RemoveInstantSendLockMined(batch, islockHash, pindexDisconnected->nHeight); + } + } + db.WriteBatch(batch); +} + bool CInstantSendDb::KnownInstantSendLock(const uint256& islockHash) const { return GetInstantSendLockByHash(islockHash) != nullptr || db.Exists(std::make_tuple(std::string(DB_ARCHIVED_BY_HASH), islockHash)); @@ -1008,27 +1048,12 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr& pb } for (const auto& tx : pblock->vtx) { - if (tx->IsCoinBase() || tx->vin.empty()) { + if (tx->IsCoinBase() || tx->vin.empty() || !masternodeSync.IsBlockchainSynced()) { // coinbase and TXs with no inputs can't be locked continue; } - uint256 islockHash; - { - LOCK(cs); - islockHash = db.GetInstantSendLockHashByTxid(tx->GetHash()); - - // update DB about when an IS lock was mined - if (!islockHash.IsNull()) { - db.WriteInstantSendLockMined(islockHash, pindex->nHeight); - } - } - - if (!masternodeSync.IsBlockchainSynced()) { - continue; - } - - bool non_locked = islockHash.IsNull() && !chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash()); + bool non_locked = !IsLocked(tx->GetHash()) && !chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash()); if (non_locked) { ProcessTx(*tx, true, Params().GetConsensus()); // TX is not locked, so make sure it is tracked @@ -1040,17 +1065,19 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr& pb RemoveNonLockedTx(tx->GetHash(), true); } } + + LOCK(cs); + db.WriteBlockInstantSendLocks(pblock, pindex); } void CInstantSendManager::BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected) { - LOCK(cs); - for (auto& tx : pblock->vtx) { - auto islockHash = db.GetInstantSendLockHashByTxid(tx->GetHash()); - if (!islockHash.IsNull()) { - db.RemoveInstantSendLockMined(islockHash, pindexDisconnected->nHeight); - } + if (!IsInstantSendEnabled()) { + return; } + + LOCK(cs); + db.RemoveBlockInstantSendLocks(pblock, pindexDisconnected); } void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx, const CBlockIndex* pindexMined) diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 7671cbbb7b4e..eabd6e07245d 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -49,6 +49,9 @@ class CInstantSendDb mutable unordered_lru_cache txidCache; mutable unordered_lru_cache outpointCache; + void WriteInstantSendLockMined(CDBBatch& batch, const uint256& hash, int nHeight); + void RemoveInstantSendLockMined(CDBBatch& batch, const uint256& hash, int nHeight); + public: explicit CInstantSendDb(CDBWrapper& _db) : db(_db) {} @@ -56,11 +59,13 @@ class CInstantSendDb void RemoveInstantSendLock(CDBBatch& batch, const uint256& hash, CInstantSendLockPtr islock, bool keep_cache = true); void WriteInstantSendLockMined(const uint256& hash, int nHeight); - void RemoveInstantSendLockMined(const uint256& hash, int nHeight); static void WriteInstantSendLockArchived(CDBBatch& batch, const uint256& hash, int nHeight); std::unordered_map RemoveConfirmedInstantSendLocks(int nUntilHeight); void RemoveArchivedInstantSendLocks(int nUntilHeight); + void WriteBlockInstantSendLocks(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected); + void RemoveBlockInstantSendLocks(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected); + bool KnownInstantSendLock(const uint256& islockHash) const; size_t GetInstantSendLockCount() const; From 8fc05212cbc989f11ec688cbe0075d853fa14b43 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 27 Jan 2021 00:16:31 +0300 Subject: [PATCH 08/15] instantsend: Mark a block with IS-locks which conflict with txes in a CL-ed block as conflicting and not as invalid --- src/llmq/quorums_instantsend.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 3da1dd43a98b..c4fec6a1f2de 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -1329,8 +1329,8 @@ void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const CValidationState state; // need non-const pointer auto pindex2 = LookupBlockIndex(pindex->GetBlockHash()); - if (!InvalidateBlock(state, Params(), pindex2)) { - LogPrintf("CInstantSendManager::%s -- InvalidateBlock failed: %s\n", __func__, FormatStateMessage(state)); + if (!MarkConflictingBlock(state, Params(), pindex2)) { + LogPrintf("CInstantSendManager::%s -- MarkConflictingBlock failed: %s\n", __func__, FormatStateMessage(state)); // This should not have happened and we are in a state were it's not safe to continue anymore assert(false); } From f8194c468f134bec7fb3afd96f769d2caa799cf6 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 27 Jan 2021 01:06:04 +0300 Subject: [PATCH 09/15] instantsend: Bail out early on disabled IS in more places --- src/llmq/quorums_instantsend.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index c4fec6a1f2de..48e8ac220487 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -248,6 +248,10 @@ size_t CInstantSendDb::GetInstantSendLockCount() const CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash, bool use_cache) const { + if (!IsInstantSendEnabled()) { + return nullptr; + } + CInstantSendLockPtr ret; if (use_cache && islockCache.get(hash, ret)) { return ret; @@ -264,6 +268,9 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash uint256 CInstantSendDb::GetInstantSendLockHashByTxid(const uint256& txid) const { + if (!IsInstantSendEnabled()) { + return {}; + } uint256 islockHash; bool found = txidCache.get(txid, islockHash); @@ -293,6 +300,9 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByTxid(const uint256& txid CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& outpoint) const { + if (!IsInstantSendEnabled()) { + return nullptr; + } uint256 islockHash; bool found = outpointCache.get(outpoint, islockHash); if (found && islockHash.IsNull()) { @@ -1189,6 +1199,10 @@ void CInstantSendManager::UpdatedBlockTip(const CBlockIndex* pindexNew) void CInstantSendManager::HandleFullyConfirmedBlock(const CBlockIndex* pindex) { + if (!IsInstantSendEnabled()) { + return; + } + LOCK(cs); auto& consensusParams = Params().GetConsensus(); From d139f534be5507589bb82ee6864525c35b94b9ab Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 27 Jan 2021 01:07:48 +0300 Subject: [PATCH 10/15] instantsend: Disable InstantSend while reindexing and importing blocks --- src/llmq/quorums_instantsend.cpp | 4 ++-- src/validation.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 48e8ac220487..2216ea81a6f0 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -1575,12 +1575,12 @@ void CInstantSendManager::WorkThreadMain() bool IsInstantSendEnabled() { - return sporkManager.IsSporkActive(SPORK_2_INSTANTSEND_ENABLED); + return !fReindex && !fImporting && sporkManager.IsSporkActive(SPORK_2_INSTANTSEND_ENABLED); } bool RejectConflictingBlocks() { - return sporkManager.IsSporkActive(SPORK_3_INSTANTSEND_BLOCK_FILTERING); + return !fReindex && !fImporting && sporkManager.IsSporkActive(SPORK_3_INSTANTSEND_BLOCK_FILTERING); } } // namespace llmq diff --git a/src/validation.cpp b/src/validation.cpp index fb74ccb2bf93..b1f547de30b6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2387,7 +2387,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl REJECT_INVALID, "conflict-tx-lock"); } } - } else { + } else if (!fReindex && !fImporting) { LogPrintf("ConnectBlock(DASH): spork is off, skipping transaction locking checks\n"); } From 0d563bc6731f8232e5a2ac17abbefb21f68b01c1 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 27 Jan 2021 03:40:51 +0300 Subject: [PATCH 11/15] refactor: Simplify GetInstantSendLock(*) methods --- src/llmq/quorums_instantsend.cpp | 35 ++++++-------------------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 2216ea81a6f0..35e4505a2700 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -248,7 +248,7 @@ size_t CInstantSendDb::GetInstantSendLockCount() const CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByHash(const uint256& hash, bool use_cache) const { - if (!IsInstantSendEnabled()) { + if (hash.IsNull() || !IsInstantSendEnabled()) { return nullptr; } @@ -272,30 +272,16 @@ uint256 CInstantSendDb::GetInstantSendLockHashByTxid(const uint256& txid) const return {}; } uint256 islockHash; - - bool found = txidCache.get(txid, islockHash); - if (found && islockHash.IsNull()) { - return {}; - } - - if (!found) { - found = db.Read(std::make_tuple(std::string(DB_HASH_BY_TXID), txid), islockHash); + if (!txidCache.get(txid, islockHash)) { + db.Read(std::make_tuple(std::string(DB_HASH_BY_TXID), txid), islockHash); txidCache.insert(txid, islockHash); } - - if (!found) { - return {}; - } return islockHash; } CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByTxid(const uint256& txid) const { - uint256 islockHash = GetInstantSendLockHashByTxid(txid); - if (islockHash.IsNull()) { - return nullptr; - } - return GetInstantSendLockByHash(islockHash); + return GetInstantSendLockByHash(GetInstantSendLockHashByTxid(txid)); } CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& outpoint) const @@ -304,19 +290,10 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& o return nullptr; } uint256 islockHash; - bool found = outpointCache.get(outpoint, islockHash); - if (found && islockHash.IsNull()) { - return nullptr; - } - - if (!found) { - found = db.Read(std::make_tuple(std::string(DB_HASH_BY_OUTPOINT), outpoint), islockHash); + if (!outpointCache.get(outpoint, islockHash)) { + db.Read(std::make_tuple(std::string(DB_HASH_BY_OUTPOINT), outpoint), islockHash); outpointCache.insert(outpoint, islockHash); } - - if (!found) { - return nullptr; - } return GetInstantSendLockByHash(islockHash); } From 172d1af47b5748451b9d462d4973dee02fff7cd9 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 27 Jan 2021 03:23:25 +0300 Subject: [PATCH 12/15] refactor: Check height inside Remove(Archived/Confirmed)InstantSendLocks --- src/llmq/quorums_instantsend.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 35e4505a2700..86449da07ddf 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -119,6 +119,10 @@ void CInstantSendDb::WriteInstantSendLockArchived(CDBBatch& batch, const uint256 std::unordered_map CInstantSendDb::RemoveConfirmedInstantSendLocks(int nUntilHeight) { + if (nUntilHeight <= 0) { + return {}; + } + auto it = std::unique_ptr(db.NewIterator()); auto firstKey = BuildInversedISLockKey(DB_MINED_BY_HEIGHT_AND_HASH, nUntilHeight, uint256()); @@ -159,6 +163,10 @@ std::unordered_map CInstantSendDb::RemoveConfirmed void CInstantSendDb::RemoveArchivedInstantSendLocks(int nUntilHeight) { + if (nUntilHeight <= 0) { + return; + } + auto it = std::unique_ptr(db.NewIterator()); auto firstKey = BuildInversedISLockKey(DB_ARCHIVED_BY_HEIGHT_AND_HASH, nUntilHeight, uint256()); @@ -1183,12 +1191,8 @@ void CInstantSendManager::HandleFullyConfirmedBlock(const CBlockIndex* pindex) LOCK(cs); auto& consensusParams = Params().GetConsensus(); - auto removeISLocks = db.RemoveConfirmedInstantSendLocks(pindex->nHeight); - if (pindex->nHeight > 100) { - db.RemoveArchivedInstantSendLocks(pindex->nHeight - 100); - } for (auto& p : removeISLocks) { auto& islockHash = p.first; auto& islock = p.second; @@ -1204,6 +1208,8 @@ void CInstantSendManager::HandleFullyConfirmedBlock(const CBlockIndex* pindex) quorumSigningManager->TruncateRecoveredSig(consensusParams.llmqTypeInstantSend, islock->GetRequestId()); } + db.RemoveArchivedInstantSendLocks(pindex->nHeight - 100); + // Find all previously unlocked TXs that got locked by this fully confirmed (ChainLock) block and remove them // from the nonLockedTxs map. Also collect all children of these TXs and mark them for retrying of IS locking. std::vector toRemove; From 7c0ff5db9b52114432f9e39f1987d6defc79c5f7 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 27 Jan 2021 03:38:17 +0300 Subject: [PATCH 13/15] refactor: Squash two loops in AddNonLockedTx into one `!info.tx == res.second` is always `true` --- src/llmq/quorums_instantsend.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 86449da07ddf..9cd70998435f 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -1082,15 +1082,10 @@ void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx, const CBlock auto& info = res.first->second; info.pindexMined = pindexMined; - if (!info.tx) { + if (res.second) { info.tx = tx; for (const auto& in : tx->vin) { nonLockedTxs[in.prevout.hash].children.emplace(tx->GetHash()); - } - } - - if (res.second) { - for (auto& in : tx->vin) { nonLockedTxsByOutpoints.emplace(in.prevout, tx->GetHash()); } } From ddfbfa988a188fee4611d1e245674e41f4617d70 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 27 Jan 2021 22:22:45 +0300 Subject: [PATCH 14/15] tests: Tweak feature_llmq_is_cl_conflicts.py to test CL overriding a block with conflicting IS-locks --- .../feature_llmq_is_cl_conflicts.py | 63 +++++++++++++++---- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/test/functional/feature_llmq_is_cl_conflicts.py b/test/functional/feature_llmq_is_cl_conflicts.py index 486df51d5132..856a731b40d8 100755 --- a/test/functional/feature_llmq_is_cl_conflicts.py +++ b/test/functional/feature_llmq_is_cl_conflicts.py @@ -8,7 +8,7 @@ from test_framework.blocktools import get_masternode_payment, create_coinbase, create_block from test_framework.mininode import * from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_raises_rpc_error, get_bip9_status +from test_framework.util import assert_equal, assert_raises_rpc_error, get_bip9_status ''' feature_llmq_is_cl_conflicts.py @@ -67,10 +67,14 @@ def run_test(self): self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash()) self.test_chainlock_overrides_islock(False) - self.test_chainlock_overrides_islock(True) + self.test_chainlock_overrides_islock(True, False) + self.test_chainlock_overrides_islock(True, True) self.test_islock_overrides_nonchainlock() - def test_chainlock_overrides_islock(self, test_block_conflict): + def test_chainlock_overrides_islock(self, test_block_conflict, mine_confllicting=False): + if not test_block_conflict: + assert not mine_confllicting + # create three raw TXs, they will conflict with each other rawtx1 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex'] rawtx2 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex'] @@ -104,6 +108,10 @@ def test_chainlock_overrides_islock(self, test_block_conflict): assert(submit_result == "conflict-tx-lock") cl = self.create_chainlock(self.nodes[0].getblockcount() + 1, block) + + if mine_confllicting: + islock_tip = self.nodes[0].generate(1)[-1] + self.test_node.send_clsig(cl) for node in self.nodes: @@ -111,6 +119,19 @@ def test_chainlock_overrides_islock(self, test_block_conflict): self.sync_blocks() + if mine_confllicting: + # The tip with IS-locked txes should be marked conflicting now + found1 = False + found2 = False + for tip in self.nodes[0].getchaintips(2): + if tip["hash"] == islock_tip: + assert tip["status"] == "conflicting" + found1 = True + elif tip["hash"] == block.hash: + assert tip["status"] == "active" + found2 = True + assert found1 and found2 + # At this point all nodes should be in sync and have the same "best chainlock" submit_result = self.nodes[1].submitblock(ToHex(block)) @@ -139,14 +160,34 @@ def test_chainlock_overrides_islock(self, test_block_conflict): for node in self.nodes: self.wait_for_instantlock(rawtx5_txid, node) - # Lets verify that the ISLOCKs got pruned - for node in self.nodes: - assert_raises_rpc_error(-5, "No such mempool or blockchain transaction", node.getrawtransaction, rawtx1_txid, True) - assert_raises_rpc_error(-5, "No such mempool or blockchain transaction", node.getrawtransaction, rawtx4_txid, True) - rawtx = node.getrawtransaction(rawtx2_txid, True) - assert(rawtx['chainlock']) - assert(rawtx['instantlock']) - assert(not rawtx['instantlock_internal']) + if mine_confllicting: + # Lets verify that the ISLOCKs got pruned and conflicting txes were mined but never confirmed + for node in self.nodes: + rawtx = node.getrawtransaction(rawtx1_txid, True) + assert not rawtx['chainlock'] + assert not rawtx['instantlock'] + assert not rawtx['instantlock_internal'] + assert_equal(rawtx['confirmations'], 0) + assert_equal(rawtx['height'], -1) + rawtx = node.getrawtransaction(rawtx4_txid, True) + assert not rawtx['chainlock'] + assert not rawtx['instantlock'] + assert not rawtx['instantlock_internal'] + assert_equal(rawtx['confirmations'], 0) + assert_equal(rawtx['height'], -1) + rawtx = node.getrawtransaction(rawtx2_txid, True) + assert rawtx['chainlock'] + assert rawtx['instantlock'] + assert not rawtx['instantlock_internal'] + else: + # Lets verify that the ISLOCKs got pruned + for node in self.nodes: + assert_raises_rpc_error(-5, "No such mempool or blockchain transaction", node.getrawtransaction, rawtx1_txid, True) + assert_raises_rpc_error(-5, "No such mempool or blockchain transaction", node.getrawtransaction, rawtx4_txid, True) + rawtx = node.getrawtransaction(rawtx2_txid, True) + assert rawtx['chainlock'] + assert rawtx['instantlock'] + assert not rawtx['instantlock_internal'] def test_islock_overrides_nonchainlock(self): # create two raw TXs, they will conflict with each other From 1b92bca2df02398a8586c20ec2f4de3e7fe0bb09 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 31 Jan 2021 11:52:27 +0300 Subject: [PATCH 15/15] instantsend|sigs: Sleep when there is no more work Instead of sleeping only when no work has been done. Avoids useless cycles, improves batching. --- src/llmq/quorums_instantsend.cpp | 24 ++++++++++-------------- src/llmq/quorums_instantsend.h | 2 +- src/llmq/quorums_signing.cpp | 2 +- src/llmq/quorums_signing_shares.cpp | 20 ++++++++------------ src/llmq/quorums_signing_shares.h | 2 +- 5 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 9cd70998435f..0668e0165083 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -765,6 +765,7 @@ bool CInstantSendManager::PreVerifyInstantSendLock(const llmq::CInstantSendLock& bool CInstantSendManager::ProcessPendingInstantSendLocks() { decltype(pendingInstantSendLocks) pend; + bool more_work{false}; { LOCK(cs); @@ -779,6 +780,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() pend.emplace(it->first, std::move(it->second)); pendingInstantSendLocks.erase(it); } + more_work = true; } } @@ -810,7 +812,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() ProcessPendingInstantSendLocks(dkgInterval, pend, true); } - return true; + return more_work; } std::unordered_set CInstantSendManager::ProcessPendingInstantSendLocks(int signOffset, const std::unordered_map, StaticSaltedHasher>& pend, bool ban) @@ -1382,7 +1384,7 @@ void CInstantSendManager::AskNodesForLockedTx(const uint256& txid) } } -bool CInstantSendManager::ProcessPendingRetryLockTxs() +void CInstantSendManager::ProcessPendingRetryLockTxs() { decltype(pendingRetryTxs) retryTxs; { @@ -1391,11 +1393,11 @@ bool CInstantSendManager::ProcessPendingRetryLockTxs() } if (retryTxs.empty()) { - return false; + return; } if (!IsInstantSendEnabled()) { - return false; + return; } int retryCount = 0; @@ -1445,8 +1447,6 @@ bool CInstantSendManager::ProcessPendingRetryLockTxs() LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- retried %d TXs. nonLockedTxs.size=%d\n", __func__, retryCount, nonLockedTxs.size()); } - - return retryCount != 0; } bool CInstantSendManager::AlreadyHave(const CInv& inv) const @@ -1538,15 +1538,11 @@ size_t CInstantSendManager::GetInstantSendLockCount() const void CInstantSendManager::WorkThreadMain() { while (!workInterrupt) { - bool didWork = false; - - didWork |= ProcessPendingInstantSendLocks(); - didWork |= ProcessPendingRetryLockTxs(); + bool more_work = ProcessPendingInstantSendLocks(); + ProcessPendingRetryLockTxs(); - if (!didWork) { - if (!workInterrupt.sleep_for(std::chrono::milliseconds(100))) { - return; - } + if (!more_work && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { + return; } } } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index eabd6e07245d..3710ba0df943 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -164,7 +164,7 @@ class CInstantSendManager : public CRecoveredSigsListener void ResolveBlockConflicts(const uint256& islockHash, const CInstantSendLock& islock); void RemoveChainLockConflictingLock(const uint256& islockHash, const CInstantSendLock& islock); static void AskNodesForLockedTx(const uint256& txid); - bool ProcessPendingRetryLockTxs(); + void ProcessPendingRetryLockTxs(); bool AlreadyHave(const CInv& inv) const; bool GetInstantSendLockByHash(const uint256& hash, CInstantSendLock& ret) const; diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index 70cbc5d29fc1..c09f31ba4834 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -675,7 +675,7 @@ bool CSigningManager::ProcessPendingRecoveredSigs() } } - return true; + return verifyCount >= 32; } // signature must be verified already diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 8859441d481e..52817e4d7bce 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -704,7 +704,7 @@ bool CSigSharesManager::ProcessPendingSigShares(CConnman& connman) ProcessPendingSigShares(v, quorums, connman); } - return true; + return verifyCount >= 32; } // It's ensured that no duplicates are passed to this method @@ -1501,12 +1501,12 @@ void CSigSharesManager::WorkThreadMain() continue; } - bool didWork = false; + bool more_work = false; RemoveBannedNodeStates(); - didWork |= quorumSigningManager->ProcessPendingRecoveredSigs(); - didWork |= ProcessPendingSigShares(*g_connman); - didWork |= SignPendingSigShares(); + more_work |= quorumSigningManager->ProcessPendingRecoveredSigs(); + more_work |= ProcessPendingSigShares(*g_connman); + SignPendingSigShares(); if (GetTimeMillis() - lastSendTime > 100) { SendMessages(); @@ -1517,10 +1517,8 @@ void CSigSharesManager::WorkThreadMain() quorumSigningManager->Cleanup(); // TODO Wakeup when pending signing is needed? - if (!didWork) { - if (!workInterrupt.sleep_for(std::chrono::milliseconds(100))) { - return; - } + if (!more_work && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { + return; } } } @@ -1531,7 +1529,7 @@ void CSigSharesManager::AsyncSign(const CQuorumCPtr& quorum, const uint256& id, pendingSigns.emplace_back(quorum, id, msgHash); } -bool CSigSharesManager::SignPendingSigShares() +void CSigSharesManager::SignPendingSigShares() { std::vector> v; { @@ -1557,8 +1555,6 @@ bool CSigSharesManager::SignPendingSigShares() } } } - - return !v.empty(); } CSigShare CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) diff --git a/src/llmq/quorums_signing_shares.h b/src/llmq/quorums_signing_shares.h index 695fcac5b88c..898a0b5e69c9 100644 --- a/src/llmq/quorums_signing_shares.h +++ b/src/llmq/quorums_signing_shares.h @@ -451,7 +451,7 @@ class CSigSharesManager : public CRecoveredSigsListener void CollectSigSharesToSend(std::unordered_map>& sigSharesToSend); void CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToSend, const std::vector& vNodes); void CollectSigSharesToAnnounce(std::unordered_map>& sigSharesToAnnounce); - bool SignPendingSigShares(); + void SignPendingSigShares(); void WorkThreadMain(); };