Merge #20275: wallet: List all wallets in non-SQLite and non-BDB builds

f3d870fc2271bf45e0269e5ae135bced1a26f620 wallet: List all wallets in non-SQLite or non-BDB builds (Russell Yanofsky)
d70dc89e78ee6355e0bc37cc36cfc04ef7a86885 refactor: Consolidate redundant wallet database path and exists functions (Russell Yanofsky)
6a7a63644cd2fc56538d323cc0d5c1d7945247fd refactor: Drop call to GetWalletEnv in wallet salvage code (Russell Yanofsky)
6ee9cbdd18a70894f89dd268c276d5eb47a34827 refactor: Replace ListWalletDir() function with ListDatabases() (Russell Yanofsky)
5aaeb6cf877055c47fa2bbd2ea5e8d3d2033933b MOVEONLY: Move IsBDBFile, IsSQLiteFile, and ListWalletDir (Russell Yanofsky)

Pull request description:

  This PR does not change behavior when bitcoin is built normally with both the SQLite and BDB libraries. It just makes non-SQLite and non-BDB builds more similar to the normal build. Specifically:

  - It makes wallet directory lists always include all wallets so wallets don't appear missing depending on the build.

  - It now triggers specific "Build does not support SQLite database format" and "Build does not support Berkeley DB database format" errors if a wallet can't be loaded instead of the more ambiguous and scary "Data is not in recognized format" error.

  Both changes are implemented in the last commit. The previous commits are just refactoring cleanups that make the last commit possible and consolidate and reduce code.

ACKs for top commit:
  achow101:
    ACK f3d870fc2271bf45e0269e5ae135bced1a26f620
  promag:
    Tested ACK f3d870fc2271bf45e0269e5ae135bced1a26f620. Tested a --without-sqlite build with sqlite wallets.

Tree-SHA512: 029ad21559dbc338b5f351d05113c51bc25bce830f4f4e18bcd82287bc528275347a60249da65b91d252632aeb70b25d057bd59c704bfcaafb9f790bc5b59762

13 files changed, 143 insertions, 166 deletions

diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 85aae0170d..6ed48593fb 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -53,16 +53,13 @@ bool WalletDatabaseFileId::operator==(const WalletDatabaseFileId& rhs) const
 }
 
 /**
- * @param[in] wallet_path Path to wallet directory. Or (for backwards compatibility only) a path to a berkeley btree data file inside a wallet directory.
- * @param[out] database_filename Filename of berkeley btree data file inside the wallet directory.
+ * @param[in] env_directory Path to environment directory
  * @return A shared pointer to the BerkeleyEnvironment object for the wallet directory, never empty because ~BerkeleyEnvironment
  * erases the weak pointer from the g_dbenvs map.
  * @post A new BerkeleyEnvironment weak pointer is inserted into g_dbenvs if the directory path key was not already in the map.
  */
