diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/core_memusage.h | 5 | ||||
-rw-r--r-- | src/init.cpp | 9 | ||||
-rw-r--r-- | src/net.cpp | 8 | ||||
-rw-r--r-- | src/net.h | 4 | ||||
-rw-r--r-- | src/net_processing.cpp | 8 | ||||
-rw-r--r-- | src/policy/fees.cpp | 6 | ||||
-rw-r--r-- | src/policy/fees.h | 2 | ||||
-rw-r--r-- | src/qt/test/wallettests.cpp | 93 | ||||
-rw-r--r-- | src/qt/transactionview.cpp | 7 | ||||
-rw-r--r-- | src/script/interpreter.cpp | 14 | ||||
-rw-r--r-- | src/script/script.h | 3 | ||||
-rw-r--r-- | src/txdb.cpp | 6 | ||||
-rw-r--r-- | src/txmempool.cpp | 2 | ||||
-rw-r--r-- | src/txmempool.h | 99 | ||||
-rw-r--r-- | src/util.h | 3 | ||||
-rw-r--r-- | src/validation.cpp | 159 | ||||
-rw-r--r-- | src/validation.h | 4 | ||||
-rw-r--r-- | src/wallet/feebumper.cpp | 2 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 2 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 18 |
20 files changed, 353 insertions, 101 deletions
diff --git a/src/core_memusage.h b/src/core_memusage.h index 5e10182075..e4ccd54c42 100644 --- a/src/core_memusage.h +++ b/src/core_memusage.h @@ -63,4 +63,9 @@ static inline size_t RecursiveDynamicUsage(const CBlockLocator& locator) { return memusage::DynamicUsage(locator.vHave); } +template<typename X> +static inline size_t RecursiveDynamicUsage(const std::shared_ptr<X>& p) { + return p ? memusage::DynamicUsage(p) + RecursiveDynamicUsage(*p) : 0; +} + #endif // BITCOIN_CORE_MEMUSAGE_H diff --git a/src/init.cpp b/src/init.cpp index 3bbdb16c3b..f343f1d965 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1386,11 +1386,6 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) } } - if (gArgs.IsArgSet("-seednode")) { - BOOST_FOREACH(const std::string& strDest, gArgs.GetArgs("-seednode")) - connman.AddOneShot(strDest); - } - #if ENABLE_ZMQ pzmqNotificationInterface = CZMQNotificationInterface::Create(); @@ -1659,6 +1654,10 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) connOptions.nMaxOutboundTimeframe = nMaxOutboundTimeframe; connOptions.nMaxOutboundLimit = nMaxOutboundLimit; + if (gArgs.IsArgSet("-seednode")) { + connOptions.vSeedNodes = gArgs.GetArgs("-seednode"); + } + if (!connman.Start(scheduler, strNodeError, connOptions)) return InitError(strNodeError); diff --git a/src/net.cpp b/src/net.cpp index 198d8f5fff..bac9b5732a 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1743,9 +1743,9 @@ void CConnman::ThreadOpenConnections() // * Increase the number of connectable addresses in the tried table. // // Method: - // * Choose a random address from new and attempt to connect to it if we can connect + // * Choose a random address from new and attempt to connect to it if we can connect // successfully it is added to tried. - // * Start attempting feeler connections only after node finishes making outbound + // * Start attempting feeler connections only after node finishes making outbound // connections. // * Only make a feeler connection once every few minutes. // @@ -2212,6 +2212,10 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c SetBestHeight(connOptions.nBestHeight); + for (const auto& strDest : connOptions.vSeedNodes) { + AddOneShot(strDest); + } + clientInterface = connOptions.uiInterface; if (clientInterface) { clientInterface->InitMessage(_("Loading P2P addresses...")); @@ -144,6 +144,7 @@ public: unsigned int nReceiveFloodSize = 0; uint64_t nMaxOutboundTimeframe = 0; uint64_t nMaxOutboundLimit = 0; + std::vector<std::string> vSeedNodes; }; CConnman(uint64_t seed0, uint64_t seed1); ~CConnman(); @@ -233,8 +234,6 @@ public: void GetBanned(banmap_t &banmap); void SetBanned(const banmap_t &banmap); - void AddOneShot(const std::string& strDest); - bool AddNode(const std::string& node); bool RemoveAddedNode(const std::string& node); std::vector<AddedNodeInfo> GetAddedNodeInfo(); @@ -292,6 +291,7 @@ private: }; void ThreadOpenAddedConnections(); + void AddOneShot(const std::string& strDest); void ProcessOneShot(); void ThreadOpenConnections(); void ThreadMessageHandler(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index aca88f2660..3102c2ef9a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -754,8 +754,8 @@ void PeerLogicValidation::BlockConnected(const std::shared_ptr<const CBlock>& pb const CTransaction& tx = *ptx; // Which orphan pool entries must we evict? - for (size_t j = 0; j < tx.vin.size(); j++) { - auto itByPrev = mapOrphanTransactionsByPrev.find(tx.vin[j].prevout); + for (const auto& txin : tx.vin) { + auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout); if (itByPrev == mapOrphanTransactionsByPrev.end()) continue; for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) { const CTransaction& orphanTx = *(*mi)->second.tx; @@ -1553,10 +1553,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr uint32_t nFetchFlags = GetFetchFlags(pfrom); - for (unsigned int nInv = 0; nInv < vInv.size(); nInv++) + for (CInv &inv : vInv) { - CInv &inv = vInv[nInv]; - if (interruptMsgProc) return true; diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index bad3de4b44..66083fe1ee 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -604,8 +604,8 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight, unsigned int countedTxs = 0; // Update averages with data points from current block - for (unsigned int i = 0; i < entries.size(); i++) { - if (processBlockTx(nBlockHeight, entries[i])) + for (const auto& entry : entries) { + if (processBlockTx(nBlockHeight, entry)) countedTxs++; } @@ -786,7 +786,7 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun * This is necessary to preserve monotonically increasing estimates. * For non-conservative estimates we do the same thing for 2*target, but * for conservative estimates we want to skip these shorter horizons - * checks for 2*target becuase we are taking the max over all time + * checks for 2*target because we are taking the max over all time * horizons so we already have monotonically increasing estimates and * the purpose of conservative estimates is not to let short term * fluctuations lower our estimates by too much. diff --git a/src/policy/fees.h b/src/policy/fees.h index f527c85004..e99fec2c39 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -51,7 +51,7 @@ class TxConfirmStats; * blocks. This is is tracked in 3 different data sets each up to a maximum * number of periods. Within each data set we have an array of counters in each * feerate bucket and we increment all the counters from Y' up to max periods - * representing that a tx was successfullly confirmed in less than or equal to + * representing that a tx was successfully confirmed in less than or equal to * that many periods. We want to save a history of this information, so at any * time we have a counter of the total number of transactions that happened in a * given feerate bucket and the total number that were confirmed in each of the diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 798d333d63..0e12a9d53e 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -8,26 +8,46 @@ #include "qt/sendcoinsdialog.h" #include "qt/sendcoinsentry.h" #include "qt/transactiontablemodel.h" +#include "qt/transactionview.h" #include "qt/walletmodel.h" #include "test/test_bitcoin.h" #include "validation.h" #include "wallet/wallet.h" #include <QAbstractButton> +#include <QAction> #include <QApplication> +#include <QCheckBox> +#include <QPushButton> #include <QTimer> #include <QVBoxLayout> namespace { -//! Press "Yes" button in modal send confirmation dialog. -void ConfirmSend() +//! Press "Ok" button in message box dialog. +void ConfirmMessage(QString* text = nullptr) { - QTimer::singleShot(0, makeCallback([](Callback* callback) { + QTimer::singleShot(0, makeCallback([text](Callback* callback) { + for (QWidget* widget : QApplication::topLevelWidgets()) { + if (widget->inherits("QMessageBox")) { + QMessageBox* messageBox = qobject_cast<QMessageBox*>(widget); + if (text) *text = messageBox->text(); + messageBox->defaultButton()->click(); + } + } + delete callback; + }), SLOT(call())); +} + +//! Press "Yes" or "Cancel" buttons in modal send confirmation dialog. +void ConfirmSend(QString* text = nullptr, bool cancel = false) +{ + QTimer::singleShot(0, makeCallback([text, cancel](Callback* callback) { for (QWidget* widget : QApplication::topLevelWidgets()) { if (widget->inherits("SendConfirmationDialog")) { SendConfirmationDialog* dialog = qobject_cast<SendConfirmationDialog*>(widget); - QAbstractButton* button = dialog->button(QMessageBox::Yes); + if (text) *text = dialog->text(); + QAbstractButton* button = dialog->button(cancel ? QMessageBox::Cancel : QMessageBox::Yes); button->setEnabled(true); button->click(); } @@ -37,12 +57,16 @@ void ConfirmSend() } //! Send coins to address and return txid. -uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CBitcoinAddress& address, CAmount amount) +uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CBitcoinAddress& address, CAmount amount, bool rbf) { QVBoxLayout* entries = sendCoinsDialog.findChild<QVBoxLayout*>("entries"); SendCoinsEntry* entry = qobject_cast<SendCoinsEntry*>(entries->itemAt(0)->widget()); entry->findChild<QValidatedLineEdit*>("payTo")->setText(QString::fromStdString(address.ToString())); entry->findChild<BitcoinAmountField*>("payAmount")->setValue(amount); + sendCoinsDialog.findChild<QFrame*>("frameFee") + ->findChild<QFrame*>("frameFeeSelection") + ->findChild<QCheckBox*>("optInRBF") + ->setCheckState(rbf ? Qt::Checked : Qt::Unchecked); uint256 txid; boost::signals2::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](CWallet*, const uint256& hash, ChangeType status) { if (status == CT_NEW) txid = hash; @@ -66,6 +90,43 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid) return {}; } +//! Request context menu (call method that is public in qt5, but protected in qt4). +void RequestContextMenu(QWidget* widget) +{ + class Qt4Hack : public QWidget + { + public: + using QWidget::customContextMenuRequested; + }; + static_cast<Qt4Hack*>(widget)->customContextMenuRequested({}); +} + +//! Invoke bumpfee on txid and check results. +void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, std::string expectError, bool cancel) +{ + QTableView* table = view.findChild<QTableView*>("transactionView"); + QModelIndex index = FindTx(*table->selectionModel()->model(), txid); + QVERIFY2(index.isValid(), "Could not find BumpFee txid"); + + // Select row in table, invoke context menu, and make sure bumpfee action is + // enabled or disabled as expected. + QAction* action = view.findChild<QAction*>("bumpFeeAction"); + table->selectionModel()->select(index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + action->setEnabled(expectDisabled); + RequestContextMenu(table); + QCOMPARE(action->isEnabled(), !expectDisabled); + + action->setEnabled(true); + QString text; + if (expectError.empty()) { + ConfirmSend(&text, cancel); + } else { + ConfirmMessage(&text); + } + action->trigger(); + QVERIFY(text.indexOf(QString::fromStdString(expectError)) != -1); +} + //! Simple qt wallet tests. // // Test widgets can be debugged interactively calling show() on them and @@ -81,9 +142,11 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid) // src/qt/test/test_bitcoin-qt -platform cocoa # macOS void TestSendCoins() { - // Set up wallet and chain with 101 blocks (1 mature block for spending). + // Set up wallet and chain with 105 blocks (5 mature blocks for spending). TestChain100Setup test; - test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); + for (int i = 0; i < 5; ++i) { + test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); + } bitdb.MakeMock(); std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); CWallet wallet(std::move(dbw)); @@ -100,19 +163,27 @@ void TestSendCoins() // Create widgets for sending coins and listing transactions. std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other")); SendCoinsDialog sendCoinsDialog(platformStyle.get()); + TransactionView transactionView(platformStyle.get()); OptionsModel optionsModel; WalletModel walletModel(platformStyle.get(), &wallet, &optionsModel); sendCoinsDialog.setModel(&walletModel); + transactionView.setModel(&walletModel); // Send two transactions, and verify they are added to transaction list. TransactionTableModel* transactionTableModel = walletModel.getTransactionTableModel(); - QCOMPARE(transactionTableModel->rowCount({}), 101); - uint256 txid1 = SendCoins(wallet, sendCoinsDialog, CBitcoinAddress(CKeyID()), 5 * COIN); - uint256 txid2 = SendCoins(wallet, sendCoinsDialog, CBitcoinAddress(CKeyID()), 10 * COIN); - QCOMPARE(transactionTableModel->rowCount({}), 103); + QCOMPARE(transactionTableModel->rowCount({}), 105); + uint256 txid1 = SendCoins(wallet, sendCoinsDialog, CBitcoinAddress(CKeyID()), 5 * COIN, false /* rbf */); + uint256 txid2 = SendCoins(wallet, sendCoinsDialog, CBitcoinAddress(CKeyID()), 10 * COIN, true /* rbf */); + QCOMPARE(transactionTableModel->rowCount({}), 107); QVERIFY(FindTx(*transactionTableModel, txid1).isValid()); QVERIFY(FindTx(*transactionTableModel, txid2).isValid()); + // Call bumpfee. Test disabled, canceled, enabled, then failing cases. + BumpFee(transactionView, txid1, true /* expect disabled */, "not BIP 125 replaceable" /* expected error */, false /* cancel */); + BumpFee(transactionView, txid2, false /* expect disabled */, {} /* expected error */, true /* cancel */); + BumpFee(transactionView, txid2, false /* expect disabled */, {} /* expected error */, false /* cancel */); + BumpFee(transactionView, txid2, false /* expect disabled */, "already bumped" /* expected error */, false /* cancel */); + bitdb.Flush(true); bitdb.Reset(); } diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 9008c81634..5da10e41b9 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -136,10 +136,12 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa view->installEventFilter(this); transactionView = view; + transactionView->setObjectName("transactionView"); // Actions abandonAction = new QAction(tr("Abandon transaction"), this); bumpFeeAction = new QAction(tr("Increase transaction fee"), this); + bumpFeeAction->setObjectName("bumpFeeAction"); QAction *copyAddressAction = new QAction(tr("Copy address"), this); QAction *copyLabelAction = new QAction(tr("Copy label"), this); QAction *copyAmountAction = new QAction(tr("Copy amount"), this); @@ -150,6 +152,7 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa QAction *showDetailsAction = new QAction(tr("Show transaction details"), this); contextMenu = new QMenu(this); + contextMenu->setObjectName("contextMenu"); contextMenu->addAction(copyAddressAction); contextMenu->addAction(copyLabelAction); contextMenu->addAction(copyAmountAction); @@ -380,7 +383,7 @@ void TransactionView::contextualMenu(const QPoint &point) if(index.isValid()) { - contextMenu->exec(QCursor::pos()); + contextMenu->popup(transactionView->viewport()->mapToGlobal(point)); } } @@ -416,7 +419,7 @@ void TransactionView::bumpFee() // Bump tx fee over the walletModel if (model->bumpFee(hash)) { // Update the table - model->getTransactionTableModel()->updateTransaction(hashQStr, CT_UPDATED, false); + model->getTransactionTableModel()->updateTransaction(hashQStr, CT_UPDATED, true); } } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index f4e5313a78..222cff59ea 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1028,7 +1028,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& } // Size limits - if (stack.size() + altstack.size() > 1000) + if (stack.size() + altstack.size() > MAX_STACK_SIZE) return set_error(serror, SCRIPT_ERR_STACK_SIZE); } } @@ -1142,24 +1142,24 @@ public: uint256 GetPrevoutHash(const CTransaction& txTo) { CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vin.size(); n++) { - ss << txTo.vin[n].prevout; + for (const auto& txin : txTo.vin) { + ss << txin.prevout; } return ss.GetHash(); } uint256 GetSequenceHash(const CTransaction& txTo) { CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vin.size(); n++) { - ss << txTo.vin[n].nSequence; + for (const auto& txin : txTo.vin) { + ss << txin.nSequence; } return ss.GetHash(); } uint256 GetOutputsHash(const CTransaction& txTo) { CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vout.size(); n++) { - ss << txTo.vout[n]; + for (const auto& txout : txTo.vout) { + ss << txout; } return ss.GetHash(); } diff --git a/src/script/script.h b/src/script/script.h index d7aaa04f80..95a5999a13 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -30,6 +30,9 @@ static const int MAX_PUBKEYS_PER_MULTISIG = 20; // Maximum script length in bytes static const int MAX_SCRIPT_SIZE = 10000; +// Maximum number of values on script interpreter stack +static const int MAX_STACK_SIZE = 1000; + // Threshold for nLockTime: below this value it is interpreted as block number, // otherwise as UNIX timestamp. static const unsigned int LOCKTIME_THRESHOLD = 500000000; // Tue Nov 5 00:53:20 1985 UTC diff --git a/src/txdb.cpp b/src/txdb.cpp index 42dc31760b..76aab23983 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -98,7 +98,11 @@ CCoinsViewCursor *CCoinsViewDB::Cursor() const that restriction. */ i->pcursor->Seek(DB_COINS); // Cache key of first record - i->pcursor->GetKey(i->keyTmp); + if (i->pcursor->Valid()) { + i->pcursor->GetKey(i->keyTmp); + } else { + i->keyTmp.first = 0; // Make sure Valid() and GetKey() return false + } return i; } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 33df0536d0..852984426f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -24,7 +24,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFe spendsCoinbase(_spendsCoinbase), sigOpCost(_sigOpsCost), lockPoints(lp) { nTxWeight = GetTransactionWeight(*tx); - nUsageSize = RecursiveDynamicUsage(*tx) + memusage::DynamicUsage(tx); + nUsageSize = RecursiveDynamicUsage(tx); nCountWithDescendants = 1; nSizeWithDescendants = GetTxSize(); diff --git a/src/txmempool.h b/src/txmempool.h index a91eb5be54..671a8b596c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -24,6 +24,7 @@ #include "boost/multi_index_container.hpp" #include "boost/multi_index/ordered_index.hpp" #include "boost/multi_index/hashed_index.hpp" +#include <boost/multi_index/sequenced_index.hpp> #include <boost/signals2/signal.hpp> @@ -185,7 +186,7 @@ private: const LockPoints& lp; }; -// extracts a TxMemPoolEntry's transaction hash +// extracts a transaction hash from CTxMempoolEntry or CTransactionRef struct mempoolentry_txid { typedef uint256 result_type; @@ -193,6 +194,11 @@ struct mempoolentry_txid { return entry.GetTx().GetHash(); } + + result_type operator() (const CTransactionRef& tx) const + { + return tx->GetHash(); + } }; /** \class CompareTxMemPoolEntryByDescendantScore @@ -662,4 +668,95 @@ public: bool HaveCoins(const uint256 &txid) const; }; +/** + * DisconnectedBlockTransactions + + * During the reorg, it's desirable to re-add previously confirmed transactions + * to the mempool, so that anything not re-confirmed in the new chain is + * available to be mined. However, it's more efficient to wait until the reorg + * is complete and process all still-unconfirmed transactions at that time, + * since we expect most confirmed transactions to (typically) still be + * confirmed in the new chain, and re-accepting to the memory pool is expensive + * (and therefore better to not do in the middle of reorg-processing). + * Instead, store the disconnected transactions (in order!) as we go, remove any + * that are included in blocks in the new chain, and then process the remaining + * still-unconfirmed transactions at the end. + */ + +// multi_index tag names +struct txid_index {}; +struct insertion_order {}; + +struct DisconnectedBlockTransactions { + typedef boost::multi_index_container< + CTransactionRef, + boost::multi_index::indexed_by< + // sorted by txid + boost::multi_index::hashed_unique< + boost::multi_index::tag<txid_index>, + mempoolentry_txid, + SaltedTxidHasher + >, + // sorted by order in the blockchain + boost::multi_index::sequenced< + boost::multi_index::tag<insertion_order> + > + > + > indexed_disconnected_transactions; + + // It's almost certainly a logic bug if we don't clear out queuedTx before + // destruction, as we add to it while disconnecting blocks, and then we + // need to re-process remaining transactions to ensure mempool consistency. + // For now, assert() that we've emptied out this object on destruction. + // This assert() can always be removed if the reorg-processing code were + // to be refactored such that this assumption is no longer true (for + // instance if there was some other way we cleaned up the mempool after a + // reorg, besides draining this object). + ~DisconnectedBlockTransactions() { assert(queuedTx.empty()); } + + indexed_disconnected_transactions queuedTx; + uint64_t cachedInnerUsage = 0; + + // Estimate the overhead of queuedTx to be 6 pointers + an allocation, as + // no exact formula for boost::multi_index_contained is implemented. + size_t DynamicMemoryUsage() const { + return memusage::MallocUsage(sizeof(CTransactionRef) + 6 * sizeof(void*)) * queuedTx.size() + cachedInnerUsage; + } + + void addTransaction(const CTransactionRef& tx) + { + queuedTx.insert(tx); + cachedInnerUsage += RecursiveDynamicUsage(tx); + } + + // Remove entries based on txid_index, and update memory usage. + void removeForBlock(const std::vector<CTransactionRef>& vtx) + { + // Short-circuit in the common case of a block being added to the tip + if (queuedTx.empty()) { + return; + } + for (auto const &tx : vtx) { + auto it = queuedTx.find(tx->GetHash()); + if (it != queuedTx.end()) { + cachedInnerUsage -= RecursiveDynamicUsage(*it); + queuedTx.erase(it); + } + } + } + + // Remove an entry by insertion_order index, and update memory usage. + void removeEntry(indexed_disconnected_transactions::index<insertion_order>::type::iterator entry) + { + cachedInnerUsage -= RecursiveDynamicUsage(*entry); + queuedTx.get<insertion_order>().erase(entry); + } + + void clear() + { + cachedInnerUsage = 0; + queuedTx.clear(); + } +}; + #endif // BITCOIN_TXMEMPOOL_H diff --git a/src/util.h b/src/util.h index 229478d835..4386ddd550 100644 --- a/src/util.h +++ b/src/util.h @@ -241,7 +241,8 @@ bool SoftSetArg(const std::string& strArg, const std::string& strValue); */ bool SoftSetBoolArg(const std::string& strArg, bool fValue); -// Forces a arg setting, used only in testing +// Forces an arg setting. Called by SoftSetArg() if the arg hasn't already +// been set. Also called directly in testing. void ForceSetArg(const std::string& strArg, const std::string& strValue); }; diff --git a/src/validation.cpp b/src/validation.cpp index 89358f9603..381d1b01c2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -309,7 +309,6 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool return EvaluateSequenceLocks(index, lockPair); } - void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) { int expired = pool.Expire(GetTime() - age); if (expired != 0) { @@ -343,6 +342,56 @@ static bool IsCurrentForFeeEstimation() return true; } +/* Make mempool consistent after a reorg, by re-adding or recursively erasing + * disconnected block transactions from the mempool, and also removing any + * other transactions from the mempool that are no longer valid given the new + * tip/height. + * + * Note: we assume that disconnectpool only contains transactions that are NOT + * confirmed in the current chain nor already in the mempool (otherwise, + * in-mempool descendants of such transactions would be removed). + * + * Passing fAddToMempool=false will skip trying to add the transactions back, + * and instead just erase from the mempool as needed. + */ + +void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool fAddToMempool) +{ + AssertLockHeld(cs_main); + std::vector<uint256> vHashUpdate; + // disconnectpool's insertion_order index sorts the entries from + // oldest to newest, but the oldest entry will be the last tx from the + // latest mined block that was disconnected. + // Iterate disconnectpool in reverse, so that we add transactions + // back to the mempool starting with the earliest transaction that had + // been previously seen in a block. + auto it = disconnectpool.queuedTx.get<insertion_order>().rbegin(); + while (it != disconnectpool.queuedTx.get<insertion_order>().rend()) { + // ignore validation errors in resurrected transactions + CValidationState stateDummy; + if (!fAddToMempool || (*it)->IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, *it, false, NULL, NULL, true)) { + // If the transaction doesn't make it in to the mempool, remove any + // transactions that depend on it (which would now be orphans). + mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); + } else if (mempool.exists((*it)->GetHash())) { + vHashUpdate.push_back((*it)->GetHash()); + } + ++it; + } + disconnectpool.queuedTx.clear(); + // AcceptToMemoryPool/addUnchecked all assume that new mempool entries have + // no in-mempool children, which is generally not true when adding + // previously-confirmed transactions back to the mempool. + // UpdateTransactionsFromBlock finds descendants of any transactions in + // the disconnectpool that were added back and cleans up the mempool state. + mempool.UpdateTransactionsFromBlock(vHashUpdate); + + // We also need to remove any now-immature transactions + mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); + // Re-limit mempool size, in case we added any transactions + LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); +} + bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, bool fLimitFree, bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced, bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector<uint256>& vHashTxnToUncache) @@ -1815,6 +1864,16 @@ void PruneAndFlush() { FlushStateToDisk(state, FLUSH_STATE_NONE); } +static void DoWarning(const std::string& strWarning) +{ + static bool fWarned = false; + SetMiscWarning(strWarning); + if (!fWarned) { + AlertNotify(strWarning); + fWarned = true; + } +} + /** Update chainActive and related internal data structures. */ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { chainActive.SetTip(pindexNew); @@ -1824,7 +1883,6 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { cvBlockChange.notify_all(); - static bool fWarned = false; std::vector<std::string> warningMessages; if (!IsInitialBlockDownload()) { @@ -1834,15 +1892,11 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { WarningBitsConditionChecker checker(bit); ThresholdState state = checker.GetStateFor(pindex, chainParams.GetConsensus(), warningcache[bit]); if (state == THRESHOLD_ACTIVE || state == THRESHOLD_LOCKED_IN) { + const std::string strWarning = strprintf(_("Warning: unknown new rules activated (versionbit %i)"), bit); if (state == THRESHOLD_ACTIVE) { - std::string strWarning = strprintf(_("Warning: unknown new rules activated (versionbit %i)"), bit); - SetMiscWarning(strWarning); - if (!fWarned) { - AlertNotify(strWarning); - fWarned = true; - } + DoWarning(strWarning); } else { - warningMessages.push_back(strprintf("unknown new rules are about to activate (versionbit %i)", bit)); + warningMessages.push_back(strWarning); } } } @@ -1855,16 +1909,12 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { pindex = pindex->pprev; } if (nUpgraded > 0) - warningMessages.push_back(strprintf("%d of last 100 blocks have unexpected version", nUpgraded)); + warningMessages.push_back(strprintf(_("%d of last 100 blocks have unexpected version"), nUpgraded)); if (nUpgraded > 100/2) { std::string strWarning = _("Warning: Unknown block versions being mined! It's possible unknown rules are in effect"); // notify GetWarnings(), called by Qt and the JSON-RPC code to warn the user: - SetMiscWarning(strWarning); - if (!fWarned) { - AlertNotify(strWarning); - fWarned = true; - } + DoWarning(strWarning); } } LogPrintf("%s: new best=%s height=%d version=0x%08x log2_work=%.8g tx=%lu date='%s' progress=%f cache=%.1fMiB(%utx)", __func__, @@ -1878,8 +1928,17 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { } -/** Disconnect chainActive's tip. You probably want to call mempool.removeForReorg and manually re-limit mempool size after this, with cs_main held. */ -bool static DisconnectTip(CValidationState& state, const CChainParams& chainparams, bool fBare = false) +/** Disconnect chainActive's tip. + * After calling, the mempool will be in an inconsistent state, with + * transactions from disconnected blocks being added to disconnectpool. You + * should make the mempool consistent again by calling UpdateMempoolForReorg. + * with cs_main held. + * + * If disconnectpool is NULL, then no disconnected transactions are added to + * disconnectpool (note that the caller is responsible for mempool consistency + * in any case). + */ +bool static DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions *disconnectpool) { CBlockIndex *pindexDelete = chainActive.Tip(); assert(pindexDelete); @@ -1902,25 +1961,17 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara if (!FlushStateToDisk(state, FLUSH_STATE_IF_NEEDED)) return false; - if (!fBare) { - // Resurrect mempool transactions from the disconnected block. - std::vector<uint256> vHashUpdate; - for (const auto& it : block.vtx) { - const CTransaction& tx = *it; - // ignore validation errors in resurrected transactions - CValidationState stateDummy; - if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, it, false, NULL, NULL, true)) { - mempool.removeRecursive(tx, MemPoolRemovalReason::REORG); - } else if (mempool.exists(tx.GetHash())) { - vHashUpdate.push_back(tx.GetHash()); - } + if (disconnectpool) { + // Save transactions to re-add to mempool at end of reorg + for (auto it = block.vtx.rbegin(); it != block.vtx.rend(); ++it) { + disconnectpool->addTransaction(*it); + } + while (disconnectpool->DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) { + // Drop the earliest entry, and remove its children from the mempool. + auto it = disconnectpool->queuedTx.get<insertion_order>().begin(); + mempool.removeRecursive(**it, MemPoolRemovalReason::REORG); + disconnectpool->removeEntry(it); } - // AcceptToMemoryPool/addUnchecked all assume that new mempool entries have - // no in-mempool children, which is generally not true when adding - // previously-confirmed transactions back to the mempool. - // UpdateTransactionsFromBlock finds descendants of any transactions in this - // block that were added back and cleans up the mempool state. - mempool.UpdateTransactionsFromBlock(vHashUpdate); } // Update chainActive and related variables. @@ -2008,7 +2059,7 @@ public: * * The block is added to connectTrace if connection succeeds. */ -bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace) +bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) { assert(pindexNew->pprev == chainActive.Tip()); // Read block from disk. @@ -2050,6 +2101,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); // Remove conflicting transactions from the mempool.; mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight); + disconnectpool.removeForBlock(blockConnecting.vtx); // Update chainActive & related variables. UpdateTip(pindexNew, chainparams); @@ -2143,9 +2195,14 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c // Disconnect active blocks which are no longer in the best chain. bool fBlocksDisconnected = false; + DisconnectedBlockTransactions disconnectpool; while (chainActive.Tip() && chainActive.Tip() != pindexFork) { - if (!DisconnectTip(state, chainparams)) + if (!DisconnectTip(state, chainparams, &disconnectpool)) { + // This is likely a fatal error, but keep the mempool consistent, + // just in case. Only remove from the mempool in this case. + UpdateMempoolForReorg(disconnectpool, false); return false; + } fBlocksDisconnected = true; } @@ -2168,7 +2225,7 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c // Connect new blocks. BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) { - if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connectTrace)) { + if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr<const CBlock>(), connectTrace, disconnectpool)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (!state.CorruptionPossible()) @@ -2179,6 +2236,9 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c break; } else { // A system error occurred (disk space, database error, ...). + // Make the mempool consistent with the current tip, just in case + // any observers try to use it before shutdown. + UpdateMempoolForReorg(disconnectpool, false); return false; } } else { @@ -2193,8 +2253,9 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c } if (fBlocksDisconnected) { - mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); - LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); + // If any blocks were disconnected, disconnectpool may be non empty. Add + // any disconnected transactions back to the mempool. + UpdateMempoolForReorg(disconnectpool, true); } mempool.check(pcoinsTip); @@ -2343,6 +2404,7 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C setDirtyBlockIndex.insert(pindex); setBlockIndexCandidates.erase(pindex); + DisconnectedBlockTransactions disconnectpool; while (chainActive.Contains(pindex)) { CBlockIndex *pindexWalk = chainActive.Tip(); pindexWalk->nStatus |= BLOCK_FAILED_CHILD; @@ -2350,13 +2412,17 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C setBlockIndexCandidates.erase(pindexWalk); // ActivateBestChain considers blocks already in chainActive // unconditionally valid already, so force disconnect away from it. - if (!DisconnectTip(state, chainparams)) { - mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); + if (!DisconnectTip(state, chainparams, &disconnectpool)) { + // It's probably hopeless to try to make the mempool consistent + // here if DisconnectTip failed, but we can try. + UpdateMempoolForReorg(disconnectpool, false); return false; } } - LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); + // DisconnectTip will add transactions to disconnectpool; try to add these + // back to the mempool. + UpdateMempoolForReorg(disconnectpool, true); // The resulting new best tip may not be in setBlockIndexCandidates anymore, so // add it again. @@ -2369,7 +2435,6 @@ bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, C } InvalidChainFound(pindex); - mempool.removeForReorg(pcoinsTip, chainActive.Tip()->nHeight + 1, STANDARD_LOCKTIME_VERIFY_FLAGS); uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev); return true; } @@ -2817,8 +2882,8 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Co // No witness data is allowed in blocks that don't commit to witness data, as this would otherwise leave room for spam if (!fHaveWitness) { - for (size_t i = 0; i < block.vtx.size(); i++) { - if (block.vtx[i]->HasWitness()) { + for (const auto& tx : block.vtx) { + if (tx->HasWitness()) { return state.DoS(100, false, REJECT_INVALID, "unexpected-witness", true, strprintf("%s : unexpected witness data found", __func__)); } } @@ -3483,7 +3548,7 @@ bool RewindBlockIndex(const CChainParams& params) // of the blockchain). break; } - if (!DisconnectTip(state, params, true)) { + if (!DisconnectTip(state, params, NULL)) { return error("RewindBlockIndex: unable to disconnect block at height %i", pindex->nHeight); } // Occasionally flush state to disk. diff --git a/src/validation.h b/src/validation.h index b19c3ff4f7..02a9b5a369 100644 --- a/src/validation.h +++ b/src/validation.h @@ -70,6 +70,8 @@ static const unsigned int DEFAULT_DESCENDANT_LIMIT = 25; static const unsigned int DEFAULT_DESCENDANT_SIZE_LIMIT = 101; /** Default for -mempoolexpiry, expiration time for mempool transactions in hours */ static const unsigned int DEFAULT_MEMPOOL_EXPIRY = 336; +/** Maximum kilobytes for transactions to store for processing during reorg */ +static const unsigned int MAX_DISCONNECTED_TX_POOL_SIZE = 20000; /** 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) */ @@ -105,7 +107,7 @@ static const unsigned int DATABASE_FLUSH_INTERVAL = 24 * 60 * 60; /** Maximum length of reject messages. */ static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111; /** Average delay between local address broadcasts in seconds. */ -static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 24 * 60; +static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 60 * 60; /** Average delay between peer address broadcasts in seconds. */ static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30; /** Average delay between trickled inventory transmissions in seconds. diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index c10a9eccd9..99120d290c 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -255,7 +255,7 @@ bool CFeeBumper::commit(CWallet *pWallet) } CWalletTx& oldWtx = pWallet->mapWallet[txid]; - // make sure the transaction still has no descendants and hasen't been mined in the meantime + // make sure the transaction still has no descendants and hasn't been mined in the meantime if (!preconditionChecks(pWallet, oldWtx)) { return false; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ae4f4f37cb..0860d3565c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2909,7 +2909,7 @@ UniValue bumpfee(const JSONRPCRequest& request) UniValue errors(UniValue::VARR); for (const std::string& err: feeBump.getErrors()) errors.push_back(err); - result.push_back(errors); + result.push_back(Pair("errors", errors)); return result; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 997515a04b..bc98435249 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1788,8 +1788,8 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const { CMutableTransaction tx1 = *this->tx; CMutableTransaction tx2 = *_tx.tx; - for (unsigned int i = 0; i < tx1.vin.size(); i++) tx1.vin[i].scriptSig = CScript(); - for (unsigned int i = 0; i < tx2.vin.size(); i++) tx2.vin[i].scriptSig = CScript(); + for (auto& txin : tx1.vin) txin.scriptSig = CScript(); + for (auto& txin : tx2.vin) txin.scriptSig = CScript(); return CTransaction(tx1) == CTransaction(tx2); } @@ -2268,10 +2268,10 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin if (nTotalLower == nTargetValue) { - for (unsigned int i = 0; i < vValue.size(); ++i) + for (const auto& input : vValue) { - setCoinsRet.insert(vValue[i]); - nValueRet += vValue[i].txout.nValue; + setCoinsRet.insert(input); + nValueRet += input.txout.nValue; } return true; } @@ -2401,7 +2401,7 @@ bool CWallet::SignTransaction(CMutableTransaction &tx) // sign the new tx CTransaction txNewConst(tx); int nIn = 0; - for (auto& input : tx.vin) { + for (const auto& input : tx.vin) { std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(input.prevout.hash); if(mi == mapWallet.end() || input.prevout.n >= mi->second.tx->vout.size()) { return false; @@ -3340,11 +3340,11 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings() } // group lone addrs by themselves - for (unsigned int i = 0; i < pcoin->tx->vout.size(); i++) - if (IsMine(pcoin->tx->vout[i])) + for (const auto& txout : pcoin->tx->vout) + if (IsMine(txout)) { CTxDestination address; - if(!ExtractDestination(pcoin->tx->vout[i].scriptPubKey, address)) + if(!ExtractDestination(txout.scriptPubKey, address)) continue; grouping.insert(address); groupings.insert(grouping); |