Skip to content

Commit 6663c80

Browse files
committed
Merge bitcoin/bitcoin#25634: wallet, tests: Expand and test when the blank wallet flag should be un/set
cdba23d wallet: Document blank flag use in descriptor wallets (Ryan Ofsky) 4331020 wallet: Ensure that the blank wallet flag is unset after imports (Andrew Chow) e9379f1 rpc, wallet: Include information about blank flag (Andrew Chow) Pull request description: The `blank` wallet flag is used to indicate that the wallet intentionally does not have any keys, scripts, or descriptors, and it prevents the automatic generation of those things for such a wallet. Once the wallet contains any of those data, it is unnecessary, and possibly incorrect, to have `blank` set. This PR fixes a few places where this was not properly happening. It also adds a test for this unset behavior. ACKs for top commit: S3RK: reACK cdba23d ryanofsky: Code review ACK cdba23d. Only change since last review is dropping the commit which makes createwallet RPC set BLANK flag automatically when DISABLE_PRIVATE_KEYS flag is set Tree-SHA512: 85bc2a9754df0531575d5c8f4ad7e8f38dcd50083dc29b3283dacf56feae842e81f34654c5e1781f2dadb0560ff80e454bbc8ca3b2d1fab1b236499ae9abd7da
2 parents 427853a + cdba23d commit 6663c80

File tree

5 files changed

+184
-2
lines changed

5 files changed

+184
-2
lines changed

src/wallet/rpc/wallet.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ static RPCHelpMan getwalletinfo()
6868
}, /*skip_type_check=*/true},
6969
{RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"},
7070
{RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"},
71+
{RPCResult::Type::BOOL, "blank", "Whether this wallet intentionally does not contain any keys, scripts, or descriptors"},
7172
RESULT_LAST_PROCESSED_BLOCK,
7273
}},
7374
},
@@ -130,6 +131,7 @@ static RPCHelpMan getwalletinfo()
130131
}
131132
obj.pushKV("descriptors", pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
132133
obj.pushKV("external_signer", pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER));
134+
obj.pushKV("blank", pwallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET));
133135

134136
AppendLastProcessedBlock(obj, *pwallet);
135137
return obj;

src/wallet/scriptpubkeyman.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,12 +757,12 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyWithDB(WalletBatch& batch, const CKey& s
757757
RemoveWatchOnly(script);
758758
}
759759

760+
m_storage.UnsetBlankWalletFlag(batch);
760761
if (!m_storage.HasEncryptionKeys()) {
761762
return batch.WriteKey(pubkey,
762763
secret.GetPrivKey(),
763764
mapKeyMetadata[pubkey.GetID()]);
764765
}
765-
m_storage.UnsetBlankWalletFlag(batch);
766766
return true;
767767
}
768768

