wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet #32489

achow101 · 2025-05-13T22:22:00Z

Currently, if a user wants to use an airgapped setup, they need to manually create the watchonly wallet that will live on the online node by importing the public descriptors. This PR introduces exportwatchonlywallet which will create a wallet file with the public descriptors to avoid exposing the specific internals to the user. Additionally, this RPC will copy any existing labels, transactions, and wallet flags. This ensures that the exported watchonly wallet is almost entirely a copy of the original wallet but without private keys.

DrahtBot · 2025-05-13T22:22:04Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32489.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	rkrux
Concept ACK	shahsb, Sjors, vicjuma, Eunovo, stringintech, enirox001
Stale ACK	pablomartin4btc, ryanofsky

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33034 (wallet: Store transactions in a separate sqlite table by achow101)
#33008 (wallet: support bip388 policy with external signer by Sjors)
#32895 (wallet: Prepare for future upgrades by recording versions of last client to open and decrypt by achow101)
#32861 (Have createwalletdescriptor auto-detect an unused(KEY) by Sjors)
#32784 (wallet: derivehdkey RPC to get xpub at arbitrary path by Sjors)
#32652 (wallet: add codex32 argument to addhdkey by roconnor-blockstream)
#32471 (wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key by Eunovo)
#29136 (wallet: addhdkey RPC to add just keys to wallets via new unused(KEY) descriptor by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot · 2025-05-13T22:39:22Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/42170822821}
_{LLM reason (✨ experimental): The CI failure is due to linting errors in Python and C++ code.}

Hints

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.

shahsb

Concept ACK

Sjors · 2025-05-19T12:50:11Z

Concept ACK

This might also be useful in a multisig airgapped scenario (where two private keys never go near each other).

E.g. create a 2-of-2 MuSig2 wallet on your offline machine, with one hot key held by Bitcoin Core and the other a hardware wallet. Export the watch-only wallet and put it on the online machine. You can then use the hardware wallet on either the online or offline machine to co-sign.

Though for that to work, the exportwatchonlywallet should have an option to set the external_signer flag (the offline wallet might not have this flag, depending on how you set it up).

jonatack · 2025-08-04T21:23:37Z

For anyone who's interested, I'm hosting a review club meeting about this PR on Wednesday: bitcoincore.reviews/32489

There is Review Club label in this repo that can be added, too.

src/wallet/rpc/wallet.cpp

pablomartin4btc

re-ACK 84d3bc6

Since my last review CanSelfExpand() has been refactored as @ryanofsky suggested.

We'd need release notes for this new RPC.

ryanofsky

Code review ACK 84d3bc6. This is great. Nice feature cleanly implemented with very good test coverage. I left a bunch of comments but none are critical.

src/wallet/wallet.cpp

ryanofsky · 2025-08-07T01:29:03Z

src/wallet/scriptpubkeyman.cpp

@@ -1174,7 +1174,7 @@ bool DescriptorScriptPubKeyMan::CanGetAddresses(bool internal) const
    LOCK(cs_desc_man);
    return m_wallet_descriptor.descriptor->IsSingleType() &&
           m_wallet_descriptor.descriptor->IsRange() &&
-           (HavePrivateKeys() || m_wallet_descriptor.next_index < m_wallet_descriptor.range_end);
+           (HavePrivateKeys() || m_wallet_descriptor.next_index < m_wallet_descriptor.range_end || m_wallet_descriptor.descriptor->CanSelfExpand());


re: #32489 (comment)

In commit "wallet: Use Descriptor::CanSelfExpand() in CanGetAddresses()" (1170ade)

Can confirm without this change the test fails as expected with self.funds.sendtoaddress(online_wallet.getnewaddress(), 10) throwing "No addresses available"

ryanofsky · 2025-08-07T01:32:10Z

src/wallet/wallet.cpp

+        // Write the existing cache to disk
+        WalletBatch batch(GetDatabase());
+        batch.WriteDescriptorCacheItems(id, desc.cache);


re: #32489 (comment)

