foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

Merge bitcoin-core/gui#824: Migrate legacy wallets that are not loaded

Browse source 
8f2522d242 gui: Use menu for wallet migration (Ava Chow)
d56a450bf5 gui: Use wallet name for wallet migration rather than WalletModel (Ava Chow)
c3918583dd gui: don't remove wallet manually before migration (furszy)
bfba63880f gui: Consolidate wallet display name to GUIUtil function (Ava Chow)
28fc562f26 wallet, interfaces: Include database format in listWalletDir (Ava Chow)

Pull request description:

  Currently the Migrate Wallet menu item can only be used to migrate the currently loaded wallet. This is not suitable for the future when legacy wallets can no longer be loaded at all, but should still be able to be migrated. This PR changes that menu item into a menu list like Open Wallet and lets users migrate any legacy wallet in their wallet directory regardless of the wallets loaded.

  One issue I ran into was dealing with encrypted wallets. Ideally, we would detect whether a wallet is encrypted, and prompt the user for their passphrase at that time. However, that's actually difficult to do in the GUI since migration will unload the wallet if it was already loaded, and reload it without connecting it to any signals or interfaces. Only then can it detect whether a wallet is encrypted, but then there is no `WalletModel` or even an `interfaces::Wallet` that the GUI could use to unlock it via a callback.

  To deal with this, I've opted to just add a button to the migration dialog box that has the user enter their passphrase first, along with instructional text to use that button if their wallet was encrypted. If the user enters the wrong passphrase or clicked the other button that does not prompt for the passphrase, migration will fail with a message indicating that the passphrase was incorrect.

ACKs for top commit:
  hebasto:
    ACK 8f2522d242.
  furszy:
    ACK 8f2522d

Tree-SHA512: a0e3b70dbfcacb89617956510ebcea94cad8617a987c68fe39fa16ac1721190b7cf7afc156c39b9032920cfb67b5d4ca28791681f5021d92d16acc691387afa1
This commit is contained in:
Hennadii Stepanov 2024-08-14 13:56:49 +01:00
commit e682e7db7e
No known key found for this signature in database
GPG key ID: 410108112E7EA81F
15 changed files with 129 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/interfaces/wallet.h
View file

@ -342,8 +342,11 @@ public:
//! Migrate a wallet
virtual util::Result<WalletMigrationResult> migrateWallet(const std::string& name, const SecureString& passphrase) = 0;
//! Returns true if wallet stores encryption keys
virtual bool isEncrypted(const std::string& wallet_name) = 0;
//! Return available wallets in wallet directory.
virtual std::vector<std::string> listWalletDir() = 0;
virtual std::vector<std::pair<std::string, std::string>> listWalletDir() = 0;
//! Return interfaces for accessing wallets (if any).
virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0;

8
src/qt/askpassphrasedialog.cpp
View file

