Skip to content

Commit eb81fc3

Browse files
achow101achow101
committed
Refactor: Allow LegacyScriptPubKeyMan to be null
In CWallet::LoadWallet, use this to detect and empty wallet with no keys This commit does not change behavior.
1 parent fadc08a commit eb81fc3

15 files changed

+86
-37
lines changed

‎src/bench/coin_selection.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ static void CoinSelection(benchmark::State& state)
3131
{
3232
NodeContext node;
3333
auto chain = interfaces::MakeChain(node);
34-
const CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
34+
CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
35+
wallet.SetupLegacyScriptPubKeyMan();
3536
std::vector<std::unique_ptr<CWalletTx>> wtxs;
3637
LOCK(wallet.cs_wallet);
3738

@@ -64,7 +65,7 @@ static void CoinSelection(benchmark::State& state)
6465
typedef std::set<CInputCoin> CoinSet;
6566
static NodeContext testNode;
6667
static auto testChain = interfaces::MakeChain(testNode);
67-
static const CWallet testWallet(testChain.get(), WalletLocation(), WalletDatabase::CreateDummy());
68+
static CWallet testWallet(testChain.get(), WalletLocation(), WalletDatabase::CreateDummy());
6869
std::vector<std::unique_ptr<CWalletTx>> wtxn;
6970

7071
// Copied from src/wallet/test/coinselector_tests.cpp
@@ -93,6 +94,7 @@ static CAmount make_hard_case(int utxos, std::vector<OutputGroup>& utxo_pool)
9394
static void BnBExhaustion(benchmark::State& state)
9495
{
9596
// Setup
97+
testWallet.SetupLegacyScriptPubKeyMan();
9698
std::vector<OutputGroup> utxo_pool;
9799
CoinSet selection;
98100
CAmount value_ret = 0;

‎src/bench/wallet_balance.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ static void WalletBalance(benchmark::State& state, const bool set_dirty, const b
2020
std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
2121
CWallet wallet{chain.get(), WalletLocation(), WalletDatabase::CreateMock()};
2222
{
23+
wallet.SetupLegacyScriptPubKeyMan();
2324
bool first_run;
2425
if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false);
2526
wallet.handleNotifications();

‎src/qt/test/addressbooktests.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
5959
{
6060
TestChain100Setup test;
6161
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), WalletDatabase::CreateMock());
62+
wallet->SetupLegacyScriptPubKeyMan();
6263
bool firstRun;
6364
wallet->LoadWallet(firstRun);
6465

‎src/qt/test/wallettests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ void TestGUI(interfaces::Node& node)
143143
bool firstRun;
144144
wallet->LoadWallet(firstRun);
145145
{
146-
auto spk_man = wallet->GetLegacyScriptPubKeyMan();
146+
auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan();
147147
auto locked_chain = wallet->chain().lock();
148148
LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore);
149149
wallet->SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type), "", "receive");

‎src/wallet/rpcdump.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ UniValue importprivkey(const JSONRPCRequest& request)
125125
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled");
126126
}
127127

128-
EnsureLegacyScriptPubKeyMan(*wallet);
128+
EnsureLegacyScriptPubKeyMan(*wallet, true);
129129

130130
WalletRescanReserver reserver(pwallet);
131131
bool fRescan = true;
@@ -253,7 +253,7 @@ UniValue importaddress(const JSONRPCRequest& request)
253253
},
254254
}.Check(request);
255255

256-
EnsureLegacyScriptPubKeyMan(*pwallet);
256+
EnsureLegacyScriptPubKeyMan(*pwallet, true);
257257

258258
std::string strLabel;
259259
if (!request.params[1].isNull())
@@ -454,7 +454,7 @@ UniValue importpubkey(const JSONRPCRequest& request)
454454
},
455455
}.Check(request);
456456

457-
EnsureLegacyScriptPubKeyMan(*wallet);
457+
EnsureLegacyScriptPubKeyMan(*wallet, true);
458458

459459
std::string strLabel;
460460
if (!request.params[1].isNull())
@@ -538,7 +538,7 @@ UniValue importwallet(const JSONRPCRequest& request)
538538
},
539539
}.Check(request);
540540

541-
EnsureLegacyScriptPubKeyMan(*wallet);
541+
EnsureLegacyScriptPubKeyMan(*wallet, true);
542542

