This addresses the remaining nits from #11466
doc/files.md with respect to the new default wallet directory13 | @@ -14,6 +14,10 @@ fs::path GetWalletDir() 14 | // If the path specified doesn't exist, we return the deliberately 15 | // invalid empty string. 16 | path = ""; 17 | + } else { 18 | + if (fs::is_directory(path / BaseParams().DataDir())) {
This is a tricky case but after thinking a bit, I think this should be unconditional. If the user wants to use their walletdir with testnet/regtest, they should create a subdir for it in the walletdir.
Otherwise this will still result in wallet directory collision by default if -walletdir is specified in bitcoin.conf.
We really want to avoid the case where accidentally, say, a regtest wallet is used on mainnet or vice versa.
Agreed, #10996 or something similar would be much cleaner. Removed.
193 | @@ -194,7 +194,11 @@ bool VerifyWallets() 194 | } 195 | 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 | + if(!fs::exists(GetWalletDir())) { 199 | + return InitError(strprintf(_("Specified wallet directory \"%s\" does not exist."), gArgs.GetArg("-walletdir", "").c_str()));
Remove default value "" here and below?GetArg()must be called with default value.
193 | @@ -194,7 +194,11 @@ bool VerifyWallets() 194 | } 195 | 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 | + if(!fs::exists(GetWalletDir())) {
Suggestion:
if (exists) {
return "not a directory";
}
return "does not exists";
nit: (in case you touch the pull for other reasons): Add space after if (or use clang-format).
5 | @@ -6,13 +6,13 @@ 6 | * blocks/rev000??.dat; block undo data (custom); since 0.8.0 (format changed since pre-0.8) 7 | * blocks/index/*; block index (LevelDB); since 0.8.0 8 | * chainstate/*; block chain state database (LevelDB); since 0.8.0 9 | -* database/*: BDB database environment; only used for wallet since 0.8.0 10 | -* db.log: wallet database log file
Follow blktree/ and blocks/index/* example:
* db.log: wallet database log file; replaced by wallets/ directory in 0.16.0
* wallets/db.log; since 0.16.0
Same for wallet.dat.
@promag I've made an attempt at updating the files.md as you suggested, feel free to suggest a different change though if you're not happy still :)
14 | * fee_estimates.dat: stores statistics used to estimate minimum transaction fees and priorities required for confirmation; since 0.10.0 15 | * mempool.dat: dump of the mempool's transactions; since 0.14.0. 16 | * peers.dat: peer IP address database (custom format); since 0.7.0 17 | -* wallet.dat: personal wallet (BDB) with keys and transactions 18 | +* wallet.dat: personal wallet (BDB) with keys and transactions; moved to wallets/ directory on new installs since 0.16.0 19 | +* wallets/database/*: BDB database environment; only used for wallet since 0.8.0; since 0.16.0
since 0.8.0; since 0.16.0...wait...huh
193 | @@ -194,7 +194,10 @@ bool VerifyWallets() 194 | } 195 | 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 | + if (fs::exists(GetWalletDir())) { 199 | + return InitError(strprintf(_("Specified wallet directory \"%s\" is not a directory."), gArgs.GetArg("-walletdir", "").c_str()));
IMO this is better Specified -walletdir \"%s\" is not a directory (no period and references parameter name).
Done
193 | @@ -194,7 +194,10 @@ bool VerifyWallets() 194 | } 195 | 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 | + if (fs::exists(GetWalletDir())) { 199 | + return InitError(strprintf(_("Specified wallet directory \"%s\" is not a directory."), gArgs.GetArg("-walletdir", "").c_str())); 200 | + } 201 | + return InitError(strprintf(_("Specified wallet directory \"%s\" does not exist."), gArgs.GetArg("-walletdir", "").c_str()));
Same as above.
Done
Updated as per @promag comments, tests are already present in multiwallet.py but I've updated them with the new error messages
utACK aac6b3f06717626b88cdfd140f2cc1a3f2dde4be
utACK aac6b3f06717626b88cdfd140f2cc1a3f2dde4be
Looks sufficiently reviewed. Going to merge.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
utACK aac6b3f06717626b88cdfd140f2cc1a3f2dde4be
-----BEGIN PGP SIGNATURE-----
iQIcBAEBCgAGBQJaOuXFAAoJENLqSFDnUosl4BkP/1DQF5ffK+3slLPF4IUvyYcU
dv4lh8ME76sjlsy/D+D2Do41o4oJ9gsBdyl9Wv9aPdxEDXOdaYm7H8Kej3ANHEJ0
yBDYwpAALqccfHo75DQqvHiZ+rQ5AJaSby1i4Pc373BhJK+zGVWov0yN7iMW5OJw
tekBwNFH11r7B8B5bE/s3YYbXO4oRHp3gRtufe0OSzHVWh+qvdo/G037LRQKEGsZ
4tmOagZNI7MO8bfMa0+Ctva+cEAD8QVgIlOoXDbG8rq6p9wGQMtZl/sBUocClV0i
Z04brGvHzhtuykHxKQH6XprDZvvax5lArtSyIYkQyIk5ad1eSU7/sFo0AiAqnUSr
8PMwTQTo878f7a+/3uhxaCzLkK/c47FaiLpb4fivY7EhWBV3gR4VTRHiLIbWS1fn
jmt728qj/dLVUYvwUKtjDTkZeTkqc92PnkMmPIKrVJZuzJIu1e44iRGLAERumsAu
z+eOcZOjBbwERi0aUzcQwe1ZDH+iWNPVXmHq/vCaRiNrgXlmKsN5v9mepVgGP0+f
3WsvP45o2JtFr942wuzWrRtsg/Bb4wVMFrBWK9iviHYBx+dkMfhWbIGh9BDejPVJ
dw9z5m4NQ01yRF8f0LUTk84sqOsctSLqp2arkk5xRO903DowbUyNUQLexXfII7ZM
ojKeCLR3vObZg/GNXaXg
=jl+C
-----END PGP SIGNATURE-----