-std::shared_ptr<BerkeleyEnvironment> GetWalletEnv(const fs::path& wallet_path, std::string& database_filename)
+std::shared_ptr<BerkeleyEnvironment> GetBerkeleyEnv(const fs::path& env_directory)
 {
-    fs::path env_directory;
-    SplitWalletPath(wallet_path, env_directory, database_filename);
     LOCK(cs_db);
     auto inserted = g_dbenvs.emplace(env_directory.string(), std::weak_ptr<BerkeleyEnvironment>());
     if (inserted.second) {
@@ -808,21 +805,14 @@ std::unique_ptr<DatabaseBatch> BerkeleyDatabase::MakeBatch(bool flush_on_close)
     return MakeUnique<BerkeleyBatch>(*this, false, flush_on_close);
 }
 
-bool ExistsBerkeleyDatabase(const fs::path& path)
-{
-    fs::path env_directory;
-    std::string data_filename;
-    SplitWalletPath(path, env_directory, data_filename);
-    return IsBDBFile(env_directory / data_filename);
-}
-
 std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error)
 {
+    fs::path data_file = BDBDataFile(path);
     std::unique_ptr<BerkeleyDatabase> db;
     {
         LOCK(cs_db); // Lock env.m_databases until insert in BerkeleyDatabase constructor
-        std::string data_filename;
-        std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(path, data_filename);
+        std::string data_filename = data_file.filename().string();
+        std::shared_ptr<BerkeleyEnvironment> env = GetBerkeleyEnv(data_file.parent_path());
         if (env->m_databases.count(data_filename)) {
             error = Untranslated(strprintf("Refusing to load database. Data file '%s' is already loaded.", (env->Directory() / data_filename).string()));
             status = DatabaseStatus::FAILED_ALREADY_LOADED;
@@ -839,28 +829,3 @@ std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, con
     status = DatabaseStatus::SUCCESS;
     return db;
 }
-
-bool IsBDBFile(const fs::path& path)
-{
-    if (!fs::exists(path)) return false;
-
-    // A Berkeley DB Btree file has at least 4K.
-    // This check also prevents opening lock files.
-    boost::system::error_code ec;
-    auto size = fs::file_size(path, ec);
-    if (ec) LogPrintf("%s: %s %s\n", __func__, ec.message(), path.string());
-    if (size < 4096) return false;
-
-    fsbridge::ifstream file(path, std::ios::binary);
-    if (!file.is_open()) return false;
-
-    file.seekg(12, std::ios::beg); // Magic bytes start at offset 12
-    uint32_t data = 0;
-    file.read((char*) &data, sizeof(data)); // Read 4 bytes of file to compare against magic
-
-    // Berkeley DB Btree magic bytes, from:
-    //  https://github.com/file/file/blob/5824af38469ec1ca9ac3ffd251e7afe9dc11e227/magic/Magdir/database#L74-L75
-    //  - big endian systems - 00 05 31 62
-    //  - little endian systems - 62 31 05 00
-    return data == 0x00053162 || data == 0x62310500;
-}
diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
index 4f97665f08..bf1617d67f 100644
--- a/src/wallet/bdb.h
+++ b/src/wallet/bdb.h
@@ -83,11 +83,8 @@ public:
     }
 };
 
-/** Get BerkeleyEnvironment and database filename given a wallet path. */
-std::shared_ptr<BerkeleyEnvironment> GetWalletEnv(const fs::path& wallet_path, std::string& database_filename);
-
-/** Check format of database file */
-bool IsBDBFile(const fs::path& path);
+/** Get BerkeleyEnvironment given a directory path. */
+std::shared_ptr<BerkeleyEnvironment> GetBerkeleyEnv(const fs::path& env_directory);
 
 class BerkeleyBatch;
 
@@ -226,9 +223,6 @@ public:
 
 std::string BerkeleyDatabaseVersion();
 
-//! Check if Berkeley database exists at specified path.
-bool ExistsBerkeleyDatabase(const fs::path& path);
-
 //! Return object giving access to Berkeley database at specified path.
 std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
 
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
index bd1d114730..cd49baeb78 100644
--- a/src/wallet/db.cpp
+++ b/src/wallet/db.cpp
@@ -3,23 +3,130 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <chainparams.h>
 #include <fs.h>
+#include <logging.h>
 #include <wallet/db.h>
 
 #include <string>
 
-void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename)
+std::vector<fs::path> ListDatabases(const fs::path& wallet_dir)
+{
+    const size_t offset = wallet_dir.string().size() + 1;
+    std::vector<fs::path> paths;
+    boost::system::error_code ec;
+
+    for (auto it = fs::recursive_directory_iterator(wallet_dir, ec); it != fs::recursive_directory_iterator(); it.increment(ec)) {
+        if (ec) {
+            LogPrintf("%s: %s %s\n", __func__, ec.message(), it->path().string());
+            continue;
+        }
+
+        try {
+            // Get wallet path relative to walletdir by removing walletdir from the wallet path.
+            // This can be replaced by boost::filesystem::lexically_relative once boost is bumped to 1.60.
+            const fs::path path = it->path().string().substr(offset);
+
+            if (it->status().type() == fs::directory_file &&
+                (IsBDBFile(BDBDataFile(it->path())) || IsSQLiteFile(SQLiteDataFile(it->path())))) {
+                // Found a directory which contains wallet.dat btree file, add it as a wallet.
+                paths.emplace_back(path);
+            } else if (it.level() == 0 && it->symlink_status().type() == fs::regular_file && IsBDBFile(it->path())) {
+                if (it->path().filename() == "wallet.dat") {
+                    // Found top-level wallet.dat btree file, add top level directory ""
+                    // as a wallet.
+                    paths.emplace_back();
+                } else {
+                    // Found top-level btree file not called wallet.dat. Current bitcoin
+                    // software will never create these files but will allow them to be
+                    // opened in a shared database environment for backwards compatibility.
+                    // Add it to the list of available wallets.
+                    paths.emplace_back(path);
+                }
+            }
+        } catch (const std::exception& e) {
+            LogPrintf("%s: Error scanning %s: %s\n", __func__, it->path().string(), e.what());
+            it.no_push();
+        }
+    }
+
+    return paths;
+}
+
+fs::path BDBDataFile(const fs::path& wallet_path)
 {
     if (fs::is_regular_file(wallet_path)) {
         // Special case for backwards compatibility: if wallet path points to an
         // existing file, treat it as the path to a BDB data file in a parent
         // directory that also contains BDB log files.
-        env_directory = wallet_path.parent_path();
-        database_filename = wallet_path.filename().string();
+        return wallet_path;
     } else {
         // Normal case: Interpret wallet path as a directory path containing
         // data and log files.
-        env_directory = wallet_path;
-        database_filename = "wallet.dat";
+        return wallet_path / "wallet.dat";
     }
 }
