Skip to content

Commit

Permalink
Merge #20199: wallet: ignore (but warn) on duplicate -wallet parameters
Browse files Browse the repository at this point in the history
58cfbc3 Ignoring (but warn) on duplicate -wallet parameters (Jonas Schnelli)

Pull request description:

  I expect that there are many users with load on startup wallet definitions in `bitcoin.conf` or via startup CLI argument.
  With the new `settings.json` r/w configuration file, users unloading and loading a wallet through the GUI or via the RPC calls might end up with a duplicate `-wallet` entry (one that still remains in bitcoin.conf or CLI) plus the new duplication in `settings.json` due to the unload/load.

  Steps to reproduce
  * create wallet (if via RPC set `load_on_startup` or unloadwallet/loadwallet then set `load_on_startup`).
  * stop bitcoin
  * start bitcoind again with same `--wallet=mywallet`

  I guess it is acceptable to skip duplicates.

ACKs for top commit:
  achow101:
    Tested ACK 58cfbc3
  meshcollider:
    Code review ACK 58cfbc3
  ryanofsky:
    Code review ACK 58cfbc3. Changes since previous review: rebased, tweaked warning message, squashed/fixed test

Tree-SHA512: f94e5a999bdd7dc291f0bc142911b0a8033929350d6f6a35b58c4a06a3c8f83147be0f0c402d4e946dedbbcc85b7e023b672c731b6d7a8984d4780017c961cfb
  • Loading branch information
MarcoFalke committed Nov 5, 2020
2 parents 6760088 + 58cfbc3 commit 83650e4
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
8 changes: 6 additions & 2 deletions src/wallet/load.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ bool VerifyWallets(interfaces::Chain& chain)
const fs::path path = fs::absolute(wallet_file, GetWalletDir());

if (!wallet_paths.insert(path).second) {
chain.initError(strprintf(_("Error loading wallet %s. Duplicate -wallet filename specified."), wallet_file));
return false;
chain.initWarning(strprintf(_("Ignoring duplicate -wallet %s."), wallet_file));
continue;
}

DatabaseOptions options;
Expand All @@ -90,7 +90,11 @@ bool VerifyWallets(interfaces::Chain& chain)
bool LoadWallets(interfaces::Chain& chain)
{
try {
std::set<fs::path> wallet_paths;
for (const std::string& name : gArgs.GetArgs("-wallet")) {
if (!wallet_paths.insert(name).second) {
continue;
}
DatabaseOptions options;
DatabaseStatus status;
options.require_existing = true;
Expand Down
4 changes: 2 additions & 2 deletions test/functional/wallet_multiwallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ def wallet_file(name):
self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir())
self.nodes[0].assert_start_raises_init_error(['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir())

# should not initialize if there are duplicate wallets
self.nodes[0].assert_start_raises_init_error(['-wallet=w1', '-wallet=w1'], 'Error: Error loading wallet w1. Duplicate -wallet filename specified.')
self.start_node(0, ['-wallet=w1', '-wallet=w1'])
self.stop_node(0, 'Warning: Ignoring duplicate -wallet w1.')

if not self.options.descriptors:
# Only BDB doesn't open duplicate wallet files. SQLite does not have this limitation. While this may be desired in the future, it is not necessary
Expand Down

0 comments on commit 83650e4

Please sign in to comment.