From 1c62e8409998f0358f2fca522678704824e332c1 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Wed, 25 Mar 2015 13:13:09 -0400 Subject: Keep mempool consistent during block-reorgs This fixes a subtle bug involving block re-orgs and non-standard transactions. Start with a block containing a non-standard transaction, and one or more transactions spending it in the memory pool. Then re-org away from that block to another chain that does not contain the non-standard transaction. Result before this fix: the dependent transactions get stuck in the mempool without their parent, putting the mempool in an inconsistent state. Tested with a new unit test (adapted for 0.10). Rebased-From: ad9e86dca11dce023d827d342e966f3806c39d27 Github-Pull: #5945 --- src/Makefile.test.include | 1 + src/test/mempool_tests.cpp | 102 +++++++++++++++++++++++++++++++++++++++++++++ src/txmempool.cpp | 12 ++++++ 3 files changed, 115 insertions(+) create mode 100644 src/test/mempool_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 0a9a335e7a..9cc479c305 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -51,6 +51,7 @@ BITCOIN_TESTS =\ test/hash_tests.cpp \ test/key_tests.cpp \ test/main_tests.cpp \ + test/mempool_tests.cpp \ test/miner_tests.cpp \ test/mruset_tests.cpp \ test/multisig_tests.cpp \ diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp new file mode 100644 index 0000000000..b85f768ed5 --- /dev/null +++ b/src/test/mempool_tests.cpp @@ -0,0 +1,102 @@ +// Copyright (c) 2011-2014 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "main.h" +#include "txmempool.h" +#include "util.h" + +#include +#include + +BOOST_AUTO_TEST_SUITE(mempool_tests) + +BOOST_AUTO_TEST_CASE(MempoolRemoveTest) +{ + // Test CTxMemPool::remove functionality + + // Parent transaction with three children, + // and three grand-children: + CMutableTransaction txParent; + txParent.vin.resize(1); + txParent.vin[0].scriptSig = CScript() << OP_11; + txParent.vout.resize(3); + for (int i = 0; i < 3; i++) + { + txParent.vout[i].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + txParent.vout[i].nValue = 33000LL; + } + CMutableTransaction txChild[3]; + for (int i = 0; i < 3; i++) + { + txChild[i].vin.resize(1); + txChild[i].vin[0].scriptSig = CScript() << OP_11; + txChild[i].vin[0].prevout.hash = txParent.GetHash(); + txChild[i].vin[0].prevout.n = i; + txChild[i].vout.resize(1); + txChild[i].vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + txChild[i].vout[0].nValue = 11000LL; + } + CMutableTransaction txGrandChild[3]; + for (int i = 0; i < 3; i++) + { + txGrandChild[i].vin.resize(1); + txGrandChild[i].vin[0].scriptSig = CScript() << OP_11; + txGrandChild[i].vin[0].prevout.hash = txChild[i].GetHash(); + txGrandChild[i].vin[0].prevout.n = 0; + txGrandChild[i].vout.resize(1); + txGrandChild[i].vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + txGrandChild[i].vout[0].nValue = 11000LL; + } + + + CTxMemPool testPool(CFeeRate(0)); + std::list removed; + + // Nothing in pool, remove should do nothing: + testPool.remove(txParent, removed, true); + BOOST_CHECK_EQUAL(removed.size(), 0); + + // Just the parent: + testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1)); + testPool.remove(txParent, removed, true); + BOOST_CHECK_EQUAL(removed.size(), 1); + removed.clear(); + + // Parent, children, grandchildren: + testPool.addUnchecked(txParent.GetHash(), CTxMemPoolEntry(txParent, 0, 0, 0.0, 1)); + for (int i = 0; i < 3; i++) + { + testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1)); + testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1)); + } + // Remove Child[0], GrandChild[0] should be removed: + testPool.remove(txChild[0], removed, true); + BOOST_CHECK_EQUAL(removed.size(), 2); + removed.clear(); + // ... make sure grandchild and child are gone: + testPool.remove(txGrandChild[0], removed, true); + BOOST_CHECK_EQUAL(removed.size(), 0); + testPool.remove(txChild[0], removed, true); + BOOST_CHECK_EQUAL(removed.size(), 0); + // Remove parent, all children/grandchildren should go: + testPool.remove(txParent, removed, true); + BOOST_CHECK_EQUAL(removed.size(), 5); + BOOST_CHECK_EQUAL(testPool.size(), 0); + removed.clear(); + + // Add children and grandchildren, but NOT the parent (simulate the parent being in a block) + for (int i = 0; i < 3; i++) + { + testPool.addUnchecked(txChild[i].GetHash(), CTxMemPoolEntry(txChild[i], 0, 0, 0.0, 1)); + testPool.addUnchecked(txGrandChild[i].GetHash(), CTxMemPoolEntry(txGrandChild[i], 0, 0, 0.0, 1)); + } + // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be + // put into the mempool (maybe because it is non-standard): + testPool.remove(txParent, removed, true); + BOOST_CHECK_EQUAL(removed.size(), 6); + BOOST_CHECK_EQUAL(testPool.size(), 0); + removed.clear(); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index dcdf5653fe..db598a1dfa 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -444,6 +444,18 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem LOCK(cs); std::deque txToRemove; txToRemove.push_back(origTx.GetHash()); + if (fRecursive && !mapTx.count(origTx.GetHash())) { + // If recursively removing but origTx isn't in the mempool + // be sure to remove any children that are in the pool. This can + // happen during chain re-orgs if origTx isn't re-accepted into + // the mempool for any reason. + for (unsigned int i = 0; i < origTx.vout.size(); i++) { + std::map::iterator it = mapNextTx.find(COutPoint(origTx.GetHash(), i)); + if (it == mapNextTx.end()) + continue; + txToRemove.push_back(it->second.ptx->GetHash()); + } + } while (!txToRemove.empty()) { uint256 hash = txToRemove.front(); -- cgit v1.2.3