maxmustermann/dogecoin
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge #14350: Add WalletLocation class

Browse source 
65f3672f3b wallet: Refactor to use WalletLocation (João Barbosa)
01a4c095c8 wallet: Add WalletLocation utility class (João Barbosa)

Pull request description:

  Advantages of this change:
   - avoid resolving wallet absolute path and name repetitively and in multiple places;
   - avoid calling `GetWalletDir` in multiple places;
   - extract these details from the actual wallet implementation.

  The `WalletLocation` class can be a way to represent a wallet not yet loaded that exists in the wallet directory.

Tree-SHA512: 71ec09786e038499710e7acafe92d66ab9883fc894964e267443ae9c10a6872a10995c3987a169c436a4e793dae96b28fb97bd7f78483c4b72ac930fa23f8686
This commit is contained in:
Wladimir J. van der Laan 2018-11-05 13:17:01 +01:00
parent 69d574ac6c 65f3672f3b
commit 46eb2755d4
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
13 changed files with 85 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/bench/coin_selection.cpp
View file

@ -33,7 +33,7 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector<Ou
// (https://github.com/bitcoin/bitcoin/issues/7883#issuecomment-224807484)
static void CoinSelection(benchmark::State& state)
{
const CWallet wallet("dummy", WalletDatabase::CreateDummy());
const CWallet wallet(WalletLocation(), WalletDatabase::CreateDummy());
LOCK(wallet.cs_wallet);
// Add coins.
@ -57,7 +57,7 @@ static void CoinSelection(benchmark::State& state)
}
typedef std::set<CInputCoin> CoinSet;
static const CWallet testWallet("dummy", WalletDatabase::CreateDummy());
static const CWallet testWallet(WalletLocation(), WalletDatabase::CreateDummy());
std::vector<std::unique_ptr<CWalletTx>> wtxn;
// Copied from src/wallet/test/coinselector_tests.cpp

2
src/qt/test/addressbooktests.cpp
View file

