diff --git a/qa/rpc-tests/autois-mempool.py b/qa/rpc-tests/autois-mempool.py index 7a63cd0ec8480..71b23727defdd 100755 --- a/qa/rpc-tests/autois-mempool.py +++ b/qa/rpc-tests/autois-mempool.py @@ -56,6 +56,10 @@ def run_test(self): self.log.info("Test old InstantSend") self.test_auto(); + # Generate 6 block to avoid retroactive signing overloading Travis + self.nodes[0].generate(6) + sync_blocks(self.nodes) + self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0) self.wait_for_sporks_same() @@ -100,9 +104,17 @@ def test_auto(self, new_is = False): assert(not self.send_regular_instantsend(sender, receiver, False) if new_is else self.send_regular_instantsend(sender, receiver)) # generate one block to clean up mempool and retry auto and regular IS - # generate 2 more blocks to have enough confirmations for IS - self.nodes[0].generate(3) + # generate 5 more blocks to avoid retroactive signing (which would overload Travis) + set_mocktime(get_mocktime() + 1) + set_node_times(self.nodes, get_mocktime()) + self.nodes[0].spork("SPORK_2_INSTANTSEND_ENABLED", 4070908800) + self.wait_for_sporks_same() + self.nodes[0].generate(6) self.sync_all() + set_mocktime(get_mocktime() + 1) + set_node_times(self.nodes, get_mocktime()) + self.nodes[0].spork("SPORK_2_INSTANTSEND_ENABLED", 0) + self.wait_for_sporks_same() assert(self.send_simple_tx(sender, receiver)) assert(self.send_regular_instantsend(sender, receiver, not new_is)) diff --git a/qa/rpc-tests/llmq-chainlocks.py b/qa/rpc-tests/llmq-chainlocks.py index 6cbcc6cdc108a..26ebe6fd3c2b1 100755 --- a/qa/rpc-tests/llmq-chainlocks.py +++ b/qa/rpc-tests/llmq-chainlocks.py @@ -88,6 +88,38 @@ def run_test(self): self.wait_for_chainlock(self.nodes[0], self.nodes[1].getbestblockhash()) assert(self.nodes[0].getbestblockhash() == self.nodes[1].getbestblockhash()) + # Enable LLMQ bases InstantSend, which also enables checks for "safe" transactions + self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0) + self.wait_for_sporks_same() + + # Isolate a node and let it create some transactions which won't get IS locked + self.nodes[0].setnetworkactive(False) + txs = [] + for i in range(3): + txs.append(self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)) + txs += self.create_chained_txs(self.nodes[0], 1) + # Assert that after block generation these TXs are NOT included (as they are "unsafe") + self.nodes[0].generate(1) + for txid in txs: + tx = self.nodes[0].getrawtransaction(txid, 1) + assert("confirmations" not in tx) + sleep(1) + assert(not self.nodes[0].getblock(self.nodes[0].getbestblockhash())["chainlock"]) + # Disable LLMQ based InstantSend for a very short time (this never gets propagated to other nodes) + self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 4070908800) + # Now the TXs should be included + self.nodes[0].generate(1) + self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0) + # Assert that TXs got included now + for txid in txs: + tx = self.nodes[0].getrawtransaction(txid, 1) + assert("confirmations" in tx and tx["confirmations"] > 0) + # Enable network on first node again, which will cause the blocks to propagate and IS locks to happen retroactively + # for the mined TXs, which will then allow the network to create a CLSIG + self.nodes[0].setnetworkactive(True) + connect_nodes(self.nodes[0], 1) + self.wait_for_chainlock(self.nodes[0], self.nodes[1].getbestblockhash()) + def wait_for_chainlock_tip_all_nodes(self): for node in self.nodes: tip = node.getbestblockhash() @@ -110,6 +142,24 @@ def wait_for_chainlock(self, node, block_hash): sleep(0.1) raise AssertionError("wait_for_chainlock timed out") + def create_chained_txs(self, node, amount): + txid = node.sendtoaddress(node.getnewaddress(), amount) + tx = node.getrawtransaction(txid, 1) + inputs = [] + valueIn = 0 + for txout in tx["vout"]: + inputs.append({"txid": txid, "vout": txout["n"]}) + valueIn += txout["value"] + outputs = { + node.getnewaddress(): round(float(valueIn) - 0.0001, 6) + } + + rawtx = node.createrawtransaction(inputs, outputs) + rawtx = node.signrawtransaction(rawtx) + rawtxid = node.sendrawtransaction(rawtx["hex"]) + + return [txid, rawtxid] + if __name__ == '__main__': LLMQChainLocksTest().main() diff --git a/qa/rpc-tests/p2p-autoinstantsend.py b/qa/rpc-tests/p2p-autoinstantsend.py index 4af1e7640f9da..707265a1c2624 100755 --- a/qa/rpc-tests/p2p-autoinstantsend.py +++ b/qa/rpc-tests/p2p-autoinstantsend.py @@ -39,6 +39,10 @@ def run_test(self): self.log.info("Test old InstantSend") self.test_auto(); + # Generate 6 block to avoid retroactive signing overloading Travis + self.nodes[0].generate(6) + sync_blocks(self.nodes) + self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0) self.wait_for_sporks_same() diff --git a/qa/rpc-tests/p2p-instantsend.py b/qa/rpc-tests/p2p-instantsend.py index 39a75cc40cce3..5be59689410b0 100755 --- a/qa/rpc-tests/p2p-instantsend.py +++ b/qa/rpc-tests/p2p-instantsend.py @@ -28,6 +28,10 @@ def run_test(self): self.log.info("Test old InstantSend") self.test_doublespend() + # Generate 6 block to avoid retroactive signing overloading Travis + self.nodes[0].generate(6) + sync_blocks(self.nodes) + self.nodes[0].spork("SPORK_20_INSTANTSEND_LLMQ_BASED", 0) self.wait_for_sporks_same() diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index 418f1f9e2879d..c758191f5f64d 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -71,11 +71,6 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con llmq::quorumDKGSessionManager->UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); } -void CDSNotificationInterface::NewPoWValidBlock(const CBlockIndex* pindex, const std::shared_ptr& block) -{ - llmq::chainLocksHandler->NewPoWValidBlock(pindex, block); -} - void CDSNotificationInterface::SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) { llmq::quorumInstantSendManager->SyncTransaction(tx, pindex, posInBlock); diff --git a/src/dsnotificationinterface.h b/src/dsnotificationinterface.h index c345551153e38..1dc83f7d085e2 100644 --- a/src/dsnotificationinterface.h +++ b/src/dsnotificationinterface.h @@ -21,7 +21,6 @@ class CDSNotificationInterface : public CValidationInterface void AcceptedBlockHeader(const CBlockIndex *pindexNew) override; void NotifyHeaderTip(const CBlockIndex *pindexNew, bool fInitialDownload) override; void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; - void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& block) override; void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) override; void NotifyMasternodeListChanged(const CDeterministicMNList& newList) override; void NotifyChainLock(const CBlockIndex* pindex) override; diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index f9a73c539108d..40bf14004b943 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -316,41 +316,34 @@ void CChainLocksHandler::TrySignChainTip() quorumSigningManager->AsyncSignIfMember(Params().GetConsensus().llmqChainLocks, requestId, msgHash); } -void CChainLocksHandler::NewPoWValidBlock(const CBlockIndex* pindex, const std::shared_ptr& block) +void CChainLocksHandler::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int posInBlock) { - LOCK(cs); - if (blockTxs.count(pindex->GetBlockHash())) { - // should actually not happen (blocks are only written once to disk and this is when NewPoWValidBlock is called) - // but be extra safe here in case this behaviour changes. - return; + bool handleTx = true; + if (tx.IsCoinBase() || tx.vin.empty()) { + handleTx = false; } - int64_t curTime = GetAdjustedTime(); + LOCK(cs); - // We listen for NewPoWValidBlock so that we can collect all TX ids of all included TXs of newly received blocks + if (handleTx) { + int64_t curTime = GetAdjustedTime(); + txFirstSeenTime.emplace(tx.GetHash(), curTime); + } + + // We listen for SyncTransaction so that we can collect all TX ids of all included TXs of newly received blocks // We need this information later when we try to sign a new tip, so that we can determine if all included TXs are // safe. - - auto txs = std::make_shared>(); - for (const auto& tx : block->vtx) { - if (tx->IsCoinBase() || tx->vin.empty()) { - continue; + if (pindex && posInBlock != CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK) { + auto it = blockTxs.find(pindex->GetBlockHash()); + if (it == blockTxs.end()) { + // we want this to be run even if handleTx == false, so that the coinbase TX triggers creation of an empty entry + it = blockTxs.emplace(pindex->GetBlockHash(), std::make_shared>()).first; + } + if (handleTx) { + auto& txs = *it->second; + txs.emplace(tx.GetHash()); } - txs->emplace(tx->GetHash()); - txFirstSeenTime.emplace(tx->GetHash(), curTime); - } - blockTxs[pindex->GetBlockHash()] = txs; -} - -void CChainLocksHandler::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int posInBlock) -{ - if (tx.IsCoinBase() || tx.vin.empty()) { - return; } - - LOCK(cs); - int64_t curTime = GetAdjustedTime(); - txFirstSeenTime.emplace(tx.GetHash(), curTime); } bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid) diff --git a/src/llmq/quorums_chainlocks.h b/src/llmq/quorums_chainlocks.h index 660a983a71186..6d0714bbc7b83 100644 --- a/src/llmq/quorums_chainlocks.h +++ b/src/llmq/quorums_chainlocks.h @@ -87,7 +87,6 @@ class CChainLocksHandler : public CRecoveredSigsListener void ProcessNewChainLock(NodeId from, const CChainLockSig& clsig, const uint256& hash); void AcceptedBlockHeader(const CBlockIndex* pindexNew); void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork); - void NewPoWValidBlock(const CBlockIndex* pindex, const std::shared_ptr& block); void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock); void TrySignChainTip(); void EnforceBestChainLock(); diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 7db72015c5ab2..2078f9f4d0ce9 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -294,20 +294,23 @@ bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebu return false; } - Coin coin; - const CBlockIndex* pindexMined = nullptr; + CTransactionRef tx; + uint256 hashBlock; + // this relies on enabled txindex and won't work if we ever try to remove the requirement for txindex for masternodes + if (!GetTransaction(outpoint.hash, tx, params, hashBlock, false)) { + if (printDebug) { + LogPrint("instantsend", "CInstantSendManager::%s -- txid=%s: failed to find parent TX %s\n", __func__, + txHash.ToString(), outpoint.hash.ToString()); + } + return false; + } + + const CBlockIndex* pindexMined; int nTxAge; { LOCK(cs_main); - if (!pcoinsTip->GetCoin(outpoint, coin) || coin.IsSpent()) { - if (printDebug) { - LogPrint("instantsend", "CInstantSendManager::%s -- txid=%s: failed to find UTXO %s\n", __func__, - txHash.ToString(), outpoint.ToStringShort()); - } - return false; - } - pindexMined = chainActive[coin.nHeight]; - nTxAge = chainActive.Height() - coin.nHeight + 1; + pindexMined = mapBlockIndex.at(hashBlock); + nTxAge = chainActive.Height() - pindexMined->nHeight + 1; } if (nTxAge < nInstantSendConfirmationsRequired) { @@ -321,7 +324,7 @@ bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebu } if (retValue) { - *retValue = coin.out.nValue; + *retValue = tx->vout[outpoint.n].nValue; } return true; @@ -673,7 +676,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has g_connman->RelayInv(inv); RemoveMempoolConflictsForLock(hash, islock); - RetryLockMempoolTxs(islock.txid); + RetryLockTxs(islock.txid); UpdateWalletTransaction(islock.txid, tx); } @@ -708,8 +711,17 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn return; } - if (IsLocked(tx.GetHash())) { - RetryLockMempoolTxs(tx.GetHash()); + if (tx.IsCoinBase() || tx.vin.empty()) { + // coinbase can't and TXs with no inputs be locked + return; + } + + bool locked = IsLocked(tx.GetHash()); + bool chainlocked = pindex && chainLocksHandler->HasChainLock(pindex->nHeight, pindex->GetBlockHash()); + if (locked || chainlocked) { + RetryLockTxs(tx.GetHash()); + } else { + ProcessTx(tx, Params().GetConsensus()); } } @@ -756,7 +768,7 @@ void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindexChainLock) db.WriteLastChainLockBlock(pindexChainLock->GetBlockHash()); } - RetryLockMempoolTxs(uint256()); + RetryLockTxs(uint256()); } void CInstantSendManager::RemoveFinalISLock(const uint256& hash, const CInstantSendLockPtr& islock) @@ -795,9 +807,9 @@ void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, con } } -void CInstantSendManager::RetryLockMempoolTxs(const uint256& lockedParentTx) +void CInstantSendManager::RetryLockTxs(const uint256& lockedParentTx) { - // Let's retry all mempool TXs which don't have an islock yet and where the parents got ChainLocked now + // Let's retry all unlocked TXs from mempool and and recently connected blocks std::unordered_map txs; @@ -817,6 +829,57 @@ void CInstantSendManager::RetryLockMempoolTxs(const uint256& lockedParentTx) } } } + + uint256 lastChainLockBlock; + const CBlockIndex* pindexLastChainLockBlock = nullptr; + const CBlockIndex* pindexWalk = nullptr; + { + LOCK(cs); + lastChainLockBlock = db.GetLastChainLockBlock(); + } + { + LOCK(cs_main); + if (!lastChainLockBlock.IsNull()) { + pindexLastChainLockBlock = mapBlockIndex.at(lastChainLockBlock); + pindexWalk = chainActive.Tip(); + } + } + + // scan blocks until we hit the last chainlocked block we know of. Also stop scanning after a depth of 6 to avoid + // signing thousands of TXs at once. Also, after a depth of 6, blocks get eligible for ChainLocking even if unsafe + // TXs are included, so there is no need to retroactively sign these. + int depth = 0; + while (pindexWalk && pindexWalk != pindexLastChainLockBlock && depth < 6) { + CBlock block; + { + LOCK(cs_main); + if (!ReadBlockFromDisk(block, pindexWalk, Params().GetConsensus())) { + pindexWalk = pindexWalk->pprev; + continue; + } + } + + for (const auto& tx : block.vtx) { + if (lockedParentTx.IsNull()) { + txs.emplace(tx->GetHash(), tx); + } else { + bool isChild = false; + for (auto& in : tx->vin) { + if (in.prevout.hash == lockedParentTx) { + isChild = true; + break; + } + } + if (isChild) { + txs.emplace(tx->GetHash(), tx); + } + } + } + + pindexWalk = pindexWalk->pprev; + depth++; + } + for (auto& p : txs) { auto& tx = p.second; { diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index bd689d641213d..da9e61b65cfa6 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -124,7 +124,7 @@ class CInstantSendManager : public CRecoveredSigsListener void RemoveFinalISLock(const uint256& hash, const CInstantSendLockPtr& islock); void RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock); - void RetryLockMempoolTxs(const uint256& lockedParentTx); + void RetryLockTxs(const uint256& lockedParentTx); bool AlreadyHave(const CInv& inv); bool GetInstantSendLockByHash(const uint256& hash, CInstantSendLock& ret); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 957f51e9c17b0..70b5e45593aa3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2164,10 +2164,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr instantsend.Vote(tx.GetHash(), connman); } - if (nInvType != MSG_TXLOCK_REQUEST) { - llmq::quorumInstantSendManager->ProcessTx(tx, chainparams.GetConsensus()); - } - mempool.check(pcoinsTip); connman.RelayTransaction(tx); for (unsigned int i = 0; i < tx.vout.size(); i++) { @@ -2212,8 +2208,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr vWorkQueue.emplace_back(orphanHash, i); } vEraseQueue.push_back(orphanHash); - - llmq::quorumInstantSendManager->ProcessTx(orphanTx, chainparams.GetConsensus()); } else if (!fMissingInputs2) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 8e598553756c6..8ece8720fc62a 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1020,7 +1020,6 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) throw JSONRPCError(RPC_TRANSACTION_ERROR, state.GetRejectReason()); } } - llmq::quorumInstantSendManager->ProcessTx(*tx, Params().GetConsensus()); } else if (fHaveChain) { throw JSONRPCError(RPC_TRANSACTION_ALREADY_IN_CHAIN, "transaction already in block chain"); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c2b63bb16eb81..b668a60f1f5c9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1921,8 +1921,6 @@ bool CWalletTx::RelayWalletTransaction(CConnman* connman, const std::string& str } } - llmq::quorumInstantSendManager->ProcessTx(*this->tx, Params().GetConsensus()); - if (connman) { connman->RelayTransaction((CTransaction)*this); return true;