Skip to content
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
13 changes: 13 additions & 0 deletions src/interfaces/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ struct WalletBalances;
struct WalletTx;
struct WalletTxOut;
struct WalletTxStatus;
struct WalletMigrationResult;

using WalletOrderForm = std::vector<std::pair<std::string, std::string>>;
using WalletValueMap = std::map<std::string, std::string>;
Expand Down Expand Up @@ -332,6 +333,9 @@ class WalletLoader : public ChainClient
//! Restore backup wallet
virtual util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0;

//! Migrate a wallet
virtual util::Result<WalletMigrationResult> migrateWallet(const std::string& name, const SecureString& passphrase) = 0;

//! Return available wallets in wallet directory.
virtual std::vector<std::string> listWalletDir() = 0;

Expand Down Expand Up @@ -423,6 +427,15 @@ struct WalletTxOut
bool is_spent = false;
};

//! Migrated wallet info
struct WalletMigrationResult
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we seem to use this style now, although it's not officially documented, so only nit

diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index 54eb720d0..d6a92cba7 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -428,8 +428,7 @@ struct WalletTxOut
 };
 
 //! Migrated wallet info
-struct WalletMigrationResult
-{
+struct WalletMigrationResult {
     std::unique_ptr<Wallet> wallet;
     std::optional<std::string> watchonly_wallet_name;
     std::optional<std::string> solvables_wallet_name;

std::unique_ptr<Wallet> wallet;
std::optional<std::string> watchonly_wallet_name;
std::optional<std::string> solvables_wallet_name;
fs::path backup_path;
};

//! Return implementation of Wallet interface. This function is defined in
//! dummywallet.cpp and throws if the wallet component is not compiled.
std::unique_ptr<Wallet> MakeWallet(wallet::WalletContext& context, const std::shared_ptr<wallet::CWallet>& wallet);
Expand Down
3 changes: 3 additions & 0 deletions src/qt/askpassphrasedialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ void AskPassphraseDialog::accept()
"passphrase to avoid this issue in the future."));
}
} else {
if (m_passphrase_out) {
m_passphrase_out->assign(oldpass);
}
Comment on lines +170 to +172
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
if (m_passphrase_out) {
m_passphrase_out->assign(oldpass);
}
if (m_passphrase_out) m_passphrase_out->assign(oldpass);

QDialog::accept(); // Success
}
} catch (const std::runtime_error& e) {
Expand Down
12 changes: 12 additions & 0 deletions src/qt/bitcoingui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,10 @@ void BitcoinGUI::createActions()
m_close_all_wallets_action = new QAction(tr("Close All Wallets…"), this);
m_close_all_wallets_action->setStatusTip(tr("Close all wallets"));

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

showHelpMessageAction = new QAction(tr("&Command-line options"), this);
showHelpMessageAction->setMenuRole(QAction::NoRole);
showHelpMessageAction->setStatusTip(tr("Show the %1 help message to get a list with possible Bitcoin command-line options").arg(PACKAGE_NAME));
Expand Down Expand Up @@ -456,6 +460,11 @@ 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_mask_values_action, &QAction::toggled, this, &BitcoinGUI::setPrivacy);
connect(m_mask_values_action, &QAction::toggled, this, &BitcoinGUI::enableHistoryAction);
}
Expand Down Expand Up @@ -483,6 +492,7 @@ void BitcoinGUI::createMenuBar()
file->addAction(m_open_wallet_action);
file->addAction(m_close_wallet_action);
file->addAction(m_close_all_wallets_action);
file->addAction(m_migrate_wallet_action);
file->addSeparator();
file->addAction(backupWalletAction);
file->addAction(m_restore_wallet_action);
Expand Down Expand Up @@ -767,6 +777,7 @@ 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 @@ -800,6 +811,7 @@ 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
2 changes: 2 additions & 0 deletions src/qt/bitcoingui.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ class BitcoinGUI : public QMainWindow
QAction* m_wallet_selector_label_action = nullptr;
QAction* m_wallet_selector_action = nullptr;
QAction* m_mask_values_action{nullptr};
QAction* m_migrate_wallet_action{nullptr};
QMenu* m_migrate_wallet_menu{nullptr};

QLabel *m_wallet_selector_label = nullptr;
QComboBox* m_wallet_selector = nullptr;
Expand Down
64 changes: 64 additions & 0 deletions src/qt/walletcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,3 +432,67 @@ void RestoreWalletActivity::finish()

Q_EMIT finished();
}

