Create walletdir if datadir doesn't exist and fix tests

meshcollider · meshcollider · commit 8263f6a5ac3f · 2017-11-18T00:50:59.000+13:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -33,7 +33,7 @@ def run_test(self):
 
         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 @@ def run_test(self):
 
         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
@@ -27,28 +27,37 @@ def run_test(self):
         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
@@ -432,7 +432,7 @@ def _initialize_chain(self):
             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
@@ -73,7 +73,7 @@ def run_test (self):
         # 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
@@ -90,9 +90,9 @@ def stop_three(self):
         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 @@ def run_test(self):
         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 @@ def run_test(self):
 
         # 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)

Original file line number	Diff line number	Diff line change
`@@ -101,10 +101,6 @@ bool AppInit(int argc, char* argv[])`
`101`	`101`	`fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str());`
`102`	`102`	`return false;`
`103`	`103`	`}`
`104`		`- if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) {`
`105`		`- fprintf(stderr, "Error: Specified wallet directory \"%s\" does not exist.\n", gArgs.GetArg("-walletdir", "").c_str());`
`106`		`- return false;`
`107`		`- }`
`108`	`104`	`try`
`109`	`105`	`{`
`110`	`106`	`gArgs.ReadConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));`