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

f3d870fc22 wallet: List all wallets in non-SQLite or non-BDB builds (Russell Yanofsky)
d70dc89e78 refactor: Consolidate redundant wallet database path and exists functions (Russell Yanofsky)
6a7a63644c refactor: Drop call to GetWalletEnv in wallet salvage code (Russell Yanofsky)
6ee9cbdd18 refactor: Replace ListWalletDir() function with ListDatabases() (Russell Yanofsky)
5aaeb6cf87 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 f3d870fc22
  promag:
    Tested ACK f3d870fc22. Tested a --without-sqlite build with sqlite wallets.

Tree-SHA512: 029ad21559dbc338b5f351d05113c51bc25bce830f4f4e18bcd82287bc528275347a60249da65b91d252632aeb70b25d057bd59c704bfcaafb9f790bc5b59762

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;
-}

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);
 

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;
+}

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

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;

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);

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;

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;
-}

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

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";

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;

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;

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
 {