diff options
44 files changed, 905 insertions, 246 deletions
diff --git a/contrib/devtools/github-merge.py b/contrib/devtools/github-merge.py index 8fce648fc2..e9816f7d19 100755 --- a/contrib/devtools/github-merge.py +++ b/contrib/devtools/github-merge.py @@ -301,8 +301,7 @@ def main(): subprocess.check_call([GIT,'commit','-q','--gpg-sign','--amend','--no-edit']) break except subprocess.CalledProcessError as e: - print("Error signing, exiting.",file=stderr) - exit(1) + print("Error while signing, asking again.",file=stderr) elif reply == 'x': print("Not signing off on merge, exiting.",file=stderr) exit(1) diff --git a/contrib/seeds/README.md b/contrib/seeds/README.md index afe902fd7f..139c03181f 100644 --- a/contrib/seeds/README.md +++ b/contrib/seeds/README.md @@ -8,7 +8,7 @@ and remove old versions as necessary. The seeds compiled into the release are created from sipa's DNS seed data, like this: - curl -s http://bitcoin.sipa.be/seeds.txt > seeds_main.txt + curl -s http://bitcoin.sipa.be/seeds.txt.gz | gzip -dc > seeds_main.txt python3 makeseeds.py < seeds_main.txt > nodes_main.txt python3 generate-seeds.py . > ../../src/chainparamsseeds.h diff --git a/doc/developer-notes.md b/doc/developer-notes.md index cf860a1bf2..ec6abda91e 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -3,41 +3,64 @@ Developer Notes Various coding styles have been used during the history of the codebase, and the result is not very consistent. However, we're now trying to converge to -a single style, so please use it in new code. Old code will be converted -gradually and you are encouraged to use the provided -[clang-format-diff script](/contrib/devtools/README.md#clang-format-diffpy) -to clean up the patch automatically before submitting a pull request. +a single style, which is specified below. When writing patches, favor the new +style over attempting to mimick the surrounding style, except for move-only +commits. + +Do not submit patches solely to modify the style of existing code. -- Basic rules specified in [src/.clang-format](/src/.clang-format). +- **Indentation and whitespace rules** as specified in +[src/.clang-format](/src/.clang-format). You can use the provided +[clang-format-diff script](/contrib/devtools/README.md#clang-format-diffpy) +tool to clean up patches automatically before submission. - Braces on new lines for namespaces, classes, functions, methods. - Braces on the same line for everything else. - 4 space indentation (no tabs) for every block except namespaces. - No indentation for `public`/`protected`/`private` or for `namespace`. - No extra spaces inside parenthesis; don't do ( this ) - No space after function names; one space after `if`, `for` and `while`. - - If an `if` only has a single-statement then-clause, it can appear - on the same line as the if, without braces. In every other case, - braces are required, and the then and else clauses must appear + - If an `if` only has a single-statement `then`-clause, it can appear + on the same line as the `if`, without braces. In every other case, + braces are required, and the `then` and `else` clauses must appear correctly indented on a new line. + +- **Symbol naming conventions**. These are preferred in new code, but are not +required when doing so would need changes to significant pieces of existing +code. + - Variable and namespace names are all lowercase, and may use `_` to + separate words. + - Class member variables have a `m_` prefix. + - Global variables have a `g_` prefix. + - Constant names are all uppercase, and use `_` to separate words. + - Class names, function names and method names are CamelCase. Do not prefix + class names with `C`. + +- **Miscellaneous** - `++i` is preferred over `i++`. Block style example: ```c++ +int g_count = 0; + namespace foo { class Class { + std::string m_name; + +public: bool Function(const std::string& s, int n) { // Comment summarising what this section of code does for (int i = 0; i < n; ++i) { + int total_sum = 0; // When something fails, return early if (!Something()) return false; ... - if (SomethingElse()) { - DoMore(); + if (SomethingElse(i)) { + total_sum += ComputeSomething(g_count); } else { - DoLess(); + DoSomething(m_name, total_sum); } } diff --git a/src/Makefile.qttest.include b/src/Makefile.qttest.include index 948e13a9e1..391b9ebdf6 100644 --- a/src/Makefile.qttest.include +++ b/src/Makefile.qttest.include @@ -46,7 +46,8 @@ qt_test_test_bitcoin_qt_SOURCES = \ if ENABLE_WALLET qt_test_test_bitcoin_qt_SOURCES += \ qt/test/paymentservertests.cpp \ - qt/test/wallettests.cpp + qt/test/wallettests.cpp \ + wallet/test/wallet_test_fixture.cpp endif nodist_qt_test_test_bitcoin_qt_SOURCES = $(TEST_QT_MOC_CPP) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 31680a8ec7..5922e45801 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -117,17 +117,14 @@ bool AppInit(int argc, char* argv[]) return false; } - // Command-line RPC - bool fCommandLine = false; - for (int i = 1; i < argc; i++) - if (!IsSwitchChar(argv[i][0]) && !boost::algorithm::istarts_with(argv[i], "bitcoin:")) - fCommandLine = true; - - if (fCommandLine) - { - fprintf(stderr, "Error: There is no RPC client functionality in bitcoind anymore. Use the bitcoin-cli utility instead.\n"); - exit(EXIT_FAILURE); + // Error out when loose non-argument tokens are encountered on command line + for (int i = 1; i < argc; i++) { + if (!IsSwitchChar(argv[i][0])) { + fprintf(stderr, "Error: Command line contains unexpected token '%s', see bitcoind -h for a list of options.\n", argv[i]); + exit(EXIT_FAILURE); + } } + // -server defaults to true for bitcoind but not for the GUI so do this here SoftSetBoolArg("-server", true); // Set this early so that parameter interactions go to console 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 a0e5364e66..378ac99d66 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1749,9 +1749,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. // @@ -2231,6 +2231,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/netbase.cpp b/src/netbase.cpp index 2584f571ee..32557dd179 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -668,13 +668,14 @@ std::string NetworkErrorString(int err) std::string NetworkErrorString(int err) { char buf[256]; - const char *s = buf; buf[0] = 0; /* Too bad there are two incompatible implementations of the * thread-safe strerror. */ + const char *s; #ifdef STRERROR_R_CHAR_P /* GNU variant can return a pointer outside the passed buffer */ s = strerror_r(err, buf, sizeof(buf)); #else /* POSIX variant always returns message in buffer */ + s = buf; if (strerror_r(err, buf, sizeof(buf))) buf[0] = 0; #endif 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/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 2a331d4fae..135cf6f701 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -513,8 +513,6 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) // Fee nPayFee = CWallet::GetMinimumFee(nBytes, nTxConfirmTarget, ::mempool, ::feeEstimator); - if (nPayFee > 0 && coinControl->nMinimumTotalFee > nPayFee) - nPayFee = coinControl->nMinimumTotalFee; if (nPayAmount > 0) { @@ -573,7 +571,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) l5->setText(((nBytes > 0) ? ASYMP_UTF8 : "") + QString::number(nBytes)); // Bytes l7->setText(fDust ? tr("yes") : tr("no")); // Dust l8->setText(BitcoinUnits::formatWithUnit(nDisplayUnit, nChange)); // Change - if (nPayFee > 0 && (coinControl->nMinimumTotalFee < nPayFee)) + if (nPayFee > 0) { l3->setText(ASYMP_UTF8 + l3->text()); l4->setText(ASYMP_UTF8 + l4->text()); diff --git a/src/qt/forms/sendcoinsdialog.ui b/src/qt/forms/sendcoinsdialog.ui index 52256ca5c4..89f9c25d14 100644 --- a/src/qt/forms/sendcoinsdialog.ui +++ b/src/qt/forms/sendcoinsdialog.ui @@ -862,19 +862,6 @@ </widget> </item> <item> - <widget class="QRadioButton" name="radioCustomAtLeast"> - <property name="toolTip"> - <string>If the custom fee is set to 1000 satoshis and the transaction is only 250 bytes, then "per kilobyte" only pays 250 satoshis in fee, while "total at least" pays 1000 satoshis. For transactions bigger than a kilobyte both pay by kilobyte.</string> - </property> - <property name="text"> - <string>total at least</string> - </property> - <attribute name="buttonGroup"> - <string notr="true">groupCustomFee</string> - </attribute> - </widget> - </item> - <item> <widget class="BitcoinAmountField" name="customFee"/> </item> <item> diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 8dcb0fd016..272ab9486a 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -109,7 +109,6 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *_platformStyle, QWidget *p ui->groupFee->setId(ui->radioCustomFee, 1); ui->groupFee->button((int)std::max(0, std::min(1, settings.value("nFeeRadio").toInt())))->setChecked(true); ui->groupCustomFee->setId(ui->radioCustomPerKilobyte, 0); - ui->groupCustomFee->setId(ui->radioCustomAtLeast, 1); ui->groupCustomFee->button((int)std::max(0, std::min(1, settings.value("nCustomFeeRadio").toInt())))->setChecked(true); ui->customFee->setValue(settings.value("nTransactionFee").toLongLong()); ui->checkBoxMinimumFee->setChecked(settings.value("fPayOnlyMinFee").toBool()); @@ -606,7 +605,6 @@ void SendCoinsDialog::updateFeeSectionControls() ui->checkBoxMinimumFee ->setEnabled(ui->radioCustomFee->isChecked()); ui->labelMinFeeWarning ->setEnabled(ui->radioCustomFee->isChecked()); ui->radioCustomPerKilobyte ->setEnabled(ui->radioCustomFee->isChecked() && !ui->checkBoxMinimumFee->isChecked()); - ui->radioCustomAtLeast ->setEnabled(ui->radioCustomFee->isChecked() && !ui->checkBoxMinimumFee->isChecked() && CoinControlDialog::coinControl->HasSelected()); ui->customFee ->setEnabled(ui->radioCustomFee->isChecked() && !ui->checkBoxMinimumFee->isChecked()); } @@ -617,19 +615,12 @@ void SendCoinsDialog::updateGlobalFeeVariables() int nConfirmTarget = ui->sliderSmartFee->maximum() - ui->sliderSmartFee->value() + 2; payTxFee = CFeeRate(0); - // set nMinimumTotalFee to 0 to not accidentally pay a custom fee - CoinControlDialog::coinControl->nMinimumTotalFee = 0; - // show the estimated required time for confirmation ui->confirmationTargetLabel->setText(GUIUtil::formatDurationStr(nConfirmTarget * Params().GetConsensus().nPowTargetSpacing) + " / " + tr("%n block(s)", "", nConfirmTarget)); } else { payTxFee = CFeeRate(ui->customFee->value()); - - // if user has selected to set a minimum absolute fee, pass the value to coincontrol - // 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; } } @@ -828,21 +819,6 @@ void SendCoinsDialog::coinControlUpdateLabels() if (!model || !model->getOptionsModel()) return; - if (model->getOptionsModel()->getCoinControlFeatures()) - { - // enable minimum absolute fee UI controls - ui->radioCustomAtLeast->setVisible(true); - - // only enable the feature if inputs are selected - ui->radioCustomAtLeast->setEnabled(ui->radioCustomFee->isChecked() && !ui->checkBoxMinimumFee->isChecked() &&CoinControlDialog::coinControl->HasSelected()); - } - else - { - // in case coin control is disabled (=default), hide minimum absolute fee UI controls - ui->radioCustomAtLeast->setVisible(false); - return; - } - // set pay amounts CoinControlDialog::payAmounts.clear(); CoinControlDialog::fSubtractFeeFromAmount = false; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index a0dce3d997..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; @@ -65,6 +89,42 @@ 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. @@ -80,11 +140,13 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid) // src/qt/test/test_bitcoin-qt -platform xcb # Linux // src/qt/test/test_bitcoin-qt -platform windows # Windows // src/qt/test/test_bitcoin-qt -platform cocoa # macOS -void WalletTests::walletTests() +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)); @@ -101,19 +163,34 @@ void WalletTests::walletTests() // 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(); } + +} + +void WalletTests::walletTests() +{ + TestSendCoins(); +} 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/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 37af8abb38..33b407ae57 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -68,14 +68,7 @@ CAmount WalletModel::getBalance(const CCoinControl *coinControl) const { if (coinControl) { - CAmount nBalance = 0; - std::vector<COutput> vCoins; - wallet->AvailableCoins(vCoins, true, coinControl); - BOOST_FOREACH(const COutput& out, vCoins) - if(out.fSpendable) - nBalance += out.tx->tx->vout[out.i].nValue; - - return nBalance; + return wallet->GetAvailableBalance(coinControl); } return wallet->GetBalance(); @@ -600,38 +593,11 @@ bool WalletModel::isSpent(const COutPoint& outpoint) const // AvailableCoins + LockedCoins grouped by wallet address (put change in one group with wallet address) void WalletModel::listCoins(std::map<QString, std::vector<COutput> >& mapCoins) const { - std::vector<COutput> vCoins; - wallet->AvailableCoins(vCoins); - - LOCK2(cs_main, wallet->cs_wallet); // ListLockedCoins, mapWallet - std::vector<COutPoint> vLockedCoins; - wallet->ListLockedCoins(vLockedCoins); - - // add locked coins - BOOST_FOREACH(const COutPoint& outpoint, vLockedCoins) - { - if (!wallet->mapWallet.count(outpoint.hash)) continue; - int nDepth = wallet->mapWallet[outpoint.hash].GetDepthInMainChain(); - if (nDepth < 0) continue; - COutput out(&wallet->mapWallet[outpoint.hash], outpoint.n, nDepth, true /* spendable */, true /* solvable */, true /* safe */); - if (outpoint.n < out.tx->tx->vout.size() && wallet->IsMine(out.tx->tx->vout[outpoint.n]) == ISMINE_SPENDABLE) - vCoins.push_back(out); - } - - BOOST_FOREACH(const COutput& out, vCoins) - { - COutput cout = out; - - while (wallet->IsChange(cout.tx->tx->vout[cout.i]) && cout.tx->tx->vin.size() > 0 && wallet->IsMine(cout.tx->tx->vin[0])) - { - if (!wallet->mapWallet.count(cout.tx->tx->vin[0].prevout.hash)) break; - cout = COutput(&wallet->mapWallet[cout.tx->tx->vin[0].prevout.hash], cout.tx->tx->vin[0].prevout.n, 0 /* depth */, true /* spendable */, true /* solvable */, true /* safe */); + for (auto& group : wallet->ListCoins()) { + auto& resultGroup = mapCoins[QString::fromStdString(CBitcoinAddress(group.first).ToString())]; + for (auto& coin : group.second) { + resultGroup.emplace_back(std::move(coin)); } - - CTxDestination address; - if(!out.fSpendable || !ExtractDestination(cout.tx->tx->vout[cout.i].scriptPubKey, address)) - continue; - mapCoins[QString::fromStdString(CBitcoinAddress(address).ToString())].push_back(out); } } @@ -661,11 +627,7 @@ void WalletModel::listLockedCoins(std::vector<COutPoint>& vOutpts) void WalletModel::loadReceiveRequests(std::vector<std::string>& vReceiveRequests) { - LOCK(wallet->cs_wallet); - BOOST_FOREACH(const PAIRTYPE(CTxDestination, CAddressBookData)& item, wallet->mapAddressBook) - BOOST_FOREACH(const PAIRTYPE(std::string, std::string)& item2, item.second.destdata) - if (item2.first.size() > 2 && item2.first.substr(0,2) == "rr") // receive request - vReceiveRequests.push_back(item2.second); + vReceiveRequests = wallet->GetDestValues("rr"); // receive request } bool WalletModel::saveReceiveRequest(const std::string &sAddress, const int64_t nId, const std::string &sRequest) @@ -685,11 +647,7 @@ bool WalletModel::saveReceiveRequest(const std::string &sAddress, const int64_t bool WalletModel::transactionCanBeAbandoned(uint256 hash) const { - LOCK2(cs_main, wallet->cs_wallet); - const CWalletTx *wtx = wallet->GetWalletTx(hash); - if (!wtx || wtx->isAbandoned() || wtx->GetDepthInMainChain() > 0 || wtx->InMempool()) - return false; - return true; + return wallet->TransactionCanBeAbandoned(hash); } bool WalletModel::abandonTransaction(uint256 hash) const @@ -702,9 +660,7 @@ bool WalletModel::transactionSignalsRBF(uint256 hash) const { LOCK2(cs_main, wallet->cs_wallet); const CWalletTx *wtx = wallet->GetWalletTx(hash); - if (wtx && SignalsOptInRBF(*wtx)) - return true; - return false; + return wtx && SignalsOptInRBF(*wtx); } bool WalletModel::bumpFee(uint256 hash) diff --git a/src/random.cpp b/src/random.cpp index 3b9df3edaa..de7553c825 100644 --- a/src/random.cpp +++ b/src/random.cpp @@ -203,10 +203,43 @@ void GetRandBytes(unsigned char* buf, int num) } } +static void AddDataToRng(void* data, size_t len); + +void RandAddSeedSleep() +{ + int64_t nPerfCounter1 = GetPerformanceCounter(); + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + int64_t nPerfCounter2 = GetPerformanceCounter(); + + // Combine with and update state + AddDataToRng(&nPerfCounter1, sizeof(nPerfCounter1)); + AddDataToRng(&nPerfCounter2, sizeof(nPerfCounter2)); + + memory_cleanse(&nPerfCounter1, sizeof(nPerfCounter1)); + memory_cleanse(&nPerfCounter2, sizeof(nPerfCounter2)); +} + + static std::mutex cs_rng_state; static unsigned char rng_state[32] = {0}; static uint64_t rng_counter = 0; +static void AddDataToRng(void* data, size_t len) { + CSHA512 hasher; + hasher.Write((const unsigned char*)&len, sizeof(len)); + hasher.Write((const unsigned char*)data, len); + unsigned char buf[64]; + { + std::unique_lock<std::mutex> lock(cs_rng_state); + hasher.Write(rng_state, sizeof(rng_state)); + hasher.Write((const unsigned char*)&rng_counter, sizeof(rng_counter)); + ++rng_counter; + hasher.Finalize(buf); + memcpy(rng_state, buf + 32, 32); + } + memory_cleanse(buf, 64); +} + void GetStrongRandBytes(unsigned char* out, int num) { assert(num <= 32); diff --git a/src/random.h b/src/random.h index 9551e1c461..6a63d57429 100644 --- a/src/random.h +++ b/src/random.h @@ -24,6 +24,13 @@ int GetRandInt(int nMax); uint256 GetRandHash(); /** + * Add a little bit of randomness to the output of GetStrongRangBytes. + * This sleeps for a millisecond, so should only be called when there is + * no other work to be done. + */ +void RandAddSeedSleep(); + +/** * Function to gather random data from multiple sources, failing whenever any * of those source fail to provide a result. */ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9d72a23e6d..b4b160aac9 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -187,7 +187,7 @@ void RPCNotifyBlockChange(bool ibd, const CBlockIndex * pindex) latestblock.hash = pindex->GetBlockHash(); latestblock.height = pindex->nHeight; } - cond_blockchange.notify_all(); + cond_blockchange.notify_all(); } UniValue waitfornewblock(const JSONRPCRequest& request) @@ -1074,6 +1074,17 @@ static UniValue BIP9SoftForkDesc(const Consensus::Params& consensusParams, Conse rv.push_back(Pair("startTime", consensusParams.vDeployments[id].nStartTime)); rv.push_back(Pair("timeout", consensusParams.vDeployments[id].nTimeout)); rv.push_back(Pair("since", VersionBitsTipStateSinceHeight(consensusParams, id))); + if (THRESHOLD_STARTED == thresholdState) + { + UniValue statsUV(UniValue::VOBJ); + BIP9Stats statsStruct = VersionBitsTipStatistics(consensusParams, id); + statsUV.push_back(Pair("period", statsStruct.period)); + statsUV.push_back(Pair("threshold", statsStruct.threshold)); + statsUV.push_back(Pair("elapsed", statsStruct.elapsed)); + statsUV.push_back(Pair("count", statsStruct.count)); + statsUV.push_back(Pair("possible", statsStruct.possible)); + rv.push_back(Pair("statistics", statsUV)); + } return rv; } @@ -1119,7 +1130,14 @@ UniValue getblockchaininfo(const JSONRPCRequest& request) " \"bit\": xx, (numeric) the bit (0-28) in the block version field used to signal this softfork (only for \"started\" status)\n" " \"startTime\": xx, (numeric) the minimum median time past of a block at which the bit gains its meaning\n" " \"timeout\": xx, (numeric) the median time past of a block at which the deployment is considered failed if not yet locked in\n" - " \"since\": xx (numeric) height of the first block to which the status applies\n" + " \"since\": xx, (numeric) height of the first block to which the status applies\n" + " \"statistics\": { (object) numeric statistics about BIP9 signalling for a softfork (only for \"started\" status)\n" + " \"period\": xx, (numeric) the length in blocks of the BIP9 signalling period \n" + " \"threshold\": xx, (numeric) the number of blocks with the version bit set required to activate the feature \n" + " \"elapsed\": xx, (numeric) the number of blocks elapsed since the beginning of the current period \n" + " \"count\": xx, (numeric) the number of blocks with the version bit set in the current period \n" + " \"possible\": xx (boolean) returns false if there are not enough blocks left in this period to pass activation threshold \n" + " }\n" " }\n" " }\n" "}\n" diff --git a/src/scheduler.cpp b/src/scheduler.cpp index 0c1cfa2718..923ba2c231 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -4,6 +4,7 @@ #include "scheduler.h" +#include "random.h" #include "reverselock.h" #include <assert.h> @@ -39,6 +40,11 @@ void CScheduler::serviceQueue() // is called. while (!shouldStop()) { try { + if (!shouldStop() && taskQueue.empty()) { + reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock); + // Use this chance to get a tiny bit more entropy + RandAddSeedSleep(); + } while (!shouldStop() && taskQueue.empty()) { // Wait until there is something to do. newTaskScheduled.wait(lock); 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 73466b9df7..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. @@ -3935,6 +4000,12 @@ ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::D return VersionBitsState(chainActive.Tip(), params, pos, versionbitscache); } +BIP9Stats VersionBitsTipStatistics(const Consensus::Params& params, Consensus::DeploymentPos pos) +{ + LOCK(cs_main); + return VersionBitsStatistics(chainActive.Tip(), params, pos); +} + int VersionBitsTipStateSinceHeight(const Consensus::Params& params, Consensus::DeploymentPos pos) { LOCK(cs_main); diff --git a/src/validation.h b/src/validation.h index 743a2973f8..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. @@ -339,6 +341,9 @@ std::string FormatStateMessage(const CValidationState &state); /** Get the BIP9 state for a given deployment at the current tip. */ ThresholdState VersionBitsTipState(const Consensus::Params& params, Consensus::DeploymentPos pos); +/** Get the numerical statistics for the BIP9 state for a given deployment at the current tip. */ +BIP9Stats VersionBitsTipStatistics(const Consensus::Params& params, Consensus::DeploymentPos pos); + /** Get the block height at which the BIP9 deployment switched into the state for the block building on the current tip. */ int VersionBitsTipStateSinceHeight(const Consensus::Params& params, Consensus::DeploymentPos pos); diff --git a/src/versionbits.cpp b/src/versionbits.cpp index 8a7cce7485..80786233f5 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "versionbits.h" - #include "consensus/params.h" const struct BIP9DeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] = { @@ -105,6 +104,36 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* return state; } +// return the numerical statistics of blocks signalling the specified BIP9 condition in this current period +BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockIndex* pindex, const Consensus::Params& params) const +{ + BIP9Stats stats; + + stats.period = Period(params); + stats.threshold = Threshold(params); + + if (pindex == NULL) + return stats; + + // Find beginning of period + const CBlockIndex* pindexEndOfPrevPeriod = pindex->GetAncestor(pindex->nHeight - ((pindex->nHeight + 1) % stats.period)); + stats.elapsed = pindex->nHeight - pindexEndOfPrevPeriod->nHeight; + + // Count from current block to beginning of period + int count = 0; + const CBlockIndex* currentIndex = pindex; + while (pindexEndOfPrevPeriod->nHeight != currentIndex->nHeight){ + if (Condition(currentIndex, params)) + count++; + currentIndex = currentIndex->pprev; + } + + stats.count = count; + stats.possible = (stats.period - stats.threshold ) >= (stats.elapsed - count); + + return stats; +} + int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const { const ThresholdState initialState = GetStateFor(pindexPrev, params, cache); @@ -167,6 +196,11 @@ ThresholdState VersionBitsState(const CBlockIndex* pindexPrev, const Consensus:: return VersionBitsConditionChecker(pos).GetStateFor(pindexPrev, params, cache.caches[pos]); } +BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos) +{ + return VersionBitsConditionChecker(pos).GetStateStatisticsFor(pindexPrev, params); +} + int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache) { return VersionBitsConditionChecker(pos).GetStateSinceHeightFor(pindexPrev, params, cache.caches[pos]); diff --git a/src/versionbits.h b/src/versionbits.h index 7a929266aa..f1d31ea0af 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -37,6 +37,14 @@ struct BIP9DeploymentInfo { bool gbt_force; }; +struct BIP9Stats { + int period; + int threshold; + int elapsed; + int count; + bool possible; +}; + extern const struct BIP9DeploymentInfo VersionBitsDeploymentInfo[]; /** @@ -51,6 +59,7 @@ protected: virtual int Threshold(const Consensus::Params& params) const =0; public: + BIP9Stats GetStateStatisticsFor(const CBlockIndex* pindex, const Consensus::Params& params) const; // Note that the functions below take a pindexPrev as input: they compute information for block B based on its parent. ThresholdState GetStateFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const; int GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const; @@ -64,6 +73,7 @@ struct VersionBitsCache }; ThresholdState VersionBitsState(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache); +BIP9Stats VersionBitsStatistics(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos); int VersionBitsStateSinceHeight(const CBlockIndex* pindexPrev, const Consensus::Params& params, Consensus::DeploymentPos pos, VersionBitsCache& cache); uint32_t VersionBitsMask(const Consensus::Params& params, Consensus::DeploymentPos pos); diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 2aa26fb00a..cb4719ae90 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -18,8 +18,6 @@ public: bool fAllowOtherInputs; //! Includes watch only addresses which match the ISMINE_WATCH_SOLVABLE criteria bool fAllowWatchOnly; - //! Minimum absolute fee (not per kilobyte) - CAmount nMinimumTotalFee; //! Override estimated feerate bool fOverrideFeeRate; //! Feerate to use if overrideFeeRate is true @@ -40,7 +38,6 @@ public: fAllowOtherInputs = false; fAllowWatchOnly = false; setSelected.clear(); - nMinimumTotalFee = 0; nFeeRate = CFeeRate(0); fOverrideFeeRate = false; nConfirmTarget = 0; 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/rpcdump.cpp b/src/wallet/rpcdump.cpp index 82708dab26..613d5d228a 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -357,7 +357,7 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 1) throw std::runtime_error( "removeprunedfunds \"txid\"\n" - "\nDeletes the specified transaction from the wallet. Meant for use with pruned wallets and as a companion to importprunedfunds. This will effect wallet balances.\n" + "\nDeletes the specified transaction from the wallet. Meant for use with pruned wallets and as a companion to importprunedfunds. This will affect wallet balances.\n" "\nArguments:\n" "1. \"txid\" (string, required) The hex-encoded id of the transaction you are deleting\n" "\nExamples:\n" @@ -536,14 +536,11 @@ UniValue importwallet(const JSONRPCRequest& request) } file.close(); pwallet->ShowProgress("", 100); // hide progress dialog in GUI - - CBlockIndex *pindex = chainActive.Tip(); - while (pindex && pindex->pprev && pindex->GetBlockTime() > nTimeBegin - TIMESTAMP_WINDOW) - pindex = pindex->pprev; - pwallet->UpdateTimeFirstKey(nTimeBegin); - LogPrintf("Rescanning last %i blocks\n", chainActive.Height() - pindex->nHeight + 1); + CBlockIndex *pindex = chainActive.FindEarliestAtLeast(nTimeBegin - TIMESTAMP_WINDOW); + + LogPrintf("Rescanning last %i blocks\n", pindex ? chainActive.Height() - pindex->nHeight + 1 : 0); pwallet->ScanForWalletTransactions(pindex); pwallet->MarkDirty(); 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/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 8eeba72a06..5c7359fdce 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -9,6 +9,7 @@ #include <utility> #include <vector> +#include "consensus/validation.h" #include "rpc/server.h" #include "test/test_bitcoin.h" #include "validation.h" @@ -19,6 +20,8 @@ #include <univalue.h> extern UniValue importmulti(const JSONRPCRequest& request); +extern UniValue dumpwallet(const JSONRPCRequest& request); +extern UniValue importwallet(const JSONRPCRequest& request); // how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles #define RUN_TESTS 100 @@ -437,6 +440,66 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) } } +// Verify importwallet RPC starts rescan at earliest block with timestamp +// greater or equal than key birthday. Previously there was a bug where +// importwallet RPC would start the scan at the latest block with timestamp less +// than or equal to key birthday. +BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) +{ + CWallet *pwalletMainBackup = ::pwalletMain; + LOCK(cs_main); + + // Create two blocks with same timestamp to verify that importwallet rescan + // will pick up both blocks, not just the first. + const int64_t BLOCK_TIME = chainActive.Tip()->GetBlockTimeMax() + 5; + SetMockTime(BLOCK_TIME); + coinbaseTxns.emplace_back(*CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); + coinbaseTxns.emplace_back(*CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); + + // Set key birthday to block time increased by the timestamp window, so + // rescan will start at the block time. + const int64_t KEY_TIME = BLOCK_TIME + TIMESTAMP_WINDOW; + SetMockTime(KEY_TIME); + coinbaseTxns.emplace_back(*CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); + + // Import key into wallet and call dumpwallet to create backup file. + { + CWallet wallet; + LOCK(wallet.cs_wallet); + wallet.mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; + wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); + + JSONRPCRequest request; + request.params.setArray(); + request.params.push_back("wallet.backup"); + ::pwalletMain = &wallet; + ::dumpwallet(request); + } + + // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME + // were scanned, and no prior blocks were scanned. + { + CWallet wallet; + + JSONRPCRequest request; + request.params.setArray(); + request.params.push_back("wallet.backup"); + ::pwalletMain = &wallet; + ::importwallet(request); + + BOOST_CHECK_EQUAL(wallet.mapWallet.size(), 3); + BOOST_CHECK_EQUAL(coinbaseTxns.size(), 103); + for (size_t i = 0; i < coinbaseTxns.size(); ++i) { + bool found = wallet.GetWalletTx(coinbaseTxns[i].GetHash()); + bool expected = i >= 100; + BOOST_CHECK_EQUAL(found, expected); + } + } + + SetMockTime(0); + ::pwalletMain = pwalletMainBackup; +} + // Check that GetImmatureCredit() returns a newly calculated value instead of // the cached value after a MarkDirty() call. // @@ -515,4 +578,104 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart) SetMockTime(0); } +BOOST_AUTO_TEST_CASE(LoadReceiveRequests) +{ + CTxDestination dest = CKeyID(); + pwalletMain->AddDestData(dest, "misc", "val_misc"); + pwalletMain->AddDestData(dest, "rr0", "val_rr0"); + pwalletMain->AddDestData(dest, "rr1", "val_rr1"); + + auto values = pwalletMain->GetDestValues("rr"); + BOOST_CHECK_EQUAL(values.size(), 2); + BOOST_CHECK_EQUAL(values[0], "val_rr0"); + BOOST_CHECK_EQUAL(values[1], "val_rr1"); +} + +class ListCoinsTestingSetup : public TestChain100Setup +{ +public: + ListCoinsTestingSetup() + { + CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + ::bitdb.MakeMock(); + wallet.reset(new CWallet(std::unique_ptr<CWalletDBWrapper>(new CWalletDBWrapper(&bitdb, "wallet_test.dat")))); + bool firstRun; + wallet->LoadWallet(firstRun); + LOCK(wallet->cs_wallet); + wallet->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); + wallet->ScanForWalletTransactions(chainActive.Genesis()); + } + + ~ListCoinsTestingSetup() + { + wallet.reset(); + ::bitdb.Flush(true); + ::bitdb.Reset(); + } + + CWalletTx& AddTx(CRecipient recipient) + { + CWalletTx wtx; + CReserveKey reservekey(wallet.get()); + CAmount fee; + int changePos = -1; + std::string error; + BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error)); + CValidationState state; + BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + auto it = wallet->mapWallet.find(wtx.GetHash()); + BOOST_CHECK(it != wallet->mapWallet.end()); + CreateAndProcessBlock({CMutableTransaction(*it->second.tx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + it->second.SetMerkleBranch(chainActive.Tip(), 1); + return it->second; + } + + std::unique_ptr<CWallet> wallet; +}; + +BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) +{ + std::string coinbaseAddress = coinbaseKey.GetPubKey().GetID().ToString(); + LOCK(wallet->cs_wallet); + + // Confirm ListCoins initially returns 1 coin grouped under coinbaseKey + // address. + auto list = wallet->ListCoins(); + BOOST_CHECK_EQUAL(list.size(), 1); + BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); + BOOST_CHECK_EQUAL(list.begin()->second.size(), 1); + + // Check initial balance from one mature coinbase transaction. + BOOST_CHECK_EQUAL(50 * COIN, wallet->GetAvailableBalance()); + + // Add a transaction creating a change address, and confirm ListCoins still + // returns the coin associated with the change address underneath the + // coinbaseKey pubkey, even though the change address has a different + // pubkey. + AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */}); + list = wallet->ListCoins(); + BOOST_CHECK_EQUAL(list.size(), 1); + BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); + BOOST_CHECK_EQUAL(list.begin()->second.size(), 2); + + // Lock both coins. Confirm number of available coins drops to 0. + std::vector<COutput> available; + wallet->AvailableCoins(available); + BOOST_CHECK_EQUAL(available.size(), 2); + for (const auto& group : list) { + for (const auto& coin : group.second) { + wallet->LockCoin(COutPoint(coin.tx->GetHash(), coin.i)); + } + } + wallet->AvailableCoins(available); + BOOST_CHECK_EQUAL(available.size(), 0); + + // Confirm ListCoins still returns same result as before, despite coins + // being locked. + list = wallet->ListCoins(); + BOOST_CHECK_EQUAL(list.size(), 1); + BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); + BOOST_CHECK_EQUAL(list.begin()->second.size(), 2); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9eba8ad9fe..bc98435249 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -982,6 +982,13 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const CBlockI return false; } +bool CWallet::TransactionCanBeAbandoned(const uint256& hashTx) const +{ + LOCK2(cs_main, cs_wallet); + const CWalletTx* wtx = GetWalletTx(hashTx); + return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() <= 0 && !wtx->InMempool(); +} + bool CWallet::AbandonTransaction(const uint256& hashTx) { LOCK2(cs_main, cs_wallet); @@ -1781,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); } @@ -1977,6 +1984,21 @@ CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth, cons return balance; } +CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const +{ + LOCK2(cs_main, cs_wallet); + + CAmount balance = 0; + std::vector<COutput> vCoins; + AvailableCoins(vCoins, true, coinControl); + for (const COutput& out : vCoins) { + if (out.fSpendable) { + balance += out.tx->tx->vout[out.i].nValue; + } + } + return balance; +} + void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const CCoinControl *coinControl, const CAmount &nMinimumAmount, const CAmount &nMaximumAmount, const CAmount &nMinimumSumAmount, const uint64_t &nMaximumCount, const int &nMinDepth, const int &nMaxDepth) const { vCoins.clear(); @@ -2088,6 +2110,69 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const } } +std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const +{ + // TODO: Add AssertLockHeld(cs_wallet) here. + // + // Because the return value from this function contains pointers to + // CWalletTx objects, callers to this function really should acquire the + // cs_wallet lock before calling it. However, the current caller doesn't + // acquire this lock yet. There was an attempt to add the missing lock in + // https://github.com/bitcoin/bitcoin/pull/10340, but that change has been + // postponed until after https://github.com/bitcoin/bitcoin/pull/10244 to + // avoid adding some extra complexity to the Qt code. + + std::map<CTxDestination, std::vector<COutput>> result; + + std::vector<COutput> availableCoins; + AvailableCoins(availableCoins); + + LOCK2(cs_main, cs_wallet); + for (auto& coin : availableCoins) { + CTxDestination address; + if (coin.fSpendable && + ExtractDestination(FindNonChangeParentOutput(*coin.tx->tx, coin.i).scriptPubKey, address)) { + result[address].emplace_back(std::move(coin)); + } + } + + std::vector<COutPoint> lockedCoins; + ListLockedCoins(lockedCoins); + for (const auto& output : lockedCoins) { + auto it = mapWallet.find(output.hash); + if (it != mapWallet.end()) { + int depth = it->second.GetDepthInMainChain(); + if (depth >= 0 && output.n < it->second.tx->vout.size() && + IsMine(it->second.tx->vout[output.n]) == ISMINE_SPENDABLE) { + CTxDestination address; + if (ExtractDestination(FindNonChangeParentOutput(*it->second.tx, output.n).scriptPubKey, address)) { + result[address].emplace_back( + &it->second, output.n, depth, true /* spendable */, true /* solvable */, false /* safe */); + } + } + } + } + + return result; +} + +const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int output) const +{ + const CTransaction* ptx = &tx; + int n = output; + while (IsChange(ptx->vout[n]) && ptx->vin.size() > 0) { + const COutPoint& prevout = ptx->vin[0].prevout; + auto it = mapWallet.find(prevout.hash); + if (it == mapWallet.end() || it->second.tx->vout.size() <= prevout.n || + !IsMine(it->second.tx->vout[prevout.n])) { + break; + } + ptx = it->second.tx.get(); + n = prevout.n; + } + return ptx->vout[n]; +} + static void ApproximateBestSubset(const std::vector<CInputCoin>& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000) { @@ -2183,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; } @@ -2316,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; @@ -2635,9 +2720,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT currentConfirmationTarget = coinControl->nConfirmTarget; CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, ::mempool, ::feeEstimator); - if (coinControl && nFeeNeeded > 0 && coinControl->nMinimumTotalFee > nFeeNeeded) { - nFeeNeeded = coinControl->nMinimumTotalFee; - } if (coinControl && coinControl->fOverrideFeeRate) nFeeNeeded = coinControl->nFeeRate.GetFee(nBytes); @@ -3258,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); @@ -3410,7 +3492,7 @@ bool CWallet::IsLockedCoin(uint256 hash, unsigned int n) const return (setLockedCoins.count(outpt) > 0); } -void CWallet::ListLockedCoins(std::vector<COutPoint>& vOutpts) +void CWallet::ListLockedCoins(std::vector<COutPoint>& vOutpts) const { AssertLockHeld(cs_wallet); // setLockedCoins for (std::set<COutPoint>::iterator it = setLockedCoins.begin(); @@ -3611,6 +3693,20 @@ bool CWallet::GetDestData(const CTxDestination &dest, const std::string &key, st return false; } +std::vector<std::string> CWallet::GetDestValues(const std::string& prefix) const +{ + LOCK(cs_wallet); + std::vector<std::string> values; + for (const auto& address : mapAddressBook) { + for (const auto& data : address.second.destdata) { + if (!data.first.compare(0, prefix.size(), prefix)) { + values.emplace_back(data.second); + } + } + } + return values; +} + std::string CWallet::GetWalletHelpString(bool showDebug) { std::string strUsage = HelpMessageGroup(_("Wallet options:")); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 69f51b3f64..11b2f7a663 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -821,6 +821,16 @@ public: void AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe=true, const CCoinControl *coinControl = NULL, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t& nMaximumCount = 0, const int& nMinDepth = 0, const int& nMaxDepth = 9999999) const; /** + * Return list of available coins and locked coins grouped by non-change output address. + */ + std::map<CTxDestination, std::vector<COutput>> ListCoins() const; + + /** + * Find non-change parent output. + */ + const CTxOut& FindNonChangeParentOutput(const CTransaction& tx, int output) const; + + /** * Shuffle and select coins until nTargetValue is reached while avoiding * small change; This method is stochastic for some inputs and upon * completion the coin set and corresponding actual target value is @@ -834,7 +844,7 @@ public: void LockCoin(const COutPoint& output); void UnlockCoin(const COutPoint& output); void UnlockAllCoins(); - void ListLockedCoins(std::vector<COutPoint>& vOutpts); + void ListLockedCoins(std::vector<COutPoint>& vOutpts) const; /* * Rescan abort properties @@ -873,6 +883,8 @@ public: bool LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value); //! Look up a destination data tuple in the store, return true if found false otherwise bool GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const; + //! Get all destination values matching a prefix. + std::vector<std::string> GetDestValues(const std::string& prefix) const; //! Adds a watch-only address to the store, and saves it to disk. bool AddWatchOnly(const CScript& dest, int64_t nCreateTime); @@ -917,6 +929,7 @@ public: CAmount GetUnconfirmedWatchOnlyBalance() const; CAmount GetImmatureWatchOnlyBalance() const; CAmount GetLegacyBalance(const isminefilter& filter, int minDepth, const std::string* account) const; + CAmount GetAvailableBalance(const CCoinControl* coinControl = nullptr) const; /** * Insert additional inputs into the transaction by @@ -1066,6 +1079,9 @@ public: /** Set whether this wallet broadcasts transactions. */ void SetBroadcastTransactions(bool broadcast) { fBroadcastTransactions = broadcast; } + /** Return whether transaction can be abandoned */ + bool TransactionCanBeAbandoned(const uint256& hashTx) const; + /* Mark a transaction (and it in-wallet descendants) as abandoned so its inputs may be respent. */ bool AbandonTransaction(const uint256& hashTx); diff --git a/test/functional/bip9-softforks.py b/test/functional/bip9-softforks.py index 1b2dff63d2..fff47fcca9 100755 --- a/test/functional/bip9-softforks.py +++ b/test/functional/bip9-softforks.py @@ -99,12 +99,43 @@ class BIP9SoftForksTest(ComparisonTestFramework): assert_equal(self.get_bip9_status(bipName)['status'], 'started') assert_equal(self.get_bip9_status(bipName)['since'], 144) + assert_equal(self.get_bip9_status(bipName)['statistics']['elapsed'], 0) + assert_equal(self.get_bip9_status(bipName)['statistics']['count'], 0) tmpl = self.nodes[0].getblocktemplate({}) assert(bipName not in tmpl['rules']) assert_equal(tmpl['vbavailable'][bipName], bitno) assert_equal(tmpl['vbrequired'], 0) assert(tmpl['version'] & activated_version) + # Test 1-A + # check stats after max number of "signalling not" blocks such that LOCKED_IN still possible this period + test_blocks = self.generate_blocks(36, 4, test_blocks) # 0x00000004 (signalling not) + test_blocks = self.generate_blocks(10, activated_version) # 0x20000001 (signalling ready) + yield TestInstance(test_blocks, sync_every_block=False) + + assert_equal(self.get_bip9_status(bipName)['statistics']['elapsed'], 46) + assert_equal(self.get_bip9_status(bipName)['statistics']['count'], 10) + assert_equal(self.get_bip9_status(bipName)['statistics']['possible'], True) + + # Test 1-B + # check stats after one additional "signalling not" block -- LOCKED_IN no longer possible this period + test_blocks = self.generate_blocks(1, 4, test_blocks) # 0x00000004 (signalling not) + yield TestInstance(test_blocks, sync_every_block=False) + + assert_equal(self.get_bip9_status(bipName)['statistics']['elapsed'], 47) + assert_equal(self.get_bip9_status(bipName)['statistics']['count'], 10) + assert_equal(self.get_bip9_status(bipName)['statistics']['possible'], False) + + # Test 1-C + # finish period with "ready" blocks, but soft fork will still fail to advance to LOCKED_IN + test_blocks = self.generate_blocks(97, activated_version) # 0x20000001 (signalling ready) + yield TestInstance(test_blocks, sync_every_block=False) + + assert_equal(self.get_bip9_status(bipName)['statistics']['elapsed'], 0) + assert_equal(self.get_bip9_status(bipName)['statistics']['count'], 0) + assert_equal(self.get_bip9_status(bipName)['statistics']['possible'], True) + assert_equal(self.get_bip9_status(bipName)['status'], 'started') + # Test 2 # Fail to achieve LOCKED_IN 100 out of 144 signal bit 1 # using a variety of bits to simulate multiple parallel softforks @@ -116,6 +147,8 @@ class BIP9SoftForksTest(ComparisonTestFramework): assert_equal(self.get_bip9_status(bipName)['status'], 'started') assert_equal(self.get_bip9_status(bipName)['since'], 144) + assert_equal(self.get_bip9_status(bipName)['statistics']['elapsed'], 0) + assert_equal(self.get_bip9_status(bipName)['statistics']['count'], 0) tmpl = self.nodes[0].getblocktemplate({}) assert(bipName not in tmpl['rules']) assert_equal(tmpl['vbavailable'][bipName], bitno) @@ -125,14 +158,24 @@ class BIP9SoftForksTest(ComparisonTestFramework): # Test 3 # 108 out of 144 signal bit 1 to achieve LOCKED_IN # using a variety of bits to simulate multiple parallel softforks - test_blocks = self.generate_blocks(58, activated_version) # 0x20000001 (signalling ready) + test_blocks = self.generate_blocks(57, activated_version) # 0x20000001 (signalling ready) test_blocks = self.generate_blocks(26, 4, test_blocks) # 0x00000004 (signalling not) test_blocks = self.generate_blocks(50, activated_version, test_blocks) # 0x20000101 (signalling ready) test_blocks = self.generate_blocks(10, 4, test_blocks) # 0x20010000 (signalling not) yield TestInstance(test_blocks, sync_every_block=False) + # check counting stats and "possible" flag before last block of this period achieves LOCKED_IN... + assert_equal(self.get_bip9_status(bipName)['statistics']['elapsed'], 143) + assert_equal(self.get_bip9_status(bipName)['statistics']['count'], 107) + assert_equal(self.get_bip9_status(bipName)['statistics']['possible'], True) + assert_equal(self.get_bip9_status(bipName)['status'], 'started') + + # ...continue with Test 3 + test_blocks = self.generate_blocks(1, activated_version) # 0x20000001 (signalling ready) + yield TestInstance(test_blocks, sync_every_block=False) + assert_equal(self.get_bip9_status(bipName)['status'], 'locked_in') - assert_equal(self.get_bip9_status(bipName)['since'], 432) + assert_equal(self.get_bip9_status(bipName)['since'], 576) tmpl = self.nodes[0].getblocktemplate({}) assert(bipName not in tmpl['rules']) @@ -142,7 +185,7 @@ class BIP9SoftForksTest(ComparisonTestFramework): yield TestInstance(test_blocks, sync_every_block=False) assert_equal(self.get_bip9_status(bipName)['status'], 'locked_in') - assert_equal(self.get_bip9_status(bipName)['since'], 432) + assert_equal(self.get_bip9_status(bipName)['since'], 576) tmpl = self.nodes[0].getblocktemplate({}) assert(bipName not in tmpl['rules']) @@ -168,7 +211,7 @@ class BIP9SoftForksTest(ComparisonTestFramework): yield TestInstance([[block, True]]) assert_equal(self.get_bip9_status(bipName)['status'], 'active') - assert_equal(self.get_bip9_status(bipName)['since'], 576) + assert_equal(self.get_bip9_status(bipName)['since'], 720) tmpl = self.nodes[0].getblocktemplate({}) assert(bipName in tmpl['rules']) assert(bipName not in tmpl['vbavailable']) diff --git a/test/functional/blockchain.py b/test/functional/blockchain.py index 8596f1edaf..b6112c7280 100755 --- a/test/functional/blockchain.py +++ b/test/functional/blockchain.py @@ -32,7 +32,7 @@ class BlockchainTest(BitcoinTestFramework): def __init__(self): super().__init__() self.setup_clean_chain = False - self.num_nodes = 2 + self.num_nodes = 1 def run_test(self): self._test_gettxoutsetinfo() @@ -50,9 +50,33 @@ class BlockchainTest(BitcoinTestFramework): assert_equal(res['height'], 200) assert_equal(res['txouts'], 200) assert_equal(res['bytes_serialized'], 13924), + assert_equal(res['bestblock'], node.getblockhash(200)) assert_equal(len(res['bestblock']), 64) assert_equal(len(res['hash_serialized']), 64) + self.log.info("Test that gettxoutsetinfo() works for blockchain with just the genesis block") + b1hash = node.getblockhash(1) + node.invalidateblock(b1hash) + + res2 = node.gettxoutsetinfo() + assert_equal(res2['transactions'], 0) + assert_equal(res2['total_amount'], Decimal('0')) + assert_equal(res2['height'], 0) + assert_equal(res2['txouts'], 0) + assert_equal(res2['bestblock'], node.getblockhash(0)) + assert_equal(len(res2['hash_serialized']), 64) + + self.log.info("Test that gettxoutsetinfo() returns the same result after invalidate/reconsider block") + node.reconsiderblock(b1hash) + + res3 = node.gettxoutsetinfo() + assert_equal(res['total_amount'], res3['total_amount']) + assert_equal(res['transactions'], res3['transactions']) + assert_equal(res['height'], res3['height']) + assert_equal(res['txouts'], res3['txouts']) + assert_equal(res['bestblock'], res3['bestblock']) + assert_equal(res['hash_serialized'], res3['hash_serialized']) + def _test_getblockheader(self): node = self.nodes[0] diff --git a/test/functional/bumpfee.py b/test/functional/bumpfee.py index 54fd7740c1..aaed420690 100755 --- a/test/functional/bumpfee.py +++ b/test/functional/bumpfee.py @@ -88,6 +88,7 @@ def test_simple_bumpfee_succeeds(rbf_node, peer_node, dest_address): sync_mempools((rbf_node, peer_node)) assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool() bumped_tx = rbf_node.bumpfee(rbfid) + assert_equal(bumped_tx["errors"], []) assert bumped_tx["fee"] - abs(rbftx["fee"]) > 0 # check that bumped_tx propogates, original tx was evicted and has a wallet conflict sync_mempools((rbf_node, peer_node)) diff --git a/test/functional/pruning.py b/test/functional/pruning.py index 17019c658b..7995e418c9 100755 --- a/test/functional/pruning.py +++ b/test/functional/pruning.py @@ -34,10 +34,11 @@ class PruneTest(BitcoinTestFramework): # Create nodes 0 and 1 to mine. # Create node 2 to test pruning. + self.full_node_default_args = ["-maxreceivebuffer=20000","-blockmaxsize=999000", "-checkblocks=5", "-limitdescendantcount=100", "-limitdescendantsize=5000", "-limitancestorcount=100", "-limitancestorsize=5000" ] # Create nodes 3 and 4 to test manual pruning (they will be re-started with manual pruning later) # Create nodes 5 to test wallet in prune mode, but do not connect - self.extra_args = [["-maxreceivebuffer=20000", "-blockmaxsize=999000", "-checkblocks=5"], - ["-maxreceivebuffer=20000", "-blockmaxsize=999000", "-checkblocks=5"], + self.extra_args = [self.full_node_default_args, + self.full_node_default_args, ["-maxreceivebuffer=20000", "-prune=550"], ["-maxreceivebuffer=20000", "-blockmaxsize=999000"], ["-maxreceivebuffer=20000", "-blockmaxsize=999000"], @@ -97,12 +98,15 @@ class PruneTest(BitcoinTestFramework): # Node 2 stays connected, so it hears about the stale blocks and then reorg's when node0 reconnects # Stopping node 0 also clears its mempool, so it doesn't have node1's transactions to accidentally mine self.stop_node(0) - self.nodes[0]=start_node(0, self.options.tmpdir, ["-maxreceivebuffer=20000","-blockmaxsize=999000", "-checkblocks=5"], timewait=900) + self.nodes[0]=start_node(0, self.options.tmpdir, self.full_node_default_args, timewait=900) # Mine 24 blocks in node 1 for i in range(24): if j == 0: mine_large_block(self.nodes[1], self.utxo_cache_1) else: + # Add node1's wallet transactions back to the mempool, to + # avoid the mined blocks from being too small. + self.nodes[1].resendwallettransactions() self.nodes[1].generate(1) #tx's already in mempool from previous disconnects # Reorg back with 25 block chain from node 0 @@ -159,6 +163,11 @@ class PruneTest(BitcoinTestFramework): self.log.info("Usage possibly still high bc of stale blocks in block files: %d" % calc_usage(self.prunedir)) self.log.info("Mine 220 more blocks so we have requisite history (some blocks will be big and cause pruning of previous chain)") + + # Get node0's wallet transactions back in its mempool, to avoid the + # mined blocks from being too small. + self.nodes[0].resendwallettransactions() + for i in range(22): # This can be slow, so do this in multiple RPC calls to avoid # RPC timeouts. |