In commit "wallet: Write new descriptor's cache in AddWalletDescriptor" (fcf7dfe)

Can confirm if I revert this change I see a Wallet corrupted (-4) error from self.online.restorewallet("imports_watchonly", res["exported_file"]) running the test with Error: Unable to expand wallet descriptor from cache in the logs

src/wallet/wallet.cpp

ryanofsky · 2025-08-07T15:16:41Z

src/wallet/wallet.cpp

+    // Delete the watchonly wallet now that it has been exported to the desired location
+    fs::path watchonly_path = fs::PathFromString(watchonly_wallet->GetDatabase().Filename()).parent_path();
+    watchonly_wallet.reset();
+    fs::remove_all(watchonly_path);


In commit "wallet: Add CWallet::ExportWatchOnly" (236970d)

I don’t think fs::remove_all is appropriate here or elsewhere in wallet code. We should have a wallet delete function that explicitly removes known database and log files, then removes the directory, rather than recursively deleting everything. remove_all is fine in the happy case, but if there’s a bug (e.g. an extra parent_path call) or if users mistakenly store other data or bind mounts in the wallet directory, the results could be catastrophic. Seems fine to keep for now since there are other instances, but this would be good to address eventually.

I'll leave that for a followup

src/wallet/rpc/wallet.cpp

test/functional/wallet_exported_watchonly.py

stringintech

Concept ACK. I did a light code review.

stringintech · 2025-08-09T18:51:37Z

src/wallet/wallet.cpp

