Skip to content

Commit d080a7d

Browse files
laanwjlaanwj
committed
Merge #11466: Specify custom wallet directory with -walletdir param
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 bitcoin/bitcoin#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
2 parents 49667a7 + c1e5d40 commit d080a7d

18 files changed

+144
-46
lines changed

‎doc/release-notes.md

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ How to Upgrade
2020
==============
2121

2222
If you are running an older version, shut it down. Wait until it has completely
23-
shut down (which might take a few minutes for older versions), then run the
23+
shut down (which might take a few minutes for older versions), then run the
2424
installer (on Windows) or just copy over `/Applications/Bitcoin-Qt` (on Mac)
2525
or `bitcoind`/`bitcoin-qt` (on Linux).
2626

@@ -62,6 +62,20 @@ Due to a backward-incompatible change in the wallet database, wallets created
6262
with version 0.16.0 will be rejected by previous versions. Also, version 0.16.0
6363
will only create hierarchical deterministic (HD) wallets.
6464

65+
Custom wallet directories
66+
---------------------
67+
The ability to specify a directory other than the default data directory in which to store
68+
wallets has been added. An existing directory can be specified using the `-walletdir=<dir>`
69+
argument. Wallets loaded via `-wallet` arguments must be in this wallet directory. Care should be taken
70+
when choosing a wallet directory location, as if it becomes unavailable during operation,
71+
funds may be lost.
72+
73+
Default wallet directory change
74+
--------------------------
75+
On new installations (if the data directory doesn't exist), wallets will now be stored in a
76+
new `wallets/` subdirectory inside the data directory. If this `wallets/` subdirectory
77+
doesn't exist (i.e. on existing nodes), the current datadir root is used instead, as it was.
78+
6579
Low-level RPC changes
6680
----------------------
6781
- The deprecated RPC `getinfo` was removed. It is recommended that the more specific RPCs are used:

‎src/Makefile.am

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ BITCOIN_CORE_H = \
168168
wallet/rpcwallet.h \
169169
wallet/wallet.h \
170170
wallet/walletdb.h \
171+
wallet/walletutil.h \
171172
warnings.h \
172173
zmq/zmqabstractnotifier.h \
173174
zmq/zmqconfig.h\
@@ -249,6 +250,7 @@ libbitcoin_wallet_a_SOURCES = \
249250
wallet/rpcwallet.cpp \
250251
wallet/wallet.cpp \
251252
wallet/walletdb.cpp \
253+
wallet/walletutil.cpp \
252254
$(BITCOIN_CORE_H)
253255

254256
# crypto primitives library

‎src/qt/intro.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,10 @@ bool Intro::pickDataDirectory()
214214
}
215215
dataDir = intro.getDataDirectory();
216216
try {
217-
TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir));
217+
if (TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir))) {
218+
// If a new data directory has been created, make wallets subdirectory too
219+
TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir) / "wallets");
220+
}
218221
break;
219222
} catch (const fs::filesystem_error&) {
220223
QMessageBox::critical(0, tr(PACKAGE_NAME),

‎src/util.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,10 @@ const fs::path &GetDataDir(bool fNetSpecific)
574574
if (fNetSpecific)
575575
path /= BaseParams().DataDir();
576576

577-
fs::create_directories(path);
577+
if (fs::create_directories(path)) {
578+
// This is the first run, create wallets subdirectory too
579+
fs::create_directories(path / "wallets");
580+
}
578581

579582
return path;
580583
}

‎src/wallet/db.cpp

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <protocol.h>
1212
#include <util.h>
1313
#include <utilstrencodings.h>
14+
#include <wallet/walletutil.h>
1415

1516
#include <stdint.h>
1617

@@ -257,23 +258,23 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco
257258
return fSuccess;
258259
}
259260

260-
bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr)
261+
bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr)
261262
{
262263
LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0));
263264
LogPrintf("Using wallet %s\n", walletFile);
264265

