From 26c06f24e5dcc32a7599abb8d670d22993c82bc2 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 14 Nov 2017 13:32:41 -0500 Subject: [PATCH] Allow wallet files not in -walletdir directory Remove restriction that -wallet filenames can only refer to files in the -walletdir directory. --- doc/release-notes.md | 9 +++++ src/bitcoin-cli.cpp | 2 +- src/wallet/db.cpp | 1 + src/wallet/init.cpp | 10 +---- test/functional/wallet_multiwallet.py | 57 ++++++++++++++------------- 5 files changed, 42 insertions(+), 37 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index ac49dc790..171880a77 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -63,6 +63,15 @@ RPC changes - The `fundrawtransaction` rpc will reject the previously deprecated `reserveChangeKey` option. +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. + Credits ======= diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index a60d3b3b6..fedfcc4f1 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -46,7 +46,7 @@ std::string HelpMessageCli() strUsage += HelpMessageOpt("-rpcport=", strprintf(_("Connect to JSON-RPC on (default: %u or testnet: %u)"), defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort())); strUsage += HelpMessageOpt("-rpcuser=", _("Username for JSON-RPC connections")); strUsage += HelpMessageOpt("-rpcwait", _("Wait for RPC server to start")); - strUsage += HelpMessageOpt("-rpcwallet=", _("Send RPC for non-default wallet on RPC server (argument is wallet filename in bitcoind directory, required if bitcoind/-Qt runs with multiple wallets)")); + strUsage += HelpMessageOpt("-rpcwallet=", _("Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind)")); strUsage += HelpMessageOpt("-stdin", _("Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.")); strUsage += HelpMessageOpt("-stdinrpcpass", strprintf(_("Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password."))); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 88463d10b..0ef3b7e92 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -122,6 +122,7 @@ bool CDBEnv::Open(bool retry) boost::this_thread::interruption_point(); fs::path pathIn = strPath; + TryCreateDirectories(pathIn); if (!LockDirectory(pathIn, ".walletlock")) { LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it.\n", strPath); return false; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 6e243c0a0..11fd067b4 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 file (within data directory)") + " " + 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.") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); 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)")); @@ -230,14 +230,6 @@ bool VerifyWallets() std::set wallet_paths; for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - if (boost::filesystem::path(walletFile).filename() != walletFile) { - return InitError(strprintf(_("Error loading wallet %s. -wallet parameter must only specify a filename (not a path)."), walletFile)); - } - - if (SanitizeString(walletFile, SAFE_CHARS_FILENAME) != walletFile) { - return InitError(strprintf(_("Error loading wallet %s. Invalid characters in -wallet filename."), walletFile)); - } - fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); if (fs::exists(wallet_path) && (!fs::is_regular_file(wallet_path) || fs::is_symlink(wallet_path))) { diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index b07e45166..871fc1a15 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -16,7 +16,6 @@ class MultiWalletTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 - self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w'], []] self.supports_cli = True def run_test(self): @@ -26,9 +25,28 @@ class MultiWalletTest(BitcoinTestFramework): wallet_dir = lambda *p: data_dir('wallets', *p) wallet = lambda name: node.get_wallet_rpc(name) - assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"}) - + # check wallet.dat is created self.stop_nodes() + assert_equal(os.path.isfile(wallet_dir('wallet.dat')), True) + + # 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'] + extra_args = ['-wallet={}'.format(n) for n in wallet_names] + self.start_node(0, extra_args) + assert_equal(set(node.listwallets()), set(wallet_names)) + + # 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) + + # 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, ['-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()) @@ -77,15 +95,17 @@ class MultiWalletTest(BitcoinTestFramework): self.restart_node(0, ['-walletdir='+competing_wallet_dir]) self.assert_start_raises_init_error(1, ['-walletdir='+competing_wallet_dir], 'Error initializing wallet database environment') - self.restart_node(0, self.extra_args[0]) + self.restart_node(0, extra_args) - w1 = wallet("w1") - w2 = wallet("w2") - w3 = wallet("w3") - w4 = wallet("w") + wallets = [wallet(w) for w in wallet_names] wallet_bad = wallet("bad") - w1.generate(1) + # check wallet names and balances + wallets[0].generate(1) + for wallet_name, wallet in zip(wallet_names, wallets): + info = wallet.getwalletinfo() + assert_equal(info['immature_balance'], 50 if wallet is wallets[0] else 0) + assert_equal(info['walletname'], wallet_name) # accessing invalid wallet fails assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", wallet_bad.getwalletinfo) @@ -93,24 +113,7 @@ class MultiWalletTest(BitcoinTestFramework): # accessing wallet RPC without using wallet endpoint fails assert_raises_rpc_error(-19, "Wallet file not specified", node.getwalletinfo) - # check w1 wallet balance - w1_info = w1.getwalletinfo() - assert_equal(w1_info['immature_balance'], 50) - w1_name = w1_info['walletname'] - assert_equal(w1_name, "w1") - - # check w2 wallet balance - w2_info = w2.getwalletinfo() - assert_equal(w2_info['immature_balance'], 0) - w2_name = w2_info['walletname'] - assert_equal(w2_name, "w2") - - w3_name = w3.getwalletinfo()['walletname'] - assert_equal(w3_name, "w3") - - w4_name = w4.getwalletinfo()['walletname'] - assert_equal(w4_name, "w") - + w1, w2, w3, w4, *_ = wallets w1.generate(101) assert_equal(w1.getbalance(), 100) assert_equal(w2.getbalance(), 0)