diff options
Diffstat (limited to 'src')
92 files changed, 542 insertions, 312 deletions
diff --git a/src/addrdb.cpp b/src/addrdb.cpp index c6083f5554..db936486b6 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -44,18 +44,30 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data fs::path pathTmp = GetDataDir() / tmpfn; FILE *file = fsbridge::fopen(pathTmp, "wb"); CAutoFile fileout(file, SER_DISK, CLIENT_VERSION); - if (fileout.IsNull()) + if (fileout.IsNull()) { + fileout.fclose(); + remove(pathTmp); return error("%s: Failed to open file %s", __func__, pathTmp.string()); + } // Serialize - if (!SerializeDB(fileout, data)) return false; - if (!FileCommit(fileout.Get())) + if (!SerializeDB(fileout, data)) { + fileout.fclose(); + remove(pathTmp); + return false; + } + if (!FileCommit(fileout.Get())) { + fileout.fclose(); + remove(pathTmp); return error("%s: Failed to flush file %s", __func__, pathTmp.string()); + } fileout.fclose(); // replace existing file, if any, with new file - if (!RenameOver(pathTmp, path)) + if (!RenameOver(pathTmp, path)) { + remove(pathTmp); return error("%s: Rename-into-place failed", __func__); + } return true; } diff --git a/src/logging.cpp b/src/logging.cpp index 418a701179..dc2d130a2a 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -75,6 +75,14 @@ bool BCLog::Logger::StartLogging() return true; } +void BCLog::Logger::DisconnectTestLogger() +{ + std::lock_guard<std::mutex> scoped_lock(m_cs); + m_buffering = true; + if (m_fileout != nullptr) fclose(m_fileout); + m_fileout = nullptr; +} + void BCLog::Logger::EnableCategory(BCLog::LogFlags flag) { m_categories |= flag; diff --git a/src/logging.h b/src/logging.h index 36b1a4045d..75cd5353c0 100644 --- a/src/logging.h +++ b/src/logging.h @@ -100,6 +100,8 @@ namespace BCLog { /** Start logging (and flush all buffered messages) */ bool StartLogging(); + /** Only for testing */ + void DisconnectTestLogger(); void ShrinkDebugFile(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5ff456fcb0..4b43b2cdf2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1555,6 +1555,11 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm } } + // Unknown types in the GetData stay in vRecvGetData and block any future + // message from this peer, see vRecvGetData check in ProcessMessages(). + // Depending on future p2p changes, we might either drop unknown getdata on + // the floor or disconnect the peer. + pfrom->vRecvGetData.erase(pfrom->vRecvGetData.begin(), it); if (!vNotFound.empty()) { @@ -3260,6 +3265,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter return false; // this maintains the order of responses + // and prevents vRecvGetData to grow unbounded if (!pfrom->vRecvGetData.empty()) return true; if (!pfrom->orphan_work_set.empty()) return true; diff --git a/src/noui.cpp b/src/noui.cpp index 0c18b0e231..caab9f326e 100644 --- a/src/noui.cpp +++ b/src/noui.cpp @@ -18,26 +18,30 @@ bool noui_ThreadSafeMessageBox(const std::string& message, const std::string& ca { bool fSecure = style & CClientUIInterface::SECURE; style &= ~CClientUIInterface::SECURE; + bool prefix = !(style & CClientUIInterface::MSG_NOPREFIX); + style &= ~CClientUIInterface::MSG_NOPREFIX; std::string strCaption; - // Check for usage of predefined caption - switch (style) { - case CClientUIInterface::MSG_ERROR: - strCaption += _("Error"); - break; - case CClientUIInterface::MSG_WARNING: - strCaption += _("Warning"); - break; - case CClientUIInterface::MSG_INFORMATION: - strCaption += _("Information"); - break; - default: - strCaption += caption; // Use supplied caption (can be empty) + if (prefix) { + switch (style) { + case CClientUIInterface::MSG_ERROR: + strCaption = "Error: "; + break; + case CClientUIInterface::MSG_WARNING: + strCaption = "Warning: "; + break; + case CClientUIInterface::MSG_INFORMATION: + strCaption = "Information: "; + break; + default: + strCaption = caption + ": "; // Use supplied caption (can be empty) + } } - if (!fSecure) - LogPrintf("%s: %s\n", strCaption, message); - tfm::format(std::cerr, "%s: %s\n", strCaption.c_str(), message.c_str()); + if (!fSecure) { + LogPrintf("%s%s\n", strCaption, message); + } + tfm::format(std::cerr, "%s%s\n", strCaption.c_str(), message.c_str()); return false; } diff --git a/src/policy/fees.h b/src/policy/fees.h index 6e61f76178..16683bf5ad 100644 --- a/src/policy/fees.h +++ b/src/policy/fees.h @@ -43,7 +43,6 @@ enum class FeeReason { PAYTXFEE, FALLBACK, REQUIRED, - MAXTXFEE, }; /* Used to determine type of fee estimation requested */ diff --git a/src/psbt.cpp b/src/psbt.cpp index d765133190..fe74002e82 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -212,6 +212,25 @@ bool PSBTInputSigned(const PSBTInput& input) return !input.final_script_sig.empty() || !input.final_script_witness.IsNull(); } +void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index) +{ + const CTxOut& out = psbt.tx->vout.at(index); + PSBTOutput& psbt_out = psbt.outputs.at(index); + + // Fill a SignatureData with output info + SignatureData sigdata; + psbt_out.FillSignatureData(sigdata); + + // Construct a would-be spend of this output, to update sigdata with. + // Note that ProduceSignature is used to fill in metadata (not actual signatures), + // so provider does not need to provide any private keys (it can be a HidingSigningProvider). + MutableTransactionSignatureCreator creator(psbt.tx.get_ptr(), /* index */ 0, out.nValue, SIGHASH_ALL); + ProduceSignature(provider, creator, out.scriptPubKey, sigdata); + + // Put redeem_script, witness_script, key paths, into PSBTOutput. + psbt_out.FromSignatureData(sigdata); +} + bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash, SignatureData* out_sigdata, bool use_dummy) { PSBTInput& input = psbt.inputs.at(index); diff --git a/src/psbt.h b/src/psbt.h index 1bc1e91a84..f3840b9ed3 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -565,6 +565,12 @@ bool PSBTInputSigned(const PSBTInput& input); /** Signs a PSBTInput, verifying that all provided data matches what is being signed. */ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr, bool use_dummy = false); +/** Updates a PSBTOutput with information from provider. + * + * This fills in the redeem_script, witness_script, and hd_keypaths where possible. + */ +void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index); + /** * Finalizes a PSBT if possible, combining partial signatures. * diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 0c232f89fe..2fdbcca043 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -330,7 +330,7 @@ void BitcoinApplication::initializeResult(bool success) if(success) { // Log this only after AppInitMain finishes, as then logging setup is guaranteed complete - qWarning() << "Platform customization:" << platformStyle->getName(); + qInfo() << "Platform customization:" << platformStyle->getName(); #ifdef ENABLE_WALLET m_wallet_controller = new WalletController(m_node, platformStyle, optionsModel, this); #ifdef ENABLE_BIP70 @@ -436,16 +436,17 @@ int GuiMain(int argc, char* argv[]) Q_INIT_RESOURCE(bitcoin); Q_INIT_RESOURCE(bitcoin_locale); - BitcoinApplication app(*node, argc, argv); // Generate high-dpi pixmaps QApplication::setAttribute(Qt::AA_UseHighDpiPixmaps); #if QT_VERSION >= 0x050600 - QGuiApplication::setAttribute(Qt::AA_EnableHighDpiScaling); + QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling); #endif #ifdef Q_OS_MAC QApplication::setAttribute(Qt::AA_DontShowIconsInMenus); #endif + BitcoinApplication app(*node, argc, argv); + // Register meta types used for QMetaObject::invokeMethod qRegisterMetaType< bool* >(); // Need to pass name here as CAmount is a typedef (see http://qt-project.org/doc/qt-5/qmetatype.html#qRegisterMetaType) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 1444dddeb1..babb2ce518 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -337,6 +337,7 @@ void BitcoinGUI::createActions() m_open_wallet_action = new QAction(tr("Open Wallet"), this); m_open_wallet_action->setEnabled(false); m_open_wallet_action->setStatusTip(tr("Open a wallet")); + m_open_wallet_menu = new QMenu(this); m_close_wallet_action = new QAction(tr("Close Wallet..."), this); m_close_wallet_action->setStatusTip(tr("Close wallet")); @@ -368,13 +369,13 @@ void BitcoinGUI::createActions() connect(usedSendingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedSendingAddresses); connect(usedReceivingAddressesAction, &QAction::triggered, walletFrame, &WalletFrame::usedReceivingAddresses); connect(openAction, &QAction::triggered, this, &BitcoinGUI::openClicked); - connect(m_open_wallet_action->menu(), &QMenu::aboutToShow, [this] { - m_open_wallet_action->menu()->clear(); + connect(m_open_wallet_menu, &QMenu::aboutToShow, [this] { + m_open_wallet_menu->clear(); std::vector<std::string> available_wallets = m_wallet_controller->getWalletsAvailableToOpen(); std::vector<std::string> wallets = m_node.listWalletDir(); for (const auto& path : wallets) { QString name = path.empty() ? QString("["+tr("default wallet")+"]") : QString::fromStdString(path); - QAction* action = m_open_wallet_action->menu()->addAction(name); + QAction* action = m_open_wallet_menu->addAction(name); if (std::find(available_wallets.begin(), available_wallets.end(), path) == available_wallets.end()) { // This wallet is already loaded @@ -410,7 +411,7 @@ void BitcoinGUI::createActions() }); } if (wallets.empty()) { - QAction* action = m_open_wallet_action->menu()->addAction(tr("No wallets available")); + QAction* action = m_open_wallet_menu->addAction(tr("No wallets available")); action->setEnabled(false); } }); @@ -634,7 +635,7 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller) m_wallet_controller = wallet_controller; m_open_wallet_action->setEnabled(true); - m_open_wallet_action->setMenu(new QMenu(this)); + m_open_wallet_action->setMenu(m_open_wallet_menu); connect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet); connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet); @@ -1038,49 +1039,51 @@ void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVer progressBar->setToolTip(tooltip); } -void BitcoinGUI::message(const QString &title, const QString &message, unsigned int style, bool *ret) +void BitcoinGUI::message(const QString& title, QString message, unsigned int style, bool* ret) { - QString strTitle = tr("Bitcoin"); // default title + // Default title. On macOS, the window title is ignored (as required by the macOS Guidelines). + QString strTitle{PACKAGE_NAME}; // Default to information icon int nMBoxIcon = QMessageBox::Information; int nNotifyIcon = Notificator::Information; - QString msgType; + bool prefix = !(style & CClientUIInterface::MSG_NOPREFIX); + style &= ~CClientUIInterface::MSG_NOPREFIX; - // Prefer supplied title over style based title + QString msgType; if (!title.isEmpty()) { msgType = title; - } - else { + } else { switch (style) { case CClientUIInterface::MSG_ERROR: msgType = tr("Error"); + if (prefix) message = tr("Error: %1").arg(message); break; case CClientUIInterface::MSG_WARNING: msgType = tr("Warning"); + if (prefix) message = tr("Warning: %1").arg(message); break; case CClientUIInterface::MSG_INFORMATION: msgType = tr("Information"); + // No need to prepend the prefix here. break; default: break; } } - // Append title to "Bitcoin - " - if (!msgType.isEmpty()) + + if (!msgType.isEmpty()) { strTitle += " - " + msgType; + } - // Check for error/warning icon if (style & CClientUIInterface::ICON_ERROR) { nMBoxIcon = QMessageBox::Critical; nNotifyIcon = Notificator::Critical; - } - else if (style & CClientUIInterface::ICON_WARNING) { + } else if (style & CClientUIInterface::ICON_WARNING) { nMBoxIcon = QMessageBox::Warning; nNotifyIcon = Notificator::Warning; } - // Display message if (style & CClientUIInterface::MODAL) { // Check for buttons, use OK as default, if none was supplied QMessageBox::StandardButton buttons; @@ -1093,9 +1096,9 @@ void BitcoinGUI::message(const QString &title, const QString &message, unsigned int r = mBox.exec(); if (ret != nullptr) *ret = r == QMessageBox::Ok; - } - else + } else { notificator->notify(static_cast<Notificator::Class>(nNotifyIcon), strTitle, message); + } } void BitcoinGUI::changeEvent(QEvent *e) diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index b58ccbb455..46ced79007 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -148,6 +148,7 @@ private: QAction* openAction = nullptr; QAction* showHelpMessageAction = nullptr; QAction* m_open_wallet_action{nullptr}; + QMenu* m_open_wallet_menu{nullptr}; QAction* m_close_wallet_action{nullptr}; QAction* m_wallet_selector_label_action = nullptr; QAction* m_wallet_selector_action = nullptr; @@ -219,7 +220,7 @@ public Q_SLOTS: @see CClientUIInterface::MessageBoxFlags @param[in] ret pointer to a bool that will be modified to whether Ok was clicked (modal only) */ - void message(const QString &title, const QString &message, unsigned int style, bool *ret = nullptr); + void message(const QString& title, QString message, unsigned int style, bool* ret = nullptr); #ifdef ENABLE_WALLET void setCurrentWallet(WalletModel* wallet_model); diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 5b4fb4cc18..f3974b1c85 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -172,7 +172,7 @@ static void CopySettings(QSettings& dst, const QSettings& src) /** Back up a QSettings to an ini-formatted file. */ static void BackupSettings(const fs::path& filename, const QSettings& src) { - qWarning() << "Backing up GUI settings to" << GUIUtil::boostPathToQString(filename); + qInfo() << "Backing up GUI settings to" << GUIUtil::boostPathToQString(filename); QSettings dst(GUIUtil::boostPathToQString(filename), QSettings::IniFormat); dst.clear(); CopySettings(dst, src); diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 43dccec4ea..c99515fe1c 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -488,7 +488,7 @@ void PaymentServer::LoadRootCAs(X509_STORE* _store) continue; } } - qWarning() << "PaymentServer::LoadRootCAs: Loaded " << nRootCerts << " root certificates"; + qInfo() << "PaymentServer::LoadRootCAs: Loaded " << nRootCerts << " root certificates"; // Project for another day: // Fetch certificate revocation lists, and add them to certStore. diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index ea1019ad1d..11a518ebd2 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -12,7 +12,6 @@ #include <key.h> #include <key_io.h> -#include <pubkey.h> #include <wallet/wallet.h> #include <QApplication> diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index b9bf933ee5..a900ec0198 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -5,6 +5,7 @@ #include <qt/test/apptests.h> #include <chainparams.h> +#include <key.h> #include <qt/bitcoin.h> #include <qt/bitcoingui.h> #include <qt/networkstyle.h> @@ -25,7 +26,6 @@ #include <QtGlobal> #include <QtTest/QtTestWidgets> #include <QtTest/QtTestGui> -#include <new> #include <string> #include <univalue.h> @@ -62,6 +62,9 @@ void AppTests::appTests() } #endif + ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference + LogInstance().DisconnectTestLogger(); + m_app.parameterSetup(); m_app.createOptionsModel(true /* reset settings */); QScopedPointer<const NetworkStyle> style( diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 7a10ada438..3735f41f9d 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -4,7 +4,6 @@ #include <qt/test/rpcnestedtests.h> -#include <fs.h> #include <interfaces/node.h> #include <rpc/server.h> #include <qt/rpcconsole.h> @@ -35,6 +34,9 @@ void RPCNestedTests::rpcNestedTests() tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]); //mempool.setSanityCheck(1.0); + ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference + LogInstance().DisconnectTestLogger(); + TestingSetup test; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); diff --git a/src/qt/test/rpcnestedtests.h b/src/qt/test/rpcnestedtests.h index e33f4e3da1..97143ff78a 100644 --- a/src/qt/test/rpcnestedtests.h +++ b/src/qt/test/rpcnestedtests.h @@ -8,9 +8,6 @@ #include <QObject> #include <QTest> -#include <txdb.h> -#include <txmempool.h> - class RPCNestedTests : public QObject { Q_OBJECT diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index a2bf53973b..79d88ab742 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -6,14 +6,13 @@ #include <config/bitcoin-config.h> #endif -#include <chainparams.h> #include <interfaces/node.h> #include <qt/bitcoin.h> #include <qt/test/apptests.h> #include <qt/test/rpcnestedtests.h> -#include <util/system.h> #include <qt/test/uritests.h> #include <qt/test/compattests.h> +#include <test/setup_common.h> #ifdef ENABLE_WALLET #include <qt/test/addressbooktests.h> @@ -48,14 +47,8 @@ extern void noui_connect(); // This is all you need to run all the tests int main(int argc, char *argv[]) { - SetupEnvironment(); - SetupNetworking(); - SelectParams(CBaseChainParams::REGTEST); - noui_connect(); - ClearDatadirCache(); - fs::path pathTemp = fs::temp_directory_path() / strprintf("test_bitcoin-qt_%lu_%i", (unsigned long)GetTime(), (int)GetRand(100000)); - fs::create_directories(pathTemp); - gArgs.ForceSetArg("-datadir", pathTemp.string()); + BasicTestingSetup test{CBaseChainParams::REGTEST}; + auto node = interfaces::MakeNode(); bool fInvalid = false; @@ -109,7 +102,5 @@ int main(int argc, char *argv[]) } #endif - fs::remove_all(pathTemp); - return fInvalid; } diff --git a/src/qt/trafficgraphwidget.cpp b/src/qt/trafficgraphwidget.cpp index 1588be8da3..006007be63 100644 --- a/src/qt/trafficgraphwidget.cpp +++ b/src/qt/trafficgraphwidget.cpp @@ -104,6 +104,7 @@ void TrafficGraphWidget::paintEvent(QPaintEvent *) } } + painter.setRenderHint(QPainter::Antialiasing); if(!vSamplesIn.empty()) { QPainterPath p; paintPath(p, vSamplesIn); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index a2b295df21..c1eba61749 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -221,9 +221,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact return TransactionCreationFailed; } - // reject absurdly high fee. (This can never happen because the - // wallet caps the fee at m_default_max_tx_fee. This merely serves as a - // belt-and-suspenders check) + // Reject absurdly high fee if (nFeeRequired > m_wallet->getDefaultMaxTxFee()) return AbsurdFee; } diff --git a/src/qt/winshutdownmonitor.cpp b/src/qt/winshutdownmonitor.cpp index 08cae76add..b177b22b3f 100644 --- a/src/qt/winshutdownmonitor.cpp +++ b/src/qt/winshutdownmonitor.cpp @@ -63,7 +63,7 @@ void WinShutdownMonitor::registerShutdownBlockReason(const QString& strReason, c } if (shutdownBRCreate(mainWinId, strReason.toStdWString().c_str())) - qWarning() << "registerShutdownBlockReason: Successfully registered: " + strReason; + qInfo() << "registerShutdownBlockReason: Successfully registered: " + strReason; else qWarning() << "registerShutdownBlockReason: Failed to register: " + strReason; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 50c4589d9f..9f3748cb65 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2247,41 +2247,12 @@ UniValue scantxoutset(const JSONRPCRequest& request) // loop through the scan objects for (const UniValue& scanobject : request.params[1].get_array().getValues()) { - std::string desc_str; - std::pair<int64_t, int64_t> range = {0, 1000}; - if (scanobject.isStr()) { - desc_str = scanobject.get_str(); - } else if (scanobject.isObject()) { - UniValue desc_uni = find_value(scanobject, "desc"); - if (desc_uni.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor needs to be provided in scan object"); - desc_str = desc_uni.get_str(); - UniValue range_uni = find_value(scanobject, "range"); - if (!range_uni.isNull()) { - range = ParseDescriptorRange(range_uni); - } - } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); - } - FlatSigningProvider provider; - auto desc = Parse(desc_str, provider); - if (!desc) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s'", desc_str)); - } - if (!desc->IsRange()) { - range.first = 0; - range.second = 0; - } - for (int i = range.first; i <= range.second; ++i) { - std::vector<CScript> scripts; - if (!desc->Expand(i, provider, scripts, provider)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys: '%s'", desc_str)); - } - for (const auto& script : scripts) { - std::string inferred = InferDescriptor(script, provider)->ToString(); - needles.emplace(script); - descriptors.emplace(std::move(script), std::move(inferred)); - } + auto scripts = EvalDescriptorStringOrObject(scanobject, provider); + for (const auto& script : scripts) { + std::string inferred = InferDescriptor(script, provider)->ToString(); + needles.emplace(script); + descriptors.emplace(std::move(script), std::move(inferred)); } } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 1dfaecc951..6636a8a29a 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -468,7 +468,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) nTransactionsUpdatedLastLP = nTransactionsUpdatedLast; } - // Release the wallet and main lock while waiting + // Release lock while waiting LEAVE_CRITICAL_SECTION(cs_main); { checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); @@ -479,6 +479,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) if (g_best_block_cv.wait_until(lock, checktxtime) == std::cv_status::timeout) { // Timeout: Check transactions for update + // without holding ::mempool.cs to avoid deadlocks if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP) break; checktxtime += std::chrono::seconds(10); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 9da24afe79..966ff3fedc 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1495,12 +1495,19 @@ UniValue converttopsbt(const JSONRPCRequest& request) UniValue utxoupdatepsbt(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 1) { + if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) { throw std::runtime_error( RPCHelpMan{"utxoupdatepsbt", - "\nUpdates a PSBT with witness UTXOs retrieved from the UTXO set or the mempool.\n", + "\nUpdates all segwit inputs and outputs in a PSBT with data from output descriptors, the UTXO set or the mempool.\n", { - {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"} + {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"}, + {"descriptors", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "An array of either strings or objects", { + {"", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "An output descriptor"}, + {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "An object with an output descriptor and extra information", { + {"desc", RPCArg::Type::STR, RPCArg::Optional::NO, "An output descriptor"}, + {"range", RPCArg::Type::RANGE, "1000", "Up to what index HD chains should be explored (either end or [begin,end])"}, + }}, + }}, }, RPCResult { " \"psbt\" (string) The base64-encoded partially signed transaction with inputs updated\n" @@ -1510,7 +1517,7 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) }}.ToString()); } - RPCTypeCheck(request.params, {UniValue::VSTR}, true); + RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR}, true); // Unserialize the transactions PartiallySignedTransaction psbtx; @@ -1519,6 +1526,17 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } + // Parse descriptors, if any. + FlatSigningProvider provider; + if (!request.params[1].isNull()) { + auto descs = request.params[1].get_array(); + for (size_t i = 0; i < descs.size(); ++i) { + EvalDescriptorStringOrObject(descs[i], provider); + } + } + // We don't actually need private keys further on; hide them as a precaution. + HidingSigningProvider public_provider(&provider, /* nosign */ true, /* nobip32derivs */ false); + // Fetch previous transactions (inputs): CCoinsView viewDummy; CCoinsViewCache view(&viewDummy); @@ -1545,11 +1563,19 @@ UniValue utxoupdatepsbt(const JSONRPCRequest& request) const Coin& coin = view.AccessCoin(psbtx.tx->vin[i].prevout); - std::vector<std::vector<unsigned char>> solutions_data; - txnouttype which_type = Solver(coin.out.scriptPubKey, solutions_data); - if (which_type == TX_WITNESS_V0_SCRIPTHASH || which_type == TX_WITNESS_V0_KEYHASH || which_type == TX_WITNESS_UNKNOWN) { + if (IsSegWitOutput(provider, coin.out.scriptPubKey)) { input.witness_utxo = coin.out; } + + // Update script/keypath information using descriptor data. + // Note that SignPSBTInput does a lot more than just constructing ECDSA signatures + // we don't actually care about those here, in fact. + SignPSBTInput(public_provider, psbtx, i, /* sighash_type */ 1); + } + + // Update script/keypath information using descriptor data. + for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { + UpdatePSBTOutput(public_provider, psbtx, i); } CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index 9c4cdc3a90..14fcb628eb 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -221,6 +221,9 @@ UniValue SignTransaction(CMutableTransaction& mtx, const UniValue& prevTxsUnival // Automatically also add the P2WSH wrapped version of the script (to deal with P2SH-P2WSH). keystore->AddCScript(GetScriptForWitness(witnessScript)); } + if (rs.isNull() && ws.isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Missing redeemScript/witnessScript"); + } } } } diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 4642cf16b1..67ccb225b5 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -6,6 +6,7 @@ #include <keystore.h> #include <outputtype.h> #include <rpc/util.h> +#include <script/descriptor.h> #include <tinyformat.h> #include <util/strencodings.h> @@ -697,3 +698,40 @@ std::pair<int64_t, int64_t> ParseDescriptorRange(const UniValue& value) } return {low, high}; } + +std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider) +{ + std::string desc_str; + std::pair<int64_t, int64_t> range = {0, 1000}; + if (scanobject.isStr()) { + desc_str = scanobject.get_str(); + } else if (scanobject.isObject()) { + UniValue desc_uni = find_value(scanobject, "desc"); + if (desc_uni.isNull()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Descriptor needs to be provided in scan object"); + desc_str = desc_uni.get_str(); + UniValue range_uni = find_value(scanobject, "range"); + if (!range_uni.isNull()) { + range = ParseDescriptorRange(range_uni); + } + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan object needs to be either a string or an object"); + } + + auto desc = Parse(desc_str, provider); + if (!desc) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid descriptor '%s'", desc_str)); + } + if (!desc->IsRange()) { + range.first = 0; + range.second = 0; + } + std::vector<CScript> ret; + for (int i = range.first; i <= range.second; ++i) { + std::vector<CScript> scripts; + if (!desc->Expand(i, provider, scripts, provider)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Cannot derive script without private keys: '%s'", desc_str)); + } + std::move(scripts.begin(), scripts.end(), std::back_inserter(ret)); + } + return ret; +} diff --git a/src/rpc/util.h b/src/rpc/util.h index 0eb2fef5c3..8762281d73 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -9,6 +9,8 @@ #include <outputtype.h> #include <pubkey.h> #include <rpc/protocol.h> +#include <script/script.h> +#include <script/sign.h> #include <script/standard.h> #include <univalue.h> @@ -84,6 +86,9 @@ UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_s //! Parse a JSON range specified as int64, or [int64, int64] std::pair<int64_t, int64_t> ParseDescriptorRange(const UniValue& value); +/** Evaluate a descriptor given as a string, or as a {"desc":...,"range":...} object, with default range of 1000. */ +std::vector<CScript> EvalDescriptorStringOrObject(const UniValue& scanobject, FlatSigningProvider& provider); + struct RPCArg { enum class Type { OBJ, diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 36dd68a3d8..5320dc0876 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -505,3 +505,19 @@ FlatSigningProvider Merge(const FlatSigningProvider& a, const FlatSigningProvide ret.origins.insert(b.origins.begin(), b.origins.end()); return ret; } + +bool IsSegWitOutput(const SigningProvider& provider, const CScript& script) +{ + std::vector<valtype> solutions; + auto whichtype = Solver(script, solutions); + if (whichtype == TX_WITNESS_V0_SCRIPTHASH || whichtype == TX_WITNESS_V0_KEYHASH || whichtype == TX_WITNESS_UNKNOWN) return true; + if (whichtype == TX_SCRIPTHASH) { + auto h160 = uint160(solutions[0]); + CScript subscript; + if (provider.GetCScript(h160, subscript)) { + whichtype = Solver(subscript, solutions); + if (whichtype == TX_WITNESS_V0_SCRIPTHASH || whichtype == TX_WITNESS_V0_KEYHASH || whichtype == TX_WITNESS_UNKNOWN) return true; + } + } + return false; +} diff --git a/src/script/sign.h b/src/script/sign.h index f746325b90..e5c0329a61 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -232,4 +232,7 @@ void UpdateInput(CTxIn& input, const SignatureData& data); * Solvability is unrelated to whether we consider this output to be ours. */ bool IsSolvable(const SigningProvider& provider, const CScript& script); +/** Check whether a scriptPubKey is known to be segwit. */ +bool IsSegWitOutput(const SigningProvider& provider, const CScript& script); + #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/test/allocator_tests.cpp b/src/test/allocator_tests.cpp index f255691704..e333763f27 100644 --- a/src/test/allocator_tests.cpp +++ b/src/test/allocator_tests.cpp @@ -2,9 +2,9 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <util/memory.h> #include <util/system.h> -#include <support/allocators/secure.h> #include <test/setup_common.h> #include <memory> diff --git a/src/test/arith_uint256_tests.cpp b/src/test/arith_uint256_tests.cpp index 809c627d27..9ac87261b6 100644 --- a/src/test/arith_uint256_tests.cpp +++ b/src/test/arith_uint256_tests.cpp @@ -11,7 +11,6 @@ #include <uint256.h> #include <arith_uint256.h> #include <string> -#include <version.h> #include <test/setup_common.h> BOOST_FIXTURE_TEST_SUITE(arith_uint256_tests, BasicTestingSetup) diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp index 0c0423c0db..662878750e 100644 --- a/src/test/bip32_tests.cpp +++ b/src/test/bip32_tests.cpp @@ -4,9 +4,10 @@ #include <boost/test/unit_test.hpp> +#include <clientversion.h> #include <key.h> #include <key_io.h> -#include <uint256.h> +#include <streams.h> #include <util/system.h> #include <util/strencodings.h> #include <test/setup_common.h> diff --git a/src/test/blockchain_tests.cpp b/src/test/blockchain_tests.cpp index 13afcca375..ca75563ef0 100644 --- a/src/test/blockchain_tests.cpp +++ b/src/test/blockchain_tests.cpp @@ -2,6 +2,7 @@ #include <stdlib.h> +#include <chain.h> #include <rpc/blockchain.h> #include <test/setup_common.h> diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index f57e1a0ebd..dac201a35f 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -6,7 +6,7 @@ #include <consensus/merkle.h> #include <chainparams.h> #include <pow.h> -#include <random.h> +#include <streams.h> #include <test/setup_common.h> diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 7ba483173c..cf87aa9303 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -10,6 +10,7 @@ #include <pow.h> #include <test/setup_common.h> #include <script/standard.h> +#include <util/time.h> #include <validation.h> #include <boost/test/unit_test.hpp> @@ -267,8 +268,6 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(blockfilter_index_init_destroy, BasicTestingSetup) { - SetDataDir("tempdir"); - BlockFilterIndex* filter_index; filter_index = GetBlockFilterIndex(BlockFilterType::BASIC); diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 408a7fbda4..d796444419 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <util/memory.h> #include <util/system.h> #include <util/time.h> #include <validation.h> @@ -17,8 +18,6 @@ #include <condition_variable> #include <unordered_set> -#include <memory> -#include <random.h> // BasicTestingSetup not sufficient because nScriptCheckThreads is not set // otherwise. diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 2c42596edc..948591196c 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -3,8 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <attributes.h> +#include <clientversion.h> #include <coins.h> #include <script/standard.h> +#include <streams.h> #include <test/setup_common.h> #include <uint256.h> #include <undo.h> diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 0bde92c18d..efcadd51fc 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -4,8 +4,8 @@ #include <dbwrapper.h> #include <uint256.h> -#include <random.h> #include <test/setup_common.h> +#include <util/memory.h> #include <memory> @@ -27,7 +27,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_obfuscate_true" : "dbwrapper_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); char key = 'k'; uint256 in = InsecureRand256(); @@ -47,7 +47,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper_batch").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_batch_obfuscate_true" : "dbwrapper_batch_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); char key = 'i'; @@ -83,7 +83,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper_iterator").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_iterator_obfuscate_true" : "dbwrapper_iterator_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); // The two keys are intentionally chosen for ordering @@ -123,7 +123,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) { // We're going to share this fs::path between two wrappers - fs::path ph = SetDataDir("existing_data_no_obfuscate"); + fs::path ph = GetDataDir() / "existing_data_no_obfuscate"; create_directories(ph); // Set up a non-obfuscated wrapper to write some initial data. @@ -164,7 +164,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) BOOST_AUTO_TEST_CASE(existing_data_reindex) { // We're going to share this fs::path between two wrappers - fs::path ph = SetDataDir("existing_data_reindex"); + fs::path ph = GetDataDir() / "existing_data_reindex"; create_directories(ph); // Set up a non-obfuscated wrapper to write some initial data. @@ -199,7 +199,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex) BOOST_AUTO_TEST_CASE(iterator_ordering) { - fs::path ph = SetDataDir("iterator_ordering"); + fs::path ph = GetDataDir() / "iterator_ordering"; CDBWrapper dbw(ph, (1 << 20), true, false, false); for (int x=0x00; x<256; ++x) { uint8_t key = x; @@ -277,7 +277,7 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) { char buf[10]; - fs::path ph = SetDataDir("iterator_string_ordering"); + fs::path ph = GetDataDir() / "iterator_string_ordering"; CDBWrapper dbw(ph, (1 << 20), true, false, false); for (int x=0x00; x<10; ++x) { for (int y = 0; y < 10; y++) { diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 3a2844861b..93883d1d98 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -11,7 +11,9 @@ #include <net_processing.h> #include <script/sign.h> #include <serialize.h> +#include <util/memory.h> #include <util/system.h> +#include <util/time.h> #include <validation.h> #include <test/setup_common.h> diff --git a/src/test/flatfile_tests.cpp b/src/test/flatfile_tests.cpp index 1db2f8054c..740d805cce 100644 --- a/src/test/flatfile_tests.cpp +++ b/src/test/flatfile_tests.cpp @@ -2,8 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <clientversion.h> #include <flatfile.h> +#include <streams.h> #include <test/setup_common.h> +#include <util/system.h> #include <boost/test/unit_test.hpp> @@ -11,7 +14,7 @@ BOOST_FIXTURE_TEST_SUITE(flatfile_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(flatfile_filename) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFilePos pos(456, 789); @@ -24,7 +27,7 @@ BOOST_AUTO_TEST_CASE(flatfile_filename) BOOST_AUTO_TEST_CASE(flatfile_open) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 16 * 1024); std::string line1("A purely peer-to-peer version of electronic cash would allow online " @@ -85,7 +88,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open) BOOST_AUTO_TEST_CASE(flatfile_allocate) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 100); bool out_of_space; @@ -105,7 +108,7 @@ BOOST_AUTO_TEST_CASE(flatfile_allocate) BOOST_AUTO_TEST_CASE(flatfile_flush) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 100); bool out_of_space; diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index 6bd6bb1be3..6d5a6641f0 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -4,6 +4,7 @@ // #include <fs.h> #include <test/setup_common.h> +#include <util/system.h> #include <boost/test/unit_test.hpp> @@ -11,7 +12,7 @@ BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(fsbridge_fstream) { - fs::path tmpfolder = SetDataDir("fsbridge_fstream"); + fs::path tmpfolder = GetDataDir(); // tmpfile1 should be the same as tmpfile2 fs::path tmpfile1 = tmpfolder / "fs_tests_₿_🏃"; fs::path tmpfile2 = tmpfolder / L"fs_tests_₿_🏃"; diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 97d7633715..9364ac4a32 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <addrdb.h> #include <addrman.h> #include <blockencodings.h> #include <chain.h> @@ -11,8 +12,6 @@ #include <net.h> #include <primitives/block.h> #include <protocol.h> -#include <pubkey.h> -#include <script/script.h> #include <streams.h> #include <undo.h> #include <version.h> @@ -20,8 +19,6 @@ #include <stdint.h> #include <unistd.h> -#include <algorithm> -#include <memory> #include <vector> #include <test/fuzz/fuzz.h> diff --git a/src/test/fuzz/fuzz.h b/src/test/fuzz/fuzz.h index 8b03a7e46e..4e009d9b54 100644 --- a/src/test/fuzz/fuzz.h +++ b/src/test/fuzz/fuzz.h @@ -5,7 +5,6 @@ #ifndef BITCOIN_TEST_FUZZ_FUZZ_H #define BITCOIN_TEST_FUZZ_FUZZ_H -#include <functional> #include <stdint.h> #include <vector> diff --git a/src/test/fuzz/script_flags.cpp b/src/test/fuzz/script_flags.cpp index 2c0bfa360c..9b90d66755 100644 --- a/src/test/fuzz/script_flags.cpp +++ b/src/test/fuzz/script_flags.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <script/interpreter.h> -#include <script/script.h> #include <streams.h> #include <version.h> diff --git a/src/test/hash_tests.cpp b/src/test/hash_tests.cpp index 325b7002f2..d91fcb0034 100644 --- a/src/test/hash_tests.cpp +++ b/src/test/hash_tests.cpp @@ -2,13 +2,12 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <clientversion.h> #include <crypto/siphash.h> #include <hash.h> #include <util/strencodings.h> #include <test/setup_common.h> -#include <vector> - #include <boost/test/unit_test.hpp> BOOST_FIXTURE_TEST_SUITE(hash_tests, BasicTestingSetup) diff --git a/src/test/key_properties.cpp b/src/test/key_properties.cpp index 8b508ed7f7..abcfc4547b 100644 --- a/src/test/key_properties.cpp +++ b/src/test/key_properties.cpp @@ -3,13 +3,9 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <key.h> -#include <base58.h> -#include <script/script.h> #include <uint256.h> #include <util/system.h> -#include <util/strencodings.h> #include <test/setup_common.h> -#include <string> #include <vector> #include <boost/test/unit_test.hpp> diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index 1b95105eab..3e99dcaa40 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -5,7 +5,6 @@ #include <key.h> #include <key_io.h> -#include <script/script.h> #include <uint256.h> #include <util/system.h> #include <util/strencodings.h> diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 0f74b379c0..c6a3de2285 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -5,11 +5,11 @@ #include <policy/policy.h> #include <txmempool.h> #include <util/system.h> +#include <util/time.h> #include <test/setup_common.h> #include <boost/test/unit_test.hpp> -#include <list> #include <vector> BOOST_FIXTURE_TEST_SUITE(mempool_tests, TestingSetup) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 9a182d7bd3..4bd40687a6 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -9,12 +9,12 @@ #include <consensus/tx_verify.h> #include <miner.h> #include <policy/policy.h> -#include <pubkey.h> #include <script/standard.h> #include <txmempool.h> #include <uint256.h> #include <util/strencodings.h> #include <util/system.h> +#include <util/time.h> #include <validation.h> #include <test/setup_common.h> diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 10a732d64d..11e79937be 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -9,6 +9,7 @@ #include <script/script_error.h> #include <script/interpreter.h> #include <script/sign.h> +#include <tinyformat.h> #include <uint256.h> #include <test/setup_common.h> diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 54d18c0a1c..fed65afdbf 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1,16 +1,19 @@ // Copyright (c) 2012-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <addrdb.h> #include <addrman.h> +#include <clientversion.h> #include <test/setup_common.h> #include <string> #include <boost/test/unit_test.hpp> -#include <hash.h> #include <serialize.h> #include <streams.h> #include <net.h> #include <netbase.h> #include <chainparams.h> +#include <util/memory.h> #include <util/system.h> #include <memory> @@ -89,7 +92,6 @@ BOOST_AUTO_TEST_CASE(cnode_listen_port) BOOST_AUTO_TEST_CASE(caddrdb_read) { - SetDataDir("caddrdb_read"); CAddrManUncorrupted addrmanUncorrupted; addrmanUncorrupted.MakeDeterministic(); @@ -135,7 +137,6 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) { - SetDataDir("caddrdb_read_corrupted"); CAddrManCorrupted addrmanCorrupted; addrmanCorrupted.MakeDeterministic(); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 149094fc00..016a4f471b 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -7,6 +7,7 @@ #include <txmempool.h> #include <uint256.h> #include <util/system.h> +#include <util/time.h> #include <test/setup_common.h> diff --git a/src/test/pow_tests.cpp b/src/test/pow_tests.cpp index 653433bfce..1123d4202c 100644 --- a/src/test/pow_tests.cpp +++ b/src/test/pow_tests.cpp @@ -5,7 +5,6 @@ #include <chain.h> #include <chainparams.h> #include <pow.h> -#include <random.h> #include <util/system.h> #include <test/setup_common.h> diff --git a/src/test/raii_event_tests.cpp b/src/test/raii_event_tests.cpp index 2b01acf7fa..41ca8029e5 100644 --- a/src/test/raii_event_tests.cpp +++ b/src/test/raii_event_tests.cpp @@ -14,8 +14,6 @@ #include <test/setup_common.h> -#include <vector> - #include <boost/test/unit_test.hpp> static std::map<void*, short> tags; diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 63bfe1d346..5ae0812243 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -9,8 +9,8 @@ #include <core_io.h> #include <init.h> #include <interfaces/chain.h> - #include <test/setup_common.h> +#include <util/time.h> #include <boost/algorithm/string.hpp> #include <boost/test/unit_test.hpp> diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 195283f89f..046b220e3f 100644 --- a/src/test/script_standard_tests.cpp +++ b/src/test/script_standard_tests.cpp @@ -5,7 +5,6 @@ #include <key.h> #include <keystore.h> #include <script/script.h> -#include <script/script_error.h> #include <script/standard.h> #include <test/setup_common.h> diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 4798909e2f..ae903df0ad 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -14,12 +14,12 @@ #include <util/strencodings.h> #include <test/setup_common.h> #include <rpc/util.h> +#include <streams.h> #if defined(HAVE_CONSENSUS_LIB) #include <script/bitcoinconsensus.h> #endif -#include <fstream> #include <stdint.h> #include <string> #include <vector> diff --git a/src/test/scriptnum10.h b/src/test/scriptnum10.h index e763b64275..2c89a18331 100644 --- a/src/test/scriptnum10.h +++ b/src/test/scriptnum10.h @@ -6,7 +6,6 @@ #ifndef BITCOIN_TEST_SCRIPTNUM10_H #define BITCOIN_TEST_SCRIPTNUM10_H -#include <algorithm> #include <limits> #include <stdexcept> #include <stdint.h> diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 2fab309aa4..8a8620938e 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -6,6 +6,7 @@ #include <streams.h> #include <hash.h> #include <test/setup_common.h> +#include <util/strencodings.h> #include <stdint.h> diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp index b11d090f67..e3ba9cddb0 100644 --- a/src/test/setup_common.cpp +++ b/src/test/setup_common.cpp @@ -10,16 +10,22 @@ #include <consensus/params.h> #include <consensus/validation.h> #include <crypto/sha256.h> +#include <init.h> #include <miner.h> -#include <net_processing.h> +#include <net.h> #include <noui.h> #include <pow.h> #include <rpc/register.h> #include <rpc/server.h> #include <script/sigcache.h> #include <streams.h> +#include <txdb.h> +#include <util/memory.h> +#include <util/strencodings.h> +#include <util/time.h> #include <util/validation.h> #include <validation.h> +#include <validationinterface.h> const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr; @@ -34,6 +40,13 @@ std::ostream& operator<<(std::ostream& os, const uint256& num) BasicTestingSetup::BasicTestingSetup(const std::string& chainName) : m_path_root(fs::temp_directory_path() / "test_common_" PACKAGE_NAME / strprintf("%lu_%i", (unsigned long)GetTime(), (int)(InsecureRandRange(1 << 30)))) { + fs::create_directories(m_path_root); + gArgs.ForceSetArg("-datadir", m_path_root.string()); + ClearDatadirCache(); + SelectParams(chainName); + gArgs.ForceSetArg("-printtoconsole", "0"); + InitLogging(); + LogInstance().StartLogging(); SHA256AutoDetect(); ECC_Start(); SetupEnvironment(); @@ -41,7 +54,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName) InitSignatureCache(); InitScriptExecutionCache(); fCheckBlockIndex = true; - SelectParams(chainName); static bool noui_connected = false; if (!noui_connected) { noui_connect(); @@ -51,27 +63,18 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName) BasicTestingSetup::~BasicTestingSetup() { + LogInstance().DisconnectTestLogger(); fs::remove_all(m_path_root); ECC_Stop(); } -fs::path BasicTestingSetup::SetDataDir(const std::string& name) -{ - fs::path ret = m_path_root / name; - fs::create_directories(ret); - gArgs.ForceSetArg("-datadir", ret.string()); - return ret; -} - TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - SetDataDir("tempdir"); const CChainParams& chainparams = Params(); // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. RegisterAllCoreRPCCommands(tableRPC); - ClearDatadirCache(); // We have to run a scheduler thread to prevent ActivateBestChain // from blocking due to queue overrun. diff --git a/src/test/setup_common.h b/src/test/setup_common.h index 893eca216d..6c9494898c 100644 --- a/src/test/setup_common.h +++ b/src/test/setup_common.h @@ -11,10 +11,8 @@ #include <pubkey.h> #include <random.h> #include <scheduler.h> -#include <txdb.h> #include <txmempool.h> -#include <memory> #include <type_traits> #include <boost/thread.hpp> @@ -54,22 +52,19 @@ static inline bool InsecureRandBool() { return g_insecure_rand_ctx.randbool(); } static constexpr CAmount CENT{1000000}; /** Basic testing setup. - * This just configures logging and chain parameters. + * This just configures logging, data dir and chain parameters. */ struct BasicTestingSetup { ECCVerifyHandle globalVerifyHandle; explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~BasicTestingSetup(); - - fs::path SetDataDir(const std::string& name); - private: const fs::path m_path_root; }; /** Testing setup that configures a complete environment. - * Included are data directory, coins database, script check threads setup. + * Included are coins database, script check threads setup. */ struct TestingSetup : public BasicTestingSetup { boost::thread_group threadGroup; diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index 5c12ec13d2..a32f2cda92 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <consensus/consensus.h> #include <consensus/tx_verify.h> -#include <consensus/validation.h> #include <pubkey.h> #include <key.h> #include <script/script.h> diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 4e37199c63..b812cef801 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -3,7 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <streams.h> -#include <support/allocators/zeroafterfree.h> #include <test/setup_common.h> #include <boost/test/unit_test.hpp> diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index f5ff18c055..f77b77a972 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -20,6 +20,7 @@ #include <script/sign.h> #include <script/script_error.h> #include <script/standard.h> +#include <streams.h> #include <util/strencodings.h> #include <map> diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 26ae7be202..2356e0ccdc 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -3,8 +3,6 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <validation.h> -#include <txmempool.h> -#include <amount.h> #include <consensus/validation.h> #include <primitives/transaction.h> #include <script/script.h> diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 352ce0295b..45c97fa2aa 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -5,14 +5,10 @@ #include <consensus/validation.h> #include <key.h> #include <validation.h> -#include <miner.h> -#include <pubkey.h> #include <txmempool.h> -#include <random.h> #include <script/standard.h> #include <script/sign.h> #include <test/setup_common.h> -#include <util/time.h> #include <keystore.h> #include <boost/test/unit_test.hpp> diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index c1749fb856..33a118c2bb 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -1,19 +1,17 @@ // Copyright (c) 2011-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. + #include <arith_uint256.h> +#include <streams.h> #include <uint256.h> #include <version.h> #include <test/setup_common.h> #include <boost/test/unit_test.hpp> -#include <stdint.h> #include <sstream> #include <iomanip> -#include <limits> -#include <cmath> #include <string> -#include <stdio.h> BOOST_FIXTURE_TEST_SUITE(uint256_tests, BasicTestingSetup) diff --git a/src/test/util.cpp b/src/test/util.cpp index bc09d00b7a..a2ea648324 100644 --- a/src/test/util.cpp +++ b/src/test/util.cpp @@ -17,8 +17,6 @@ #include <wallet/wallet.h> #endif -#include <boost/thread.hpp> - const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq3xueyj"; #ifdef ENABLE_WALLET diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 8fee66d6c3..9960573b33 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -5,14 +5,15 @@ #include <util/system.h> #include <clientversion.h> -#include <primitives/transaction.h> #include <sync.h> #include <test/util.h> #include <util/strencodings.h> #include <util/moneystr.h> +#include <util/time.h> #include <test/setup_common.h> #include <stdint.h> +#include <thread> #include <vector> #ifndef WIN32 #include <signal.h> @@ -1398,7 +1399,7 @@ static void TestOtherProcess(fs::path dirname, std::string lockname, int fd) BOOST_AUTO_TEST_CASE(test_LockDirectory) { - fs::path dirname = SetDataDir("test_LockDirectory") / fs::unique_path(); + fs::path dirname = GetDataDir() / "lock_dir"; const std::string lockname = ".lock"; #ifndef WIN32 // Revert SIGCHLD to default, otherwise boost.test will catch and fail on @@ -1487,7 +1488,7 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory) BOOST_AUTO_TEST_CASE(test_DirIsWritable) { // Should be able to write to the data dir. - fs::path tmpdirname = SetDataDir("test_DirIsWritable"); + fs::path tmpdirname = GetDataDir(); BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), true); // Should not be able to write to a non-existent dir. diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 5dee034b20..b3368d44b6 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -10,14 +10,20 @@ #include <miner.h> #include <pow.h> #include <random.h> +#include <script/standard.h> #include <test/setup_common.h> +#include <util/time.h> #include <validation.h> #include <validationinterface.h> +#include <thread> + struct RegtestingSetup : public TestingSetup { RegtestingSetup() : TestingSetup(CBaseChainParams::REGTEST) {} }; +static const std::vector<unsigned char> V_OP_TRUE{OP_TRUE}; + BOOST_FIXTURE_TEST_SUITE(validation_block_tests, RegtestingSetup) struct TestSubscriber : public CValidationInterface { @@ -59,8 +65,21 @@ std::shared_ptr<CBlock> Block(const uint256& prev_hash) pblock->hashPrevBlock = prev_hash; pblock->nTime = ++time; + pubKey.clear(); + { + WitnessV0ScriptHash witness_program; + CSHA256().Write(&V_OP_TRUE[0], V_OP_TRUE.size()).Finalize(witness_program.begin()); + pubKey << OP_0 << ToByteVector(witness_program); + } + + // Make the coinbase transaction with two outputs: + // One zero-value one that has a unique pubkey to make sure that blocks at the same height can have a different hash + // Another one that has the coinbase reward in a P2WSH with OP_TRUE as witness program to make it easy to spend CMutableTransaction txCoinbase(*pblock->vtx[0]); - txCoinbase.vout.resize(1); + txCoinbase.vout.resize(2); + txCoinbase.vout[1].scriptPubKey = pubKey; + txCoinbase.vout[1].nValue = txCoinbase.vout[0].nValue; + txCoinbase.vout[0].nValue = 0; txCoinbase.vin[0].scriptWitness.SetNull(); pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); @@ -69,6 +88,9 @@ std::shared_ptr<CBlock> Block(const uint256& prev_hash) std::shared_ptr<CBlock> FinalizeBlock(std::shared_ptr<CBlock> pblock) { + LOCK(cs_main); // For LookupBlockIndex + GenerateCoinbaseCommitment(*pblock, LookupBlockIndex(pblock->hashPrevBlock), Params().GetConsensus()); + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); while (!CheckProofOfWork(pblock->GetHash(), pblock->nBits, Params().GetConsensus())) { @@ -79,13 +101,13 @@ std::shared_ptr<CBlock> FinalizeBlock(std::shared_ptr<CBlock> pblock) } // construct a valid block -const std::shared_ptr<const CBlock> GoodBlock(const uint256& prev_hash) +std::shared_ptr<const CBlock> GoodBlock(const uint256& prev_hash) { return FinalizeBlock(Block(prev_hash)); } // construct an invalid block (but with a valid header) -const std::shared_ptr<const CBlock> BadBlock(const uint256& prev_hash) +std::shared_ptr<const CBlock> BadBlock(const uint256& prev_hash) { auto pblock = Block(prev_hash); @@ -185,4 +207,131 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) BOOST_CHECK_EQUAL(sub.m_expected_tip, ::ChainActive().Tip()->GetBlockHash()); } +/** + * Test that mempool updates happen atomically with reorgs. + * + * This prevents RPC clients, among others, from retrieving immediately-out-of-date mempool data + * during large reorgs. + * + * The test verifies this by creating a chain of `num_txs` blocks, matures their coinbases, and then + * submits txns spending from their coinbase to the mempool. A fork chain is then processed, + * invalidating the txns and evicting them from the mempool. + * + * We verify that the mempool updates atomically by polling it continuously + * from another thread during the reorg and checking that its size only changes + * once. The size changing exactly once indicates that the polling thread's + * view of the mempool is either consistent with the chain state before reorg, + * or consistent with the chain state after the reorg, and not just consistent + * with some intermediate state during the reorg. + */ +BOOST_AUTO_TEST_CASE(mempool_locks_reorg) +{ + bool ignored; + auto ProcessBlock = [&ignored](std::shared_ptr<const CBlock> block) -> bool { + return ProcessNewBlock(Params(), block, /* fForceProcessing */ true, /* fNewBlock */ &ignored); + }; + + // Process all mined blocks + BOOST_REQUIRE(ProcessBlock(std::make_shared<CBlock>(Params().GenesisBlock()))); + auto last_mined = GoodBlock(Params().GenesisBlock().GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + + // Run the test multiple times + for (int test_runs = 3; test_runs > 0; --test_runs) { + BOOST_CHECK_EQUAL(last_mined->GetHash(), ::ChainActive().Tip()->GetBlockHash()); + + // Later on split from here + const uint256 split_hash{last_mined->hashPrevBlock}; + + // Create a bunch of transactions to spend the miner rewards of the + // most recent blocks + std::vector<CTransactionRef> txs; + for (int num_txs = 22; num_txs > 0; --num_txs) { + CMutableTransaction mtx; + mtx.vin.push_back(CTxIn{COutPoint{last_mined->vtx[0]->GetHash(), 1}, CScript{}}); + mtx.vin[0].scriptWitness.stack.push_back(V_OP_TRUE); + mtx.vout.push_back(last_mined->vtx[0]->vout[1]); + mtx.vout[0].nValue -= 1000; + txs.push_back(MakeTransactionRef(mtx)); + + last_mined = GoodBlock(last_mined->GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + } + + // Mature the inputs of the txs + for (int j = COINBASE_MATURITY; j > 0; --j) { + last_mined = GoodBlock(last_mined->GetHash()); + BOOST_REQUIRE(ProcessBlock(last_mined)); + } + + // Mine a reorg (and hold it back) before adding the txs to the mempool + const uint256 tip_init{last_mined->GetHash()}; + + std::vector<std::shared_ptr<const CBlock>> reorg; + last_mined = GoodBlock(split_hash); + reorg.push_back(last_mined); + for (size_t j = COINBASE_MATURITY + txs.size() + 1; j > 0; --j) { + last_mined = GoodBlock(last_mined->GetHash()); + reorg.push_back(last_mined); + } + + // Add the txs to the tx pool + { + LOCK(cs_main); + CValidationState state; + std::list<CTransactionRef> plTxnReplaced; + for (const auto& tx : txs) { + BOOST_REQUIRE(AcceptToMemoryPool( + ::mempool, + state, + tx, + /* pfMissingInputs */ &ignored, + &plTxnReplaced, + /* bypass_limits */ false, + /* nAbsurdFee */ 0)); + } + } + + // Check that all txs are in the pool + { + LOCK(::mempool.cs); + BOOST_CHECK_EQUAL(::mempool.mapTx.size(), txs.size()); + } + + // Run a thread that simulates an RPC caller that is polling while + // validation is doing a reorg + std::thread rpc_thread{[&]() { + // This thread is checking that the mempool either contains all of + // the transactions invalidated by the reorg, or none of them, and + // not some intermediate amount. + while (true) { + LOCK(::mempool.cs); + if (::mempool.mapTx.size() == 0) { + // We are done with the reorg + break; + } + // Internally, we might be in the middle of the reorg, but + // externally the reorg to the most-proof-of-work chain should + // be atomic. So the caller assumes that the returned mempool + // is consistent. That is, it has all txs that were there + // before the reorg. + assert(::mempool.mapTx.size() == txs.size()); + continue; + } + LOCK(cs_main); + // We are done with the reorg, so the tip must have changed + assert(tip_init != ::ChainActive().Tip()->GetBlockHash()); + }}; + + // Submit the reorg in this thread to invalidate and remove the txs from the tx pool + for (const auto& b : reorg) { + ProcessBlock(b); + } + // Check that the reorg was eventually successful + BOOST_CHECK_EQUAL(last_mined->GetHash(), ::ChainActive().Tip()->GetBlockHash()); + + // We can join the other thread, which returns when the reorg was successful + rpc_thread.join(); + } +} BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.cpp b/src/txmempool.cpp index cac7beb6a1..9257cff718 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -104,7 +104,7 @@ void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendan // for each such descendant, also update the ancestor state to include the parent. void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<uint256> &vHashesToUpdate) { - LOCK(cs); + AssertLockHeld(cs); // For each entry in vHashesToUpdate, store the set of in-mempool, but not // in-vHashesToUpdate transactions, so that we don't have to recalculate // descendants when we come across a previously seen entry. @@ -322,8 +322,8 @@ void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee, assert(int(nSigOpCostWithAncestors) >= 0); } -CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) : - nTransactionsUpdated(0), minerPolicyEstimator(estimator) +CTxMemPool::CTxMemPool(CBlockPolicyEstimator* estimator) + : nTransactionsUpdated(0), minerPolicyEstimator(estimator) { _clear(); //lock free clear @@ -341,13 +341,11 @@ bool CTxMemPool::isSpent(const COutPoint& outpoint) const unsigned int CTxMemPool::GetTransactionsUpdated() const { - LOCK(cs); return nTransactionsUpdated; } void CTxMemPool::AddTransactionsUpdated(unsigned int n) { - LOCK(cs); nTransactionsUpdated += n; } @@ -459,8 +457,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries& setDescendants void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReason reason) { // Remove transaction from memory pool - { - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; txiter origit = mapTx.find(origTx.GetHash()); if (origit != mapTx.end()) { @@ -485,13 +482,12 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso } RemoveStaged(setAllRemoves, false, reason); - } } void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) { // Remove transactions spending a coinbase which are now immature and no-longer-final transactions - LOCK(cs); + AssertLockHeld(cs); setEntries txToRemove; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { const CTransaction& tx = it->GetTx(); @@ -547,7 +543,7 @@ void CTxMemPool::removeConflicts(const CTransaction &tx) */ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) { - LOCK(cs); + AssertLockHeld(cs); std::vector<const CTxMemPoolEntry*> entries; for (const auto& tx : vtx) { @@ -922,7 +918,7 @@ void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPool } int CTxMemPool::Expire(int64_t time) { - LOCK(cs); + AssertLockHeld(cs); indexed_transaction_set::index<entry_time>::type::iterator it = mapTx.get<entry_time>().begin(); setEntries toremove; while (it != mapTx.get<entry_time>().end() && it->GetTime() < time) { @@ -1015,7 +1011,7 @@ void CTxMemPool::trackPackageRemoved(const CFeeRate& rate) { } void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining) { - LOCK(cs); + AssertLockHeld(cs); unsigned nTxnRemoved = 0; CFeeRate maxFeeRateRemoved(0); diff --git a/src/txmempool.h b/src/txmempool.h index ce0b762336..565dd61f0f 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -6,12 +6,13 @@ #ifndef BITCOIN_TXMEMPOOL_H #define BITCOIN_TXMEMPOOL_H +#include <atomic> +#include <map> #include <memory> #include <set> -#include <map> -#include <vector> -#include <utility> #include <string> +#include <utility> +#include <vector> #include <amount.h> #include <coins.h> @@ -443,7 +444,7 @@ class CTxMemPool { private: uint32_t nCheckFrequency GUARDED_BY(cs); //!< Value n means that n times in 2^32 we check. - unsigned int nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation + std::atomic<unsigned int> nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation CBlockPolicyEstimator* minerPolicyEstimator; uint64_t totalTxSize; //!< sum of all mempool tx's virtual sizes. Differs from serialized tx size since witness data is discounted. Defined in BIP 141. @@ -513,21 +514,12 @@ public: * `mempool.cs` whenever adding transactions to the mempool and whenever * changing the chain tip. It's necessary to keep both mutexes locked until * the mempool is consistent with the new chain tip and fully populated. - * - * @par Consistency bug - * - * The second guarantee above is not currently enforced, but - * https://github.com/bitcoin/bitcoin/pull/14193 will fix it. No known code - * in bitcoin currently depends on second guarantee, but it is important to - * fix for third party code that needs be able to frequently poll the - * mempool without locking `cs_main` and without encountering missing - * transactions during reorgs. */ mutable RecursiveMutex cs; indexed_transaction_set mapTx GUARDED_BY(cs); using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; - std::vector<std::pair<uint256, txiter> > vTxHashes; //!< All tx witness hashes/entries in mapTx, in random order + std::vector<std::pair<uint256, txiter>> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order struct CompareIteratorByHash { bool operator()(const txiter &a, const txiter &b) const { @@ -582,10 +574,10 @@ public: void addUnchecked(const CTxMemPoolEntry& entry, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); - void removeRecursive(const CTransaction &tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); - void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void removeConflicts(const CTransaction &tx) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight); + void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); + void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); void clear(); void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free @@ -598,7 +590,7 @@ public: * Check that none of this transactions inputs are in the mempool, and thus * the tx is not dependent on other mempool transactions to be included in a block. */ - bool HasNoInputsOf(const CTransaction& tx) const; + bool HasNoInputsOf(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Affect CreateNewBlock prioritisation of transactions */ void PrioritiseTransaction(const uint256& hash, const CAmount& nFeeDelta); @@ -632,7 +624,7 @@ public: * for). Note: vHashesToUpdate should be the set of transactions from the * disconnected block that have been accepted back into the mempool. */ - void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void UpdateTransactionsFromBlock(const std::vector<uint256>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); /** Try to calculate all in-mempool ancestors of entry. * (these are all calculated including the tx itself) @@ -663,10 +655,10 @@ public: * pvNoSpendsRemaining, if set, will be populated with the list of outpoints * which are not in mempool which no longer have any spends in this mempool. */ - void TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining=nullptr); + void TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpendsRemaining = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Expire all transaction (and their dependencies) in the mempool older than time. Return the number of removed transactions. */ - int Expire(int64_t time); + int Expire(int64_t time) EXCLUSIVE_LOCKS_REQUIRED(cs); /** * Calculate the ancestor and descendant count for the given transaction. diff --git a/src/ui_interface.h b/src/ui_interface.h index d408f6f889..5e0380dc45 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -69,6 +69,9 @@ public: /** Force blocking, modal message box dialog (not just OS notification) */ MODAL = 0x10000000U, + /** Do not prepend error/warning prefix */ + MSG_NOPREFIX = 0x20000000U, + /** Do not print contents of message to debug log */ SECURE = 0x40000000U, diff --git a/src/util/error.cpp b/src/util/error.cpp index 68ffd8b046..9331a92ad7 100644 --- a/src/util/error.cpp +++ b/src/util/error.cpp @@ -27,6 +27,8 @@ std::string TransactionErrorString(const TransactionError err) return "PSBTs not compatible (different transactions)"; case TransactionError::SIGHASH_MISMATCH: return "Specified sighash value does not match existing value"; + case TransactionError::MAX_FEE_EXCEEDED: + return "Fee exceeds maximum configured by -maxtxfee"; // no default case, so the compiler can warn about missing cases } assert(false); diff --git a/src/util/error.h b/src/util/error.h index d93309551b..0fd474b962 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -27,6 +27,7 @@ enum class TransactionError { INVALID_PSBT, PSBT_MISMATCH, SIGHASH_MISMATCH, + MAX_FEE_EXCEEDED, }; std::string TransactionErrorString(const TransactionError error); diff --git a/src/util/fees.cpp b/src/util/fees.cpp index 5fdaa1284c..cf16d5e44f 100644 --- a/src/util/fees.cpp +++ b/src/util/fees.cpp @@ -18,7 +18,6 @@ std::string StringForFeeReason(FeeReason reason) { {FeeReason::PAYTXFEE, "PayTxFee set"}, {FeeReason::FALLBACK, "Fallback fee"}, {FeeReason::REQUIRED, "Minimum Required Fee"}, - {FeeReason::MAXTXFEE, "MaxTxFee limit"} }; auto reason_string = fee_reason_strings.find(reason); diff --git a/src/util/system.cpp b/src/util/system.cpp index fca29a9f31..87ff6e62ba 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -705,19 +705,16 @@ fs::path GetDefaultDataDir() static fs::path g_blocks_path_cache_net_specific; static fs::path pathCached; static fs::path pathCachedNetSpecific; -static CCriticalSection csPathCached; +static RecursiveMutex csPathCached; const fs::path &GetBlocksDir() { - LOCK(csPathCached); - fs::path &path = g_blocks_path_cache_net_specific; - // This can be called during exceptions by LogPrintf(), so we cache the - // value so we don't have to do memory allocations after that. - if (!path.empty()) - return path; + // Cache the path to avoid calling fs::create_directories on every call of + // this function + if (!path.empty()) return path; if (gArgs.IsArgSet("-blocksdir")) { path = fs::system_complete(gArgs.GetArg("-blocksdir", "")); @@ -737,15 +734,12 @@ const fs::path &GetBlocksDir() const fs::path &GetDataDir(bool fNetSpecific) { - LOCK(csPathCached); - fs::path &path = fNetSpecific ? pathCachedNetSpecific : pathCached; - // This can be called during exceptions by LogPrintf(), so we cache the - // value so we don't have to do memory allocations after that. - if (!path.empty()) - return path; + // Cache the path to avoid calling fs::create_directories on every call of + // this function + if (!path.empty()) return path; if (gArgs.IsArgSet("-datadir")) { path = fs::system_complete(gArgs.GetArg("-datadir", "")); diff --git a/src/util/system.h b/src/util/system.h index 1a83cb67b1..15d7b1b402 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -20,18 +20,16 @@ #include <fs.h> #include <logging.h> #include <sync.h> -#include <util/threadnames.h> #include <tinyformat.h> #include <util/memory.h> +#include <util/threadnames.h> #include <util/time.h> -#include <atomic> #include <exception> #include <map> #include <set> #include <stdint.h> #include <string> -#include <unordered_set> #include <utility> #include <vector> @@ -85,6 +83,7 @@ fs::path GetDefaultDataDir(); // The blocks directory is always net specific. const fs::path &GetBlocksDir(); const fs::path &GetDataDir(bool fNetSpecific = true); +/** Tests only */ void ClearDatadirCache(); fs::path GetConfigFile(const std::string& confPath); #ifdef WIN32 diff --git a/src/validation.cpp b/src/validation.cpp index 2d2252c251..c6995ed24b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -304,7 +304,8 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag // Returns the script flags which should be checked for a given block static unsigned int GetBlockScriptFlags(const CBlockIndex* pindex, const Consensus::Params& chainparams); -static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) { +static void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) +{ int expired = pool.Expire(GetTime() - age); if (expired != 0) { LogPrint(BCLog::MEMPOOL, "Expired %i transactions from the memory pool\n", expired); @@ -341,7 +342,7 @@ static bool IsCurrentForFeeEstimation() EXCLUSIVE_LOCKS_REQUIRED(cs_main) * and instead just erase from the mempool as needed. */ -static void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool, bool fAddToMempool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs) { AssertLockHeld(cs_main); std::vector<uint256> vHashUpdate; @@ -1374,20 +1375,22 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex) } /** Abort with a message */ -static bool AbortNode(const std::string& strMessage, const std::string& userMessage="") +static bool AbortNode(const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0) { SetMiscWarning(strMessage); LogPrintf("*** %s\n", strMessage); - uiInterface.ThreadSafeMessageBox( - userMessage.empty() ? _("Error: A fatal internal error occurred, see debug.log for details") : userMessage, - "", CClientUIInterface::MSG_ERROR); + if (!userMessage.empty()) { + uiInterface.ThreadSafeMessageBox(userMessage, "", CClientUIInterface::MSG_ERROR | prefix); + } else { + uiInterface.ThreadSafeMessageBox(_("Error: A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR | CClientUIInterface::MSG_NOPREFIX); + } StartShutdown(); return false; } -static bool AbortNode(CValidationState& state, const std::string& strMessage, const std::string& userMessage="") +static bool AbortNode(CValidationState& state, const std::string& strMessage, const std::string& userMessage = "", unsigned int prefix = 0) { - AbortNode(strMessage, userMessage); + AbortNode(strMessage, userMessage, prefix); return state.Error(strMessage); } @@ -1998,7 +2001,7 @@ bool CChainState::FlushStateToDisk( if (fDoFullFlush || fPeriodicWrite) { // Depend on nMinDiskSpace to ensure we can write block index if (!CheckDiskSpace(GetBlocksDir())) { - return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!")); + return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX); } // First make sure all block and undo data is flushed to disk. FlushBlockFile(); @@ -2033,7 +2036,7 @@ bool CChainState::FlushStateToDisk( // an overestimation, as most will delete an existing entry or // overwrite one. Still, use a conservative safety factor of 2. if (!CheckDiskSpace(GetDataDir(), 48 * 2 * 2 * pcoinsTip->GetCacheSize())) { - return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!")); + return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX); } // Flush the chainstate (which may refer to block index entries). if (!pcoinsTip->Flush()) @@ -2545,7 +2548,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& LimitValidationInterfaceQueue(); { - LOCK(cs_main); + LOCK2(cs_main, ::mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed CBlockIndex* starting_tip = m_chain.Tip(); bool blocks_connected = false; do { @@ -2665,6 +2668,7 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c LimitValidationInterfaceQueue(); LOCK(cs_main); + LOCK(::mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between if (!m_chain.Contains(pindex)) break; pindex_was_in_chain = true; CBlockIndex *invalid_walk_tip = m_chain.Tip(); @@ -2899,7 +2903,7 @@ static bool FindBlockPos(FlatFilePos &pos, unsigned int nAddSize, unsigned int n bool out_of_space; size_t bytes_allocated = BlockFileSeq().Allocate(pos, nAddSize, out_of_space); if (out_of_space) { - return AbortNode("Disk space is low!", _("Error: Disk space is low!")); + return AbortNode("Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX); } if (bytes_allocated != 0 && fPruneMode) { fCheckForPruning = true; @@ -2923,7 +2927,7 @@ static bool FindUndoPos(CValidationState &state, int nFile, FlatFilePos &pos, un bool out_of_space; size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space); if (out_of_space) { - return AbortNode(state, "Disk space is low!", _("Error: Disk space is low!")); + return AbortNode(state, "Disk space is too low!", _("Error: Disk space is too low!"), CClientUIInterface::MSG_NOPREFIX); } if (bytes_allocated != 0 && fPruneMode) { fCheckForPruning = true; @@ -4098,7 +4102,7 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // Loop until the tip is below nHeight, or we reach a pruned block. while (!ShutdownRequested()) { { - LOCK(cs_main); + LOCK2(cs_main, ::mempool.cs); // Make sure nothing changed from under us (this won't happen because RewindBlockIndex runs before importing/network are active) assert(tip == m_chain.Tip()); if (tip == nullptr || tip->nHeight < nHeight) break; diff --git a/src/validation.h b/src/validation.h index 638229952d..9573d62048 100644 --- a/src/validation.h +++ b/src/validation.h @@ -18,6 +18,7 @@ #include <protocol.h> // For CMessageHeader::MessageStartChars #include <script/script_error.h> #include <sync.h> +#include <txmempool.h> // For CTxMemPool::cs #include <versionbits.h> #include <algorithm> @@ -553,7 +554,7 @@ public: CCoinsViewCache& view, const CChainParams& chainparams, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Block disconnection on our pcoinsTip: - bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); // Manual block validity manipulation: bool PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex* pindex) LOCKS_EXCLUDED(cs_main); @@ -572,8 +573,8 @@ public: bool IsInitialBlockDownload() const; private: - bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions &disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Create a new block index entry for a given block hash */ diff --git a/src/validationinterface.h b/src/validationinterface.h index ea1b2e7e76..3ce617b827 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -16,7 +16,6 @@ extern CCriticalSection cs_main; class CBlock; class CBlockIndex; struct CBlockLocator; -class CBlockIndex; class CConnman; class CValidationInterface; class CValidationState; diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index ad69e84358..2792058f2a 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -18,14 +18,7 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes) CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, FeeCalculation* feeCalc) { - CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes); - // Always obey the maximum - const CAmount max_tx_fee = wallet.m_default_max_tx_fee; - if (fee_needed > max_tx_fee) { - fee_needed = max_tx_fee; - if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; - } - return fee_needed; + return GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes); } CFeeRate GetRequiredFeeRate(const CWallet& wallet) diff --git a/src/wallet/psbtwallet.cpp b/src/wallet/psbtwallet.cpp index ce4788dee1..721a244afb 100644 --- a/src/wallet/psbtwallet.cpp +++ b/src/wallet/psbtwallet.cpp @@ -44,16 +44,7 @@ TransactionError FillPSBT(const CWallet* pwallet, PartiallySignedTransaction& ps // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { - const CTxOut& out = psbtx.tx->vout.at(i); - PSBTOutput& psbt_out = psbtx.outputs.at(i); - - // Fill a SignatureData with output info - SignatureData sigdata; - psbt_out.FillSignatureData(sigdata); - - MutableTransactionSignatureCreator creator(psbtx.tx.get_ptr(), 0, out.nValue, 1); - ProduceSignature(HidingSigningProvider(pwallet, true, !bip32derivs), creator, out.scriptPubKey, sigdata); - psbt_out.FromSignatureData(sigdata); + UpdatePSBTOutput(HidingSigningProvider(pwallet, true, !bip32derivs), psbtx, i); } return TransactionError::OK; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 3232c65bca..eae5f876ea 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -383,7 +383,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) " \"UNSET\"\n" " \"ECONOMICAL\"\n" " \"CONSERVATIVE\""}, - {"avoid_reuse", RPCArg::Type::BOOL, /* default */ pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) ? "true" : "unavailable", "Avoid spending from dirty addresses; addresses are considered\n" + {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Avoid spending from dirty addresses; addresses are considered\n" " dirty if they have previously been used in a transaction."}, }, RPCResult{ @@ -743,7 +743,7 @@ static UniValue getbalance(const JSONRPCRequest& request) {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."}, {"minconf", RPCArg::Type::NUM, /* default */ "0", "Only include transactions confirmed at least this many times."}, {"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Also include balance in watch-only addresses (see 'importaddress')"}, - {"avoid_reuse", RPCArg::Type::BOOL, /* default */ pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) ? "true" : "unavailable", "Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."}, + {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."}, }, RPCResult{ "amount (numeric) The total amount in " + CURRENCY_UNIT + " received for this wallet.\n" @@ -2409,6 +2409,7 @@ static UniValue getbalances(const JSONRPCRequest& request) " \"trusted\": xxx (numeric) trusted balance (outputs created by the wallet or confirmed outputs)\n" " \"untrusted_pending\": xxx (numeric) untrusted pending balance (outputs created by others that are in the mempool)\n" " \"immature\": xxx (numeric) balance from immature coinbase outputs\n" + " \"used\": xxx (numeric) (only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)\n" " },\n" " \"watchonly\": { (object) watchonly balances (not present if wallet does not watch anything)\n" " \"trusted\": xxx (numeric) trusted balance (outputs created by the wallet or confirmed outputs)\n" @@ -2441,6 +2442,12 @@ static UniValue getbalances(const JSONRPCRequest& request) balances_mine.pushKV("trusted", ValueFromAmount(bal.m_mine_trusted)); balances_mine.pushKV("untrusted_pending", ValueFromAmount(bal.m_mine_untrusted_pending)); balances_mine.pushKV("immature", ValueFromAmount(bal.m_mine_immature)); + if (wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { + // If the AVOID_REUSE flag is set, bal has been set to just the un-reused address balance. Get + // the total balance, and then subtract bal to get the reused address balance. + const auto full_bal = wallet.GetBalance(0, false); + balances_mine.pushKV("used", ValueFromAmount(full_bal.m_mine_trusted + full_bal.m_mine_untrusted_pending - bal.m_mine_trusted - bal.m_mine_untrusted_pending)); + } balances.pushKV("mine", balances_mine); } if (wallet.HaveWatchOnly()) { @@ -2885,11 +2892,8 @@ static UniValue listunspent(const JSONRPCRequest& request) return NullUniValue; } - bool avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); - - if (request.fHelp || request.params.size() > 5) - throw std::runtime_error( - RPCHelpMan{"listunspent", + const RPCHelpMan help{ + "listunspent", "\nReturns array of unspent transaction outputs\n" "with between minconf and maxconf (inclusive) confirmations.\n" "Optionally filter to only include txouts paid to specified addresses.\n", @@ -2926,9 +2930,7 @@ static UniValue listunspent(const JSONRPCRequest& request) " \"witnessScript\" : \"script\" (string) witnessScript if the scriptPubKey is P2WSH or P2SH-P2WSH\n" " \"spendable\" : xxx, (bool) Whether we have the private keys to spend this output\n" " \"solvable\" : xxx, (bool) Whether we know how to spend this output, ignoring the lack of keys\n" - + (avoid_reuse ? - " \"reused\" : xxx, (bool) Whether this output is reused/dirty (sent to an address that was previously spent from)\n" : - "") + + " \"reused\" : xxx, (bool) (only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)\n" " \"desc\" : xxx, (string, only when solvable) A descriptor for spending this output\n" " \"safe\" : xxx (bool) Whether this output is considered safe to spend. Unconfirmed transactions\n" " from outside keys and unconfirmed replacement transactions are considered unsafe\n" @@ -2944,7 +2946,11 @@ static UniValue listunspent(const JSONRPCRequest& request) + HelpExampleCli("listunspent", "6 9999999 '[]' true '{ \"minimumAmount\": 0.005 }'") + HelpExampleRpc("listunspent", "6, 9999999, [] , true, { \"minimumAmount\": 0.005 } ") }, - }.ToString()); + }; + + if (request.fHelp || !help.IsValidNumArgs(request.params.size())) { + throw std::runtime_error(help.ToString()); + } int nMinDepth = 1; if (!request.params[0].isNull()) { @@ -3017,6 +3023,8 @@ static UniValue listunspent(const JSONRPCRequest& request) LOCK(pwallet->cs_wallet); + const bool avoid_reuse = pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); + for (const COutput& out : vecOutputs) { CTxDestination address; const CScript& scriptPubKey = out.tx->tx->vout[out.i].scriptPubKey; diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index e4950af4e5..c961456572 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -16,7 +16,7 @@ BOOST_FIXTURE_TEST_SUITE(db_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(getwalletenv_file) { std::string test_name = "test_name.dat"; - fs::path datadir = SetDataDir("tempdir"); + const fs::path datadir = GetDataDir(); fs::path file_path = datadir / test_name; std::ofstream f(file_path.BOOST_FILESYSTEM_C_STR); f.close(); @@ -30,7 +30,7 @@ BOOST_AUTO_TEST_CASE(getwalletenv_file) BOOST_AUTO_TEST_CASE(getwalletenv_directory) { std::string expected_name = "wallet.dat"; - fs::path datadir = SetDataDir("tempdir"); + const fs::path datadir = GetDataDir(); std::string filename; std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(datadir, filename); @@ -40,8 +40,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_directory) BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple) { - fs::path datadir = SetDataDir("tempdir"); - fs::path datadir_2 = SetDataDir("tempdir_2"); + fs::path datadir = GetDataDir() / "1"; + fs::path datadir_2 = GetDataDir() / "2"; std::string filename; std::shared_ptr<BerkeleyEnvironment> env_1 = GetWalletEnv(datadir, filename); @@ -54,8 +54,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple) BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_free_instance) { - fs::path datadir = SetDataDir("tempdir"); - fs::path datadir_2 = SetDataDir("tempdir_2"); + fs::path datadir = GetDataDir() / "1"; + fs::path datadir_2 = GetDataDir() / "2"; std::string filename; std::shared_ptr <BerkeleyEnvironment> env_1_a = GetWalletEnv(datadir, filename); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 3b828d57f9..86ba0013fe 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <fs.h> +#include <util/system.h> #include <wallet/test/init_test_fixture.h> @@ -13,7 +14,7 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam std::string sep; sep += fs::path::preferred_separator; - m_datadir = SetDataDir("tempdir"); + m_datadir = GetDataDir(); m_cwd = fs::current_path(); m_walletdir_path_cases["default"] = m_datadir / "wallets"; @@ -41,4 +42,4 @@ InitWalletDirTestingSetup::~InitWalletDirTestingSetup() void InitWalletDirTestingSetup::SetWalletDir(const fs::path& walletdir_path) { gArgs.ForceSetArg("-walletdir", walletdir_path.string()); -}
\ No newline at end of file +} diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp index 67e2847963..1816fca257 100644 --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -5,6 +5,7 @@ #include <boost/test/unit_test.hpp> #include <test/setup_common.h> +#include <util/system.h> #include <wallet/test/init_test_fixture.h> BOOST_FIXTURE_TEST_SUITE(init_tests, InitWalletDirTestingSetup) diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index 0cae055676..062fef7748 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -4,7 +4,6 @@ #include <key.h> #include <script/script.h> -#include <script/script_error.h> #include <script/standard.h> #include <test/setup_common.h> #include <wallet/ismine.h> diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index cdf7113203..0400f1207c 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -3,12 +3,10 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <key_io.h> -#include <script/sign.h> #include <util/bip32.h> #include <util/strencodings.h> #include <wallet/psbtwallet.h> #include <wallet/wallet.h> -#include <univalue.h> #include <boost/test/unit_test.hpp> #include <test/setup_common.h> diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 7db0bc4249..ba0843f352 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -4,8 +4,6 @@ #include <wallet/test/wallet_test_fixture.h> -#include <wallet/db.h> - WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), m_wallet(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 922bb0fe65..ef95d0544f 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -5,9 +5,7 @@ #include <wallet/wallet.h> #include <memory> -#include <set> #include <stdint.h> -#include <utility> #include <vector> #include <consensus/validation.h> @@ -192,7 +190,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) auto locked_chain = chain->lock(); LockAssertion lock(::cs_main); - std::string backup_file = (SetDataDir("importwallet_rescan") / "wallet.backup").string(); + std::string backup_file = (GetDataDir() / "wallet.backup").string(); // Import key into wallet and call dumpwallet to create backup file. { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0472334bc1..8807acb6b7 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2266,7 +2266,7 @@ void MaybeResendWalletTxs() CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) const { Balance ret; - isminefilter reuse_filter = avoid_reuse ? 0 : ISMINE_USED; + isminefilter reuse_filter = avoid_reuse ? ISMINE_NO : ISMINE_USED; { auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -2694,6 +2694,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC } } + if (nFeeRet > this->m_default_max_tx_fee) { + strFailReason = TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED); + return false; + } + return true; } |