sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-25 20:12:57 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge #11466: Specify custom wallet directory with -walletdir param

Browse Source 
c1e5d40 Make debugging test crash easier (MeshCollider)
8263f6a Create walletdir if datadir doesn't exist and fix tests (MeshCollider)
9587a9c Default walletdir is wallets/ if it exists (MeshCollider)
d987889 Add release notes for -walletdir and wallets/ dir (MeshCollider)
80c5cbc Add test for -walletdir (MeshCollider)
0530ba0 Add -walletdir parameter to specify custom wallet dir (MeshCollider)

Pull request description:

  Closes #11348

  Adds a `-walletdir` parameter which specifies a directory to use for wallets, allowing them to be stored separately from the 'main' data directory. Creates a new `wallets/` directory in datadir if this is the first time running, and defaults to using it if it exists.

  Includes tests and release notes. Things which might need to be considered more:
  - there is no 'lock' on the wallets directory, which might be needed?
  - because this uses a new wallets/ directory by default, downgrading to an earlier version won't see the wallets in that directory (not a big deal though, users can just copy them up to the main dir)
  - jnewbery suggested putting each wallet in its own directory, which is a good idea, but out of scope for this PR IMO. EDIT: this is being done in https://github.com/bitcoin/bitcoin/pull/11687
  - doc/files.md needs updating (will do soon)

  I also considered including  a cleanup by removing caching of data directory paths and instead just initialise them once on startup (c.f. #3073), but decided it wasn't super relevant here will just complicate review.

Tree-SHA512: c8ac04bfe9a810c32055f2c8b8fa0d535e56125ceb8d96f12447dd3538bf3e5ee992b60b1cd2173bf5f3fa023a9feab12c9963593bf27ed419df929bb413398d
This commit is contained in:
Wladimir J. van der Laan 2017-11-18 14:32:50 +01:00
commit d080a7d503
No known key found for this signature in database
GPG Key ID: 1E4AED62986CD25D
18 changed files with 144 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
doc/release-notes.md
View File

@ -62,6 +62,20 @@ Due to a backward-incompatible change in the wallet database, wallets created
with version 0.16.0 will be rejected by previous versions. Also, version 0.16.0
will only create hierarchical deterministic (HD) wallets.
Custom wallet directories
---------------------
The ability to specify a directory other than the default data directory in which to store
wallets has been added. An existing directory can be specified using the `-walletdir=<dir>`
argument. Wallets loaded via `-wallet` arguments must be in this wallet directory. Care should be taken
when choosing a wallet directory location, as if it becomes unavailable during operation,
funds may be lost.
Default wallet directory change
--------------------------
On new installations (if the data directory doesn't exist), wallets will now be stored in a
new `wallets/` subdirectory inside the data directory. If this `wallets/` subdirectory
doesn't exist (i.e. on existing nodes), the current datadir root is used instead, as it was.
Low-level RPC changes
----------------------
- The deprecated RPC `getinfo` was removed. It is recommended that the more specific RPCs are used:

2
src/Makefile.am
View File

@ -168,6 +168,7 @@ BITCOIN_CORE_H = \
wallet/rpcwallet.h \
wallet/wallet.h \
wallet/walletdb.h \
wallet/walletutil.h \
warnings.h \
zmq/zmqabstractnotifier.h \
zmq/zmqconfig.h\
@ -249,6 +250,7 @@ libbitcoin_wallet_a_SOURCES = \
wallet/rpcwallet.cpp \
wallet/wallet.cpp \
wallet/walletdb.cpp \
wallet/walletutil.cpp \
$(BITCOIN_CORE_H)
# crypto primitives library

5
src/qt/intro.cpp
View File

@ -214,7 +214,10 @@ bool Intro::pickDataDirectory()
}
dataDir = intro.getDataDirectory();
try {
TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir));
if (TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir))) {
// If a new data directory has been created, make wallets subdirectory too
TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir) / "wallets");
}
break;
} catch (const fs::filesystem_error&) {
QMessageBox::critical(0, tr(PACKAGE_NAME),

5
src/util.cpp
View File

@ -574,7 +574,10 @@ const fs::path &GetDataDir(bool fNetSpecific)
if (fNetSpecific)
path /= BaseParams().DataDir();
fs::create_directories(path);
if (fs::create_directories(path)) {
// This is the first run, create wallets subdirectory too
fs::create_directories(path / "wallets");
}
return path;
}

25
src/wallet/db.cpp
View File

@ -11,6 +11,7 @@
#include <protocol.h>
#include <util.h>
#include <utilstrencodings.h>
#include <wallet/walletutil.h>
#include <stdint.h>
@ -257,7 +258,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco
return fSuccess;
}
bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr)
bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr)
{
LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0));
LogPrintf("Using wallet %s\n", walletFile);
@ -265,15 +266,15 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataD
// Wallet file must be a plain filename without a directory
if (walletFile != fs::basename(walletFile) + fs::extension(walletFile))
{
errorStr = strprintf(_("Wallet %s resides outside data directory %s"), walletFile, dataDir.string());
errorStr = strprintf(_("Wallet %s resides outside wallet directory %s"), walletFile, walletDir.string());
return false;
}
if (!bitdb.Open(dataDir))
if (!bitdb.Open(walletDir))
{
// try moving the database env out of the way
fs::path pathDatabase = dataDir / "database";
fs::path pathDatabaseBak = dataDir / strprintf("database.%d.bak", GetTime());
fs::path pathDatabase = walletDir / "database";
fs::path pathDatabaseBak = walletDir / strprintf("database.%d.bak", GetTime());
try {
fs::rename(pathDatabase, pathDatabaseBak);
LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string());
@ -282,18 +283,18 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataD
}
// try again
if (!bitdb.Open(dataDir)) {
if (!bitdb.Open(walletDir)) {
// if it still fails, it probably means we can't even create the database env
errorStr = strprintf(_("Error initializing wallet database environment %s!"), GetDataDir());
errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir);
return false;
}
}
return true;
}
bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc)
bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc)
{
if (fs::exists(dataDir / walletFile))
if (fs::exists(walletDir / walletFile))
{
std::string backup_filename;
CDBEnv::VerifyResult r = bitdb.Verify(walletFile, recoverFunc, backup_filename);
@ -303,7 +304,7 @@ bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& data
" Original %s saved as %s in %s; if"
" your balance or transactions are incorrect you should"
" restore from a backup."),
walletFile, backup_filename, dataDir);
walletFile, backup_filename, walletDir);
}
if (r == CDBEnv::RECOVER_FAIL)
{
@ -407,7 +408,7 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb
{
LOCK(env->cs_db);
if (!env->Open(GetDataDir()))
if (!env->Open(GetWalletDir()))
throw std::runtime_error("CDB: Failed to open database environment.");
pdb = env->mapDb[strFilename];
@ -695,7 +696,7 @@ bool CWalletDBWrapper::Backup(const std::string& strDest)
env->mapFileUseCount.erase(strFile);
// Copy wallet file
fs::path pathSrc = GetDataDir() / strFile;
fs::path pathSrc = GetWalletDir() / strFile;
fs::path pathDest(strDest);
if (fs::is_directory(pathDest))
pathDest /= strFile;

4
src/wallet/db.h
View File

@ -167,9 +167,9 @@ public:
ideal to be called periodically */
static bool PeriodicFlush(CWalletDBWrapper& dbw);
/* verifies the database environment */
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr);
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr);
/* verifies the database file */
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc);
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc);
public:
template <typename K, typename T>