+
+fs::path SQLiteDataFile(const fs::path& path)
+{
+    return path / "wallet.dat";
+}
+
+bool IsBDBFile(const fs::path& path)
+{
+    if (!fs::exists(path)) return false;
+
+    // A Berkeley DB Btree file has at least 4K.
+    // This check also prevents opening lock files.
+    boost::system::error_code ec;
+    auto size = fs::file_size(path, ec);
+    if (ec) LogPrintf("%s: %s %s\n", __func__, ec.message(), path.string());
+    if (size < 4096) return false;
+
+    fsbridge::ifstream file(path, std::ios::binary);
+    if (!file.is_open()) return false;
+
+    file.seekg(12, std::ios::beg); // Magic bytes start at offset 12
+    uint32_t data = 0;
+    file.read((char*) &data, sizeof(data)); // Read 4 bytes of file to compare against magic
+
+    // Berkeley DB Btree magic bytes, from:
+    //  https://github.com/file/file/blob/5824af38469ec1ca9ac3ffd251e7afe9dc11e227/magic/Magdir/database#L74-L75
+    //  - big endian systems - 00 05 31 62
+    //  - little endian systems - 62 31 05 00
+    return data == 0x00053162 || data == 0x62310500;
+}
+
+bool IsSQLiteFile(const fs::path& path)
+{
+    if (!fs::exists(path)) return false;
+
+    // A SQLite Database file is at least 512 bytes.
+    boost::system::error_code ec;
+    auto size = fs::file_size(path, ec);
+    if (ec) LogPrintf("%s: %s %s\n", __func__, ec.message(), path.string());
+    if (size < 512) return false;
+
+    fsbridge::ifstream file(path, std::ios::binary);
+    if (!file.is_open()) return false;
+
+    // Magic is at beginning and is 16 bytes long
+    char magic[16];
+    file.read(magic, 16);
+
+    // Application id is at offset 68 and 4 bytes long
+    file.seekg(68, std::ios::beg);
+    char app_id[4];
+    file.read(app_id, 4);
+
+    file.close();
+
+    // Check the magic, see https://sqlite.org/fileformat2.html
+    std::string magic_str(magic, 16);
+    if (magic_str != std::string("SQLite format 3", 16)) {
+        return false;
+    }
+
+    // Check the application id matches our network magic
+    return memcmp(Params().MessageStart(), app_id, 4) == 0;
+}
diff --git a/src/wallet/db.h b/src/wallet/db.h
index 940d1cd242..2c75486a44 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -223,6 +223,14 @@ enum class DatabaseStatus {
     FAILED_ENCRYPT,
 };
 
+/** Recursively list database paths in directory. */
+std::vector<fs::path> ListDatabases(const fs::path& path);
+
 std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
 
