diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -37,7 +37,8 @@ // (https://github.com/bitcoin/bitcoin/issues/7883#issuecomment-224807484) static void CoinSelection(benchmark::State &state) { SelectParams(CBaseChainParams::REGTEST); - const CWallet wallet(Params(), "dummy", WalletDatabase::CreateDummy()); + const CWallet wallet(Params(), WalletLocation(), + WalletDatabase::CreateDummy()); LOCK(wallet.cs_wallet); // Add coins. @@ -97,7 +98,8 @@ static void BnBExhaustion(benchmark::State &state) { SelectParams(CBaseChainParams::REGTEST); - const CWallet wallet(Params(), "dummy", WalletDatabase::CreateDummy()); + const CWallet wallet(Params(), WalletLocation(), + WalletDatabase::CreateDummy()); LOCK(wallet.cs_wallet); // Setup diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -59,7 +59,7 @@ void TestAddAddressesToSendBook() { TestChain100Setup test; std::shared_ptr wallet = std::make_shared( - Params(), "mock", WalletDatabase::CreateMock()); + Params(), WalletLocation(), WalletDatabase::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -110,7 +110,7 @@ {}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } std::shared_ptr wallet = std::make_shared( - Params(), "mock", WalletDatabase::CreateMock()); + Params(), WalletLocation(), WalletDatabase::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); { diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -314,9 +314,9 @@ std::set wallet_paths; for (const auto &wallet_file : wallet_files) { - fs::path wallet_path = fs::absolute(wallet_file, GetWalletDir()); + WalletLocation location(wallet_file); - if (!wallet_paths.insert(wallet_path).second) { + if (!wallet_paths.insert(location.GetPath()).second) { return InitError(strprintf(_("Error loading wallet %s. Duplicate " "-wallet filename specified."), wallet_file)); @@ -325,8 +325,8 @@ std::string error_string; std::string warning_string; bool verify_success = - CWallet::Verify(chainParams, wallet_file, salvage_wallet, - error_string, warning_string); + CWallet::Verify(chainParams, location, salvage_wallet, error_string, + warning_string); if (!error_string.empty()) { InitError(error_string); } @@ -349,7 +349,7 @@ for (const std::string &walletFile : gArgs.GetArgs("-wallet")) { std::shared_ptr pwallet = CWallet::CreateWalletFromFile( - chainParams, walletFile, fs::absolute(walletFile, GetWalletDir())); + chainParams, WalletLocation(walletFile)); if (!pwallet) { return false; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3731,32 +3731,31 @@ const CChainParams &chainParams = config.GetChainParams(); - std::string wallet_file = request.params[0].get_str(); + WalletLocation location(request.params[0].get_str()); std::string error; - fs::path wallet_path = fs::absolute(wallet_file, GetWalletDir()); - if (fs::symlink_status(wallet_path).type() == fs::file_not_found) { + if (!location.Exists()) { throw JSONRPCError(RPC_WALLET_NOT_FOUND, - "Wallet " + wallet_file + " not found."); - } else if (fs::is_directory(wallet_path)) { + "Wallet " + location.GetName() + " not found."); + } else if (fs::is_directory(location.GetPath())) { // The given filename is a directory. Check that there's a wallet.dat // file. - fs::path wallet_dat_file = wallet_path / "wallet.dat"; + fs::path wallet_dat_file = location.GetPath() / "wallet.dat"; if (fs::symlink_status(wallet_dat_file).type() == fs::file_not_found) { throw JSONRPCError(RPC_WALLET_NOT_FOUND, - "Directory " + wallet_file + + "Directory " + location.GetName() + " does not contain a wallet.dat file."); } } std::string warning; - if (!CWallet::Verify(chainParams, wallet_file, false, error, warning)) { + if (!CWallet::Verify(chainParams, location, false, error, warning)) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet file verification failed: " + error); } - std::shared_ptr const wallet = CWallet::CreateWalletFromFile( - chainParams, wallet_file, fs::absolute(wallet_file, GetWalletDir())); + std::shared_ptr const wallet = + CWallet::CreateWalletFromFile(chainParams, location); if (!wallet) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet loading failed."); } @@ -3800,7 +3799,6 @@ const CChainParams &chainParams = config.GetChainParams(); - std::string wallet_name = request.params[0].get_str(); std::string error; std::string warning; @@ -3809,21 +3807,21 @@ disable_privatekeys = request.params[1].get_bool(); } - fs::path wallet_path = fs::absolute(wallet_name, GetWalletDir()); - if (fs::symlink_status(wallet_path).type() != fs::file_not_found) { + WalletLocation location(request.params[0].get_str()); + if (location.Exists()) { throw JSONRPCError(RPC_WALLET_ERROR, - "Wallet " + wallet_name + " already exists."); + "Wallet " + location.GetName() + " already exists."); } // Wallet::Verify will check if we're trying to create a wallet with a // duplicate name. - if (!CWallet::Verify(chainParams, wallet_name, false, error, warning)) { + if (!CWallet::Verify(chainParams, location, false, error, warning)) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet file verification failed: " + error); } std::shared_ptr const wallet = CWallet::CreateWalletFromFile( - chainParams, wallet_name, fs::absolute(wallet_name, GetWalletDir()), + chainParams, location, (disable_privatekeys ? uint64_t(WALLET_FLAG_DISABLE_PRIVATE_KEYS) : 0)); if (!wallet) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet creation failed."); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -291,7 +291,8 @@ } BOOST_AUTO_TEST_CASE(knapsack_solver_test) { - CWallet testWallet(Params(), "dummy", WalletDatabase::CreateDummy()); + CWallet testWallet(Params(), WalletLocation(), + WalletDatabase::CreateDummy()); CoinSet setCoinsRet, setCoinsRet2; Amount nValueRet; @@ -705,7 +706,8 @@ // Tests that with the ideal conditions, the coin selector will always be able // to find a solution that can pay the target value BOOST_AUTO_TEST_CASE(SelectCoins_test) { - CWallet testWallet(Params(), "dummy", WalletDatabase::CreateDummy()); + CWallet testWallet(Params(), WalletLocation(), + WalletDatabase::CreateDummy()); // Random generator stuff std::default_random_engine generator; diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -8,11 +8,12 @@ #include #include #include +#include #include WalletTestingSetup::WalletTestingSetup(const std::string &chainName) : TestingSetup(chainName), - m_wallet(Params(), "mock", WalletDatabase::CreateMock()) { + m_wallet(Params(), WalletLocation(), WalletDatabase::CreateMock()) { bool fFirstRun; m_wallet.LoadWallet(fFirstRun); RegisterValidationInterface(&m_wallet); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -45,7 +45,8 @@ // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet(Params(), "dummy", WalletDatabase::CreateDummy()); + CWallet wallet(Params(), WalletLocation(), + WalletDatabase::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -61,7 +62,8 @@ // Verify ScanForWalletTransactions only picks transactions in the new block // file. { - CWallet wallet(Params(), "dummy", WalletDatabase::CreateDummy()); + CWallet wallet(Params(), WalletLocation(), + WalletDatabase::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -75,7 +77,7 @@ // after. { std::shared_ptr wallet = std::make_shared( - Params(), "dummy", WalletDatabase::CreateDummy()); + Params(), WalletLocation(), WalletDatabase::CreateDummy()); AddWallet(wallet); UniValue keys; keys.setArray(); @@ -154,7 +156,7 @@ // Import key into wallet and call dumpwallet to create backup file. { std::shared_ptr wallet = std::make_shared( - Params(), "dummy", WalletDatabase::CreateDummy()); + Params(), WalletLocation(), WalletDatabase::CreateDummy()); LOCK(wallet->cs_wallet); wallet->mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; @@ -172,7 +174,7 @@ // were scanned, and no prior blocks were scanned. { std::shared_ptr wallet = std::make_shared( - Params(), "dummy", WalletDatabase::CreateDummy()); + Params(), WalletLocation(), WalletDatabase::CreateDummy()); JSONRPCRequest request; request.params.setArray(); @@ -201,7 +203,7 @@ // function. Similar tests probably should be written for the other credit and // debit functions. BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { - CWallet wallet(Params(), "dummy", WalletDatabase::CreateDummy()); + CWallet wallet(Params(), WalletLocation(), WalletDatabase::CreateDummy()); CWalletTx wtx(&wallet, m_coinbase_txns.back()); LOCK2(cs_main, wallet.cs_wallet); wtx.hashBlock = chainActive.Tip()->GetBlockHash(); @@ -292,7 +294,7 @@ ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - wallet = std::make_unique(Params(), "mock", + wallet = std::make_unique(Params(), WalletLocation(), WalletDatabase::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); @@ -401,7 +403,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) { std::shared_ptr wallet = std::make_shared( - Params(), "dummy", WalletDatabase::CreateDummy()); + Params(), WalletLocation(), WalletDatabase::CreateDummy()); wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); BOOST_CHECK(!wallet->TopUpKeyPool(1000)); CPubKey pubkey; diff --git a/src/wallet/test/walletdb_tests.cpp b/src/wallet/test/walletdb_tests.cpp --- a/src/wallet/test/walletdb_tests.cpp +++ b/src/wallet/test/walletdb_tests.cpp @@ -16,8 +16,8 @@ namespace { static std::unique_ptr LoadWallet(WalletBatch &batch) { - std::unique_ptr wallet( - new CWallet(Params(), "dummy", WalletDatabase::CreateDummy())); + std::unique_ptr wallet = std::make_unique( + Params(), WalletLocation(), WalletDatabase::CreateDummy()); DBErrors res = batch.LoadWallet(wallet.get()); BOOST_CHECK(res == DBErrors::LOAD_OK); return wallet; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -813,11 +814,9 @@ EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** - * Wallet filename from wallet= command line or config option. - * Used in debug logs and to send RPCs to the right wallet instance when - * more than one wallet is loaded. + * Wallet location which includes wallet name (see WalletLocation). */ - std::string m_name; + WalletLocation m_location; /** Internal database handle. */ std::unique_ptr database; @@ -860,10 +859,12 @@ CoinSelectionParams &coin_selection_params, bool &bnb_used) const; + const WalletLocation &GetLocation() const { return m_location; } + /** * Get a name for this wallet for logging/debugging purposes. */ - std::string GetName() const { return m_name; } + const std::string &GetName() const { return m_location.GetName(); } void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -880,9 +881,9 @@ unsigned int nMasterKeyMaxID = 0; /** Construct wallet with specified name and database implementation. */ - CWallet(const CChainParams &chainParamsIn, std::string name, + CWallet(const CChainParams &chainParamsIn, const WalletLocation &location, std::unique_ptr databaseIn) - : m_name(std::move(name)), database(std::move(databaseIn)), + : m_location(location), database(std::move(databaseIn)), chainParams(chainParamsIn) {} ~CWallet() { @@ -1322,9 +1323,9 @@ bool AbandonTransaction(const TxId &txid); //! Verify wallet naming and perform salvage on the wallet if required - static bool Verify(const CChainParams &chainParams, std::string wallet_file, - bool salvage_wallet, std::string &error_string, - std::string &warning_string); + static bool Verify(const CChainParams &chainParams, + const WalletLocation &location, bool salvage_wallet, + std::string &error_string, std::string &warning_string); /** * Initializes the wallet, returns a new CWallet instance or a null pointer @@ -1332,7 +1333,7 @@ */ static std::shared_ptr CreateWalletFromFile(const CChainParams &chainParams, - const std::string &name, const fs::path &path, + const WalletLocation &location, uint64_t wallet_creation_flags = 0); /** diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -33,7 +33,6 @@ #include #include #include -#include #include @@ -4288,9 +4287,9 @@ return values; } -bool CWallet::Verify(const CChainParams &chainParams, std::string wallet_file, - bool salvage_wallet, std::string &error_string, - std::string &warning_string) { +bool CWallet::Verify(const CChainParams &chainParams, + const WalletLocation &location, bool salvage_wallet, + std::string &error_string, std::string &warning_string) { // Do some checking on wallet path. It should be either a: // // 1. Path where a directory can be created. @@ -4298,12 +4297,12 @@ // 3. Path to a symlink to a directory. // 4. For backwards compatibility, the name of a data file in -walletdir. LOCK(cs_wallets); - fs::path wallet_path = fs::absolute(wallet_file, GetWalletDir()); + const fs::path &wallet_path = location.GetPath(); 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(wallet_file).filename() == wallet_file))) { + fs::path(location.GetName()).filename() == location.GetName()))) { error_string = strprintf("Invalid -wallet path '%s'. -wallet path should point to " "a directory where wallet.dat and " @@ -4311,16 +4310,16 @@ "where such a directory could be created, " "or (for backwards compatibility) the name of an " "existing data file in -walletdir (%s)", - wallet_file, GetWalletDir()); + location.GetName(), GetWalletDir()); return false; } // Make sure that the wallet path doesn't clash with an existing wallet path for (auto wallet : GetWallets()) { - if (fs::absolute(wallet->GetName(), GetWalletDir()) == wallet_path) { + if (wallet->GetLocation().GetPath() == wallet_path) { error_string = strprintf("Error loading wallet %s. Duplicate " "-wallet filename specified.", - wallet_file); + location.GetName()); return false; } } @@ -4330,14 +4329,14 @@ return false; } } catch (const fs::filesystem_error &e) { - error_string = - strprintf("Error loading wallet %s. %s", wallet_file, e.what()); + error_string = strprintf("Error loading wallet %s. %s", + location.GetName(), e.what()); return false; } if (salvage_wallet) { // Recover readable keypairs: - CWallet dummyWallet(chainParams, "dummy", + CWallet dummyWallet(chainParams, WalletLocation(), WalletDatabase::CreateDummy()); std::string backup_filename; if (!WalletBatch::Recover( @@ -4373,9 +4372,9 @@ std::shared_ptr CWallet::CreateWalletFromFile(const CChainParams &chainParams, - const std::string &name, const fs::path &path, + const WalletLocation &location, uint64_t wallet_creation_flags) { - const std::string &walletFile = name; + const std::string &walletFile = location.GetName(); // Needed to restore wallet transaction meta data after -zapwallettxes std::vector vWtx; @@ -4384,7 +4383,7 @@ uiInterface.InitMessage(_("Zapping all transactions from wallet...")); std::unique_ptr tempWallet = std::make_unique( - chainParams, name, WalletDatabase::Create(path)); + chainParams, location, WalletDatabase::Create(location.GetPath())); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DBErrors::LOAD_OK) { InitError( @@ -4400,7 +4399,8 @@ // 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(chainParams, name, WalletDatabase::Create(path)), + new CWallet(chainParams, location, + WalletDatabase::Create(location.GetPath())), ReleaseWallet); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DBErrors::LOAD_OK) { diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -10,4 +10,23 @@ //! Get the path of the wallet directory. fs::path GetWalletDir(); +//! 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; +}; + #endif // BITCOIN_WALLET_WALLETUTIL_H diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -25,3 +25,10 @@ return path; } + +WalletLocation::WalletLocation(const std::string &name) + : m_name(name), m_path(fs::absolute(name, GetWalletDir())) {} + +bool WalletLocation::Exists() const { + return fs::symlink_status(m_path).type() != fs::file_not_found; +}