highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-20 07:09:15 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge #18973: [0.20] Final backports for rc2

Browse Source 
245c862cfd test: disable script fuzz tests (fanquake)
9a8fb4cf4b fuzz: Remove enumeration of expected deserialization exceptions in ProcessMessage(...) fuzzer (practicalswift)
6161c94a61 [net processing] Only send a getheaders for one block in an INV (John Newbery)
cf2a6e2a39 test: Remove const to work around compiler error on xenial (Wladimir J. van der Laan)
cc7d34465b miner: Avoid stack-use-after-return in validationinterface (MarcoFalke)
37a620748b test: Add unregister_validation_interface_race test (MarcoFalke)
ff4dc20750 gui: Fix manual coin control with multiple wallets loaded (João Barbosa)
ed0afe8c1f test: Add test for conflicted wallet tx notifications (Russell Yanofsky)
251e321ad7 rpc: Relock wallet only if most recent callback (João Barbosa)
ca4dac48c5 rpc: Add mutex to guard deadlineTimers (João Barbosa)
a3fe458a1e [docs] Improve commenting in ProcessGetData() (John Newbery)
011532e380 [test] test that an invalid GETDATA doesn't prevent processing of future messages (Amiti Uttarwar)
1e73d7248a [net processing] ignore unknown INV types in GETDATA messages (Amiti Uttarwar)
fb821731eb [net processing] ignore tx GETDATA from blocks-only peers (Amiti Uttarwar)
315ae14f3f gui: Fix itemWalletAddress leak when not tree mode (João Barbosa)

Pull request description:

  Backports the following PRs to the 0.20 branch:

  * https://github.com/bitcoin/bitcoin/pull/18578: gui: Fix leak in CoinControlDialog::updateView
  * https://github.com/bitcoin/bitcoin/pull/18808: [net processing] Drop unknown types in getdata
  * https://github.com/bitcoin/bitcoin/pull/18814: rpc: Relock wallet only if most recent callback
  * https://github.com/bitcoin/bitcoin/pull/18878: test: Add test for conflicted wallet tx notifications
  * https://github.com/bitcoin/bitcoin/pull/18894: gui: Fix manual coin control with multiple wallets loaded
  * https://github.com/bitcoin/bitcoin/pull/18742: miner: Avoid stack-use-after-return in validationinterface
  * https://github.com/bitcoin/bitcoin/pull/18962: net processing: Only send a getheaders for one block in an INV
  * https://github.com/bitcoin/bitcoin/pull/18975: test: Remove const to work around compiler error on xenial

ACKs for top commit:
  promag:
    Tested ACK 245c862cfd coin control with multiple wallets.
  laanwj:
    ACK 245c862cfd
  MarcoFalke:
    ACK 245c862cfd solved the conflicts myself as a sanity check. Did not re-review 🍷

Tree-SHA512: 285e5a5fad5bbeba6032742c65dc68836e8eccfcceda9e69fec4ddd162a3f61679a96f9bbe3d434267835af67c21ac4c05accf6f63e827c2eb47203c6daafe31
This commit is contained in:
MarcoFalke
2020-05-15 07:54:29 -04:00
parent aa7c6858e6 245c862cfd
commit 17bdf2afae
16 changed files with 273 additions and 150 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/Makefile.test.include
View File

@@ -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
# 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_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)

64
src/net_processing.cpp
View File

@@ -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;
}

57
src/qt/coincontroldialog.cpp
View File

@@ -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);
}

9
src/qt/coincontroldialog.h
View File

@@ -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;

50
src/qt/sendcoinsdialog.cpp
View File

@@ -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();

2
src/qt/sendcoinsdialog.h
View File

@@ -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;

12
src/rpc/mining.cpp
View File

@@ -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)

6
src/rpc/server.cpp
View File

@@ -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)));

24
src/test/fuzz/process_message.cpp
View File

@@ -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();
}

10
src/test/validation_block_tests.cpp
View File

