Skip to content

Commit 6378dee

Browse files
ryanofskyvijaydasmp
authored andcommitted
Merge 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
1 parent fe31eca commit 6378dee

5 files changed

Lines changed: 186 additions & 2 deletions

File tree

src/wallet/rpc/wallet.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ static RPCHelpMan getwalletinfo()
147147
}},
148148
{RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"},
149149
{RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"},
150+
{RPCResult::Type::BOOL, "blank", "Whether this wallet intentionally does not contain any keys, scripts, or descriptors"},
150151
RESULT_LAST_PROCESSED_BLOCK,
151152
},
152153
},
@@ -226,6 +227,7 @@ static RPCHelpMan getwalletinfo()
226227
}
227228
obj.pushKV("descriptors", pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
228229
obj.pushKV("external_signer", pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER));
230+
obj.pushKV("blank", pwallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET));
229231

230232
AppendLastProcessedBlock(obj, *pwallet);
231233
return obj;

src/wallet/scriptpubkeyman.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -899,12 +899,12 @@ bool LegacyScriptPubKeyMan::AddKeyPubKeyWithDB(WalletBatch& batch, const CKey& s
899899
RemoveWatchOnly(script);
900900
}
901901

902+
m_storage.UnsetBlankWalletFlag(batch);
902903
if (!m_storage.HasEncryptionKeys()) {
903904
return batch.WriteKey(pubkey,
904905
secret.GetPrivKey(),
905906
mapKeyMetadata[pubkey.GetID()]);
906907
}
907-
m_storage.UnsetBlankWalletFlag(batch);
908908
return true;
909909
}
910910

src/wallet/walletutil.h

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

test/functional/test_runner.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@
168168
'feature_abortnode.py',
169169
# vv Tests less than 30s vv
170170
'rpc_quorum.py',
171+
'wallet_blank.py --legacy-wallet',
172+
'wallet_blank.py --descriptors',
171173
'wallet_keypool_topup.py --legacy-wallet',
172174
'wallet_keypool_topup.py --descriptors',
173175
'wallet_fast_rescan.py --descriptors',

test/functional/wallet_blank.py

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
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+
[{
88+
"desc": ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR,
89+
"timestamp": "now",
90+
}])
91+
assert_equal(wallet.getwalletinfo()["blank"], False)
92+
93+
def test_importdescriptors(self):
94+
if not self.options.descriptors:
95+
return
96+
self.log.info("Test that importdescriptors preserves the blank flag")
97+
self.nodes[0].createwallet(wallet_name="idesc", disable_private_keys=True, blank=True)
98+
wallet = self.nodes[0].get_wallet_rpc("idesc")
99+
info = wallet.getwalletinfo()
100+
assert_equal(info["descriptors"], True)
101+
assert_equal(info["blank"], True)
102+
wallet.importdescriptors(
103+
[{
104+
"desc": ADDRESS_BCRT1_UNSPENDABLE_DESCRIPTOR,
105+
"timestamp": "now",
106+
}])
107+
assert_equal(wallet.getwalletinfo()["blank"], True)
108+
109+
def test_importwallet(self):
110+
if self.options.descriptors:
111+
return
112+
self.log.info("Test that importwallet unsets the blank flag")
113+
def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
114+
115+
self.nodes[0].createwallet(wallet_name="iwallet", blank=True)
116+
wallet = self.nodes[0].get_wallet_rpc("iwallet")
117+
info = wallet.getwalletinfo()
118+
assert_equal(info["descriptors"], False)
119+
assert_equal(info["blank"], True)
120+
121+
wallet_dump_path = os.path.join(self.nodes[0].datadir, "wallet.dump")
122+
def_wallet.dumpwallet(wallet_dump_path)
123+
124+
wallet.importwallet(wallet_dump_path)
125+
assert_equal(wallet.getwalletinfo()["blank"], False)
126+
127+
def test_encrypt_legacy(self):
128+
if self.options.descriptors:
129+
return
130+
self.log.info("Test that encrypting a blank legacy wallet preserves the blank flag and does not generate a seed")
131+
self.nodes[0].createwallet(wallet_name="encblanklegacy", blank=True)
132+
wallet = self.nodes[0].get_wallet_rpc("encblanklegacy")
133+
134+
info = wallet.getwalletinfo()
135+
assert_equal(info["descriptors"], False)
136+
assert_equal(info["blank"], True)
137+
assert "hdseedid" not in info
138+
139+
wallet.encryptwallet("pass")
140+
info = wallet.getwalletinfo()
141+
assert_equal(info["blank"], True)
142+
assert "hdseedid" not in info
143+
144+
def test_encrypt_descriptors(self):
145+
if not self.options.descriptors:
146+
return
147+
self.log.info("Test that encrypting a blank descriptor wallet preserves the blank flag and descriptors remain the same")
148+
self.nodes[0].createwallet(wallet_name="encblankdesc", blank=True)
149+
wallet = self.nodes[0].get_wallet_rpc("encblankdesc")
150+
151+
info = wallet.getwalletinfo()
152+
assert_equal(info["descriptors"], True)
153+
assert_equal(info["blank"], True)
154+
descs = wallet.listdescriptors()
155+
156+
wallet.encryptwallet("pass")
157+
assert_equal(wallet.getwalletinfo()["blank"], True)
158+
assert_equal(descs, wallet.listdescriptors())
159+
160+
def run_test(self):
161+
self.test_importaddress()
162+
self.test_importpubkey()
163+
self.test_importprivkey()
164+
self.test_importmulti()
165+
self.test_importdescriptors()
166+
self.test_importwallet()
167+
self.test_encrypt_legacy()
168+
self.test_encrypt_descriptors()
169+
170+
171+
if __name__ == '__main__':
172+
WalletBlankTest().main()

0 commit comments

Comments
 (0)