Skip to content

Commit 288b4ff

Browse files
ryanofskyryanofsky
committed
Remove WalletLocation class
This removes a source of complexity and indirection that makes it harder to understand path checking code. Path checks will be simplified in upcoming commits. There is no change in behavior in this commit other than a slightly more descriptive error message in `loadwallet` if the default "" wallet can't be found. (The error message is improved more in upcoming commit "wallet: Remove path checking code from loadwallet RPC".)
1 parent a0a422c commit 288b4ff

18 files changed

+86
-122
lines changed

‎src/bench/coin_selection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ static void CoinSelection(benchmark::Bench& bench)
3131
{
3232
NodeContext node;
3333
auto chain = interfaces::MakeChain(node);
34-
CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
34+
CWallet wallet(chain.get(), "", CreateDummyWalletDatabase());
3535
wallet.SetupLegacyScriptPubKeyMan();
3636
std::vector<std::unique_ptr<CWalletTx>> wtxs;
3737
LOCK(wallet.cs_wallet);
@@ -65,7 +65,7 @@ static void CoinSelection(benchmark::Bench& bench)
6565
typedef std::set<CInputCoin> CoinSet;
6666
static NodeContext testNode;
6767
static auto testChain = interfaces::MakeChain(testNode);
68-
static CWallet testWallet(testChain.get(), WalletLocation(), CreateDummyWalletDatabase());
68+
static CWallet testWallet(testChain.get(), "", CreateDummyWalletDatabase());
6969
std::vector<std::unique_ptr<CWalletTx>> wtxn;
7070

7171
// Copied from src/wallet/test/coinselector_tests.cpp

‎src/bench/wallet_balance.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b
2626

2727
NodeContext node;
2828
std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
29-
CWallet wallet{chain.get(), WalletLocation(), CreateMockWalletDatabase()};
29+
CWallet wallet{chain.get(), "", CreateMockWalletDatabase()};
3030
{
3131
wallet.SetupLegacyScriptPubKeyMan();
3232
bool first_run;

‎src/interfaces/wallet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ class WalletClientImpl : public WalletClient
522522
}
523523
std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
524524
{
525-
return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), true /* load_on_start */, error, warnings));
525+
return MakeWallet(LoadWallet(*m_context.chain, name, true /* load_on_start */, error, warnings));
526526
}
527527
std::string getWalletDir() override
528528
{

‎src/qt/test/addressbooktests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
6161
{
6262
TestChain100Setup test;
6363
node.setContext(&test.m_node);
64-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase());
64+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
6565
wallet->SetupLegacyScriptPubKeyMan();
6666
bool firstRun;
6767
wallet->LoadWallet(firstRun);

‎src/qt/test/wallettests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ void TestGUI(interfaces::Node& node)
139139
test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
140140
}
141141
node.setContext(&test.m_node);
142-
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase());
142+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase());
143143
bool firstRun;
144144
wallet->LoadWallet(firstRun);
145145
{

‎src/wallet/load.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include <wallet/load.h>
77

8+
#include <fs.h>
89
#include <interfaces/chain.h>
910
#include <scheduler.h>
1011
#include <util/string.h>
@@ -44,16 +45,16 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
4445
std::set<fs::path> wallet_paths;
4546

4647
for (const auto& wallet_file : wallet_files) {
47-
WalletLocation location(wallet_file);
48+
const fs::path path = fs::absolute(wallet_file, GetWalletDir());
4849

49-
if (!wallet_paths.insert(location.GetPath()).second) {
50+
if (!wallet_paths.insert(path).second) {
5051
chain.initError(strprintf(_("Error loading wallet %s. Duplicate -wallet filename specified."), wallet_file));
5152
return false;
5253
}
5354

5455
bilingual_str error_string;
5556
std::vector<bilingual_str> warnings;
56-
bool verify_success = CWallet::Verify(chain, location, error_string, warnings);
57+
bool verify_success = CWallet::Verify(chain, wallet_file, error_string, warnings);
5758
if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n")));
5859
if (!verify_success) {
5960
chain.initError(error_string);
@@ -70,7 +71,7 @@ bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& walle
7071
for (const std::string& walletFile : wallet_files) {
7172
bilingual_str error;
7273
std::vector<bilingual_str> warnings;
73-
std::shared_ptr<CWallet> pwallet = CWallet::CreateWalletFromFile(chain, WalletLocation(walletFile), error, warnings);
74+
std::shared_ptr<CWallet> pwallet = CWallet::CreateWalletFromFile(chain, walletFile, error, warnings);
7475
if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n")));
7576
if (!pwallet) {
7677
chain.initError(error);

‎src/wallet/rpcwallet.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2502,22 +2502,23 @@ static UniValue loadwallet(const JSONRPCRequest& request)
25022502
}.Check(request);
25032503

25042504
WalletContext& context = EnsureWalletContext(request.context);
2505-
WalletLocation location(request.params[0].get_str());
2505+
const std::string name(request.params[0].get_str());
2506+
fs::path path(fs::absolute(name, GetWalletDir()));
25062507

2507-
if (!location.Exists()) {
2508-
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Wallet " + location.GetName() + " not found.");
2509-
} else if (fs::is_directory(location.GetPath())) {
2508+
if (fs::symlink_status(path).type() == fs::file_not_found) {
2509+
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Wallet " + name + " not found.");
2510+
} else if (fs::is_directory(path)) {
25102511
// The given filename is a directory. Check that there's a wallet.dat file.
2511-
fs::path wallet_dat_file = location.GetPath() / "wallet.dat";
2512+
fs::path wallet_dat_file = path / "wallet.dat";
25122513
if (fs::symlink_status(wallet_dat_file).type() == fs::file_not_found) {
2513-
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Directory " + location.GetName() + " does not contain a wallet.dat file.");
2514+
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Directory " + name + " does not contain a wallet.dat file.");
25142515
}
25152516
}
25162517

25172518
bilingual_str error;
25182519
std::vector<bilingual_str> warnings;
25192520
Optional<bool> load_on_start = request.params[1].isNull() ? nullopt : Optional<bool>(request.params[1].get_bool());
2520-
std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, location, load_on_start, error, warnings);
2521+
std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, name, load_on_start, error, warnings);
25212522
if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original);
25222523

25232524
UniValue obj(UniValue::VOBJ);

‎src/wallet/salvage.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::v
123123
}
124124

125125
DbTxn* ptxn = env->TxnBegin();
126-
CWallet dummyWallet(nullptr, WalletLocation(), CreateDummyWalletDatabase());
126+
CWallet dummyWallet(nullptr, "", CreateDummyWalletDatabase());
127127
for (KeyValPair& row : salvagedData)
128128
{
129129
/* Filter for only private key type KV pairs to be added to the salvaged wallet */

‎src/wallet/test/coinselector_tests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ typedef std::set<CInputCoin> CoinSet;
2929
static std::vector<COutput> vCoins;
3030
static NodeContext testNode;
3131
static auto testChain = interfaces::MakeChain(testNode);
32-
static CWallet testWallet(testChain.get(), WalletLocation(), CreateDummyWalletDatabase());
32+
static CWallet testWallet(testChain.get(), "", CreateDummyWalletDatabase());
3333
static CAmount balance = 0;
3434

3535
CoinEligibilityFilter filter_standard(1, 6, 0);
@@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
283283
// Make sure that can use BnB when there are preset inputs
284284
empty_wallet();
285285
{
286-
std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), CreateMockWalletDatabase());
286+
std::unique_ptr<CWallet> wallet = MakeUnique<CWallet>(m_chain.get(), "", CreateMockWalletDatabase());
287287
bool firstRun;
288288
wallet->LoadWallet(firstRun);
289289
wallet->SetupLegacyScriptPubKeyMan();

‎src/wallet/test/ismine_tests.cpp

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
3535

3636
// P2PK compressed
3737
{
38-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
38+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
3939
keystore.SetupLegacyScriptPubKeyMan();
4040
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
4141
scriptPubKey = GetScriptForRawPubKey(pubkeys[0]);
@@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
5252

5353
// P2PK uncompressed
5454
{
55-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
55+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
5656
keystore.SetupLegacyScriptPubKeyMan();
5757
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
5858
scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey);
@@ -69,7 +69,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
6969

7070
// P2PKH compressed
7171
{
72-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
72+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
7373
keystore.SetupLegacyScriptPubKeyMan();
7474
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
7575
scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0]));
@@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
8686

8787
// P2PKH uncompressed
8888
{
89-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
89+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
9090
keystore.SetupLegacyScriptPubKeyMan();
9191
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
9292
scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey));
@@ -103,7 +103,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
103103