265266
// Wallet file must be a plain filename without a directory
266267
if (walletFile != fs::basename(walletFile) + fs::extension(walletFile))
267268
{
268-
errorStr = strprintf(_("Wallet %s resides outside data directory %s"), walletFile, dataDir.string());
269+
errorStr = strprintf(_("Wallet %s resides outside wallet directory %s"), walletFile, walletDir.string());
269270
return false;
270271
}
271272

272-
if (!bitdb.Open(dataDir))
273+
if (!bitdb.Open(walletDir))
273274
{
274275
// try moving the database env out of the way
275-
fs::path pathDatabase = dataDir / "database";
276-
fs::path pathDatabaseBak = dataDir / strprintf("database.%d.bak", GetTime());
276+
fs::path pathDatabase = walletDir / "database";
277+
fs::path pathDatabaseBak = walletDir / strprintf("database.%d.bak", GetTime());
277278
try {
278279
fs::rename(pathDatabase, pathDatabaseBak);
279280
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
282283
}
283284

284285
// try again
285-
if (!bitdb.Open(dataDir)) {
286+
if (!bitdb.Open(walletDir)) {
286287
// if it still fails, it probably means we can't even create the database env
287-
errorStr = strprintf(_("Error initializing wallet database environment %s!"), GetDataDir());
288+
errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir);
288289
return false;
289290
}
290291
}
291292
return true;
292293
}
293294

294-
bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc)
295+
bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc)
295296
{
296-
if (fs::exists(dataDir / walletFile))
297+
if (fs::exists(walletDir / walletFile))
297298
{
298299
std::string backup_filename;
299300
CDBEnv::VerifyResult r = bitdb.Verify(walletFile, recoverFunc, backup_filename);
@@ -303,7 +304,7 @@ bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& data
303304
" Original %s saved as %s in %s; if"
304305
" your balance or transactions are incorrect you should"
305306
" restore from a backup."),
306-
walletFile, backup_filename, dataDir);
307+
walletFile, backup_filename, walletDir);
307308
}
308309
if (r == CDBEnv::RECOVER_FAIL)
309310
{
@@ -407,7 +408,7 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb
407408

408409
{
409410
LOCK(env->cs_db);
410-
if (!env->Open(GetDataDir()))
411+
if (!env->Open(GetWalletDir()))
411412
throw std::runtime_error("CDB: Failed to open database environment.");
412413

413414
pdb = env->mapDb[strFilename];
@@ -695,7 +696,7 @@ bool CWalletDBWrapper::Backup(const std::string& strDest)
695696
env->mapFileUseCount.erase(strFile);
696697

697698
// Copy wallet file
698-
fs::path pathSrc = GetDataDir() / strFile;
699+
fs::path pathSrc = GetWalletDir() / strFile;
699700
fs::path pathDest(strDest);
700701
if (fs::is_directory(pathDest))
701702
pathDest /= strFile;

‎src/wallet/db.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ class CDB
167167
ideal to be called periodically */
168168
static bool PeriodicFlush(CWalletDBWrapper& dbw);
169169
/* verifies the database environment */
170-
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr);
170+
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr);
171171
/* verifies the database file */
172-
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc);
172+
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc);
173173

174174
public:
175175
template <typename K, typename T>

‎src/wallet/init.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99
#include <util.h>
1010
#include <utilmoneystr.h>
1111
#include <validation.h>
12-
#include <wallet/wallet.h>
1312
#include <wallet/rpcwallet.h>
13+
#include <wallet/wallet.h>
14+
#include <wallet/walletutil.h>
1415

