wallet: Remove path checking code from bitcoin-wallet tool

This commit does not change behavior except for error messages which now
include more complete information.

Author: ryanofsky

src/wallet/salvage.cpp

@@ -23,6 +23,13 @@ static bool KeyFilter(const std::string& type)
 
 bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings)
 {
+    DatabaseOptions options;
+    DatabaseStatus status;
+    options.require_existing = true;
+    options.verify = false;
+    std::unique_ptr<WalletDatabase> database = MakeDatabase(file_path, options, status, error);
+    if (!database) return false;
+
     std::string filename;
     std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, filename);
 

src/wallet/walletdb.cpp

@@ -1032,13 +1032,6 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas
     return MakeBerkeleyDatabase(path, options, status, error);
 }
 
-/** Return object for accessing database at specified path. */
-std::unique_ptr<WalletDatabase> CreateWalletDatabase(const fs::path& path)
-{
-    std::string filename;
-    return MakeUnique<BerkeleyDatabase>(GetWalletEnv(path, filename), std::move(filename));
-}
-
 /** Return object for accessing dummy database with no read/write capabilities. */
 std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase()
 {

src/wallet/walletdb.h

@@ -285,9 +285,6 @@ using KeyFilterFn = std::function<bool(const std::string&)>;
 //! Unserialize a given Key-Value pair and load it into the wallet
 bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr);
 
-/** Return object for accessing database at specified path. */
-std::unique_ptr<WalletDatabase> CreateWalletDatabase(const fs::path& path);
-
 /** Return object for accessing dummy database with no read/write capabilities. */
 std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase();
 

src/wallet/wallettool.cpp

@@ -21,21 +21,9 @@ static void WalletToolReleaseWallet(CWallet* wallet)
     delete wallet;
 }
 
-static std::shared_ptr<CWallet> CreateWallet(const std::string& name, const fs::path& path)
+static void WalletCreate(CWallet* wallet_instance)
 {
-    if (fs::exists(path)) {
-        tfm::format(std::cerr, "Error: File exists already\n");
-        return nullptr;
-    }
-    // dummy chain interface
-    std::shared_ptr<CWallet> wallet_instance(new CWallet(nullptr /* chain */, name, CreateWalletDatabase(path)), WalletToolReleaseWallet);
     LOCK(wallet_instance->cs_wallet);
-    bool first_run = true;
-    DBErrors load_wallet_ret = wallet_instance->LoadWallet(first_run);
-    if (load_wallet_ret != DBErrors::LOAD_OK) {
-        tfm::format(std::cerr, "Error creating %s", name);
-        return nullptr;
-    }
 
     wallet_instance->SetMinVersion(FEATURE_HD_SPLIT);
 
@@ -46,18 +34,26 @@ static std::shared_ptr<CWallet> CreateWallet(const std::string& name, const fs::
 
     tfm::format(std::cout, "Topping up keypool...\n");
     wallet_instance->TopUpKeyPool();
-    return wallet_instance;
 }
 
-static std::shared_ptr<CWallet> LoadWallet(const std::string& name, const fs::path& path)
+static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::path& path, bool create)
 {
-    if (!fs::exists(path)) {
-        tfm::format(std::cerr, "Error: Wallet files does not exist\n");
+    DatabaseOptions options;
+    DatabaseStatus status;
+    if (create) {
+        options.require_create = true;
+    } else {
+        options.require_existing = true;
+    }
+    bilingual_str error;
+    std::unique_ptr<WalletDatabase> database = MakeDatabase(path, options, status, error);
+    if (!database) {
+        tfm::format(std::cerr, "%s\n", error.original);
         return nullptr;
     }
 
     // dummy chain interface
-    std::shared_ptr<CWallet> wallet_instance(new CWallet(nullptr /* chain */, name, CreateWalletDatabase(path)), WalletToolReleaseWallet);
+    std::shared_ptr<CWallet> wallet_instance{new CWallet(nullptr /* chain */, name, std::move(database)), WalletToolReleaseWallet};
     DBErrors load_wallet_ret;
     try {
         bool first_run;
@@ -89,6 +85,8 @@ static std::shared_ptr<CWallet> LoadWallet(const std::string& name, const fs::pa
         }
     }
 
+    if (create) WalletCreate(wallet_instance.get());
+
     return wallet_instance;
 }
 
@@ -109,19 +107,14 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)
     fs::path path = fs::absolute(name, GetWalletDir());
 
     if (command == "create") {
-        std::shared_ptr<CWallet> wallet_instance = CreateWallet(name, path);
+        std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, /* create= */ true);
         if (wallet_instance) {
             WalletShowInfo(wallet_instance.get());
             wallet_instance->Close();
         }
     } else if (command == "info" || command == "salvage") {
-        if (!fs::exists(path)) {
-            tfm::format(std::cerr, "Error: no wallet file at %s\n", name);
-            return false;
-        }
-
         if (command == "info") {
-            std::shared_ptr<CWallet> wallet_instance = LoadWallet(name, path);
+            std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, /* create= */ false);
             if (!wallet_instance) return false;
             WalletShowInfo(wallet_instance.get());
             wallet_instance->Close();

src/wallet/wallettool.h

@@ -9,8 +9,6 @@
 
 namespace WalletTool {
 
-std::shared_ptr<CWallet> CreateWallet(const std::string& name, const fs::path& path);
-std::shared_ptr<CWallet> LoadWallet(const std::string& name, const fs::path& path);
 void WalletShowInfo(CWallet* wallet_instance);
 bool ExecuteWalletToolFunc(const std::string& command, const std::string& file);
 

test/functional/tool_wallet.py

@@ -70,12 +70,14 @@ def test_invalid_tool_commands_and_args(self):
         self.assert_raises_tool_error('Invalid command: help', 'help')
         self.assert_raises_tool_error('Error: two methods provided (info and create). Only one method should be provided.', 'info', 'create')
         self.assert_raises_tool_error('Error parsing command line arguments: Invalid parameter -foo', '-foo')
+        locked_dir = os.path.join(self.options.tmpdir, "node0", "regtest", "wallets")
         self.assert_raises_tool_error(
-            'Error loading wallet.dat. Is wallet being used by another process?',
+            'Error initializing wallet database environment "{}"!'.format(locked_dir),
             '-wallet=wallet.dat',
             'info',
         )
-        self.assert_raises_tool_error('Error: no wallet file at nonexistent.dat', '-wallet=nonexistent.dat', 'info')
+        path = os.path.join(self.options.tmpdir, "node0", "regtest", "wallets", "nonexistent.dat")
+        self.assert_raises_tool_error("Failed to load database path '{}'. Path does not exist.".format(path), '-wallet=nonexistent.dat', 'info')
 
     def test_tool_wallet_info(self):
         # Stop the node to close the wallet to call the info command.