@@ -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());
}
/**

34
src/test/validationinterface_tests.cpp
View File

@@ -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:

8
src/wallet/rpcwallet.cpp
View File

@@ -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;
}

4
src/wallet/wallet.h
View File

@@ -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);

68
test/functional/feature_notifications.py
View File

@@ -5,12 +5,14 @@
"""Test the -alertnotify, -blocknotify and -walletnotify options."""
import os
from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE
from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE, keyhash_to_p2pkh
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
wait_until,
connect_nodes,
disconnect_nodes,
hex_str_to_bytes,
)
# Linux allow all characters other than \x00
@@ -81,8 +83,72 @@ class NotificationsTest(BitcoinTestFramework):
# directory content should equal the generated transaction hashes
txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", block_count)))
assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir)))
for tx_file in os.listdir(self.walletnotify_dir):
os.remove(os.path.join(self.walletnotify_dir, tx_file))
# Conflicting transactions tests. Give node 0 same wallet seed as
# node 1, generate spends from node 0, and check notifications
# triggered by node 1
self.log.info("test -walletnotify with conflicting transactions")
self.nodes[0].sethdseed(seed=self.nodes[1].dumpprivkey(keyhash_to_p2pkh(hex_str_to_bytes(self.nodes[1].getwalletinfo()['hdseedid'])[::-1])))
self.nodes[0].rescanblockchain()
self.nodes[0].generatetoaddress(100, ADDRESS_BCRT1_UNSPENDABLE)
# Generate transaction on node 0, sync mempools, and check for
# notification on node 1.
tx1 = self.nodes[0].sendtoaddress(address=ADDRESS_BCRT1_UNSPENDABLE, amount=1, replaceable=True)
assert_equal(tx1 in self.nodes[0].getrawmempool(), True)
self.sync_mempools()
self.expect_wallet_notify([tx1])
# Generate bump transaction, sync mempools, and check for bump1
# notification. In the future, per
# https://github.com/bitcoin/bitcoin/pull/9371, it might be better
# to have notifications for both tx1 and bump1.
bump1 = self.nodes[0].bumpfee(tx1)["txid"]
assert_equal(bump1 in self.nodes[0].getrawmempool(), True)
self.sync_mempools()
self.expect_wallet_notify([bump1])
# Add bump1 transaction to new block, checking for a notification
# and the correct number of confirmations.
self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
self.sync_blocks()
self.expect_wallet_notify([bump1])
assert_equal(self.nodes[1].gettransaction(bump1)["confirmations"], 1)
# Generate a second transaction to be bumped.
tx2 = self.nodes[0].sendtoaddress(address=ADDRESS_BCRT1_UNSPENDABLE, amount=1, replaceable=True)
assert_equal(tx2 in self.nodes[0].getrawmempool(), True)
self.sync_mempools()
self.expect_wallet_notify([tx2])
# Bump tx2 as bump2 and generate a block on node 0 while
# disconnected, then reconnect and check for notifications on node 1
# about newly confirmed bump2 and newly conflicted tx2. Currently
# only the bump2 notification is sent. Ideally, notifications would
# be sent both for bump2 and tx2, which was the previous behavior
# before being broken by an accidental change in PR
# https://github.com/bitcoin/bitcoin/pull/16624. The bug is reported
# in issue https://github.com/bitcoin/bitcoin/issues/18325.
disconnect_nodes(self.nodes[0], 1)
bump2 = self.nodes[0].bumpfee(tx2)["txid"]
self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
assert_equal(self.nodes[0].gettransaction(bump2)["confirmations"], 1)
assert_equal(tx2 in self.nodes[1].getrawmempool(), True)
connect_nodes(self.nodes[0], 1)
self.sync_blocks()
self.expect_wallet_notify([bump2])
assert_equal(self.nodes[1].gettransaction(bump2)["confirmations"], 1)
# TODO: add test for `-alertnotify` large fork notifications
def expect_wallet_notify(self, tx_ids):
wait_until(lambda: len(os.listdir(self.walletnotify_dir)) >= len(tx_ids), timeout=10)
assert_equal(sorted(notify_outputname(self.wallet, tx_id) for tx_id in tx_ids), sorted(os.listdir(self.walletnotify_dir)))
for tx_file in os.listdir(self.walletnotify_dir):
os.remove(os.path.join(self.walletnotify_dir, tx_file))
if __name__ == '__main__':
NotificationsTest().main()

51
test/functional/p2p_getdata.py Executable file
View File

@@ -0,0 +1,51 @@
#!/usr/bin/env python3
# Copyright (c) 2020 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test GETDATA processing behavior"""
from collections import defaultdict
from test_framework.messages import (
CInv,
msg_getdata,
)
from test_framework.mininode import (
mininode_lock,
P2PInterface,
)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import wait_until
class P2PStoreBlock(P2PInterface):
def __init__(self):
super().__init__()
self.blocks = defaultdict(int)
def on_block(self, message):
message.block.calc_sha256()
self.blocks[message.block.sha256] += 1
class GetdataTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
def run_test(self):
self.nodes[0].add_p2p_connection(P2PStoreBlock())
self.log.info("test that an invalid GETDATA doesn't prevent processing of future messages")
# Send invalid message and verify that node responds to later ping
invalid_getdata = msg_getdata()
invalid_getdata.inv.append(CInv(t=0, h=0)) # INV type 0 is invalid.
self.nodes[0].p2ps[0].send_and_ping(invalid_getdata)
# Check getdata still works by fetching tip block
best_block = int(self.nodes[0].getbestblockhash(), 16)
good_getdata = msg_getdata()
good_getdata.inv.append(CInv(t=2, h=best_block))
self.nodes[0].p2ps[0].send_and_ping(good_getdata)
wait_until(lambda: self.nodes[0].p2ps[0].blocks[best_block] == 1, timeout=30, lock=mininode_lock)
if __name__ == '__main__':
GetdataTest().main()

1
test/functional/test_runner.py
View File

@@ -145,6 +145,7 @@ BASE_SCRIPTS = [
'rpc_deprecated.py',
'wallet_disable.py',
'p2p_addr_relay.py',
'p2p_getdata.py',
'rpc_net.py',
'wallet_keypool.py',
'p2p_mempool.py',