From da4950eb0df0d7d71495503dabb7e9456ce69164 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 15:24:53 +0200 Subject: [PATCH 01/17] Return ISLOCK instead of conflicting txid in GetConflictingTx/GetConflictingLock --- src/llmq/quorums_instantsend.cpp | 13 +++++-------- src/llmq/quorums_instantsend.h | 2 +- src/validation.cpp | 14 +++++++------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 0b79d476dc0a..09004827dced 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -1158,15 +1158,13 @@ bool CInstantSendManager::IsLocked(const uint256& txHash) bool CInstantSendManager::IsConflicted(const CTransaction& tx) { - LOCK(cs); - uint256 dummy; - return GetConflictingTx(tx, dummy); + return GetConflictingLock(tx) != nullptr; } -bool CInstantSendManager::GetConflictingTx(const CTransaction& tx, uint256& retConflictTxHash) +CInstantSendLockPtr CInstantSendManager::GetConflictingLock(const CTransaction& tx) { if (!IsNewInstantSendEnabled()) { - return false; + return nullptr; } LOCK(cs); @@ -1177,11 +1175,10 @@ bool CInstantSendManager::GetConflictingTx(const CTransaction& tx, uint256& retC } if (otherIsLock->txid != tx.GetHash()) { - retConflictTxHash = otherIsLock->txid; - return true; + return otherIsLock; } } - return false; + return nullptr; } void CInstantSendManager::WorkThreadMain() diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index d75460aeb197..9ff24742a801 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -120,7 +120,7 @@ class CInstantSendManager : public CRecoveredSigsListener 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); - bool GetConflictingTx(const CTransaction& tx, uint256& retConflictTxHash); + CInstantSendLockPtr GetConflictingLock(const CTransaction& tx); virtual void HandleNewRecoveredSig(const CRecoveredSig& recoveredSig); void HandleNewInputLockRecoveredSig(const CRecoveredSig& recoveredSig, const uint256& txid); diff --git a/src/validation.cpp b/src/validation.cpp index 51e0e666e74e..a01139d8ad07 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -692,15 +692,15 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C REJECT_INVALID, "tx-txlock-conflict"); } - uint256 txConflictHash; - if (llmq::quorumInstantSendManager->GetConflictingTx(tx, txConflictHash)) { + llmq::CInstantSendLockPtr conflictLock = llmq::quorumInstantSendManager->GetConflictingLock(tx); + if (conflictLock) { CTransactionRef txConflict; uint256 hashBlock; - if (GetTransaction(txConflictHash, txConflict, Params().GetConsensus(), hashBlock)) { + if (GetTransaction(conflictLock->txid, txConflict, Params().GetConsensus(), hashBlock)) { GetMainSignals().NotifyInstantSendDoubleSpendAttempt(tx, *txConflict); } return state.DoS(10, error("AcceptToMemoryPool : Transaction %s conflicts with locked TX %s", - hash.ToString(), txConflictHash.ToString()), + hash.ToString(), conflictLock->txid.ToString()), REJECT_INVALID, "tx-txlock-conflict"); } @@ -2217,13 +2217,13 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd REJECT_INVALID, "conflict-tx-lock"); } } - uint256 txConflict; - if (llmq::quorumInstantSendManager->GetConflictingTx(*tx, txConflict)) { + llmq::CInstantSendLockPtr conflictLock = llmq::quorumInstantSendManager->GetConflictingLock(*tx); + if (conflictLock) { // The node which relayed this should switch to correct chain. // TODO: relay instantsend data/proof. LOCK(cs_main); mapRejectedBlocks.insert(std::make_pair(block.GetHash(), GetTime())); - return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), txConflict.ToString()), + return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), conflictLock->txid.ToString()), REJECT_INVALID, "conflict-tx-lock"); } } From 3d40285eafe7bd4adc5b95be3c8c908e30934fca Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Fri, 3 May 2019 19:38:55 +0200 Subject: [PATCH 02/17] Move code to write archived ISLOCKs into its own method We'll need this from another method as well later. --- src/llmq/quorums_instantsend.cpp | 9 +++++++-- src/llmq/quorums_instantsend.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 09004827dced..74b8ce0cfe2e 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -105,6 +105,12 @@ void CInstantSendDb::RemoveInstantSendLockMined(const uint256& hash, int nHeight db.Erase(BuildInversedISLockKey("is_m", nHeight, hash)); } +void CInstantSendDb::WriteInstantSendLockArchived(CDBBatch& batch, const uint256& hash, int nHeight) +{ + batch.Write(BuildInversedISLockKey("is_a1", nHeight, hash), true); + batch.Write(std::make_tuple(std::string("is_a2"), hash), true); +} + std::unordered_map CInstantSendDb::RemoveConfirmedInstantSendLocks(int nUntilHeight) { auto it = std::unique_ptr(db.NewIterator()); @@ -133,8 +139,7 @@ std::unordered_map CInstantSendDb::RemoveConfirmed } // archive the islock hash, so that we're still able to check if we've seen the islock in the past - batch.Write(BuildInversedISLockKey("is_a1", nHeight, islockHash), true); - batch.Write(std::make_tuple(std::string("is_a2"), islockHash), true); + WriteInstantSendLockArchived(batch, islockHash, nHeight); batch.Erase(curKey); diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 9ff24742a801..dfd47aa3f3f3 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -58,6 +58,7 @@ class CInstantSendDb void WriteInstantSendLockMined(const uint256& hash, int nHeight); void RemoveInstantSendLockMined(const uint256& hash, int nHeight); + void WriteInstantSendLockArchived(CDBBatch& batch, const uint256& hash, int nHeight); std::unordered_map RemoveConfirmedInstantSendLocks(int nUntilHeight); void RemoveArchivedInstantSendLocks(int nUntilHeight); bool HasArchivedInstantSendLock(const uint256& islockHash); From 0723d740ed8e8da7e374400073f3757fb0dd5b9d Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 15:35:50 +0200 Subject: [PATCH 03/17] Implement GetInstantSendLocksByParent and RemoveChainedInstantSendLocks These allow to easily delete multiple chains (actually trees) of ISLOCKs in one go. --- src/llmq/quorums_instantsend.cpp | 67 ++++++++++++++++++++++++++++++++ src/llmq/quorums_instantsend.h | 3 ++ 2 files changed, 70 insertions(+) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 74b8ce0cfe2e..980d2052d42c 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -249,6 +249,73 @@ CInstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& o return GetInstantSendLockByHash(islockHash); } +std::vector CInstantSendDb::GetInstantSendLocksByParent(const uint256& parent) +{ + auto it = std::unique_ptr(db.NewIterator()); + auto firstKey = std::make_tuple(std::string("is_in"), COutPoint(parent, 0)); + it->Seek(firstKey); + + std::vector result; + + while (it->Valid()) { + decltype(firstKey) curKey; + if (!it->GetKey(curKey) || std::get<0>(curKey) != "is_in") { + break; + } + auto& outpoint = std::get<1>(curKey); + if (outpoint.hash != parent) { + break; + } + + uint256 islockHash; + if (!it->GetValue(islockHash)) { + break; + } + result.emplace_back(islockHash); + it->Next(); + } + + return result; +} + +std::vector CInstantSendDb::RemoveChainedInstantSendLocks(const uint256& islockHash, const uint256& txid, int nHeight) +{ + std::vector result; + + std::vector stack; + std::unordered_set added; + stack.emplace_back(txid); + + CDBBatch batch(db); + while (!stack.empty()) { + auto children = GetInstantSendLocksByParent(stack.back()); + stack.pop_back(); + + for (auto& childIslockHash : children) { + auto childIsLock = GetInstantSendLockByHash(childIslockHash); + if (!childIsLock) { + continue; + } + + RemoveInstantSendLock(batch, childIslockHash, childIsLock); + WriteInstantSendLockArchived(batch, childIslockHash, nHeight); + result.emplace_back(childIslockHash); + + if (added.emplace(childIsLock->txid).second) { + stack.emplace_back(childIsLock->txid); + } + } + } + + RemoveInstantSendLock(batch, islockHash, nullptr); + WriteInstantSendLockArchived(batch, islockHash, nHeight); + result.emplace_back(islockHash); + + db.WriteBatch(batch); + + return result; +} + //////////////// CInstantSendManager::CInstantSendManager(CDBWrapper& _llmqDb) : diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index dfd47aa3f3f3..ca4bc11934a4 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -67,6 +67,9 @@ class CInstantSendDb uint256 GetInstantSendLockHashByTxid(const uint256& txid); CInstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid); CInstantSendLockPtr GetInstantSendLockByInput(const COutPoint& outpoint); + + std::vector GetInstantSendLocksByParent(const uint256& parent); + std::vector RemoveChainedInstantSendLocks(const uint256& islockHash, const uint256& txid, int nHeight); }; class CInstantSendManager : public CRecoveredSigsListener From d93f699e8cea1c065ed667f60edfddf80cd062a1 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 15:39:18 +0200 Subject: [PATCH 04/17] Implement RemoveConflictedTx and call it from RemoveMempoolConflictsForLock Also add "retryChildren" parameter to RemoveNonLockedTx so that we can skip retrying of non-locked children TXs. --- src/llmq/quorums_instantsend.cpp | 33 +++++++++++++++++++++++++------- src/llmq/quorums_instantsend.h | 3 ++- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 980d2052d42c..8aec2f6a1aa9 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -879,7 +879,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has } // This will also add children TXs to pendingRetryTxs - RemoveNonLockedTx(islock.txid); + RemoveNonLockedTx(islock.txid, true); } CInv inv(MSG_ISLOCK, hash); @@ -957,7 +957,7 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn nonLockedTxs.at(tx.GetHash()).pindexMined = posInBlock == CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK ? pindex : nullptr; } else { // TX is locked, so make sure we don't track it anymore - RemoveNonLockedTx(tx.GetHash()); + RemoveNonLockedTx(tx.GetHash(), true); } } @@ -975,7 +975,7 @@ void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx) } } -void CInstantSendManager::RemoveNonLockedTx(const uint256& txid) +void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChildren) { AssertLockHeld(cs); @@ -985,9 +985,11 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid) } auto& info = it->second; - // TX got locked, so we can retry locking children - for (auto& childTxid : info.children) { - pendingRetryTxs.emplace(childTxid); + if (retryChildren) { + // TX got locked, so we can retry locking children + for (auto& childTxid : info.children) { + pendingRetryTxs.emplace(childTxid); + } } if (info.tx) { @@ -1005,6 +1007,17 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid) nonLockedTxs.erase(it); } +void CInstantSendManager::RemoveConflictedTx(const CTransaction& tx) +{ + AssertLockHeld(cs); + RemoveNonLockedTx(tx.GetHash(), false); + + for (const auto& in : tx.vin) { + auto inputRequestId = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in)); + inputRequestIds.erase(inputRequestId); + } +} + void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindexChainLock) { HandleFullyConfirmedBlock(pindexChainLock); @@ -1062,7 +1075,7 @@ void CInstantSendManager::HandleFullyConfirmedBlock(const CBlockIndex* pindex) } for (auto& txid : toRemove) { // This will also add children to pendingRetryTxs - RemoveNonLockedTx(txid); + RemoveNonLockedTx(txid, true); } } @@ -1097,6 +1110,12 @@ void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, con } if (!toDelete.empty()) { + { + LOCK(cs); + for (auto& p : toDelete) { + RemoveConflictedTx(*p.second); + } + } AskNodesForLockedTx(islock.txid); } } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index ca4bc11934a4..85906ef84eff 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -141,7 +141,8 @@ class CInstantSendManager : public CRecoveredSigsListener void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock); void AddNonLockedTx(const CTransactionRef& tx); - void RemoveNonLockedTx(const uint256& txid); + void RemoveNonLockedTx(const uint256& txid, bool retryChildren); + void RemoveConflictedTx(const CTransaction& tx); void NotifyChainLock(const CBlockIndex* pindexChainLock); void UpdatedBlockTip(const CBlockIndex* pindexNew); From 8064cd0593d1cc653dc2e5a2479d60eaae9cf323 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 15:40:41 +0200 Subject: [PATCH 05/17] Properly handle/remove conflicted TXs (between mempool and new blocks) --- src/llmq/quorums_instantsend.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 8aec2f6a1aa9..64a35c6f14f9 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -930,6 +930,19 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn return; } + bool inMempool = mempool.get(tx.GetHash()) != nullptr; + + // Are we called from validation.cpp/MemPoolConflictRemovalTracker? + // TODO refactor this when we backport the BlockConnected signal from Bitcoin, as it gives better info about + // conflicted TXs + bool isConflictRemoved = !pindex && posInBlock == CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK && !inMempool; + + if (isConflictRemoved) { + LOCK(cs); + RemoveConflictedTx(tx); + return; + } + uint256 islockHash; { LOCK(cs); From cd2b26ede9b7f4fc77e3fbf23d84e0c3d8090e5b Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 15:41:10 +0200 Subject: [PATCH 06/17] Track non-locked TXs by inputs --- src/llmq/quorums_instantsend.cpp | 14 ++++++++++++++ src/llmq/quorums_instantsend.h | 1 + 2 files changed, 15 insertions(+) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 64a35c6f14f9..6f90fd5792f3 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -986,6 +986,10 @@ void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx) nonLockedTxs[in.prevout.hash].children.emplace(tx->GetHash()); } } + + for (auto& in : tx->vin) { + nonLockedTxsByInputs.emplace(in.prevout.hash, std::make_pair(in.prevout.n, tx->GetHash())); + } } void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChildren) @@ -1014,6 +1018,16 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChild nonLockedTxs.erase(jt); } } + + auto its = nonLockedTxsByInputs.equal_range(in.prevout.hash); + for (auto kt = its.first; kt != its.second; ) { + if (kt->second.first != in.prevout.n) { + ++kt; + continue; + } else { + kt = nonLockedTxsByInputs.erase(kt); + } + } } } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 85906ef84eff..a8899e0c7d3e 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -107,6 +107,7 @@ class CInstantSendManager : public CRecoveredSigsListener std::unordered_set children; }; std::unordered_map nonLockedTxs; + std::unordered_multimap> nonLockedTxsByInputs; std::unordered_set pendingRetryTxs; From 9ec2ff390fdafbd5251c3000891fd6ed3089eb3c Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 15:50:16 +0200 Subject: [PATCH 07/17] Implement and call ResolveBlockConflicts --- src/llmq/quorums_instantsend.cpp | 107 +++++++++++++++++++++++++++++++ src/llmq/quorums_instantsend.h | 1 + 2 files changed, 108 insertions(+) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 6f90fd5792f3..46d55abe5033 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -892,6 +892,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has } RemoveMempoolConflictsForLock(hash, islock); + ResolveBlockConflicts(hash, islock, true); UpdateWalletTransaction(islock.txid, tx); } @@ -1147,6 +1148,112 @@ void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, con } } +void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const llmq::CInstantSendLock& islock, bool allowInvalidate) +{ + // Lets first collect all non-locked TXs which conflict with the given ISLOCK + std::unordered_map> conflicts; + { + LOCK(cs); + for (auto& in : islock.inputs) { + auto its = nonLockedTxsByInputs.equal_range(in.hash); + for (auto it = its.first; it != its.second; ++it) { + if (it->second.first != in.n) { + continue; + } + auto& conflictTxid = it->second.second; + if (conflictTxid == islock.txid) { + continue; + } + auto jt = nonLockedTxs.find(conflictTxid); + if (jt == nonLockedTxs.end()) { + continue; + } + auto& info = jt->second; + if (!info.pindexMined || !info.tx) { + continue; + } + LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: mined TX %s with input %s and mined in block %s conflicts with islock\n", __func__, + islock.txid.ToString(), islockHash.ToString(), conflictTxid.ToString(), in.ToStringShort(), info.pindexMined->GetBlockHash().ToString()); + conflicts[info.pindexMined].emplace(conflictTxid, info.tx); + } + } + } + + // Lets see if any of the conflicts was already mined into a ChainLocked block + bool hasChainLockedConflict = false; + for (const auto& p : conflicts) { + auto pindex = p.first; + if (chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash())) { + hasChainLockedConflict = true; + break; + } + } + + // If a conflict was mined into a ChainLocked block, then we have no other choice and must prune the ISLOCK and all + // chained ISLOCKs that build on top of this one. The probability of this is practically zero and can only happen + // 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) { + LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: at least one conflicted TX already got a ChainLock. Removing ISLOCK and its chained children.\n", __func__, + islock.txid.ToString(), islockHash.ToString()); + int tipHeight; + { + LOCK(cs_main); + tipHeight = chainActive.Height(); + } + + LOCK(cs); + auto removedIslocks = db.RemoveChainedInstantSendLocks(islockHash, islock.txid, tipHeight); + for (auto& h : removedIslocks) { + LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: removed (child) ISLOCk %s\n", __func__, + islock.txid.ToString(), islockHash.ToString(), h.ToString()); + } + return; + } + + // This method is called from 2 different places. Once when a fresh ISLOCK arrives, in which case it is fine/desired + // to invalidate blocks which contain conflicts. This method might also be called from ConnectBlock, in which case + // we can not call InvalidateBlock/ActivateBestChain due to the cs_main lock requirements. This is fine as when called + // from ConnectBlock, we know that the conflicting block got a ChainLock, so the above code has already handled this. + if (!allowInvalidate) { + return; + } + + bool activateBestChain = false; + for (const auto& p : conflicts) { + auto pindex = p.first; + { + LOCK(cs); + for (auto& p2 : p.second) { + const auto& tx = *p2.second; + RemoveConflictedTx(tx); + } + } + + LogPrintf("CInstantSendManager::%s -- invalidating block %s\n", __func__, pindex->GetBlockHash().ToString()); + + LOCK(cs_main); + CValidationState state; + // need non-const pointer + auto pindex2 = mapBlockIndex.at(pindex->GetBlockHash()); + if (!InvalidateBlock(state, Params(), pindex2)) { + LogPrintf("CInstantSendManager::%s -- InvalidateBlock 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); + } + activateBestChain = true; + } + + if (activateBestChain) { + CValidationState state; + if (!ActivateBestChain(state, Params())) { + LogPrintf("CChainLocksHandler::%s -- ActivateBestChain 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); + } + } +} + void CInstantSendManager::AskNodesForLockedTx(const uint256& txid) { std::vector nodesToAskFor; diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index a8899e0c7d3e..f330fdeaf837 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -151,6 +151,7 @@ class CInstantSendManager : public CRecoveredSigsListener void HandleFullyConfirmedBlock(const CBlockIndex* pindex); void RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock); + void ResolveBlockConflicts(const uint256& islockHash, const CInstantSendLock& islock, bool allowInvalidate); void AskNodesForLockedTx(const uint256& txid); bool ProcessPendingRetryLockTxs(); From c2be224fd7ab7ceff3896d83afbc2a5b8e6e8a81 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 15:51:16 +0200 Subject: [PATCH 08/17] Also call ResolveBlockConflicts from ConnectBlock But only when a block is known to have a conflict and at the same time is ChainLocked, which causes the ISLOCK to be pruned. --- src/validation.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index a01139d8ad07..debf88ed97e5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2219,12 +2219,17 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd } llmq::CInstantSendLockPtr conflictLock = llmq::quorumInstantSendManager->GetConflictingLock(*tx); if (conflictLock) { - // The node which relayed this should switch to correct chain. - // TODO: relay instantsend data/proof. - LOCK(cs_main); - mapRejectedBlocks.insert(std::make_pair(block.GetHash(), GetTime())); - return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), conflictLock->txid.ToString()), - REJECT_INVALID, "conflict-tx-lock"); + if (llmq::chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash())) { + llmq::quorumInstantSendManager->ResolveBlockConflicts(::SerializeHash(*conflictLock), *conflictLock, false); + assert(llmq::quorumInstantSendManager->GetConflictingLock(*tx) == nullptr); + } else { + // The node which relayed this should switch to correct chain. + // TODO: relay instantsend data/proof. + LOCK(cs_main); + mapRejectedBlocks.insert(std::make_pair(block.GetHash(), GetTime())); + return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), conflictLock->txid.ToString()), + REJECT_INVALID, "conflict-tx-lock"); + } } } } else { From ee6b07f33a0061ea4e3ca1f05c341ff94892d9b2 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 19:32:28 +0200 Subject: [PATCH 09/17] Split out RemoveChainLockConflictingLock from ResolveBlockConflicts --- src/llmq/quorums_instantsend.cpp | 45 +++++++++++++++----------------- src/llmq/quorums_instantsend.h | 3 ++- src/validation.cpp | 2 +- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 46d55abe5033..7d24e1c440c8 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -892,7 +892,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has } RemoveMempoolConflictsForLock(hash, islock); - ResolveBlockConflicts(hash, islock, true); + ResolveBlockConflicts(hash, islock); UpdateWalletTransaction(islock.txid, tx); } @@ -1148,7 +1148,7 @@ void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, con } } -void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const llmq::CInstantSendLock& islock, bool allowInvalidate) +void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const llmq::CInstantSendLock& islock) { // Lets first collect all non-locked TXs which conflict with the given ISLOCK std::unordered_map> conflicts; @@ -1194,28 +1194,7 @@ 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) { - LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: at least one conflicted TX already got a ChainLock. Removing ISLOCK and its chained children.\n", __func__, - islock.txid.ToString(), islockHash.ToString()); - int tipHeight; - { - LOCK(cs_main); - tipHeight = chainActive.Height(); - } - - LOCK(cs); - auto removedIslocks = db.RemoveChainedInstantSendLocks(islockHash, islock.txid, tipHeight); - for (auto& h : removedIslocks) { - LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: removed (child) ISLOCk %s\n", __func__, - islock.txid.ToString(), islockHash.ToString(), h.ToString()); - } - return; - } - - // This method is called from 2 different places. Once when a fresh ISLOCK arrives, in which case it is fine/desired - // to invalidate blocks which contain conflicts. This method might also be called from ConnectBlock, in which case - // we can not call InvalidateBlock/ActivateBestChain due to the cs_main lock requirements. This is fine as when called - // from ConnectBlock, we know that the conflicting block got a ChainLock, so the above code has already handled this. - if (!allowInvalidate) { + RemoveChainLockConflictingLock(islockHash, islock); return; } @@ -1254,6 +1233,24 @@ void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const } } +void CInstantSendManager::RemoveChainLockConflictingLock(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__, + islock.txid.ToString(), islockHash.ToString()); + int tipHeight; + { + LOCK(cs_main); + tipHeight = chainActive.Height(); + } + + LOCK(cs); + auto removedIslocks = db.RemoveChainedInstantSendLocks(islockHash, islock.txid, tipHeight); + for (auto& h : removedIslocks) { + LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: removed (child) ISLOCK %s\n", __func__, + islock.txid.ToString(), islockHash.ToString(), h.ToString()); + } +} + void CInstantSendManager::AskNodesForLockedTx(const uint256& txid) { std::vector nodesToAskFor; diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index f330fdeaf837..955b1322355d 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -151,7 +151,8 @@ class CInstantSendManager : public CRecoveredSigsListener void HandleFullyConfirmedBlock(const CBlockIndex* pindex); void RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock); - void ResolveBlockConflicts(const uint256& islockHash, const CInstantSendLock& islock, bool allowInvalidate); + void ResolveBlockConflicts(const uint256& islockHash, const CInstantSendLock& islock); + void RemoveChainLockConflictingLock(const uint256& islockHash, const CInstantSendLock& islock); void AskNodesForLockedTx(const uint256& txid); bool ProcessPendingRetryLockTxs(); diff --git a/src/validation.cpp b/src/validation.cpp index debf88ed97e5..cf419c3aec41 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2220,7 +2220,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd llmq::CInstantSendLockPtr conflictLock = llmq::quorumInstantSendManager->GetConflictingLock(*tx); if (conflictLock) { if (llmq::chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash())) { - llmq::quorumInstantSendManager->ResolveBlockConflicts(::SerializeHash(*conflictLock), *conflictLock, false); + llmq::quorumInstantSendManager->RemoveChainLockConflictingLock(::SerializeHash(*conflictLock), *conflictLock); assert(llmq::quorumInstantSendManager->GetConflictingLock(*tx) == nullptr); } else { // The node which relayed this should switch to correct chain. From 76ac0651288d550e506674f505f3ccaa614b260e Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 7 May 2019 06:35:17 +0200 Subject: [PATCH 10/17] Fix condition for update of nonLockedTxs.pindexMined --- src/llmq/quorums_instantsend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 7d24e1c440c8..ee1fa16249eb 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -968,7 +968,7 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn if (!chainlocked && islockHash.IsNull()) { // TX is not locked, so make sure it is tracked AddNonLockedTx(MakeTransactionRef(tx)); - nonLockedTxs.at(tx.GetHash()).pindexMined = posInBlock == CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK ? pindex : nullptr; + nonLockedTxs.at(tx.GetHash()).pindexMined = posInBlock != CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK ? pindex : nullptr; } else { // TX is locked, so make sure we don't track it anymore RemoveNonLockedTx(tx.GetHash(), true); From 928cfe55bd2720861565790211e6b5e4bca024b5 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Tue, 7 May 2019 06:35:44 +0200 Subject: [PATCH 11/17] Only add entries to nonLockedTxsByInputs when AddNonLockedTx is called for the first time --- src/llmq/quorums_instantsend.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index ee1fa16249eb..fe81cd3246c5 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -978,8 +978,8 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx) { AssertLockHeld(cs); - auto it = nonLockedTxs.emplace(tx->GetHash(), NonLockedTxInfo()).first; - auto& info = it->second; + auto res = nonLockedTxs.emplace(tx->GetHash(), NonLockedTxInfo()); + auto& info = res.first->second; if (!info.tx) { info.tx = tx; @@ -988,8 +988,10 @@ void CInstantSendManager::AddNonLockedTx(const CTransactionRef& tx) } } - for (auto& in : tx->vin) { - nonLockedTxsByInputs.emplace(in.prevout.hash, std::make_pair(in.prevout.n, tx->GetHash())); + if (res.second) { + for (auto& in : tx->vin) { + nonLockedTxsByInputs.emplace(in.prevout.hash, std::make_pair(in.prevout.n, tx->GetHash())); + } } } From 977f712daa62a971bee566c1c5d59a4932f55a7b Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 19:33:18 +0200 Subject: [PATCH 12/17] Implement "quorum getrecsig" RPC --- src/llmq/quorums_signing.cpp | 20 ++++++++++++++++++++ src/llmq/quorums_signing.h | 4 ++++ src/rpc/rpcquorums.cpp | 25 ++++++++++++++++++++++++- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index ea2f4795c361..d5502c4209c0 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -24,6 +24,18 @@ namespace llmq CSigningManager* quorumSigningManager; +UniValue CRecoveredSig::ToJson() const +{ + UniValue ret(UniValue::VOBJ); + ret.push_back(Pair("llmqType", (int)llmqType)); + ret.push_back(Pair("quorumHash", quorumHash.ToString())); + ret.push_back(Pair("id", id.ToString())); + ret.push_back(Pair("msgHash", msgHash.ToString())); + ret.push_back(Pair("sig", sig.GetSig().ToString())); + ret.push_back(Pair("hash", sig.GetSig().GetHash().ToString())); + return ret; +} + CRecoveredSigsDb::CRecoveredSigsDb(CDBWrapper& _db) : db(_db) { @@ -637,6 +649,14 @@ bool CSigningManager::HasRecoveredSigForSession(const uint256& signHash) return db.HasRecoveredSigForSession(signHash); } +bool CSigningManager::GetRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id, llmq::CRecoveredSig& retRecSig) +{ + if (!db.GetRecoveredSigById(llmqType, id, retRecSig)) { + return false; + } + return true; +} + bool CSigningManager::IsConflicting(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) { if (!db.HasRecoveredSigForId(llmqType, id)) { diff --git a/src/llmq/quorums_signing.h b/src/llmq/quorums_signing.h index 31ef38c586e2..fa61dbeadb5c 100644 --- a/src/llmq/quorums_signing.h +++ b/src/llmq/quorums_signing.h @@ -10,6 +10,7 @@ #include "net.h" #include "chainparams.h" #include "saltedhasher.h" +#include "univalue.h" #include "unordered_lru_cache.h" #include @@ -56,6 +57,8 @@ class CRecoveredSig assert(!hash.IsNull()); return hash; } + + UniValue ToJson() const; }; class CRecoveredSigsDb @@ -157,6 +160,7 @@ class CSigningManager bool HasRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); bool HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id); bool HasRecoveredSigForSession(const uint256& signHash); + bool GetRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& retRecSig); bool IsConflicting(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); bool HasVotedOnId(Consensus::LLMQType llmqType, const uint256& id); diff --git a/src/rpc/rpcquorums.cpp b/src/rpc/rpcquorums.cpp index 27f4b6eb70f5..165dfc8feb27 100644 --- a/src/rpc/rpcquorums.cpp +++ b/src/rpc/rpcquorums.cpp @@ -204,6 +204,17 @@ void quorum_hasrecsig_help() ); } +void quorum_getrecsig_help() +{ + throw std::runtime_error( + "quorum getrecsig llmqType \"id\" \"msgHash\"\n" + "\nArguments:\n" + "1. llmqType (int, required) LLMQ type.\n" + "2. \"id\" (string, required) Request id.\n" + "3. \"msgHash\" (string, required) Message hash.\n" + ); +} + void quorum_isconflicting_help() { throw std::runtime_error( @@ -223,6 +234,8 @@ UniValue quorum_sigs_cmd(const JSONRPCRequest& request) quorum_sign_help(); } else if (cmd == "hasrecsig") { quorum_hasrecsig_help(); + } else if (cmd == "getrecsig") { + quorum_getrecsig_help(); } else if (cmd == "isconflicting") { quorum_isconflicting_help(); } else { @@ -243,6 +256,15 @@ UniValue quorum_sigs_cmd(const JSONRPCRequest& request) return llmq::quorumSigningManager->AsyncSignIfMember(llmqType, id, msgHash); } else if (cmd == "hasrecsig") { return llmq::quorumSigningManager->HasRecoveredSig(llmqType, id, msgHash); + } else if (cmd == "getrecsig") { + llmq::CRecoveredSig recSig; + if (!llmq::quorumSigningManager->GetRecoveredSigForId(llmqType, id, recSig)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "recovered signature not found"); + } + if (recSig.msgHash != msgHash) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "recovered signature not found"); + } + return recSig.ToJson(); } else if (cmd == "isconflicting") { return llmq::quorumSigningManager->IsConflicting(llmqType, id, msgHash); } else { @@ -265,6 +287,7 @@ UniValue quorum_sigs_cmd(const JSONRPCRequest& request) " dkgstatus - Return the status of the current DKG process\n" " sign - Threshold-sign a message\n" " hasrecsig - Test if a valid recovered signature is present\n" + " getrecsig - Get a recovered signature\n" " isconflicting - Test if a conflict exists\n" ); } @@ -286,7 +309,7 @@ UniValue quorum(const JSONRPCRequest& request) return quorum_info(request); } else if (command == "dkgstatus") { return quorum_dkgstatus(request); - } else if (command == "sign" || command == "hasrecsig" || command == "isconflicting") { + } else if (command == "sign" || command == "hasrecsig" || command == "getrecsig" || command == "isconflicting") { return quorum_sigs_cmd(request); } else { quorum_help(); From aa0f6bdcfa6a42af4785a922b5c3bc1be6e213d4 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 19:33:45 +0200 Subject: [PATCH 13/17] Include decoded TX data in result of create_raw_tx --- qa/rpc-tests/test_framework/test_framework.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/test_framework/test_framework.py b/qa/rpc-tests/test_framework/test_framework.py index d507f4c0aa79..63ee91c4f09e 100755 --- a/qa/rpc-tests/test_framework/test_framework.py +++ b/qa/rpc-tests/test_framework/test_framework.py @@ -515,7 +515,10 @@ def create_raw_tx(self, node_from, node_to, amount, min_inputs, max_inputs): outputs[receiver_address] = satoshi_round(amount) outputs[change_address] = satoshi_round(in_amount - amount - fee) rawtx = node_from.createrawtransaction(inputs, outputs) - return node_from.signrawtransaction(rawtx) + ret = node_from.signrawtransaction(rawtx) + decoded = node_from.decoderawtransaction(ret['hex']) + ret = {**decoded, **ret} + return ret # sends regular instantsend with high fee def send_regular_instantsend(self, sender, receiver, check_fee = True): From 70cf0d94b9909f7c4677451f3eddba1065718a97 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 19:34:05 +0200 Subject: [PATCH 14/17] Implement support for CLSIG in mininode.py --- qa/rpc-tests/test_framework/mininode.py | 28 ++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index 489e2b8cc4a7..b310471cc59d 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -1547,6 +1547,30 @@ def __repr__(self): return "msg_mnlistdiff(baseBlockHash=%064x, blockHash=%064x)" % (self.baseBlockHash, self.blockHash) +class msg_clsig(object): + command = b"clsig" + + def __init__(self, height=0, blockHash=0, sig=b'\\x0' * 96): + self.height = height + self.blockHash = blockHash + self.sig = sig + + def deserialize(self, f): + self.height = struct.unpack(' Date: Tue, 7 May 2019 06:34:39 +0200 Subject: [PATCH 15/17] Implement support for ISLOCK in mininode.py --- qa/rpc-tests/test_framework/mininode.py | 28 ++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index b310471cc59d..3ebf11540075 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -1571,6 +1571,30 @@ def __repr__(self): return "msg_clsig(height=%d, blockHash=%064x)" % (self.height, self.blockHash) +class msg_islock(object): + command = b"islock" + + def __init__(self, inputs=[], txid=0, sig=b'\\x0' * 96): + self.inputs = inputs + self.txid = txid + self.sig = sig + + def deserialize(self, f): + self.inputs = deser_vector(f, COutPoint) + self.txid = deser_uint256(f) + self.sig = f.read(96) + + def serialize(self): + r = b"" + r += ser_vector(self.inputs) + r += ser_uint256(self.txid) + r += self.sig + return r + + def __repr__(self): + return "msg_islock(inputs=%s, txid=%064x)" % (repr(self.inputs), self.txid) + + # This is what a callback should look like for NodeConn # Reimplement the on_* functions to provide handling for events class NodeConnCB(object): @@ -1656,6 +1680,7 @@ def on_getblocktxn(self, conn, message): pass def on_blocktxn(self, conn, message): pass def on_mnlistdiff(self, conn, message): pass def on_clsig(self, conn, message): pass + def on_islock(self, conn, message): pass # More useful callbacks and functions for NodeConnCB's which have a single NodeConn class SingleNodeConnCB(NodeConnCB): @@ -1714,7 +1739,8 @@ class NodeConn(asyncore.dispatcher): b"getblocktxn": msg_getblocktxn, b"blocktxn": msg_blocktxn, b"mnlistdiff": msg_mnlistdiff, - b"clsig": msg_clsig + b"clsig": msg_clsig, + b"islock": msg_islock } MAGIC_BYTES = { "mainnet": b"\xbf\x0c\x6b\xbd", # mainnet From 0acd8c7d43dac7a38f028270a650aa54fb741e07 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 19:34:45 +0200 Subject: [PATCH 16/17] Implement tests for ChainLock vs InstantSend lock conflict resolution --- qa/pull-tester/rpc-tests.py | 1 + qa/rpc-tests/llmq-is-cl-conflicts.py | 338 +++++++++++++++++++++++++++ 2 files changed, 339 insertions(+) create mode 100755 qa/rpc-tests/llmq-is-cl-conflicts.py diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 031fbdbeb78b..c769e1e52e88 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -47,6 +47,7 @@ 'llmq-signing.py', # NOTE: needs dash_hash to pass 'llmq-chainlocks.py', # NOTE: needs dash_hash to pass 'llmq-simplepose.py', # NOTE: needs dash_hash to pass + 'llmq-is-cl-conflicts.py', # NOTE: needs dash_hash to pass 'dip4-coinbasemerkleroots.py', # NOTE: needs dash_hash to pass # vv Tests less than 60s vv 'sendheaders.py', # NOTE: needs dash_hash to pass diff --git a/qa/rpc-tests/llmq-is-cl-conflicts.py b/qa/rpc-tests/llmq-is-cl-conflicts.py new file mode 100755 index 000000000000..4351728e856e --- /dev/null +++ b/qa/rpc-tests/llmq-is-cl-conflicts.py @@ -0,0 +1,338 @@ +#!/usr/bin/env python3 +# Copyright (c) 2015-2018 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +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 * +from time import * + +''' +llmq-is-cl-conflicts.py + +Checks conflict handling between ChainLocks and InstantSend + +''' + +class TestNode(SingleNodeConnCB): + def __init__(self): + SingleNodeConnCB.__init__(self) + self.clsigs = {} + self.islocks = {} + + def send_clsig(self, clsig): + hash = uint256_from_str(hash256(clsig.serialize())) + self.clsigs[hash] = clsig + + inv = msg_inv([CInv(29, hash)]) + self.send_message(inv) + + def send_islock(self, islock): + hash = uint256_from_str(hash256(islock.serialize())) + self.islocks[hash] = islock + + inv = msg_inv([CInv(30, hash)]) + self.send_message(inv) + + def on_getdata(self, conn, message): + for inv in message.inv: + if inv.hash in self.clsigs: + self.send_message(self.clsigs[inv.hash]) + if inv.hash in self.islocks: + self.send_message(self.islocks[inv.hash]) + + +class LLMQ_IS_CL_Conflicts(DashTestFramework): + def __init__(self): + super().__init__(6, 5, [], fast_dip3_enforcement=True) + #disable_mocktime() + + def run_test(self): + + while self.nodes[0].getblockchaininfo()["bip9_softforks"]["dip0008"]["status"] != "active": + self.nodes[0].generate(10) + sync_blocks(self.nodes, timeout=60*5) + + self.test_node = TestNode() + self.test_node.add_connection(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], self.test_node)) + NetworkThread().start() # Start up network handling in another thread + self.test_node.wait_for_verack() + + self.nodes[0].spork("SPORK_17_QUORUM_DKG_ENABLED", 0) + self.nodes[0].spork("SPORK_19_CHAINLOCKS_ENABLED", 0) + self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0) + self.wait_for_sporks_same() + + self.mine_quorum() + + # mine single block, wait for chainlock + self.nodes[0].generate(1) + self.wait_for_chainlock_tip_all_nodes() + + self.test_chainlock_overrides_islock(False) + self.test_chainlock_overrides_islock(True) + self.test_islock_overrides_nonchainlock() + + def test_chainlock_overrides_islock(self, test_block_conflict): + # 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'] + rawtx3 = self.create_raw_tx(self.nodes[0], self.nodes[0], 1, 1, 100)['hex'] + rawtx1_obj = FromHex(CTransaction(), rawtx1) + rawtx2_obj = FromHex(CTransaction(), rawtx2) + rawtx3_obj = FromHex(CTransaction(), rawtx3) + + rawtx1_txid = self.nodes[0].sendrawtransaction(rawtx1) + rawtx2_txid = encode(hash256(hex_str_to_bytes(rawtx2))[::-1], 'hex_codec').decode('ascii') + rawtx3_txid = encode(hash256(hex_str_to_bytes(rawtx3))[::-1], 'hex_codec').decode('ascii') + + # Create a chained TX on top of tx1 + inputs = [] + n = 0 + for out in rawtx1_obj.vout: + if out.nValue == 100000000: + inputs.append({"txid": rawtx1_txid, "vout": n}) + n += 1 + rawtx4 = self.nodes[0].createrawtransaction(inputs, {self.nodes[0].getnewaddress(): 0.999}) + rawtx4 = self.nodes[0].signrawtransaction(rawtx4)['hex'] + rawtx4_txid = self.nodes[0].sendrawtransaction(rawtx4) + + for node in self.nodes: + self.wait_for_instantlock(rawtx1_txid, node) + self.wait_for_instantlock(rawtx4_txid, node) + + block = self.create_block(self.nodes[0], [rawtx2_obj]) + if test_block_conflict: + submit_result = self.nodes[0].submitblock(ToHex(block)) + assert(submit_result == "conflict-tx-lock") + + cl = self.create_chainlock(self.nodes[0].getblockcount() + 1, block.sha256) + self.test_node.send_clsig(cl) + + # Give the CLSIG some time to propagate. We unfortunately can't check propagation here as "getblock/getblockheader" + # is required to check for CLSIGs, but this requires the block header to be propagated already + sleep(1) + + # The block should get accepted now, and at the same time prune the conflicting ISLOCKs + submit_result = self.nodes[1].submitblock(ToHex(block)) + if test_block_conflict: + assert(submit_result == "duplicate") + else: + assert(submit_result is None) + + for node in self.nodes: + self.wait_for_chainlock(node, "%064x" % block.sha256) + + # Create a chained TX on top of tx2 + inputs = [] + n = 0 + for out in rawtx2_obj.vout: + if out.nValue == 100000000: + inputs.append({"txid": rawtx2_txid, "vout": n}) + n += 1 + rawtx5 = self.nodes[0].createrawtransaction(inputs, {self.nodes[0].getnewaddress(): 0.999}) + rawtx5 = self.nodes[0].signrawtransaction(rawtx5)['hex'] + rawtx5_txid = self.nodes[0].sendrawtransaction(rawtx5) + 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_jsonrpc(-5, "No such mempool or blockchain transaction", node.getrawtransaction, rawtx1_txid, True) + assert_raises_jsonrpc(-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 + 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'] + + rawtx1_txid = encode(hash256(hex_str_to_bytes(rawtx1))[::-1], 'hex_codec').decode('ascii') + rawtx2_txid = encode(hash256(hex_str_to_bytes(rawtx2))[::-1], 'hex_codec').decode('ascii') + + # Create an ISLOCK but don't broadcast it yet + islock = self.create_islock(rawtx2) + + # Stop enough MNs so that ChainLocks don't work anymore + for i in range(3): + self.stop_node(len(self.nodes) - 1) + self.nodes.pop(len(self.nodes) - 1) + self.mninfo.pop(len(self.mninfo) - 1) + + # Send tx1, which will later conflict with the ISLOCK + self.nodes[0].sendrawtransaction(rawtx1) + + # fast forward 11 minutes, so that the TX is considered safe and included in the next block + set_mocktime(get_mocktime() + int(60 * 11)) + set_node_times(self.nodes, get_mocktime()) + + # Mine the conflicting TX into a block + good_tip = self.nodes[0].getbestblockhash() + self.nodes[0].generate(2) + self.sync_all() + + # Assert that the conflicting tx got mined and the locked TX is not valid + assert(self.nodes[0].getrawtransaction(rawtx1_txid, True)['confirmations'] > 0) + assert_raises_jsonrpc(-25, "Missing inputs", self.nodes[0].sendrawtransaction, rawtx2) + + # Send the ISLOCK, which should result in the last 2 blocks to be invalidated, even though the nodes don't know + # the locked transaction yet + self.test_node.send_islock(islock) + sleep(5) + + assert(self.nodes[0].getbestblockhash() == good_tip) + assert(self.nodes[1].getbestblockhash() == good_tip) + + # Send the actual transaction and mine it + self.nodes[0].sendrawtransaction(rawtx2) + self.nodes[0].generate(1) + self.sync_all() + + assert(self.nodes[0].getrawtransaction(rawtx2_txid, True)['confirmations'] > 0) + assert(self.nodes[1].getrawtransaction(rawtx2_txid, True)['confirmations'] > 0) + assert(self.nodes[0].getrawtransaction(rawtx2_txid, True)['instantlock']) + assert(self.nodes[1].getrawtransaction(rawtx2_txid, True)['instantlock']) + assert(self.nodes[0].getbestblockhash() != good_tip) + assert(self.nodes[1].getbestblockhash() != good_tip) + + def wait_for_chainlock_tip_all_nodes(self): + for node in self.nodes: + tip = node.getbestblockhash() + self.wait_for_chainlock(node, tip) + + def wait_for_chainlock_tip(self, node): + tip = node.getbestblockhash() + self.wait_for_chainlock(node, tip) + + def wait_for_chainlock(self, node, block_hash): + t = time() + while time() - t < 15: + try: + block = node.getblockheader(block_hash) + if block["confirmations"] > 0 and block["chainlock"]: + return + except: + # block might not be on the node yet + pass + sleep(0.1) + raise AssertionError("wait_for_chainlock timed out") + + def create_block(self, node, vtx=[]): + bt = node.getblocktemplate() + height = bt['height'] + tip_hash = bt['previousblockhash'] + + coinbasevalue = bt['coinbasevalue'] + miner_address = node.getnewaddress() + mn_payee = bt['masternode'][0]['payee'] + + # calculate fees that the block template included (we'll have to remove it from the coinbase as we won't + # include the template's transactions + bt_fees = 0 + for tx in bt['transactions']: + bt_fees += tx['fee'] + + new_fees = 0 + for tx in vtx: + in_value = 0 + out_value = 0 + for txin in tx.vin: + txout = node.gettxout("%064x" % txin.prevout.hash, txin.prevout.n, False) + in_value += int(txout['value'] * COIN) + for txout in tx.vout: + out_value += txout.nValue + new_fees += in_value - out_value + + # fix fees + coinbasevalue -= bt_fees + coinbasevalue += new_fees + + mn_amount = get_masternode_payment(height, coinbasevalue) + miner_amount = coinbasevalue - mn_amount + + outputs = {miner_address: str(Decimal(miner_amount) / COIN)} + if mn_amount > 0: + outputs[mn_payee] = str(Decimal(mn_amount) / COIN) + + coinbase = FromHex(CTransaction(), node.createrawtransaction([], outputs)) + coinbase.vin = create_coinbase(height).vin + + # We can't really use this one as it would result in invalid merkle roots for masternode lists + if len(bt['coinbase_payload']) != 0: + cbtx = FromHex(CCbTx(version=1), bt['coinbase_payload']) + coinbase.nVersion = 3 + coinbase.nType = 5 # CbTx + coinbase.vExtraPayload = cbtx.serialize() + + coinbase.calc_sha256() + + block = create_block(int(tip_hash, 16), coinbase, nTime=bt['curtime']) + block.vtx += vtx + + # Add quorum commitments from template + for tx in bt['transactions']: + tx2 = FromHex(CTransaction(), tx['data']) + if tx2.nType == 6: + block.vtx.append(tx2) + + block.hashMerkleRoot = block.calc_merkle_root() + block.solve() + return block + + def create_chainlock(self, height, blockHash): + request_id = "%064x" % uint256_from_str(hash256(ser_string(b"clsig") + struct.pack(" Date: Tue, 7 May 2019 06:40:39 +0200 Subject: [PATCH 17/17] Handle review comment Bail out (continue) early --- src/validation.cpp | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index cf419c3aec41..822eb4c8ce9c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2218,18 +2218,19 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd } } llmq::CInstantSendLockPtr conflictLock = llmq::quorumInstantSendManager->GetConflictingLock(*tx); - if (conflictLock) { - if (llmq::chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash())) { - llmq::quorumInstantSendManager->RemoveChainLockConflictingLock(::SerializeHash(*conflictLock), *conflictLock); - assert(llmq::quorumInstantSendManager->GetConflictingLock(*tx) == nullptr); - } else { - // The node which relayed this should switch to correct chain. - // TODO: relay instantsend data/proof. - LOCK(cs_main); - mapRejectedBlocks.insert(std::make_pair(block.GetHash(), GetTime())); - return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), conflictLock->txid.ToString()), - REJECT_INVALID, "conflict-tx-lock"); - } + if (!conflictLock) { + continue; + } + if (llmq::chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash())) { + llmq::quorumInstantSendManager->RemoveChainLockConflictingLock(::SerializeHash(*conflictLock), *conflictLock); + assert(llmq::quorumInstantSendManager->GetConflictingLock(*tx) == nullptr); + } else { + // The node which relayed this should switch to correct chain. + // TODO: relay instantsend data/proof. + LOCK(cs_main); + mapRejectedBlocks.insert(std::make_pair(block.GetHash(), GetTime())); + return state.DoS(10, error("ConnectBlock(DASH): transaction %s conflicts with transaction lock %s", tx->GetHash().ToString(), conflictLock->txid.ToString()), + REJECT_INVALID, "conflict-tx-lock"); } } } else {