From b26c196c8f1739bb94bb52dbf33e4b7b3eb222e7 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 13:29:50 -0700 Subject: [PATCH 01/15] wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC If the locked coin needs to be persisted to the wallet database, insteead of having the RPC figure out when to create a WalletBatch and having LockCoin's behavior depend on it, have LockCoin take whether to persist as a parameter so it makes the batch. Since unlocking a persisted locked coin requires a database write as well, we need to track whether the locked coin was persisted to the wallet database so that it can erase the locked coin when necessary. Keeping track of whether a locked coin was persisted is also useful information for future PRs. --- src/wallet/interfaces.cpp | 6 ++-- src/wallet/rpc/coins.cpp | 8 ++--- src/wallet/rpc/spend.cpp | 2 +- src/wallet/spend.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 4 +-- src/wallet/wallet.cpp | 57 +++++++++++++++++--------------- src/wallet/wallet.h | 15 +++++---- src/wallet/walletdb.cpp | 2 +- 8 files changed, 49 insertions(+), 47 deletions(-) diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 0048c025a29..ad75e69eccc 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -249,14 +249,12 @@ class WalletImpl : public Wallet bool lockCoin(const COutPoint& output, const bool write_to_db) override { LOCK(m_wallet->cs_wallet); - std::unique_ptr batch = write_to_db ? std::make_unique(m_wallet->GetDatabase()) : nullptr; - return m_wallet->LockCoin(output, batch.get()); + return m_wallet->LockCoin(output, write_to_db); } bool unlockCoin(const COutPoint& output) override { LOCK(m_wallet->cs_wallet); - std::unique_ptr batch = std::make_unique(m_wallet->GetDatabase()); - return m_wallet->UnlockCoin(output, batch.get()); + return m_wallet->UnlockCoin(output); } bool isLockedCoin(const COutPoint& output) override { diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index cce9b26babe..9351259a5a5 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -356,16 +356,12 @@ RPCHelpMan lockunspent() outputs.push_back(outpt); } - std::unique_ptr batch = nullptr; - // Unlock is always persistent - if (fUnlock || persistent) batch = std::make_unique(pwallet->GetDatabase()); - // Atomically set (un)locked status for the outputs. for (const COutPoint& outpt : outputs) { if (fUnlock) { - if (!pwallet->UnlockCoin(outpt, batch.get())) throw JSONRPCError(RPC_WALLET_ERROR, "Unlocking coin failed"); + if (!pwallet->UnlockCoin(outpt)) throw JSONRPCError(RPC_WALLET_ERROR, "Unlocking coin failed"); } else { - if (!pwallet->LockCoin(outpt, batch.get())) throw JSONRPCError(RPC_WALLET_ERROR, "Locking coin failed"); + if (!pwallet->LockCoin(outpt, persistent)) throw JSONRPCError(RPC_WALLET_ERROR, "Locking coin failed"); } } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 27bcbc3b94c..e2499199912 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1582,7 +1582,7 @@ RPCHelpMan sendall() const bool lock_unspents{options.exists("lock_unspents") ? options["lock_unspents"].get_bool() : false}; if (lock_unspents) { for (const CTxIn& txin : rawTx.vin) { - pwallet->LockCoin(txin.prevout); + pwallet->LockCoin(txin.prevout, /*persist=*/false); } } diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index d96946e7b89..50a4bb3377d 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1467,7 +1467,7 @@ util::Result FundTransaction(CWallet& wallet, const CM if (lockUnspents) { for (const CTxIn& txin : res->tx->vin) { - wallet.LockCoin(txin.prevout); + wallet.LockCoin(txin.prevout, /*persist=*/false); } } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 966c6d2c4ba..fa141696616 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -458,7 +458,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) for (const auto& group : list) { for (const auto& coin : group.second) { LOCK(wallet->cs_wallet); - wallet->LockCoin(coin.outpoint); + wallet->LockCoin(coin.outpoint, /*persist=*/false); } } { @@ -486,7 +486,7 @@ void TestCoinsResult(ListCoinsTest& context, OutputType out_type, CAmount amount filter.skip_locked = false; CoinsResult available_coins = AvailableCoins(*context.wallet, nullptr, std::nullopt, filter); // Lock outputs so they are not spent in follow-up transactions - for (uint32_t i = 0; i < wtx.tx->vout.size(); i++) context.wallet->LockCoin({wtx.GetHash(), i}); + for (uint32_t i = 0; i < wtx.tx->vout.size(); i++) context.wallet->LockCoin({wtx.GetHash(), i}, /*persist=*/false); for (const auto& [type, size] : expected_coins_sizes) BOOST_CHECK_EQUAL(size, available_coins.coins[type].size()); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6542abc23df..4497e613e78 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -785,16 +785,11 @@ bool CWallet::IsSpent(const COutPoint& outpoint) const return false; } -void CWallet::AddToSpends(const COutPoint& outpoint, const Txid& txid, WalletBatch* batch) +void CWallet::AddToSpends(const COutPoint& outpoint, const Txid& txid) { mapTxSpends.insert(std::make_pair(outpoint, txid)); - if (batch) { - UnlockCoin(outpoint, batch); - } else { - WalletBatch temp_batch(GetDatabase()); - UnlockCoin(outpoint, &temp_batch); - } + UnlockCoin(outpoint); std::pair range; range = mapTxSpends.equal_range(outpoint); @@ -802,13 +797,13 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const Txid& txid, WalletBat } -void CWallet::AddToSpends(const CWalletTx& wtx, WalletBatch* batch) +void CWallet::AddToSpends(const CWalletTx& wtx) { if (wtx.IsCoinBase()) // Coinbases don't spend anything! return; for (const CTxIn& txin : wtx.tx->vin) - AddToSpends(txin.prevout, wtx.GetHash(), batch); + AddToSpends(txin.prevout, wtx.GetHash()); } bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) @@ -1058,7 +1053,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx, rescanning_old_block); - AddToSpends(wtx, &batch); + AddToSpends(wtx); // Update birth time when tx time is older than it. MaybeUpdateBirthTime(wtx.GetTxTime()); @@ -2608,22 +2603,34 @@ util::Result CWallet::DisplayAddress(const CTxDestination& dest) return util::Error{_("There is no ScriptPubKeyManager for this address")}; } -bool CWallet::LockCoin(const COutPoint& output, WalletBatch* batch) +void CWallet::LoadLockedCoin(const COutPoint& coin, bool persistent) { AssertLockHeld(cs_wallet); - setLockedCoins.insert(output); - if (batch) { - return batch->WriteLockedUTXO(output); + m_locked_coins.emplace(coin, persistent); +} + +bool CWallet::LockCoin(const COutPoint& output, bool persist) +{ + AssertLockHeld(cs_wallet); + LoadLockedCoin(output, persist); + if (persist) { + WalletBatch batch(GetDatabase()); + return batch.WriteLockedUTXO(output); } return true; } -bool CWallet::UnlockCoin(const COutPoint& output, WalletBatch* batch) +bool CWallet::UnlockCoin(const COutPoint& output) { AssertLockHeld(cs_wallet); - bool was_locked = setLockedCoins.erase(output); - if (batch && was_locked) { - return batch->EraseLockedUTXO(output); + auto locked_coin_it = m_locked_coins.find(output); + if (locked_coin_it != m_locked_coins.end()) { + bool persisted = locked_coin_it->second; + m_locked_coins.erase(locked_coin_it); + if (persisted) { + WalletBatch batch(GetDatabase()); + return batch.EraseLockedUTXO(output); + } } return true; } @@ -2633,26 +2640,24 @@ bool CWallet::UnlockAllCoins() AssertLockHeld(cs_wallet); bool success = true; WalletBatch batch(GetDatabase()); - for (auto it = setLockedCoins.begin(); it != setLockedCoins.end(); ++it) { - success &= batch.EraseLockedUTXO(*it); + for (const auto& [coin, persistent] : m_locked_coins) { + if (persistent) success = success && batch.EraseLockedUTXO(coin); } - setLockedCoins.clear(); + m_locked_coins.clear(); return success; } bool CWallet::IsLockedCoin(const COutPoint& output) const { AssertLockHeld(cs_wallet); - return setLockedCoins.count(output) > 0; + return m_locked_coins.count(output) > 0; } void CWallet::ListLockedCoins(std::vector& vOutpts) const { AssertLockHeld(cs_wallet); - for (std::set::iterator it = setLockedCoins.begin(); - it != setLockedCoins.end(); it++) { - COutPoint outpt = (*it); - vOutpts.push_back(outpt); + for (const auto& [coin, _] : m_locked_coins) { + vOutpts.push_back(coin); } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0c4f89ccf5c..ec122175e31 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -333,8 +333,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati */ typedef std::unordered_multimap TxSpends; TxSpends mapTxSpends GUARDED_BY(cs_wallet); - void AddToSpends(const COutPoint& outpoint, const Txid& txid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void AddToSpends(const CWalletTx& wtx, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void AddToSpends(const COutPoint& outpoint, const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void AddToSpends(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Add a transaction to the wallet, or update it. confirm.block_* should @@ -497,8 +497,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati /** Set of Coins owned by this wallet that we won't try to spend from. A * Coin may be locked if it has already been used to fund a transaction * that hasn't confirmed yet. We wouldn't consider the Coin spent already, - * but also shouldn't try to use it again. */ - std::set setLockedCoins GUARDED_BY(cs_wallet); + * but also shouldn't try to use it again. + * bool to track whether this locked coin is persisted to disk. + */ + std::map m_locked_coins GUARDED_BY(cs_wallet); /** Registered interfaces::Chain::Notifications handler. */ std::unique_ptr m_chain_notifications_handler; @@ -546,8 +548,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati util::Result DisplayAddress(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsLockedCoin(const COutPoint& output) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool LockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool UnlockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void LoadLockedCoin(const COutPoint& coin, bool persistent) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool LockCoin(const COutPoint& output, bool persist) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool UnlockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool UnlockAllCoins() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void ListLockedCoins(std::vector& vOutpts) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index a3fcb0584ea..6872099944f 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1071,7 +1071,7 @@ static DBErrors LoadTxRecords(CWallet* pwallet, DatabaseBatch& batch, std::vecto uint32_t n; key >> hash; key >> n; - pwallet->LockCoin(COutPoint(hash, n)); + pwallet->LoadLockedCoin(COutPoint(hash, n), /*persistent=*/true); return DBErrors::LOAD_OK; }); result = std::max(result, locked_utxo_res.m_result); From df71b2485fe550459be63a3748321187f2e31c41 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 16:19:31 -0700 Subject: [PATCH 02/15] wallet, rpc: Push the normalized parent descriptor Instead of providing the descriptor string as stored in the db, use the normalized descriptor as is done for getaddressinfo's parent_desc field. --- src/wallet/rpc/util.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index a840a657f5f..1d344f2a241 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -109,7 +109,10 @@ void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, { UniValue parent_descs(UniValue::VARR); for (const auto& desc: wallet.GetWalletDescriptors(script_pubkey)) { - parent_descs.push_back(desc.descriptor->ToString()); + std::string desc_str; + FlatSigningProvider dummy_provider; + if (!CHECK_NONFATAL(desc.descriptor->ToNormalizedString(dummy_provider, desc_str, &desc.cache))) continue; + parent_descs.push_back(desc_str); } entry.pushKV("parent_descs", std::move(parent_descs)); } From d4cd585bf81ff77642801ebaa8aa9fc7ded2b17e Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 22 May 2025 14:12:45 -0700 Subject: [PATCH 03/15] test: Enable default wallet for wallet_descriptor.py --- test/functional/wallet_descriptor.py | 38 +++++++++++++++------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py index 356b1ac47e3..d5641c8afe3 100755 --- a/test/functional/wallet_descriptor.py +++ b/test/functional/wallet_descriptor.py @@ -24,20 +24,22 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [['-keypool=100']] - self.wallet_names = [] def skip_test_if_missing_module(self): self.skip_if_no_wallet() self.skip_if_no_py_sqlite3() def run_test(self): + self.generate(self.nodes[0], COINBASE_MATURITY + 1) + # Make a descriptor wallet self.log.info("Making a descriptor wallet") self.nodes[0].createwallet(wallet_name="desc1") + wallet = self.nodes[0].get_wallet_rpc("desc1") # A descriptor wallet should have 100 addresses * 4 types = 400 keys self.log.info("Checking wallet info") - wallet_info = self.nodes[0].getwalletinfo() + wallet_info = wallet.getwalletinfo() assert_equal(wallet_info['format'], 'sqlite') assert_equal(wallet_info['keypoolsize'], 400) assert_equal(wallet_info['keypoolsize_hd_internal'], 400) @@ -45,44 +47,44 @@ def run_test(self): # Check that getnewaddress works self.log.info("Test that getnewaddress and getrawchangeaddress work") - addr = self.nodes[0].getnewaddress("", "legacy") - addr_info = self.nodes[0].getaddressinfo(addr) + addr = wallet.getnewaddress("", "legacy") + addr_info = wallet.getaddressinfo(addr) assert addr_info['desc'].startswith('pkh(') assert_equal(addr_info['hdkeypath'], 'm/44h/1h/0h/0/0') - addr = self.nodes[0].getnewaddress("", "p2sh-segwit") - addr_info = self.nodes[0].getaddressinfo(addr) + addr = wallet.getnewaddress("", "p2sh-segwit") + addr_info = wallet.getaddressinfo(addr) assert addr_info['desc'].startswith('sh(wpkh(') assert_equal(addr_info['hdkeypath'], 'm/49h/1h/0h/0/0') - addr = self.nodes[0].getnewaddress("", "bech32") - addr_info = self.nodes[0].getaddressinfo(addr) + addr = wallet.getnewaddress("", "bech32") + addr_info = wallet.getaddressinfo(addr) assert addr_info['desc'].startswith('wpkh(') assert_equal(addr_info['hdkeypath'], 'm/84h/1h/0h/0/0') - addr = self.nodes[0].getnewaddress("", "bech32m") - addr_info = self.nodes[0].getaddressinfo(addr) + addr = wallet.getnewaddress("", "bech32m") + addr_info = wallet.getaddressinfo(addr) assert addr_info['desc'].startswith('tr(') assert_equal(addr_info['hdkeypath'], 'm/86h/1h/0h/0/0') # Check that getrawchangeaddress works - addr = self.nodes[0].getrawchangeaddress("legacy") - addr_info = self.nodes[0].getaddressinfo(addr) + addr = wallet.getrawchangeaddress("legacy") + addr_info = wallet.getaddressinfo(addr) assert addr_info['desc'].startswith('pkh(') assert_equal(addr_info['hdkeypath'], 'm/44h/1h/0h/1/0') - addr = self.nodes[0].getrawchangeaddress("p2sh-segwit") - addr_info = self.nodes[0].getaddressinfo(addr) + addr = wallet.getrawchangeaddress("p2sh-segwit") + addr_info = wallet.getaddressinfo(addr) assert addr_info['desc'].startswith('sh(wpkh(') assert_equal(addr_info['hdkeypath'], 'm/49h/1h/0h/1/0') - addr = self.nodes[0].getrawchangeaddress("bech32") - addr_info = self.nodes[0].getaddressinfo(addr) + addr = wallet.getrawchangeaddress("bech32") + addr_info = wallet.getaddressinfo(addr) assert addr_info['desc'].startswith('wpkh(') assert_equal(addr_info['hdkeypath'], 'm/84h/1h/0h/1/0') - addr = self.nodes[0].getrawchangeaddress("bech32m") - addr_info = self.nodes[0].getaddressinfo(addr) + addr = wallet.getrawchangeaddress("bech32m") + addr_info = wallet.getaddressinfo(addr) assert addr_info['desc'].startswith('tr(') assert_equal(addr_info['hdkeypath'], 'm/86h/1h/0h/1/0') From c556c289f8b165c174c4537eb5ac31d5fd402deb Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 22 May 2025 14:13:08 -0700 Subject: [PATCH 04/15] test: Verify parent_desc in RPCs getaddressinfo, listunspent, listtransactions, listsinceblock, and gettransaction all include parent_desc(s). Make sure that these are consistent with each other, as well as being in normalized form. --- test/functional/wallet_descriptor.py | 51 ++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py index d5641c8afe3..4ab79b799ee 100755 --- a/test/functional/wallet_descriptor.py +++ b/test/functional/wallet_descriptor.py @@ -9,6 +9,8 @@ except ImportError: pass +import re + from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -29,6 +31,54 @@ def skip_test_if_missing_module(self): self.skip_if_no_wallet() self.skip_if_no_py_sqlite3() + def test_parent_descriptors(self): + self.log.info("Check that parent_descs is the same for all RPCs and is normalized") + self.nodes[0].createwallet(wallet_name="parent_descs") + wallet = self.nodes[0].get_wallet_rpc("parent_descs") + default_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + + addr = wallet.getnewaddress() + parent_desc = wallet.getaddressinfo(addr)["parent_desc"] + + # Verify that the parent descriptor is normalized + # First remove the checksum + desc_verify = parent_desc.split("#")[0] + # Next extract the xpub + desc_verify = re.sub(r"tpub\w+?(?=/)", "", desc_verify) + # Extract origin info + origin_match = re.search(r'\[([\da-fh/]+)\]', desc_verify) + origin_part = origin_match.group(1) if origin_match else "" + # Split on "]" for everything after the origin info + after_origin = desc_verify.split("]", maxsplit=1)[-1] + # Look for the hardened markers “h” inside each piece + # We don't need to check for aspostrophe as normalization will not output aspostrophe + found_hardened_in_origin = "h" in origin_part + found_hardened_after_origin = "h" in after_origin + assert_equal(found_hardened_in_origin, True) + assert_equal(found_hardened_after_origin, False) + + # Send some coins so we can check listunspent, listtransactions, listunspent, and gettransaction + since_block = self.nodes[0].getbestblockhash() + txid = default_wallet.sendtoaddress(addr, 1) + self.generate(self.nodes[0], 1) + + unspent = wallet.listunspent() + assert_equal(len(unspent), 1) + assert_equal(unspent[0]["parent_descs"], [parent_desc]) + + txs = wallet.listtransactions() + assert_equal(len(txs), 1) + assert_equal(txs[0]["parent_descs"], [parent_desc]) + + txs = wallet.listsinceblock(since_block)["transactions"] + assert_equal(len(txs), 1) + assert_equal(txs[0]["parent_descs"], [parent_desc]) + + tx = wallet.gettransaction(txid=txid, verbose=True) + assert_equal(tx["details"][0]["parent_descs"], [parent_desc]) + + wallet.unloadwallet() + def run_test(self): self.generate(self.nodes[0], COINBASE_MATURITY + 1) @@ -218,6 +268,7 @@ def run_test(self): conn.close() assert_raises_rpc_error(-4, "Unexpected legacy entry in descriptor wallet found.", self.nodes[0].loadwallet, "crashme") + self.test_parent_descriptors() if __name__ == '__main__': WalletDescriptorTest(__file__).main() From c81f8df97a1cf73fb9240f532ce97f9a25190c34 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 13 May 2025 14:33:45 -0700 Subject: [PATCH 05/15] wallet: Set upgraded descriptor cache flag for newly created wallets Although WalletBatch::LoadWallet performs the descriptor cache upgrade, because new wallets do not have the descriptor flag set yet, the upgrade does not run and set the flag. Since new wallets will always being using the upgraded cache, there's no reason to wait to set the flag, so set it when the wallet flags are being initialized for new wallets. --- src/wallet/wallet.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4497e613e78..0ab75315e3a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2895,7 +2895,9 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri // ensure this wallet.dat can only be opened by clients supporting HD with chain split and expects no default key walletInstance->SetMinVersion(FEATURE_LATEST); - walletInstance->InitWalletFlags(wallet_creation_flags); + // Init with passed flags. + // Always set the cache upgrade flag as this feature is supported from the beginning. + walletInstance->InitWalletFlags(wallet_creation_flags | WALLET_FLAG_LAST_HARDENED_XPUB_CACHED); // Only descriptor wallets can be created assert(walletInstance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); From c1c8d8f4e1d0cf656ed47f58fc6bd25e320e3416 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 14:00:28 -0700 Subject: [PATCH 06/15] wallet: Add GetWalletFlags --- src/wallet/wallet.cpp | 5 +++++ src/wallet/wallet.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0ab75315e3a..9794892a4b1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1734,6 +1734,11 @@ void CWallet::InitWalletFlags(uint64_t flags) if (!LoadWalletFlags(flags)) assert(false); } +uint64_t CWallet::GetWalletFlags() const +{ + return m_wallet_flags; +} + void CWallet::MaybeUpdateBirthTime(int64_t time) { int64_t birthtime = m_birth_time.load(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ec122175e31..70378064230 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -907,6 +907,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void InitWalletFlags(uint64_t flags); /** Loads the flags into the wallet. (used by LoadWallet) */ bool LoadWalletFlags(uint64_t flags); + //! Retrieve all of the wallet's flags + uint64_t GetWalletFlags() const; /** Returns a bracketed wallet name for displaying in logs, will return [default wallet] if the wallet has no name */ std::string GetDisplayName() const override From 27236255141d18134dc94942f7369dd67f2439bf Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 22 May 2025 17:29:21 -0700 Subject: [PATCH 07/15] wallet, rpc: Output wallet flags in getwalletinfo --- src/wallet/rpc/wallet.cpp | 25 ++++++++++++++++++++++--- src/wallet/wallet.h | 26 ++++++++++++++++++-------- test/functional/wallet_avoidreuse.py | 2 ++ test/functional/wallet_createwallet.py | 1 + 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 4bd4851609d..e2ea4aba698 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -61,6 +61,10 @@ static RPCHelpMan getwalletinfo() {RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"}, {RPCResult::Type::BOOL, "blank", "Whether this wallet intentionally does not contain any keys, scripts, or descriptors"}, {RPCResult::Type::NUM_TIME, "birthtime", /*optional=*/true, "The start time for blocks scanning. It could be modified by (re)importing any descriptor with an earlier timestamp."}, + {RPCResult::Type::ARR, "flags", "The flags currently set on the wallet", + { + {RPCResult::Type::STR, "flag", "The name of the flag"}, + }}, RESULT_LAST_PROCESSED_BLOCK, }}, }, @@ -116,6 +120,21 @@ static RPCHelpMan getwalletinfo() obj.pushKV("birthtime", birthtime); } + // Push known flags + UniValue flags(UniValue::VARR); + uint64_t wallet_flags = pwallet->GetWalletFlags(); + for (uint64_t i = 0; i < 64; ++i) { + uint64_t flag = uint64_t{1} << i; + if (flag & wallet_flags) { + if (flag & KNOWN_WALLET_FLAGS) { + flags.push_back(WALLET_FLAG_TO_STRING.at(WalletFlags{flag})); + } else { + flags.push_back(strprintf("unknown_flag_%u", i)); + } + } + } + obj.pushKV("flags", flags); + AppendLastProcessedBlock(obj, *pwallet); return obj; }, @@ -267,7 +286,7 @@ static RPCHelpMan loadwallet() static RPCHelpMan setwalletflag() { std::string flags; - for (auto& it : WALLET_FLAG_MAP) + for (auto& it : STRING_TO_WALLET_FLAG) if (it.second & MUTABLE_WALLET_FLAGS) flags += (flags == "" ? "" : ", ") + it.first; @@ -298,11 +317,11 @@ static RPCHelpMan setwalletflag() std::string flag_str = request.params[0].get_str(); bool value = request.params[1].isNull() || request.params[1].get_bool(); - if (!WALLET_FLAG_MAP.count(flag_str)) { + if (!STRING_TO_WALLET_FLAG.count(flag_str)) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Unknown wallet flag: %s", flag_str)); } - auto flag = WALLET_FLAG_MAP.at(flag_str); + auto flag = STRING_TO_WALLET_FLAG.at(flag_str); if (!(flag & MUTABLE_WALLET_FLAGS)) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Wallet flag is immutable: %s", flag_str)); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 70378064230..108da7d91be 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -160,14 +160,24 @@ static constexpr uint64_t KNOWN_WALLET_FLAGS = static constexpr uint64_t MUTABLE_WALLET_FLAGS = WALLET_FLAG_AVOID_REUSE; -static const std::map WALLET_FLAG_MAP{ - {"avoid_reuse", WALLET_FLAG_AVOID_REUSE}, - {"blank", WALLET_FLAG_BLANK_WALLET}, - {"key_origin_metadata", WALLET_FLAG_KEY_ORIGIN_METADATA}, - {"last_hardened_xpub_cached", WALLET_FLAG_LAST_HARDENED_XPUB_CACHED}, - {"disable_private_keys", WALLET_FLAG_DISABLE_PRIVATE_KEYS}, - {"descriptor_wallet", WALLET_FLAG_DESCRIPTORS}, - {"external_signer", WALLET_FLAG_EXTERNAL_SIGNER} +static const std::map WALLET_FLAG_TO_STRING{ + {WALLET_FLAG_AVOID_REUSE, "avoid_reuse"}, + {WALLET_FLAG_BLANK_WALLET, "blank"}, + {WALLET_FLAG_KEY_ORIGIN_METADATA, "key_origin_metadata"}, + {WALLET_FLAG_LAST_HARDENED_XPUB_CACHED, "last_hardened_xpub_cached"}, + {WALLET_FLAG_DISABLE_PRIVATE_KEYS, "disable_private_keys"}, + {WALLET_FLAG_DESCRIPTORS, "descriptor_wallet"}, + {WALLET_FLAG_EXTERNAL_SIGNER, "external_signer"} +}; + +static const std::map STRING_TO_WALLET_FLAG{ + {WALLET_FLAG_TO_STRING.at(WALLET_FLAG_AVOID_REUSE), WALLET_FLAG_AVOID_REUSE}, + {WALLET_FLAG_TO_STRING.at(WALLET_FLAG_BLANK_WALLET), WALLET_FLAG_BLANK_WALLET}, + {WALLET_FLAG_TO_STRING.at(WALLET_FLAG_KEY_ORIGIN_METADATA), WALLET_FLAG_KEY_ORIGIN_METADATA}, + {WALLET_FLAG_TO_STRING.at(WALLET_FLAG_LAST_HARDENED_XPUB_CACHED), WALLET_FLAG_LAST_HARDENED_XPUB_CACHED}, + {WALLET_FLAG_TO_STRING.at(WALLET_FLAG_DISABLE_PRIVATE_KEYS), WALLET_FLAG_DISABLE_PRIVATE_KEYS}, + {WALLET_FLAG_TO_STRING.at(WALLET_FLAG_DESCRIPTORS), WALLET_FLAG_DESCRIPTORS}, + {WALLET_FLAG_TO_STRING.at(WALLET_FLAG_EXTERNAL_SIGNER), WALLET_FLAG_EXTERNAL_SIGNER} }; /** A wrapper to reserve an address from a wallet diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 2ae153a937e..44fa16ff64c 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -104,7 +104,9 @@ def test_persistence(self): # Flags should be node1.avoid_reuse=false, node2.avoid_reuse=true assert_equal(self.nodes[0].getwalletinfo()["avoid_reuse"], False) + assert_equal(sorted(self.nodes[0].getwalletinfo()["flags"]), sorted(["descriptor_wallet", "last_hardened_xpub_cached"])) assert_equal(self.nodes[1].getwalletinfo()["avoid_reuse"], True) + assert_equal(sorted(self.nodes[1].getwalletinfo()["flags"]), sorted(["descriptor_wallet", "last_hardened_xpub_cached", "avoid_reuse"])) self.restart_node(1) self.connect_nodes(0, 1) diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py index 312d22fce48..a2e7aae33ea 100755 --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -44,6 +44,7 @@ def run_test(self): assert_raises_rpc_error(-4, "Error: This wallet has no available keys", w1.getrawchangeaddress) import_res = w1.importdescriptors([{"desc": w0.getaddressinfo(address1)['desc'], "timestamp": "now"}]) assert_equal(import_res[0]["success"], True) + assert_equal(sorted(w1.getwalletinfo()["flags"]), sorted(["last_hardened_xpub_cached", "descriptor_wallet", "disable_private_keys"])) self.log.info('Test that private keys cannot be imported') privkey, pubkey = generate_keypair(wif=True) From 16f80545ef0892af0bc3f0574c4d2c2b7bb62fb9 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 15:41:53 -0700 Subject: [PATCH 08/15] descriptor: Add CanSelfExpand() CanSelfExpand() reports whether a descriptor can be expanded without needing any caches or private keys to be provided by the caller of Expand(). --- src/script/descriptor.cpp | 43 ++++++++++++++++++++++++++++ src/script/descriptor.h | 3 ++ src/wallet/test/walletload_tests.cpp | 1 + 3 files changed, 47 insertions(+) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 71645c87462..0cb8f3706a3 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -221,6 +221,9 @@ struct PubkeyProvider /** Make a deep copy of this PubkeyProvider */ virtual std::unique_ptr Clone() const = 0; + + /** Whether this PubkeyProvider can always provide a public key without cache or private key arguments */ + virtual bool CanSelfExpand() const = 0; }; class OriginPubkeyProvider final : public PubkeyProvider @@ -290,6 +293,7 @@ class OriginPubkeyProvider final : public PubkeyProvider { return std::make_unique(m_expr_index, m_origin, m_provider->Clone(), m_apostrophe); } + bool CanSelfExpand() const override { return m_provider->CanSelfExpand(); } }; /** An object representing a parsed constant public key in a descriptor. */ @@ -350,6 +354,7 @@ class ConstPubkeyProvider final : public PubkeyProvider { return std::make_unique(m_expr_index, m_pubkey, m_xonly); } + bool CanSelfExpand() const final { return true; } }; enum class DeriveType { @@ -572,6 +577,7 @@ class BIP32PubkeyProvider final : public PubkeyProvider { return std::make_unique(m_expr_index, m_root_extkey, m_path, m_derive, m_apostrophe); } + bool CanSelfExpand() const override { return !IsHardened(); } }; /** Base class for all Descriptor implementations. */ @@ -800,6 +806,7 @@ class AddressDescriptor final : public DescriptorImpl } bool IsSingleType() const final { return true; } bool ToPrivateString(const SigningProvider& arg, std::string& out) const final { return false; } + bool CanSelfExpand() const final { return true; } std::optional ScriptSize() const override { return GetScriptForDestination(m_destination).size(); } std::unique_ptr Clone() const override @@ -827,6 +834,7 @@ class RawDescriptor final : public DescriptorImpl } bool IsSingleType() const final { return true; } bool ToPrivateString(const SigningProvider& arg, std::string& out) const final { return false; } + bool CanSelfExpand() const final { return true; } std::optional ScriptSize() const override { return m_script.size(); } @@ -854,6 +862,7 @@ class PKDescriptor final : public DescriptorImpl public: PKDescriptor(std::unique_ptr prov, bool xonly = false) : DescriptorImpl(Vector(std::move(prov)), "pk"), m_xonly(xonly) {} bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { return m_pubkey_args[0]->CanSelfExpand(); } std::optional ScriptSize() const override { return 1 + (m_xonly ? 32 : m_pubkey_args[0]->GetSize()) + 1; @@ -889,6 +898,7 @@ class PKHDescriptor final : public DescriptorImpl PKHDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "pkh") {} std::optional GetOutputType() const override { return OutputType::LEGACY; } bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { return m_pubkey_args[0]->CanSelfExpand(); } std::optional ScriptSize() const override { return 1 + 1 + 1 + 20 + 1 + 1; } @@ -922,6 +932,7 @@ class WPKHDescriptor final : public DescriptorImpl WPKHDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "wpkh") {} std::optional GetOutputType() const override { return OutputType::BECH32; } bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { return m_pubkey_args[0]->CanSelfExpand(); } std::optional ScriptSize() const override { return 1 + 1 + 20; } @@ -963,6 +974,7 @@ class ComboDescriptor final : public DescriptorImpl public: ComboDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "combo") {} bool IsSingleType() const final { return false; } + bool CanSelfExpand() const override { return m_pubkey_args[0]->CanSelfExpand(); } std::unique_ptr Clone() const override { return std::make_unique(m_pubkey_args.at(0)->Clone()); @@ -987,6 +999,13 @@ class MultisigDescriptor final : public DescriptorImpl public: MultisigDescriptor(int threshold, std::vector> providers, bool sorted = false) : DescriptorImpl(std::move(providers), sorted ? "sortedmulti" : "multi"), m_threshold(threshold), m_sorted(sorted) {} bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { + bool can_expand = true; + for (const auto& key : m_pubkey_args) { + can_expand &= key->CanSelfExpand(); + } + return can_expand; + } std::optional ScriptSize() const override { const auto n_keys = m_pubkey_args.size(); @@ -1038,6 +1057,13 @@ class MultiADescriptor final : public DescriptorImpl public: MultiADescriptor(int threshold, std::vector> providers, bool sorted = false) : DescriptorImpl(std::move(providers), sorted ? "sortedmulti_a" : "multi_a"), m_threshold(threshold), m_sorted(sorted) {} bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { + bool can_expand = true; + for (const auto& key : m_pubkey_args) { + can_expand &= key->CanSelfExpand(); + } + return can_expand; + } std::optional ScriptSize() const override { const auto n_keys = m_pubkey_args.size(); @@ -1084,6 +1110,7 @@ class SHDescriptor final : public DescriptorImpl return OutputType::LEGACY; } bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { return m_subdescriptor_args[0]->CanSelfExpand(); } std::optional ScriptSize() const override { return 1 + 1 + 20 + 1; } @@ -1125,6 +1152,7 @@ class WSHDescriptor final : public DescriptorImpl WSHDescriptor(std::unique_ptr desc) : DescriptorImpl({}, std::move(desc), "wsh") {} std::optional GetOutputType() const override { return OutputType::BECH32; } bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { return m_subdescriptor_args[0]->CanSelfExpand(); } std::optional ScriptSize() const override { return 1 + 1 + 32; } @@ -1202,6 +1230,13 @@ class TRDescriptor final : public DescriptorImpl } std::optional GetOutputType() const override { return OutputType::BECH32M; } bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { + bool can_expand = m_pubkey_args[0]->CanSelfExpand(); + for (const auto& sub : m_subdescriptor_args) { + can_expand &= sub->CanSelfExpand(); + } + return can_expand; + } std::optional ScriptSize() const override { return 1 + 1 + 32; } @@ -1329,6 +1364,13 @@ class MiniscriptDescriptor final : public DescriptorImpl bool IsSolvable() const override { return true; } bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { + bool can_expand = true; + for (const auto& key : m_pubkey_args) { + can_expand &= key->CanSelfExpand(); + } + return can_expand; + } std::optional ScriptSize() const override { return m_node->ScriptSize(); } @@ -1368,6 +1410,7 @@ class RawTRDescriptor final : public DescriptorImpl RawTRDescriptor(std::unique_ptr output_key) : DescriptorImpl(Vector(std::move(output_key)), "rawtr") {} std::optional GetOutputType() const override { return OutputType::BECH32M; } bool IsSingleType() const final { return true; } + bool CanSelfExpand() const override { return m_pubkey_args[0]->CanSelfExpand(); } std::optional ScriptSize() const override { return 1 + 1 + 32; } diff --git a/src/script/descriptor.h b/src/script/descriptor.h index 473649a3144..e1147e64f8f 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -117,6 +117,9 @@ struct Descriptor { /** Convert the descriptor to a normalized string. Normalized descriptors have the xpub at the last hardened step. This fails if the provided provider does not have the private keys to derive that xpub. */ virtual bool ToNormalizedString(const SigningProvider& provider, std::string& out, const DescriptorCache* cache = nullptr) const = 0; + /** Whether the descriptor can be used to get more addresses without needing a cache or private keys. */ + virtual bool CanSelfExpand() const = 0; + /** Expand a descriptor at a specified position. * * @param[in] pos The position at which to expand the descriptor. If IsRange() is false, this is ignored. diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index 0c69849d0b6..79da5ca268b 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -35,6 +35,7 @@ class DummyDescriptor final : public Descriptor { std::optional MaxSatisfactionWeight(bool) const override { return {}; } std::optional MaxSatisfactionElems() const override { return {}; } void GetPubKeys(std::set& pubkeys, std::set& ext_pubs) const override {} + bool CanSelfExpand() const final { return false; } }; BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup) From 32e272537daf632cdc555ccfebfbf0ad0c211f69 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 15:42:51 -0700 Subject: [PATCH 09/15] wallet: Use Descriptor::CanSelfExpand() in CanGetAddresses() If a descriptor does not need any caches or private keys in order to expand, then CanGetAddresses() should return true for that descriptor. --- src/wallet/scriptpubkeyman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 6a090c77334..8ccb7ecb3df 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/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()); } bool DescriptorScriptPubKeyMan::HavePrivateKeys() const From 76dd4ed69accffc86b35c97b299de32796b667e7 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 13 May 2025 14:36:04 -0700 Subject: [PATCH 10/15] wallet: Write new descriptor's cache in AddWalletDescriptor If a new WalletDescriptor is provided to us with a cache, write the cache to disk as well. --- src/wallet/wallet.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9794892a4b1..9c1bd1aa6e5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3744,6 +3744,10 @@ util::Result> CWallet::AddWall // Save the descriptor to memory uint256 id = new_spk_man->GetID(); AddScriptPubKeyMan(id, std::move(new_spk_man)); + + // Write the existing cache to disk + WalletBatch batch(GetDatabase()); + batch.WriteDescriptorCacheItems(id, desc.cache); } // Add the private keys to the descriptor From 2ea360578e80cba46789a523574f6e0f091bc0b1 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 12:35:53 -0700 Subject: [PATCH 11/15] wallet: Move listdescriptors retrieving from RPC to CWallet 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. --- src/wallet/rpc/backup.cpp | 37 ++++--------------------------------- src/wallet/wallet.cpp | 28 ++++++++++++++++++++++++++++ src/wallet/wallet.h | 15 +++++++++++++++ 3 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 74f2b6dc7d6..e7062c15720 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -503,40 +503,11 @@ RPCHelpMan listdescriptors() } LOCK(wallet->cs_wallet); - - const auto active_spk_mans = wallet->GetActiveScriptPubKeyMans(); - - struct WalletDescInfo { - std::string descriptor; - uint64_t creation_time; - bool active; - std::optional internal; - std::optional> range; - int64_t next_index; - }; - - std::vector wallet_descriptors; - for (const auto& spk_man : wallet->GetAllScriptPubKeyMans()) { - const auto desc_spk_man = dynamic_cast(spk_man); - if (!desc_spk_man) { - throw JSONRPCError(RPC_WALLET_ERROR, "Unexpected ScriptPubKey manager type."); - } - LOCK(desc_spk_man->cs_desc_man); - const auto& wallet_descriptor = desc_spk_man->GetWalletDescriptor(); - std::string descriptor; - if (!desc_spk_man->GetDescriptorString(descriptor, priv)) { - throw JSONRPCError(RPC_WALLET_ERROR, "Can't get descriptor string."); - } - const bool is_range = wallet_descriptor.descriptor->IsRange(); - wallet_descriptors.push_back({ - descriptor, - wallet_descriptor.creation_time, - active_spk_mans.count(desc_spk_man) != 0, - wallet->IsInternalScriptPubKeyMan(desc_spk_man), - is_range ? std::optional(std::make_pair(wallet_descriptor.range_start, wallet_descriptor.range_end)) : std::nullopt, - wallet_descriptor.next_index - }); + util::Result> exported = wallet->ExportDescriptors(priv); + if (!exported) { + throw JSONRPCError(RPC_WALLET_ERROR, util::ErrorString(exported).original); } + std::vector wallet_descriptors = *exported; std::sort(wallet_descriptors.begin(), wallet_descriptors.end(), [](const auto& a, const auto& b) { return a.descriptor < b.descriptor; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9c1bd1aa6e5..f90700aa6cd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4469,4 +4469,32 @@ void CWallet::WriteBestBlock() const batch.WriteBestBlock(loc); } } + +util::Result> CWallet::ExportDescriptors(bool export_private) const +{ + AssertLockHeld(cs_wallet); + std::vector wallet_descriptors; + for (const auto& spk_man : GetAllScriptPubKeyMans()) { + const auto desc_spk_man = dynamic_cast(spk_man); + if (!desc_spk_man) { + return util::Error{_("Unexpected ScriptPubKey manager type.")}; + } + LOCK(desc_spk_man->cs_desc_man); + const auto& wallet_descriptor = desc_spk_man->GetWalletDescriptor(); + std::string descriptor; + if (!desc_spk_man->GetDescriptorString(descriptor, export_private)) { + return util::Error{_("Can't get descriptor string.")}; + } + const bool is_range = wallet_descriptor.descriptor->IsRange(); + wallet_descriptors.push_back({ + descriptor, + wallet_descriptor.creation_time, + IsActiveScriptPubKeyMan(*desc_spk_man), + IsInternalScriptPubKeyMan(desc_spk_man), + is_range ? std::optional(std::make_pair(wallet_descriptor.range_start, wallet_descriptor.range_end)) : std::nullopt, + wallet_descriptor.next_index + }); + } + return wallet_descriptors; +} } // namespace wallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 108da7d91be..ca8e918a0c0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -303,6 +303,18 @@ struct CRecipient bool fSubtractFeeFromAmount; }; +// Struct containing all of the info from WalletDescriptor, except with the descriptor as a string, +// and without its ID or cache. +// Used when exporting descriptors from the wallet. +struct WalletDescInfo { + std::string descriptor; + uint64_t creation_time; + bool active; + std::optional internal; + std::optional> range; + int64_t next_index; +}; + class WalletRescanReserver; //forward declarations for ScanForWalletTransactions/RescanFromTime /** * A CWallet maintains a set of transactions and balances, and provides the ability to create new transactions. @@ -1061,6 +1073,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Find the private key for the given key id from the wallet's descriptors, if available //! Returns nullopt when no descriptor has the key or if the wallet is locked. std::optional GetKey(const CKeyID& keyid) const; + + //! Export the descriptors from this wallet so that they can be imported elsewhere + util::Result> ExportDescriptors(bool export_private) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); }; /** From 1d4a8a1d614e030eab9ee06939b497223b56006e Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 13:39:26 -0700 Subject: [PATCH 12/15] wallet: Add CWallet::ExportWatchOnly 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. --- src/wallet/wallet.cpp | 165 ++++++++++++++++++++++++++++++++++++++++++ src/wallet/wallet.h | 4 + 2 files changed, 169 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f90700aa6cd..68b0d8f9273 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4497,4 +4497,169 @@ util::Result> CWallet::ExportDescriptors(bool export } return wallet_descriptors; } + +util::Result CWallet::ExportWatchOnlyWallet(const fs::path& destination, WalletContext& context) const +{ + AssertLockHeld(cs_wallet); + + if (destination.empty()) { + return util::Error{_("Error: Export destination cannot be empty")}; + } + if (fs::exists(destination)) { + return util::Error{strprintf(_("Error: Export destination '%s' already exists"), fs::PathToString(destination))}; + } + fs::path canonical_dest = fs::canonical(destination.parent_path()); + canonical_dest /= destination.filename(); + + // Get the descriptors from this wallet + util::Result> exported = ExportDescriptors(/*export_private=*/false); + if (!exported) { + return util::Error{util::ErrorString(exported)}; + } + + // Setup DatabaseOptions to create a new sqlite database + DatabaseOptions options; + options.require_existing = false; + options.require_create = true; + options.require_format = DatabaseFormat::SQLITE; + + // Make the wallet with the same flags as this wallet, but without private keys + options.create_flags = GetWalletFlags() | WALLET_FLAG_DISABLE_PRIVATE_KEYS; + + // Make the watchonly wallet + DatabaseStatus status; + std::vector warnings; + std::string wallet_name = GetName() + "_watchonly"; + bilingual_str error; + std::unique_ptr database = MakeWalletDatabase(wallet_name, options, status, error); + if (!database) { + return util::Error{strprintf(_("Wallet file creation failed: %s"), error)}; + } + WalletContext empty_context; + empty_context.args = context.args; + std::shared_ptr watchonly_wallet = CWallet::Create(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings); + if (!watchonly_wallet) { + return util::Error{_("Error: Failed to create new watchonly wallet")}; + } + + { + LOCK(watchonly_wallet->cs_wallet); + + // Parse the descriptors and add them to the new wallet + for (const WalletDescInfo& desc_info : *exported) { + // Parse the descriptor + FlatSigningProvider keys; + std::string parse_err; + std::vector> descs = Parse(desc_info.descriptor, keys, parse_err, /*require_checksum=*/true); + assert(descs.size() == 1); // All of our descriptors should be valid, and not multipath + + // Get the range if there is one + int32_t range_start = 0; + int32_t range_end = 0; + if (desc_info.range) { + range_start = desc_info.range->first; + range_end = desc_info.range->second; + } + + WalletDescriptor w_desc(std::move(descs.at(0)), desc_info.creation_time, range_start, range_end, desc_info.next_index); + + // 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(GetScriptPubKeyMan(desc_id)); + w_desc.cache = WITH_LOCK(desc_spkm->cs_desc_man, return desc_spkm->GetWalletDescriptor().cache); + } + + // Add to the watchonly wallet + if (auto spkm_res = watchonly_wallet->AddWalletDescriptor(w_desc, keys, "", false); !spkm_res) { + return util::Error{util::ErrorString(spkm_res)}; + } + + // Set active spkms as active + if (desc_info.active) { + // Determine whether this descriptor is internal + // This is only set for active spkms + bool internal = false; + if (desc_info.internal) { + internal = *desc_info.internal; + } + watchonly_wallet->AddActiveScriptPubKeyMan(desc_id, *w_desc.descriptor->GetOutputType(), internal); + } + } + + // Copy locked coins that are persisted + for (const auto& [coin, persisted] : m_locked_coins) { + if (!persisted) continue; + watchonly_wallet->LockCoin(coin, persisted); + } + + { + // Make a WalletBatch for the watchonly_wallet so that everything else can be written atomically + WalletBatch watchonly_batch(watchonly_wallet->GetDatabase()); + if (!watchonly_batch.TxnBegin()) { + return util::Error{strprintf(_("Error: database transaction cannot be executed for new watchonly wallet %s"), watchonly_wallet->GetName())}; + } + + // Copy minversion + // Don't use SetMinVersion to account for the newly created wallet having FEATURE_LATEST + // while the source wallet doesn't. + watchonly_wallet->LoadMinVersion(GetVersion()); + watchonly_batch.WriteMinVersion(watchonly_wallet->GetVersion()); + + // Copy orderPosNext + watchonly_batch.WriteOrderPosNext(watchonly_wallet->nOrderPosNext); + + // Write the best block locator to avoid rescanning on reload + CBlockLocator best_block_locator; + { + WalletBatch local_wallet_batch(GetDatabase()); + if (!local_wallet_batch.ReadBestBlock(best_block_locator)) { + return util::Error{_("Error: Unable to read wallet's best block locator record")}; + } + } + if (!watchonly_batch.WriteBestBlock(best_block_locator)) { + return util::Error{_("Error: Unable to write watchonly wallet best block locator record")}; + } + + // Copy the transactions + for (const auto& [txid, wtx] : mapWallet) { + const CWalletTx& to_copy_wtx = wtx; + if (!watchonly_wallet->LoadToWallet(txid, [&](CWalletTx& ins_wtx, bool new_tx) EXCLUSIVE_LOCKS_REQUIRED(watchonly_wallet->cs_wallet) { + if (!new_tx) return false; + ins_wtx.SetTx(to_copy_wtx.tx); + ins_wtx.CopyFrom(to_copy_wtx); + return true; + })) { + return util::Error{strprintf(_("Error: Could not add tx %s to watchonly wallet"), txid.GetHex())}; + } + watchonly_batch.WriteTx(watchonly_wallet->mapWallet.at(txid)); + } + + // Copy address book + for (const auto& [dest, entry] : m_address_book) { + auto address{EncodeDestination(dest)}; + if (entry.purpose) watchonly_batch.WritePurpose(address, PurposeToString(*entry.purpose)); + if (entry.label) watchonly_batch.WriteName(address, *entry.label); + for (const auto& [id, request] : entry.receive_requests) { + watchonly_batch.WriteAddressReceiveRequest(dest, id, request); + } + if (entry.previously_spent) watchonly_batch.WriteAddressPreviouslySpent(dest, true); + } + + if (!watchonly_batch.TxnCommit()) { + return util::Error{_("Error: cannot commit db transaction for watchonly wallet export")}; + } + } + + // Make a backup of this wallet at the specified destination directory + watchonly_wallet->BackupWallet(fs::PathToString(canonical_dest)); + } + + // 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); + + return fs::PathToString(canonical_dest); +} } // namespace wallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ca8e918a0c0..ff53a7af93b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1076,6 +1076,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! Export the descriptors from this wallet so that they can be imported elsewhere util::Result> ExportDescriptors(bool export_private) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + //! Make a new watchonly wallet file containing the public descriptors from this wallet + //! The exported watchonly wallet file will be named and placed at the path specified in 'destination' + util::Result ExportWatchOnlyWallet(const fs::path& destination, WalletContext& context) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); }; /** From 0c9e40107b3b948c4166c31d1095c6a06b892f8d Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 14:16:28 -0700 Subject: [PATCH 13/15] wallet, rpc: Add exportwatchonlywallet RPC --- src/wallet/rpc/wallet.cpp | 41 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index e2ea4aba698..a9b21820385 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -957,6 +957,46 @@ static RPCHelpMan createwalletdescriptor() }; } +static RPCHelpMan exportwatchonlywallet() +{ + return RPCHelpMan{"exportwatchonlywallet", + "Creates a wallet file at the specified path and name containing a watchonly version " + "of the wallet. This watchonly wallet contains the wallet's public descriptors, " + "its transactions, and address book data. The watchonly wallet can be imported to " + "another node using 'restorewallet'.", + { + {"destination", RPCArg::Type::STR, RPCArg::Optional::NO, "The path to the filename the exported watchonly wallet will be saved to"}, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR, "exported_file", "The full path that the file has been exported to"}, + }, + }, + RPCExamples{ + HelpExampleCli("exportwatchonlywallet", "\"home\\user\\\"") + + HelpExampleRpc("exportwatchonlywallet", "\"home\\user\\\"") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return UniValue::VNULL; + WalletContext& context = EnsureWalletContext(request.context); + + std::string dest = request.params[0].get_str(); + + LOCK(pwallet->cs_wallet); + util::Result exported = pwallet->ExportWatchOnlyWallet(fs::PathFromString(dest), context); + if (!exported) { + throw JSONRPCError(RPC_WALLET_ERROR, util::ErrorString(exported).original); + } + UniValue out{UniValue::VOBJ}; + out.pushKV("exported_file", *exported); + return out; + } + }; +} + // addresses RPCHelpMan getaddressinfo(); RPCHelpMan getnewaddress(); @@ -1033,6 +1073,7 @@ std::span GetWalletRPCCommands() {"wallet", &createwalletdescriptor}, {"wallet", &restorewallet}, {"wallet", &encryptwallet}, + {"wallet", &exportwatchonlywallet}, {"wallet", &getaddressesbylabel}, {"wallet", &getaddressinfo}, {"wallet", &getbalance}, From 7e4d00bc81eae8275bcc3d66ea3acd8d47b6ceb0 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 12 May 2025 15:45:11 -0700 Subject: [PATCH 14/15] test: Test for exportwatchonlywallet --- test/functional/test_runner.py | 1 + test/functional/wallet_exported_watchonly.py | 287 +++++++++++++++++++ 2 files changed, 288 insertions(+) create mode 100755 test/functional/wallet_exported_watchonly.py diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 340a418e104..86d1ad98d90 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -161,6 +161,7 @@ 'wallet_fast_rescan.py', 'wallet_gethdkeys.py', 'wallet_createwalletdescriptor.py', + 'wallet_exported_watchonly.py', 'interface_zmq.py', 'rpc_invalid_address_message.py', 'rpc_validateaddress.py', diff --git a/test/functional/wallet_exported_watchonly.py b/test/functional/wallet_exported_watchonly.py new file mode 100755 index 00000000000..354522b18a8 --- /dev/null +++ b/test/functional/wallet_exported_watchonly.py @@ -0,0 +1,287 @@ +#!/usr/bin/env python3 +# Copyright (c) 2025-present The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://www.opensource.org/licenses/mit-license.php. + +import os + +from test_framework.descriptors import descsum_create +from test_framework.key import H_POINT +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_not_equal, + assert_raises_rpc_error, +) +from test_framework.wallet_util import generate_keypair + +class WalletExportedWatchOnly(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 2 + + def setup_network(self): + # Setup the nodes but don't connect them to each other + self.setup_nodes() + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def test_basic_export(self): + self.log.info("Test basic watchonly wallet export") + self.offline.createwallet("basic") + offline_wallet = self.offline.get_wallet_rpc("basic") + + # Bad RPC args + assert_raises_rpc_error(-4, "Error: Export ", offline_wallet.exportwatchonlywallet, "") + assert_raises_rpc_error(-4, "Error: Export destination '.' already exists", offline_wallet.exportwatchonlywallet, ".") + assert_raises_rpc_error(-4, f"Error: Export destination '{self.export_path}' already exists", offline_wallet.exportwatchonlywallet, self.export_path) + + # Export the watchonly wallet file and load onto online node + watchonly_export = os.path.join(self.export_path, "basic_watchonly.dat") + res = offline_wallet.exportwatchonlywallet(watchonly_export) + assert_equal(res["exported_file"], watchonly_export) + self.online.restorewallet("basic_watchonly", res["exported_file"]) + online_wallet = self.online.get_wallet_rpc("basic_watchonly") + + # Exporting watchonly from a watchonly also works + watchonly_export = os.path.join(self.export_path, "basic_watchonly2.dat") + res = offline_wallet.exportwatchonlywallet(watchonly_export) + assert_equal(res["exported_file"], watchonly_export) + self.online.restorewallet("basic_watchonly2", res["exported_file"]) + online_wallet2 = self.online.get_wallet_rpc("basic_watchonly2") + + # Verify that the wallets have the same descriptors + addr = offline_wallet.getnewaddress() + assert_equal(addr, online_wallet.getnewaddress()) + assert_equal(addr, online_wallet2.getnewaddress()) + assert_equal(offline_wallet.listdescriptors()["descriptors"], online_wallet.listdescriptors()["descriptors"]) + assert_equal(offline_wallet.listdescriptors()["descriptors"], online_wallet2.listdescriptors()["descriptors"]) + + # Expand offline's keypool so that it will recognize the scriptPubKeys it can sign + offline_wallet.keypoolrefill(100) + + # Verify that online wallet cannot spend, but offline can + self.funds.sendtoaddress(online_wallet.getnewaddress(), 10) + self.generate(self.online, 1, sync_fun=self.no_op) + assert_equal(online_wallet.getbalances()["mine"]["trusted"], 10) + assert_equal(offline_wallet.getbalances()["mine"]["trusted"], 0) + funds_addr = self.funds.getnewaddress() + send_res = online_wallet.send([{funds_addr: 5}]) + assert_equal(send_res["complete"], False) + assert "psbt" in send_res + signed_psbt = offline_wallet.walletprocesspsbt(send_res["psbt"])["psbt"] + finalized = self.online.finalizepsbt(signed_psbt)["hex"] + self.online.sendrawtransaction(finalized) + + # Verify that the change address is known to both wallets + dec_tx = self.online.decoderawtransaction(finalized) + for txout in dec_tx["vout"]: + if txout["scriptPubKey"]["address"] == funds_addr: + continue + assert_equal(online_wallet.getaddressinfo(txout["scriptPubKey"]["address"])["ismine"], True) + assert_equal(offline_wallet.getaddressinfo(txout["scriptPubKey"]["address"])["ismine"], True) + + self.generate(self.online, 1, sync_fun=self.no_op) + offline_wallet.unloadwallet() + online_wallet.unloadwallet() + + def test_export_with_address_book(self): + self.log.info("Test all address book entries appear in the exported wallet") + self.offline.createwallet("addrbook") + offline_wallet = self.offline.get_wallet_rpc("addrbook") + + # Create some address book entries + receive_addr = offline_wallet.getnewaddress(label="addrbook_receive") + send_addr = self.funds.getnewaddress() + offline_wallet.setlabel(send_addr, "addrbook_send") # Sets purpose "send" + + # Export the watchonly wallet file and load onto online node + watchonly_export = os.path.join(self.export_path, "addrbook_watchonly.dat") + res = offline_wallet.exportwatchonlywallet(watchonly_export) + assert_equal(res["exported_file"], watchonly_export) + self.online.restorewallet("addrbook_watchonly", res["exported_file"]) + online_wallet = self.online.get_wallet_rpc("addrbook_watchonly") + + # Verify the labels are in both wallets + for wallet in [online_wallet, offline_wallet]: + for purpose in ["receive", "send"]: + label = f"addrbook_{purpose}" + assert_equal(wallet.listlabels(purpose), [label]) + addr = send_addr if purpose == "send" else receive_addr + assert_equal(offline_wallet.getaddressesbylabel(label), {addr: {"purpose": purpose}}) + + offline_wallet.unloadwallet() + online_wallet.unloadwallet() + + def test_export_with_txs_and_locked_coins(self): + self.log.info("Test all transactions and locked coins appear in the exported wallet") + self.offline.createwallet("txs") + offline_wallet = self.offline.get_wallet_rpc("txs") + + # In order to make transactions in the offline wallet, briefly connect offline to online + self.connect_nodes(0, 1) + txids = [self.funds.sendtoaddress(offline_wallet.getnewaddress("funds"), i) for i in range(1, 4)] + self.generate(self.online, 1) + self.disconnect_nodes(0 ,1) + + # lock some coins + persistent_lock = [{"txid": txids[0], "vout": 0}] + temp_lock = [{"txid": txids[1], "vout": 0}] + offline_wallet.lockunspent(unlock=False, transactions=persistent_lock, persistent=True) + offline_wallet.lockunspent(unlock=False, transactions=temp_lock, persistent=False) + + # Export the watchonly wallet file and load onto online node + watchonly_export = os.path.join(self.export_path, "txs_watchonly.dat") + res = offline_wallet.exportwatchonlywallet(watchonly_export) + assert_equal(res["exported_file"], watchonly_export) + self.online.restorewallet("txs_watchonly", res["exported_file"]) + online_wallet = self.online.get_wallet_rpc("txs_watchonly") + + # Verify the transactions are in both wallets + for txid in txids: + assert_equal(online_wallet.gettransaction(txid), offline_wallet.gettransaction(txid)) + + # Verify that the persistent locked coin is locked in both wallets + assert_equal(online_wallet.listlockunspent(), persistent_lock) + assert_equal(sorted(offline_wallet.listlockunspent(), key=lambda x: x["txid"]), sorted(persistent_lock + temp_lock, key=lambda x: x["txid"])) + + offline_wallet.unloadwallet() + online_wallet.unloadwallet() + + def test_export_imported_descriptors(self): + self.log.info("Test imported descriptors are exported to the watchonly wallet") + self.offline.createwallet("imports") + offline_wallet = self.offline.get_wallet_rpc("imports") + + import_res = offline_wallet.importdescriptors( + [ + # A single key, non-ranged + {"desc": descsum_create(f"pkh({generate_keypair(wif=True)[0]})"), "timestamp": "now"}, + # hardened derivation + {"desc": descsum_create("sh(wpkh(tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg/0'/*'))"), "timestamp": "now", "active": True}, + # multisig + {"desc": descsum_create("wsh(multi(1,tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/*,tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg/*))"), "timestamp": "now", "active": True, "internal": True}, + # taproot multi scripts + {"desc": descsum_create(f"tr({H_POINT},{{pk(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/*),pk(tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg/0h/*)}})"), "timestamp": "now", "active": True}, + # miniscript + {"desc": descsum_create(f"tr({H_POINT},or_b(pk(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1/2/*),s:pk(tprv8ZgxMBicQKsPeuVhWwi6wuMQGfPKi9Li5GtX35jVNknACgqe3CY4g5xgkfDDJcmtF7o1QnxWDRYw4H5P26PXq7sbcUkEqeR4fg3Kxp2tigg/1h/2/*)))"), "timestamp": "now", "active": True, "internal": True}, + ] + ) + assert_equal(all([r["success"] for r in import_res]), True) + + # Make sure that the hardened derivation has some pregenerated keys + offline_wallet.keypoolrefill(10) + + # Export the watchonly wallet file and load onto online node + watchonly_export = os.path.join(self.export_path, "imports_watchonly.dat") + res = offline_wallet.exportwatchonlywallet(watchonly_export) + assert_equal(res["exported_file"], watchonly_export) + self.online.restorewallet("imports_watchonly", res["exported_file"]) + online_wallet = self.online.get_wallet_rpc("imports_watchonly") + + # Verify all the addresses are the same + for address_type in ["legacy", "p2sh-segwit", "bech32", "bech32m"]: + for internal in [False, True]: + if internal: + addr = offline_wallet.getrawchangeaddress(address_type=address_type) + assert_equal(addr, online_wallet.getrawchangeaddress(address_type=address_type)) + else: + addr = offline_wallet.getnewaddress(address_type=address_type) + assert_equal(addr, online_wallet.getnewaddress(address_type=address_type)) + self.funds.sendtoaddress(addr, 1) + self.generate(self.online, 1, sync_fun=self.no_op) + + # The hardened derivation should have 9 remaining addresses + for _ in range(9): + online_wallet.getnewaddress(address_type="p2sh-segwit") + assert_raises_rpc_error(-12, "No addresses available", online_wallet.getnewaddress, address_type="p2sh-segwit") + + # Verify that the offline wallet can sign and send + send_res = online_wallet.sendall([self.funds.getnewaddress()]) + assert_equal(send_res["complete"], False) + assert "psbt" in send_res + signed_psbt = offline_wallet.walletprocesspsbt(send_res["psbt"])["psbt"] + finalized = self.online.finalizepsbt(signed_psbt)["hex"] + self.online.sendrawtransaction(finalized) + + self.generate(self.online, 1, sync_fun=self.no_op) + offline_wallet.unloadwallet() + online_wallet.unloadwallet() + + def test_avoid_reuse(self): + self.log.info("Test that the avoid reuse flag appears in the exported wallet") + self.offline.createwallet(wallet_name="avoidreuse", avoid_reuse=True) + offline_wallet = self.offline.get_wallet_rpc("avoidreuse") + assert_equal(offline_wallet.getwalletinfo()["avoid_reuse"], True) + + # The avoid_reuse flag also sets some specific address book entries to track reused addresses + # In order for these to be set, a few transactions need to be made, so briefly connect offline to online + self.connect_nodes(0, 1) + addr = offline_wallet.getnewaddress() + self.funds.sendtoaddress(addr, 1) + self.generate(self.online, 1) + # Spend funds in order to mark addr as previously spent + offline_wallet.sendall([offline_wallet.getnewaddress()]) + self.funds.sendtoaddress(addr, 1) + self.generate(self.online, 1) + assert_equal(offline_wallet.listunspent()[0]["reused"], True) + self.disconnect_nodes(0 ,1) + + # Export the watchonly wallet file and load onto online node + watchonly_export = os.path.join(self.export_path, "avoidreuse_watchonly.dat") + res = offline_wallet.exportwatchonlywallet(watchonly_export) + assert_equal(res["exported_file"], watchonly_export) + self.online.restorewallet("avoidreuse_watchonly", res["exported_file"]) + online_wallet = self.online.get_wallet_rpc("avoidreuse_watchonly") + + # check avoid_reuse is still set + assert_equal(online_wallet.getwalletinfo()["avoid_reuse"], True) + assert_equal(online_wallet.listunspent()[0]["reused"], True) + + offline_wallet.unloadwallet() + online_wallet.unloadwallet() + + def test_encrypted_wallet(self): + self.log.info("Test that a watchonly wallet can be exported from a locked wallet") + self.offline.createwallet(wallet_name="encrypted", passphrase="pass") + offline_wallet = self.offline.get_wallet_rpc("encrypted") + assert_equal(offline_wallet.getwalletinfo()["unlocked_until"], 0) + + # Export the watchonly wallet file and load onto online node + watchonly_export = os.path.join(self.export_path, "encrypted_watchonly.dat") + res = offline_wallet.exportwatchonlywallet(watchonly_export) + assert_equal(res["exported_file"], watchonly_export) + self.online.restorewallet("encrypted_watchonly", res["exported_file"]) + online_wallet = self.online.get_wallet_rpc("encrypted_watchonly") + + # watchonly wallet does not have encryption because it doesn't have private keys + assert "unlocked_until" not in online_wallet.getwalletinfo() + # But it still has all of the public descriptors + assert_equal(offline_wallet.listdescriptors()["descriptors"], online_wallet.listdescriptors()["descriptors"]) + + offline_wallet.unloadwallet() + online_wallet.unloadwallet() + + def run_test(self): + self.online = self.nodes[0] + self.offline = self.nodes[1] + self.funds = self.online.get_wallet_rpc(self.default_wallet_name) + self.export_path = os.path.join(self.options.tmpdir, "exported_wallets") + os.makedirs(self.export_path, exist_ok=True) + + # Mine some blocks, and verify disconnected + self.generate(self.online, 101, sync_fun=self.no_op) + assert_not_equal(self.online.getbestblockhash(), self.offline.getbestblockhash()) + assert_equal(self.online.getblockcount(), 101) + assert_equal(self.offline.getblockcount(), 0) + + self.test_basic_export() + self.test_export_with_address_book() + self.test_export_with_txs_and_locked_coins() + self.test_export_imported_descriptors() + self.test_avoid_reuse() + self.test_encrypted_wallet() + +if __name__ == '__main__': + WalletExportedWatchOnly(__file__).main() From 7f318f08461dca9166c063d6092f3a3cf1e7f7b6 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 13 May 2025 16:26:18 -0700 Subject: [PATCH 15/15] gui: Menu action for exporting a watchonly wallet --- src/interfaces/wallet.h | 3 +++ src/qt/bitcoingui.cpp | 11 +++++++++++ src/qt/bitcoingui.h | 1 + src/wallet/interfaces.cpp | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 94869aff5af..b6b8d5d7f07 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -310,6 +310,9 @@ class Wallet //! Return pointer to internal wallet class, useful for testing. virtual wallet::CWallet* wallet() { return nullptr; } + + //! Export a watchonly wallet file. See CWallet::ExportWatchOnlyWallet + virtual util::Result exportWatchOnlyWallet(const fs::path& destination) = 0; }; //! Wallet chain client that in addition to having chain client methods for diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 9413356b412..d515e06dce0 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -371,6 +371,10 @@ 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); @@ -488,6 +492,11 @@ void BitcoinGUI::createActions() }); connect(m_mask_values_action, &QAction::toggled, this, &BitcoinGUI::setPrivacy); connect(m_mask_values_action, &QAction::toggled, this, &BitcoinGUI::enableHistoryAction); + connect(m_export_watchonly_action, &QAction::triggered, [this] { + QString destination = GUIUtil::getSaveFileName(this, tr("Save Watch-only Wallet Export"), QString(), QString(), nullptr); + if (destination.isEmpty()) return; + walletFrame->currentWalletModel()->wallet().exportWatchOnlyWallet(GUIUtil::QStringToPath(destination)); + }); } #endif // ENABLE_WALLET @@ -511,6 +520,7 @@ void BitcoinGUI::createMenuBar() file->addSeparator(); file->addAction(backupWalletAction); file->addAction(m_restore_wallet_action); + file->addAction(m_export_watchonly_action); file->addSeparator(); file->addAction(openAction); file->addAction(signMessageAction); @@ -719,6 +729,7 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller, bool s m_restore_wallet_action->setEnabled(true); m_migrate_wallet_action->setEnabled(true); m_migrate_wallet_action->setMenu(m_migrate_wallet_menu); + m_export_watchonly_action->setEnabled(true); GUIUtil::ExceptionSafeConnect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet); connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet); diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 32fb7488fb0..acb69b3f674 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -163,6 +163,7 @@ class BitcoinGUI : public QMainWindow QAction* m_mask_values_action{nullptr}; QAction* m_migrate_wallet_action{nullptr}; QMenu* m_migrate_wallet_menu{nullptr}; + QAction* m_export_watchonly_action{nullptr}; QLabel *m_wallet_selector_label = nullptr; QComboBox* m_wallet_selector = nullptr; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index ad75e69eccc..ed4acde4572 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -538,6 +538,11 @@ class WalletImpl : public Wallet } CWallet* wallet() override { return m_wallet.get(); } + util::Result exportWatchOnlyWallet(const fs::path& destination) override { + LOCK(m_wallet->cs_wallet); + return m_wallet->ExportWatchOnlyWallet(destination, m_context); + } + WalletContext& m_context; std::shared_ptr m_wallet; };