From 8263f6a5ac3f3af102a2819b7e179b00db7e0437 Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Thu, 12 Oct 2017 22:04:46 +1300 Subject: [PATCH] Create walletdir if datadir doesn't exist and fix tests --- src/bitcoind.cpp | 4 --- src/init.cpp | 1 - src/qt/bitcoin.cpp | 5 ---- src/qt/intro.cpp | 5 +++- src/util.cpp | 5 +++- src/wallet/init.cpp | 6 +++++ test/functional/keypool-topup.py | 4 +-- test/functional/multiwallet.py | 25 +++++++++++++------ .../test_framework/test_framework.py | 2 +- test/functional/wallet-hd.py | 2 +- test/functional/walletbackup.py | 20 +++++++-------- 11 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index b1f1342aa..bc23912d2 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -101,10 +101,6 @@ bool AppInit(int argc, char* argv[]) fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str()); return false; } - if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { - fprintf(stderr, "Error: Specified wallet directory \"%s\" does not exist.\n", gArgs.GetArg("-walletdir", "").c_str()); - return false; - } try { gArgs.ReadConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)); diff --git a/src/init.cpp b/src/init.cpp index 93174a11e..439eaacfc 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1220,7 +1220,6 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) LogPrintf("Startup time: %s\n", DateTimeStrFormat("%Y-%m-%d %H:%M:%S", GetTime())); LogPrintf("Default data directory %s\n", GetDefaultDataDir().string()); LogPrintf("Using data directory %s\n", GetDataDir().string()); - LogPrintf("Using wallet directory %s\n", GetWalletDir().string()); LogPrintf("Using config file %s\n", GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)).string()); LogPrintf("Using at most %i automatic connections (%i file descriptors available)\n", nMaxConnections, nFD); diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 05af10a52..557d7efce 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -626,11 +626,6 @@ int main(int argc, char *argv[]) QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); return EXIT_FAILURE; } - if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { - QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), - QObject::tr("Error: Specified wallet directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-walletdir", "")))); - return EXIT_FAILURE; - } try { gArgs.ReadConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)); } catch (const std::exception& e) { diff --git a/src/qt/intro.cpp b/src/qt/intro.cpp index 9e4c76510..7f8a8394e 100644 --- a/src/qt/intro.cpp +++ b/src/qt/intro.cpp @@ -214,7 +214,10 @@ bool Intro::pickDataDirectory() } dataDir = intro.getDataDirectory(); try { - TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir)); + if (TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir))) { + // If a new data directory has been created, make wallets subdirectory too + TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir) / "wallets"); + } break; } catch (const fs::filesystem_error&) { QMessageBox::critical(0, tr(PACKAGE_NAME), diff --git a/src/util.cpp b/src/util.cpp index b87dd091b..d58f39e96 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -574,7 +574,10 @@ const fs::path &GetDataDir(bool fNetSpecific) if (fNetSpecific) path /= BaseParams().DataDir(); - fs::create_directories(path); + if (fs::create_directories(path)) { + // This is the first run, create wallets subdirectory too + fs::create_directories(path / "wallets"); + } return path; } diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 754ed8a67..67c46df87 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -193,6 +193,12 @@ bool VerifyWallets() return true; } + if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { + return InitError(strprintf(_("Error: Specified wallet directory \"%s\" does not exist."), gArgs.GetArg("-walletdir", "").c_str())); + } + + LogPrintf("Using wallet directory %s\n", GetWalletDir().string()); + uiInterface.InitMessage(_("Verifying wallet(s)...")); // Keep track of each wallet absolute path to detect duplicates. diff --git a/test/functional/keypool-topup.py b/test/functional/keypool-topup.py index 160a0f7ae..e7af3c398 100755 --- a/test/functional/keypool-topup.py +++ b/test/functional/keypool-topup.py @@ -33,7 +33,7 @@ class KeypoolRestoreTest(BitcoinTestFramework): self.stop_node(1) - shutil.copyfile(self.tmpdir + "/node1/regtest/wallet.dat", self.tmpdir + "/wallet.bak") + shutil.copyfile(self.tmpdir + "/node1/regtest/wallets/wallet.dat", self.tmpdir + "/wallet.bak") self.start_node(1, self.extra_args[1]) connect_nodes_bi(self.nodes, 0, 1) @@ -56,7 +56,7 @@ class KeypoolRestoreTest(BitcoinTestFramework): self.stop_node(1) - shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallet.dat") + shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallets/wallet.dat") self.log.info("Verify keypool is restored and balance is correct") diff --git a/test/functional/multiwallet.py b/test/functional/multiwallet.py index 2f30d9f1a..4a721d110 100755 --- a/test/functional/multiwallet.py +++ b/test/functional/multiwallet.py @@ -27,28 +27,37 @@ class MultiWalletTest(BitcoinTestFramework): 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(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11')) + wallet_dir = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'wallets') + os.mkdir(os.path.join(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(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w2'), - os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w22')) + shutil.copyfile(os.path.join(wallet_dir, 'w2'), os.path.join(wallet_dir, 'w22')) self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') # should not initialize if wallet file is a symlink - os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12')) + os.symlink(os.path.join(wallet_dir, 'w1'), os.path.join(wallet_dir, 'w12')) self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') # should not initialize if the specified walletdir does not exist self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified wallet directory "bad" does not exist.') - # running the node with specified walletdir should only have the default wallet in it - os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'walletdir')) - self.start_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + os.path.join(self.options.tmpdir, 'node0', 'regtest', 'walletdir')]) + # if wallets/ doesn't exist, datadir should be the default wallet dir + wallet_dir2 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'walletdir') + os.rename(wallet_dir, wallet_dir2) + self.start_node(0, ['-wallet=w4', '-wallet=w5']) + assert_equal(set(self.nodes[0].listwallets()), {"w4", "w5"}) + w5 = self.nodes[0].get_wallet_rpc("w5") + w5.generate(1) + self.stop_node(0) + + # now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded + os.rename(wallet_dir2, wallet_dir) + self.start_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + os.path.join(self.options.tmpdir, 'node0', 'regtest')]) assert_equal(set(self.nodes[0].listwallets()), {"w4", "w5"}) w5 = self.nodes[0].get_wallet_rpc("w5") w5_info = w5.getwalletinfo() - assert_equal(w5_info['immature_balance'], 0) + assert_equal(w5_info['immature_balance'], 50) self.stop_node(0) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 8df50474f..4590b4c65 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -432,7 +432,7 @@ class BitcoinTestFramework(): self.disable_mocktime() for i in range(MAX_NODES): os.remove(log_filename(self.options.cachedir, i, "debug.log")) - os.remove(log_filename(self.options.cachedir, i, "db.log")) + os.remove(log_filename(self.options.cachedir, i, "wallets/db.log")) os.remove(log_filename(self.options.cachedir, i, "peers.dat")) os.remove(log_filename(self.options.cachedir, i, "fee_estimates.dat")) diff --git a/test/functional/wallet-hd.py b/test/functional/wallet-hd.py index 9b6ce6860..d21656a97 100755 --- a/test/functional/wallet-hd.py +++ b/test/functional/wallet-hd.py @@ -73,7 +73,7 @@ class WalletHDTest(BitcoinTestFramework): # otherwise node1 would auto-recover all funds in flag the keypool keys as used shutil.rmtree(os.path.join(tmpdir, "node1/regtest/blocks")) shutil.rmtree(os.path.join(tmpdir, "node1/regtest/chainstate")) - shutil.copyfile(os.path.join(tmpdir, "hd.bak"), os.path.join(tmpdir, "node1/regtest/wallet.dat")) + shutil.copyfile(os.path.join(tmpdir, "hd.bak"), os.path.join(tmpdir, "node1/regtest/wallets/wallet.dat")) self.start_node(1) # Assert that derivation is deterministic diff --git a/test/functional/walletbackup.py b/test/functional/walletbackup.py index 85a149793..8ef5620cd 100755 --- a/test/functional/walletbackup.py +++ b/test/functional/walletbackup.py @@ -90,9 +90,9 @@ class WalletBackupTest(BitcoinTestFramework): self.stop_node(2) def erase_three(self): - os.remove(self.options.tmpdir + "/node0/regtest/wallet.dat") - os.remove(self.options.tmpdir + "/node1/regtest/wallet.dat") - os.remove(self.options.tmpdir + "/node2/regtest/wallet.dat") + os.remove(self.options.tmpdir + "/node0/regtest/wallets/wallet.dat") + os.remove(self.options.tmpdir + "/node1/regtest/wallets/wallet.dat") + os.remove(self.options.tmpdir + "/node2/regtest/wallets/wallet.dat") def run_test(self): self.log.info("Generating initial blockchain") @@ -154,9 +154,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/wallet.bak", tmpdir + "/node0/regtest/wallets/wallet.dat") + shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallets/wallet.dat") + shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallets/wallet.dat") self.log.info("Re-starting nodes") self.start_three() @@ -192,10 +192,10 @@ class WalletBackupTest(BitcoinTestFramework): # Backup to source wallet file must fail sourcePaths = [ - tmpdir + "/node0/regtest/wallet.dat", - tmpdir + "/node0/./regtest/wallet.dat", - tmpdir + "/node0/regtest/", - tmpdir + "/node0/regtest"] + tmpdir + "/node0/regtest/wallets/wallet.dat", + tmpdir + "/node0/./regtest/wallets/wallet.dat", + tmpdir + "/node0/regtest/wallets/", + tmpdir + "/node0/regtest/wallets"] for sourcePath in sourcePaths: assert_raises_rpc_error(-4, "backup failed", self.nodes[0].backupwallet, sourcePath)