@ -44,6 +44,7 @@ AskPassphraseDialog::AskPassphraseDialog(Mode _mode, QWidget *parent, SecureStri
ui->passEdit1->hide();
setWindowTitle(tr("Encrypt wallet"));
break;
case UnlockMigration:
case Unlock: // Ask passphrase
ui->warningLabel->setText(tr("This operation needs your wallet passphrase to unlock the wallet."));
ui->passLabel2->hide();
@ -80,7 +81,7 @@ void AskPassphraseDialog::setModel(WalletModel *_model)
void AskPassphraseDialog::accept()
{
SecureString oldpass, newpass1, newpass2;
if (!model && mode != Encrypt)
if (!model && mode != Encrypt && mode != UnlockMigration)
return;
oldpass.reserve(MAX_PASSPHRASE_SIZE);
newpass1.reserve(MAX_PASSPHRASE_SIZE);
@ -181,6 +182,10 @@ void AskPassphraseDialog::accept()
QMessageBox::critical(this, tr("Wallet unlock failed"), e.what());
}
break;
case UnlockMigration:
Assume(m_passphrase_out)->assign(oldpass);
QDialog::accept();
break;
case ChangePass:
if(newpass1 == newpass2)
{
@ -224,6 +229,7 @@ void AskPassphraseDialog::textChanged()
case Encrypt: // New passphrase x2
acceptable = !ui->passEdit2->text().isEmpty() && !ui->passEdit3->text().isEmpty();
break;
case UnlockMigration:
case Unlock: // Old passphrase x1
acceptable = !ui->passEdit1->text().isEmpty();
break;

1
src/qt/askpassphrasedialog.h
View file

@ -26,6 +26,7 @@ public:
Encrypt, /**< Ask passphrase twice and encrypt */
Unlock, /**< Ask passphrase and unlock */
ChangePass, /**< Ask old passphrase + new passphrase twice */
UnlockMigration, /**< Ask passphrase for unlocking during migration */
};
explicit AskPassphraseDialog(Mode mode, QWidget *parent, SecureString* passphrase_out = nullptr);

42
src/qt/bitcoingui.cpp
View file

@ -360,6 +360,7 @@ void BitcoinGUI::createActions()
m_migrate_wallet_action = new QAction(tr("Migrate Wallet"), this);
m_migrate_wallet_action->setEnabled(false);
m_migrate_wallet_action->setStatusTip(tr("Migrate a wallet"));
m_migrate_wallet_menu = new QMenu(this);
showHelpMessageAction = new QAction(tr("&Command-line options"), this);
showHelpMessageAction->setMenuRole(QAction::NoRole);
@ -396,15 +397,15 @@ void BitcoinGUI::createActions()
connect(openAction, &QAction::triggered, this, &BitcoinGUI::openClicked);
connect(m_open_wallet_menu, &QMenu::aboutToShow, [this] {
m_open_wallet_menu->clear();
for (const std::pair<const std::string, bool>& i : m_wallet_controller->listWalletDir()) {
const std::string& path = i.first;
QString name = path.empty() ? QString("["+tr("default wallet")+"]") : QString::fromStdString(path);
for (const auto& [path, info] : m_wallet_controller->listWalletDir()) {
const auto& [loaded, _] = info;
QString name = GUIUtil::WalletDisplayName(path);
// An single ampersand in the menu item's text sets a shortcut for this item.
// Single & are shown when && is in the string. So replace & with &&.
name.replace(QChar('&'), QString("&&"));
QAction* action = m_open_wallet_menu->addAction(name);
if (i.second) {
if (loaded) {
// This wallet is already loaded
action->setEnabled(false);
continue;
@ -455,10 +456,31 @@ void BitcoinGUI::createActions()
connect(m_close_all_wallets_action, &QAction::triggered, [this] {
m_wallet_controller->closeAllWallets(this);
});
connect(m_migrate_wallet_action, &QAction::triggered, [this] {
auto activity = new MigrateWalletActivity(m_wallet_controller, this);
connect(activity, &MigrateWalletActivity::migrated, this, &BitcoinGUI::setCurrentWallet);
activity->migrate(walletFrame->currentWalletModel());
connect(m_migrate_wallet_menu, &QMenu::aboutToShow, [this] {
m_migrate_wallet_menu->clear();
for (const auto& [wallet_name, info] : m_wallet_controller->listWalletDir()) {
const auto& [loaded, format] = info;
if (format != "bdb") { // Skip already migrated wallets
continue;
}
QString name = GUIUtil::WalletDisplayName(wallet_name);
// An single ampersand in the menu item's text sets a shortcut for this item.
// Single & are shown when && is in the string. So replace & with &&.
name.replace(QChar('&'), QString("&&"));
QAction* action = m_migrate_wallet_menu->addAction(name);
connect(action, &QAction::triggered, [this, wallet_name] {
auto activity = new MigrateWalletActivity(m_wallet_controller, this);
connect(activity, &MigrateWalletActivity::migrated, this, &BitcoinGUI::setCurrentWallet);
activity->migrate(wallet_name);
});
}
if (m_migrate_wallet_menu->isEmpty()) {
QAction* action = m_migrate_wallet_menu->addAction(tr("No wallets available"));
action->setEnabled(false);
}
});
connect(m_mask_values_action, &QAction::toggled, this, &BitcoinGUI::setPrivacy);
connect(m_mask_values_action, &QAction::toggled, this, &BitcoinGUI::enableHistoryAction);
@ -691,6 +713,8 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller, bool s
m_open_wallet_action->setEnabled(true);
m_open_wallet_action->setMenu(m_open_wallet_menu);
m_restore_wallet_action->setEnabled(true);
m_migrate_wallet_action->setEnabled(true);
m_migrate_wallet_action->setMenu(m_migrate_wallet_menu);
GUIUtil::ExceptionSafeConnect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet);
connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet);
@ -771,7 +795,6 @@ void BitcoinGUI::setCurrentWallet(WalletModel* wallet_model)
}
}
updateWindowTitle();
m_migrate_wallet_action->setEnabled(wallet_model->wallet().isLegacy());
}
void BitcoinGUI::setCurrentWalletBySelectorIndex(int index)
@ -805,7 +828,6 @@ void BitcoinGUI::setWalletActionsEnabled(bool enabled)
openAction->setEnabled(enabled);
m_close_wallet_action->setEnabled(enabled);
m_close_all_wallets_action->setEnabled(enabled);
m_migrate_wallet_action->setEnabled(enabled);
}
void BitcoinGUI::createTrayIcon()

9
src/qt/guiutil.cpp
View file

@ -1008,4 +1008,13 @@ void ShowModalDialogAsynchronously(QDialog* dialog)
dialog->show();
}
QString WalletDisplayName(const QString& name)
{
return name.isEmpty() ? "[" + QObject::tr("default wallet") + "]" : name;
}
QString WalletDisplayName(const std::string& name)
{
return WalletDisplayName(QString::fromStdString(name));
}
} // namespace GUIUtil

3
src/qt/guiutil.h
View file

@ -436,6 +436,9 @@ namespace GUIUtil
return false;
}
QString WalletDisplayName(const std::string& name);
QString WalletDisplayName(const QString& name);
} // namespace GUIUtil
#endif // BITCOIN_QT_GUIUTIL_H