+            // For descriptors that cannot self expand (i.e. needs private keys or cache), retrieve the cache
+            uint256 desc_id = w_desc.id;
+            if (!w_desc.descriptor->CanSelfExpand()) {
+                DescriptorScriptPubKeyMan* desc_spkm = dynamic_cast<DescriptorScriptPubKeyMan*>(GetScriptPubKeyMan(desc_id));


Perhaps dynamic_cast could be wrapped by CHECK_NONFATAL?

I don't think it's really useful to have a check on that since earlier functions already enforce that the retrieved ScriptPubKeyMan must be a DescriptorScriptPubKeyMan.

enirox001

Concept ACK 84d3bc6

Left some nits, comments

src/wallet/wallet.cpp

enirox001 · 2025-08-13T17:17:35Z

src/wallet/wallet.cpp

+    if (!watchonly_wallet) {
+        return util::Error{_("Error: Failed to create new watchonly wallet")};
+    }
+


can we add a runtime sanity check here to ensure the newly created watch-only wallet does not have private keys enabled (i.e. !watchonly_wallet->HavePrivateKeys())? If the flag didn’t take effect we should fail early with a clear error rather than continuing.

I don't think that's necessary. It uses code that is well tested to behave correctly.

src/wallet/rpc/wallet.cpp

src/wallet/wallet.cpp

CanSelfExpand() reports whether a descriptor can be expanded without needing any caches or private keys to be provided by the caller of Expand().

If a descriptor does not need any caches or private keys in order to expand, then CanGetAddresses() should return true for that descriptor.

If a new WalletDescriptor is provided to us with a cache, write the cache to disk as well.

When listdescriptors retrieves the descriptors from the wallet, instead of having this logic in the RPC, move it into CWallet itself. This will enable other functions to get the descriptors in an exportable form.

rkrux

Started reviewing.

Code review cf58860

test/functional/wallet_exported_watchonly.py

rkrux · 2025-08-25T14:40:17Z

test/functional/wallet_exported_watchonly.py

+        assert_equal(online_wallet.getbalances()["mine"]["trusted"], 10)
+        assert_equal(offline_wallet.getbalances()["mine"]["trusted"], 0)


In cf58860 "test: Test for exportwatchonlywallet"

In this subtest, can also assert for few getwalletinfo RPC response values being equal as well such as txcount, keypoolsize, keypoolsize_hd_internal, birthtime, lastprocessedblock. For the flags RPC response key, it can be asserted that for the watch-only one, the value is the union of the original flags and disable_private_keys.

Might as well put all these assertions in a assert_common_values function that can be called in all the subtests.

I don't think any of those are actually useful things to assert. The point of the offline wallet is that it doesn't have access to the blockchain, so actually txcount, and lastprocessedblock should be different. The keypool sizes may be different depending on the node startup options, and birthtime may be different if something was used before being imported to the offline wallet.

Fair point.
I can imagine one of the use cases being that the user creates a offline wallet and immediately exports the watchonly version that's to be imported on a connected node.

test/functional/wallet_exported_watchonly.py

src/wallet/rpc/wallet.cpp

rkrux

Code review c9eb6c8

Looking good, mentioned few points.

src/wallet/wallet.cpp

rkrux · 2025-08-26T12:18:54Z

src/wallet/wallet.cpp

+            }
+
+            // Add to the watchonly wallet
+            if (auto spkm_res = watchonly_wallet->AddWalletDescriptor(w_desc, keys, "", false); !spkm_res) {


In 3a30ce4 "wallet: Add CWallet::ExportWatchOnly"

All the inactive descriptors are hardcoded to not being internal, is this correct? I can't seem to come up with a test case that fails this assumption.

internal is only used for setting the labels in the address book within AddWalletDescriptor, and only if the descriptor is not ranged and that there is a label provided. Since we copy the labels from the address book right after this, there's no need for us to provide label or internal to this call.

test/functional/wallet_exported_watchonly.py

rkrux · 2025-08-26T13:53:17Z

src/wallet/rpc/wallet.cpp

+
+            LOCK(pwallet->cs_wallet);
+            pwallet->TopUpKeyPool();
+            util::Result<std::string> exported = pwallet->ExportWatchOnlyWallet(fs::PathFromString(dest), context);


In 34137b3 "wallet, rpc: Add exportwatchonlywallet RPC"

Any particular reason to convert from string to path before passing it to ExportWatchOnlyWallet? Asking because I noticed 4 fs::PathToString(destination) calls inside ExportWatchOnlyWallet that could be avoided if the string is accepted and this conversion is done over there.

I think it conceptually makes more sense to be passing paths as fs::Path, rather than strings. Ideally, BackupWallet should be taking a fs::Path instead of a string, and the other conversions are only necessary for logging.

src/wallet/wallet.cpp

ExportWatchOnly produces a watchonly wallet file from a CWallet. This can be restored onto another instance of Bitcoin Core to allow that instance to watch the same descriptors, and also have all of the same initial address book and transactions.

rkrux

lgtm ACK 0f54714

Thanks for addressing the comments.

DrahtBot added the Wallet label May 13, 2025

achow101 force-pushed the export-watchonly-wallet branch from dc39a64 to 30ebc97 Compare May 13, 2025 22:38

DrahtBot added the CI failed label May 13, 2025

achow101 force-pushed the export-watchonly-wallet branch 2 times, most recently from 211dbe3 to 9fd93da Compare May 13, 2025 23:28

achow101 mentioned this pull request May 13, 2025

Menu action to export a watchonly wallet bitcoin-core/gui#872

Draft

achow101 force-pushed the export-watchonly-wallet branch from 9fd93da to 4ef9315 Compare May 14, 2025 18:00

DrahtBot removed the CI failed label May 14, 2025

shahsb reviewed May 15, 2025

View reviewed changes

This was referenced May 15, 2025

refactor: bdb removals #32511

Merged

wallet: Remove isminetypes #32523

Merged

DrahtBot added the Needs rebase label May 16, 2025

achow101 force-pushed the export-watchonly-wallet branch from 4ef9315 to def9594 Compare May 16, 2025 21:43

DrahtBot removed the Needs rebase label May 16, 2025

Sjors mentioned this pull request May 19, 2025

Awesome multisig PR labyrinth guide #24861

Open

16 tasks

DrahtBot added the Needs rebase label May 19, 2025

achow101 force-pushed the export-watchonly-wallet branch from def9594 to d41059b Compare May 20, 2025 00:55

DrahtBot removed the Needs rebase label May 20, 2025

ryanofsky added the Review club label Aug 4, 2025

pablomartin4btc reviewed Aug 6, 2025

View reviewed changes

src/wallet/rpc/wallet.cpp Outdated Show resolved Hide resolved

pablomartin4btc reviewed Aug 6, 2025

View reviewed changes

ryanofsky approved these changes Aug 7, 2025

View reviewed changes

stringintech reviewed Aug 9, 2025

View reviewed changes

DrahtBot mentioned this pull request Aug 13, 2025

Silent Payments: Receiving #32966

Draft

enirox001 reviewed Aug 13, 2025

View reviewed changes

achow101 force-pushed the export-watchonly-wallet branch 2 times, most recently from 7d63d08 to 4cc7be4 Compare August 14, 2025 19:56

pablomartin4btc reviewed Aug 20, 2025

View reviewed changes

src/wallet/wallet.cpp Outdated Show resolved Hide resolved

achow101 added 4 commits August 20, 2025 14:37

descriptor: Add CanSelfExpand()

a3be2d8

CanSelfExpand() reports whether a descriptor can be expanded without needing any caches or private keys to be provided by the caller of Expand().

wallet: Use Descriptor::CanSelfExpand() in CanGetAddresses()

9317ea0

If a descriptor does not need any caches or private keys in order to expand, then CanGetAddresses() should return true for that descriptor.

wallet: Write new descriptor's cache in AddWalletDescriptor

4d7aeab

If a new WalletDescriptor is provided to us with a cache, write the cache to disk as well.

wallet: Move listdescriptors retrieving from RPC to CWallet

6721670

When listdescriptors retrieves the descriptors from the wallet, instead of having this logic in the RPC, move it into CWallet itself. This will enable other functions to get the descriptors in an exportable form.

achow101 force-pushed the export-watchonly-wallet branch from 4cc7be4 to cf58860 Compare August 20, 2025 22:18

rkrux reviewed Aug 25, 2025

View reviewed changes

achow101 force-pushed the export-watchonly-wallet branch from cf58860 to c9eb6c8 Compare August 25, 2025 21:09

rkrux reviewed Aug 26, 2025

View reviewed changes

notmandatory mentioned this pull request Aug 26, 2025

Add example that builds from a bitcoin core exported data structure ValuedMammal/multi-keychain-wallet#1

Open

achow101 added 3 commits August 26, 2025 11:22

wallet: Add CWallet::ExportWatchOnly

2e833c1

ExportWatchOnly produces a watchonly wallet file from a CWallet. This can be restored onto another instance of Bitcoin Core to allow that instance to watch the same descriptors, and also have all of the same initial address book and transactions.

wallet, rpc: Add exportwatchonlywallet RPC

7775c54

test: Test for exportwatchonlywallet

6e8cddb

achow101 force-pushed the export-watchonly-wallet branch from c9eb6c8 to 6e8cddb Compare August 26, 2025 18:26

doc: update offline-signing-tutorial to use exportwatchonlywallet rpc

0f54714

rkrux approved these changes Aug 28, 2025

View reviewed changes

DrahtBot requested review from enirox001, ryanofsky, stringintech and pablomartin4btc August 28, 2025 11:31

		assert_equal(online_wallet.getbalances()["mine"]["trusted"], 10)
		assert_equal(offline_wallet.getbalances()["mine"]["trusted"], 0)

wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet #32489

Are you sure you want to change the base?

wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet #32489

Conversation

achow101 commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

DrahtBot commented May 13, 2025

Uh oh!

shahsb left a comment

Choose a reason for hiding this comment

Uh oh!

Sjors commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonatack commented Aug 4, 2025

Uh oh!

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stringintech left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enirox001 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rkrux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rkrux left a comment

Choose a reason for hiding this comment

Uh oh!

wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet #32489

wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet #32489

achow101 commented May 13, 2025 •

edited

Loading

DrahtBot commented May 13, 2025 •

edited

Loading

Sjors commented May 19, 2025 •

edited

Loading