From 288b4ffb6b291f0466d513ff3c40af6758ca7c88 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 28 Jul 2020 19:25:14 -0400 Subject: 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".) --- src/bench/coin_selection.cpp | 4 +-- src/bench/wallet_balance.cpp | 2 +- src/interfaces/wallet.cpp | 2 +- src/qt/test/addressbooktests.cpp | 2 +- src/qt/test/wallettests.cpp | 2 +- src/wallet/load.cpp | 9 ++++--- src/wallet/rpcwallet.cpp | 15 +++++------ src/wallet/salvage.cpp | 2 +- src/wallet/test/coinselector_tests.cpp | 4 +-- src/wallet/test/ismine_tests.cpp | 40 ++++++++++++++--------------- src/wallet/test/scriptpubkeyman_tests.cpp | 2 +- src/wallet/test/wallet_test_fixture.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 22 ++++++++-------- src/wallet/wallet.cpp | 42 +++++++++++++++---------------- src/wallet/wallet.h | 18 ++++++------- src/wallet/wallettool.cpp | 4 +-- src/wallet/walletutil.cpp | 16 ------------ src/wallet/walletutil.h | 20 --------------- 18 files changed, 86 insertions(+), 122 deletions(-) (limited to 'src') diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 3a71a6ca03..99aafd8dfc 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -31,7 +31,7 @@ static void CoinSelection(benchmark::Bench& bench) { NodeContext node; auto chain = interfaces::MakeChain(node); - CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); wallet.SetupLegacyScriptPubKeyMan(); std::vector> wtxs; LOCK(wallet.cs_wallet); @@ -65,7 +65,7 @@ static void CoinSelection(benchmark::Bench& bench) typedef std::set CoinSet; static NodeContext testNode; static auto testChain = interfaces::MakeChain(testNode); -static CWallet testWallet(testChain.get(), WalletLocation(), CreateDummyWalletDatabase()); +static CWallet testWallet(testChain.get(), "", CreateDummyWalletDatabase()); std::vector> wtxn; // Copied from src/wallet/test/coinselector_tests.cpp diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index e16182b48e..b3b73284d8 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -26,7 +26,7 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b NodeContext node; std::unique_ptr chain = interfaces::MakeChain(node); - CWallet wallet{chain.get(), WalletLocation(), CreateMockWalletDatabase()}; + CWallet wallet{chain.get(), "", CreateMockWalletDatabase()}; { wallet.SetupLegacyScriptPubKeyMan(); bool first_run; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 28839b2ffc..d17110a0f6 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -522,7 +522,7 @@ public: } std::unique_ptr loadWallet(const std::string& name, bilingual_str& error, std::vector& warnings) override { - return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), true /* load_on_start */, error, warnings)); + return MakeWallet(LoadWallet(*m_context.chain, name, true /* load_on_start */, error, warnings)); } std::string getWalletDir() override { diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index d6781460a7..35fcb2b0ca 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -61,7 +61,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node) { TestChain100Setup test; node.setContext(&test.m_node); - std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase()); + std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); bool firstRun; wallet->LoadWallet(firstRun); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 98065803e9..d6d2d0e3df 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -139,7 +139,7 @@ void TestGUI(interfaces::Node& node) test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } node.setContext(&test.m_node); - std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), WalletLocation(), CreateMockWalletDatabase()); + std::shared_ptr wallet = std::make_shared(node.context()->chain.get(), "", CreateMockWalletDatabase()); bool firstRun; wallet->LoadWallet(firstRun); { diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 5bbc8b91f7..dde29842ec 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -5,6 +5,7 @@ #include +#include #include #include #include @@ -44,16 +45,16 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal std::set wallet_paths; for (const auto& wallet_file : wallet_files) { - WalletLocation location(wallet_file); + const fs::path path = fs::absolute(wallet_file, GetWalletDir()); - if (!wallet_paths.insert(location.GetPath()).second) { + if (!wallet_paths.insert(path).second) { chain.initError(strprintf(_("Error loading wallet %s. Duplicate -wallet filename specified."), wallet_file)); return false; } bilingual_str error_string; std::vector warnings; - bool verify_success = CWallet::Verify(chain, location, error_string, warnings); + bool verify_success = CWallet::Verify(chain, wallet_file, error_string, warnings); if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!verify_success) { chain.initError(error_string); @@ -70,7 +71,7 @@ bool LoadWallets(interfaces::Chain& chain, const std::vector& walle for (const std::string& walletFile : wallet_files) { bilingual_str error; std::vector warnings; - std::shared_ptr pwallet = CWallet::CreateWalletFromFile(chain, WalletLocation(walletFile), error, warnings); + std::shared_ptr pwallet = CWallet::CreateWalletFromFile(chain, walletFile, error, warnings); if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!pwallet) { chain.initError(error); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 887c5b632b..312b345518 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2502,22 +2502,23 @@ static UniValue loadwallet(const JSONRPCRequest& request) }.Check(request); WalletContext& context = EnsureWalletContext(request.context); - WalletLocation location(request.params[0].get_str()); + const std::string name(request.params[0].get_str()); + fs::path path(fs::absolute(name, GetWalletDir())); - if (!location.Exists()) { - throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Wallet " + location.GetName() + " not found."); - } else if (fs::is_directory(location.GetPath())) { + if (fs::symlink_status(path).type() == fs::file_not_found) { + throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Wallet " + name + " not found."); + } else if (fs::is_directory(path)) { // The given filename is a directory. Check that there's a wallet.dat file. - fs::path wallet_dat_file = location.GetPath() / "wallet.dat"; + fs::path wallet_dat_file = path / "wallet.dat"; if (fs::symlink_status(wallet_dat_file).type() == fs::file_not_found) { - throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Directory " + location.GetName() + " does not contain a wallet.dat file."); + throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Directory " + name + " does not contain a wallet.dat file."); } } bilingual_str error; std::vector warnings; Optional load_on_start = request.params[1].isNull() ? nullopt : Optional(request.params[1].get_bool()); - std::shared_ptr const wallet = LoadWallet(*context.chain, location, load_on_start, error, warnings); + std::shared_ptr const wallet = LoadWallet(*context.chain, name, load_on_start, error, warnings); if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original); UniValue obj(UniValue::VOBJ); diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index 934e3d5c86..96fba21339 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -123,7 +123,7 @@ bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::v } DbTxn* ptxn = env->TxnBegin(); - CWallet dummyWallet(nullptr, WalletLocation(), CreateDummyWalletDatabase()); + CWallet dummyWallet(nullptr, "", CreateDummyWalletDatabase()); for (KeyValPair& row : salvagedData) { /* Filter for only private key type KV pairs to be added to the salvaged wallet */ diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 1deedede4c..f38ccba384 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -29,7 +29,7 @@ typedef std::set CoinSet; static std::vector vCoins; static NodeContext testNode; static auto testChain = interfaces::MakeChain(testNode); -static CWallet testWallet(testChain.get(), WalletLocation(), CreateDummyWalletDatabase()); +static CWallet testWallet(testChain.get(), "", CreateDummyWalletDatabase()); static CAmount balance = 0; CoinEligibilityFilter filter_standard(1, 6, 0); @@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Make sure that can use BnB when there are preset inputs empty_wallet(); { - std::unique_ptr wallet = MakeUnique(m_chain.get(), WalletLocation(), CreateMockWalletDatabase()); + std::unique_ptr wallet = MakeUnique(m_chain.get(), "", CreateMockWalletDatabase()); bool firstRun; wallet->LoadWallet(firstRun); wallet->SetupLegacyScriptPubKeyMan(); diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index e416f16044..d5aed99d99 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -35,7 +35,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PK compressed { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForRawPubKey(pubkeys[0]); @@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PK uncompressed { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey); @@ -69,7 +69,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PKH compressed { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0])); @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PKH uncompressed { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey)); @@ -103,7 +103,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2SH { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2SH inside P2SH (invalid) { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2SH inside P2WSH (invalid) { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH inside P2WSH (invalid) { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -179,7 +179,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2WSH inside P2WSH (invalid) { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -197,7 +197,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH compressed { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -212,7 +212,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH uncompressed { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); @@ -231,7 +231,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // scriptPubKey multisig { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -262,7 +262,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2SH multisig { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); @@ -283,7 +283,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig with compressed keys { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -309,7 +309,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig with uncompressed key { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); @@ -335,7 +335,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig wrapped in P2SH { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -362,7 +362,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // OP_RETURN { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -376,7 +376,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // witness unspendable { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -390,7 +390,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // witness unknown { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -404,7 +404,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Nonstandard { - CWallet keystore(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp index 4f12079768..f7c1337b0d 100644 --- a/src/wallet/test/scriptpubkeyman_tests.cpp +++ b/src/wallet/test/scriptpubkeyman_tests.cpp @@ -19,7 +19,7 @@ BOOST_AUTO_TEST_CASE(CanProvide) // Set up wallet and keyman variables. NodeContext node; std::unique_ptr chain = interfaces::MakeChain(node); - CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); LegacyScriptPubKeyMan& keyman = *wallet.GetOrCreateLegacyScriptPubKeyMan(); // Make a 1 of 2 multisig script diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 9241470745..4d6f427618 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -6,7 +6,7 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), - m_wallet(m_chain.get(), WalletLocation(), CreateMockWalletDatabase()) + m_wallet(m_chain.get(), "", CreateMockWalletDatabase()) { bool fFirstRun; m_wallet.LoadWallet(fFirstRun); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index f400e7e944..b3001efd55 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -39,7 +39,7 @@ static std::shared_ptr TestLoadWallet(interfaces::Chain& chain) { bilingual_str error; std::vector warnings; - auto wallet = CWallet::CreateWalletFromFile(chain, WalletLocation(""), error, warnings); + auto wallet = CWallet::CreateWalletFromFile(chain, "", error, warnings); wallet->postInitProcess(); return wallet; } @@ -85,7 +85,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions fails to read an unknown start block. { - CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -104,7 +104,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -130,7 +130,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions only picks transactions in the new block // file. { - CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -155,7 +155,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions scans no blocks. { - CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -194,7 +194,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) // before the missing block, and success for a key whose creation time is // after. { - std::shared_ptr wallet = std::make_shared(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + std::shared_ptr wallet = std::make_shared(chain.get(), "", CreateDummyWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash())); AddWallet(wallet); @@ -259,7 +259,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { - std::shared_ptr wallet = std::make_shared(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + std::shared_ptr wallet = std::make_shared(chain.get(), "", CreateDummyWalletDatabase()); { auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); @@ -281,7 +281,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { - std::shared_ptr wallet = std::make_shared(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + std::shared_ptr wallet = std::make_shared(chain.get(), "", CreateDummyWalletDatabase()); LOCK(wallet->cs_wallet); wallet->SetupLegacyScriptPubKeyMan(); @@ -317,7 +317,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) NodeContext node; auto chain = interfaces::MakeChain(node); - CWallet wallet(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); CWalletTx wtx(&wallet, m_coinbase_txns.back()); @@ -492,7 +492,7 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - wallet = MakeUnique(m_chain.get(), WalletLocation(), CreateMockWalletDatabase()); + wallet = MakeUnique(m_chain.get(), "", CreateMockWalletDatabase()); { LOCK2(wallet->cs_wallet, ::cs_main); wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); @@ -610,7 +610,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) { NodeContext node; auto chain = interfaces::MakeChain(node); - std::shared_ptr wallet = std::make_shared(chain.get(), WalletLocation(), CreateDummyWalletDatabase()); + std::shared_ptr wallet = std::make_shared(chain.get(), "", CreateDummyWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); wallet->SetMinVersion(FEATURE_LATEST); wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index afe676078c..43dbd48262 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -200,15 +200,15 @@ void UnloadWallet(std::shared_ptr&& wallet) } namespace { -std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const WalletLocation& location, Optional load_on_start, bilingual_str& error, std::vector& warnings) +std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const std::string& name, Optional load_on_start, bilingual_str& error, std::vector& warnings) { try { - if (!CWallet::Verify(chain, location, error, warnings)) { + if (!CWallet::Verify(chain, name, error, warnings)) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; return nullptr; } - std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, location, error, warnings); + std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, name, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; return nullptr; @@ -217,7 +217,7 @@ std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const Wall wallet->postInitProcess(); // Write the wallet setting - UpdateWalletSetting(chain, location.GetName(), load_on_start, warnings); + UpdateWalletSetting(chain, name, load_on_start, warnings); return wallet; } catch (const std::runtime_error& e) { @@ -227,14 +227,14 @@ std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const Wall } } // namespace -std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, Optional load_on_start, bilingual_str& error, std::vector& warnings) +std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, Optional load_on_start, bilingual_str& error, std::vector& warnings) { - auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(location.GetName())); + auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(name)); if (!result.second) { error = Untranslated("Wallet already being loading."); return nullptr; } - auto wallet = LoadWalletInternal(chain, location, load_on_start, error, warnings); + auto wallet = LoadWalletInternal(chain, name, load_on_start, error, warnings); WITH_LOCK(g_loading_wallet_mutex, g_loading_wallet_set.erase(result.first)); return wallet; } @@ -250,14 +250,13 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& } // Check the wallet file location - WalletLocation location(name); - if (location.Exists()) { - error = strprintf(Untranslated("Wallet %s already exists."), location.GetName()); + if (fs::symlink_status(fs::absolute(name.empty() ? "wallet.dat" : name, GetWalletDir())).type() != fs::file_not_found) { + error = strprintf(Untranslated("Wallet %s already exists."), name); return WalletCreationStatus::CREATION_FAILED; } // Wallet::Verify will check if we're trying to create a wallet with a duplicate name. - if (!CWallet::Verify(chain, location, error, warnings)) { + if (!CWallet::Verify(chain, name, error, warnings)) { error = Untranslated("Wallet file verification failed.") + Untranslated(" ") + error; return WalletCreationStatus::CREATION_FAILED; } @@ -269,7 +268,7 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& } // Make the wallet - std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, location, error, warnings, wallet_creation_flags); + std::shared_ptr wallet = CWallet::CreateWalletFromFile(chain, name, error, warnings, wallet_creation_flags); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; return WalletCreationStatus::CREATION_FAILED; @@ -3770,7 +3769,7 @@ std::vector CWallet::GetDestValues(const std::string& prefix) const return values; } -bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error_string, std::vector& warnings) +bool CWallet::Verify(interfaces::Chain& chain, const std::string& name, bilingual_str& error_string, std::vector& warnings) { // Do some checking on wallet path. It should be either a: // @@ -3779,22 +3778,22 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b // 3. Path to a symlink to a directory. // 4. For backwards compatibility, the name of a data file in -walletdir. LOCK(cs_wallets); - const fs::path& wallet_path = location.GetPath(); + const fs::path& wallet_path = fs::absolute(name, GetWalletDir()); fs::file_type path_type = fs::symlink_status(wallet_path).type(); if (!(path_type == fs::file_not_found || path_type == fs::directory_file || (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || - (path_type == fs::regular_file && fs::path(location.GetName()).filename() == location.GetName()))) { + (path_type == fs::regular_file && fs::path(name).filename() == name))) { error_string = Untranslated(strprintf( "Invalid -wallet path '%s'. -wallet path should point to a directory where wallet.dat and " "database/log.?????????? files can be stored, a location where such a directory could be created, " "or (for backwards compatibility) the name of an existing data file in -walletdir (%s)", - location.GetName(), GetWalletDir())); + name, GetWalletDir())); return false; } // Make sure that the wallet path doesn't clash with an existing wallet path if (IsWalletLoaded(wallet_path)) { - error_string = Untranslated(strprintf("Error loading wallet %s. Duplicate -wallet filename specified.", location.GetName())); + error_string = Untranslated(strprintf("Error loading wallet %s. Duplicate -wallet filename specified.", name)); return false; } @@ -3804,14 +3803,15 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b try { return database->Verify(error_string); } catch (const fs::filesystem_error& e) { - error_string = Untranslated(strprintf("Error loading wallet %s. %s", location.GetName(), fsbridge::get_filesystem_error_message(e))); + error_string = Untranslated(strprintf("Error loading wallet %s. %s", name, fsbridge::get_filesystem_error_message(e))); return false; } } -std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings, uint64_t wallet_creation_flags) +std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector& warnings, uint64_t wallet_creation_flags) { - const std::string walletFile = WalletDataFilePath(location.GetPath()).string(); + fs::path path = fs::absolute(name, GetWalletDir()); + const std::string walletFile = WalletDataFilePath(path).string(); chain.initMessage(_("Loading wallet...").translated); @@ -3819,7 +3819,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, bool fFirstRun = true; // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. - std::shared_ptr walletInstance(new CWallet(&chain, location, CreateWalletDatabase(location.GetPath())), ReleaseWallet); + std::shared_ptr walletInstance(new CWallet(&chain, name, CreateWalletDatabase(path)), ReleaseWallet); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 06069029ea..9c04873c5b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -54,7 +54,7 @@ bool RemoveWallet(const std::shared_ptr& wallet, Optional load_on bool RemoveWallet(const std::shared_ptr& wallet, Optional load_on_start); std::vector> GetWallets(); std::shared_ptr GetWallet(const std::string& name); -std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, Optional load_on_start, bilingual_str& error, std::vector& warnings); +std::shared_ptr LoadWallet(interfaces::Chain& chain, const std::string& name, Optional load_on_start, bilingual_str& error, std::vector& warnings); std::unique_ptr HandleLoadWallet(LoadWalletFn load_wallet); enum class WalletCreationStatus { @@ -700,8 +700,8 @@ private: /** Interface for accessing chain state. */ interfaces::Chain* m_chain; - /** Wallet location which includes wallet name (see WalletLocation). */ - WalletLocation m_location; + /** Wallet name: relative directory name or "" for default wallet. */ + std::string m_name; /** Internal database handle. */ std::unique_ptr database; @@ -755,20 +755,18 @@ public: bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - const WalletLocation& GetLocation() const { return m_location; } - /** Get a name for this wallet for logging/debugging purposes. */ - const std::string& GetName() const { return m_location.GetName(); } + const std::string& GetName() const { return m_name; } typedef std::map MasterKeyMap; MasterKeyMap mapMasterKeys; unsigned int nMasterKeyMaxID = 0; /** Construct wallet with specified name and database implementation. */ - CWallet(interfaces::Chain* chain, const WalletLocation& location, std::unique_ptr database) + CWallet(interfaces::Chain* chain, const std::string& name, std::unique_ptr database) : m_chain(chain), - m_location(location), + m_name(name), database(std::move(database)) { } @@ -1154,10 +1152,10 @@ public: bool MarkReplaced(const uint256& originalHash, const uint256& newHash); //! Verify wallet naming and perform salvage on the wallet if required - static bool Verify(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error_string, std::vector& warnings); + static bool Verify(interfaces::Chain& chain, const std::string& name, bilingual_str& error_string, std::vector& warnings); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr CreateWalletFromFile(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings, uint64_t wallet_creation_flags = 0); + static std::shared_ptr CreateWalletFromFile(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector& warnings, uint64_t wallet_creation_flags = 0); /** * Wallet post-init setup diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 9b51461843..abd5652fe7 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -28,7 +28,7 @@ static std::shared_ptr CreateWallet(const std::string& name, const fs:: return nullptr; } // dummy chain interface - std::shared_ptr wallet_instance(new CWallet(nullptr /* chain */, WalletLocation(name), CreateWalletDatabase(path)), WalletToolReleaseWallet); + std::shared_ptr wallet_instance(new CWallet(nullptr /* chain */, name, CreateWalletDatabase(path)), WalletToolReleaseWallet); LOCK(wallet_instance->cs_wallet); bool first_run = true; DBErrors load_wallet_ret = wallet_instance->LoadWallet(first_run); @@ -57,7 +57,7 @@ static std::shared_ptr LoadWallet(const std::string& name, const fs::pa } // dummy chain interface - std::shared_ptr wallet_instance(new CWallet(nullptr /* chain */, WalletLocation(name), CreateWalletDatabase(path)), WalletToolReleaseWallet); + std::shared_ptr wallet_instance(new CWallet(nullptr /* chain */, name, CreateWalletDatabase(path)), WalletToolReleaseWallet); DBErrors load_wallet_ret; try { bool first_run; diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index 8bac0608a9..d6a8ee9e02 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -91,19 +91,3 @@ std::vector ListWalletDir() return paths; } - -WalletLocation::WalletLocation(const std::string& name) - : m_name(name) - , m_path(fs::absolute(name, GetWalletDir())) -{ -} - -bool WalletLocation::Exists() const -{ - fs::path path = m_path; - // For the default wallet, check specifically for the wallet.dat file - if (m_name.empty()) { - path = fs::absolute("wallet.dat", m_path); - } - return fs::symlink_status(path).type() != fs::file_not_found; -} diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index a4e4fda8a1..afdcb2e18a 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -67,26 +67,6 @@ fs::path GetWalletDir(); //! Get wallets in wallet directory. std::vector ListWalletDir(); -//! The WalletLocation class provides wallet information. -class WalletLocation final -{ - std::string m_name; - fs::path m_path; - -public: - explicit WalletLocation() {} - explicit WalletLocation(const std::string& name); - - //! Get wallet name. - const std::string& GetName() const { return m_name; } - - //! Get wallet absolute path. - const fs::path& GetPath() const { return m_path; } - - //! Return whether the wallet exists. - bool Exists() const; -}; - /** Descriptor with some wallet metadata */ class WalletDescriptor { -- cgit v1.2.3