void MigrateWalletActivity::migrate(WalletModel* wallet_model)
{
// 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.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"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmed that all the new strings introduced here are nicely transferred over to be translated after running make translate

While not a blocker; the longer I look at this, the more I think it can be slimmed down. Will have another look tomorrow.

At least this is a chunky boy to translate, and will need a good translator comment:

<message>
        <location line="+1"/>
        <source>Migrating the wallet will convert this wallet to one or more descriptor wallets. A new wallet backup will need to be made.
If this wallet contains any watchonly scripts, a new wallet will be created which contains those watchonly scripts.
If this wallet contains any solvable but not watched scripts, a different and new wallet will be created which contains those scripts.

The migration process will create a backup of the wallet before migrating. This backup file will be named &lt;wallet name&gt;-&lt;timestamp&gt;.legacy.bak and can be found in the directory for this wallet. In the event of an incorrect migration, the backup can be restored with the &quot;Restore Wallet&quot; functionality.</source>
        <translation type="unfinished"></translation>
</message>

"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"
"The migration process will create a backup of the wallet before migrating. This backup file will be named "
"<wallet name>-<timestamp>.legacy.bak and can be found in the directory for this wallet. In the event of "
"an incorrect migration, the backup can be restored with the \"Restore Wallet\" functionality."));
box.setStandardButtons(QMessageBox::Yes|QMessageBox::Cancel);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
box.setStandardButtons(QMessageBox::Yes|QMessageBox::Cancel);
box.setStandardButtons(QMessageBox::Yes | QMessageBox::Cancel);

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();
Comment thread
achow101 marked this conversation as resolved.
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();
}

// 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()));
if (res->watchonly_wallet_name) {
m_success_message += tr(" Watchonly scripts have been migrated to a new wallet named '%1'.").arg(GUIUtil::HtmlEscape(res->watchonly_wallet_name.value()));
}
if (res->solvables_wallet_name) {
m_success_message += tr(" Solvable but not watched scripts have been migrated to a new wallet named '%1'.").arg(GUIUtil::HtmlEscape(res->solvables_wallet_name.value()));
}
Comment on lines +471 to +477
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps we can refactor this piece a bit, maybe extracting the message building to other functions?

m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(res->wallet));
} else {
m_error_message = util::ErrorString(res);
}

QTimer::singleShot(0, this, &MigrateWalletActivity::finish);
});
}

void MigrateWalletActivity::finish()
{
if (!m_error_message.empty()) {
QMessageBox::critical(m_parent_widget, tr("Migration failed"), QString::fromStdString(m_error_message.translated));
} else {
QMessageBox::information(m_parent_widget, tr("Migration Successful"), m_success_message);
}

if (m_wallet_model) Q_EMIT migrated(m_wallet_model);

Q_EMIT finished();
}
22 changes: 22 additions & 0 deletions src/qt/walletcontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class path;
class AskPassphraseDialog;
class CreateWalletActivity;
class CreateWalletDialog;
class MigrateWalletActivity;
class OpenWalletActivity;
class WalletControllerActivity;

Expand All @@ -65,6 +66,8 @@ class WalletController : public QObject
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 All @@ -83,6 +86,7 @@ class WalletController : public QObject
std::unique_ptr<interfaces::Handler> m_handler_load_wallet;

friend class WalletControllerActivity;
friend class MigrateWalletActivity;
};

class WalletControllerActivity : public QObject
Expand Down Expand Up @@ -175,4 +179,22 @@ class RestoreWalletActivity : public WalletControllerActivity
void finish();
};

class MigrateWalletActivity : public WalletControllerActivity
{
Q_OBJECT

public:
MigrateWalletActivity(WalletController* wallet_controller, QWidget* parent) : WalletControllerActivity(wallet_controller, parent) {}

void migrate(WalletModel* wallet_model);

Q_SIGNALS:
void migrated(WalletModel* wallet_model);

private:
QString m_success_message;

void finish();
};

#endif // BITCOIN_QT_WALLETCONTROLLER_H
13 changes: 13 additions & 0 deletions src/wallet/interfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ using interfaces::Wallet;
using interfaces::WalletAddress;
using interfaces::WalletBalances;
using interfaces::WalletLoader;
using interfaces::WalletMigrationResult;
using interfaces::WalletOrderForm;
using interfaces::WalletTx;
using interfaces::WalletTxOut;
Expand Down Expand Up @@ -631,6 +632,18 @@ class WalletLoaderImpl : public WalletLoader
return util::Error{error};
}
}
util::Result<WalletMigrationResult> migrateWallet(const std::string& name, const SecureString& passphrase) override
{
auto res = wallet::MigrateLegacyToDescriptor(name, passphrase, m_context);
if (!res) return util::Error{util::ErrorString(res)};
WalletMigrationResult out{
.wallet = MakeWallet(m_context, res->wallet),
.watchonly_wallet_name = res->watchonly_wallet ? std::make_optional(res->watchonly_wallet->GetName()) : std::nullopt,
.solvables_wallet_name = res->solvables_wallet ? std::make_optional(res->solvables_wallet->GetName()) : std::nullopt,
.backup_path = res->backup_path,
};
return {std::move(out)}; // std::move to work around clang bug
}
std::string getWalletDir() override
{
return fs::PathToString(GetWalletDir());
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4253,7 +4253,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
// Migration successful, unload the wallet locally, then reload it.
assert(local_wallet.use_count() == 1);
local_wallet.reset();
LoadWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error, warnings);
res.wallet = LoadWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error, warnings);
res.wallet_name = wallet_name;
} else {
// Migration failed, cleanup
Expand Down
1 change: 1 addition & 0 deletions src/wallet/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,7 @@ bool FillInputToWeight(CTxIn& txin, int64_t target_weight);

struct MigrationResult {
std::string wallet_name;
std::shared_ptr<CWallet> wallet;
std::shared_ptr<CWallet> watchonly_wallet;
std::shared_ptr<CWallet> solvables_wallet;
fs::path backup_path;
Expand Down