From 7fdc66dd865a647ff095678ecd63d06d9e69259c Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 6 May 2019 15:26:27 +0200 Subject: [PATCH] Ask for locked TXs after removing conflicting TXs (#2898) * Also test conflicts in mempool instead of only in blocks * Ask for locked TXs after removing conflicting TXs When we removed a conflicting TX from the mempool, the correct/locked TX is not available locally as the first-seen rule would have filtered before. We need to re-request this TX if any other node announced it before. * Apply suggestions from code review Co-Authored-By: codablock --- qa/rpc-tests/p2p-instantsend.py | 50 +++++++++++++++++++++++------ src/llmq/quorums_instantsend.cpp | 55 +++++++++++++++++++++++++------- src/llmq/quorums_instantsend.h | 1 + 3 files changed, 85 insertions(+), 21 deletions(-) diff --git a/qa/rpc-tests/p2p-instantsend.py b/qa/rpc-tests/p2p-instantsend.py index 5be59689410b0..193e0729fae6a 100755 --- a/qa/rpc-tests/p2p-instantsend.py +++ b/qa/rpc-tests/p2p-instantsend.py @@ -26,7 +26,7 @@ def run_test(self): self.mine_quorum() self.log.info("Test old InstantSend") - self.test_doublespend() + self.test_block_doublespend() # Generate 6 block to avoid retroactive signing overloading Travis self.nodes[0].generate(6) @@ -36,23 +36,21 @@ def run_test(self): self.wait_for_sporks_same() self.log.info("Test new InstantSend") - self.test_doublespend() + self.test_mempool_doublespend() + self.test_block_doublespend() - def test_doublespend(self, new_is = False): + def test_block_doublespend(self): sender = self.nodes[self.sender_idx] receiver = self.nodes[self.receiver_idx] isolated = self.nodes[self.isolated_idx] + # feed the sender with some balance sender_addr = sender.getnewaddress() self.nodes[0].sendtoaddress(sender_addr, 1) - # make sender funds mature for InstantSend - for i in range(0, 2): - set_mocktime(get_mocktime() + 1) - set_node_times(self.nodes, get_mocktime()) - self.nodes[0].generate(1) + self.nodes[0].generate(2) self.sync_all() - # create doublepending transaction, but don't relay it + # create doublespending transaction, but don't relay it dblspnd_tx = self.create_raw_tx(sender, isolated, 0.5, 1, 100) # stop one node to isolate it from network isolated.setnetworkactive(False) @@ -89,5 +87,39 @@ def test_doublespend(self, new_is = False): self.nodes[0].generate(2) self.sync_all() + def test_mempool_doublespend(self): + sender = self.nodes[self.sender_idx] + receiver = self.nodes[self.receiver_idx] + isolated = self.nodes[self.isolated_idx] + + # feed the sender with some balance + sender_addr = sender.getnewaddress() + self.nodes[0].sendtoaddress(sender_addr, 1) + self.nodes[0].generate(2) + self.sync_all() + + # create doublespending transaction, but don't relay it + dblspnd_tx = self.create_raw_tx(sender, isolated, 0.5, 1, 100) + dblspnd_txid = bytes_to_hex_str(hash256(hex_str_to_bytes(dblspnd_tx['hex']))[::-1]) + # isolate one node from network + isolated.setnetworkactive(False) + # send doublespend transaction to isolated node + isolated.sendrawtransaction(dblspnd_tx['hex']) + # let isolated node rejoin the network + # The previously isolated node should NOT relay the doublespending TX + isolated.setnetworkactive(True) + for i in range(0, self.isolated_idx): + connect_nodes(self.nodes[i], self.isolated_idx) + for node in self.nodes: + if node is not isolated: + assert_raises_jsonrpc(-5, "No such mempool or blockchain transaction", node.getrawtransaction, dblspnd_txid) + # instantsend to receiver. The previously isolated node should prune the doublespend TX and request the correct + # TX from other nodes. + receiver_addr = receiver.getnewaddress() + is_id = sender.instantsendtoaddress(receiver_addr, 0.9) + for node in self.nodes: + self.wait_for_instantlock(is_id, node) + assert_raises_jsonrpc(-5, "No such mempool or blockchain transaction", isolated.getrawtransaction, dblspnd_txid) + if __name__ == '__main__': InstantSendTest().main() diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index fbfd52915cfc1..0b79d476dc0ae 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -1001,25 +1001,56 @@ void CInstantSendManager::HandleFullyConfirmedBlock(const CBlockIndex* pindex) void CInstantSendManager::RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock) { - LOCK(mempool.cs); - std::unordered_map toDelete; - for (auto& in : islock.inputs) { - auto it = mempool.mapNextTx.find(in); - if (it == mempool.mapNextTx.end()) { - continue; + { + LOCK(mempool.cs); + + for (auto& in : islock.inputs) { + auto it = mempool.mapNextTx.find(in); + if (it == mempool.mapNextTx.end()) { + continue; + } + if (it->second->GetHash() != islock.txid) { + toDelete.emplace(it->second->GetHash(), mempool.get(it->second->GetHash())); + + LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: mempool TX %s with input %s conflicts with islock\n", __func__, + islock.txid.ToString(), hash.ToString(), it->second->GetHash().ToString(), in.ToStringShort()); + } } - if (it->second->GetHash() != islock.txid) { - toDelete.emplace(it->second->GetHash(), mempool.get(it->second->GetHash())); - LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: mempool TX %s with input %s conflicts with islock\n", __func__, - islock.txid.ToString(), hash.ToString(), it->second->GetHash().ToString(), in.ToStringShort()); + for (auto& p : toDelete) { + mempool.removeRecursive(*p.second, MemPoolRemovalReason::CONFLICT); } } - for (auto& p : toDelete) { - mempool.removeRecursive(*p.second, MemPoolRemovalReason::CONFLICT); + if (!toDelete.empty()) { + AskNodesForLockedTx(islock.txid); + } +} + +void CInstantSendManager::AskNodesForLockedTx(const uint256& txid) +{ + std::vector nodesToAskFor; + g_connman->ForEachNode([&](CNode* pnode) { + LOCK(pnode->cs_filter); + if (pnode->filterInventoryKnown.contains(txid)) { + pnode->AddRef(); + nodesToAskFor.emplace_back(pnode); + } + }); + { + LOCK(cs_main); + for (CNode* pnode : nodesToAskFor) { + LogPrintf("CInstantSendManager::%s -- txid=%s: asking other peer %d for correct TX\n", __func__, + txid.ToString(), pnode->id); + + CInv inv(MSG_TX, txid); + pnode->AskFor(inv); + } + } + for (CNode* pnode : nodesToAskFor) { + pnode->Release(); } } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 7b97a77fb144c..d75460aeb1974 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -145,6 +145,7 @@ class CInstantSendManager : public CRecoveredSigsListener void HandleFullyConfirmedBlock(const CBlockIndex* pindex); void RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock); + void AskNodesForLockedTx(const uint256& txid); bool ProcessPendingRetryLockTxs(); bool AlreadyHave(const CInv& inv);