merge bitcoin#20202: Make BDB support optional

build-aux/m4/bitcoin_find_bdb48.m4

@@ -6,7 +6,9 @@ AC_DEFUN([BITCOIN_FIND_BDB48],[
   AC_ARG_VAR(BDB_CFLAGS, [C compiler flags for BerkeleyDB, bypasses autodetection])
   AC_ARG_VAR(BDB_LIBS, [Linker flags for BerkeleyDB, bypasses autodetection])
 
-  if test "x$BDB_CFLAGS" = "x"; then
+  if test "x$use_bdb" = "xno"; then
+    use_bdb=no
+  elif test "x$BDB_CFLAGS" = "x"; then
     AC_MSG_CHECKING([for Berkeley DB C++ headers])
     BDB_CPPFLAGS=
     bdbpath=X
@@ -44,25 +46,30 @@ AC_DEFUN([BITCOIN_FIND_BDB48],[
       ],[])
     done
     if test "x$bdbpath" = "xX"; then
+      use_bdb=no
       AC_MSG_RESULT([no])
-      AC_MSG_ERROR([libdb_cxx headers missing, ]AC_PACKAGE_NAME[ requires this library for wallet functionality (--disable-wallet to disable wallet functionality)])
+      AC_MSG_ERROR([libdb_cxx headers missing, ]AC_PACKAGE_NAME[ requires this library for BDB wallet support (--without-bdb to disable BDB wallet support)])
     elif test "x$bdb48path" = "xX"; then
       BITCOIN_SUBDIR_TO_INCLUDE(BDB_CPPFLAGS,[${bdbpath}],db_cxx)
       AC_ARG_WITH([incompatible-bdb],[AS_HELP_STRING([--with-incompatible-bdb], [allow using a bdb version other than 4.8])],[
-        AC_MSG_WARN([Found Berkeley DB other than 4.8; wallets opened by this build will not be portable!])
+        AC_MSG_WARN([Found Berkeley DB other than 4.8; BDB wallets opened by this build will not be portable!])
       ],[
-        AC_MSG_ERROR([Found Berkeley DB other than 4.8, required for portable wallets (--with-incompatible-bdb to ignore or --disable-wallet to disable wallet functionality)])
+        AC_MSG_ERROR([Found Berkeley DB other than 4.8, required for portable BDB wallets (--with-incompatible-bdb to ignore or --without-bdb to disable BDB wallet support)])
       ])
+      use_bdb=yes
     else
       BITCOIN_SUBDIR_TO_INCLUDE(BDB_CPPFLAGS,[${bdb48path}],db_cxx)
       bdbpath="${bdb48path}"
+      use_bdb=yes
     fi
   else
     BDB_CPPFLAGS=${BDB_CFLAGS}
   fi
   AC_SUBST(BDB_CPPFLAGS)
 
-  if test "x$BDB_LIBS" = "x"; then
+  if test "x$use_bdb" = "xno"; then
+    use_bdb=no
+  elif test "x$BDB_LIBS" = "x"; then
     # TODO: Ideally this could find the library version and make sure it matches the headers being used
     for searchlib in db_cxx-4.8 db_cxx db4_cxx; do
       AC_CHECK_LIB([$searchlib],[main],[
@@ -71,8 +78,12 @@ AC_DEFUN([BITCOIN_FIND_BDB48],[
       ])
     done
     if test "x$BDB_LIBS" = "x"; then
-        AC_MSG_ERROR([libdb_cxx missing, ]AC_PACKAGE_NAME[ requires this library for wallet functionality (--disable-wallet to disable wallet functionality)])
+        AC_MSG_ERROR([libdb_cxx missing, ]AC_PACKAGE_NAME[ requires this library for BDB wallet support (--without-bdb to disable BDB wallet support)])
     fi
   fi
+  if test "x$use_bdb" != "xno"; then
     AC_SUBST(BDB_LIBS)
+    AC_DEFINE([USE_BDB], [1], [Define if BDB support should be compiled in])
+    use_bdb=yes
+  fi
 ])

configure.ac

@@ -118,7 +118,7 @@ AC_ARG_ENABLE([wallet],
   [AS_HELP_STRING([--disable-wallet],
   [disable wallet (enabled by default)])],
   [enable_wallet=$enableval],
-  [enable_wallet=yes])
+  [enable_wallet=auto])
 
 AC_ARG_WITH([sqlite],
   [AS_HELP_STRING([--with-sqlite=yes|no|auto],
@@ -126,6 +126,12 @@ AC_ARG_WITH([sqlite],
   [use_sqlite=$withval],
   [use_sqlite=auto])
 
+AC_ARG_WITH([bdb],
+  [AS_HELP_STRING([--without-bdb],
+  [disable bdb wallet support (default is enabled if wallet is enabled)])],
+  [use_bdb=$withval],
+  [use_bdb=auto])
+
 AC_ARG_WITH([miniupnpc],
   [AS_HELP_STRING([--with-miniupnpc],
   [enable UPNP (default is yes if libminiupnpc is found)])],
@@ -1305,6 +1311,14 @@ if test x$enable_wallet != xno; then
       fi
     fi
     AC_MSG_RESULT([$use_sqlite])
+
+    dnl Disable wallet if both --without-bdb and --without-sqlite
+    if test "x$use_bdb$use_sqlite" = "xnono"; then
+        if test "x$enable_wallet" = "xyes"; then
+            AC_MSG_ERROR([wallet functionality requested but no BDB or SQLite support available.])
+        fi
+        enable_wallet=no
+    fi
 fi
 
 dnl Check for libminiupnpc (optional)
@@ -1530,6 +1544,7 @@ AC_MSG_CHECKING([if wallet should be enabled])
 if test x$enable_wallet != xno; then
   AC_MSG_RESULT(yes)
   AC_DEFINE_UNQUOTED([ENABLE_WALLET],[1],[Define to 1 to enable wallet functions])
+  enable_wallet=yes
 
 else
   AC_MSG_RESULT(no)
@@ -1673,6 +1688,7 @@ AM_CONDITIONAL([TARGET_LINUX], [test x$TARGET_OS = xlinux])
 AM_CONDITIONAL([TARGET_WINDOWS], [test x$TARGET_OS = xwindows])
 AM_CONDITIONAL([ENABLE_WALLET],[test x$enable_wallet = xyes])
 AM_CONDITIONAL([USE_SQLITE], [test "x$use_sqlite" = "xyes"])
+AM_CONDITIONAL([USE_BDB], [test "x$use_bdb" = "xyes"])
 AM_CONDITIONAL([ENABLE_TESTS],[test x$BUILD_TEST = xyes])
 AM_CONDITIONAL([ENABLE_FUZZ],[test x$enable_fuzz = xyes])
 AM_CONDITIONAL([ENABLE_FUZZ_LINK_ALL],[test x$enable_danger_fuzz_link_all = xyes])
@@ -1739,6 +1755,7 @@ AC_SUBST(ARM_CRC_CXXFLAGS)
 AC_SUBST(ARM_SHANI_CXXFLAGS)
 AC_SUBST(LIBTOOL_APP_LDFLAGS)
 AC_SUBST(USE_SQLITE)
+AC_SUBST(USE_BDB)
 AC_SUBST(USE_UPNP)
 AC_SUBST(USE_QRCODE)
 AC_SUBST(BOOST_LIBS)
@@ -1814,6 +1831,7 @@ echo "  with wallet         = $enable_wallet"
 echo "  with gui / qt       = $bitcoin_enable_qt"
 if test "x$enable_wallet" != "xno"; then
     echo "  with sqlite         = $use_sqlite"
+    echo "  with bdb            = $use_bdb"
 fi
 if test x$bitcoin_enable_qt != xno; then
     echo "    with qr           = $use_qr"

doc/dependencies.md

@@ -38,7 +38,7 @@ Some dependencies are not needed in all configurations. The following are some f
 
 #### Options passed to `./configure`
 * MiniUPnPc is not needed with `--without-miniupnpc`.
-* Berkeley DB is not needed with `--disable-wallet`.
+* Berkeley DB is not needed with `--disable-wallet` or `--without-bdb`.
 * SQLite is not needed with `--disable-wallet` or `--without-sqlite`.
 * libnatpmp is not needed with `--without-natpmp`.
 * Qt is not needed with `--without-gui`.

src/Makefile.am

@@ -491,7 +491,6 @@ libbitcoin_wallet_a_SOURCES = \
   coinjoin/options.cpp \
   coinjoin/util.cpp \
   interfaces/wallet.cpp \
-  wallet/bdb.cpp \
   wallet/coincontrol.cpp \
   wallet/context.cpp \
   wallet/crypter.cpp \
@@ -501,7 +500,6 @@ libbitcoin_wallet_a_SOURCES = \
   wallet/psbtwallet.cpp \
   wallet/rpcdump.cpp \
   wallet/rpcwallet.cpp \
-  wallet/salvage.cpp \
   wallet/scriptpubkeyman.cpp \
   wallet/wallet.cpp \
   wallet/walletdb.cpp \
@@ -512,6 +510,9 @@ libbitcoin_wallet_a_SOURCES = \
 if USE_SQLITE
 libbitcoin_wallet_a_SOURCES += wallet/sqlite.cpp
 endif
+if USE_BDB
+libbitcoin_wallet_a_SOURCES += wallet/bdb.cpp wallet/salvage.cpp
+endif
 
 libbitcoin_wallet_tool_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
 libbitcoin_wallet_tool_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)

src/Makefile.test.include

@@ -165,7 +165,6 @@ BITCOIN_TESTS =\
 if ENABLE_WALLET
 BITCOIN_TESTS += \
   wallet/test/coinjoin_tests.cpp \
-  wallet/test/db_tests.cpp \
   wallet/test/psbt_wallet_tests.cpp \
   wallet/test/wallet_tests.cpp \
   wallet/test/wallet_crypto_tests.cpp \
@@ -173,6 +172,10 @@ BITCOIN_TESTS += \
   wallet/test/init_tests.cpp \
   wallet/test/ismine_tests.cpp
 
+if USE_BDB
+BITCOIN_TESTS += wallet/test/db_tests.cpp
+endif
+
 BITCOIN_TEST_SUITE += \
   wallet/test/wallet_test_fixture.cpp \
   wallet/test/wallet_test_fixture.h \

src/qt/rpcconsole.cpp

@@ -26,6 +26,9 @@
 #include <univalue.h>
 
 #ifdef ENABLE_WALLET
+#ifdef USE_BDB
+#include <wallet/bdb.h>
+#endif
 #include <wallet/db.h>
 #include <wallet/walletutil.h>
 #endif

src/wallet/init.cpp

@@ -17,9 +17,12 @@
 #include <util/moneystr.h>
 #include <util/translation.h>
 #include <validation.h>
-#include <walletinitinterface.h>
+#ifdef USE_BDB
+#include <wallet/bdb.h>
+#endif
 #include <wallet/coincontrol.h>
 #include <wallet/wallet.h>
+#include <walletinitinterface.h>
 
 #include <coinjoin/client.h>
 #include <coinjoin/options.h>
@@ -95,9 +98,14 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
     argsman.AddArg("-coinjoinrounds=<n>", strprintf("Use N separate masternodes for each denominated input to mix funds (%u-%u, default: %u)", MIN_COINJOIN_ROUNDS, MAX_COINJOIN_ROUNDS, DEFAULT_COINJOIN_ROUNDS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_COINJOIN);
     argsman.AddArg("-coinjoinsessions=<n>", strprintf("Use N separate masternodes in parallel to mix funds (%u-%u, default: %u)", MIN_COINJOIN_SESSIONS, MAX_COINJOIN_SESSIONS, DEFAULT_COINJOIN_SESSIONS), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_COINJOIN);
 
+#ifdef USE_BDB
     argsman.AddArg("-dblogsize=<n>", strprintf("Flush wallet database activity from memory to disk log every <n> megabytes (default: %u)", DEFAULT_WALLET_DBLOGSIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
     argsman.AddArg("-flushwallet", strprintf("Run a thread to flush wallet periodically (default: %u)", DEFAULT_FLUSHWALLET), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
     argsman.AddArg("-privdb", strprintf("Sets the DB_PRIVATE flag in the wallet db environment (default: %u)", DEFAULT_WALLET_PRIVDB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
+#else
+    argsman.AddHiddenArgs({"-dblogsize", "-flushwallet", "-privdb"});
+#endif
+
     argsman.AddArg("-walletrejectlongchains", strprintf("Wallet will not create transactions that violate mempool chain limits (default: %u)", DEFAULT_WALLET_REJECT_LONG_CHAINS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
 
     argsman.AddHiddenArgs({"-zapwallettxes"});

src/wallet/salvage.cpp

@@ -6,6 +6,7 @@
 #include <fs.h>
 #include <streams.h>
 #include <util/translation.h>
+#include <wallet/bdb.h>
 #include <wallet/salvage.h>
 #include <wallet/wallet.h>
 #include <wallet/walletdb.h>
@@ -27,6 +28,7 @@ bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::v
     DatabaseStatus status;
     options.require_existing = true;
     options.verify = false;
+    options.require_format = DatabaseFormat::BERKELEY;
     std::unique_ptr<WalletDatabase> database = MakeDatabase(file_path, options, status, error);
     if (!database) return false;
 

src/wallet/scriptpubkeyman.cpp

@@ -5,9 +5,11 @@
 #include <key_io.h>
 #include <chainparams.h>
 #include <coinjoin/client.h>
+#include <logging.h>
 #include <script/descriptor.h>
 #include <util/bip32.h>
 #include <util/strencodings.h>
+#include <util/system.h>
 #include <util/translation.h>
 #include <wallet/scriptpubkeyman.h>
 #include <wallet/wallet.h>

src/wallet/scriptpubkeyman.h

@@ -7,6 +7,7 @@
 
 #include <script/signingprovider.h>
 #include <script/standard.h>
+#include <util/time.h>
 #include <wallet/crypter.h>
 #include <wallet/ismine.h>
 #include <wallet/walletdb.h>

src/wallet/sqlite.cpp

@@ -205,7 +205,9 @@ void SQLiteDatabase::Open()
     }
 
     if (m_db == nullptr) {
+        if (!m_mock) {
             TryCreateDirectories(m_dir_path);
+        }
         int ret = sqlite3_open_v2(m_file_path.c_str(), &m_db, flags, nullptr);
         if (ret != SQLITE_OK) {
             throw std::runtime_error(strprintf("SQLiteDatabase: Failed to open database: %s\n", sqlite3_errstr(ret)));

src/wallet/wallet.cpp

@@ -34,6 +34,9 @@
 #include <util/translation.h>
 #include <util/validation.h>
 #include <validation.h>
+#ifdef USE_BDB
+#include <wallet/bdb.h>
+#endif
 #include <wallet/coincontrol.h>
 #include <wallet/coinselection.h>
 #include <wallet/fees.h>
@@ -4626,6 +4629,7 @@ bool CWallet::BackupWallet(const std::string& strDest)
 
 // This should be called carefully:
 // either supply the actual wallet_path to make a raw copy of wallet.dat or "" to backup current instance via BackupWallet()
+#ifdef USE_BDB
 bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error_string, std::vector<bilingual_str>& warnings)
 {
     std::string strWalletName = GetName();
@@ -4764,6 +4768,13 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error
 
     return true;
 }
+#elif USE_SQLITE
+bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error_string, std::vector<bilingual_str>& warnings)
+{
+    WalletLogPrintf("Automatic wallet backups are currently only supported with Berkeley DB!\n");
+    return false;
+}
+#endif // USE_BDB
 
 void CWallet::NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr<const llmq::CInstantSendLock>& islock)
 {

src/wallet/walletdb.cpp

@@ -18,7 +18,9 @@
 #include <util/system.h>
 #include <util/time.h>
 #include <util/translation.h>
+#ifdef USE_BDB
 #include <wallet/bdb.h>
+#endif
 #ifdef USE_SQLITE
 #include <wallet/sqlite.h>
 #endif
@@ -828,9 +830,11 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas
 
     std::optional<DatabaseFormat> format;
     if (exists) {
+#ifdef USE_BDB
         if (ExistsBerkeleyDatabase(path)) {
             format = DatabaseFormat::BERKELEY;
         }
+#endif
 #ifdef USE_SQLITE
         if (ExistsSQLiteDatabase(path)) {
             if (format) {
@@ -869,15 +873,31 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas
     // Format is not set when a db doesn't already exist, so use the format specified by the options if it is set.
     if (!format && options.require_format) format = options.require_format;
 
+    // If the format is not specified or detected, choose the default format based on what is available. We prefer BDB over SQLite for now..
+    if (!format) {
 #ifdef USE_SQLITE
-    if (format && format == DatabaseFormat::SQLITE) {
-        return MakeSQLiteDatabase(path, options, status, error);
-    }
-#else
-    assert(format != DatabaseFormat::SQLITE);
+        format = DatabaseFormat::SQLITE;
 #endif
+#ifdef USE_BDB
+        format = DatabaseFormat::BERKELEY;
+#endif
+    }
 
+    if (format == DatabaseFormat::SQLITE) {
+#ifdef USE_SQLITE
+        return MakeSQLiteDatabase(path, options, status, error);
+#endif
+        error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support SQLite database format.", path.string()));
+        status = DatabaseStatus::FAILED_BAD_FORMAT;
+        return nullptr;
+    }
+
+#ifdef USE_BDB
     return MakeBerkeleyDatabase(path, options, status, error);
+#endif
+    error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", path.string()));
+    status = DatabaseStatus::FAILED_BAD_FORMAT;
+    return nullptr;
 }
 
 /** Return object for accessing dummy database with no read/write capabilities. */
@@ -889,5 +909,9 @@ std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase()
 /** Return object for accessing temporary in-memory database. */
 std::unique_ptr<WalletDatabase> CreateMockWalletDatabase()
 {
+#ifdef USE_BDB
     return std::make_unique<BerkeleyDatabase>(std::make_shared<BerkeleyEnvironment>(), "");
+#elif USE_SQLITE
+    return std::make_unique<SQLiteDatabase>("", "", true);
+#endif
 }

src/wallet/walletdb.h

@@ -8,7 +8,6 @@
 
 #include <amount.h>
 #include <script/sign.h>
-#include <wallet/bdb.h>
 #include <wallet/db.h>
 #include <key.h>
 

src/wallet/wallettool.cpp

@@ -119,6 +119,7 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)
             WalletShowInfo(wallet_instance.get());
             wallet_instance->Close();
         } else if (command == "salvage") {
+#ifdef USE_BDB
             bilingual_str error;
             std::vector<bilingual_str> warnings;
             bool ret = RecoverDatabaseFile(path, error, warnings);
@@ -131,6 +132,10 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)
                 }
             }
             return ret;
+#else
+            tfm::format(std::cerr, "Salvage command is not available as BDB support is not compiled");
+            return false;
+#endif
         }
     } else {
         tfm::format(std::cerr, "Invalid command: %s\n", command);

src/wallet/walletutil.cpp

@@ -7,7 +7,11 @@
 #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