Menu action to export a watchonly wallet #872

achow101 commented at 11:31 PM on May 13, 2025: member

Allows a user to export a watchonly version of their wallet to be used in an airgapped setup.

Built on https://github.com/bitcoin/bitcoin/pull/32489

DrahtBot commented at 11:31 PM on May 13, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	polespinasa, pablomartin4btc, hebasto
Concept ACK	Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

DrahtBot added the label CI failed on May 14, 2025

DrahtBot added the label Needs rebase on May 16, 2025

achow101 force-pushed on May 16, 2025

DrahtBot removed the label Needs rebase on May 16, 2025

DrahtBot removed the label CI failed on May 17, 2025

DrahtBot added the label Needs rebase on May 19, 2025

achow101 force-pushed on May 20, 2025

DrahtBot removed the label Needs rebase on May 20, 2025

hebasto renamed this:
~~gui: Menu action to export a watchonly wallet~~
Menu action to export a watchonly wallet
on May 20, 2025

DrahtBot added the label Needs rebase on May 21, 2025

achow101 force-pushed on May 21, 2025

DrahtBot removed the label Needs rebase on May 21, 2025

DrahtBot added the label CI failed on May 21, 2025

achow101 force-pushed on Jun 2, 2025

DrahtBot removed the label CI failed on Jun 2, 2025

achow101 force-pushed on Jun 4, 2025

DrahtBot added the label Needs rebase on Jul 1, 2025

achow101 force-pushed on Sep 11, 2025

DrahtBot removed the label Needs rebase on Sep 11, 2025

DrahtBot added the label Needs rebase on Dec 17, 2025

achow101 force-pushed on Apr 28, 2026

DrahtBot removed the label Needs rebase on Apr 28, 2026

DrahtBot added the label CI failed on Apr 28, 2026

DrahtBot commented at 8:12 PM on April 28, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task tidy: https://github.com/bitcoin-core/gui/actions/runs/25068575825/job/73442374300</sub> <sub>LLM reason (✨ experimental): CI failed because clang-tidy reported an bugprone-unused-return-value warning in qt/bitcoingui.cpp (neglecting a returned value), which was treated as an error.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

achow101 force-pushed on Apr 29, 2026

DrahtBot removed the label CI failed on Apr 29, 2026

DrahtBot added the label Needs rebase on May 14, 2026

achow101 force-pushed on May 26, 2026

DrahtBot removed the label Needs rebase on May 26, 2026

DrahtBot added the label CI failed on May 26, 2026

DrahtBot added the label Needs rebase on Jun 14, 2026

pablomartin4btc commented at 10:23 PM on July 4, 2026: contributor