16
src/wallet/init.cpp
View File

@ -9,8 +9,9 @@
#include <util.h>
#include <utilmoneystr.h>
#include <validation.h>
#include <wallet/wallet.h>
#include <wallet/rpcwallet.h>
#include <wallet/wallet.h>
#include <wallet/walletutil.h>
std::string GetWalletHelpString(bool showDebug)
{
@ -34,6 +35,7 @@ std::string GetWalletHelpString(bool showDebug)
strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup"));
strUsage += HelpMessageOpt("-wallet=<file>", _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT));
strUsage += HelpMessageOpt("-walletbroadcast", _("Make the wallet broadcast transactions") + " " + strprintf(_("(default: %u)"), DEFAULT_WALLETBROADCAST));
strUsage += HelpMessageOpt("-walletdir=<dir>", _("Specify directory to hold wallets (default: <datadir>/wallets if it exists, otherwise <datadir>)"));
strUsage += HelpMessageOpt("-walletnotify=<cmd>", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)"));
strUsage += HelpMessageOpt("-zapwallettxes=<mode>", _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") +
" " + _("(1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)"));
@ -191,6 +193,12 @@ bool VerifyWallets()
return true;
}
if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) {
return InitError(strprintf(_("Error: Specified wallet directory \"%s\" does not exist."), gArgs.GetArg("-walletdir", "").c_str()));
}
LogPrintf("Using wallet directory %s\n", GetWalletDir().string());
uiInterface.InitMessage(_("Verifying wallet(s)..."));
// Keep track of each wallet absolute path to detect duplicates.
@ -205,7 +213,7 @@ bool VerifyWallets()
return InitError(strprintf(_("Error loading wallet %s. Invalid characters in -wallet filename."), walletFile));
}
fs::path wallet_path = fs::absolute(walletFile, GetDataDir());
fs::path wallet_path = fs::absolute(walletFile, GetWalletDir());
if (fs::exists(wallet_path) && (!fs::is_regular_file(wallet_path) || fs::is_symlink(wallet_path))) {
return InitError(strprintf(_("Error loading wallet %s. -wallet filename must be a regular file."), walletFile));
@ -216,7 +224,7 @@ bool VerifyWallets()
}
std::string strError;
if (!CWalletDB::VerifyEnvironment(walletFile, GetDataDir().string(), strError)) {
if (!CWalletDB::VerifyEnvironment(walletFile, GetWalletDir().string(), strError)) {
return InitError(strError);
}
@ -230,7 +238,7 @@ bool VerifyWallets()
}
std::string strWarning;
bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetDataDir().string(), strWarning, strError);
bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetWalletDir().string(), strWarning, strError);
if (!strWarning.empty()) {
InitWarning(strWarning);
}