1516
std::string GetWalletHelpString(bool showDebug)
1617
{
@@ -34,6 +35,7 @@ std::string GetWalletHelpString(bool showDebug)
3435
strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup"));
3536
strUsage += HelpMessageOpt("-wallet=<file>", _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT));
3637
strUsage += HelpMessageOpt("-walletbroadcast", _("Make the wallet broadcast transactions") + " " + strprintf(_("(default: %u)"), DEFAULT_WALLETBROADCAST));
38+
strUsage += HelpMessageOpt("-walletdir=<dir>", _("Specify directory to hold wallets (default: <datadir>/wallets if it exists, otherwise <datadir>)"));
3739
strUsage += HelpMessageOpt("-walletnotify=<cmd>", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)"));
3840
strUsage += HelpMessageOpt("-zapwallettxes=<mode>", _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") +
3941
" " + _("(1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)"));
@@ -191,6 +193,12 @@ bool VerifyWallets()
191193
return true;
192194
}
193195

196+
if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) {
197+
return InitError(strprintf(_("Error: Specified wallet directory \"%s\" does not exist."), gArgs.GetArg("-walletdir", "").c_str()));
198+
}
199+
200+
LogPrintf("Using wallet directory %s\n", GetWalletDir().string());
201+
194202
uiInterface.InitMessage(_("Verifying wallet(s)..."));
195203

196204
// Keep track of each wallet absolute path to detect duplicates.
@@ -205,7 +213,7 @@ bool VerifyWallets()
205213
return InitError(strprintf(_("Error loading wallet %s. Invalid characters in -wallet filename."), walletFile));
206214
}
207215

208-
fs::path wallet_path = fs::absolute(walletFile, GetDataDir());
216+
fs::path wallet_path = fs::absolute(walletFile, GetWalletDir());
209217

210218
if (fs::exists(wallet_path) && (!fs::is_regular_file(wallet_path) || fs::is_symlink(wallet_path))) {
211219
return InitError(strprintf(_("Error loading wallet %s. -wallet filename must be a regular file."), walletFile));
@@ -216,7 +224,7 @@ bool VerifyWallets()
216224
}
217225

218226
std::string strError;
219-
if (!CWalletDB::VerifyEnvironment(walletFile, GetDataDir().string(), strError)) {
227+
if (!CWalletDB::VerifyEnvironment(walletFile, GetWalletDir().string(), strError)) {
220228
return InitError(strError);
221229
}
222230

@@ -230,7 +238,7 @@ bool VerifyWallets()
230238
}
231239

232240
std::string strWarning;
233-
bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetDataDir().string(), strWarning, strError);
241+
bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetWalletDir().string(), strWarning, strError);
234242
if (!strWarning.empty()) {
235243
InitWarning(strWarning);
236244
}

‎src/wallet/rpcwallet.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include <wallet/feebumper.h>
2727
#include <wallet/wallet.h>
2828
#include <wallet/walletdb.h>
29+
#include <wallet/walletutil.h>
2930

3031
#include <init.h> // For StartShutdown
3132

‎src/wallet/walletdb.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -814,14 +814,14 @@ bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDa
814814
return true;
815815
}
816816

817-
bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr)
817+
bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr)
818818
{
819-
return CDB::VerifyEnvironment(walletFile, dataDir, errorStr);
819+
return CDB::VerifyEnvironment(walletFile, walletDir, errorStr);
820820
}
821821

822-
bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr)
822+
bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr)
823823
{
824-
return CDB::VerifyDatabaseFile(walletFile, dataDir, warningStr, errorStr, CWalletDB::Recover);
824+
return CDB::VerifyDatabaseFile(walletFile, walletDir, warningStr, errorStr, CWalletDB::Recover);
825825
}
826826

827827
bool CWalletDB::WriteDestData(const std::string &address, const std::string &key, const std::string &value)

‎src/wallet/walletdb.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,9 @@ class CWalletDB
226226
/* Function to determine if a certain KV/key-type is a key (cryptographical key) type */
227227
static bool IsKeyType(const std::string& strType);
228228
/* verifies the database environment */
229-
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr);
229+
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr);
230230
/* verifies the database file */
231-
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr);
231+
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr);
232232

233233
//! write the hdchain model (external chain child index counter)
234234
bool WriteHDChain(const CHDChain& chain);

0 commit comments

Comments
 (0)