diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/Makefile.test.include | 25 | ||||
-rw-r--r-- | src/net_processing.cpp | 64 | ||||
-rw-r--r-- | src/qt/coincontroldialog.cpp | 57 | ||||
-rw-r--r-- | src/qt/coincontroldialog.h | 9 | ||||
-rw-r--r-- | src/qt/sendcoinsdialog.cpp | 50 | ||||
-rw-r--r-- | src/qt/sendcoinsdialog.h | 2 | ||||
-rw-r--r-- | src/rpc/mining.cpp | 12 | ||||
-rw-r--r-- | src/rpc/server.cpp | 6 | ||||
-rw-r--r-- | src/test/fuzz/process_message.cpp | 24 | ||||
-rw-r--r-- | src/test/validation_block_tests.cpp | 10 | ||||
-rw-r--r-- | src/test/validationinterface_tests.cpp | 34 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 8 | ||||
-rw-r--r-- | src/wallet/wallet.h | 4 |
13 files changed, 155 insertions, 150 deletions
diff --git a/src/Makefile.test.include b/src/Makefile.test.include index d53477793c..56c887af78 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -104,8 +104,6 @@ FUZZ_TARGETS = \ test/fuzz/script \ test/fuzz/script_deserialize \ test/fuzz/script_flags \ - test/fuzz/script_ops \ - test/fuzz/scriptnum_ops \ test/fuzz/service_deserialize \ test/fuzz/signature_checker \ test/fuzz/snapshotmetadata_deserialize \ @@ -893,17 +891,18 @@ test_fuzz_script_flags_LDADD = $(FUZZ_SUITE_LD_COMMON) test_fuzz_script_flags_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) test_fuzz_script_flags_SOURCES = test/fuzz/script_flags.cpp -test_fuzz_script_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -test_fuzz_script_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -test_fuzz_script_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) -test_fuzz_script_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) -test_fuzz_script_ops_SOURCES = test/fuzz/script_ops.cpp - -test_fuzz_scriptnum_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -test_fuzz_scriptnum_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) -test_fuzz_scriptnum_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) -test_fuzz_scriptnum_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) -test_fuzz_scriptnum_ops_SOURCES = test/fuzz/scriptnum_ops.cpp +# Disabled for now, as #18413 has not been backported. +# test_fuzz_script_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +# test_fuzz_script_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +# test_fuzz_script_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) +# test_fuzz_script_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +# test_fuzz_script_ops_SOURCES = test/fuzz/script_ops.cpp + +# test_fuzz_scriptnum_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +# test_fuzz_scriptnum_ops_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) +# test_fuzz_scriptnum_ops_LDADD = $(FUZZ_SUITE_LD_COMMON) +# test_fuzz_scriptnum_ops_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +# test_fuzz_scriptnum_ops_SOURCES = test/fuzz/scriptnum_ops.cpp test_fuzz_service_deserialize_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -DSERVICE_DESERIALIZE=1 test_fuzz_service_deserialize_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d3089c4176..2a61f84a08 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1564,26 +1564,32 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm std::vector<CInv> vNotFound; const CNetMsgMaker msgMaker(pfrom->GetSendVersion()); - // Note that if we receive a getdata for a MSG_TX or MSG_WITNESS_TX from a - // block-relay-only outbound peer, we will stop processing further getdata - // messages from this peer (likely resulting in our peer eventually - // disconnecting us). - if (pfrom->m_tx_relay != nullptr) { - // mempool entries added before this time have likely expired from mapRelay - const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME; - const std::chrono::seconds mempool_req = pfrom->m_tx_relay->m_last_mempool_req.load(); + // mempool entries added before this time have likely expired from mapRelay + const std::chrono::seconds longlived_mempool_time = GetTime<std::chrono::seconds>() - RELAY_TX_CACHE_TIME; + // Get last mempool request time + const std::chrono::seconds mempool_req = pfrom->m_tx_relay != nullptr ? pfrom->m_tx_relay->m_last_mempool_req.load() + : std::chrono::seconds::min(); + { LOCK(cs_main); + // Process as many TX items from the front of the getdata queue as + // possible, since they're common and it's efficient to batch process + // them. while (it != pfrom->vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX)) { if (interruptMsgProc) return; - // Don't bother if send buffer is too full to respond anyway + // The send buffer provides backpressure. If there's no space in + // the buffer, pause processing until the next call. if (pfrom->fPauseSend) break; - const CInv &inv = *it; - it++; + const CInv &inv = *it++; + + if (pfrom->m_tx_relay == nullptr) { + // Ignore GETDATA requests for transactions from blocks-only peers. + continue; + } // Send stream from relay memory bool push = false; @@ -1611,19 +1617,17 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm } } // release cs_main + // Only process one BLOCK item per call, since they're uncommon and can be + // expensive to process. if (it != pfrom->vRecvGetData.end() && !pfrom->fPauseSend) { - const CInv &inv = *it; + const CInv &inv = *it++; if (inv.type == MSG_BLOCK || inv.type == MSG_FILTERED_BLOCK || inv.type == MSG_CMPCT_BLOCK || inv.type == MSG_WITNESS_BLOCK) { - it++; ProcessGetBlockData(pfrom, chainparams, inv, connman); } + // else: If the first item on the queue is an unknown type, we erase it + // and continue processing the queue on the next call. } - // 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()) { @@ -2257,6 +2261,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec uint32_t nFetchFlags = GetFetchFlags(pfrom); const auto current_time = GetTime<std::chrono::microseconds>(); + uint256* best_block{nullptr}; for (CInv &inv : vInv) { @@ -2273,17 +2278,14 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec if (inv.type == MSG_BLOCK) { UpdateBlockAvailability(pfrom->GetId(), inv.hash); if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) { - // We used to request the full block here, but since headers-announcements are now the - // primary method of announcement on the network, and since, in the case that a node - // fell back to inv we probably have a reorg which we should get the headers for first, - // we now only provide a getheaders response here. When we receive the headers, we will - // then ask for the blocks we need. - connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), inv.hash)); - LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, inv.hash.ToString(), pfrom->GetId()); + // Headers-first is the primary method of announcement on + // the network. If a node fell back to sending blocks by inv, + // it's probably for a re-org. The final block hash + // provided should be the highest, so send a getheaders and + // then fetch the blocks we need to catch up. + best_block = &inv.hash; } - } - else - { + } else { pfrom->AddInventoryKnown(inv); if (fBlocksOnly) { LogPrint(BCLog::NET, "transaction (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.hash.ToString(), pfrom->GetId()); @@ -2294,6 +2296,12 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec } } } + + if (best_block != nullptr) { + connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::GETHEADERS, ::ChainActive().GetLocator(pindexBestHeader), *best_block)); + LogPrint(BCLog::NET, "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, best_block->ToString(), pfrom->GetId()); + } + return true; } diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 9495ba389a..88e7304383 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -41,10 +41,11 @@ bool CCoinControlWidgetItem::operator<(const QTreeWidgetItem &other) const { return QTreeWidgetItem::operator<(other); } -CoinControlDialog::CoinControlDialog(const PlatformStyle *_platformStyle, QWidget *parent) : +CoinControlDialog::CoinControlDialog(CCoinControl& coin_control, WalletModel* _model, const PlatformStyle *_platformStyle, QWidget *parent) : QDialog(parent), ui(new Ui::CoinControlDialog), - model(nullptr), + m_coin_control(coin_control), + model(_model), platformStyle(_platformStyle) { ui->setupUi(this); @@ -134,6 +135,13 @@ CoinControlDialog::CoinControlDialog(const PlatformStyle *_platformStyle, QWidge ui->radioTreeMode->click(); if (settings.contains("nCoinControlSortColumn") && settings.contains("nCoinControlSortOrder")) sortView(settings.value("nCoinControlSortColumn").toInt(), (static_cast<Qt::SortOrder>(settings.value("nCoinControlSortOrder").toInt()))); + + if(_model->getOptionsModel() && _model->getAddressTableModel()) + { + updateView(); + updateLabelLocked(); + CoinControlDialog::updateLabels(m_coin_control, _model, this); + } } CoinControlDialog::~CoinControlDialog() @@ -146,18 +154,6 @@ CoinControlDialog::~CoinControlDialog() delete ui; } -void CoinControlDialog::setModel(WalletModel *_model) -{ - this->model = _model; - - if(_model && _model->getOptionsModel() && _model->getAddressTableModel()) - { - updateView(); - updateLabelLocked(); - CoinControlDialog::updateLabels(_model, this); - } -} - // ok button void CoinControlDialog::buttonBoxClicked(QAbstractButton* button) { @@ -183,8 +179,8 @@ void CoinControlDialog::buttonSelectAllClicked() ui->treeWidget->topLevelItem(i)->setCheckState(COLUMN_CHECKBOX, state); ui->treeWidget->setEnabled(true); if (state == Qt::Unchecked) - coinControl()->UnSelectAll(); // just to be sure - CoinControlDialog::updateLabels(model, this); + m_coin_control.UnSelectAll(); // just to be sure + CoinControlDialog::updateLabels(m_coin_control, model, this); } // context menu @@ -369,15 +365,15 @@ void CoinControlDialog::viewItemChanged(QTreeWidgetItem* item, int column) COutPoint outpt(uint256S(item->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), item->data(COLUMN_ADDRESS, VOutRole).toUInt()); if (item->checkState(COLUMN_CHECKBOX) == Qt::Unchecked) - coinControl()->UnSelect(outpt); + m_coin_control.UnSelect(outpt); else if (item->isDisabled()) // locked (this happens if "check all" through parent node) item->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked); else - coinControl()->Select(outpt); + m_coin_control.Select(outpt); // selection changed -> update labels if (ui->treeWidget->isEnabled()) // do not update on every click for (un)select all - CoinControlDialog::updateLabels(model, this); + CoinControlDialog::updateLabels(m_coin_control, model, this); } // TODO: Remove this temporary qt5 fix after Qt5.3 and Qt5.4 are no longer used. @@ -402,7 +398,7 @@ void CoinControlDialog::updateLabelLocked() else ui->labelLocked->setVisible(false); } -void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) +void CoinControlDialog::updateLabels(CCoinControl& m_coin_control, WalletModel *model, QDialog* dialog) { if (!model) return; @@ -434,7 +430,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) bool fWitness = false; std::vector<COutPoint> vCoinControl; - coinControl()->ListSelected(vCoinControl); + m_coin_control.ListSelected(vCoinControl); size_t i = 0; for (const auto& out : model->wallet().getCoins(vCoinControl)) { @@ -445,7 +441,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) const COutPoint& outpt = vCoinControl[i++]; if (out.is_spent) { - coinControl()->UnSelect(outpt); + m_coin_control.UnSelect(outpt); continue; } @@ -498,7 +494,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) nBytes -= 34; // Fee - nPayFee = model->wallet().getMinimumFee(nBytes, *coinControl(), nullptr /* returned_target */, nullptr /* reason */); + nPayFee = model->wallet().getMinimumFee(nBytes, m_coin_control, nullptr /* returned_target */, nullptr /* reason */); if (nPayAmount > 0) { @@ -590,12 +586,6 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) label->setVisible(nChange < 0); } -CCoinControl* CoinControlDialog::coinControl() -{ - static CCoinControl coin_control; - return &coin_control; -} - void CoinControlDialog::updateView() { if (!model || !model->getOptionsModel() || !model->getAddressTableModel()) @@ -612,8 +602,7 @@ void CoinControlDialog::updateView() int nDisplayUnit = model->getOptionsModel()->getDisplayUnit(); for (const auto& coins : model->wallet().listCoins()) { - CCoinControlWidgetItem *itemWalletAddress = new CCoinControlWidgetItem(); - itemWalletAddress->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked); + CCoinControlWidgetItem* itemWalletAddress{nullptr}; QString sWalletAddress = QString::fromStdString(EncodeDestination(coins.first)); QString sWalletLabel = model->getAddressTableModel()->labelForAddress(sWalletAddress); if (sWalletLabel.isEmpty()) @@ -622,7 +611,7 @@ void CoinControlDialog::updateView() if (treeMode) { // wallet address - ui->treeWidget->addTopLevelItem(itemWalletAddress); + itemWalletAddress = new CCoinControlWidgetItem(ui->treeWidget); itemWalletAddress->setFlags(flgTristate); itemWalletAddress->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked); @@ -696,13 +685,13 @@ void CoinControlDialog::updateView() // disable locked coins if (model->wallet().isLockedCoin(output)) { - coinControl()->UnSelect(output); // just to be sure + m_coin_control.UnSelect(output); // just to be sure itemOutput->setDisabled(true); itemOutput->setIcon(COLUMN_CHECKBOX, platformStyle->SingleColorIcon(":/icons/lock_closed")); } // set checkbox - if (coinControl()->IsSelected(output)) + if (m_coin_control.IsSelected(output)) itemOutput->setCheckState(COLUMN_CHECKBOX, Qt::Checked); } diff --git a/src/qt/coincontroldialog.h b/src/qt/coincontroldialog.h index efc06a7656..c8a91e5f58 100644 --- a/src/qt/coincontroldialog.h +++ b/src/qt/coincontroldialog.h @@ -31,7 +31,6 @@ class CCoinControlWidgetItem : public QTreeWidgetItem { public: explicit CCoinControlWidgetItem(QTreeWidget *parent, int type = Type) : QTreeWidgetItem(parent, type) {} - explicit CCoinControlWidgetItem(int type = Type) : QTreeWidgetItem(type) {} explicit CCoinControlWidgetItem(QTreeWidgetItem *parent, int type = Type) : QTreeWidgetItem(parent, type) {} bool operator<(const QTreeWidgetItem &other) const; @@ -43,20 +42,18 @@ class CoinControlDialog : public QDialog Q_OBJECT public: - explicit CoinControlDialog(const PlatformStyle *platformStyle, QWidget *parent = nullptr); + explicit CoinControlDialog(CCoinControl& coin_control, WalletModel* model, const PlatformStyle *platformStyle, QWidget *parent = nullptr); ~CoinControlDialog(); - void setModel(WalletModel *model); - // static because also called from sendcoinsdialog - static void updateLabels(WalletModel*, QDialog*); + static void updateLabels(CCoinControl& m_coin_control, WalletModel*, QDialog*); static QList<CAmount> payAmounts; - static CCoinControl *coinControl(); static bool fSubtractFeeFromAmount; private: Ui::CoinControlDialog *ui; + CCoinControl& m_coin_control; WalletModel *model; int sortColumn; Qt::SortOrder sortOrder; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index a8c82aaf6c..2a6ef41221 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -57,6 +57,7 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *_platformStyle, QWidget *p ui(new Ui::SendCoinsDialog), clientModel(nullptr), model(nullptr), + m_coin_control(new CCoinControl), fNewRecipientAllowed(true), fFeeMinimized(true), platformStyle(_platformStyle) @@ -262,14 +263,9 @@ void SendCoinsDialog::on_sendButton_clicked() WalletModelTransaction currentTransaction(recipients); WalletModel::SendCoinsReturn prepareStatus; - // Always use a CCoinControl instance, use the CoinControlDialog instance if CoinControl has been enabled - CCoinControl ctrl; - if (model->getOptionsModel()->getCoinControlFeatures()) - ctrl = *CoinControlDialog::coinControl(); + updateCoinControlState(*m_coin_control); - updateCoinControlState(ctrl); - - prepareStatus = model->prepareTransaction(currentTransaction, ctrl); + prepareStatus = model->prepareTransaction(currentTransaction, *m_coin_control); // process prepareStatus and on error generate message shown to user processSendCoinsReturn(prepareStatus, @@ -413,7 +409,7 @@ void SendCoinsDialog::on_sendButton_clicked() } if (!send_failure) { accept(); - CoinControlDialog::coinControl()->UnSelectAll(); + m_coin_control->UnSelectAll(); coinControlUpdateLabels(); } fNewRecipientAllowed = true; @@ -422,7 +418,7 @@ void SendCoinsDialog::on_sendButton_clicked() void SendCoinsDialog::clear() { // Clear coin control settings - CoinControlDialog::coinControl()->UnSelectAll(); + m_coin_control->UnSelectAll(); ui->checkBoxCoinControlChange->setChecked(false); ui->lineEditCoinControlChange->clear(); coinControlUpdateLabels(); @@ -645,17 +641,11 @@ void SendCoinsDialog::on_buttonMinimizeFee_clicked() void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry) { - // Get CCoinControl instance if CoinControl is enabled or create a new one. - CCoinControl coin_control; - if (model->getOptionsModel()->getCoinControlFeatures()) { - coin_control = *CoinControlDialog::coinControl(); - } - // Include watch-only for wallets without private key - coin_control.fAllowWatchOnly = model->wallet().privateKeysDisabled(); + m_coin_control->fAllowWatchOnly = model->wallet().privateKeysDisabled(); // Calculate available amount to send. - CAmount amount = model->wallet().getAvailableBalance(coin_control); + CAmount amount = model->wallet().getAvailableBalance(*m_coin_control); for (int i = 0; i < ui->entries->count(); ++i) { SendCoinsEntry* e = qobject_cast<SendCoinsEntry*>(ui->entries->itemAt(i)->widget()); if (e && !e->isHidden() && e != entry) { @@ -714,12 +704,11 @@ void SendCoinsDialog::updateSmartFeeLabel() { if(!model || !model->getOptionsModel()) return; - CCoinControl coin_control; - updateCoinControlState(coin_control); - coin_control.m_feerate.reset(); // Explicitly use only fee estimation rate for smart fee labels + updateCoinControlState(*m_coin_control); + m_coin_control->m_feerate.reset(); // Explicitly use only fee estimation rate for smart fee labels int returned_target; FeeReason reason; - CFeeRate feeRate = CFeeRate(model->wallet().getMinimumFee(1000, coin_control, &returned_target, &reason)); + CFeeRate feeRate = CFeeRate(model->wallet().getMinimumFee(1000, *m_coin_control, &returned_target, &reason)); ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), feeRate.GetFeePerK()) + "/kB"); @@ -790,7 +779,7 @@ void SendCoinsDialog::coinControlFeatureChanged(bool checked) ui->frameCoinControl->setVisible(checked); if (!checked && model) // coin control features disabled - CoinControlDialog::coinControl()->SetNull(); + m_coin_control->SetNull(); coinControlUpdateLabels(); } @@ -798,8 +787,7 @@ void SendCoinsDialog::coinControlFeatureChanged(bool checked) // Coin Control: button inputs -> show actual coin control dialog void SendCoinsDialog::coinControlButtonClicked() { - CoinControlDialog dlg(platformStyle); - dlg.setModel(model); + CoinControlDialog dlg(*m_coin_control, model, platformStyle); dlg.exec(); coinControlUpdateLabels(); } @@ -809,7 +797,7 @@ void SendCoinsDialog::coinControlChangeChecked(int state) { if (state == Qt::Unchecked) { - CoinControlDialog::coinControl()->destChange = CNoDestination(); + m_coin_control->destChange = CNoDestination(); ui->labelCoinControlChangeLabel->clear(); } else @@ -825,7 +813,7 @@ void SendCoinsDialog::coinControlChangeEdited(const QString& text) if (model && model->getAddressTableModel()) { // Default to no change address until verified - CoinControlDialog::coinControl()->destChange = CNoDestination(); + m_coin_control->destChange = CNoDestination(); ui->labelCoinControlChangeLabel->setStyleSheet("QLabel{color:red;}"); const CTxDestination dest = DecodeDestination(text.toStdString()); @@ -848,7 +836,7 @@ void SendCoinsDialog::coinControlChangeEdited(const QString& text) QMessageBox::Yes | QMessageBox::Cancel, QMessageBox::Cancel); if(btnRetVal == QMessageBox::Yes) - CoinControlDialog::coinControl()->destChange = dest; + m_coin_control->destChange = dest; else { ui->lineEditCoinControlChange->setText(""); @@ -867,7 +855,7 @@ void SendCoinsDialog::coinControlChangeEdited(const QString& text) else ui->labelCoinControlChangeLabel->setText(tr("(no label)")); - CoinControlDialog::coinControl()->destChange = dest; + m_coin_control->destChange = dest; } } } @@ -879,7 +867,7 @@ void SendCoinsDialog::coinControlUpdateLabels() if (!model || !model->getOptionsModel()) return; - updateCoinControlState(*CoinControlDialog::coinControl()); + updateCoinControlState(*m_coin_control); // set pay amounts CoinControlDialog::payAmounts.clear(); @@ -897,10 +885,10 @@ void SendCoinsDialog::coinControlUpdateLabels() } } - if (CoinControlDialog::coinControl()->HasSelected()) + if (m_coin_control->HasSelected()) { // actual coin control calculation - CoinControlDialog::updateLabels(model, this); + CoinControlDialog::updateLabels(*m_coin_control, model, this); // show coin control stats ui->labelCoinControlAutomaticallySelected->hide(); diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h index 86422c4030..8eadc5a225 100644 --- a/src/qt/sendcoinsdialog.h +++ b/src/qt/sendcoinsdialog.h @@ -12,6 +12,7 @@ #include <QString> #include <QTimer> +class CCoinControl; class ClientModel; class PlatformStyle; class SendCoinsEntry; @@ -60,6 +61,7 @@ private: Ui::SendCoinsDialog *ui; ClientModel *clientModel; WalletModel *model; + std::unique_ptr<CCoinControl> m_coin_control; bool fNewRecipientAllowed; bool fFeeMinimized; const PlatformStyle *platformStyle; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index da9d583fa7..94590e0da4 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -724,7 +724,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) return result; } -class submitblock_StateCatcher : public CValidationInterface +class submitblock_StateCatcher final : public CValidationInterface { public: uint256 hash; @@ -792,17 +792,17 @@ static UniValue submitblock(const JSONRPCRequest& request) } bool new_block; - submitblock_StateCatcher sc(block.GetHash()); - RegisterValidationInterface(&sc); + auto sc = std::make_shared<submitblock_StateCatcher>(block.GetHash()); + RegisterSharedValidationInterface(sc); bool accepted = ProcessNewBlock(Params(), blockptr, /* fForceProcessing */ true, /* fNewBlock */ &new_block); - UnregisterValidationInterface(&sc); + UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { return "duplicate"; } - if (!sc.found) { + if (!sc->found) { return "inconclusive"; } - return BIP22ValidationResult(sc.state); + return BIP22ValidationResult(sc->state); } static UniValue submitheader(const JSONRPCRequest& request) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index e2618c16da..219979f095 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -25,7 +25,8 @@ static std::string rpcWarmupStatus GUARDED_BY(cs_rpcWarmup) = "RPC server starte /* Timer-creating functions */ static RPCTimerInterface* timerInterface = nullptr; /* Map of name to timer. */ -static std::map<std::string, std::unique_ptr<RPCTimerBase> > deadlineTimers; +static Mutex g_deadline_timers_mutex; +static std::map<std::string, std::unique_ptr<RPCTimerBase> > deadlineTimers GUARDED_BY(g_deadline_timers_mutex); static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler); struct RPCCommandExecutionInfo @@ -298,7 +299,7 @@ void InterruptRPC() void StopRPC() { LogPrint(BCLog::RPC, "Stopping RPC\n"); - deadlineTimers.clear(); + WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear()); DeleteAuthCookie(); g_rpcSignals.Stopped(); } @@ -486,6 +487,7 @@ void RPCRunLater(const std::string& name, std::function<void()> func, int64_t nS { if (!timerInterface) throw JSONRPCError(RPC_INTERNAL_ERROR, "No timer handler registered for RPC"); + LOCK(g_deadline_timers_mutex); deadlineTimers.erase(name); LogPrint(BCLog::RPC, "queue run of timer %s in %i seconds (using %s)\n", name, nSeconds, timerInterface->Name()); deadlineTimers.emplace(name, std::unique_ptr<RPCTimerBase>(timerInterface->NewTimer(func, nSeconds*1000))); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 9e3586d162..4255031904 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -19,16 +19,13 @@ #include <validationinterface.h> #include <version.h> -#include <algorithm> #include <atomic> #include <cassert> #include <chrono> #include <cstdint> #include <iosfwd> #include <iostream> -#include <map> #include <memory> -#include <set> #include <string> #include <vector> @@ -44,19 +41,6 @@ const std::string LIMIT_TO_MESSAGE_TYPE{TO_STRING(MESSAGE_TYPE)}; const std::string LIMIT_TO_MESSAGE_TYPE; #endif -const std::map<std::string, std::set<std::string>> EXPECTED_DESERIALIZATION_EXCEPTIONS = { - {"CDataStream::read(): end of data: iostream error", {"addr", "block", "blocktxn", "cmpctblock", "feefilter", "filteradd", "filterload", "getblocks", "getblocktxn", "getdata", "getheaders", "headers", "inv", "notfound", "ping", "sendcmpct", "tx"}}, - {"CompactSize exceeds limit of type: iostream error", {"cmpctblock"}}, - {"differential value overflow: iostream error", {"getblocktxn"}}, - {"index overflowed 16 bits: iostream error", {"getblocktxn"}}, - {"index overflowed 16-bits: iostream error", {"cmpctblock"}}, - {"indexes overflowed 16 bits: iostream error", {"getblocktxn"}}, - {"non-canonical ReadCompactSize(): iostream error", {"addr", "block", "blocktxn", "cmpctblock", "filteradd", "filterload", "getblocks", "getblocktxn", "getdata", "getheaders", "headers", "inv", "notfound", "tx"}}, - {"ReadCompactSize(): size too large: iostream error", {"addr", "block", "blocktxn", "cmpctblock", "filteradd", "filterload", "getblocks", "getblocktxn", "getdata", "getheaders", "headers", "inv", "notfound", "tx"}}, - {"Superfluous witness record: iostream error", {"block", "blocktxn", "cmpctblock", "tx"}}, - {"Unknown transaction optional data: iostream error", {"block", "blocktxn", "cmpctblock", "tx"}}, -}; - const RegTestingSetup* g_setup; } // namespace @@ -86,13 +70,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) g_setup->m_node.peer_logic->InitializeNode(&p2p_node); try { (void)ProcessMessage(&p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.mempool, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic<bool>{false}); - } catch (const std::ios_base::failure& e) { - const std::string exception_message{e.what()}; - const auto p = EXPECTED_DESERIALIZATION_EXCEPTIONS.find(exception_message); - if (p == EXPECTED_DESERIALIZATION_EXCEPTIONS.cend() || p->second.count(random_message_type) == 0) { - std::cout << "Unexpected exception when processing message type \"" << random_message_type << "\": " << exception_message << std::endl; - assert(false); - } + } catch (const std::ios_base::failure&) { } SyncWithValidationInterfaceQueue(); } diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index afb3db36a2..e61ea13b02 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -32,7 +32,7 @@ struct MinerTestingSetup : public RegTestingSetup { BOOST_FIXTURE_TEST_SUITE(validation_block_tests, MinerTestingSetup) -struct TestSubscriber : public CValidationInterface { +struct TestSubscriber final : public CValidationInterface { uint256 m_expected_tip; explicit TestSubscriber(uint256 tip) : m_expected_tip(tip) {} @@ -175,8 +175,8 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) LOCK(cs_main); initial_tip = ::ChainActive().Tip(); } - TestSubscriber sub(initial_tip->GetBlockHash()); - RegisterValidationInterface(&sub); + auto sub = std::make_shared<TestSubscriber>(initial_tip->GetBlockHash()); + RegisterSharedValidationInterface(sub); // create a bunch of threads that repeatedly process a block generated above at random // this will create parallelism and randomness inside validation - the ValidationInterface @@ -208,10 +208,10 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) UninterruptibleSleep(std::chrono::milliseconds{100}); } - UnregisterValidationInterface(&sub); + UnregisterSharedValidationInterface(sub); LOCK(cs_main); - BOOST_CHECK_EQUAL(sub.m_expected_tip, ::ChainActive().Tip()->GetBlockHash()); + BOOST_CHECK_EQUAL(sub->m_expected_tip, ::ChainActive().Tip()->GetBlockHash()); } /** diff --git a/src/test/validationinterface_tests.cpp b/src/test/validationinterface_tests.cpp index 208be92852..ceba689e52 100644 --- a/src/test/validationinterface_tests.cpp +++ b/src/test/validationinterface_tests.cpp @@ -12,6 +12,40 @@ BOOST_FIXTURE_TEST_SUITE(validationinterface_tests, TestingSetup) +struct TestSubscriberNoop final : public CValidationInterface { + void BlockChecked(const CBlock&, const BlockValidationState&) override {} +}; + +BOOST_AUTO_TEST_CASE(unregister_validation_interface_race) +{ + std::atomic<bool> generate{true}; + + // Start thread to generate notifications + std::thread gen{[&] { + const CBlock block_dummy; + BlockValidationState state_dummy; + while (generate) { + GetMainSignals().BlockChecked(block_dummy, state_dummy); + } + }}; + + // Start thread to consume notifications + std::thread sub{[&] { + // keep going for about 1 sec, which is 250k iterations + for (int i = 0; i < 250000; i++) { + auto sub = std::make_shared<TestSubscriberNoop>(); + RegisterSharedValidationInterface(sub); + UnregisterSharedValidationInterface(sub); + } + // tell the other thread we are done + generate = false; + }}; + + gen.join(); + sub.join(); + BOOST_CHECK(!generate); +} + class TestInterface : public CValidationInterface { public: diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 61ad2f1198..a5036413c8 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1917,6 +1917,9 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) }.Check(request); int64_t nSleepTime; + int64_t relock_time; + // Prevent concurrent calls to walletpassphrase with the same wallet. + LOCK(pwallet->m_unlock_mutex); { auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); @@ -1955,6 +1958,7 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) pwallet->TopUpKeyPool(); pwallet->nRelockTime = GetTime() + nSleepTime; + relock_time = pwallet->nRelockTime; } // rpcRunLater must be called without cs_wallet held otherwise a deadlock @@ -1966,9 +1970,11 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) // wallet before the following callback is called. If a valid shared pointer // is acquired in the callback then the wallet is still loaded. std::weak_ptr<CWallet> weak_wallet = wallet; - pwallet->chain().rpcRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet] { + pwallet->chain().rpcRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet, relock_time] { if (auto shared_wallet = weak_wallet.lock()) { LOCK(shared_wallet->cs_wallet); + // Skip if this is not the most recent rpcRunLater callback. + if (shared_wallet->nRelockTime != relock_time) return; shared_wallet->Lock(); shared_wallet->nRelockTime = 0; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6c54c72e76..7446a4889a 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -867,8 +867,10 @@ public: std::vector<std::string> GetDestValues(const std::string& prefix) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Holds a timestamp at which point the wallet is scheduled (externally) to be relocked. Caller must arrange for actual relocking to occur via Lock(). - int64_t nRelockTime = 0; + int64_t nRelockTime GUARDED_BY(cs_wallet){0}; + // Used to prevent concurrent calls to walletpassphrase RPC. + Mutex m_unlock_mutex; bool Unlock(const SecureString& strWalletPassphrase, bool accept_no_keys = false); bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); bool EncryptWallet(const SecureString& strWalletPassphrase); |