diff options
| -rw-r--r-- | doc/release-notes.md | 29 | ||||
| -rw-r--r-- | src/bitcoin-cli.cpp | 4 | ||||
| -rw-r--r-- | src/wallet/db.cpp | 15 | ||||
| -rw-r--r-- | src/wallet/init.cpp | 22 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 1 | ||||
| -rw-r--r-- | src/wallet/wallet.h | 2 | ||||
| -rwxr-xr-x | test/functional/feature_config_args.py | 4 | ||||
| -rwxr-xr-x | 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=<path>` 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 <walletdir>/database/log.?????????? -files. +to be located in the -walletdir directory. + +Newly created wallet format +--------------------------- + +If `-wallet=<path>` 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=<path>` 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=<path>` options, there is no change in behavior, and the + name of any wallet is just its `<path>` 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<std::string, CDBEnv> 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=<n>", 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=<path>", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to <walletdir> if it is not absolute, and will be created if it does not exist.") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); + strUsage += HelpMessageOpt("-wallet=<path>", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to <walletdir> 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 <walletdir>.)")); 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)")); @@ -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<fs::path> 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; //<! will be defined via chainparams -const char * DEFAULT_WALLET_DAT = "wallet.dat"; const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; /** diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 76a411d81b..3e2d1794d8 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -68,8 +68,6 @@ static const bool DEFAULT_WALLET_RBF = false; static const bool DEFAULT_WALLETBROADCAST = true; static const bool DEFAULT_DISABLE_WALLET = false; -extern const char * DEFAULT_WALLET_DAT; - static const int64_t TIMESTAMP_MIN = 0; class CBlockIndex; diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 61abba8082..c6cec0596b 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -37,13 +37,13 @@ class ConfArgsTest(BitcoinTestFramework): os.mkdir(new_data_dir) self.start_node(0, ['-conf='+conf_file, '-wallet=w1']) self.stop_node(0) - assert os.path.isfile(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) + assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) # Ensure command line argument overrides datadir in conf os.mkdir(new_data_dir_2) self.nodes[0].datadir = new_data_dir_2 self.start_node(0, ['-datadir='+new_data_dir_2, '-conf='+conf_file, '-wallet=w2']) - assert os.path.isfile(os.path.join(new_data_dir_2, 'regtest', 'wallets', 'w2')) + assert os.path.exists(os.path.join(new_data_dir_2, 'regtest', 'wallets', 'w2')) if __name__ == '__main__': ConfArgsTest().main() diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 871fc1a151..378c06ee59 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -29,13 +29,24 @@ class MultiWalletTest(BitcoinTestFramework): self.stop_nodes() assert_equal(os.path.isfile(wallet_dir('wallet.dat')), True) + # create symlink to verify wallet directory path can be referenced + # through symlink + os.mkdir(wallet_dir('w7')) + os.symlink('w7', wallet_dir('w7_symlink')) + + # rename wallet.dat to make sure plain wallet file paths (as opposed to + # directory paths) can be loaded + os.rename(wallet_dir("wallet.dat"), wallet_dir("w8")) + # restart node with a mix of wallet names: # w1, w2, w3 - to verify new wallets created when non-existing paths specified # w - to verify wallet name matching works when one wallet path is prefix of another # sub/w5 - to verify relative wallet path is created correctly # extern/w6 - to verify absolute wallet path is created correctly - # wallet.dat - to verify existing wallet file is loaded correctly - wallet_names = ['w1', 'w2', 'w3', 'w', 'sub/w5', os.path.join(self.options.tmpdir, 'extern/w6'), 'wallet.dat'] + # w7_symlink - to verify symlinked wallet path is initialized correctly + # w8 - to verify existing wallet file is loaded correctly + # '' - to verify default wallet file is created correctly + wallet_names = ['w1', 'w2', 'w3', 'w', 'sub/w5', os.path.join(self.options.tmpdir, 'extern/w6'), 'w7_symlink', 'w8', ''] extra_args = ['-wallet={}'.format(n) for n in wallet_names] self.start_node(0, extra_args) assert_equal(set(node.listwallets()), set(wallet_names)) @@ -43,10 +54,13 @@ class MultiWalletTest(BitcoinTestFramework): # check that all requested wallets were created self.stop_node(0) for wallet_name in wallet_names: - assert_equal(os.path.isfile(wallet_dir(wallet_name)), True) + if os.path.isdir(wallet_dir(wallet_name)): + assert_equal(os.path.isfile(wallet_dir(wallet_name, "wallet.dat")), True) + else: + assert_equal(os.path.isfile(wallet_dir(wallet_name)), True) # should not initialize if wallet path can't be created - self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'File exists') + self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'Not a directory') 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()) @@ -55,17 +69,13 @@ class MultiWalletTest(BitcoinTestFramework): # 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.') - # should not initialize if wallet file is a directory - os.mkdir(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(wallet_dir('w2'), wallet_dir('w22')) - self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') + shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) + self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid') # should not initialize if wallet file is a symlink - os.symlink(wallet_dir('w1'), wallet_dir('w12')) - self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') + os.symlink('w8', wallet_dir('w8_symlink')) + self.assert_start_raises_init_error(0, ['-wallet=w8_symlink'], 'Invalid -wallet path') # should not initialize if the specified walletdir does not exist self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist') |