Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate legacy wallets that are not loaded #824

Merged
merged 5 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/interfaces/wallet.h
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
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
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
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);
Comment on lines +474 to +477
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compilation error: error: 'wallet_name' in capture list does not name a variable.

It seems the lambda expression cannot capture the structured binding. A local reference of the variable fixes it.

Same happens on the m_open_wallet_menu action lambda with the "path" variable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What compiler? I'm not seeing this error, and it seems neither did CI.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What compiler? I'm not seeing this error, and it seems neither did CI.

clang 14.0.3 (clang-1403.0.22.14.1).

});
}
if (m_migrate_wallet_menu->isEmpty()) {
QAction* action = m_migrate_wallet_menu->addAction(tr("No wallets available"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe a more descriptive text would be good?
something like No wallets that need to be migrated

for people who don't know about the migration and why/what it is.
seeing No wallets available while they have wallet(s) might seem wrong/confusing.

sry for commenting post-merge

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

Comment on lines -69 to -70
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, this code has been dead since introducing in #738.

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
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
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
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
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
Loading