From 1dc22190bc1b6e48dd0f18462c5143dd0967c489 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 18 Jan 2018 13:15:00 -0500 Subject: [PATCH] Don't allow relative -walletdir paths Also warn if bitcoind is configured to use a relative -datadir path. Specifying paths relative to the current working directory in a daemon process can be dangerous, because files can fail to be located even if the configuration doesn't change, but the daemon is started up differently. Specifying a relative -datadir now adds a warning to the debug log. It would not be backwards-compatible to forbid relative -datadir paths entirely, and it could also be also inconvenient for command line testing. Specifying a relative -walletdir now results in a startup error. But since the -walletdir option is new in 0.16.0, there should be no compatibility issues. Another reason not to use working directory paths for -walletdir specifically is that the default -walletdir is a "wallets" subdirectory inside the datadir, so it could be surprising that setting -walletdir manually would choose a directory rooted in a completely different location. --- doc/release-notes.md | 33 ++++++++++++++++----------- src/init.cpp | 9 ++++++++ src/wallet/init.cpp | 12 ++++++---- src/wallet/walletutil.cpp | 2 +- test/functional/wallet_multiwallet.py | 5 ++-- 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index 62d81590fc4bb..6067946c925c6 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -299,19 +299,26 @@ The `getwalletinfo` RPC method returns the name of the wallet used for the query Note that while multi-wallet is now fully supported, the RPC multi-wallet interface should be considered unstable for version 6.0.0, and there may backwards-incompatible changes in future versions. -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=` -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. +Wallets directory configuration (`-walletdir`) +---------------------------------------------- + +PIVX Core now has more flexibility in where the wallets directory can be +located. Previously wallet database files were stored at the top level of the +PIVX data directory. The behavior is now: + +- For new installations (where the data directory doesn't already exist), + wallets will now be stored in a new `wallets/` subdirectory inside the data + directory by default. +- For existing nodes (where the data directory already exists), wallets will be + stored in the data directory root by default. If a `wallets/` subdirectory + already exists in the data directory root, then wallets will be stored in the + `wallets/` subdirectory by default. +- The location of the wallets directory can be overridden by specifying a + `-walletdir=` option where `` can be an absolute path to a + directory or directory symlink. + +Care should be taken when choosing the wallets directory location, as if it +becomes unavailable during operation, funds may be lost. Database cache memory increased -------------------------------- diff --git a/src/init.cpp b/src/init.cpp index 9a80c481892be..236e668cb47d9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1272,6 +1272,15 @@ bool AppInitMain() LogPrintf("Using at most %i connections (%i file descriptors available)\n", nMaxConnections, nFD); std::ostringstream strErrors; + // Warn about relative -datadir path. + if (gArgs.IsArgSet("-datadir") && !fs::path(gArgs.GetArg("-datadir", "")).is_absolute()) { + LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " + "current working directory '%s'. This is fragile, because if bitcoin is started in the future " + "from a different location, it will be unable to locate the current data files. There could " + "also be data loss if bitcoin is started while in a temporary directory.\n", + gArgs.GetArg("-datadir", ""), fs::current_path().string()); + } + InitSignatureCache(); LogPrintf("Using %u threads for script verification\n", nScriptCheckThreads); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 0f7e0fd065167..4ce0ccf492790 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -148,11 +148,15 @@ bool WalletVerify() return true; } - if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { - if (fs::exists(fs::system_complete(gArgs.GetArg("-walletdir", "")))) { - return UIError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), gArgs.GetArg("-walletdir", "").c_str())); + if (gArgs.IsArgSet("-walletdir")) { + fs::path wallet_dir = gArgs.GetArg("-walletdir", ""); + if (!fs::exists(wallet_dir)) { + return UIError(strprintf(_("Specified -walletdir \"%s\" does not exist"), wallet_dir.string())); + } else if (!fs::is_directory(wallet_dir)) { + return UIError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), wallet_dir.string())); + } else if (!wallet_dir.is_absolute()) { + return UIError(strprintf(_("Specified -walletdir \"%s\" is a relative path"), wallet_dir.string())); } - return UIError(strprintf(_("Specified -walletdir \"%s\" does not exist"), gArgs.GetArg("-walletdir", "").c_str())); } LogPrintf("Using wallet directory %s\n", GetWalletDir().string()); diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index 2f47d911640a8..6bccc7182d985 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -10,7 +10,7 @@ fs::path GetWalletDir() fs::path path; if (gArgs.IsArgSet("-walletdir")) { - path = fs::system_complete(gArgs.GetArg("-walletdir", "")); + path = gArgs.GetArg("-walletdir", ""); if (!fs::is_directory(path)) { // If the path specified doesn't exist, we return the deliberately // invalid empty string. diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index ee3594bd15149..00dc3b9930116 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -33,10 +33,9 @@ def run_test(self): self.stop_nodes() - # !TODO: backport bitcoin#12220 self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') - #self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) - #self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir()) + self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) + self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir()) # should not initialize if there are duplicate wallets self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')