@ -56,7 +56,7 @@ void EditAddressAndSubmit(
void TestAddAddressesToSendBook()
{
TestChain100Setup test;
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>("mock", WalletDatabase::CreateMock());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(WalletLocation(), WalletDatabase::CreateMock());
bool firstRun;
wallet->LoadWallet(firstRun);

2
src/qt/test/wallettests.cpp
View file

@ -132,7 +132,7 @@ void TestGUI()
for (int i = 0; i < 5; ++i) {
test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
}
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>("mock", WalletDatabase::CreateMock());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(WalletLocation(), WalletDatabase::CreateMock());
bool firstRun;
wallet->LoadWallet(firstRun);
{

8
src/wallet/init.cpp
View file

@ -211,15 +211,15 @@ bool WalletInit::Verify() const
std::set<fs::path> 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));
}
std::string error_string;
std::string warning_string;
bool verify_success = CWallet::Verify(wallet_file, salvage_wallet, error_string, warning_string);
bool verify_success = CWallet::Verify(location, salvage_wallet, error_string, warning_string);
if (!error_string.empty()) InitError(error_string);
if (!warning_string.empty()) InitWarning(warning_string);
if (!verify_success) return false;
@ -236,7 +236,7 @@ bool WalletInit::Open() const
}
for (const std::string& walletFile : gArgs.GetArgs("-wallet")) {
std::shared_ptr<CWallet> pwallet = CWallet::CreateWalletFromFile(walletFile, fs::absolute(walletFile, GetWalletDir()));
std::shared_ptr<CWallet> pwallet = CWallet::CreateWalletFromFile(WalletLocation(walletFile));
if (!pwallet) {
return false;
}

29
src/wallet/rpcwallet.cpp
View file

@ -2405,26 +2405,26 @@ static UniValue loadwallet(const JSONRPCRequest& request)
+ HelpExampleCli("loadwallet", "\"test.dat\"")
+ HelpExampleRpc("loadwallet", "\"test.dat\"")
);
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) {
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Wallet " + wallet_file + " not found.");
} else if (fs::is_directory(wallet_path)) {
if (!location.Exists()) {
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "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 + " does not contain a wallet.dat file.");
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Directory " + location.GetName() + " does not contain a wallet.dat file.");
}
}
std::string warning;
if (!CWallet::Verify(wallet_file, false, error, warning)) {
if (!CWallet::Verify(location, false, error, warning)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet file verification failed: " + error);
}
std::shared_ptr<CWallet> const wallet = CWallet::CreateWalletFromFile(wallet_file, fs::absolute(wallet_file, GetWalletDir()));
std::shared_ptr<CWallet> const wallet = CWallet::CreateWalletFromFile(location);
if (!wallet) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet loading failed.");
}
@ -2458,7 +2458,6 @@ static UniValue createwallet(const JSONRPCRequest& request)
+ HelpExampleRpc("createwallet", "\"testwallet\"")
);
}
std::string wallet_name = request.params[0].get_str();
std::string error;
std::string warning;
@ -2467,17 +2466,17 @@ static UniValue createwallet(const JSONRPCRequest& request)
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) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet " + wallet_name + " already exists.");
WalletLocation location(request.params[0].get_str());
if (location.Exists()) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet " + location.GetName() + " already exists.");
}
// Wallet::Verify will check if we're trying to create a wallet with a duplication name.
if (!CWallet::Verify(wallet_name, false, error, warning)) {
if (!CWallet::Verify(location, false, error, warning)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet file verification failed: " + error);
}
std::shared_ptr<CWallet> const wallet = CWallet::CreateWalletFromFile(wallet_name, fs::absolute(wallet_name, GetWalletDir()), (disable_privatekeys ? (uint64_t)WALLET_FLAG_DISABLE_PRIVATE_KEYS : 0));
std::shared_ptr<CWallet> const wallet = CWallet::CreateWalletFromFile(location, (disable_privatekeys ? (uint64_t)WALLET_FLAG_DISABLE_PRIVATE_KEYS : 0));
if (!wallet) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet creation failed.");
}

2
src/wallet/test/coinselector_tests.cpp
View file

@ -28,7 +28,7 @@ std::vector<std::unique_ptr<CWalletTx>> wtxn;
typedef std::set<CInputCoin> CoinSet;
static std::vector<COutput> vCoins;
static CWallet testWallet("dummy", WalletDatabase::CreateDummy());
static CWallet testWallet(WalletLocation(), WalletDatabase::CreateDummy());
static CAmount balance = 0;
CoinEligibilityFilter filter_standard(1, 6, 0);

3
src/wallet/test/wallet_test_fixture.cpp
View file

