diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -20,10 +20,31 @@ --------------------- 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 bitcoind 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 bitcoind is started with + any `-wallet=` options, there is no change in behavior, and the name of + any wallet is just its `` string. Transaction index changes ------------------------- diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -407,8 +407,8 @@ // 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) { diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -67,8 +67,19 @@ 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 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -92,8 +92,10 @@ "-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), + "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 .)"), false, OptionsCategory::WALLET); gArgs.AddArg("-walletbroadcast", _("Make the wallet broadcast transactions") + " " + @@ -140,7 +142,7 @@ bool WalletInit::ParameterInteraction() const { CFeeRate minRelayTxFee = GetConfig().GetMinFeePerKB(); - gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); + gArgs.SoftSetArg("-wallet", ""); const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { @@ -326,13 +328,28 @@ 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.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -61,8 +61,6 @@ //! if set, all keys will be derived by using BIP32 static const bool DEFAULT_USE_HD_WALLET = true; -extern const char *DEFAULT_WALLET_DAT; - static const int64_t TIMESTAMP_MIN = 0; class CBlockIndex; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -48,7 +48,6 @@ OutputType g_address_type = OutputType::NONE; OutputType g_change_type = OutputType::NONE; -const char *DEFAULT_WALLET_DAT = "wallet.dat"; const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; /** diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -45,7 +45,7 @@ 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( + assert os.path.exists(os.path.join( new_data_dir, 'regtest', 'wallets', 'w1')) # Ensure command line argument overrides datadir in conf @@ -53,7 +53,7 @@ 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( + assert os.path.exists(os.path.join( new_data_dir_2, 'regtest', 'wallets', 'w2')) diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -34,14 +34,25 @@ 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 + # 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'), 'wallet.dat'] + 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)) @@ -49,11 +60,15 @@ # 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') + 0, ['-wallet=wallet.dat/bad'], 'Not a directory') self.assert_start_raises_init_error( 0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') @@ -66,20 +81,15 @@ 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')) + shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) self.assert_start_raises_init_error( - 0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') + 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')) + os.symlink('w8', wallet_dir('w8_symlink')) self.assert_start_raises_init_error( - 0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') + 0, ['-wallet=w8_symlink'], 'Invalid -wallet path') # should not initialize if the specified walletdir does not exist self.assert_start_raises_init_error(