1
src/wallet/rpcwallet.cpp
View File

@ -26,6 +26,7 @@
#include <wallet/feebumper.h>
#include <wallet/wallet.h>
#include <wallet/walletdb.h>
#include <wallet/walletutil.h>
#include <init.h> // For StartShutdown

8
src/wallet/walletdb.cpp
View File

@ -814,14 +814,14 @@ bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDa
return true;
}
bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr)
bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr)
{
return CDB::VerifyEnvironment(walletFile, dataDir, errorStr);
return CDB::VerifyEnvironment(walletFile, walletDir, errorStr);
}
bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr)
bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr)
{
return CDB::VerifyDatabaseFile(walletFile, dataDir, warningStr, errorStr, CWalletDB::Recover);
return CDB::VerifyDatabaseFile(walletFile, walletDir, warningStr, errorStr, CWalletDB::Recover);
}
bool CWalletDB::WriteDestData(const std::string &address, const std::string &key, const std::string &value)

4
src/wallet/walletdb.h
View File

@ -226,9 +226,9 @@ public:
/* Function to determine if a certain KV/key-type is a key (cryptographical key) type */
static bool IsKeyType(const std::string& strType);
/* verifies the database environment */
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr);
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr);
/* verifies the database file */
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr);
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr);
//! write the hdchain model (external chain child index counter)
bool WriteHDChain(const CHDChain& chain);

27
src/wallet/walletutil.cpp Normal file
View File

@ -0,0 +1,27 @@
// Copyright (c) 2017 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include "wallet/walletutil.h"
fs::path GetWalletDir()
{
fs::path path;
if (gArgs.IsArgSet("-walletdir")) {
path = fs::system_complete(gArgs.GetArg("-walletdir", ""));
if (!fs::is_directory(path)) {
// If the path specified doesn't exist, we return the deliberately
// invalid empty string.
path = "";
}
} else {
path = GetDataDir();
// If a wallets directory exists, use that, otherwise default to GetDataDir
if (fs::is_directory(path / "wallets")) {
path /= "wallets";
}
}
return path;
}

13
src/wallet/walletutil.h Normal file
View File

@ -0,0 +1,13 @@
// Copyright (c) 2017 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef BITCOIN_WALLET_UTIL_H
#define BITCOIN_WALLET_UTIL_H
#include "util.h"
//! Get the path of the wallet directory.
fs::path GetWalletDir();
#endif // BITCOIN_WALLET_UTIL_H

4
test/functional/keypool-topup.py
View File

@ -33,7 +33,7 @@ class KeypoolRestoreTest(BitcoinTestFramework):
self.stop_node(1)
shutil.copyfile(self.tmpdir + "/node1/regtest/wallet.dat", self.tmpdir + "/wallet.bak")
shutil.copyfile(self.tmpdir + "/node1/regtest/wallets/wallet.dat", self.tmpdir + "/wallet.bak")
self.start_node(1, self.extra_args[1])
connect_nodes_bi(self.nodes, 0, 1)
@ -56,7 +56,7 @@ class KeypoolRestoreTest(BitcoinTestFramework):
self.stop_node(1)
shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallet.dat")
shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallets/wallet.dat")
self.log.info("Verify keypool is restored and balance is correct")

30
test/functional/multiwallet.py
View File