The base PR (bitcoin/bitcoin#32489) has been merged, this will need a rebase.

Reviewed the GUI commit (59fd6f9):

The action is enabled for all wallets. Please consider disabling it for already-watchonly wallets — here cdd15e1775ba316f1a2e3a44921fc7c6fbabd137 you have a Qt test covering the action verifying it's enabled/disabled correctly per wallet type.
Missing release notes for a new user-facing menu action.
Please consider the comment from @Sjors regarding CHECK_NONFATAL (if it's still relevant).

achow101 force-pushed on Jul 27, 2026

gui: Add ExceptionSafeConnect that takes a lambda

There is a variant of QObject::connect which takes 3 arguments. Add a
variant of ExceptioNSafeConnect that does the same thing.

5907a5c7dc

DrahtBot removed the label Needs rebase on Jul 27, 2026

achow101 force-pushed on Jul 27, 2026

achow101 commented at 8:33 PM on July 27, 2026: member

Rebased.

The action is enabled for all wallets. Please consider disabling it for already-watchonly wallets

Done

here cdd15e1 you have a Qt test covering the action verifying it's enabled/disabled correctly per wallet type.

This test is not useful, it isn't testing the action directly. I don't think it is possible with the current GUI tests to properly test this behavior as there is no access to the BitcoinGUI actions.

Missing release notes for a new user-facing menu action.

Done

Please consider the comment from @Sjors regarding CHECK_NONFATAL (if it's still relevant).

I think the GUI should add global exception handling, but I've changed this to add another GUIUtil::ExceptionSafeConnect that can catch those exceptions.

achow101 marked this as ready for review on Jul 27, 2026

Sjors commented at 8:32 AM on July 28, 2026: member

Concept ACK. CI is unhappy.

polespinasa commented at 9:37 AM on July 28, 2026: member

Concept ACK

CI is unhappy.

I think that is because of the declaration outside the #ifdef ENABLE_WALLET block.

In the header (bitcoingui.h) is declared inside the #ifdef ENABLE_WALLET:

    QAction* m_migrate_wallet_action{nullptr};
    QMenu* m_migrate_wallet_menu{nullptr};
#ifdef ENABLE_WALLET
    QLabel *m_wallet_selector_label = nullptr;
    QComboBox* m_wallet_selector = nullptr;
    QAction* m_export_watchonly_action{nullptr};
#endif // ENABLE_WALLET
    QSystemTrayIcon* trayIcon = nullptr;
    const std::unique_ptr<QMenu> trayIconMenu;

But the new QAction declaration is outside, I think this should fix it:

$ git diff
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index 690ad86390..da8989a564 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -377,10 +377,6 @@ void BitcoinGUI::createActions()
     m_mask_values_action->setStatusTip(tr("Mask the values in the Overview tab"));
     m_mask_values_action->setCheckable(true);
 
-    m_export_watchonly_action = new QAction(tr("Export watch-only wallet"), this);
-    m_export_watchonly_action->setEnabled(false);
-    m_export_watchonly_action->setStatusTip(tr("Export a watch-only version of the current wallet that can be restored onto another node."));
-
     connect(quitAction, &QAction::triggered, this, &BitcoinGUI::quitRequested);
     connect(aboutAction, &QAction::triggered, this, &BitcoinGUI::aboutClicked);
     connect(aboutQtAction, &QAction::triggered, qApp, QApplication::aboutQt);
@@ -393,6 +389,10 @@ void BitcoinGUI::createActions()
 #ifdef ENABLE_WALLET
     if(walletFrame)
     {
+        m_export_watchonly_action = new QAction(tr("Export watch-only wallet"), this);
+        m_export_watchonly_action->setEnabled(false);
+        m_export_watchonly_action->setStatusTip(tr("Export a watch-only version of the current wallet that can be restored onto another node."));
+
         connect(encryptWalletAction, &QAction::triggered, walletFrame, &WalletFrame::encryptWallet);
         connect(backupWalletAction, &QAction::triggered, walletFrame, &WalletFrame::backupWallet);
         connect(changePassphraseAction, &QAction::triggered, walletFrame, &WalletFrame::changePassphrase);

Or you can maybe move the declaration under m_migrate_wallet_action declaration.

in src/qt/bitcoingui.cpp:884 in a4f67c85c6

polespinasa commented at 10:07 AM on July 28, 2026:

in a4f67c85c6835724009cb6b6336c93288c324336 gui: Menu action for exporting a watchonly wallet

If you close all wallets, the option to export a watch-only-wallet remains usable, but it should be grey and unusable. Fix:

$ git diff
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index 690ad86390..ad1ecf572d 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -882,6 +882,7 @@ void BitcoinGUI::setWalletActionsEnabled(bool enabled)
     openAction->setEnabled(enabled);
     m_close_wallet_action->setEnabled(enabled);
     m_close_all_wallets_action->setEnabled(enabled);
+    m_export_watchonly_action->setEnabled(enabled);
 }
 
 void BitcoinGUI::createTrayIcon()

achow101 commented at 5:41 PM on July 28, 2026:

Done

in src/wallet/interfaces.cpp:527 in a4f67c85c6 outdated

 522 | @@ -522,6 +523,11 @@ class WalletImpl : public Wallet
 523 |      }
 524 |      CWallet* wallet() override { return m_wallet.get(); }
 525 |  
 526 | +    util::Result<std::string> exportWatchOnlyWallet(const fs::path& destination) override {
 527 | +        LOCK(m_wallet->cs_wallet);

polespinasa commented at 10:10 AM on July 28, 2026:

in a4f67c8 gui: Menu action for exporting a watchonly wallet

Shouldn't here call TopUpKeyPool() like we do in the RPC?¿

achow101 commented at 5:41 PM on July 28, 2026:

Done

in src/qt/bitcoingui.cpp:534 in a4f67c85c6

 527 | @@ -524,6 +528,16 @@ void BitcoinGUI::createActions()
 528 |          });
 529 |          connect(m_mask_values_action, &QAction::toggled, this, &BitcoinGUI::setPrivacy);
 530 |          connect(m_mask_values_action, &QAction::toggled, this, &BitcoinGUI::enableHistoryAction);
 531 | +        GUIUtil::ExceptionSafeConnect(m_export_watchonly_action, &QAction::triggered, [this](bool) {
 532 | +            QString destination = GUIUtil::getSaveFileName(this, tr("Save Watch-only Wallet Export"), QString(), QString(), nullptr);
 533 | +            if (destination.isEmpty()) return;
 534 | +            util::Result<std::string> export_res = walletFrame->currentWalletModel()->wallet().exportWatchOnlyWallet(GUIUtil::QStringToPath(destination));

polespinasa commented at 10:17 AM on July 28, 2026:

in a4f67c8 gui: Menu action for exporting a watchonly wallet

What if walletFrame->currentWalletModel() is nullptr? That causes a segmentation fault:

This can be triggered when no wallet is loaded and you try to export a watchonly wallet. Which should not happen but can happen because of https://github.com/bitcoin-core/gui/pull/872/changes/a4f67c85c6835724009cb6b6336c93288c324336#r3664616220

We could probably also protect this here with:

$ git diff
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index 690ad86390..eee44330dd 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -531,7 +531,9 @@ void BitcoinGUI::createActions()
         GUIUtil::ExceptionSafeConnect(m_export_watchonly_action, &QAction::triggered, [this](bool) {
             QString destination = GUIUtil::getSaveFileName(this, tr("Save Watch-only Wallet Export"), QString(), QString(), nullptr);
             if (destination.isEmpty()) return;
-            util::Result<std::string> export_res = walletFrame->currentWalletModel()->wallet().exportWatchOnlyWallet(GUIUtil::QStringToPath(destination));
+            WalletModel* model = walletFrame->currentWalletModel();
+            if (!model) return;
+            util::Result<std::string> export_res = model->wallet().exportWatchOnlyWallet(GUIUtil::QStringToPath(destination));
             if (export_res) {
                 QMessageBox::information(nullptr, tr("Export Successful"), tr("The wallet has been exported to ") + QString::fromStdString(*export_res));
             } else {

achow101 commented at 5:42 PM on July 28, 2026:

The model should never be nullptr. Added an Assume

gui: Menu action for exporting a watchonly wallet cb51f97f6c

doc: Release note for export watchonly wallet gui action 6573196e63

achow101 force-pushed on Jul 28, 2026

polespinasa commented at 6:11 PM on July 28, 2026: member

lgtm ACK 6573196e63bf4113d332c802f409e3336aebab41

DrahtBot requested review from Sjors on Jul 28, 2026

pablomartin4btc commented at 6:21 PM on July 28, 2026: contributor

ACK 6573196e63bf4113d332c802f409e3336aebab41

hebasto approved

hebasto commented at 2:31 PM on August 3, 2026: member

ACK 6573196e63bf4113d332c802f409e3336aebab41.

hebasto merged this on Aug 3, 2026

hebasto closed this on Aug 3, 2026

fanquake commented at 3:42 PM on August 4, 2026: member

cc https://github.com/bitcoin-core/gui/issues/956