From e9379f1ffa7a4eebce397f1150317e840655e021 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 18 Jul 2022 12:15:53 -0400 Subject: [PATCH 1/3] rpc, wallet: Include information about blank flag This allows us to test that the blank flag is being set appropriately. --- src/wallet/rpc/wallet.cpp | 2 + test/functional/test_runner.py | 2 + test/functional/wallet_blank.py | 172 ++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+) create mode 100755 test/functional/wallet_blank.py diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index d728b2fb96..d71359508d 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -68,6 +68,7 @@ static RPCHelpMan getwalletinfo() }, /*skip_type_check=*/true}, {RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"}, {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"}, RESULT_LAST_PROCESSED_BLOCK, }}, }, @@ -130,6 +131,7 @@ static RPCHelpMan getwalletinfo() } obj.pushKV("descriptors", pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); obj.pushKV("external_signer", pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)); + obj.pushKV("blank", pwallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET)); AppendLastProcessedBlock(obj, *pwallet); return obj; diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 77e4dab6e4..4fc3d4a4df 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -166,6 +166,8 @@ BASE_SCRIPTS = [ 'p2p_compactblocks_blocksonly.py', 'wallet_hd.py --legacy-wallet', 'wallet_hd.py --descriptors', + 'wallet_blank.py --legacy-wallet', + 'wallet_blank.py --descriptors', 'wallet_keypool_topup.py --legacy-wallet', 'wallet_keypool_topup.py --descriptors', 'wallet_fast_rescan.py --descriptors', diff --git a/test/functional/wallet_blank.py b/test/functional/wallet_blank.py new file mode 100755 index 0000000000..2767e5ffd7 --- /dev/null +++ b/test/functional/wallet_blank.py @@ -0,0 +1,172 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 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.test_framework import BitcoinTestFramework +from test_framework.address import ( + ADDRESS_BCRT1_UNSPENDABLE, + ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR, +) +from test_framework.key import ECKey +from test_framework.util import ( + assert_equal, +) +from test_framework.wallet_util import bytes_to_wif + + +class WalletBlankTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def add_options(self, options): + self.add_wallet_options(options) + + def test_importaddress(self): + if self.options.descriptors: + return + self.log.info("Test that importaddress unsets the blank flag") + self.nodes[0].createwallet(wallet_name="iaddr", disable_private_keys=True, blank=True) + wallet = self.nodes[0].get_wallet_rpc("iaddr") + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], False) + assert_equal(info["blank"], True) + wallet.importaddress(ADDRESS_BCRT1_UNSPENDABLE) + assert_equal(wallet.getwalletinfo()["blank"], False) + + def test_importpubkey(self): + if self.options.descriptors: + return + self.log.info("Test that importpubkey unsets the blank flag") + for i, comp in enumerate([True, False]): + self.nodes[0].createwallet(wallet_name=f"ipub{i}", disable_private_keys=True, blank=True) + wallet = self.nodes[0].get_wallet_rpc(f"ipub{i}") + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], False) + assert_equal(info["blank"], True) + + eckey = ECKey() + eckey.generate(compressed=comp) + + wallet.importpubkey(eckey.get_pubkey().get_bytes().hex()) + assert_equal(wallet.getwalletinfo()["blank"], False) + + def test_importprivkey(self): + if self.options.descriptors: + return + self.log.info("Test that importprivkey unsets the blank flag") + for i, comp in enumerate([True, False]): + self.nodes[0].createwallet(wallet_name=f"ipriv{i}", blank=True) + wallet = self.nodes[0].get_wallet_rpc(f"ipriv{i}") + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], False) + assert_equal(info["blank"], True) + + eckey = ECKey() + eckey.generate(compressed=comp) + wif = bytes_to_wif(eckey.get_bytes(), eckey.is_compressed) + + wallet.importprivkey(wif) + # FIXME: A bug results in blank remaining set + assert_equal(wallet.getwalletinfo()["blank"], True) + + def test_importmulti(self): + if self.options.descriptors: + return + self.log.info("Test that importmulti unsets the blank flag") + self.nodes[0].createwallet(wallet_name="imulti", disable_private_keys=True, blank=True) + wallet = self.nodes[0].get_wallet_rpc("imulti") + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], False) + assert_equal(info["blank"], True) + wallet.importmulti([{ + "desc": ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR, + "timestamp": "now", + }]) + assert_equal(wallet.getwalletinfo()["blank"], False) + + def test_importdescriptors(self): + if not self.options.descriptors: + return + self.log.info("Test that importdescriptors preserves the blank flag") + self.nodes[0].createwallet(wallet_name="idesc", disable_private_keys=True, blank=True) + wallet = self.nodes[0].get_wallet_rpc("idesc") + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], True) + assert_equal(info["blank"], True) + wallet.importdescriptors([{ + "desc": ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR, + "timestamp": "now", + }]) + assert_equal(wallet.getwalletinfo()["blank"], True) + + def test_importwallet(self): + if self.options.descriptors: + return + self.log.info("Test that importwallet unsets the blank flag") + def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + + self.nodes[0].createwallet(wallet_name="iwallet", blank=True) + wallet = self.nodes[0].get_wallet_rpc("iwallet") + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], False) + assert_equal(info["blank"], True) + + wallet_dump_path = os.path.join(self.nodes[0].datadir, "wallet.dump") + def_wallet.dumpwallet(wallet_dump_path) + + wallet.importwallet(wallet_dump_path) + # FIXME: A bug results in blank remaining set + assert_equal(wallet.getwalletinfo()["blank"], True) + + def test_encrypt_legacy(self): + if self.options.descriptors: + return + self.log.info("Test that encrypting a blank legacy wallet preserves the blank flag and does not generate a seed") + self.nodes[0].createwallet(wallet_name="encblanklegacy", blank=True) + wallet = self.nodes[0].get_wallet_rpc("encblanklegacy") + + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], False) + assert_equal(info["blank"], True) + assert "hdseedid" not in info + + wallet.encryptwallet("pass") + info = wallet.getwalletinfo() + assert_equal(info["blank"], True) + assert "hdseedid" not in info + + def test_encrypt_descriptors(self): + if not self.options.descriptors: + return + self.log.info("Test that encrypting a blank descriptor wallet preserves the blank flag and descriptors remain the same") + self.nodes[0].createwallet(wallet_name="encblankdesc", blank=True) + wallet = self.nodes[0].get_wallet_rpc("encblankdesc") + + info = wallet.getwalletinfo() + assert_equal(info["descriptors"], True) + assert_equal(info["blank"], True) + descs = wallet.listdescriptors() + + wallet.encryptwallet("pass") + assert_equal(wallet.getwalletinfo()["blank"], True) + assert_equal(descs, wallet.listdescriptors()) + + def run_test(self): + self.test_importaddress() + self.test_importpubkey() + self.test_importprivkey() + self.test_importmulti() + self.test_importdescriptors() + self.test_importwallet() + self.test_encrypt_legacy() + self.test_encrypt_descriptors() + + +if __name__ == '__main__': + WalletBlankTest().main() From 43310200dce8d450ae5808824af788cefaa5d6db Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 18 Jul 2022 13:31:19 -0400 Subject: [PATCH 2/3] wallet: Ensure that the blank wallet flag is unset after imports --- src/wallet/scriptpubkeyman.cpp | 2 +- test/functional/wallet_blank.py | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 62bd0c06cd..8c34c45e49 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -755,12 +755,12 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyWithDB(WalletBatch& batch, const CKey& s RemoveWatchOnly(script); } + m_storage.UnsetBlankWalletFlag(batch); if (!m_storage.HasEncryptionKeys()) { return batch.WriteKey(pubkey, secret.GetPrivKey(), mapKeyMetadata[pubkey.GetID()]); } - m_storage.UnsetBlankWalletFlag(batch); return true; } diff --git a/test/functional/wallet_blank.py b/test/functional/wallet_blank.py index 2767e5ffd7..eda3fda35b 100755 --- a/test/functional/wallet_blank.py +++ b/test/functional/wallet_blank.py @@ -72,8 +72,7 @@ class WalletBlankTest(BitcoinTestFramework): wif = bytes_to_wif(eckey.get_bytes(), eckey.is_compressed) wallet.importprivkey(wif) - # FIXME: A bug results in blank remaining set - assert_equal(wallet.getwalletinfo()["blank"], True) + assert_equal(wallet.getwalletinfo()["blank"], False) def test_importmulti(self): if self.options.descriptors: @@ -121,8 +120,7 @@ class WalletBlankTest(BitcoinTestFramework): def_wallet.dumpwallet(wallet_dump_path) wallet.importwallet(wallet_dump_path) - # FIXME: A bug results in blank remaining set - assert_equal(wallet.getwalletinfo()["blank"], True) + assert_equal(wallet.getwalletinfo()["blank"], False) def test_encrypt_legacy(self): if self.options.descriptors: From cdba23db353a1beff831ff4fc83d01ed64e8c2a9 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 8 Jun 2023 07:03:22 -0400 Subject: [PATCH 3/3] wallet: Document blank flag use in descriptor wallets --- src/wallet/walletutil.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index f639078de5..37847b64ef 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -53,10 +53,18 @@ enum WalletFlags : uint64_t { //! Flag set when a wallet contains no HD seed and no private keys, scripts, //! addresses, and other watch only things, and is therefore "blank." //! - //! The only function this flag serves is to distinguish a blank wallet from + //! The main function this flag serves is to distinguish a blank wallet from //! a newly created wallet when the wallet database is loaded, to avoid //! initialization that should only happen on first run. //! + //! A secondary function of this flag, which applies to descriptor wallets + //! only, is to serve as an ongoing indication that descriptors in the + //! wallet should be created manually, and that the wallet should not + //! generate automatically generate new descriptors if it is later + //! encrypted. To support this behavior, descriptor wallets unlike legacy + //! wallets do not automatically unset the BLANK flag when things are + //! imported. + //! //! This flag is also a mandatory flag to prevent previous versions of //! bitcoin from opening the wallet, thinking it was newly created, and //! then improperly reinitializing it.