543543
if (pwallet->chain().havePruned()) {
544544
// Exit early and print an error.
@@ -1334,7 +1334,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
13341334

13351335
RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ});
13361336

1337-
EnsureLegacyScriptPubKeyMan(*wallet);
1337+
EnsureLegacyScriptPubKeyMan(*wallet, true);
13381338

13391339
const UniValue& requests = mainRequest.params[0];
13401340

‎src/wallet/rpcwallet.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,13 @@ void EnsureWalletIsUnlocked(const CWallet* pwallet)
124124
}
125125
}
126126

127-
LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet)
127+
// also_create should only be set to true only when the RPC is expected to add things to a blank wallet and make it no longer blank
128+
LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create)
128129
{
129130
LegacyScriptPubKeyMan* spk_man = wallet.GetLegacyScriptPubKeyMan();
131+
if (!spk_man && also_create) {
132+
spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan();
133+
}
130134
if (!spk_man) {
131135
throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command");
132136
}
@@ -4003,7 +4007,7 @@ UniValue sethdseed(const JSONRPCRequest& request)
40034007
},
40044008
}.Check(request);
40054009

4006-
LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet);
4010+
LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet, true);
40074011

40084012
if (pwallet->chain().isInitialBlockDownload()) {
40094013
throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set a new HD seed while still in Initial Block Download");

‎src/wallet/rpcwallet.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& reques
4141
std::string HelpRequiringPassphrase(const CWallet*);
4242
void EnsureWalletIsUnlocked(const CWallet*);
4343
bool EnsureWalletIsAvailable(const CWallet*, bool avoidException);
44-
LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet);
44+
LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false);
4545

4646
UniValue getaddressinfo(const JSONRPCRequest& request);
4747
UniValue signrawtransactionwithwallet(const JSONRPCRequest& request);

‎src/wallet/test/coinselector_tests.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
136136
{
137137

138138
LOCK(testWallet.cs_wallet);
139+
testWallet.SetupLegacyScriptPubKeyMan();
139140

140141
// Setup
141142
std::vector<CInputCoin> utxo_pool;
@@ -278,6 +279,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
278279
std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock());
279280
bool firstRun;
280281
wallet->LoadWallet(firstRun);
282+
wallet->SetupLegacyScriptPubKeyMan();
281283
LOCK(wallet->cs_wallet);
282284
add_coin(*wallet, 5 * CENT, 6 * 24, false, 0, true);
283285
add_coin(*wallet, 3 * CENT, 6 * 24, false, 0, true);
@@ -299,6 +301,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test)
299301
bool bnb_used;
300302

301303
LOCK(testWallet.cs_wallet);
304+
testWallet.SetupLegacyScriptPubKeyMan();
302305

303306
// test multiple times to allow for differences in the shuffle order
304307
for (int i = 0; i < RUN_TESTS; i++)
@@ -578,6 +581,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
578581
bool bnb_used;
579582

580583
LOCK(testWallet.cs_wallet);
584+
testWallet.SetupLegacyScriptPubKeyMan();
581585

582586
empty_wallet();
583587

@@ -596,6 +600,8 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
596600
// Tests that with the ideal conditions, the coin selector will always be able to find a solution that can pay the target value
597601
BOOST_AUTO_TEST_CASE(SelectCoins_test)
598602
{
603+
testWallet.SetupLegacyScriptPubKeyMan();
604+
599605
// Random generator stuff
600606
std::default_random_engine generator;
601607
std::exponential_distribution<double> distribution (100);

‎src/wallet/test/ismine_tests.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
3636
// P2PK compressed
3737
{
3838
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
39+
keystore.SetupLegacyScriptPubKeyMan();
3940
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
4041
scriptPubKey = GetScriptForRawPubKey(pubkeys[0]);
4142

@@ -52,6 +53,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
5253
// P2PK uncompressed
5354
{
5455
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
56+
keystore.SetupLegacyScriptPubKeyMan();
5557
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
5658
scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey);
5759

@@ -68,6 +70,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
6870
// P2PKH compressed
6971
{
7072
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
73+
keystore.SetupLegacyScriptPubKeyMan();
7174
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
7275
scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0]));
7376

@@ -84,6 +87,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
8487
// P2PKH uncompressed
8588
{
8689
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
90+
keystore.SetupLegacyScriptPubKeyMan();
8791
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
8892
scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey));
8993