104104
// P2SH
105105
{
106-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
106+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
107107
keystore.SetupLegacyScriptPubKeyMan();
108108
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
109109

@@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
127127

128128
// (P2PKH inside) P2SH inside P2SH (invalid)
129129
{
130-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
130+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
131131
keystore.SetupLegacyScriptPubKeyMan();
132132
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
133133

@@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
145145

146146
// (P2PKH inside) P2SH inside P2WSH (invalid)
147147
{
148-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
148+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
149149
keystore.SetupLegacyScriptPubKeyMan();
150150
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
151151

@@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
163163

164164
// P2WPKH inside P2WSH (invalid)
165165
{
166-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
166+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
167167
keystore.SetupLegacyScriptPubKeyMan();
168168
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
169169

@@ -179,7 +179,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
179179

180180
// (P2PKH inside) P2WSH inside P2WSH (invalid)
181181
{
182-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
182+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
183183
keystore.SetupLegacyScriptPubKeyMan();
184184
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
185185

@@ -197,7 +197,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
197197

198198
// P2WPKH compressed
199199
{
200-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
200+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
201201
keystore.SetupLegacyScriptPubKeyMan();
202202
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
203203
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -212,7 +212,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
212212

213213
// P2WPKH uncompressed
214214
{
215-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
215+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
216216
keystore.SetupLegacyScriptPubKeyMan();
217217
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
218218
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -231,7 +231,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
231231

232232
// scriptPubKey multisig
233233
{
234-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
234+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
235235
keystore.SetupLegacyScriptPubKeyMan();
236236
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
237237

@@ -262,7 +262,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
262262

263263
// P2SH multisig
264264
{
265-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
265+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
266266
keystore.SetupLegacyScriptPubKeyMan();
267267
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
268268
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
283283

284284
// P2WSH multisig with compressed keys
285285
{
286-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
286+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
287287
keystore.SetupLegacyScriptPubKeyMan();
288288
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
289289
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -309,7 +309,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
309309

310310
// P2WSH multisig with uncompressed key
311311
{
312-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
312+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
313313
keystore.SetupLegacyScriptPubKeyMan();
314314
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
315315
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey));
@@ -335,7 +335,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
335335

336336
// P2WSH multisig wrapped in P2SH
337337
{
338-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
338+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
339339
keystore.SetupLegacyScriptPubKeyMan();
340340
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
341341

@@ -362,7 +362,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
362362

363363
// OP_RETURN
364364
{
365-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
365+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
366366
keystore.SetupLegacyScriptPubKeyMan();
367367
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
368368
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -376,7 +376,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
376376

377377
// witness unspendable
378378
{
379-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
379+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
380380
keystore.SetupLegacyScriptPubKeyMan();
381381
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
382382
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -390,7 +390,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
390390

391391
// witness unknown
392392
{
393-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
393+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
394394
keystore.SetupLegacyScriptPubKeyMan();
395395
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
396396
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));
@@ -404,7 +404,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard)
404404

405405
// Nonstandard
406406
{
407-
CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase());
407+
CWallet keystore(chain.get(), "", CreateDummyWalletDatabase());
408408
keystore.SetupLegacyScriptPubKeyMan();
409409
LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore);
410410
BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0]));

0 commit comments

Comments
 (0)