mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-06-02 11:11:04 +02:00
9a1d73fdffa4f35e33bc187ac9b3afbba28b1950 Fix segfault when shutdown during wallet open (John Moffett) Pull request description: Fixes #689 ## Summary If you open a wallet and send a shutdown signal during that process, you'll get a segfault when the wallet finishes opening. That's because the `WalletController` object gets deleted manually in bitcoin.cpp during shutdown, but copies of the pointer (and pointers to child objects) are dangling in various places and are accessed in queued events after the deletion. ## Details The issue in #689 is caused by the following sequence of events: 1. Wallet open modal dialog is shown and worker thread does the actual work. 2. Every 200ms, the main event loop checks to see if a shutdown has been requested, but only if a modal is not being shown. 3. Request a shutdown while the modal window is shown. 4. The wallet open process completes, the modal window is dismissed, and various `finish` signals are sent. 5. During handling of one of the `finish` signals, `qApp->processEvents()` is [called](e9262ea32a/src/qt/sendcoinsdialog.cpp (L603)
), which causes the main event loop to detect the shutdown (now that the modal window has been dismissed). The `WalletController` and all the `WalletModel`s are [deleted](65de8eeeca/src/qt/bitcoin.cpp (L394-L401)
). 6. Control returns to the `finish` method, which eventually tries to send a [signal](e9262ea32a/src/qt/sendcoinsdialog.cpp (L167)
) from a wallet model, but it's been deleted already (and the signal is sent from a now-[dangling](d8bdee0fc8/src/qt/walletview.cpp (L65)
) pointer). The simplest fix for that is to change the `qApp->processEvents()` into a `QueuedConnection` call. (The `qApp->processEvents() was a [workaround](https://github.com/bitcoin/bitcoin/pull/593#issuecomment-3050699) to get the GUI to scroll to the last item in a list that just got added, and this is just a safer way of doing that). However, once that segfault is fixed, another segfault occurs due to some queued wallet events happening after the wallet controller object is deleted here:65de8eeeca/src/qt/bitcoin.cpp (L394-L401)
Since `m_wallet_controller` is a copy of that pointer in `bitcoingui.cpp`, it's now dangling and `if(null)` checks won't work correctly. For instance, this line:65de8eeeca/src/qt/bitcoingui.cpp (L413)
sets up a `QueuedConnection` to `setCurrentWallet`, but by the time control reaches that method (one event cycle after shutdown deleted `m_wallet_controller` in `bitcoin.cpp`), the underlying objects have been destroyed (but the pointers are still dangling). Ideally, we'd use a `QPointer` or `std::shared_ptr / std::weak_ptr`s for these, but the changes would be more involved. This is a minimal fix for the issues. Just set `m_wallet_controller` to `nullptr` in `bitcoingui.cpp`, check its value in a couple places, and avoid a use of `qApp->processEvents`. ACKs for top commit: hebasto: ACK 9a1d73fdffa4f35e33bc187ac9b3afbba28b1950, I have reviewed the code and it looks OK. furszy: ACK9a1d73fdff
Tree-SHA512: a1b94676eb2fcb7606e68fab443b1565b4122aab93c35382b561842a049f4b43fecc459535370d67a64d6ebc4bcec0ebcda981fff633ebd41bdba6f7093ea540
333 lines
15 KiB
C++
333 lines
15 KiB
C++
// Copyright (c) 2015-2022 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 <qt/test/wallettests.h>
|
|
#include <qt/test/util.h>
|
|
|
|
#include <interfaces/chain.h>
|
|
#include <interfaces/node.h>
|
|
#include <key_io.h>
|
|
#include <qt/bitcoinamountfield.h>
|
|
#include <qt/bitcoinunits.h>
|
|
#include <qt/clientmodel.h>
|
|
#include <qt/optionsmodel.h>
|
|
#include <qt/overviewpage.h>
|
|
#include <qt/platformstyle.h>
|
|
#include <qt/qvalidatedlineedit.h>
|
|
#include <qt/receivecoinsdialog.h>
|
|
#include <qt/receiverequestdialog.h>
|
|
#include <qt/recentrequeststablemodel.h>
|
|
#include <qt/sendcoinsdialog.h>
|
|
#include <qt/sendcoinsentry.h>
|
|
#include <qt/transactiontablemodel.h>
|
|
#include <qt/transactionview.h>
|
|
#include <qt/walletmodel.h>
|
|
#include <test/util/setup_common.h>
|
|
#include <validation.h>
|
|
#include <wallet/wallet.h>
|
|
|
|
#include <chrono>
|
|
#include <memory>
|
|
|
|
#include <QAbstractButton>
|
|
#include <QAction>
|
|
#include <QApplication>
|
|
#include <QCheckBox>
|
|
#include <QPushButton>
|
|
#include <QTimer>
|
|
#include <QVBoxLayout>
|
|
#include <QTextEdit>
|
|
#include <QListView>
|
|
#include <QDialogButtonBox>
|
|
|
|
using wallet::AddWallet;
|
|
using wallet::CWallet;
|
|
using wallet::CreateMockWalletDatabase;
|
|
using wallet::RemoveWallet;
|
|
using wallet::WALLET_FLAG_DESCRIPTORS;
|
|
using wallet::WalletContext;
|
|
using wallet::WalletDescriptor;
|
|
using wallet::WalletRescanReserver;
|
|
|
|
namespace
|
|
{
|
|
//! Press "Yes" or "Cancel" buttons in modal send confirmation dialog.
|
|
void ConfirmSend(QString* text = nullptr, bool cancel = false)
|
|
{
|
|
QTimer::singleShot(0, [text, cancel]() {
|
|
for (QWidget* widget : QApplication::topLevelWidgets()) {
|
|
if (widget->inherits("SendConfirmationDialog")) {
|
|
SendConfirmationDialog* dialog = qobject_cast<SendConfirmationDialog*>(widget);
|
|
if (text) *text = dialog->text();
|
|
QAbstractButton* button = dialog->button(cancel ? QMessageBox::Cancel : QMessageBox::Yes);
|
|
button->setEnabled(true);
|
|
button->click();
|
|
}
|
|
}
|
|
});
|
|
}
|
|
|
|
//! Send coins to address and return txid.
|
|
uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDestination& address, CAmount amount, bool rbf)
|
|
{
|
|
QVBoxLayout* entries = sendCoinsDialog.findChild<QVBoxLayout*>("entries");
|
|
SendCoinsEntry* entry = qobject_cast<SendCoinsEntry*>(entries->itemAt(0)->widget());
|
|
entry->findChild<QValidatedLineEdit*>("payTo")->setText(QString::fromStdString(EncodeDestination(address)));
|
|
entry->findChild<BitcoinAmountField*>("payAmount")->setValue(amount);
|
|
sendCoinsDialog.findChild<QFrame*>("frameFee")
|
|
->findChild<QFrame*>("frameFeeSelection")
|
|
->findChild<QCheckBox*>("optInRBF")
|
|
->setCheckState(rbf ? Qt::Checked : Qt::Unchecked);
|
|
uint256 txid;
|
|
boost::signals2::scoped_connection c(wallet.NotifyTransactionChanged.connect([&txid](const uint256& hash, ChangeType status) {
|
|
if (status == CT_NEW) txid = hash;
|
|
}));
|
|
ConfirmSend();
|
|
bool invoked = QMetaObject::invokeMethod(&sendCoinsDialog, "sendButtonClicked", Q_ARG(bool, false));
|
|
assert(invoked);
|
|
return txid;
|
|
}
|
|
|
|
//! Find index of txid in transaction list.
|
|
QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
|
|
{
|
|
QString hash = QString::fromStdString(txid.ToString());
|
|
int rows = model.rowCount({});
|
|
for (int row = 0; row < rows; ++row) {
|
|
QModelIndex index = model.index(row, 0, {});
|
|
if (model.data(index, TransactionTableModel::TxHashRole) == hash) {
|
|
return index;
|
|
}
|
|
}
|
|
return {};
|
|
}
|
|
|
|
//! Invoke bumpfee on txid and check results.
|
|
void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, std::string expectError, bool cancel)
|
|
{
|
|
QTableView* table = view.findChild<QTableView*>("transactionView");
|
|
QModelIndex index = FindTx(*table->selectionModel()->model(), txid);
|
|
QVERIFY2(index.isValid(), "Could not find BumpFee txid");
|
|
|
|
// Select row in table, invoke context menu, and make sure bumpfee action is
|
|
// enabled or disabled as expected.
|
|
QAction* action = view.findChild<QAction*>("bumpFeeAction");
|
|
table->selectionModel()->select(index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
|
|
action->setEnabled(expectDisabled);
|
|
table->customContextMenuRequested({});
|
|
QCOMPARE(action->isEnabled(), !expectDisabled);
|
|
|
|
action->setEnabled(true);
|
|
QString text;
|
|
if (expectError.empty()) {
|
|
ConfirmSend(&text, cancel);
|
|
} else {
|
|
ConfirmMessage(&text, 0ms);
|
|
}
|
|
action->trigger();
|
|
QVERIFY(text.indexOf(QString::fromStdString(expectError)) != -1);
|
|
}
|
|
|
|
void CompareBalance(WalletModel& walletModel, CAmount expected_balance, QLabel* balance_label_to_check)
|
|
{
|
|
BitcoinUnit unit = walletModel.getOptionsModel()->getDisplayUnit();
|
|
QString balanceComparison = BitcoinUnits::formatWithUnit(unit, expected_balance, false, BitcoinUnits::SeparatorStyle::ALWAYS);
|
|
QCOMPARE(balance_label_to_check->text().trimmed(), balanceComparison);
|
|
}
|
|
|
|
//! Simple qt wallet tests.
|
|
//
|
|
// Test widgets can be debugged interactively calling show() on them and
|
|
// manually running the event loop, e.g.:
|
|
//
|
|
// sendCoinsDialog.show();
|
|
// QEventLoop().exec();
|
|
//
|
|
// This also requires overriding the default minimal Qt platform:
|
|
//
|
|
// QT_QPA_PLATFORM=xcb src/qt/test/test_bitcoin-qt # Linux
|
|
// QT_QPA_PLATFORM=windows src/qt/test/test_bitcoin-qt # Windows
|
|
// QT_QPA_PLATFORM=cocoa src/qt/test/test_bitcoin-qt # macOS
|
|
void TestGUI(interfaces::Node& node)
|
|
{
|
|
// Set up wallet and chain with 105 blocks (5 mature blocks for spending).
|
|
TestChain100Setup test;
|
|
for (int i = 0; i < 5; ++i) {
|
|
test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
|
|
}
|
|
auto wallet_loader = interfaces::MakeWalletLoader(*test.m_node.chain, *Assert(test.m_node.args));
|
|
test.m_node.wallet_loader = wallet_loader.get();
|
|
node.setContext(&test.m_node);
|
|
const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
|
|
wallet->LoadWallet();
|
|
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
|
|
{
|
|
LOCK(wallet->cs_wallet);
|
|
wallet->SetupDescriptorScriptPubKeyMans();
|
|
|
|
// Add the coinbase key
|
|
FlatSigningProvider provider;
|
|
std::string error;
|
|
std::unique_ptr<Descriptor> desc = Parse("combo(" + EncodeSecret(test.coinbaseKey) + ")", provider, error, /* require_checksum=*/ false);
|
|
assert(desc);
|
|
WalletDescriptor w_desc(std::move(desc), 0, 0, 1, 1);
|
|
if (!wallet->AddWalletDescriptor(w_desc, provider, "", false)) assert(false);
|
|
CTxDestination dest = GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type);
|
|
wallet->SetAddressBook(dest, "", "receive");
|
|
wallet->SetLastBlockProcessed(105, WITH_LOCK(node.context()->chainman->GetMutex(), return node.context()->chainman->ActiveChain().Tip()->GetBlockHash()));
|
|
}
|
|
{
|
|
WalletRescanReserver reserver(*wallet);
|
|
reserver.reserve();
|
|
CWallet::ScanResult result = wallet->ScanForWalletTransactions(Params().GetConsensus().hashGenesisBlock, /*start_height=*/0, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/false);
|
|
QCOMPARE(result.status, CWallet::ScanResult::SUCCESS);
|
|
QCOMPARE(result.last_scanned_block, WITH_LOCK(node.context()->chainman->GetMutex(), return node.context()->chainman->ActiveChain().Tip()->GetBlockHash()));
|
|
QVERIFY(result.last_failed_block.IsNull());
|
|
}
|
|
wallet->SetBroadcastTransactions(true);
|
|
|
|
// Create widgets for sending coins and listing transactions.
|
|
std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other"));
|
|
SendCoinsDialog sendCoinsDialog(platformStyle.get());
|
|
TransactionView transactionView(platformStyle.get());
|
|
OptionsModel optionsModel(node);
|
|
bilingual_str error;
|
|
QVERIFY(optionsModel.Init(error));
|
|
ClientModel clientModel(node, &optionsModel);
|
|
WalletContext& context = *node.walletLoader().context();
|
|
AddWallet(context, wallet);
|
|
WalletModel walletModel(interfaces::MakeWallet(context, wallet), clientModel, platformStyle.get());
|
|
RemoveWallet(context, wallet, /* load_on_start= */ std::nullopt);
|
|
sendCoinsDialog.setModel(&walletModel);
|
|
transactionView.setModel(&walletModel);
|
|
|
|
// Update walletModel cached balance which will trigger an update for the 'labelBalance' QLabel.
|
|
walletModel.pollBalanceChanged();
|
|
// Check balance in send dialog
|
|
CompareBalance(walletModel, walletModel.wallet().getBalance(), sendCoinsDialog.findChild<QLabel*>("labelBalance"));
|
|
|
|
// Send two transactions, and verify they are added to transaction list.
|
|
TransactionTableModel* transactionTableModel = walletModel.getTransactionTableModel();
|
|
QCOMPARE(transactionTableModel->rowCount({}), 105);
|
|
uint256 txid1 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 5 * COIN, /*rbf=*/false);
|
|
uint256 txid2 = SendCoins(*wallet.get(), sendCoinsDialog, PKHash(), 10 * COIN, /*rbf=*/true);
|
|
// Transaction table model updates on a QueuedConnection, so process events to ensure it's updated.
|
|
qApp->processEvents();
|
|
QCOMPARE(transactionTableModel->rowCount({}), 107);
|
|
QVERIFY(FindTx(*transactionTableModel, txid1).isValid());
|
|
QVERIFY(FindTx(*transactionTableModel, txid2).isValid());
|
|
|
|
// Call bumpfee. Test disabled, canceled, enabled, then failing cases.
|
|
BumpFee(transactionView, txid1, /*expectDisabled=*/true, /*expectError=*/"not BIP 125 replaceable", /*cancel=*/false);
|
|
BumpFee(transactionView, txid2, /*expectDisabled=*/false, /*expectError=*/{}, /*cancel=*/true);
|
|
BumpFee(transactionView, txid2, /*expectDisabled=*/false, /*expectError=*/{}, /*cancel=*/false);
|
|
BumpFee(transactionView, txid2, /*expectDisabled=*/true, /*expectError=*/"already bumped", /*cancel=*/false);
|
|
|
|
// Check current balance on OverviewPage
|
|
OverviewPage overviewPage(platformStyle.get());
|
|
overviewPage.setWalletModel(&walletModel);
|
|
walletModel.pollBalanceChanged(); // Manual balance polling update
|
|
CompareBalance(walletModel, walletModel.wallet().getBalance(), overviewPage.findChild<QLabel*>("labelBalance"));
|
|
|
|
// Check Request Payment button
|
|
ReceiveCoinsDialog receiveCoinsDialog(platformStyle.get());
|
|
receiveCoinsDialog.setModel(&walletModel);
|
|
RecentRequestsTableModel* requestTableModel = walletModel.getRecentRequestsTableModel();
|
|
|
|
// Label input
|
|
QLineEdit* labelInput = receiveCoinsDialog.findChild<QLineEdit*>("reqLabel");
|
|
labelInput->setText("TEST_LABEL_1");
|
|
|
|
// Amount input
|
|
BitcoinAmountField* amountInput = receiveCoinsDialog.findChild<BitcoinAmountField*>("reqAmount");
|
|
amountInput->setValue(1);
|
|
|
|
// Message input
|
|
QLineEdit* messageInput = receiveCoinsDialog.findChild<QLineEdit*>("reqMessage");
|
|
messageInput->setText("TEST_MESSAGE_1");
|
|
int initialRowCount = requestTableModel->rowCount({});
|
|
QPushButton* requestPaymentButton = receiveCoinsDialog.findChild<QPushButton*>("receiveButton");
|
|
requestPaymentButton->click();
|
|
QString address;
|
|
for (QWidget* widget : QApplication::topLevelWidgets()) {
|
|
if (widget->inherits("ReceiveRequestDialog")) {
|
|
ReceiveRequestDialog* receiveRequestDialog = qobject_cast<ReceiveRequestDialog*>(widget);
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("payment_header")->text(), QString("Payment information"));
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("uri_tag")->text(), QString("URI:"));
|
|
QString uri = receiveRequestDialog->QObject::findChild<QLabel*>("uri_content")->text();
|
|
QCOMPARE(uri.count("bitcoin:"), 2);
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("address_tag")->text(), QString("Address:"));
|
|
QVERIFY(address.isEmpty());
|
|
address = receiveRequestDialog->QObject::findChild<QLabel*>("address_content")->text();
|
|
QVERIFY(!address.isEmpty());
|
|
|
|
QCOMPARE(uri.count("amount=0.00000001"), 2);
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("amount_tag")->text(), QString("Amount:"));
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("amount_content")->text(), QString::fromStdString("0.00000001 " + CURRENCY_UNIT));
|
|
|
|
QCOMPARE(uri.count("label=TEST_LABEL_1"), 2);
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("label_tag")->text(), QString("Label:"));
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("label_content")->text(), QString("TEST_LABEL_1"));
|
|
|
|
QCOMPARE(uri.count("message=TEST_MESSAGE_1"), 2);
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("message_tag")->text(), QString("Message:"));
|
|
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("message_content")->text(), QString("TEST_MESSAGE_1"));
|
|
}
|
|
}
|
|
|
|
// Clear button
|
|
QPushButton* clearButton = receiveCoinsDialog.findChild<QPushButton*>("clearButton");
|
|
clearButton->click();
|
|
QCOMPARE(labelInput->text(), QString(""));
|
|
QCOMPARE(amountInput->value(), CAmount(0));
|
|
QCOMPARE(messageInput->text(), QString(""));
|
|
|
|
// Check addition to history
|
|
int currentRowCount = requestTableModel->rowCount({});
|
|
QCOMPARE(currentRowCount, initialRowCount+1);
|
|
|
|
// Check addition to wallet
|
|
std::vector<std::string> requests = walletModel.wallet().getAddressReceiveRequests();
|
|
QCOMPARE(requests.size(), size_t{1});
|
|
RecentRequestEntry entry;
|
|
DataStream{MakeUCharSpan(requests[0])} >> entry;
|
|
QCOMPARE(entry.nVersion, int{1});
|
|
QCOMPARE(entry.id, int64_t{1});
|
|
QVERIFY(entry.date.isValid());
|
|
QCOMPARE(entry.recipient.address, address);
|
|
QCOMPARE(entry.recipient.label, QString{"TEST_LABEL_1"});
|
|
QCOMPARE(entry.recipient.amount, CAmount{1});
|
|
QCOMPARE(entry.recipient.message, QString{"TEST_MESSAGE_1"});
|
|
QCOMPARE(entry.recipient.sPaymentRequest, std::string{});
|
|
QCOMPARE(entry.recipient.authenticatedMerchant, QString{});
|
|
|
|
// Check Remove button
|
|
QTableView* table = receiveCoinsDialog.findChild<QTableView*>("recentRequestsView");
|
|
table->selectRow(currentRowCount-1);
|
|
QPushButton* removeRequestButton = receiveCoinsDialog.findChild<QPushButton*>("removeRequestButton");
|
|
removeRequestButton->click();
|
|
QCOMPARE(requestTableModel->rowCount({}), currentRowCount-1);
|
|
|
|
// Check removal from wallet
|
|
QCOMPARE(walletModel.wallet().getAddressReceiveRequests().size(), size_t{0});
|
|
}
|
|
|
|
} // namespace
|
|
|
|
void WalletTests::walletTests()
|
|
{
|
|
#ifdef Q_OS_MACOS
|
|
if (QApplication::platformName() == "minimal") {
|
|
// Disable for mac on "minimal" platform to avoid crashes inside the Qt
|
|
// framework when it tries to look up unimplemented cocoa functions,
|
|
// and fails to handle returned nulls
|
|
// (https://bugreports.qt.io/browse/QTBUG-49686).
|
|
QWARN("Skipping WalletTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke "
|
|
"with 'QT_QPA_PLATFORM=cocoa test_bitcoin-qt' on mac, or else use a linux or windows build.");
|
|
return;
|
|
}
|
|
#endif
|
|
TestGUI(m_node);
|
|
}
|