Menu action to export a watchonly wallet

[achow101]

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

Built on bitcoin/bitcoin#32489

[DrahtBot]

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
Concept ACK	Sjors

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

[pablomartin4btc]

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 cdd15e1 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]

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.

[Sjors]

Concept ACK. CI is unhappy.

[polespinasa]

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.

[polespinasa]

lgtm ACK 6573196

[pablomartin4btc]

ACK 6573196