+fs::path BDBDataFile(const fs::path& path);
+fs::path SQLiteDataFile(const fs::path& path);
+bool IsBDBFile(const fs::path& path);
+bool IsSQLiteFile(const fs::path& path);
+
 #endif // BITCOIN_WALLET_DB_H
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index e8dbc20e56..e4e8c50f4f 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -551,7 +551,7 @@ public:
     std::vector<std::string> listWalletDir() override
     {
         std::vector<std::string> paths;
-        for (auto& path : ListWalletDir()) {
+        for (auto& path : ListDatabases(GetWalletDir())) {
             paths.push_back(path.string());
         }
         return paths;
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index e580c9a8ea..94a73b67df 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -2537,7 +2537,7 @@ static RPCHelpMan listwalletdir()
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     UniValue wallets(UniValue::VARR);
-    for (const auto& path : ListWalletDir()) {
+    for (const auto& path : ListDatabases(GetWalletDir())) {
         UniValue wallet(UniValue::VOBJ);
         wallet.pushKV("name", path.string());
         wallets.push_back(wallet);
diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp
index da5ca7858f..09a9ec68cd 100644
--- a/src/wallet/salvage.cpp
+++ b/src/wallet/salvage.cpp
@@ -32,8 +32,9 @@ bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::v
     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);
+    BerkeleyDatabase& berkeley_database = static_cast<BerkeleyDatabase&>(*database);
+    std::string filename = berkeley_database.Filename();
+    std::shared_ptr<BerkeleyEnvironment> env = berkeley_database.env;
 
     if (!env->Open(error)) {
         return false;
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index d278d96476..0fb3b1d3c4 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -17,7 +17,6 @@
 #include <sqlite3.h>
 #include <stdint.h>
 
-static const char* const DATABASE_FILENAME = "wallet.dat";
 static constexpr int32_t WALLET_SCHEMA_VERSION = 0;
 
 static Mutex g_sqlite_mutex;
@@ -568,17 +567,11 @@ bool SQLiteBatch::TxnAbort()
     return res == SQLITE_OK;
 }
 
-bool ExistsSQLiteDatabase(const fs::path& path)
-{
-    const fs::path file = path / DATABASE_FILENAME;
-    return fs::symlink_status(file).type() == fs::regular_file && IsSQLiteFile(file);
-}
-
 std::unique_ptr<SQLiteDatabase> MakeSQLiteDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error)
 {
-    const fs::path file = path / DATABASE_FILENAME;
     try {
-        auto db = MakeUnique<SQLiteDatabase>(path, file);
+        fs::path data_file = SQLiteDataFile(path);
+        auto db = MakeUnique<SQLiteDatabase>(data_file.parent_path(), data_file);
         if (options.verify && !db->Verify(error)) {
             status = DatabaseStatus::FAILED_VERIFY;
             return nullptr;
@@ -596,37 +589,3 @@ std::string SQLiteDatabaseVersion()
 {
     return std::string(sqlite3_libversion());
 }
-
-bool IsSQLiteFile(const fs::path& path)
-{
-    if (!fs::exists(path)) return false;
-
-    // A SQLite Database file is at least 512 bytes.
-    boost::system::error_code ec;
-    auto size = fs::file_size(path, ec);
-    if (ec) LogPrintf("%s: %s %s\n", __func__, ec.message(), path.string());
-    if (size < 512) return false;
-
-    fsbridge::ifstream file(path, std::ios::binary);
-    if (!file.is_open()) return false;
-
-    // Magic is at beginning and is 16 bytes long
-    char magic[16];
-    file.read(magic, 16);
-
-    // Application id is at offset 68 and 4 bytes long
-    file.seekg(68, std::ios::beg);
-    char app_id[4];
-    file.read(app_id, 4);
-
-    file.close();
-
-    // Check the magic, see https://sqlite.org/fileformat2.html
-    std::string magic_str(magic, 16);
-    if (magic_str != std::string("SQLite format 3", 16)) {
-        return false;
-    }
-
-    // Check the application id matches our network magic
-    return memcmp(Params().MessageStart(), app_id, 4) == 0;
-}
diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h
index 693a2ef55a..442563184e 100644
--- a/src/wallet/sqlite.h
+++ b/src/wallet/sqlite.h
@@ -113,10 +113,8 @@ public:
     sqlite3* m_db{nullptr};
 };
 
-bool ExistsSQLiteDatabase(const fs::path& path);
 std::unique_ptr<SQLiteDatabase> MakeSQLiteDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
 
 std::string SQLiteDatabaseVersion();
-bool IsSQLiteFile(const fs::path& path);
 
 #endif // BITCOIN_WALLET_SQLITE_H
diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp
index 8f0083cd2e..1a28852a6b 100644
--- a/src/wallet/test/db_tests.cpp
+++ b/src/wallet/test/db_tests.cpp
@@ -13,6 +13,13 @@
 
 BOOST_FIXTURE_TEST_SUITE(db_tests, BasicTestingSetup)
 
+static std::shared_ptr<BerkeleyEnvironment> GetWalletEnv(const fs::path& path, std::string& database_filename)
+{
+    fs::path data_file = BDBDataFile(path);
+    database_filename = data_file.filename().string();
+    return GetBerkeleyEnv(data_file.parent_path());
+}
+
 BOOST_AUTO_TEST_CASE(getwalletenv_file)
 {
     std::string test_name = "test_name.dat";
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index c0521d3386..5b72a01939 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -1013,13 +1013,10 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas
 
     Optional<DatabaseFormat> format;
     if (exists) {
-#ifdef USE_BDB
-        if (ExistsBerkeleyDatabase(path)) {
+        if (IsBDBFile(BDBDataFile(path))) {
             format = DatabaseFormat::BERKELEY;
         }
-#endif
-#ifdef USE_SQLITE
-        if (ExistsSQLiteDatabase(path)) {
+        if (IsSQLiteFile(SQLiteDataFile(path))) {
             if (format) {
                 error = Untranslated(strprintf("Failed to load database path '%s'. Data is in ambiguous format.", path.string()));
                 status = DatabaseStatus::FAILED_BAD_FORMAT;
@@ -1027,7 +1024,6 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas
             }
             format = DatabaseFormat::SQLITE;
         }
-#endif
     } else if (options.require_existing) {
         error = Untranslated(strprintf("Failed to load database path '%s'. Path does not exist.", path.string()));
         status = DatabaseStatus::FAILED_NOT_FOUND;
diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp
index d6e6f015db..16ddad3a84 100644
--- a/src/wallet/walletutil.cpp
+++ b/src/wallet/walletutil.cpp
@@ -7,17 +7,6 @@
 #include <logging.h>
 #include <util/system.h>
 
-#ifdef USE_BDB
-bool ExistsBerkeleyDatabase(const fs::path& path);
-#else
-#   define ExistsBerkeleyDatabase(path)  (false)
-#endif
-#ifdef USE_SQLITE
-bool ExistsSQLiteDatabase(const fs::path& path);
-#else
-#   define ExistsSQLiteDatabase(path)  (false)
-#endif
-
 fs::path GetWalletDir()
 {
     fs::path path;
@@ -40,50 +29,6 @@ fs::path GetWalletDir()
     return path;
 }
 
-std::vector<fs::path> ListWalletDir()
-{
-    const fs::path wallet_dir = GetWalletDir();
-    const size_t offset = wallet_dir.string().size() + 1;
-    std::vector<fs::path> paths;
-    boost::system::error_code ec;
-
-    for (auto it = fs::recursive_directory_iterator(wallet_dir, ec); it != fs::recursive_directory_iterator(); it.increment(ec)) {
-        if (ec) {
-            LogPrintf("%s: %s %s\n", __func__, ec.message(), it->path().string());
-            continue;
-        }
-
-        try {
-            // Get wallet path relative to walletdir by removing walletdir from the wallet path.
-            // This can be replaced by boost::filesystem::lexically_relative once boost is bumped to 1.60.
-            const fs::path path = it->path().string().substr(offset);
-
-            if (it->status().type() == fs::directory_file &&
-                (ExistsBerkeleyDatabase(it->path()) || ExistsSQLiteDatabase(it->path()))) {
-                // Found a directory which contains wallet.dat btree file, add it as a wallet.
-                paths.emplace_back(path);
-            } else if (it.level() == 0 && it->symlink_status().type() == fs::regular_file && ExistsBerkeleyDatabase(it->path())) {
-                if (it->path().filename() == "wallet.dat") {
-                    // Found top-level wallet.dat btree file, add top level directory ""
-                    // as a wallet.
-                    paths.emplace_back();
-                } else {
-                    // Found top-level btree file not called wallet.dat. Current bitcoin
-                    // software will never create these files but will allow them to be
-                    // opened in a shared database environment for backwards compatibility.
-                    // Add it to the list of available wallets.
-                    paths.emplace_back(path);
-                }
-            }
-        } catch (const std::exception& e) {
-            LogPrintf("%s: Error scanning %s: %s\n", __func__, it->path().string(), e.what());
-            it.no_push();
-        }
-    }
-
-    return paths;
-}
-
 bool IsFeatureSupported(int wallet_version, int feature_version)
 {
     return wallet_version >= feature_version;
diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h
index 27521abd81..d4143ceff4 100644
--- a/src/wallet/walletutil.h
+++ b/src/wallet/walletutil.h
@@ -65,9 +65,6 @@ enum WalletFlags : uint64_t {
 //! Get the path of the wallet directory.
 fs::path GetWalletDir();
 
-//! Get wallets in wallet directory.
-std::vector<fs::path> ListWalletDir();
-
 /** Descriptor with some wallet metadata */
 class WalletDescriptor
 {