@ -27,18 +27,40 @@ class MultiWalletTest(BitcoinTestFramework):
self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')
# should not initialize if wallet file is a directory
os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11'))
wallet_dir = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'wallets')
os.mkdir(os.path.join(wallet_dir, 'w11'))
self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.')
# should not initialize if one wallet is a copy of another
shutil.copyfile(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w2'),
os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w22'))
shutil.copyfile(os.path.join(wallet_dir, 'w2'), os.path.join(wallet_dir, 'w22'))
self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid')
# should not initialize if wallet file is a symlink
os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12'))
os.symlink(os.path.join(wallet_dir, 'w1'), os.path.join(wallet_dir, 'w12'))
self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.')
# should not initialize if the specified walletdir does not exist
self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified wallet directory "bad" does not exist.')
# if wallets/ doesn't exist, datadir should be the default wallet dir
wallet_dir2 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'walletdir')
os.rename(wallet_dir, wallet_dir2)
self.start_node(0, ['-wallet=w4', '-wallet=w5'])
assert_equal(set(self.nodes[0].listwallets()), {"w4", "w5"})
w5 = self.nodes[0].get_wallet_rpc("w5")
w5.generate(1)
self.stop_node(0)
# now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded
os.rename(wallet_dir2, wallet_dir)
self.start_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + os.path.join(self.options.tmpdir, 'node0', 'regtest')])
assert_equal(set(self.nodes[0].listwallets()), {"w4", "w5"})
w5 = self.nodes[0].get_wallet_rpc("w5")
w5_info = w5.getwalletinfo()
assert_equal(w5_info['immature_balance'], 50)
self.stop_node(0)
self.start_node(0, self.extra_args[0])
w1 = self.nodes[0].get_wallet_rpc("w1")

2
test/functional/test_framework/test_framework.py
View File

@ -432,7 +432,7 @@ class BitcoinTestFramework():
self.disable_mocktime()
for i in range(MAX_NODES):
os.remove(log_filename(self.options.cachedir, i, "debug.log"))
os.remove(log_filename(self.options.cachedir, i, "db.log"))
os.remove(log_filename(self.options.cachedir, i, "wallets/db.log"))
os.remove(log_filename(self.options.cachedir, i, "peers.dat"))
os.remove(log_filename(self.options.cachedir, i, "fee_estimates.dat"))

4
test/functional/test_runner.py
View File

@ -300,7 +300,11 @@ def run_tests(test_list, src_dir, build_dir, exeext, tmpdir, jobs=1, enable_cove
if len(test_list) > 1 and jobs > 1:
# Populate cache
try:
subprocess.check_output([tests_dir + 'create_cache.py'] + flags + ["--tmpdir=%s/cache" % tmpdir])
except Exception as e:
print(e.output)
raise e
#Run Tests
job_queue = TestHandler(jobs, tests_dir, tmpdir, test_list, flags)

2
test/functional/wallet-hd.py
View File

@ -73,7 +73,7 @@ class WalletHDTest(BitcoinTestFramework):
# otherwise node1 would auto-recover all funds in flag the keypool keys as used
shutil.rmtree(os.path.join(tmpdir, "node1/regtest/blocks"))
shutil.rmtree(os.path.join(tmpdir, "node1/regtest/chainstate"))
shutil.copyfile(os.path.join(tmpdir, "hd.bak"), os.path.join(tmpdir, "node1/regtest/wallet.dat"))
shutil.copyfile(os.path.join(tmpdir, "hd.bak"), os.path.join(tmpdir, "node1/regtest/wallets/wallet.dat"))
self.start_node(1)
# Assert that derivation is deterministic

20
test/functional/walletbackup.py
View File

@ -90,9 +90,9 @@ class WalletBackupTest(BitcoinTestFramework):
self.stop_node(2)
def erase_three(self):
os.remove(self.options.tmpdir + "/node0/regtest/wallet.dat")
os.remove(self.options.tmpdir + "/node1/regtest/wallet.dat")
os.remove(self.options.tmpdir + "/node2/regtest/wallet.dat")
os.remove(self.options.tmpdir + "/node0/regtest/wallets/wallet.dat")
os.remove(self.options.tmpdir + "/node1/regtest/wallets/wallet.dat")
os.remove(self.options.tmpdir + "/node2/regtest/wallets/wallet.dat")
def run_test(self):
self.log.info("Generating initial blockchain")
@ -154,9 +154,9 @@ class WalletBackupTest(BitcoinTestFramework):
shutil.rmtree(self.options.tmpdir + "/node2/regtest/chainstate")
# Restore wallets from backup
shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallet.dat")
shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallet.dat")
shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallet.dat")
shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallets/wallet.dat")
shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallets/wallet.dat")
shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallets/wallet.dat")
self.log.info("Re-starting nodes")
self.start_three()
@ -192,10 +192,10 @@ class WalletBackupTest(BitcoinTestFramework):
# Backup to source wallet file must fail
sourcePaths = [
tmpdir + "/node0/regtest/wallet.dat",
tmpdir + "/node0/./regtest/wallet.dat",
tmpdir + "/node0/regtest/",
tmpdir + "/node0/regtest"]
tmpdir + "/node0/regtest/wallets/wallet.dat",
tmpdir + "/node0/./regtest/wallets/wallet.dat",
tmpdir + "/node0/regtest/wallets/",
tmpdir + "/node0/regtest/wallets"]
for sourcePath in sourcePaths:
assert_raises_rpc_error(-4, "backup failed", self.nodes[0].backupwallet, sourcePath)