From c05f969569ec40748cdfd92aea0564bc5c14b3ad Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 11 Jul 2015 18:52:03 +0200 Subject: [PATCH 1/5] Avoid 'foreach' in Qt code as it clashes with boost::foreach --- src/chain.h | 1 + src/coins.h | 1 + src/memusage.h | 1 + src/net.h | 1 + src/qt/addressbookpage.cpp | 2 +- src/qt/bitcoingui.cpp | 6 +++--- src/qt/coincontroldialog.cpp | 2 +- src/qt/optionsdialog.cpp | 2 +- src/qt/paymentserver.cpp | 10 +++++----- src/qt/peertablemodel.cpp | 4 ++-- src/qt/receivecoinsdialog.cpp | 2 +- src/qt/rpcconsole.cpp | 2 +- src/qt/sendcoinsdialog.cpp | 4 ++-- src/qt/trafficgraphwidget.cpp | 4 ++-- src/qt/transactiondesc.cpp | 4 ++-- src/qt/transactiontablemodel.cpp | 2 +- src/qt/utilitydialog.cpp | 2 +- src/qt/walletmodel.cpp | 6 +++--- src/qt/walletmodeltransaction.cpp | 2 +- 19 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/chain.h b/src/chain.h index 01be2d6e5c940..d12195aceb9b3 100644 --- a/src/chain.h +++ b/src/chain.h @@ -14,6 +14,7 @@ #include +#undef foreach #include struct CDiskBlockPos diff --git a/src/coins.h b/src/coins.h index a4671645df5db..cb8d788faf1b2 100644 --- a/src/coins.h +++ b/src/coins.h @@ -14,6 +14,7 @@ #include #include +#undef foreach #include #include diff --git a/src/memusage.h b/src/memusage.h index 7a831e6d33cc7..8a52812a7c4e6 100644 --- a/src/memusage.h +++ b/src/memusage.h @@ -11,6 +11,7 @@ #include #include +#undef foreach #include #include #include diff --git a/src/net.h b/src/net.h index 86d74e21741b5..b34480ec630ec 100644 --- a/src/net.h +++ b/src/net.h @@ -26,6 +26,7 @@ #include #endif +#undef foreach #include #include #include diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index 54635f1d548a3..af6801919c564 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -254,7 +254,7 @@ void AddressBookPage::done(int retval) // Figure out which address was selected, and return it QModelIndexList indexes = table->selectionModel()->selectedRows(AddressTableModel::Address); - foreach (const QModelIndex& index, indexes) { + Q_FOREACH (const QModelIndex& index, indexes) { QVariant address = table->model()->data(index); returnValue = address.toString(); } diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index efba0f5e18ed6..22c7f1e6c96ab 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -886,7 +886,7 @@ void BitcoinGUI::dropEvent(QDropEvent *event) { if(event->mimeData()->hasUrls()) { - foreach(const QUrl &uri, event->mimeData()->urls()) + Q_FOREACH(const QUrl &uri, event->mimeData()->urls()) { emit receivedURI(uri.toString()); } @@ -1050,7 +1050,7 @@ UnitDisplayStatusBarControl::UnitDisplayStatusBarControl() : QList units = BitcoinUnits::availableUnits(); int max_width = 0; const QFontMetrics fm(font()); - foreach (const BitcoinUnits::Unit unit, units) + Q_FOREACH (const BitcoinUnits::Unit unit, units) { max_width = qMax(max_width, fm.width(BitcoinUnits::name(unit))); } @@ -1069,7 +1069,7 @@ void UnitDisplayStatusBarControl::mousePressEvent(QMouseEvent *event) void UnitDisplayStatusBarControl::createContextMenu() { menu = new QMenu(); - foreach(BitcoinUnits::Unit u, BitcoinUnits::availableUnits()) + Q_FOREACH(BitcoinUnits::Unit u, BitcoinUnits::availableUnits()) { QAction *menuAction = new QAction(QString(BitcoinUnits::name(u)), this); menuAction->setData(QVariant(u)); diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index eea450353385f..cb888a07c4fe6 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -461,7 +461,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) CAmount nPayAmount = 0; bool fDust = false; CMutableTransaction txDummy; - foreach(const CAmount &amount, CoinControlDialog::payAmounts) + Q_FOREACH(const CAmount &amount, CoinControlDialog::payAmounts) { nPayAmount += amount; diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 87c727335eb83..fda97fd4052f3 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -73,7 +73,7 @@ OptionsDialog::OptionsDialog(QWidget *parent, bool enableWallet) : /* Display elements init */ QDir translations(":translations"); ui->lang->addItem(QString("(") + tr("default") + QString(")"), QVariant("")); - foreach(const QString &langStr, translations.entryList()) + Q_FOREACH(const QString &langStr, translations.entryList()) { QLocale locale(langStr); diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 09e9949b1035d..388a9f5a789cb 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -148,7 +148,7 @@ void PaymentServer::LoadRootCAs(X509_STORE* _store) int nRootCerts = 0; const QDateTime currentTime = QDateTime::currentDateTime(); - foreach (const QSslCertificate& cert, certList) { + Q_FOREACH (const QSslCertificate& cert, certList) { // Don't log NULL certificates if (cert.isNull()) continue; @@ -269,7 +269,7 @@ void PaymentServer::ipcParseCommandLine(int argc, char* argv[]) bool PaymentServer::ipcSendCommandLine() { bool fResult = false; - foreach (const QString& r, savedPaymentRequests) + Q_FOREACH (const QString& r, savedPaymentRequests) { QLocalSocket* socket = new QLocalSocket(); socket->connectToServer(ipcServerName(), QIODevice::WriteOnly); @@ -394,7 +394,7 @@ void PaymentServer::uiReady() initNetManager(); saveURIs = false; - foreach (const QString& s, savedPaymentRequests) + Q_FOREACH (const QString& s, savedPaymentRequests) { handleURIOrFile(s); } @@ -562,7 +562,7 @@ bool PaymentServer::processPaymentRequest(const PaymentRequestPlus& request, Sen QList > sendingTos = request.getPayTo(); QStringList addresses; - foreach(const PAIRTYPE(CScript, CAmount)& sendingTo, sendingTos) { + Q_FOREACH(const PAIRTYPE(CScript, CAmount)& sendingTo, sendingTos) { // Extract and check destination addresses CTxDestination dest; if (ExtractDestination(sendingTo.first, dest)) { @@ -750,7 +750,7 @@ void PaymentServer::reportSslErrors(QNetworkReply* reply, const QList Q_UNUSED(reply); QString errString; - foreach (const QSslError& err, errs) { + Q_FOREACH (const QSslError& err, errs) { qWarning() << "PaymentServer::reportSslErrors: " << err; errString += err.errorString() + "\n"; } diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index f5904a4d8e33f..890a195ad681c 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -63,7 +63,7 @@ class PeerTablePriv #if QT_VERSION >= 0x040700 cachedNodeStats.reserve(vNodes.size()); #endif - foreach (CNode* pnode, vNodes) + Q_FOREACH (CNode* pnode, vNodes) { CNodeCombinedStats stats; stats.nodeStateStats.nMisbehavior = 0; @@ -92,7 +92,7 @@ class PeerTablePriv // build index map mapNodeRows.clear(); int row = 0; - foreach (const CNodeCombinedStats& stats, cachedNodeStats) + Q_FOREACH (const CNodeCombinedStats& stats, cachedNodeStats) mapNodeRows.insert(std::pair(stats.nodeStats.nodeid, row++)); } diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index fd225f51a6651..43b46c63b5291 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -185,7 +185,7 @@ void ReceiveCoinsDialog::on_showRequestButton_clicked() return; QModelIndexList selection = ui->recentRequestsView->selectionModel()->selectedRows(); - foreach (const QModelIndex& index, selection) { + Q_FOREACH (const QModelIndex& index, selection) { on_recentRequestsView_doubleClicked(index); } } diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index f828ce25344fa..87df31b757215 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -95,7 +95,7 @@ bool parseCommandLine(std::vector &args, const std::string &strComm STATE_ESCAPE_DOUBLEQUOTED } state = STATE_EATING_SPACES; std::string curarg; - foreach(char ch, strCommand) + Q_FOREACH(char ch, strCommand) { switch(state) { diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 3d57711568ae8..f239d2e4b7063 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -251,7 +251,7 @@ void SendCoinsDialog::on_sendButton_clicked() // Format confirmation message QStringList formatted; - foreach(const SendCoinsRecipient &rcp, currentTransaction.getRecipients()) + Q_FOREACH(const SendCoinsRecipient &rcp, currentTransaction.getRecipients()) { // generate bold amount string QString amount = "" + BitcoinUnits::formatHtmlWithUnit(model->getOptionsModel()->getDisplayUnit(), rcp.amount); @@ -305,7 +305,7 @@ void SendCoinsDialog::on_sendButton_clicked() questionString.append("
"); CAmount totalAmount = currentTransaction.getTotalTransactionAmount() + txFee; QStringList alternativeUnits; - foreach(BitcoinUnits::Unit u, BitcoinUnits::availableUnits()) + Q_FOREACH(BitcoinUnits::Unit u, BitcoinUnits::availableUnits()) { if(u != model->getOptionsModel()->getDisplayUnit()) alternativeUnits.append(BitcoinUnits::formatHtmlWithUnit(u, totalAmount)); diff --git a/src/qt/trafficgraphwidget.cpp b/src/qt/trafficgraphwidget.cpp index 0b2eb9eaf2e05..9b67445bc0f42 100644 --- a/src/qt/trafficgraphwidget.cpp +++ b/src/qt/trafficgraphwidget.cpp @@ -139,10 +139,10 @@ void TrafficGraphWidget::updateRates() } float tmax = 0.0f; - foreach(float f, vSamplesIn) { + Q_FOREACH(float f, vSamplesIn) { if(f > tmax) tmax = f; } - foreach(float f, vSamplesOut) { + Q_FOREACH(float f, vSamplesOut) { if(f > tmax) tmax = f; } fMax = tmax; diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp index 5662b1665751a..d7ee3d4c78f0f 100644 --- a/src/qt/transactiondesc.cpp +++ b/src/qt/transactiondesc.cpp @@ -243,14 +243,14 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, TransactionReco strHTML += "" + tr("Transaction ID") + ": " + TransactionRecord::formatSubTxId(wtx.GetHash(), rec->idx) + "
"; // Message from normal bitcoin:URI (bitcoin:123...?message=example) - foreach (const PAIRTYPE(string, string)& r, wtx.vOrderForm) + Q_FOREACH (const PAIRTYPE(string, string)& r, wtx.vOrderForm) if (r.first == "Message") strHTML += "
" + tr("Message") + ":
" + GUIUtil::HtmlEscape(r.second, true) + "
"; // // PaymentRequest info: // - foreach (const PAIRTYPE(string, string)& r, wtx.vOrderForm) + Q_FOREACH (const PAIRTYPE(string, string)& r, wtx.vOrderForm) { if (r.first == "PaymentRequest") { diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 34464b4075e5d..03104f38dc318 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -142,7 +142,7 @@ class TransactionTablePriv { parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex+toInsert.size()-1); int insert_idx = lowerIndex; - foreach(const TransactionRecord &rec, toInsert) + Q_FOREACH(const TransactionRecord &rec, toInsert) { cachedWallet.insert(insert_idx, rec); insert_idx += 1; diff --git a/src/qt/utilitydialog.cpp b/src/qt/utilitydialog.cpp index 386cf31d73685..5e26f3e01b6bb 100644 --- a/src/qt/utilitydialog.cpp +++ b/src/qt/utilitydialog.cpp @@ -84,7 +84,7 @@ HelpMessageDialog::HelpMessageDialog(QWidget *parent, bool about) : QTextCharFormat bold; bold.setFontWeight(QFont::Bold); - foreach (const QString &line, coreOptions.split("\n")) { + Q_FOREACH (const QString &line, coreOptions.split("\n")) { if (line.startsWith(" -")) { cursor.currentTable()->appendRows(1); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 9b8be76bebfa9..eca4f09e5888b 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -205,7 +205,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact int nAddresses = 0; // Pre-check input data for validity - foreach(const SendCoinsRecipient &rcp, recipients) + Q_FOREACH(const SendCoinsRecipient &rcp, recipients) { if (rcp.fSubtractFeeFromAmount) fSubtractFeeFromAmount = true; @@ -306,7 +306,7 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran LOCK2(cs_main, wallet->cs_wallet); CWalletTx *newTx = transaction.getTransaction(); - foreach(const SendCoinsRecipient &rcp, transaction.getRecipients()) + Q_FOREACH(const SendCoinsRecipient &rcp, transaction.getRecipients()) { if (rcp.paymentRequest.IsInitialized()) { @@ -337,7 +337,7 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran // Add addresses / update labels that we've sent to to the address book, // and emit coinsSent signal for each recipient - foreach(const SendCoinsRecipient &rcp, transaction.getRecipients()) + Q_FOREACH(const SendCoinsRecipient &rcp, transaction.getRecipients()) { // Don't touch the address book when we have a payment request if (!rcp.paymentRequest.IsInitialized()) diff --git a/src/qt/walletmodeltransaction.cpp b/src/qt/walletmodeltransaction.cpp index 206bb7c77494e..6a9b2d5bd3115 100644 --- a/src/qt/walletmodeltransaction.cpp +++ b/src/qt/walletmodeltransaction.cpp @@ -81,7 +81,7 @@ void WalletModelTransaction::reassignAmounts(int nChangePosRet) CAmount WalletModelTransaction::getTotalTransactionAmount() { CAmount totalTransactionAmount = 0; - foreach(const SendCoinsRecipient &rcp, recipients) + Q_FOREACH(const SendCoinsRecipient &rcp, recipients) { totalTransactionAmount += rcp.amount; } From fbb66c0b2878c39fb0f8baf56ec7c504b7a61e43 Mon Sep 17 00:00:00 2001 From: Ashley Holman Date: Wed, 24 Jun 2015 03:32:20 -0500 Subject: [PATCH 2/5] TxMemPool: Change mapTx to a boost::multi_index_container Indexes on: - Tx Hash - Fee Rate (fee-per-kb) --- src/miner.cpp | 9 ++++--- src/rpcblockchain.cpp | 5 ++-- src/test/mempool_tests.cpp | 52 ++++++++++++++++++++++++++++++++++++++ src/txmempool.cpp | 52 ++++++++++++++++++++------------------ src/txmempool.h | 42 +++++++++++++++++++++++++++++- 5 files changed, 128 insertions(+), 32 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 5e575f45f18fe..49100b9ddb1b3 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -16,6 +16,7 @@ #include "pow.h" #include "primitives/transaction.h" #include "timedata.h" +#include "txmempool.h" #include "util.h" #include "utilmoneystr.h" #include "validationinterface.h" @@ -148,10 +149,10 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) // This vector will be sorted into a priority queue: vector vecPriority; vecPriority.reserve(mempool.mapTx.size()); - for (map::iterator mi = mempool.mapTx.begin(); + for (CTxMemPool::indexed_transaction_set::iterator mi = mempool.mapTx.begin(); mi != mempool.mapTx.end(); ++mi) { - const CTransaction& tx = mi->second.GetTx(); + const CTransaction& tx = mi->GetTx(); if (tx.IsCoinBase() || !IsFinalTx(tx, nHeight, pblock->nTime)) continue; @@ -186,7 +187,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) } mapDependers[txin.prevout.hash].push_back(porphan); porphan->setDependsOn.insert(txin.prevout.hash); - nTotalIn += mempool.mapTx[txin.prevout.hash].GetTx().vout[txin.prevout.n].nValue; + nTotalIn += mempool.mapTx.find(txin.prevout.hash)->GetTx().vout[txin.prevout.n].nValue; continue; } const CCoins* coins = view.AccessCoins(txin.prevout.hash); @@ -216,7 +217,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) porphan->feeRate = feeRate; } else - vecPriority.push_back(TxPriority(dPriority, feeRate, &mi->second.GetTx())); + vecPriority.push_back(TxPriority(dPriority, feeRate, &(mi->GetTx()))); } // Collect transactions into block diff --git a/src/rpcblockchain.cpp b/src/rpcblockchain.cpp index c2de6cb244fe8..7e087a68d1344 100644 --- a/src/rpcblockchain.cpp +++ b/src/rpcblockchain.cpp @@ -211,10 +211,9 @@ UniValue getrawmempool(const UniValue& params, bool fHelp) { LOCK(mempool.cs); UniValue o(UniValue::VOBJ); - BOOST_FOREACH(const PAIRTYPE(uint256, CTxMemPoolEntry)& entry, mempool.mapTx) + BOOST_FOREACH(const CTxMemPoolEntry& e, mempool.mapTx) { - const uint256& hash = entry.first; - const CTxMemPoolEntry& e = entry.second; + const uint256& hash = e.GetTx().GetHash(); UniValue info(UniValue::VOBJ); info.push_back(Pair("size", (int)e.GetTxSize())); info.push_back(Pair("fee", ValueFromAmount(e.GetFee()))); diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 0996e13c4850c..85971f018f393 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -101,4 +101,56 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) removed.clear(); } +BOOST_AUTO_TEST_CASE(MempoolIndexingTest) +{ + CTxMemPool pool(CFeeRate(0)); + + /* 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(), CTxMemPoolEntry(tx1, 10000LL, 0, 10.0, 1, true)); + + /* 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(), CTxMemPoolEntry(tx2, 20000LL, 0, 9.0, 1, true)); + + /* 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(), CTxMemPoolEntry(tx3, 0LL, 0, 100.0, 1, true)); + + /* 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(), CTxMemPoolEntry(tx4, 15000LL, 0, 1.0, 1, true)); + + /* 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(), CTxMemPoolEntry(tx5, 10000LL, 1, 10.0, 1, true)); + + // there should be 4 transactions in the mempool + BOOST_CHECK_EQUAL(pool.size(), 5); + + // Check the fee-rate index is in order, should be tx2, tx4, tx1, tx5, tx3 + CTxMemPool::indexed_transaction_set::nth_index<1>::type::iterator it = pool.mapTx.get<1>().begin(); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx2.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx4.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx1.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx5.GetHash().ToString()); + BOOST_CHECK_EQUAL(it++->GetTx().GetHash().ToString(), tx3.GetHash().ToString()); + BOOST_CHECK(it == pool.mapTx.get<1>().end()); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4caa5fc821b93..b2b85652d17c5 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -32,6 +32,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); nModSize = tx.CalculateModifiedSize(nTxSize); nUsageSize = tx.DynamicMemoryUsage(); + feeRate = CFeeRate(nFee, nTxSize); } CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) @@ -96,8 +97,8 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, // Used by main.cpp AcceptToMemoryPool(), which DOES do // all the appropriate checks. LOCK(cs); - mapTx[hash] = entry; - const CTransaction& tx = mapTx[hash].GetTx(); + mapTx.insert(entry); + const CTransaction& tx = mapTx.find(hash)->GetTx(); for (unsigned int i = 0; i < tx.vin.size(); i++) mapNextTx[tx.vin[i].prevout] = CInPoint(&tx, i); nTransactionsUpdated++; @@ -134,7 +135,7 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem txToRemove.pop_front(); if (!mapTx.count(hash)) continue; - const CTransaction& tx = mapTx[hash].GetTx(); + const CTransaction& tx = mapTx.find(hash)->GetTx(); if (fRecursive) { for (unsigned int i = 0; i < tx.vout.size(); i++) { std::map::iterator it = mapNextTx.find(COutPoint(hash, i)); @@ -147,8 +148,8 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem mapNextTx.erase(txin.prevout); removed.push_back(tx); - totalTxSize -= mapTx[hash].GetTxSize(); - cachedInnerUsage -= mapTx[hash].DynamicMemoryUsage(); + totalTxSize -= mapTx.find(hash)->GetTxSize(); + cachedInnerUsage -= mapTx.find(hash)->DynamicMemoryUsage(); mapTx.erase(hash); nTransactionsUpdated++; minerPolicyEstimator->removeTx(hash); @@ -161,10 +162,10 @@ void CTxMemPool::removeCoinbaseSpends(const CCoinsViewCache *pcoins, unsigned in // Remove transactions spending a coinbase which are now immature LOCK(cs); list transactionsToRemove; - for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { - const CTransaction& tx = it->second.GetTx(); + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + const CTransaction& tx = it->GetTx(); BOOST_FOREACH(const CTxIn& txin, tx.vin) { - std::map::const_iterator it2 = mapTx.find(txin.prevout.hash); + indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) continue; const CCoins *coins = pcoins->AccessCoins(txin.prevout.hash); @@ -209,8 +210,10 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned i BOOST_FOREACH(const CTransaction& tx, vtx) { uint256 hash = tx.GetHash(); - if (mapTx.count(hash)) - entries.push_back(mapTx[hash]); + + indexed_transaction_set::iterator i = mapTx.find(hash); + if (i != mapTx.end()) + entries.push_back(*i); } BOOST_FOREACH(const CTransaction& tx, vtx) { @@ -247,17 +250,17 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const LOCK(cs); list waitingOnDependants; - for (std::map::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { unsigned int i = 0; - checkTotal += it->second.GetTxSize(); - innerUsage += it->second.DynamicMemoryUsage(); - const CTransaction& tx = it->second.GetTx(); + checkTotal += it->GetTxSize(); + innerUsage += it->DynamicMemoryUsage(); + const CTransaction& tx = it->GetTx(); bool fDependsWait = false; BOOST_FOREACH(const CTxIn &txin, tx.vin) { // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's. - std::map::const_iterator it2 = mapTx.find(txin.prevout.hash); + indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash); if (it2 != mapTx.end()) { - const CTransaction& tx2 = it2->second.GetTx(); + const CTransaction& tx2 = it2->GetTx(); assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); fDependsWait = true; } else { @@ -272,7 +275,7 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const i++; } if (fDependsWait) - waitingOnDependants.push_back(&it->second); + waitingOnDependants.push_back(&(*it)); else { CValidationState state; assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, NULL)); @@ -296,8 +299,8 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const } for (std::map::const_iterator it = mapNextTx.begin(); it != mapNextTx.end(); it++) { uint256 hash = it->second.ptx->GetHash(); - map::const_iterator it2 = mapTx.find(hash); - const CTransaction& tx = it2->second.GetTx(); + indexed_transaction_set::const_iterator it2 = mapTx.find(hash); + const CTransaction& tx = it2->GetTx(); assert(it2 != mapTx.end()); assert(&tx == it->second.ptx); assert(tx.vin.size() > it->second.n); @@ -314,16 +317,16 @@ void CTxMemPool::queryHashes(vector& vtxid) LOCK(cs); vtxid.reserve(mapTx.size()); - for (map::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) - vtxid.push_back((*mi).first); + for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) + vtxid.push_back(mi->GetTx().GetHash()); } bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const { LOCK(cs); - map::const_iterator i = mapTx.find(hash); + indexed_transaction_set::const_iterator i = mapTx.find(hash); if (i == mapTx.end()) return false; - result = i->second.GetTx(); + result = i->GetTx(); return true; } @@ -429,5 +432,6 @@ bool CCoinsViewMemPool::HaveCoins(const uint256 &txid) const { size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); - return memusage::DynamicUsage(mapTx) + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + cachedInnerUsage; + // Estimate the overhead of mapTx to be 5 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 5 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + cachedInnerUsage; } diff --git a/src/txmempool.h b/src/txmempool.h index ea36ce1ad53e5..6b6b05454a4ee 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -13,6 +13,10 @@ #include "primitives/transaction.h" #include "sync.h" +#undef foreach +#include "boost/multi_index_container.hpp" +#include "boost/multi_index/ordered_index.hpp" + class CAutoFile; inline double AllowFreeThreshold() @@ -41,6 +45,7 @@ class CTxMemPoolEntry size_t nTxSize; //! ... and avoid recomputing tx size size_t nModSize; //! ... and modified size for priority size_t nUsageSize; //! ... and total memory usage + CFeeRate feeRate; //! ... and fee per kB int64_t nTime; //! Local time when entering the mempool double dPriority; //! Priority when entering the mempool unsigned int nHeight; //! Chain height when entering the mempool @@ -55,6 +60,7 @@ class CTxMemPoolEntry const CTransaction& GetTx() const { return this->tx; } double GetPriority(unsigned int currentHeight) const; CAmount GetFee() const { return nFee; } + CFeeRate GetFeeRate() const { return feeRate; } size_t GetTxSize() const { return nTxSize; } int64_t GetTime() const { return nTime; } unsigned int GetHeight() const { return nHeight; } @@ -62,6 +68,27 @@ class CTxMemPoolEntry size_t DynamicMemoryUsage() const { return nUsageSize; } }; +// extracts a TxMemPoolEntry's transaction hash +struct mempoolentry_txid +{ + typedef uint256 result_type; + result_type operator() (const CTxMemPoolEntry &entry) const + { + return entry.GetTx().GetHash(); + } +}; + +class CompareTxMemPoolEntryByFee +{ +public: + bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) + { + if (a.GetFeeRate() == b.GetFeeRate()) + return a.GetTime() < b.GetTime(); + return a.GetFeeRate() > b.GetFeeRate(); + } +}; + class CBlockPolicyEstimator; /** An inpoint - a combination of a transaction and an index n into its vin */ @@ -99,8 +126,21 @@ class CTxMemPool uint64_t cachedInnerUsage; //! sum of dynamic memory usage of all the map elements (NOT the maps themselves) public: + typedef boost::multi_index_container< + CTxMemPoolEntry, + boost::multi_index::indexed_by< + // sorted by txid + boost::multi_index::ordered_unique, + // sorted by fee rate + boost::multi_index::ordered_non_unique< + boost::multi_index::identity, + CompareTxMemPoolEntryByFee + > + > + > indexed_transaction_set; + mutable CCriticalSection cs; - std::map mapTx; + indexed_transaction_set mapTx; std::map mapNextTx; std::map > mapDeltas; From 7f27773625ed14e843e9a275befc59b329af7b35 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 10 Jul 2015 17:52:53 -0400 Subject: [PATCH 3/5] Implement on-the-fly mempool size limitation. --- src/init.cpp | 1 + src/main.cpp | 33 +++++++++++--- src/main.h | 2 + src/memusage.h | 12 +++++ src/txmempool.cpp | 110 +++++++++++++++++++++++++++++++++++++++++----- src/txmempool.h | 15 ++++++- 6 files changed, 156 insertions(+), 17 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 4addc663c88b5..e41a282c8e852 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -283,6 +283,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-dbcache=", strprintf(_("Set database cache size in megabytes (%d to %d, default: %d)"), nMinDbCache, nMaxDbCache, nDefaultDbCache)); strUsage += HelpMessageOpt("-loadblock=", _("Imports blocks from external blk000??.dat file") + " " + _("on startup")); strUsage += HelpMessageOpt("-maxorphantx=", strprintf(_("Keep at most unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS)); + strUsage += HelpMessageOpt("-maxmempool=", strprintf(_("Keep the transaction memory pool below megabytes (default: %u)"), DEFAULT_MAX_MEMPOOL_SIZE)); strUsage += HelpMessageOpt("-par=", strprintf(_("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)"), -GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS)); #ifndef WIN32 diff --git a/src/main.cpp b/src/main.cpp index 03c09f0a279fe..584042370314c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -859,22 +859,39 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainActive.Height(), mempool.HasNoInputsOf(tx)); unsigned int nSize = entry.GetTxSize(); + // Try to make space in mempool + std::set protect; + BOOST_FOREACH(const CTxIn& in, tx.vin) { + protect.insert(in.prevout.hash); + } + std::set stagedelete; + CAmount nFeesDeleted = 0; + size_t nSizeDeleted = 0; + if (!mempool.StageTrimToSize(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, nFees, entry.GetFeeRate(), protect, stagedelete, nFeesDeleted, nSizeDeleted)) { + return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool-full"); + } + + if ((double)nFeesDeleted * nSize > (double)nFees * nSizeDeleted) { + // The new transaction's feerate is below that of the replaced transactions. + return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool-full-no-improve"); + } + // Don't accept it if it can't get into a block CAmount txMinFee = GetMinRelayFee(tx, nSize, true); - if (fLimitFree && nFees < txMinFee) + if (fLimitFree && nFees - nFeesDeleted < txMinFee) return state.DoS(0, error("AcceptToMemoryPool: not enough fees %s, %d < %d", hash.ToString(), nFees, txMinFee), REJECT_INSUFFICIENTFEE, "insufficient fee"); // Require that free transactions have sufficient priority to be mined in the next block. - if (GetBoolArg("-relaypriority", true) && nFees < ::minRelayTxFee.GetFee(nSize) && !AllowFree(view.GetPriority(tx, chainActive.Height() + 1))) { + if (GetBoolArg("-relaypriority", true) && nFees - nFeesDeleted < ::minRelayTxFee.GetFee(nSize) && !AllowFree(view.GetPriority(tx, chainActive.Height() + 1))) { return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "insufficient priority"); } // Continuously rate-limit free (really, very-low-fee) transactions // This mitigates 'penny-flooding' -- sending thousands of free transactions just to // be annoying or make others' transactions take longer to confirm. - if (fLimitFree && nFees < ::minRelayTxFee.GetFee(nSize)) + if (fLimitFree && nFees - nFeesDeleted < ::minRelayTxFee.GetFee(nSize)) { static CCriticalSection csFreeLimiter; static double dFreeCount; @@ -891,8 +908,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa if (dFreeCount >= GetArg("-limitfreerelay", 15)*10*1000) return state.DoS(0, error("AcceptToMemoryPool: free transaction rejected by rate limiter"), REJECT_INSUFFICIENTFEE, "rate limited free transaction"); - LogPrint("mempool", "Rate limit dFreeCount: %g => %g\n", dFreeCount, dFreeCount+nSize); - dFreeCount += nSize; + LogPrint("mempool", "Rate limit dFreeCount: %g => %g\n", dFreeCount, dFreeCount+nSize+nSizeDeleted); + dFreeCount += nSize+nSizeDeleted; } if (fRejectAbsurdFee && nFees > ::minRelayTxFee.GetFee(nSize) * 10000) @@ -921,6 +938,12 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return error("AcceptToMemoryPool: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s", hash.ToString()); } + // Make actually space + if (!stagedelete.empty()) { + LogPrint("mempool", "Removing %u transactions (%d fees, %d bytes) from the mempool to make space for %s\n", stagedelete.size(), nFeesDeleted, nSizeDeleted, tx.GetHash().ToString()); + pool.RemoveStaged(stagedelete); + } + // Store transaction in memory pool.addUnchecked(hash, entry, !IsInitialBlockDownload()); } diff --git a/src/main.h b/src/main.h index ce18bd709f592..58fc495e756eb 100644 --- a/src/main.h +++ b/src/main.h @@ -50,6 +50,8 @@ struct CNodeStateStats; static const bool DEFAULT_ALERTS = true; /** Default for -maxorphantx, maximum number of orphan transactions kept in memory */ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100; +/** Default for -maxmempool, maximum megabytes of mempool memory usage */ +static const unsigned int DEFAULT_MAX_MEMPOOL_SIZE = 300; /** The maximum size of a blk?????.dat file (since 0.8) */ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB /** The pre-allocation chunk size for blk?????.dat files (since 0.8) */ diff --git a/src/memusage.h b/src/memusage.h index 8a52812a7c4e6..1e83aa6fb6c94 100644 --- a/src/memusage.h +++ b/src/memusage.h @@ -105,6 +105,12 @@ static inline size_t DynamicUsage(const std::set& s) return MallocUsage(sizeof(stl_tree_node)) * s.size(); } +template +static inline size_t IncrementalDynamicUsage(const std::set& s) +{ + return MallocUsage(sizeof(stl_tree_node)); +} + template static inline size_t RecursiveDynamicUsage(const std::set& v) { @@ -121,6 +127,12 @@ static inline size_t DynamicUsage(const std::map& m) return MallocUsage(sizeof(stl_tree_node >)) * m.size(); } +template +static inline size_t IncrementalDynamicUsage(const std::map& m) +{ + return MallocUsage(sizeof(stl_tree_node >)); +} + template static inline size_t RecursiveDynamicUsage(const std::map& v) { diff --git a/src/txmempool.cpp b/src/txmempool.cpp index b2b85652d17c5..b4eb05b3adb32 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -49,9 +49,10 @@ CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const return dResult; } -CTxMemPool::CTxMemPool(const CFeeRate& _minRelayFee) : - nTransactionsUpdated(0) +CTxMemPool::CTxMemPool(const CFeeRate& _minRelayFee) { + clear(); + // Sanity checks off by default for performance, because otherwise // accepting transactions becomes O(N^2) where N is the number // of transactions in the pool @@ -109,6 +110,19 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, return true; } +void CTxMemPool::removeUnchecked(const uint256& hash) +{ + indexed_transaction_set::iterator it = mapTx.find(hash); + + BOOST_FOREACH(const CTxIn& txin, it->GetTx().vin) + mapNextTx.erase(txin.prevout); + + totalTxSize -= it->GetTxSize(); + cachedInnerUsage -= it->DynamicMemoryUsage(); + mapTx.erase(it); + nTransactionsUpdated++; + minerPolicyEstimator->removeTx(hash); +} void CTxMemPool::remove(const CTransaction &origTx, std::list& removed, bool fRecursive) { @@ -144,15 +158,8 @@ void CTxMemPool::remove(const CTransaction &origTx, std::list& rem txToRemove.push_back(it->second.ptx->GetHash()); } } - BOOST_FOREACH(const CTxIn& txin, tx.vin) - mapNextTx.erase(txin.prevout); - removed.push_back(tx); - totalTxSize -= mapTx.find(hash)->GetTxSize(); - cachedInnerUsage -= mapTx.find(hash)->DynamicMemoryUsage(); - mapTx.erase(hash); - nTransactionsUpdated++; - minerPolicyEstimator->removeTx(hash); + removeUnchecked(hash); } } } @@ -435,3 +442,86 @@ size_t CTxMemPool::DynamicMemoryUsage() const { // Estimate the overhead of mapTx to be 5 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 5 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + cachedInnerUsage; } + +size_t CTxMemPool::GuessDynamicMemoryUsage(const CTxMemPoolEntry& entry) const { + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 5 * sizeof(void*)) + entry.DynamicMemoryUsage() + memusage::IncrementalDynamicUsage(mapNextTx) * entry.GetTx().vin.size(); +} + +bool CTxMemPool::StageTrimToSize(size_t sizelimit, const CAmount& maxfeeremove, const CFeeRate& repfeerate, const std::set& prot, std::set& stage, CAmount& totalfeeremoved, size_t& totalsizeremoved) { + size_t expsize = DynamicMemoryUsage(); // Track the expected resulting memory usage of the mempool. + indexed_transaction_set::nth_index<1>::type::reverse_iterator it = mapTx.get<1>().rbegin(); + int fails = 0; // Number of mempool transactions iterated over that were not included in the stage. + // Iterate from lowest feerate to highest feerate in the mempool: + while (expsize > sizelimit && it != mapTx.get<1>().rend()) { + const uint256& hash = it->GetTx().GetHash(); + if (stage.count(hash)) { + // If the transaction is already staged for deletion, we know its descendants are already processed, so skip it. + it++; + continue; + } + if (it->GetFeeRate() > repfeerate) { + // If the transaction's feerate is worse than what we're looking for, we have processed everything in the mempool + // that could improve the staged set. If we don't have an acceptable solution by now, bail out. + return false; + } + std::deque todo; // List of hashes that we still need to process (descendants of 'hash'). + std::set now; // Set of hashes that will need to be added to stage if 'hash' is included. + CAmount nowfee = 0; // Sum of the fees in 'now'. + size_t nowsize = 0; // Sum of the tx sizes in 'now'. + size_t nowusage = 0; // Sum of the memory usages of transactions in 'now'. + int iternow = 0; // Transactions we've inspected so far while determining whether 'hash' is acceptable. + todo.push_back(it->GetTx().GetHash()); // Add 'hash' to the todo list, to initiate processing its children. + bool good = true; // Whether including 'hash' (and all its descendants) is a good idea. + // Iterate breadth-first over all descendants of transaction with hash 'hash'. + while (!todo.empty()) { + uint256 hashnow = todo.front(); + if (prot.count(hashnow)) { + // If this transaction is in the protected set, we're done with 'hash'. + good = false; + break; + } + iternow++; // We only count transactions we actually had to go find in the mempool. + const CTxMemPoolEntry* origTx = &*mapTx.find(hashnow); + nowfee += origTx->GetFee(); + if (totalfeeremoved + nowfee > maxfeeremove) { + // If this pushes up to the total fees deleted too high, we're done with 'hash'. + good = false; + break; + } + todo.pop_front(); + // Add 'hashnow' to the 'now' set, and update its statistics. + now.insert(hashnow); + nowusage += GuessDynamicMemoryUsage(*origTx); + nowsize += origTx->GetTxSize(); + // Find dependencies of 'hashnow' and them to todo. + std::map::iterator iter = mapNextTx.lower_bound(COutPoint(hashnow, 0)); + while (iter != mapNextTx.end() && iter->first.hash == hashnow) { + const uint256& nexthash = iter->second.ptx->GetHash(); + if (!(stage.count(nexthash) || now.count(nexthash))) { + todo.push_back(nexthash); + } + iter++; + } + } + if (good) { + stage.insert(now.begin(), now.end()); + totalfeeremoved += nowfee; + totalsizeremoved += nowsize; + expsize -= nowusage; + } else { + fails += iternow; + if (fails == 20) { + // Bail out after traversing 20 transactions that are not acceptable. + return false; + } + } + it++; + } + return true; +} + +void CTxMemPool::RemoveStaged(std::set& stage) { + BOOST_FOREACH(const uint256& hash, stage) { + removeUnchecked(hash); + } +} diff --git a/src/txmempool.h b/src/txmempool.h index 6b6b05454a4ee..2e19ff32bc1f5 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -59,8 +59,8 @@ class CTxMemPoolEntry const CTransaction& GetTx() const { return this->tx; } double GetPriority(unsigned int currentHeight) const; - CAmount GetFee() const { return nFee; } - CFeeRate GetFeeRate() const { return feeRate; } + const CAmount& GetFee() const { return nFee; } + const CFeeRate& GetFeeRate() const { return feeRate; } size_t GetTxSize() const { return nTxSize; } int64_t GetTime() const { return nTime; } unsigned int GetHeight() const { return nHeight; } @@ -157,6 +157,7 @@ class CTxMemPool void setSanityCheck(bool _fSanityCheck) { fSanityCheck = _fSanityCheck; } bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, bool fCurrentEstimate = true); + void removeUnchecked(const uint256& hash); void remove(const CTransaction &tx, std::list& removed, bool fRecursive = false); void removeCoinbaseSpends(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight); void removeConflicts(const CTransaction &tx, std::list& removed); @@ -178,6 +179,15 @@ class CTxMemPool void ApplyDeltas(const uint256 hash, double &dPriorityDelta, CAmount &nFeeDelta); void ClearPrioritisation(const uint256 hash); + /** Build a list of transaction (hashes) to remove such that: + * - The list is consistent (if a parent is included, all its dependencies are included as well). + * - Removing said list will reduce the DynamicMemoryUsage below sizelimit. + * - At most maxfeeremove worth of fees will be removed. + * - No transaction whose hash is in prot will be removed. + */ + bool StageTrimToSize(size_t sizelimit, const CAmount& maxfeeremove, const CFeeRate& repfeerate, const std::set& prot, std::set& stage, CAmount& totalfeeremoved, size_t& totalsizeremoved); + void RemoveStaged(std::set& stage); + unsigned long size() { LOCK(cs); @@ -209,6 +219,7 @@ class CTxMemPool bool ReadFeeEstimates(CAutoFile& filein); size_t DynamicMemoryUsage() const; + size_t GuessDynamicMemoryUsage(const CTxMemPoolEntry& entry) const; }; /** From 0a917df8e2c91a58fa8ed2667c867e27162b62e6 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 11 Jul 2015 16:49:11 +0200 Subject: [PATCH 4/5] Move orphan tx handling to a separate log class --- src/main.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 584042370314c..c2a0b4bc1bad4 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -543,7 +543,7 @@ bool AddOrphanTx(const CTransaction& tx, NodeId peer) unsigned int sz = tx.GetSerializeSize(SER_NETWORK, CTransaction::CURRENT_VERSION); if (sz > 5000) { - LogPrint("mempool", "ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString()); + LogPrint("orphan", "ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString()); return false; } @@ -552,7 +552,7 @@ bool AddOrphanTx(const CTransaction& tx, NodeId peer) BOOST_FOREACH(const CTxIn& txin, tx.vin) mapOrphanTransactionsByPrev[txin.prevout.hash].insert(hash); - LogPrint("mempool", "stored orphan tx %s (mapsz %u prevsz %u)\n", hash.ToString(), + LogPrint("orphan", "stored orphan tx %s (mapsz %u prevsz %u)\n", hash.ToString(), mapOrphanTransactions.size(), mapOrphanTransactionsByPrev.size()); return true; } @@ -587,7 +587,7 @@ void EraseOrphansFor(NodeId peer) ++nErased; } } - if (nErased > 0) LogPrint("mempool", "Erased %d orphan tx from peer %d\n", nErased, peer); + if (nErased > 0) LogPrint("orphan", "Erased %d orphan tx from peer %d\n", nErased, peer); } @@ -4263,7 +4263,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, continue; if (AcceptToMemoryPool(mempool, stateDummy, orphanTx, true, &fMissingInputs2)) { - LogPrint("mempool", " accepted orphan tx %s\n", orphanHash.ToString()); + LogPrint("orphan", " accepted orphan tx %s\n", orphanHash.ToString()); RelayTransaction(orphanTx); vWorkQueue.push_back(orphanHash); vEraseQueue.push_back(orphanHash); @@ -4276,11 +4276,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // Punish peer that gave us an invalid orphan tx Misbehaving(fromPeer, nDos); setMisbehaving.insert(fromPeer); - LogPrint("mempool", " invalid orphan tx %s\n", orphanHash.ToString()); + LogPrint("orphan", " invalid orphan tx %s\n", orphanHash.ToString()); } // Has inputs but not accepted to mempool // Probably non-standard or insufficient fee/priority - LogPrint("mempool", " removed orphan tx %s\n", orphanHash.ToString()); + LogPrint("orphan", " removed orphan tx %s\n", orphanHash.ToString()); vEraseQueue.push_back(orphanHash); } mempool.check(pcoinsTip); @@ -4298,7 +4298,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, GetArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS)); unsigned int nEvicted = LimitOrphanTxSize(nMaxOrphanTx); if (nEvicted > 0) - LogPrint("mempool", "mapOrphan overflow, removed %u tx\n", nEvicted); + LogPrint("orphan", "mapOrphan overflow, removed %u tx\n", nEvicted); } else if (pfrom->fWhitelisted) { // Always relay transactions received from whitelisted peers, even // if they are already in the mempool (allowing the node to function From d455fe8a35fa7b13a7b600daba9428262b17aac2 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Mon, 13 Jul 2015 08:20:56 -0400 Subject: [PATCH 5/5] minor: nits --- src/txmempool.cpp | 2 +- src/txmempool.h | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index b4eb05b3adb32..f0a2f1fedc7c1 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -461,7 +461,7 @@ bool CTxMemPool::StageTrimToSize(size_t sizelimit, const CAmount& maxfeeremove, } if (it->GetFeeRate() > repfeerate) { // If the transaction's feerate is worse than what we're looking for, we have processed everything in the mempool - // that could improve the staged set. If we don't have an acceptable solution by now, bail out. + // that could improve the staged set. We don't have an acceptable solution, so bail out. return false; } std::deque todo; // List of hashes that we still need to process (descendants of 'hash'). diff --git a/src/txmempool.h b/src/txmempool.h index 2e19ff32bc1f5..00b2b8e94f453 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -183,6 +183,7 @@ class CTxMemPool * - The list is consistent (if a parent is included, all its dependencies are included as well). * - Removing said list will reduce the DynamicMemoryUsage below sizelimit. * - At most maxfeeremove worth of fees will be removed. + * - No transaction with fee-rate >= repfeerate will be removed. * - No transaction whose hash is in prot will be removed. */ bool StageTrimToSize(size_t sizelimit, const CAmount& maxfeeremove, const CFeeRate& repfeerate, const std::set& prot, std::set& stage, CAmount& totalfeeremoved, size_t& totalsizeremoved); @@ -219,6 +220,11 @@ class CTxMemPool bool ReadFeeEstimates(CAutoFile& filein); size_t DynamicMemoryUsage() const; + + /** + * Guess the incremental dynamic mem usage for a single entry, assuming we + * were to add/remove it from the mempool. + */ size_t GuessDynamicMemoryUsage(const CTxMemPoolEntry& entry) const; };