@ -6,9 +6,10 @@
#include <rpc/server.h>
#include <wallet/db.h>
#include <wallet/rpcwallet.h>
WalletTestingSetup::WalletTestingSetup(const std::string& chainName):
TestingSetup(chainName), m_wallet("mock", WalletDatabase::CreateMock())
TestingSetup(chainName), m_wallet(WalletLocation(), WalletDatabase::CreateMock())
{
bool fFirstRun;
m_wallet.LoadWallet(fFirstRun);

16
src/wallet/test/wallet_tests.cpp
View file

@ -46,7 +46,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
// Verify ScanForWalletTransactions picks up transactions in both the old
// and new block files.
{
CWallet wallet("dummy", WalletDatabase::CreateDummy());
CWallet wallet(WalletLocation(), WalletDatabase::CreateDummy());
AddKey(wallet, coinbaseKey);
WalletRescanReserver reserver(&wallet);
reserver.reserve();
@ -61,7 +61,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
// Verify ScanForWalletTransactions only picks transactions in the new block
// file.
{
CWallet wallet("dummy", WalletDatabase::CreateDummy());
CWallet wallet(WalletLocation(), WalletDatabase::CreateDummy());
AddKey(wallet, coinbaseKey);
WalletRescanReserver reserver(&wallet);
reserver.reserve();
@ -73,7 +73,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
// before the missing block, and success for a key whose creation time is
// after.
{
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>("dummy", WalletDatabase::CreateDummy());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(WalletLocation(), WalletDatabase::CreateDummy());
AddWallet(wallet);
UniValue keys;
keys.setArray();
@ -134,7 +134,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
// Import key into wallet and call dumpwallet to create backup file.
{
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>("dummy", WalletDatabase::CreateDummy());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(WalletLocation(), WalletDatabase::CreateDummy());
LOCK(wallet->cs_wallet);
wallet->mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME;
wallet->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
@ -150,7 +150,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<CWallet> wallet = std::make_shared<CWallet>("dummy", WalletDatabase::CreateDummy());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(WalletLocation(), WalletDatabase::CreateDummy());
JSONRPCRequest request;
request.params.setArray();
@ -180,7 +180,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
// debit functions.
BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
{
CWallet wallet("dummy", WalletDatabase::CreateDummy());
CWallet wallet(WalletLocation(), WalletDatabase::CreateDummy());
CWalletTx wtx(&wallet, m_coinbase_txns.back());
LOCK2(cs_main, wallet.cs_wallet);
wtx.hashBlock = chainActive.Tip()->GetBlockHash();
@ -273,7 +273,7 @@ public:
ListCoinsTestingSetup()
{
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
wallet = MakeUnique<CWallet>("mock", WalletDatabase::CreateMock());
wallet = MakeUnique<CWallet>(WalletLocation(), WalletDatabase::CreateMock());
bool firstRun;
wallet->LoadWallet(firstRun);
AddKey(*wallet, coinbaseKey);
@ -377,7 +377,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
{
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>("dummy", WalletDatabase::CreateDummy());
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(WalletLocation(), WalletDatabase::CreateDummy());
wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
BOOST_CHECK(!wallet->TopUpKeyPool(1000));
CPubKey pubkey;

25
src/wallet/wallet.cpp
View file

@ -27,7 +27,6 @@
#include <txmempool.h>
#include <util/moneystr.h>
#include <wallet/fees.h>
#include <wallet/walletutil.h>
#include <algorithm>
#include <assert.h>
@ -3826,7 +3825,7 @@ void CWallet::MarkPreSplitKeys()
}
}
bool CWallet::Verify(std::string wallet_file, bool salvage_wallet, std::string& error_string, std::string& warning_string)
bool CWallet::Verify(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:
//
@ -3835,23 +3834,23 @@ bool CWallet::Verify(std::string wallet_file, bool salvage_wallet, std::string&
// 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))) {
(path_type == fs::regular_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 "
"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)",
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) {
error_string = strprintf("Error loading wallet %s. Duplicate -wallet filename specified.", wallet_file);
if (wallet->GetLocation().GetPath() == wallet_path) {
error_string = strprintf("Error loading wallet %s. Duplicate -wallet filename specified.", location.GetName());
return false;
}
}
@ -3861,13 +3860,13 @@ bool CWallet::Verify(std::string wallet_file, bool salvage_wallet, std::string&
return false;
}
} catch (const fs::filesystem_error& e) {
error_string = strprintf("Error loading wallet %s. %s", wallet_file, fsbridge::get_filesystem_error_message(e));
error_string = strprintf("Error loading wallet %s. %s", location.GetName(), fsbridge::get_filesystem_error_message(e));
return false;
}
if (salvage_wallet) {
// Recover readable keypairs:
CWallet dummyWallet("dummy", WalletDatabase::CreateDummy());
CWallet dummyWallet(WalletLocation(), WalletDatabase::CreateDummy());
std::string backup_filename;
if (!WalletBatch::Recover(wallet_path, (void *)&dummyWallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename)) {
return false;
@ -3877,9 +3876,9 @@ bool CWallet::Verify(std::string wallet_file, bool salvage_wallet, std::string&
return WalletBatch::VerifyDatabaseFile(wallet_path, warning_string, error_string);
}
std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(const std::string& name, const fs::path& path, uint64_t wallet_creation_flags)
std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(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<CWalletTx> vWtx;
@ -3887,7 +3886,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(const std::string& name,
if (gArgs.GetBoolArg("-zapwallettxes", false)) {
uiInterface.InitMessage(_("Zapping all transactions from wallet..."));
std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(name, WalletDatabase::Create(path));
std::unique_ptr<CWallet> tempWallet = MakeUnique<CWallet>(location, WalletDatabase::Create(location.GetPath()));
DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx);
if (nZapWalletRet != DBErrors::LOAD_OK) {
InitError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile));
@ -3901,7 +3900,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(const std::string& name,
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<CWallet> walletInstance(new CWallet(name, WalletDatabase::Create(path)), ReleaseWallet);
std::shared_ptr<CWallet> walletInstance(new CWallet(location, WalletDatabase::Create(location.GetPath())), ReleaseWallet);
DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun);
if (nLoadWalletRet != DBErrors::LOAD_OK)
{

20
src/wallet/wallet.h
View file

@ -20,7 +20,7 @@
#include <wallet/crypter.h>
#include <wallet/coinselection.h>
#include <wallet/walletdb.h>
#include <wallet/rpcwallet.h>
#include <wallet/walletutil.h>
#include <algorithm>
#include <atomic>
@ -676,12 +676,8 @@ private:
*/
bool AddWatchOnly(const CScript& dest) override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/**
* Wallet filename from wallet=<path> 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.
*/
std::string m_name;
/** Wallet location which includes wallet name (see WalletLocation). */
WalletLocation m_location;
/** Internal database handle. */
std::unique_ptr<WalletDatabase> database;
@ -721,9 +717,11 @@ public:
bool SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& 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_name; }
const std::string& GetName() const { return m_location.GetName(); }
void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void MarkPreSplitKeys() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
@ -739,7 +737,7 @@ public:
unsigned int nMasterKeyMaxID = 0;
/** Construct wallet with specified name and database implementation. */
CWallet(std::string name, std::unique_ptr<WalletDatabase> database) : m_name(std::move(name)), database(std::move(database))
CWallet(const WalletLocation& location, std::unique_ptr<WalletDatabase> database) : m_location(location), database(std::move(database))
{
}
@ -1059,10 +1057,10 @@ public:
bool MarkReplaced(const uint256& originalHash, const uint256& newHash);
//! Verify wallet naming and perform salvage on the wallet if required
static bool Verify(std::string wallet_file, bool salvage_wallet, std::string& error_string, std::string& warning_string);
static bool Verify(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 in case of an error */
static std::shared_ptr<CWallet> CreateWalletFromFile(const std::string& name, const fs::path& path, uint64_t wallet_creation_flags = 0);
static std::shared_ptr<CWallet> CreateWalletFromFile(const WalletLocation& location, uint64_t wallet_creation_flags = 0);
/**
* Wallet post-init setup

11
src/wallet/walletutil.cpp
View file

@ -80,3 +80,14 @@ std::vector<fs::path> ListWalletDir()
return paths;
}
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;
}

20
src/wallet/walletutil.h
View file

@ -15,4 +15,24 @@ fs::path GetWalletDir();
//! Get wallets in wallet directory.
std::vector<fs::path> 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;
};
#endif // BITCOIN_WALLET_WALLETUTIL_H

1
test/lint/lint-circular-dependencies.sh
View file

@ -29,7 +29,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"validation -> validationinterface -> validation"
"wallet/coincontrol -> wallet/wallet -> wallet/coincontrol"
"wallet/fees -> wallet/wallet -> wallet/fees"
"wallet/rpcwallet -> wallet/wallet -> wallet/rpcwallet"
"wallet/wallet -> wallet/walletdb -> wallet/wallet"
"policy/fees -> policy/policy -> validation -> policy/fees"
"policy/rbf -> txmempool -> validation -> policy/rbf"