diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/main.cpp | 107 | ||||
-rw-r--r-- | src/main.h | 12 | ||||
-rw-r--r-- | src/qt/forms/sendcoinsdialog.ui | 55 | ||||
-rw-r--r-- | src/qt/sendcoinsdialog.cpp | 8 | ||||
-rw-r--r-- | src/test/mempool_tests.cpp | 124 | ||||
-rw-r--r-- | src/test/test_bitcoin.cpp | 2 | ||||
-rw-r--r-- | src/test/test_bitcoin.h | 4 | ||||
-rw-r--r-- | src/test/util_tests.cpp | 5 | ||||
-rw-r--r-- | src/txmempool.cpp | 206 | ||||
-rw-r--r-- | src/txmempool.h | 128 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 3 |
11 files changed, 441 insertions, 213 deletions
diff --git a/src/main.cpp b/src/main.cpp index e4567a8974..1bc88326b6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -795,7 +795,25 @@ bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeig return EvaluateSequenceLocks(block, CalculateSequenceLocks(tx, flags, prevHeights, block)); } -bool CheckSequenceLocks(const CTransaction &tx, int flags) +bool TestLockPointValidity(const LockPoints* lp) +{ + AssertLockHeld(cs_main); + assert(lp); + // If there are relative lock times then the maxInputBlock will be set + // If there are no relative lock times, the LockPoints don't depend on the chain + if (lp->maxInputBlock) { + // Check whether chainActive is an extension of the block at which the LockPoints + // calculation was valid. If not LockPoints are no longer valid + if (!chainActive.Contains(lp->maxInputBlock)) { + return false; + } + } + + // LockPoints still valid + return true; +} + +bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool useExistingLockPoints) { AssertLockHeld(cs_main); AssertLockHeld(mempool.cs); @@ -811,25 +829,57 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags) // *next* block, we need to use one more than chainActive.Height() index.nHeight = tip->nHeight + 1; - // pcoinsTip contains the UTXO set for chainActive.Tip() - CCoinsViewMemPool viewMemPool(pcoinsTip, mempool); - std::vector<int> prevheights; - prevheights.resize(tx.vin.size()); - for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { - const CTxIn& txin = tx.vin[txinIndex]; - CCoins coins; - if (!viewMemPool.GetCoins(txin.prevout.hash, coins)) { - return error("%s: Missing input", __func__); + std::pair<int, int64_t> lockPair; + if (useExistingLockPoints) { + assert(lp); + lockPair.first = lp->height; + lockPair.second = lp->time; + } + else { + // pcoinsTip contains the UTXO set for chainActive.Tip() + CCoinsViewMemPool viewMemPool(pcoinsTip, mempool); + std::vector<int> prevheights; + prevheights.resize(tx.vin.size()); + for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { + const CTxIn& txin = tx.vin[txinIndex]; + CCoins coins; + if (!viewMemPool.GetCoins(txin.prevout.hash, coins)) { + return error("%s: Missing input", __func__); + } + if (coins.nHeight == MEMPOOL_HEIGHT) { + // Assume all mempool transaction confirm in the next block + prevheights[txinIndex] = tip->nHeight + 1; + } else { + prevheights[txinIndex] = coins.nHeight; + } } - if (coins.nHeight == MEMPOOL_HEIGHT) { - // Assume all mempool transaction confirm in the next block - prevheights[txinIndex] = tip->nHeight + 1; - } else { - prevheights[txinIndex] = coins.nHeight; + lockPair = CalculateSequenceLocks(tx, flags, &prevheights, index); + if (lp) { + lp->height = lockPair.first; + lp->time = lockPair.second; + // Also store the hash of the block with the highest height of + // all the blocks which have sequence locked prevouts. + // This hash needs to still be on the chain + // for these LockPoint calculations to be valid + // Note: It is impossible to correctly calculate a maxInputBlock + // if any of the sequence locked inputs depend on unconfirmed txs, + // except in the special case where the relative lock time/height + // is 0, which is equivalent to no sequence lock. Since we assume + // input height of tip+1 for mempool txs and test the resulting + // lockPair from CalculateSequenceLocks against tip+1. We know + // EvaluateSequenceLocks will fail if there was a non-zero sequence + // lock on a mempool input, so we can use the return value of + // CheckSequenceLocks to indicate the LockPoints validity + int maxInputHeight = 0; + BOOST_FOREACH(int height, prevheights) { + // Can ignore mempool inputs since we'll fail if they had non-zero locks + if (height != tip->nHeight+1) { + maxInputHeight = std::max(maxInputHeight, height); + } + } + lp->maxInputBlock = tip->GetAncestor(maxInputHeight); } } - - std::pair<int, int64_t> lockPair = CalculateSequenceLocks(tx, flags, &prevheights, index); return EvaluateSequenceLocks(index, lockPair); } @@ -1018,6 +1068,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C CCoinsViewCache view(&dummy); CAmount nValueIn = 0; + LockPoints lp; { LOCK(pool.cs); CCoinsViewMemPool viewMemPool(pcoinsTip, pool); @@ -1061,7 +1112,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // be mined yet. // Must keep pool.cs for this unless we change CheckSequenceLocks to take a // CoinsViewCache instead of create its own - if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS)) + if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final"); } @@ -1093,7 +1144,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C } } - CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbase, nSigOps); + CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbase, nSigOps, lp); unsigned int nSize = entry.GetTxSize(); // Check that the transaction doesn't have an excessive number of @@ -1195,20 +1246,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // Save these to avoid repeated lookups setIterConflicting.insert(mi); - // If this entry is "dirty", then we don't have descendant - // state for this transaction, which means we probably have - // lots of in-mempool descendants. - // Don't allow replacements of dirty transactions, to ensure - // that we don't spend too much time walking descendants. - // This should be rare. - if (mi->IsDirty()) { - return state.DoS(0, false, - REJECT_NONSTANDARD, "too many potential replacements", false, - strprintf("too many potential replacements: rejecting replacement %s; cannot replace tx %s with untracked descendants", - hash.ToString(), - mi->GetTx().GetHash().ToString())); - } - // Don't allow the replacement to reduce the feerate of the // mempool. // @@ -1338,7 +1375,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C FormatMoney(nModifiedFees - nConflictingFees), (int)nSize - (int)nConflictingSize); } - pool.RemoveStaged(allConflicting); + pool.RemoveStaged(allConflicting, false); // Store transaction in memory pool.addUnchecked(hash, entry, setAncestors, !IsInitialBlockDownload()); @@ -2566,7 +2603,7 @@ bool static DisconnectTip(CValidationState& state, const Consensus::Params& cons list<CTransaction> removed; CValidationState stateDummy; if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL, true)) { - mempool.remove(tx, removed, true); + mempool.removeRecursive(tx, removed); } else if (mempool.exists(tx.GetHash())) { vHashUpdate.push_back(tx.GetHash()); } diff --git a/src/main.h b/src/main.h index b66ad53c8a..6936b5379a 100644 --- a/src/main.h +++ b/src/main.h @@ -40,6 +40,7 @@ class CValidationInterface; class CValidationState; struct CNodeStateStats; +struct LockPoints; /** Default for accepting alerts from the P2P network. */ static const bool DEFAULT_ALERTS = true; @@ -373,6 +374,11 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime); bool CheckFinalTx(const CTransaction &tx, int flags = -1); /** + * Test whether the LockPoints height and time are still valid on the current chain + */ +bool TestLockPointValidity(const LockPoints* lp); + +/** * Check if transaction is final per BIP 68 sequence numbers and can be included in a block. * Consensus critical. Takes as input a list of heights at which tx's inputs (in order) confirmed. */ @@ -382,10 +388,14 @@ bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeig * Check if transaction will be BIP 68 final in the next block to be created. * * Simulates calling SequenceLocks() with data from the tip of the current active chain. + * Optionally stores in LockPoints the resulting height and time calculated and the hash + * of the block needed for calculation or skips the calculation and uses the LockPoints + * passed in for evaluation. + * The LockPoints should not be considered valid if CheckSequenceLocks returns false. * * See consensus/consensus.h for flag definitions. */ -bool CheckSequenceLocks(const CTransaction &tx, int flags); +bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = NULL, bool useExistingLockPoints = false); /** * Closure representing one script verification diff --git a/src/qt/forms/sendcoinsdialog.ui b/src/qt/forms/sendcoinsdialog.ui index 8911b41cbf..12d6a62c08 100644 --- a/src/qt/forms/sendcoinsdialog.ui +++ b/src/qt/forms/sendcoinsdialog.ui @@ -617,7 +617,7 @@ <x>0</x> <y>0</y> <width>830</width> - <height>68</height> + <height>104</height> </rect> </property> <layout class="QVBoxLayout" name="verticalLayout_2" stretch="0,1"> @@ -1167,59 +1167,6 @@ </item> </layout> </item> - <item> - <layout class="QHBoxLayout" name="horizontalLayoutFee5" stretch="0,0,0"> - <property name="spacing"> - <number>8</number> - </property> - <property name="bottomMargin"> - <number>4</number> - </property> - <item> - <widget class="QCheckBox" name="checkBoxFreeTx"> - <property name="text"> - <string>Send as zero-fee transaction if possible</string> - </property> - </widget> - </item> - <item> - <widget class="QLabel" name="labelFreeTx"> - <property name="text"> - <string>(confirmation may take longer)</string> - </property> - <property name="margin"> - <number>5</number> - </property> - </widget> - </item> - <item> - <spacer name="horizontalSpacerFee5"> - <property name="orientation"> - <enum>Qt::Horizontal</enum> - </property> - <property name="sizeHint" stdset="0"> - <size> - <width>1</width> - <height>1</height> - </size> - </property> - </spacer> - </item> - </layout> - </item> - <item> - <spacer name="verticalSpacerFee2"> - <property name="orientation"> - <enum>Qt::Vertical</enum> - </property> - <property name="sizeHint" stdset="0"> - <size> - <width>1</width> - <height>1</height> - </size> - </property> - </spacer> - </item> </layout> </widget> </item> diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 5fc7b57a41..780a6c9709 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -104,8 +104,6 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *platformStyle, QWidget *pa settings.setValue("nTransactionFee", (qint64)DEFAULT_TRANSACTION_FEE); if (!settings.contains("fPayOnlyMinFee")) settings.setValue("fPayOnlyMinFee", false); - if (!settings.contains("fSendFreeTransactions")) - settings.setValue("fSendFreeTransactions", false); ui->groupFee->setId(ui->radioSmartFee, 0); ui->groupFee->setId(ui->radioCustomFee, 1); ui->groupFee->button((int)std::max(0, std::min(1, settings.value("nFeeRadio").toInt())))->setChecked(true); @@ -115,7 +113,6 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *platformStyle, QWidget *pa ui->sliderSmartFee->setValue(settings.value("nSmartFeeSliderPosition").toInt()); ui->customFee->setValue(settings.value("nTransactionFee").toLongLong()); ui->checkBoxMinimumFee->setChecked(settings.value("fPayOnlyMinFee").toBool()); - ui->checkBoxFreeTx->setChecked(settings.value("fSendFreeTransactions").toBool()); minimizeFeeSection(settings.value("fFeeSectionMinimized").toBool()); } @@ -170,8 +167,6 @@ void SendCoinsDialog::setModel(WalletModel *model) connect(ui->checkBoxMinimumFee, SIGNAL(stateChanged(int)), this, SLOT(updateFeeSectionControls())); connect(ui->checkBoxMinimumFee, SIGNAL(stateChanged(int)), this, SLOT(updateGlobalFeeVariables())); connect(ui->checkBoxMinimumFee, SIGNAL(stateChanged(int)), this, SLOT(coinControlUpdateLabels())); - connect(ui->checkBoxFreeTx, SIGNAL(stateChanged(int)), this, SLOT(updateGlobalFeeVariables())); - connect(ui->checkBoxFreeTx, SIGNAL(stateChanged(int)), this, SLOT(coinControlUpdateLabels())); ui->customFee->setSingleStep(CWallet::GetRequiredFee(1000)); updateFeeSectionControls(); updateMinFeeLabel(); @@ -189,7 +184,6 @@ SendCoinsDialog::~SendCoinsDialog() settings.setValue("nSmartFeeSliderPosition", ui->sliderSmartFee->value()); settings.setValue("nTransactionFee", (qint64)ui->customFee->value()); settings.setValue("fPayOnlyMinFee", ui->checkBoxMinimumFee->isChecked()); - settings.setValue("fSendFreeTransactions", ui->checkBoxFreeTx->isChecked()); delete ui; } @@ -605,8 +599,6 @@ void SendCoinsDialog::updateGlobalFeeVariables() // set nMinimumTotalFee to 0 in case of user has selected that the fee is per KB CoinControlDialog::coinControl->nMinimumTotalFee = ui->radioCustomAtLeast->isChecked() ? ui->customFee->value() : 0; } - - fSendFreeTransactions = ui->checkBoxFreeTx->isChecked(); } void SendCoinsDialog::updateFeeMinimizedLabel() diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index fa352ace8f..c8b43df26c 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -57,12 +57,12 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) std::list<CTransaction> removed; // Nothing in pool, remove should do nothing: - testPool.remove(txParent, removed, true); + testPool.removeRecursive(txParent, removed); BOOST_CHECK_EQUAL(removed.size(), 0); // Just the parent: testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); - testPool.remove(txParent, removed, true); + testPool.removeRecursive(txParent, removed); BOOST_CHECK_EQUAL(removed.size(), 1); removed.clear(); @@ -74,16 +74,16 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); } // Remove Child[0], GrandChild[0] should be removed: - testPool.remove(txChild[0], removed, true); + testPool.removeRecursive(txChild[0], removed); BOOST_CHECK_EQUAL(removed.size(), 2); removed.clear(); // ... make sure grandchild and child are gone: - testPool.remove(txGrandChild[0], removed, true); + testPool.removeRecursive(txGrandChild[0], removed); BOOST_CHECK_EQUAL(removed.size(), 0); - testPool.remove(txChild[0], removed, true); + testPool.removeRecursive(txChild[0], removed); BOOST_CHECK_EQUAL(removed.size(), 0); // Remove parent, all children/grandchildren should go: - testPool.remove(txParent, removed, true); + testPool.removeRecursive(txParent, removed); BOOST_CHECK_EQUAL(removed.size(), 5); BOOST_CHECK_EQUAL(testPool.size(), 0); removed.clear(); @@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) } // 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); + testPool.removeRecursive(txParent, removed); BOOST_CHECK_EQUAL(removed.size(), 6); BOOST_CHECK_EQUAL(testPool.size(), 0); removed.clear(); @@ -281,11 +281,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) // Now try removing tx10 and verify the sort order returns to normal std::list<CTransaction> removed; - pool.remove(pool.mapTx.find(tx10.GetHash())->GetTx(), removed, true); + pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), removed); CheckSort<descendant_score>(pool, snapshotOrder); - pool.remove(pool.mapTx.find(tx9.GetHash())->GetTx(), removed, true); - pool.remove(pool.mapTx.find(tx8.GetHash())->GetTx(), removed, true); + pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), removed); + pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), removed); /* Now check the sort on the mining score index. * Final order should be: * @@ -317,6 +317,110 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) CheckSort<mining_score>(pool, sortedOrder); } +BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) +{ + CTxMemPool pool(CFeeRate(0)); + TestMemPoolEntryHelper entry; + entry.hadNoDependencies = true; + + /* 3rd highest fee */ + CMutableTransaction tx1 = CMutableTransaction(); + tx1.vout.resize(1); + tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx1.vout[0].nValue = 10 * COIN; + pool.addUnchecked(tx1.GetHash(), entry.Fee(10000LL).Priority(10.0).FromTx(tx1)); + + /* highest fee */ + CMutableTransaction tx2 = CMutableTransaction(); + tx2.vout.resize(1); + tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx2.vout[0].nValue = 2 * COIN; + pool.addUnchecked(tx2.GetHash(), entry.Fee(20000LL).Priority(9.0).FromTx(tx2)); + uint64_t tx2Size = ::GetSerializeSize(tx2, SER_NETWORK, PROTOCOL_VERSION); + + /* lowest fee */ + CMutableTransaction tx3 = CMutableTransaction(); + tx3.vout.resize(1); + tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx3.vout[0].nValue = 5 * COIN; + pool.addUnchecked(tx3.GetHash(), entry.Fee(0LL).Priority(100.0).FromTx(tx3)); + + /* 2nd highest fee */ + CMutableTransaction tx4 = CMutableTransaction(); + tx4.vout.resize(1); + tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx4.vout[0].nValue = 6 * COIN; + pool.addUnchecked(tx4.GetHash(), entry.Fee(15000LL).Priority(1.0).FromTx(tx4)); + + /* equal fee rate to tx1, but newer */ + CMutableTransaction tx5 = CMutableTransaction(); + tx5.vout.resize(1); + tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx5.vout[0].nValue = 11 * COIN; + pool.addUnchecked(tx5.GetHash(), entry.Fee(10000LL).FromTx(tx5)); + BOOST_CHECK_EQUAL(pool.size(), 5); + + std::vector<std::string> sortedOrder; + sortedOrder.resize(5); + sortedOrder[0] = tx2.GetHash().ToString(); // 20000 + sortedOrder[1] = tx4.GetHash().ToString(); // 15000 + // tx1 and tx5 are both 10000 + // Ties are broken by hash, not timestamp, so determine which + // hash comes first. + if (tx1.GetHash() < tx5.GetHash()) { + sortedOrder[2] = tx1.GetHash().ToString(); + sortedOrder[3] = tx5.GetHash().ToString(); + } else { + sortedOrder[2] = tx5.GetHash().ToString(); + sortedOrder[3] = tx1.GetHash().ToString(); + } + sortedOrder[4] = tx3.GetHash().ToString(); // 0 + + CheckSort<ancestor_score>(pool, sortedOrder); + + /* low fee parent with high fee child */ + /* tx6 (0) -> tx7 (high) */ + CMutableTransaction tx6 = CMutableTransaction(); + tx6.vout.resize(1); + tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx6.vout[0].nValue = 20 * COIN; + uint64_t tx6Size = ::GetSerializeSize(tx6, SER_NETWORK, PROTOCOL_VERSION); + + pool.addUnchecked(tx6.GetHash(), entry.Fee(0LL).FromTx(tx6)); + BOOST_CHECK_EQUAL(pool.size(), 6); + sortedOrder.push_back(tx6.GetHash().ToString()); + CheckSort<ancestor_score>(pool, sortedOrder); + + CMutableTransaction tx7 = CMutableTransaction(); + tx7.vin.resize(1); + tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0); + tx7.vin[0].scriptSig = CScript() << OP_11; + tx7.vout.resize(1); + tx7.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx7.vout[0].nValue = 10 * COIN; + uint64_t tx7Size = ::GetSerializeSize(tx7, SER_NETWORK, PROTOCOL_VERSION); + + /* set the fee to just below tx2's feerate when including ancestor */ + CAmount fee = (20000/tx2Size)*(tx7Size + tx6Size) - 1; + + //CTxMemPoolEntry entry7(tx7, fee, 2, 10.0, 1, true); + pool.addUnchecked(tx7.GetHash(), entry.Fee(fee).FromTx(tx7)); + BOOST_CHECK_EQUAL(pool.size(), 7); + sortedOrder.insert(sortedOrder.begin()+1, tx7.GetHash().ToString()); + CheckSort<ancestor_score>(pool, sortedOrder); + + /* after tx6 is mined, tx7 should move up in the sort */ + std::vector<CTransaction> vtx; + vtx.push_back(tx6); + std::list<CTransaction> dummy; + pool.removeForBlock(vtx, 1, dummy, false); + + sortedOrder.erase(sortedOrder.begin()+1); + sortedOrder.pop_back(); + sortedOrder.insert(sortedOrder.begin(), tx7.GetHash().ToString()); + CheckSort<ancestor_score>(pool, sortedOrder); +} + BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) { diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 39586d7bb4..dadc8b948f 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -152,7 +152,7 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(CMutableTransaction &tx, CTxMemPo CAmount inChainValue = hasNoDependencies ? txn.GetValueOut() : 0; return CTxMemPoolEntry(txn, nFee, nTime, dPriority, nHeight, - hasNoDependencies, inChainValue, spendsCoinbase, sigOpCount); + hasNoDependencies, inChainValue, spendsCoinbase, sigOpCount, lp); } void Shutdown(void* parg) diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h index c623920880..769ae5a132 100644 --- a/src/test/test_bitcoin.h +++ b/src/test/test_bitcoin.h @@ -9,6 +9,7 @@ #include "key.h" #include "pubkey.h" #include "txdb.h" +#include "txmempool.h" #include <boost/filesystem.hpp> #include <boost/thread.hpp> @@ -71,7 +72,8 @@ struct TestMemPoolEntryHelper bool hadNoDependencies; bool spendsCoinbase; unsigned int sigOpCount; - + LockPoints lp; + TestMemPoolEntryHelper() : nFee(0), nTime(0), dPriority(0.0), nHeight(1), hadNoDependencies(false), spendsCoinbase(false), sigOpCount(1) { } diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 43e8ae9b36..b99f952a0d 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -200,6 +200,8 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) BOOST_CHECK_EQUAL(ret, COIN*10); BOOST_CHECK(ParseMoney("1.00", ret)); BOOST_CHECK_EQUAL(ret, COIN); + BOOST_CHECK(ParseMoney("1", ret)); + BOOST_CHECK_EQUAL(ret, COIN); BOOST_CHECK(ParseMoney("0.1", ret)); BOOST_CHECK_EQUAL(ret, COIN/10); BOOST_CHECK(ParseMoney("0.01", ret)); @@ -219,6 +221,9 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) // Attempted 63 bit overflow should fail BOOST_CHECK(!ParseMoney("92233720368.54775808", ret)); + + // Parsing negative amounts must fail + BOOST_CHECK(!ParseMoney("-1", ret)); } BOOST_AUTO_TEST_CASE(util_IsHex) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index eee6cbf855..088e5edde5 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -22,10 +22,10 @@ using namespace std; CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, int64_t _nTime, double _entryPriority, unsigned int _entryHeight, bool poolHasNoInputsOf, CAmount _inChainInputValue, - bool _spendsCoinbase, unsigned int _sigOps): + bool _spendsCoinbase, unsigned int _sigOps, LockPoints lp): tx(_tx), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight), hadNoDependencies(poolHasNoInputsOf), inChainInputValue(_inChainInputValue), - spendsCoinbase(_spendsCoinbase), sigOpCount(_sigOps) + spendsCoinbase(_spendsCoinbase), sigOpCount(_sigOps), lockPoints(lp) { nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); nModSize = tx.CalculateModifiedSize(nTxSize); @@ -38,6 +38,11 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, assert(inChainInputValue <= nValueIn); feeDelta = 0; + + nCountWithAncestors = 1; + nSizeWithAncestors = nTxSize; + nModFeesWithAncestors = nFee; + nSigOpCountWithAncestors = sigOpCount; } CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) @@ -58,27 +63,25 @@ CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta) { nModFeesWithDescendants += newFeeDelta - feeDelta; + nModFeesWithAncestors += newFeeDelta - feeDelta; feeDelta = newFeeDelta; } +void CTxMemPoolEntry::UpdateLockPoints(const LockPoints& lp) +{ + lockPoints = lp; +} + // Update the given tx for any in-mempool descendants. // Assumes that setMemPoolChildren is correct for the given tx and all // descendants. -bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit, cacheMap &cachedDescendants, const std::set<uint256> &setExclude) +void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, const std::set<uint256> &setExclude) { - // Track the number of entries (outside setExclude) that we'd need to visit - // (will bail out if it exceeds maxDescendantsToVisit) - int nChildrenToVisit = 0; - setEntries stageEntries, setAllDescendants; stageEntries = GetMemPoolChildren(updateIt); while (!stageEntries.empty()) { const txiter cit = *stageEntries.begin(); - if (cit->IsDirty()) { - // Don't consider any more children if any descendant is dirty - return false; - } setAllDescendants.insert(cit); stageEntries.erase(cit); const setEntries &setChildren = GetMemPoolChildren(cit); @@ -88,22 +91,11 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit // We've already calculated this one, just add the entries for this set // but don't traverse again. BOOST_FOREACH(const txiter cacheEntry, cacheIt->second) { - // update visit count only for new child transactions - // (outside of setExclude and stageEntries) - if (setAllDescendants.insert(cacheEntry).second && - !setExclude.count(cacheEntry->GetTx().GetHash()) && - !stageEntries.count(cacheEntry)) { - nChildrenToVisit++; - } + setAllDescendants.insert(cacheEntry); } } else if (!setAllDescendants.count(childEntry)) { - // Schedule for later processing and update our visit count - if (stageEntries.insert(childEntry).second && !setExclude.count(childEntry->GetTx().GetHash())) { - nChildrenToVisit++; - } - } - if (nChildrenToVisit > maxDescendantsToVisit) { - return false; + // Schedule for later processing + stageEntries.insert(childEntry); } } } @@ -118,16 +110,18 @@ bool CTxMemPool::UpdateForDescendants(txiter updateIt, int maxDescendantsToVisit modifyFee += cit->GetModifiedFee(); modifyCount++; cachedDescendants[updateIt].insert(cit); + // Update ancestor state for each descendant + mapTx.modify(cit, update_ancestor_state(updateIt->GetTxSize(), updateIt->GetModifiedFee(), 1, updateIt->GetSigOpCount())); } } mapTx.modify(updateIt, update_descendant_state(modifySize, modifyFee, modifyCount)); - return true; } // vHashesToUpdate is the set of transaction hashes from a disconnected block // which has been re-added to the mempool. // for each entry, look for descendants that are outside hashesToUpdate, and // add fee/size information for such descendants to the parent. +// for each such descendant, also update the ancestor state to include the parent. void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate) { LOCK(cs); @@ -167,14 +161,11 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashes UpdateParent(childIter, it, true); } } - if (!UpdateForDescendants(it, 100, mapMemPoolDescendantsToUpdate, setAlreadyIncluded)) { - // Mark as dirty if we can't do the calculation. - mapTx.modify(it, set_dirty()); - } + UpdateForDescendants(it, mapMemPoolDescendantsToUpdate, setAlreadyIncluded); } } -bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */) +bool CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents /* = true */) const { setEntries parentHashes; const CTransaction &tx = entry.GetTx(); @@ -251,6 +242,20 @@ void CTxMemPool::UpdateAncestorsOf(bool add, txiter it, setEntries &setAncestors } } +void CTxMemPool::UpdateEntryForAncestors(txiter it, const setEntries &setAncestors) +{ + int64_t updateCount = setAncestors.size(); + int64_t updateSize = 0; + CAmount updateFee = 0; + int updateSigOps = 0; + BOOST_FOREACH(txiter ancestorIt, setAncestors) { + updateSize += ancestorIt->GetTxSize(); + updateFee += ancestorIt->GetModifiedFee(); + updateSigOps += ancestorIt->GetSigOpCount(); + } + mapTx.modify(it, update_ancestor_state(updateSize, updateFee, updateCount, updateSigOps)); +} + void CTxMemPool::UpdateChildrenForRemoval(txiter it) { const setEntries &setMemPoolChildren = GetMemPoolChildren(it); @@ -259,11 +264,30 @@ void CTxMemPool::UpdateChildrenForRemoval(txiter it) } } -void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove) +void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants) { // For each entry, walk back all ancestors and decrement size associated with this // transaction const uint64_t nNoLimit = std::numeric_limits<uint64_t>::max(); + if (updateDescendants) { + // updateDescendants should be true whenever we're not recursively + // removing a tx and all its descendants, eg when a transaction is + // confirmed in a block. + // Here we only update statistics and not data in mapLinks (which + // we need to preserve until we're finished with all operations that + // need to traverse the mempool). + BOOST_FOREACH(txiter removeIt, entriesToRemove) { + setEntries setDescendants; + CalculateDescendants(removeIt, setDescendants); + setDescendants.erase(removeIt); // don't update state for self + int64_t modifySize = -((int64_t)removeIt->GetTxSize()); + CAmount modifyFee = -removeIt->GetModifiedFee(); + int modifySigOps = -removeIt->GetSigOpCount(); + BOOST_FOREACH(txiter dit, setDescendants) { + mapTx.modify(dit, update_ancestor_state(modifySize, modifyFee, -1, modifySigOps)); + } + } + } BOOST_FOREACH(txiter removeIt, entriesToRemove) { setEntries setAncestors; const CTxMemPoolEntry &entry = *removeIt; @@ -287,10 +311,7 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove) // transactions as the set of things to update for removal. CalculateMemPoolAncestors(entry, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy, false); // Note that UpdateAncestorsOf severs the child links that point to - // removeIt in the entries for the parents of removeIt. This is - // fine since we don't need to use the mempool children of any entries - // to walk back over our ancestors (but we do need the mempool - // parents!) + // removeIt in the entries for the parents of removeIt. UpdateAncestorsOf(false, removeIt, setAncestors); } // After updating all the ancestor sizes, we can now sever the link between each @@ -301,22 +322,24 @@ void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove) } } -void CTxMemPoolEntry::SetDirty() +void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) { - nCountWithDescendants = 0; - nSizeWithDescendants = nTxSize; - nModFeesWithDescendants = GetModifiedFee(); + nSizeWithDescendants += modifySize; + assert(int64_t(nSizeWithDescendants) > 0); + nModFeesWithDescendants += modifyFee; + nCountWithDescendants += modifyCount; + assert(int64_t(nCountWithDescendants) > 0); } -void CTxMemPoolEntry::UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount) +void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int modifySigOps) { - if (!IsDirty()) { - nSizeWithDescendants += modifySize; - assert(int64_t(nSizeWithDescendants) > 0); - nModFeesWithDescendants += modifyFee; - nCountWithDescendants += modifyCount; - assert(int64_t(nCountWithDescendants) > 0); - } + nSizeWithAncestors += modifySize; + assert(int64_t(nSizeWithAncestors) > 0); + nModFeesWithAncestors += modifyFee; + nCountWithAncestors += modifyCount; + assert(int64_t(nCountWithAncestors) > 0); + nSigOpCountWithAncestors += modifySigOps; + assert(int(nSigOpCountWithAncestors) >= 0); } CTxMemPool::CTxMemPool(const CFeeRate& _minReasonableRelayFee) : @@ -409,6 +432,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, } } UpdateAncestorsOf(true, newit, setAncestors); + UpdateEntryForAncestors(newit, setAncestors); nTransactionsUpdated++; totalTxSize += entry.GetTxSize(); @@ -461,7 +485,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants } } -void CTxMemPool::remove(const CTransaction &origTx, std::list<CTransaction>& removed, bool fRecursive) +void CTxMemPool::removeRecursive(const CTransaction &origTx, std::list<CTransaction>& removed) { // Remove transaction from memory pool { @@ -470,8 +494,8 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list<CTransaction>& rem txiter origit = mapTx.find(origTx.GetHash()); if (origit != mapTx.end()) { txToRemove.insert(origit); - } else if (fRecursive) { - // If recursively removing but origTx isn't in the mempool + } else { + // When 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. @@ -485,17 +509,13 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list<CTransaction>& rem } } setEntries setAllRemoves; - if (fRecursive) { - BOOST_FOREACH(txiter it, txToRemove) { - CalculateDescendants(it, setAllRemoves); - } - } else { - setAllRemoves.swap(txToRemove); + BOOST_FOREACH(txiter it, txToRemove) { + CalculateDescendants(it, setAllRemoves); } BOOST_FOREACH(txiter it, setAllRemoves) { removed.push_back(it->GetTx()); } - RemoveStaged(setAllRemoves); + RemoveStaged(setAllRemoves, false); } } @@ -506,7 +526,11 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem list<CTransaction> transactionsToRemove; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { const CTransaction& tx = it->GetTx(); - if (!CheckFinalTx(tx, flags) || !CheckSequenceLocks(tx, flags)) { + LockPoints lp = it->GetLockPoints(); + bool validLP = TestLockPointValidity(&lp); + if (!CheckFinalTx(tx, flags) || !CheckSequenceLocks(tx, flags, &lp, validLP)) { + // Note if CheckSequenceLocks fails the LockPoints may still be invalid + // So it's critical that we remove the tx and not depend on the LockPoints. transactionsToRemove.push_back(tx); } else if (it->GetSpendsCoinbase()) { BOOST_FOREACH(const CTxIn& txin, tx.vin) { @@ -521,10 +545,13 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem } } } + if (!validLP) { + mapTx.modify(it, update_lock_points(lp)); + } } BOOST_FOREACH(const CTransaction& tx, transactionsToRemove) { list<CTransaction> removed; - remove(tx, removed, true); + removeRecursive(tx, removed); } } @@ -539,7 +566,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::list<CTransaction> const CTransaction &txConflict = *it->second.ptx; if (txConflict != tx) { - remove(txConflict, removed, true); + removeRecursive(txConflict, removed); ClearPrioritisation(txConflict.GetHash()); } } @@ -564,8 +591,12 @@ void CTxMemPool::removeForBlock(const std::vector<CTransaction>& vtx, unsigned i } BOOST_FOREACH(const CTransaction& tx, vtx) { - std::list<CTransaction> dummy; - remove(tx, dummy, false); + txiter it = mapTx.find(tx.GetHash()); + if (it != mapTx.end()) { + setEntries stage; + stage.insert(it); + RemoveStaged(stage, true); + } removeConflicts(tx, conflicts); ClearPrioritisation(tx.GetHash()); } @@ -622,6 +653,8 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const innerUsage += memusage::DynamicUsage(links.parents) + memusage::DynamicUsage(links.children); bool fDependsWait = false; setEntries setParentCheck; + int64_t parentSizes = 0; + unsigned int parentSigOpCount = 0; BOOST_FOREACH(const CTxIn &txin, tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); @@ -629,7 +662,10 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const const CTransaction& tx2 = it2->GetTx(); assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); fDependsWait = true; - setParentCheck.insert(it2); + if (setParentCheck.insert(it2).second) { + parentSizes += it2->GetTxSize(); + parentSigOpCount += it2->GetSigOpCount(); + } } else { const CCoins* coins = pcoins->AccessCoins(txin.prevout.hash); assert(coins && coins->IsAvailable(txin.prevout.n)); @@ -642,28 +678,42 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const i++; } assert(setParentCheck == GetMemPoolParents(it)); + // Verify ancestor state is correct. + setEntries setAncestors; + uint64_t nNoLimit = std::numeric_limits<uint64_t>::max(); + std::string dummy; + CalculateMemPoolAncestors(*it, setAncestors, nNoLimit, nNoLimit, nNoLimit, nNoLimit, dummy); + uint64_t nCountCheck = setAncestors.size() + 1; + uint64_t nSizeCheck = it->GetTxSize(); + CAmount nFeesCheck = it->GetModifiedFee(); + unsigned int nSigOpCheck = it->GetSigOpCount(); + + BOOST_FOREACH(txiter ancestorIt, setAncestors) { + nSizeCheck += ancestorIt->GetTxSize(); + nFeesCheck += ancestorIt->GetModifiedFee(); + nSigOpCheck += ancestorIt->GetSigOpCount(); + } + + assert(it->GetCountWithAncestors() == nCountCheck); + assert(it->GetSizeWithAncestors() == nSizeCheck); + assert(it->GetSigOpCountWithAncestors() == nSigOpCheck); + assert(it->GetModFeesWithAncestors() == nFeesCheck); + // Check children against mapNextTx CTxMemPool::setEntries setChildrenCheck; std::map<COutPoint, CInPoint>::const_iterator iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0)); int64_t childSizes = 0; - CAmount childModFee = 0; for (; iter != mapNextTx.end() && iter->first.hash == it->GetTx().GetHash(); ++iter) { txiter childit = mapTx.find(iter->second.ptx->GetHash()); assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions if (setChildrenCheck.insert(childit).second) { childSizes += childit->GetTxSize(); - childModFee += childit->GetModifiedFee(); } } assert(setChildrenCheck == GetMemPoolChildren(it)); // Also check to make sure size is greater than sum with immediate children. // just a sanity check, not definitive that this calc is correct... - if (!it->IsDirty()) { - assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize()); - } else { - assert(it->GetSizeWithDescendants() == it->GetTxSize()); - assert(it->GetModFeesWithDescendants() == it->GetModifiedFee()); - } + assert(it->GetSizeWithDescendants() >= childSizes + it->GetTxSize()); if (fDependsWait) waitingOnDependants.push_back(&(*it)); @@ -845,13 +895,13 @@ bool CCoinsViewMemPool::HaveCoins(const uint256 &txid) const { size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); - // Estimate the overhead of mapTx to be 12 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. - return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 12 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + cachedInnerUsage; + // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + cachedInnerUsage; } -void CTxMemPool::RemoveStaged(setEntries &stage) { +void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants) { AssertLockHeld(cs); - UpdateForRemoveFromMempool(stage); + UpdateForRemoveFromMempool(stage, updateDescendants); BOOST_FOREACH(const txiter& it, stage) { removeUnchecked(it); } @@ -869,7 +919,7 @@ int CTxMemPool::Expire(int64_t time) { BOOST_FOREACH(txiter removeit, toremove) { CalculateDescendants(removeit, stage); } - RemoveStaged(stage); + RemoveStaged(stage, false); return stage.size(); } @@ -978,7 +1028,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<uint256>* pvNoSpendsRe BOOST_FOREACH(txiter it, stage) txn.push_back(it->GetTx()); } - RemoveStaged(stage); + RemoveStaged(stage, false); if (pvNoSpendsRemaining) { BOOST_FOREACH(const CTransaction& tx, txn) { BOOST_FOREACH(const CTxIn& txin, tx.vin) { diff --git a/src/txmempool.h b/src/txmempool.h index 7a2a1ef432..665bb44cf3 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -19,6 +19,7 @@ #include "boost/multi_index/ordered_index.hpp" class CAutoFile; +class CBlockIndex; inline double AllowFreeThreshold() { @@ -35,6 +36,21 @@ inline bool AllowFree(double dPriority) /** Fake height value used in CCoins to signify they are only in the memory pool (since 0.8) */ static const unsigned int MEMPOOL_HEIGHT = 0x7FFFFFFF; +struct LockPoints +{ + // Will be set to the blockchain height and median time past + // values that would be necessary to satisfy all relative locktime + // constraints (BIP68) of this tx given our view of block chain history + int height; + int64_t time; + // As long as the current chain descends from the highest height block + // containing one of the inputs used in the calculation, then the cached + // values are still valid even after a reorg. + CBlockIndex* maxInputBlock; + + LockPoints() : height(0), time(0), maxInputBlock(NULL) { } +}; + class CTxMemPool; /** \class CTxMemPoolEntry @@ -70,6 +86,7 @@ private: bool spendsCoinbase; //! keep track of transactions that spend a coinbase unsigned int sigOpCount; //! Legacy sig ops plus P2SH sig op count int64_t feeDelta; //! Used for determining the priority of the transaction for mining in a block + LockPoints lockPoints; //! Track the height and time at which tx was final // Information about descendants of this transaction that are in the // mempool; if we remove this transaction we must remove all of these @@ -80,11 +97,17 @@ private: uint64_t nSizeWithDescendants; //! ... and size CAmount nModFeesWithDescendants; //! ... and total fees (all including us) + // Analogous statistics for ancestor transactions + uint64_t nCountWithAncestors; + uint64_t nSizeWithAncestors; + CAmount nModFeesWithAncestors; + unsigned int nSigOpCountWithAncestors; + public: CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, int64_t _nTime, double _entryPriority, unsigned int _entryHeight, bool poolHasNoInputsOf, CAmount _inChainInputValue, bool spendsCoinbase, - unsigned int nSigOps); + unsigned int nSigOps, LockPoints lp); CTxMemPoolEntry(const CTxMemPoolEntry& other); const CTransaction& GetTx() const { return this->tx; } @@ -101,25 +124,28 @@ public: unsigned int GetSigOpCount() const { return sigOpCount; } int64_t GetModifiedFee() const { return nFee + feeDelta; } size_t DynamicMemoryUsage() const { return nUsageSize; } + const LockPoints& GetLockPoints() const { return lockPoints; } // Adjusts the descendant state, if this entry is not dirty. - void UpdateState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); + void UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount); + // Adjusts the ancestor state + void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int modifySigOps); // Updates the fee delta used for mining priority score, and the // modified fees with descendants. void UpdateFeeDelta(int64_t feeDelta); - - /** We can set the entry to be dirty if doing the full calculation of in- - * mempool descendants will be too expensive, which can potentially happen - * when re-adding transactions from a block back to the mempool. - */ - void SetDirty(); - bool IsDirty() const { return nCountWithDescendants == 0; } + // Update the LockPoints after a reorg + void UpdateLockPoints(const LockPoints& lp); uint64_t GetCountWithDescendants() const { return nCountWithDescendants; } uint64_t GetSizeWithDescendants() const { return nSizeWithDescendants; } CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; } bool GetSpendsCoinbase() const { return spendsCoinbase; } + + uint64_t GetCountWithAncestors() const { return nCountWithAncestors; } + uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } + CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } + unsigned int GetSigOpCountWithAncestors() const { return nSigOpCountWithAncestors; } }; // Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index. @@ -130,7 +156,7 @@ struct update_descendant_state {} void operator() (CTxMemPoolEntry &e) - { e.UpdateState(modifySize, modifyFee, modifyCount); } + { e.UpdateDescendantState(modifySize, modifyFee, modifyCount); } private: int64_t modifySize; @@ -138,10 +164,20 @@ struct update_descendant_state int64_t modifyCount; }; -struct set_dirty +struct update_ancestor_state { + update_ancestor_state(int64_t _modifySize, CAmount _modifyFee, int64_t _modifyCount, int _modifySigOps) : + modifySize(_modifySize), modifyFee(_modifyFee), modifyCount(_modifyCount), modifySigOps(_modifySigOps) + {} + void operator() (CTxMemPoolEntry &e) - { e.SetDirty(); } + { e.UpdateAncestorState(modifySize, modifyFee, modifyCount, modifySigOps); } + + private: + int64_t modifySize; + CAmount modifyFee; + int64_t modifyCount; + int modifySigOps; }; struct update_fee_delta @@ -154,6 +190,16 @@ private: int64_t feeDelta; }; +struct update_lock_points +{ + update_lock_points(const LockPoints& _lp) : lp(_lp) { } + + void operator() (CTxMemPoolEntry &e) { e.UpdateLockPoints(lp); } + +private: + const LockPoints& lp; +}; + // extracts a TxMemPoolEntry's transaction hash struct mempoolentry_txid { @@ -228,10 +274,34 @@ public: } }; +class CompareTxMemPoolEntryByAncestorFee +{ +public: + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) + { + double aFees = a.GetModFeesWithAncestors(); + double aSize = a.GetSizeWithAncestors(); + + double bFees = b.GetModFeesWithAncestors(); + double bSize = b.GetSizeWithAncestors(); + + // Avoid division by rewriting (a/b > c/d) as (a*d > c*b). + double f1 = aFees * bSize; + double f2 = aSize * bFees; + + if (f1 == f2) { + return a.GetTx().GetHash() < b.GetTx().GetHash(); + } + + return f1 > f2; + } +}; + // Multi_index tag names struct descendant_score {}; struct entry_time {}; struct mining_score {}; +struct ancestor_score {}; class CBlockPolicyEstimator; @@ -364,12 +434,18 @@ public: boost::multi_index::tag<entry_time>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime - >, + >, // sorted by score (for mining prioritization) boost::multi_index::ordered_unique< boost::multi_index::tag<mining_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByScore + >, + // sorted by fee rate with ancestors + boost::multi_index::ordered_non_unique< + boost::multi_index::tag<ancestor_score>, + boost::multi_index::identity<CTxMemPoolEntry>, + CompareTxMemPoolEntryByAncestorFee > > > indexed_transaction_set; @@ -428,7 +504,7 @@ public: bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate = true); bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true); - void remove(const CTransaction &tx, std::list<CTransaction>& removed, bool fRecursive = false); + void removeRecursive(const CTransaction &tx, std::list<CTransaction>& removed); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); void removeConflicts(const CTransaction &tx, std::list<CTransaction>& removed); void removeForBlock(const std::vector<CTransaction>& vtx, unsigned int nBlockHeight, @@ -453,8 +529,12 @@ public: public: /** Remove a set of transactions from the mempool. * If a transaction is in this set, then all in-mempool descendants must - * also be in the set.*/ - void RemoveStaged(setEntries &stage); + * also be in the set, unless this transaction is being removed for being + * in a block. + * Set updateDescendants to true when removing a tx that was in a block, so + * that any in-mempool descendants have their ancestor state updated. + */ + void RemoveStaged(setEntries &stage, bool updateDescendants); /** When adding transactions from a disconnected block back to the mempool, * new mempool entries may have children in the mempool (which is generally @@ -477,7 +557,7 @@ public: * fSearchForParents = whether to search a tx's vin for in-mempool parents, or * look up parents from mapLinks. Must be true for entries not in the mempool */ - bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents = true); + bool CalculateMemPoolAncestors(const CTxMemPoolEntry &entry, setEntries &setAncestors, uint64_t limitAncestorCount, uint64_t limitAncestorSize, uint64_t limitDescendantCount, uint64_t limitDescendantSize, std::string &errString, bool fSearchForParents = true) const; /** Populate setDescendants with all in-mempool descendants of hash. * Assumes that setDescendants includes all in-mempool descendants of anything @@ -555,21 +635,21 @@ private: * updated and hence their state is already reflected in the parent * state). * - * If updating an entry requires looking at more than maxDescendantsToVisit - * transactions, outside of the ones in setExclude, then give up. - * * cachedDescendants will be updated with the descendants of the transaction * being updated, so that future invocations don't need to walk the * same transaction again, if encountered in another transaction chain. */ - bool UpdateForDescendants(txiter updateIt, - int maxDescendantsToVisit, + void UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, const std::set<uint256> &setExclude); /** Update ancestors of hash to add/remove it as a descendant transaction. */ void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors); - /** For each transaction being removed, update ancestors and any direct children. */ - void UpdateForRemoveFromMempool(const setEntries &entriesToRemove); + /** Set ancestor state for an entry */ + void UpdateEntryForAncestors(txiter it, const setEntries &setAncestors); + /** For each transaction being removed, update ancestors and any direct children. + * If updateDescendants is true, then also update in-mempool descendants' + * ancestor state. */ + void UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants); /** Sever link between specified transaction and direct children. */ void UpdateChildrenForRemoval(txiter entry); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bcfefa27ff..1ef055e552 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2970,7 +2970,8 @@ std::string CWallet::GetWalletHelpString(bool showDebug) CURRENCY_UNIT, FormatMoney(payTxFee.GetFeePerK()))); strUsage += HelpMessageOpt("-rescan", _("Rescan the block chain for missing wallet transactions on startup")); strUsage += HelpMessageOpt("-salvagewallet", _("Attempt to recover private keys from a corrupt wallet on startup")); - strUsage += HelpMessageOpt("-sendfreetransactions", strprintf(_("Send transactions as zero-fee transactions if possible (default: %u)"), DEFAULT_SEND_FREE_TRANSACTIONS)); + if (showDebug) + strUsage += HelpMessageOpt("-sendfreetransactions", strprintf(_("Send transactions as zero-fee transactions if possible (default: %u)"), DEFAULT_SEND_FREE_TRANSACTIONS)); strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE)); strUsage += HelpMessageOpt("-txconfirmtarget=<n>", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET)); strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup")); |