41
src/qt/walletcontroller.cpp
View file

@ -65,16 +65,16 @@ WalletController::~WalletController()
delete m_activity_worker;
}
std::map<std::string, bool> WalletController::listWalletDir() const
std::map<std::string, std::pair<bool, std::string>> WalletController::listWalletDir() const
{
QMutexLocker locker(&m_mutex);
std::map<std::string, bool> wallets;
for (const std::string& name : m_node.walletLoader().listWalletDir()) {
wallets[name] = false;
std::map<std::string, std::pair<bool, std::string>> wallets;
for (const auto& [name, format] : m_node.walletLoader().listWalletDir()) {
wallets[name] = std::make_pair(false, format);
}
for (WalletModel* wallet_model : m_wallets) {
auto it = wallets.find(wallet_model->wallet().getWalletName());
if (it != wallets.end()) it->second = true;
if (it != wallets.end()) it->second.first = true;
}
return wallets;
}
@ -150,9 +150,10 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
assert(called);
connect(wallet_model, &WalletModel::unload, this, [this, wallet_model] {
// Defer removeAndDeleteWallet when no modal widget is active.
// Defer removeAndDeleteWallet when no modal widget is actively waiting for an action.
// TODO: remove this workaround by removing usage of QDialog::exec.
if (QApplication::activeModalWidget()) {
QWidget* active_dialog = QApplication::activeModalWidget();
if (active_dialog && dynamic_cast<QProgressDialog*>(active_dialog) == nullptr) {
connect(qApp, &QApplication::focusWindowChanged, wallet_model, [this, wallet_model]() {
if (!QApplication::activeModalWidget()) {
removeAndDeleteWallet(wallet_model);
@ -343,7 +344,7 @@ void OpenWalletActivity::finish()
void OpenWalletActivity::open(const std::string& path)
{
QString name = path.empty() ? QString("["+tr("default wallet")+"]") : QString::fromStdString(path);
QString name = GUIUtil::WalletDisplayName(path);
showProgressDialog(
//: Title of window indicating the progress of opening of a wallet.
@ -436,12 +437,12 @@ void RestoreWalletActivity::finish()
Q_EMIT finished();
}
void MigrateWalletActivity::migrate(WalletModel* wallet_model)
void MigrateWalletActivity::migrate(const std::string& name)
{
// Warn the user about migration
QMessageBox box(m_parent_widget);
box.setWindowTitle(tr("Migrate wallet"));
box.setText(tr("Are you sure you wish to migrate the wallet <i>%1</i>?").arg(GUIUtil::HtmlEscape(wallet_model->getDisplayName())));
box.setText(tr("Are you sure you wish to migrate the wallet <i>%1</i>?").arg(GUIUtil::HtmlEscape(GUIUtil::WalletDisplayName(name))));
box.setInformativeText(tr("Migrating the wallet will convert this wallet to one or more descriptor wallets. A new wallet backup will need to be made.\n"
"If this wallet contains any watchonly scripts, a new wallet will be created which contains those watchonly scripts.\n"
"If this wallet contains any solvable but not watched scripts, a different and new wallet will be created which contains those scripts.\n\n"
@ -452,31 +453,25 @@ void MigrateWalletActivity::migrate(WalletModel* wallet_model)
box.setDefaultButton(QMessageBox::Yes);
if (box.exec() != QMessageBox::Yes) return;
// Get the passphrase if it is encrypted regardless of it is locked or unlocked. We need the passphrase itself.
SecureString passphrase;
WalletModel::EncryptionStatus enc_status = wallet_model->getEncryptionStatus();
if (enc_status == WalletModel::EncryptionStatus::Locked || enc_status == WalletModel::EncryptionStatus::Unlocked) {
AskPassphraseDialog dlg(AskPassphraseDialog::Unlock, m_parent_widget, &passphrase);
dlg.setModel(wallet_model);
dlg.exec();
if (node().walletLoader().isEncrypted(name)) {
// Get the passphrase for the wallet
AskPassphraseDialog dlg(AskPassphraseDialog::UnlockMigration, m_parent_widget, &passphrase);
if (dlg.exec() == QDialog::Rejected) return;
}
// GUI needs to remove the wallet so that it can actually be unloaded by migration
const std::string name = wallet_model->wallet().getWalletName();
m_wallet_controller->removeAndDeleteWallet(wallet_model);
showProgressDialog(tr("Migrate Wallet"), tr("Migrating Wallet <b>%1</b>…").arg(GUIUtil::HtmlEscape(name)));
QTimer::singleShot(0, worker(), [this, name, passphrase] {
auto res{node().walletLoader().migrateWallet(name, passphrase)};
if (res) {
m_success_message = tr("The wallet '%1' was migrated successfully.").arg(GUIUtil::HtmlEscape(res->wallet->getWalletName()));
m_success_message = tr("The wallet '%1' was migrated successfully.").arg(GUIUtil::HtmlEscape(GUIUtil::WalletDisplayName(res->wallet->getWalletName())));
if (res->watchonly_wallet_name) {
m_success_message += QChar(' ') + tr("Watchonly scripts have been migrated to a new wallet named '%1'.").arg(GUIUtil::HtmlEscape(res->watchonly_wallet_name.value()));
m_success_message += QChar(' ') + tr("Watchonly scripts have been migrated to a new wallet named '%1'.").arg(GUIUtil::HtmlEscape(GUIUtil::WalletDisplayName(res->watchonly_wallet_name.value())));
}
if (res->solvables_wallet_name) {
m_success_message += QChar(' ') + tr("Solvable but not watched scripts have been migrated to a new wallet named '%1'.").arg(GUIUtil::HtmlEscape(res->solvables_wallet_name.value()));
m_success_message += QChar(' ') + tr("Solvable but not watched scripts have been migrated to a new wallet named '%1'.").arg(GUIUtil::HtmlEscape(GUIUtil::WalletDisplayName(res->solvables_wallet_name.value())));
}
m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(res->wallet));
} else {

6
src/qt/walletcontroller.h
View file

@ -61,13 +61,11 @@ public:
//! Returns all wallet names in the wallet dir mapped to whether the wallet
//! is loaded.
std::map<std::string, bool> listWalletDir() const;
std::map<std::string, std::pair<bool, std::string>> listWalletDir() const;
void closeWallet(WalletModel* wallet_model, QWidget* parent = nullptr);
void closeAllWallets(QWidget* parent = nullptr);
void migrateWallet(WalletModel* wallet_model, QWidget* parent = nullptr);
Q_SIGNALS:
void walletAdded(WalletModel* wallet_model);
void walletRemoved(WalletModel* wallet_model);
@ -186,7 +184,7 @@ class MigrateWalletActivity : public WalletControllerActivity
public:
MigrateWalletActivity(WalletController* wallet_controller, QWidget* parent) : WalletControllerActivity(wallet_controller, parent) {}
void migrate(WalletModel* wallet_model);
void migrate(const std::string& path);
Q_SIGNALS:
void migrated(WalletModel* wallet_model);

3
src/qt/walletmodel.cpp
View file

@ -594,8 +594,7 @@ QString WalletModel::getWalletName() const
QString WalletModel::getDisplayName() const
{
const QString name = getWalletName();
return name.isEmpty() ? "["+tr("default wallet")+"]" : name;
return GUIUtil::WalletDisplayName(getWalletName());
}
bool WalletModel::isMultiwallet() const

26
src/wallet/db.cpp
View file

@ -19,9 +19,9 @@ namespace wallet {
bool operator<(BytePrefix a, Span<const std::byte> b) { return a.prefix < b.subspan(0, std::min(a.prefix.size(), b.size())); }
bool operator<(Span<const std::byte> a, BytePrefix b) { return a.subspan(0, std::min(a.size(), b.prefix.size())) < b.prefix; }
std::vector<fs::path> ListDatabases(const fs::path& wallet_dir)
std::vector<std::pair<fs::path, std::string>> ListDatabases(const fs::path& wallet_dir)
{
std::vector<fs::path> paths;
std::vector<std::pair<fs::path, std::string>> paths;
std::error_code ec;
for (auto it = fs::recursive_directory_iterator(wallet_dir, ec); it != fs::recursive_directory_iterator(); it.increment(ec)) {
@ -38,21 +38,29 @@ std::vector<fs::path> ListDatabases(const fs::path& wallet_dir)
try {
const fs::path path{it->path().lexically_relative(wallet_dir)};
if (it->status().type() == fs::file_type::directory &&
(IsBDBFile(BDBDataFile(it->path())) || IsSQLiteFile(SQLiteDataFile(it->path())))) {
// Found a directory which contains wallet.dat btree file, add it as a wallet.
paths.emplace_back(path);
if (it->status().type() == fs::file_type::directory) {
if (IsBDBFile(BDBDataFile(it->path()))) {
// Found a directory which contains wallet.dat btree file, add it as a wallet with BERKELEY format.
paths.emplace_back(path, "bdb");
} else if (IsSQLiteFile(SQLiteDataFile(it->path()))) {
// Found a directory which contains wallet.dat sqlite file, add it as a wallet with SQLITE format.
paths.emplace_back(path, "sqlite");
}
} else if (it.depth() == 0 && it->symlink_status().type() == fs::file_type::regular && it->path().extension() != ".bak") {
if (it->path().filename() == "wallet.dat") {
// Found top-level wallet.dat btree file, add top level directory ""
// Found top-level wallet.dat file, add top level directory ""
// as a wallet.
paths.emplace_back();
if (IsBDBFile(it->path())) {
paths.emplace_back(fs::path(), "bdb");
} else if (IsSQLiteFile(it->path())) {
paths.emplace_back(fs::path(), "sqlite");
}
} else if (IsBDBFile(it->path())) {
// Found top-level btree file not called wallet.dat. Current bitcoin
// software will never create these files but will allow them to be
// opened in a shared database environment for backwards compatibility.
// Add it to the list of available wallets.
paths.emplace_back(path);
paths.emplace_back(path, "bdb");
}
}
} catch (const std::exception& e) {

2
src/wallet/db.h
View file

@ -216,7 +216,7 @@ enum class DatabaseStatus {
};
/** Recursively list database paths in directory. */
std::vector<fs::path> ListDatabases(const fs::path& path);
std::vector<std::pair<fs::path, std::string>> ListDatabases(const fs::path& path);
void ReadDatabaseArgs(const ArgsManager& args, DatabaseOptions& options);
std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);

23
src/wallet/interfaces.cpp
View file

@ -652,15 +652,30 @@ public:
};
return out;
}
bool isEncrypted(const std::string& wallet_name) override
{
auto wallets{GetWallets(m_context)};
auto it = std::find_if(wallets.begin(), wallets.end(), [&](std::shared_ptr<CWallet> w){ return w->GetName() == wallet_name; });
if (it != wallets.end()) return (*it)->IsCrypted();
// Unloaded wallet, read db
DatabaseOptions options;
options.require_existing = true;
DatabaseStatus status;
bilingual_str error;
auto db = MakeWalletDatabase(wallet_name, options, status, error);
if (!db) return false;
return WalletBatch(*db).IsEncrypted();
}
std::string getWalletDir() override
{
return fs::PathToString(GetWalletDir());
}
std::vector<std::string> listWalletDir() override
std::vector<std::pair<std::string, std::string>> listWalletDir() override
{
std::vector<std::string> paths;
for (auto& path : ListDatabases(GetWalletDir())) {
paths.push_back(fs::PathToString(path));
std::vector<std::pair<std::string, std::string>> paths;
for (auto& [path, format] : ListDatabases(GetWalletDir())) {
paths.emplace_back(fs::PathToString(path), format);
}
return paths;
}

2
src/wallet/rpc/wallet.cpp
View file

@ -169,7 +169,7 @@ static RPCHelpMan listwalletdir()
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
UniValue wallets(UniValue::VARR);
for (const auto& path : ListDatabases(GetWalletDir())) {
for (const auto& [path, _] : ListDatabases(GetWalletDir())) {
UniValue wallet(UniValue::VOBJ);
wallet.pushKV("name", path.utf8string());
wallets.push_back(std::move(wallet));

11
src/wallet/walletdb.cpp
View file

@ -187,6 +187,17 @@ bool WalletBatch::ReadBestBlock(CBlockLocator& locator)
return m_batch->Read(DBKeys::BESTBLOCK_NOMERKLE, locator);
}
bool WalletBatch::IsEncrypted()
{
DataStream prefix;
prefix << DBKeys::MASTER_KEY;
if (auto cursor = m_batch->GetNewPrefixCursor(prefix)) {
DataStream k, v;
if (cursor->Next(k, v) == DatabaseCursor::Status::MORE) return true;
}
return false;
}
bool WalletBatch::WriteOrderPosNext(int64_t nOrderPosNext)
{
return WriteIC(DBKeys::ORDERPOSNEXT, nOrderPosNext);

3
src/wallet/walletdb.h
View file

@ -247,6 +247,9 @@ public:
bool WriteBestBlock(const CBlockLocator& locator);
bool ReadBestBlock(CBlockLocator& locator);
// Returns true if wallet stores encryption keys
bool IsEncrypted();
bool WriteOrderPosNext(int64_t nOrderPosNext);
bool ReadPool(int64_t nPool, CKeyPool& keypool);