Skip to content

Commit

Permalink
Merge bitcoin-core/gui#824: Migrate legacy wallets that are not loaded
Browse files Browse the repository at this point in the history 
8f2522d gui: Use menu for wallet migration (Ava Chow)
d56a450 gui: Use wallet name for wallet migration rather than WalletModel (Ava Chow)
c391858 gui: don't remove wallet manually before migration (furszy)
bfba638 gui: Consolidate wallet display name to GUIUtil function (Ava Chow)
28fc562 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 8f2522d.
  furszy:
    ACK 8f2522d

Tree-SHA512: a0e3b70dbfcacb89617956510ebcea94cad8617a987c68fe39fa16ac1721190b7cf7afc156c39b9032920cfb67b5d4ca28791681f5021d92d16acc691387afa1
  • Loading branch information
@hebasto
hebasto committed Aug 14, 2024
2 parents 1a41e63 + 8f2522d commit e682e7d
Show file tree
Hide file tree
Showing 15 changed files with 129 additions and 56 deletions.
5 changes: 4 additions & 1 deletion src/interfaces/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,11 @@ class WalletLoader : public ChainClient
//! 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;
Expand Down
8 changes: 7 additions & 1 deletion src/qt/askpassphrasedialog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/qt/askpassphrasedialog.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class AskPassphraseDialog : public QDialog
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);
Expand Down
42 changes: 32 additions & 10 deletions src/qt/bitcoingui.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
9 changes: 9 additions & 0 deletions src/qt/guiutil.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 3 additions & 0 deletions src/qt/guiutil.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 18 additions & 23 deletions src/qt/walletcontroller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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"
Expand All @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions src/qt/walletcontroller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,11 @@ class WalletController : public QObject

//! 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);
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/qt/walletmodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 17 additions & 9 deletions src/wallet/db.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/db.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
23 changes: 19 additions & 4 deletions src/wallet/interfaces.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,15 +652,30 @@ class WalletLoaderImpl : public WalletLoader
};
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;
}
Expand Down
Loading

0 comments on commit e682e7d

Please sign in to comment.