From be8ab7d082228d09ca529d1a08730d7d5aacb0ed Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 15 Nov 2017 15:44:36 -0500 Subject: [PATCH] Create new wallet databases as directories rather than files This change should make it easier for users to make complete backups of wallets because they can now just back up the specified `-wallet=` path directly, instead of having to back up the specified path as well as the transaction log directory (for incompletely flushed wallets). Another advantage of this change is that if two wallets are located in the same directory, they will now use their own BerkeleyDB environments instead using a shared environment. Using a shared environment makes it difficult to manage and back up wallets separately because transaction log files will contain a mix of data from all wallets in the environment. --- doc/release-notes.md | 29 +++++++++++++++++++--- src/bitcoin-cli.cpp | 4 +-- src/wallet/db.cpp | 15 ++++++++++-- src/wallet/init.cpp | 22 +++++++++++++---- src/wallet/wallet.cpp | 1 - src/wallet/wallet.h | 2 -- test/functional/feature_config_args.py | 4 +-- test/functional/wallet_multiwallet.py | 34 +++++++++++++++++--------- 8 files changed, 81 insertions(+), 30 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index 171880a77b..8fcd2a9163 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -67,10 +67,31 @@ External wallet files --------------------- The `-wallet=` option now accepts full paths instead of requiring wallets -to be located in the -walletdir directory. When wallets are located in -different directories, wallet data will be stored independently, so data from -every wallet is not mixed into the same /database/log.?????????? -files. +to be located in the -walletdir directory. + +Newly created wallet format +--------------------------- + +If `-wallet=` is specified with a path that does not exist, it will now +create a wallet directory at the specified location (containing a wallet.dat +data file, a db.log file, and database/log.?????????? files) instead of just +creating a data file at the path and storing log files in the parent +directory. This should make backing up wallets more straightforward than +before because the specified wallet path can just be directly archived without +having to look in the parent directory for transaction log files. + +For backwards compatibility, wallet paths that are names of existing data files +in the `-walletdir` directory will continue to be accepted and interpreted the +same as before. + +Low-level RPC changes +--------------------- + +- When bitcoin is not started with any `-wallet=` options, the name of + the default wallet returned by `getwalletinfo` and `listwallets` RPCs is + now the empty string `""` instead of `"wallet.dat"`. If bitcoin is started + with any `-wallet=` options, there is no change in behavior, and the + name of any wallet is just its `` string. Credits ======= diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index fedfcc4f10..41f1e5786c 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -339,8 +339,8 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co // check if we should use a special wallet endpoint std::string endpoint = "/"; - std::string walletName = gArgs.GetArg("-rpcwallet", ""); - if (!walletName.empty()) { + if (!gArgs.GetArgs("-rpcwallet").empty()) { + std::string walletName = gArgs.GetArg("-rpcwallet", ""); char *encodedURI = evhttp_uriencode(walletName.c_str(), walletName.size(), false); if (encodedURI) { endpoint = "/wallet/"+ std::string(encodedURI); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 0ef3b7e926..ebe7b48da0 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -59,8 +59,19 @@ std::map g_dbenvs; //!< Map from directory name to open db CDBEnv* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename) { - fs::path env_directory = wallet_path.parent_path(); - database_filename = wallet_path.filename().string(); + fs::path env_directory; + if (fs::is_regular_file(wallet_path)) { + // Special case for backwards compatibility: if wallet path points to an + // existing file, treat it as the path to a BDB data file in a parent + // directory that also contains BDB log files. + env_directory = wallet_path.parent_path(); + database_filename = wallet_path.filename().string(); + } else { + // Normal case: Interpret wallet path as a directory path containing + // data and log files. + env_directory = wallet_path; + database_filename = "wallet.dat"; + } LOCK(cs_db); // Note: An ununsed temporary CDBEnv object may be created inside the // emplace function if the key already exists. This is a little inefficient, diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 11fd067b4b..e028cf4210 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -35,7 +35,7 @@ std::string GetWalletHelpString(bool showDebug) strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE)); strUsage += HelpMessageOpt("-txconfirmtarget=", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), DEFAULT_TX_CONFIRM_TARGET)); strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup")); - strUsage += HelpMessageOpt("-wallet=", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to if it is not absolute, and will be created if it does not exist.") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); + strUsage += HelpMessageOpt("-wallet=", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to if it is not absolute, and will be created if it does not exist (as a directory containing a wallet.dat file and log files). For backwards compatibility this will also accept names of existing data files in .)")); strUsage += HelpMessageOpt("-walletbroadcast", _("Make the wallet broadcast transactions") + " " + strprintf(_("(default: %u)"), DEFAULT_WALLETBROADCAST)); strUsage += HelpMessageOpt("-walletdir=", _("Specify directory to hold wallets (default: /wallets if it exists, otherwise )")); strUsage += HelpMessageOpt("-walletnotify=", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)")); @@ -66,7 +66,7 @@ bool WalletParameterInteraction() return true; } - gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); + gArgs.SoftSetArg("-wallet", ""); const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) { @@ -230,10 +230,22 @@ bool VerifyWallets() std::set wallet_paths; for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { + // Do some checking on wallet path. It should be either a: + // + // 1. Path where a directory can be created. + // 2. Path to an existing directory. + // 3. Path to a symlink to a directory. + // 4. For backwards compatibility, the name of a data file in -walletdir. 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)); + 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(walletFile).filename() == walletFile))) { + return InitError(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)"), + walletFile, GetWalletDir())); } if (!wallet_paths.insert(wallet_path).second) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b1e2181ec7..960c192640 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -45,7 +45,6 @@ OutputType g_address_type = OUTPUT_TYPE_NONE; OutputType g_change_type = OUTPUT_TYPE_NONE; bool g_wallet_allow_fallback_fee = true; //