diff --git a/qa/rpc-tests/wallet-dump.py b/qa/rpc-tests/wallet-dump.py index 7fb261358..2184481c7 100755 --- a/qa/rpc-tests/wallet-dump.py +++ b/qa/rpc-tests/wallet-dump.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import (start_nodes, start_node, assert_equal, bitcoind_processes) +from test_framework.util import (start_nodes, start_node, assert_equal, bitcoind_processes, JSONRPCException) def read_dump(file_name, addrs, hd_master_addr_old): @@ -69,6 +69,7 @@ class WalletDumpTest(BitcoinTestFramework): def run_test (self): tmpdir = self.options.tmpdir + backupsdir = tmpdir + "/node0/regtest/backups/" # generate 20 addresses to compare against the dump test_addr_count = 20 @@ -81,10 +82,12 @@ class WalletDumpTest(BitcoinTestFramework): self.nodes[0].keypoolrefill() # dump unencrypted wallet - self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.unencrypted.dump") + # note that the RPC extracts only the filename + # thus writing to the backups/ default directory + self.nodes[0].dumpwallet("/node0/wallet.unencrypted.dump") found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_unenc = \ - read_dump(tmpdir + "/node0/wallet.unencrypted.dump", addrs, None) + read_dump(backupsdir + "wallet.unencrypted.dump", addrs, None) assert_equal(found_addr, test_addr_count) # all keys must be in the dump assert_equal(found_addr_chg, 30) # 30 blocks where mined assert_equal(found_addr_rsv, 90 + 1) # keypool size (TODO: fix off-by-one) @@ -99,10 +102,16 @@ class WalletDumpTest(BitcoinTestFramework): self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.encrypted.dump") found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_enc = \ - read_dump(tmpdir + "/node0/wallet.encrypted.dump", addrs, hd_master_addr_unenc) + read_dump(backupsdir + "wallet.encrypted.dump", addrs, hd_master_addr_unenc) assert_equal(found_addr, test_addr_count) assert_equal(found_addr_chg, 90 + 1 + 30) # old reserve keys are marked as change now assert_equal(found_addr_rsv, 90 + 1) # keypool size (TODO: fix off-by-one) + try: + self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.encrypted.dump") + raise AssertionError("dumpwallet should throw exception instead of overwriting existing file") + except JSONRPCException as e: + assert("Wallet dump file already exists; not overwriting" in e.error["message"]) + if __name__ == '__main__': WalletDumpTest().main () diff --git a/qa/rpc-tests/wallet-hd.py b/qa/rpc-tests/wallet-hd.py index 0c96a5ad0..bcd773d39 100755 --- a/qa/rpc-tests/wallet-hd.py +++ b/qa/rpc-tests/wallet-hd.py @@ -23,6 +23,7 @@ class WalletHDTest(BitcoinTestFramework): self.node_args = [['-usehd=0'], ['-usehd=1', '-keypool=0']] def setup_network(self): + self.node_args[1].append('-backupdir=' + self.options.tmpdir) self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, self.node_args) self.is_network_split = False connect_nodes_bi(self.nodes, 0, 1) @@ -39,8 +40,8 @@ class WalletHDTest(BitcoinTestFramework): self.nodes[1].importprivkey(self.nodes[0].dumpprivkey(non_hd_add)) # This should be enough to keep the master key and the non-HD key - self.nodes[1].backupwallet(tmpdir + "/hd.bak") - #self.nodes[1].dumpwallet(tmpdir + "/hd.dump") + self.nodes[1].backupwallet("hd.bak") + #self.nodes[1].dumpwallet("hd.dump") # Derive some HD addresses and remember the last # Also send funds to each add @@ -62,9 +63,9 @@ class WalletHDTest(BitcoinTestFramework): print("Restore backup ...") self.stop_node(1) - os.remove(self.options.tmpdir + "/node1/regtest/wallet.dat") + os.remove(tmpdir + "/node1/regtest/wallet.dat") shutil.copyfile(tmpdir + "/hd.bak", tmpdir + "/node1/regtest/wallet.dat") - self.nodes[1] = start_node(1, self.options.tmpdir, self.node_args[1]) + self.nodes[1] = start_node(1, tmpdir, self.node_args[1]) #connect_nodes_bi(self.nodes, 0, 1) # Assert that derivation is deterministic @@ -78,7 +79,7 @@ class WalletHDTest(BitcoinTestFramework): # Needs rescan self.stop_node(1) - self.nodes[1] = start_node(1, self.options.tmpdir, self.node_args[1] + ['-rescan']) + self.nodes[1] = start_node(1, tmpdir, self.node_args[1] + ['-rescan']) #connect_nodes_bi(self.nodes, 0, 1) assert_equal(self.nodes[1].getbalance(), num_hd_adds + 1) diff --git a/qa/rpc-tests/walletbackup.py b/qa/rpc-tests/walletbackup.py index 7997e68cc..4f132c744 100755 --- a/qa/rpc-tests/walletbackup.py +++ b/qa/rpc-tests/walletbackup.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 # Copyright (c) 2014-2016 The Bitcoin Core developers +# Copyright (c) 2022 The Dogecoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -124,11 +125,11 @@ class WalletBackupTest(BitcoinTestFramework): logging.info("Backing up") tmpdir = self.options.tmpdir - self.nodes[0].backupwallet(tmpdir + "/node0/wallet.bak") + self.nodes[0].backupwallet(tmpdir + "/node0/wallet0.bak") self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.dump") - self.nodes[1].backupwallet(tmpdir + "/node1/wallet.bak") + self.nodes[1].backupwallet(tmpdir + "/node1/wallet1.bak") self.nodes[1].dumpwallet(tmpdir + "/node1/wallet.dump") - self.nodes[2].backupwallet(tmpdir + "/node2/wallet.bak") + self.nodes[2].backupwallet(tmpdir + "/node2/wallet2.bak") self.nodes[2].dumpwallet(tmpdir + "/node2/wallet.dump") logging.info("More transactions") @@ -161,9 +162,9 @@ class WalletBackupTest(BitcoinTestFramework): shutil.rmtree(self.options.tmpdir + "/node2/regtest/chainstate") # Restore wallets from backup - shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallet.dat") - shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallet.dat") - shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallet.dat") + shutil.copyfile(tmpdir + "/node0/regtest/backups/wallet0.bak", tmpdir + "/node0/regtest/wallet.dat") + shutil.copyfile(tmpdir + "/node1/regtest/backups/wallet1.bak", tmpdir + "/node1/regtest/wallet.dat") + shutil.copyfile(tmpdir + "/node2/regtest/backups/wallet2.bak", tmpdir + "/node2/regtest/wallet.dat") logging.info("Re-starting nodes") self.start_three() @@ -187,9 +188,9 @@ class WalletBackupTest(BitcoinTestFramework): assert_equal(self.nodes[1].getbalance(), 0) assert_equal(self.nodes[2].getbalance(), 0) - self.nodes[0].importwallet(tmpdir + "/node0/wallet.dump") - self.nodes[1].importwallet(tmpdir + "/node1/wallet.dump") - self.nodes[2].importwallet(tmpdir + "/node2/wallet.dump") + self.nodes[0].importwallet(tmpdir + "/node0/regtest/backups/wallet.dump") + self.nodes[1].importwallet(tmpdir + "/node1/regtest/backups/wallet.dump") + self.nodes[2].importwallet(tmpdir + "/node2/regtest/backups/wallet.dump") sync_blocks(self.nodes) @@ -197,6 +198,17 @@ class WalletBackupTest(BitcoinTestFramework): assert_equal(self.nodes[1].getbalance(), balance1) assert_equal(self.nodes[2].getbalance(), balance2) + # start a node that writes backups with a -backupdir + initialize_datadir(tmpdir, 4) + backupdir = tmpdir + "/onebackup" + os.makedirs(backupdir) + backupdirnode = start_node(4, tmpdir, extra_args=["-backupdir=" + backupdir]) + backupdirnode.dumpwallet('backupwallet.dump') + backupdirnode.importwallet(tmpdir + "/onebackup/backupwallet.dump") + stop_node(backupdirnode, 4) + assert(os.path.exists(tmpdir + "/onebackup/backupwallet.dump")) + + if __name__ == '__main__': WalletBackupTest().main() diff --git a/src/util.cpp b/src/util.cpp index 61935c749..a0e7b4684 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -558,22 +558,31 @@ const boost::filesystem::path &GetBackupDir() fs::path &path = backupPathCached; - if (!path.empty()) + // ensure that any cached path still exists (not an unmounted filesystem, for example) + if (!path.empty() && fs::is_directory(path)) return path; + // start with a default, and overwrite if provided a valid path + path = GetDataDir() / "backups"; + if (IsArgSet("-backupdir")) { - LogPrintf("Starting with backupdir arg %s\n", GetArg("-backupdir", "")); - path = fs::system_complete(GetArg("-backupdir", "")); + const std::string backupDir = GetArg("-backupdir", ""); + fs::path backupDirPath = fs::system_complete(backupDir); + + if (fs::create_directories(backupDirPath) || fs::is_directory(backupDirPath)) { + path = backupDirPath; + } else { + LogPrintf("Backupdir %s is not a directory, so using default path\n", backupDirPath); + } } - if (!fs::is_directory(path)) { - LogPrintf( "Backupdir %s is not a directory, apparently\n", path ); - path = GetDataDir() / "backups"; + // ensure the path exists or fall back to a path that does exist + if (!fs::is_directory(path) && !fs::create_directories(path)) { + LogPrintf("Failed to create directory %s, so using default path %s\n", path, GetDataDir()); + path = GetDataDir(); } - fs::create_directories(path); - - LogPrintf( "Set backupdir %s\n", path ); + LogPrintf("Set backupdir %s\n", path); return path; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index ee6481f9d..b961ae52b 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -21,6 +21,8 @@ #include #include +#include +#include #include @@ -553,7 +555,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) { if (!EnsureWalletIsAvailable(request.fHelp)) return NullUniValue; - + if (request.fHelp || request.params.size() != 1) throw runtime_error( "dumpwallet \"filename\"\n" @@ -570,10 +572,27 @@ UniValue dumpwallet(const JSONRPCRequest& request) EnsureWalletIsUnlocked(); ofstream file; - file.open(request.params[0].get_str().c_str()); + + string userFilename = request.params[0].get_str(); + boost::filesystem::path path; + + if (userFilename != "") { + boost::filesystem::path p(userFilename); + boost::filesystem::path filename = p.filename(); + if (!filename.empty()) { + path = GetBackupDir() / filename; + } + } + + if (boost::filesystem::exists(path)) + throw JSONRPCError(RPC_INVALID_PARAMETER, "Wallet dump file already exists; not overwriting"); + + file.open(path.string()); if (!file.is_open()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); + LogPrintf("Dumping wallet to " + path.string() + "\n"); + std::map mapKeyBirth; std::set setKeyPool; pwalletMain->GetKeyBirthTimes(mapKeyBirth); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 436192021..b08dfbac0 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1885,9 +1885,9 @@ UniValue backupwallet(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 1) throw runtime_error( "backupwallet \"destination\"\n" - "\nSafely copies current wallet file to destination, which can be a directory or a path with filename.\n" + "\nSafely copies current wallet file to destination file.\n" "\nArguments:\n" - "1. \"destination\" (string) The destination directory or file\n" + "1. \"destination\" (string, required) The destination filename\n" "\nExamples:\n" + HelpExampleCli("backupwallet", "\"backup.dat\"") + HelpExampleRpc("backupwallet", "\"backup.dat\"") @@ -1895,8 +1895,21 @@ UniValue backupwallet(const JSONRPCRequest& request) LOCK2(cs_main, pwalletMain->cs_wallet); - string strDest = request.params[0].get_str(); - if (!pwalletMain->BackupWallet(strDest)) + string userFilename = request.params[0].get_str(); + boost::filesystem::path path; + + if (userFilename != "") { + boost::filesystem::path p(userFilename); + boost::filesystem::path filename = p.filename(); + if (!filename.empty()) { + path = GetBackupDir() / filename; + } + } + + if (boost::filesystem::exists(path)) + throw JSONRPCError(RPC_INVALID_PARAMETER, "Wallet dump file already exists; not overwriting"); + + if (!pwalletMain->BackupWallet(path.string())) throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet backup failed!"); return NullUniValue; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b79c33798..ce0813920 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -476,7 +476,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain240Setup) JSONRPCRequest request; request.params.setArray(); - request.params.push_back("wallet.backup"); + request.params.push_back(TestChain240Setup::pathTemp.string() + "/regtest/backups/wallet.backup"); ::pwalletMain = &wallet; ::importwallet(request);