@@ -100,6 +104,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
100104
// P2SH
101105
{
102106
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
107+
keystore.SetupLegacyScriptPubKeyMan();
103108
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
104109

105110
CScript redeemScript = GetScriptForDestination(PKHash(pubkeys[0]));
@@ -123,6 +128,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
123128
// (P2PKH inside) P2SH inside P2SH (invalid)
124129
{
125130
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
131+
keystore.SetupLegacyScriptPubKeyMan();
126132
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
127133

128134
CScript redeemscript_inner = GetScriptForDestination(PKHash(pubkeys[0]));
@@ -140,6 +146,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
140146
// (P2PKH inside) P2SH inside P2WSH (invalid)
141147
{
142148
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
149+
keystore.SetupLegacyScriptPubKeyMan();
143150
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
144151

145152
CScript redeemscript = GetScriptForDestination(PKHash(pubkeys[0]));
@@ -157,6 +164,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
157164
// P2WPKH inside P2WSH (invalid)
158165
{
159166
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
167+
keystore.SetupLegacyScriptPubKeyMan();
160168
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
161169

162170
CScript witnessscript = GetScriptForDestination(WitnessV0KeyHash(PKHash(pubkeys[0])));
@@ -172,6 +180,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
172180
// (P2PKH inside) P2WSH inside P2WSH (invalid)
173181
{
174182
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
183+
keystore.SetupLegacyScriptPubKeyMan();
175184
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
176185

177186
CScript witnessscript_inner = GetScriptForDestination(PKHash(pubkeys[0]));
@@ -189,6 +198,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
189198
// P2WPKH compressed
190199
{
191200
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
201+
keystore.SetupLegacyScriptPubKeyMan();
192202
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
193203
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
194204

@@ -203,6 +213,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
203213
// P2WPKH uncompressed
204214
{
205215
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
216+
keystore.SetupLegacyScriptPubKeyMan();
206217
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
207218
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
208219

@@ -221,6 +232,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
221232
// scriptPubKey multisig
222233
{
223234
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
235+
keystore.SetupLegacyScriptPubKeyMan();
224236
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
225237

226238
scriptPubKey = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
@@ -251,6 +263,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
251263
// P2SH multisig
252264
{
253265
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
266+
keystore.SetupLegacyScriptPubKeyMan();
254267
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
255268
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
256269
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[1]));
@@ -271,6 +284,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
271284
// P2WSH multisig with compressed keys
272285
{
273286
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
287+
keystore.SetupLegacyScriptPubKeyMan();
274288
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
275289
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
276290
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[1]));
@@ -296,6 +310,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
296310
// P2WSH multisig with uncompressed key
297311
{
298312
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
313+
keystore.SetupLegacyScriptPubKeyMan();
299314
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
300315
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
301316
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[1]));
@@ -321,6 +336,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
321336
// P2WSH multisig wrapped in P2SH
322337
{
323338
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
339+
keystore.SetupLegacyScriptPubKeyMan();
324340
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
325341

326342
CScript witnessScript = GetScriptForMultisig(2, {pubkeys[0], pubkeys[1]});
@@ -347,6 +363,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
347363
// OP_RETURN
348364
{
349365
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
366+
keystore.SetupLegacyScriptPubKeyMan();
350367
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
351368
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
352369

@@ -360,6 +377,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
360377
// witness unspendable
361378
{
362379
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
380+
keystore.SetupLegacyScriptPubKeyMan();
363381
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
364382
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
365383

@@ -373,6 +391,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
373391
// witness unknown
374392
{
375393
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
394+
keystore.SetupLegacyScriptPubKeyMan();
376395
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
377396
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
378397

@@ -386,6 +405,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
386405
// Nonstandard
387406
{
388407
CWallet keystore(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
408+
keystore.SetupLegacyScriptPubKeyMan();
389409
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
390410
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
391411

‎src/wallet/test/psbt_wallet_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ BOOST_FIXTURE_TEST_SUITE(psbt_wallet_tests, WalletTestingSetup)
1616

1717
BOOST_AUTO_TEST_CASE(psbt_updater_test)
1818
{
19-
auto spk_man = m_wallet.GetLegacyScriptPubKeyMan();
19+
auto spk_man = m_wallet.GetOrCreateLegacyScriptPubKeyMan();
2020
LOCK2(m_wallet.cs_wallet, spk_man->cs_KeyStore);
2121

2222
// Create prevtxs and add to wallet

0 commit comments

Comments
 (0)