src/wallet/walletutil.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,18 @@ enum WalletFlags : uint64_t {
5353
//! Flag set when a wallet contains no HD seed and no private keys, scripts,
5454
//! addresses, and other watch only things, and is therefore "blank."
5555
//!
56-
//! The only function this flag serves is to distinguish a blank wallet from
56+
//! The main function this flag serves is to distinguish a blank wallet from
5757
//! a newly created wallet when the wallet database is loaded, to avoid
5858
//! initialization that should only happen on first run.
5959
//!
60+
//! A secondary function of this flag, which applies to descriptor wallets
61+
//! only, is to serve as an ongoing indication that descriptors in the
62+
//! wallet should be created manually, and that the wallet should not
63+
//! generate automatically generate new descriptors if it is later
64+
//! encrypted. To support this behavior, descriptor wallets unlike legacy
65+
//! wallets do not automatically unset the BLANK flag when things are
66+
//! imported.
67+
//!
6068
//! This flag is also a mandatory flag to prevent previous versions of
6169
//! bitcoin from opening the wallet, thinking it was newly created, and
6270
//! then improperly reinitializing it.

test/functional/test_runner.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@
166166
'p2p_compactblocks_blocksonly.py',
167167
'wallet_hd.py --legacy-wallet',
168168
'wallet_hd.py --descriptors',
169+
'wallet_blank.py --legacy-wallet',
170+
'wallet_blank.py --descriptors',
169171
'wallet_keypool_topup.py --legacy-wallet',
170172
'wallet_keypool_topup.py --descriptors',
171173
'wallet_fast_rescan.py --descriptors',

test/functional/wallet_blank.py

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2022 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or https://www.opensource.org/licenses/mit-license.php.
5+
6+
import os
7+
8+
from test_framework.test_framework import BitcoinTestFramework
9+
from test_framework.address import (
10+
ADDRESS_BCRT1_UNSPENDABLE,
11+
ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR,
12+
)
13+
from test_framework.key import ECKey
14+
from test_framework.util import (
15+
assert_equal,
16+
)
17+
from test_framework.wallet_util import bytes_to_wif
18+
19+
20+
class WalletBlankTest(BitcoinTestFramework):
21+
def set_test_params(self):
22+
self.num_nodes = 1
23+
24+
def skip_test_if_missing_module(self):
25+
self.skip_if_no_wallet()
26+
27+
def add_options(self, options):
28+
self.add_wallet_options(options)
29+
30+
def test_importaddress(self):
31+
if self.options.descriptors:
32+
return
33+
self.log.info("Test that importaddress unsets the blank flag")
34+
self.nodes[0].createwallet(wallet_name="iaddr", disable_private_keys=True, blank=True)
35+
wallet = self.nodes[0].get_wallet_rpc("iaddr")
36+
info = wallet.getwalletinfo()
37+
assert_equal(info["descriptors"], False)
38+
assert_equal(info["blank"], True)
39+
wallet.importaddress(ADDRESS_BCRT1_UNSPENDABLE)
40+
assert_equal(wallet.getwalletinfo()["blank"], False)
41+
42+
def test_importpubkey(self):
43+
if self.options.descriptors:
44+
return
45+
self.log.info("Test that importpubkey unsets the blank flag")
46+
for i, comp in enumerate([True, False]):
47+
self.nodes[0].createwallet(wallet_name=f"ipub{i}", disable_private_keys=True, blank=True)
48+
wallet = self.nodes[0].get_wallet_rpc(f"ipub{i}")
49+
info = wallet.getwalletinfo()
50+
assert_equal(info["descriptors"], False)
51+
assert_equal(info["blank"], True)
52+
53+
eckey = ECKey()
54+
eckey.generate(compressed=comp)
55+
56+
wallet.importpubkey(eckey.get_pubkey().get_bytes().hex())
57+
assert_equal(wallet.getwalletinfo()["blank"], False)
58+
59+
def test_importprivkey(self):
60+
if self.options.descriptors:
61+
return
62+
self.log.info("Test that importprivkey unsets the blank flag")
63+
for i, comp in enumerate([True, False]):
64+
self.nodes[0].createwallet(wallet_name=f"ipriv{i}", blank=True)
65+
wallet = self.nodes[0].get_wallet_rpc(f"ipriv{i}")
66+
info = wallet.getwalletinfo()
67+
assert_equal(info["descriptors"], False)
68+
assert_equal(info["blank"], True)
69+
70+
eckey = ECKey()
71+
eckey.generate(compressed=comp)
72+
wif = bytes_to_wif(eckey.get_bytes(), eckey.is_compressed)
73+
74+
wallet.importprivkey(wif)
75+
assert_equal(wallet.getwalletinfo()["blank"], False)
76+
77+
def test_importmulti(self):
78+
if self.options.descriptors:
79+
return
80+
self.log.info("Test that importmulti unsets the blank flag")
81+
self.nodes[0].createwallet(wallet_name="imulti", disable_private_keys=True, blank=True)
82+
wallet = self.nodes[0].get_wallet_rpc("imulti")
83+
info = wallet.getwalletinfo()
84+
assert_equal(info["descriptors"], False)
85+
assert_equal(info["blank"], True)
86+
wallet.importmulti([{
87+
"desc": ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR,
88+
"timestamp": "now",
89+
}])
90+
assert_equal(wallet.getwalletinfo()["blank"], False)
91+
92+
def test_importdescriptors(self):
93+
if not self.options.descriptors:
94+
return
95+
self.log.info("Test that importdescriptors preserves the blank flag")
96+
self.nodes[0].createwallet(wallet_name="idesc", disable_private_keys=True, blank=True)
97+
wallet = self.nodes[0].get_wallet_rpc("idesc")
98+
info = wallet.getwalletinfo()
99+
assert_equal(info["descriptors"], True)
100+
assert_equal(info["blank"], True)
101+
wallet.importdescriptors([{
102+
"desc": ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR,
103+
"timestamp": "now",
104+
}])
105+
assert_equal(wallet.getwalletinfo()["blank"], True)
106+
107+
def test_importwallet(self):
108+
if self.options.descriptors:
109+
return
110+
self.log.info("Test that importwallet unsets the blank flag")
111+
def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
112+
113+
self.nodes[0].createwallet(wallet_name="iwallet", blank=True)
114+
wallet = self.nodes[0].get_wallet_rpc("iwallet")
115+
info = wallet.getwalletinfo()
116+
assert_equal(info["descriptors"], False)
117+
assert_equal(info["blank"], True)
118+
119+
wallet_dump_path = os.path.join(self.nodes[0].datadir, "wallet.dump")
120+
def_wallet.dumpwallet(wallet_dump_path)
121+
122+
wallet.importwallet(wallet_dump_path)
123+
assert_equal(wallet.getwalletinfo()["blank"], False)
124+
125+
def test_encrypt_legacy(self):
126+
if self.options.descriptors:
127+
return
128+
self.log.info("Test that encrypting a blank legacy wallet preserves the blank flag and does not generate a seed")
129+
self.nodes[0].createwallet(wallet_name="encblanklegacy", blank=True)
130+
wallet = self.nodes[0].get_wallet_rpc("encblanklegacy")
131+
132+
info = wallet.getwalletinfo()
133+
assert_equal(info["descriptors"], False)
134+
assert_equal(info["blank"], True)
135+
assert "hdseedid" not in info
136+
137+
wallet.encryptwallet("pass")
138+
info = wallet.getwalletinfo()
139+
assert_equal(info["blank"], True)
140+
assert "hdseedid" not in info
141+
142+
def test_encrypt_descriptors(self):
143+
if not self.options.descriptors:
144+
return
145+
self.log.info("Test that encrypting a blank descriptor wallet preserves the blank flag and descriptors remain the same")
146+
self.nodes[0].createwallet(wallet_name="encblankdesc", blank=True)
147+
wallet = self.nodes[0].get_wallet_rpc("encblankdesc")
148+
149+
info = wallet.getwalletinfo()
150+
assert_equal(info["descriptors"], True)
151+
assert_equal(info["blank"], True)
152+
descs = wallet.listdescriptors()
153+
154+
wallet.encryptwallet("pass")
155+
assert_equal(wallet.getwalletinfo()["blank"], True)
156+
assert_equal(descs, wallet.listdescriptors())
157+
158+
def run_test(self):
159+
self.test_importaddress()
160+
self.test_importpubkey()
161+
self.test_importprivkey()
162+
self.test_importmulti()
163+
self.test_importdescriptors()
164+
self.test_importwallet()
165+
self.test_encrypt_legacy()
166+
self.test_encrypt_descriptors()
167+
168+
169+
if __name__ == '__main__':
170+
WalletBlankTest().main()

0 commit comments

Comments
 (0)