Merge bitcoin#31961: Require sqlite when building the wallet

36b6f36 build: require sqlite when building the wallet (Sjors Provoost)

Pull request description:

  Require that sqlite is available in order to compile the wallet. Removes instances of `USE_SQLITE` since it is no longer possible to not have sqlite available.

  The `NO_SQLITE` option is dropped from depends.

  This is another step towards dropping the legacy wallet, extracted from bitcoin#31250.

ACKs for top commit:
  m3dwards:
    ACK 36b6f36
  davidgumberg:
    crACK bitcoin@36b6f36
  hebasto:
    re-ACK 36b6f36.

Tree-SHA512: 870a0135671c80c4f28602119eb8637a1ed43b51b1673bfe88425782fb62ec6ef0f3d6baf0d5984d6a243779b0f63423fd4c4dc324ef87bffba13d63e05ad793

Author: fanquake

.github/workflows/ci.yml

@@ -171,7 +171,7 @@ jobs:
             generate-options: '-DBUILD_GUI=ON -DWITH_BDB=ON -DWITH_ZMQ=ON -DBUILD_BENCH=ON -DWERROR=ON'
             job-name: 'Win64 native, VS 2022'
           - job-type: fuzz
-            generate-options: '-DVCPKG_MANIFEST_NO_DEFAULT_FEATURES=ON -DVCPKG_MANIFEST_FEATURES="sqlite" -DBUILD_GUI=OFF -DBUILD_FOR_FUZZING=ON -DWERROR=ON'
+            generate-options: '-DVCPKG_MANIFEST_NO_DEFAULT_FEATURES=ON -DVCPKG_MANIFEST_FEATURES="wallet" -DBUILD_GUI=OFF -DBUILD_FOR_FUZZING=ON -DWERROR=ON'
             job-name: 'Win64 native fuzz, VS 2022'
 
     steps:

CMakeLists.txt

@@ -102,15 +102,13 @@ option(BUILD_UTIL_CHAINSTATE "Build experimental bitcoin-chainstate executable."
 option(BUILD_KERNEL_LIB "Build experimental bitcoinkernel library." ${BUILD_UTIL_CHAINSTATE})
 
 option(ENABLE_WALLET "Enable wallet." ON)
-option(WITH_SQLITE "Enable SQLite wallet support." ${ENABLE_WALLET})
-if(WITH_SQLITE)
+if(ENABLE_WALLET)
   if(VCPKG_TARGET_TRIPLET)
     # Use of the `unofficial::` namespace is a vcpkg package manager convention.
     find_package(unofficial-sqlite3 CONFIG REQUIRED)
   else()
     find_package(SQLite3 3.7.17 REQUIRED)
   endif()
-  set(USE_SQLITE ON)
 endif()
 option(WITH_BDB "Enable Berkeley DB (BDB) wallet support." OFF)
 cmake_dependent_option(WARN_INCOMPATIBLE_BDB "Warn when using a Berkeley DB (BDB) version other than 4.8." ON "WITH_BDB" OFF)
@@ -652,7 +650,6 @@ message("  libbitcoinkernel (experimental) ..... ${BUILD_KERNEL_LIB}")
 message("Optional features:")
 message("  wallet support ...................... ${ENABLE_WALLET}")
 if(ENABLE_WALLET)
-  message("   - descriptor wallets (SQLite) ...... ${WITH_SQLITE}")
   message("   - legacy wallets (Berkeley DB) ..... ${WITH_BDB}")
 endif()
 message("  external signer ..................... ${ENABLE_EXTERNAL_SIGNER}")

CMakePresets.json

@@ -83,7 +83,6 @@
         "WITH_BDB": "ON",
         "WITH_MULTIPROCESS": "ON",
         "WITH_QRENCODE": "ON",
-        "WITH_SQLITE": "ON",
         "WITH_USDT": "ON",
         "WITH_ZMQ": "ON"
       }

cmake/bitcoin-build-config.h.in

@@ -144,7 +144,4 @@
 /* Define if QR support should be compiled in */
 #cmakedefine USE_QRCODE 1
 
-/* Define if sqlite support should be compiled in */
-#cmakedefine USE_SQLITE 1
-
 #endif //BITCOIN_CONFIG_H

depends/Makefile

@@ -37,7 +37,6 @@ NO_LIBEVENT ?=
 NO_QT ?=
 NO_QR ?=
 NO_BDB ?=
-NO_SQLITE ?=
 NO_WALLET ?=
 NO_ZMQ ?=
 NO_USDT ?=
@@ -160,8 +159,7 @@ qrencode_packages_$(NO_QR) = $(qrencode_$(host_os)_packages)
 qt_packages_$(NO_QT) = $(qt_packages) $(qt_$(host_os)_packages) $(qt_$(host_arch)_$(host_os)_packages) $(qrencode_packages_)
 
 bdb_packages_$(NO_BDB) = $(bdb_packages)
-sqlite_packages_$(NO_SQLITE) = $(sqlite_packages)
-wallet_packages_$(NO_WALLET) = $(bdb_packages_) $(sqlite_packages_)
+wallet_packages_$(NO_WALLET) = $(bdb_packages_) $(sqlite_packages)
 
 zmq_packages_$(NO_ZMQ) = $(zmq_packages)
 multiprocess_packages_$(MULTIPROCESS) = $(multiprocess_packages)
@@ -233,7 +231,6 @@ $(host_prefix)/toolchain.cmake : toolchain.cmake.in $(host_prefix)/.stamp_$(fina
             -e 's|@zmq_packages@|$(zmq_packages_)|' \
             -e 's|@wallet_packages@|$(wallet_packages_)|' \
             -e 's|@bdb_packages@|$(bdb_packages_)|' \
-            -e 's|@sqlite_packages@|$(sqlite_packages_)|' \
             -e 's|@usdt_packages@|$(usdt_packages_)|' \
             -e 's|@multiprocess@|$(MULTIPROCESS)|' \
             $< > $@

depends/README.md

@@ -115,9 +115,8 @@ The following can be set when running make: `make FOO=bar`
 - `NO_QT`: Don't download/build/cache Qt and its dependencies
 - `NO_QR`: Don't download/build/cache packages needed for enabling qrencode
 - `NO_ZMQ`: Don't download/build/cache packages needed for enabling ZeroMQ
-- `NO_WALLET`: Don't download/build/cache libs needed to enable the wallet
+- `NO_WALLET`: Don't download/build/cache libs needed to enable the wallet (SQLite)
 - `NO_BDB`: Don't download/build/cache BerkeleyDB
-- `NO_SQLITE`: Don't download/build/cache SQLite
 - `NO_USDT`: Don't download/build/cache packages needed for enabling USDT tracepoints
 - `MULTIPROCESS`: Build libmultiprocess (experimental)
 - `DEBUG`: Disable some optimizations and enable more runtime checking

depends/toolchain.cmake.in

@@ -132,13 +132,6 @@ else()
   set(WITH_BDB ON CACHE BOOL "")
 endif()
 
-set(sqlite_packages @sqlite_packages@)
-if("${wallet_packages}" STREQUAL "" OR "${sqlite_packages}" STREQUAL "")
-  set(WITH_SQLITE OFF CACHE BOOL "")
-else()
-  set(WITH_SQLITE ON CACHE BOOL "")
-endif()
-
 set(usdt_packages @usdt_packages@)
 if("${usdt_packages}" STREQUAL "")
   set(WITH_USDT OFF CACHE BOOL "")

doc/build-freebsd.md

@@ -42,7 +42,7 @@ from ports. However, you can build DB 4.8 yourself [using depends](/depends).
 
 ```bash
 pkg install gmake
-gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_ZMQ=1 NO_USDT=1
+gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_ZMQ=1 NO_USDT=1
 ```
 
 When the build is complete, the Berkeley DB installation location will be displayed:

doc/build-openbsd.md

@@ -44,7 +44,7 @@ from ports. However you can build it yourself, [using depends](/depends).
 Refer to [depends/README.md](/depends/README.md) for detailed instructions.
 
 ```bash
-gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_ZMQ=1 NO_USDT=1
+gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_ZMQ=1 NO_USDT=1
 ...
 to: /path/to/bitcoin/depends/*-unknown-openbsd*
 ```
@@ -103,7 +103,7 @@ There are many ways to configure Bitcoin Core, here are a few common examples:
 This enables descriptor wallet support and the GUI, assuming SQLite and Qt 5 are installed.
 
 ```bash
-cmake -B build -DWITH_SQLITE=ON -DBUILD_GUI=ON
+cmake -B build -DBUILD_GUI=ON
 ```
 
 Run `cmake -B build -LH` to see the full list of available options.

doc/build-unix.md

@@ -145,7 +145,7 @@ The legacy wallet uses Berkeley DB. To ensure backwards compatibility it is
 recommended to use Berkeley DB 4.8. If you have to build it yourself, and don't
 want to use any other libraries built in depends, you can do:
 ```bash
-make -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_ZMQ=1 NO_USDT=1
+make -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_ZMQ=1 NO_USDT=1
 ...
 to: /path/to/bitcoin/depends/x86_64-pc-linux-gnu
 ```

src/bench/wallet_create.cpp

@@ -3,7 +3,6 @@
 // file COPYING or https://www.opensource.org/licenses/mit-license.php.
 
 #include <bench/bench.h>
-#include <bitcoin-build-config.h> // IWYU pragma: keep
 #include <random.h>
 #include <support/allocators/secure.h>
 #include <test/util/setup_common.h>
@@ -61,9 +60,7 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted)
 static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); }
 static void WalletCreateEncrypted(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/true); }
 
-#ifdef USE_SQLITE
 BENCHMARK(WalletCreatePlain, benchmark::PriorityLevel::LOW);
 BENCHMARK(WalletCreateEncrypted, benchmark::PriorityLevel::LOW);
-#endif
 
 } // namespace wallet

src/bench/wallet_ismine.cpp

@@ -75,10 +75,8 @@ static void WalletIsMineLegacy(benchmark::Bench& bench) { WalletIsMine(bench, /*
 BENCHMARK(WalletIsMineLegacy, benchmark::PriorityLevel::LOW);
 #endif
 
-#ifdef USE_SQLITE
 static void WalletIsMineDescriptors(benchmark::Bench& bench) { WalletIsMine(bench, /*legacy_wallet=*/false); }
 static void WalletIsMineMigratedDescriptors(benchmark::Bench& bench) { WalletIsMine(bench, /*legacy_wallet=*/false, /*num_combo=*/2000); }
 BENCHMARK(WalletIsMineDescriptors, benchmark::PriorityLevel::LOW);
 BENCHMARK(WalletIsMineMigratedDescriptors, benchmark::PriorityLevel::LOW);
-#endif
 } // namespace wallet

src/bench/wallet_loading.cpp

@@ -73,8 +73,6 @@ static void WalletLoadingLegacy(benchmark::Bench& bench) { WalletLoading(bench,
 BENCHMARK(WalletLoadingLegacy, benchmark::PriorityLevel::HIGH);
 #endif
 
-#ifdef USE_SQLITE
 static void WalletLoadingDescriptors(benchmark::Bench& bench) { WalletLoading(bench, /*legacy_wallet=*/false); }
 BENCHMARK(WalletLoadingDescriptors, benchmark::PriorityLevel::HIGH);
-#endif
 } // namespace wallet

src/bench/wallet_migration.cpp

@@ -2,8 +2,6 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or https://www.opensource.org/licenses/mit-license.php.
 
-#include <bitcoin-build-config.h> // IWYU pragma: keep
-
 #include <bench/bench.h>
 #include <kernel/chain.h>
 #include <interfaces/chain.h>
@@ -17,8 +15,6 @@
 
 #include <optional>
 
-#if defined(USE_SQLITE) // only enable benchmark when sqlite is enabled
-
 namespace wallet{
 
 static void WalletMigration(benchmark::Bench& bench)
@@ -75,5 +71,3 @@ static void WalletMigration(benchmark::Bench& bench)
 BENCHMARK(WalletMigration, benchmark::PriorityLevel::LOW);
 
 } // namespace wallet
-
-#endif // end USE_SQLITE && USE_BDB

src/qt/bitcoingui.cpp

@@ -1215,11 +1215,6 @@ void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVer
 void BitcoinGUI::createWallet()
 {
 #ifdef ENABLE_WALLET
-#ifndef USE_SQLITE
-    // Compiled without sqlite support (required for descriptor wallets)
-    message(tr("Error creating wallet"), tr("Cannot create new wallet, the software was compiled without sqlite support (required for descriptor wallets)"), CClientUIInterface::MSG_ERROR);
-    return;
-#endif // USE_SQLITE
     auto activity = new CreateWalletActivity(getWalletController(), this);
     connect(activity, &CreateWalletActivity::created, this, &BitcoinGUI::setCurrentWallet);
     connect(activity, &CreateWalletActivity::created, rpcConsole, &RPCConsole::setCurrentWallet);

src/wallet/CMakeLists.txt

@@ -28,6 +28,7 @@ add_library(bitcoin_wallet STATIC EXCLUDE_FROM_ALL
   rpc/wallet.cpp
   scriptpubkeyman.cpp
   spend.cpp
+  sqlite.cpp
   transaction.cpp
   wallet.cpp
   walletdb.cpp
@@ -37,22 +38,13 @@ target_link_libraries(bitcoin_wallet
   PRIVATE
     core_interface
     bitcoin_common
+    $<TARGET_NAME_IF_EXISTS:unofficial::sqlite3::sqlite3>
+    $<TARGET_NAME_IF_EXISTS:SQLite::SQLite3>
     univalue
     Boost::headers
     $<TARGET_NAME_IF_EXISTS:USDT::headers>
 )
 
-if(NOT USE_SQLITE AND NOT USE_BDB)
-  message(FATAL_ERROR "Wallet functionality requested but no BDB or SQLite support available.")
-endif()
-if(USE_SQLITE)
-  target_sources(bitcoin_wallet PRIVATE sqlite.cpp)
-  target_link_libraries(bitcoin_wallet
-    PRIVATE
-      $<TARGET_NAME_IF_EXISTS:unofficial::sqlite3::sqlite3>
-      $<TARGET_NAME_IF_EXISTS:SQLite::SQLite3>
-  )
-endif()
 if(USE_BDB)
   target_sources(bitcoin_wallet PRIVATE bdb.cpp salvage.cpp)
   target_link_libraries(bitcoin_wallet PUBLIC BerkeleyDB::BerkeleyDB)

src/wallet/init.cpp

@@ -90,11 +90,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
     argsman.AddHiddenArgs({"-dblogsize", "-flushwallet", "-privdb", "-swapbdbendian"});
 #endif
 
-#ifdef USE_SQLITE
     argsman.AddArg("-unsafesqlitesync", "Set SQLite synchronous=OFF to disable waiting for the database to sync to disk. This is unsafe and can cause data loss and corruption. This option is only used by tests to improve their performance (default: false)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);
-#else
-    argsman.AddHiddenArgs({"-unsafesqlitesync"});
-#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.AddArg("-walletcrosschain", strprintf("Allow reusing wallet files across chains (default: %u)", DEFAULT_WALLETCROSSCHAIN), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::WALLET_DEBUG_TEST);

src/wallet/rpc/wallet.cpp

@@ -403,9 +403,6 @@ static RPCHelpMan createwallet()
         flags |= WALLET_FLAG_AVOID_REUSE;
     }
     if (self.Arg<bool>("descriptors")) {
-#ifndef USE_SQLITE
-        throw JSONRPCError(RPC_WALLET_ERROR, "Compiled without sqlite support (required for descriptor wallets)");
-#endif
         flags |= WALLET_FLAG_DESCRIPTORS;
     } else {
         if (!context.chain->rpcEnableDeprecated("create_bdb")) {

src/wallet/test/db_tests.cpp

@@ -13,9 +13,7 @@
 #ifdef USE_BDB
 #include <wallet/bdb.h>
 #endif
-#ifdef USE_SQLITE
 #include <wallet/sqlite.h>
-#endif
 #include <wallet/migrate.h>
 #include <wallet/test/util.h>
 #include <wallet/walletutil.h> // for WALLET_FLAG_DESCRIPTORS
@@ -138,9 +136,7 @@ static std::vector<std::unique_ptr<WalletDatabase>> TestDatabases(const fs::path
     // Needs BDB to make the DB to read
     dbs.emplace_back(std::make_unique<BerkeleyRODatabase>(BDBDataFile(path_root / "bdb"), /*open=*/false));
 #endif
-#ifdef USE_SQLITE
     dbs.emplace_back(MakeSQLiteDatabase(path_root / "sqlite", options, status, error));
-#endif
     dbs.emplace_back(CreateMockableWalletDatabase());
     return dbs;
 }
@@ -293,8 +289,6 @@ BOOST_AUTO_TEST_CASE(erase_prefix)
     }
 }
 
-#ifdef USE_SQLITE
-
 // Test-only statement execution error
 constexpr int TEST_SQLITE_ERROR = -999;
 
@@ -385,7 +379,6 @@ BOOST_AUTO_TEST_CASE(concurrent_txn_dont_interfere)
     BOOST_CHECK(handler2->Read(key, read_value));
     BOOST_CHECK_EQUAL(read_value, value2);
 }
-#endif // USE_SQLITE
 
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet

src/wallet/test/fuzz/CMakeLists.txt

@@ -8,8 +8,8 @@ target_sources(fuzz
     coinselection.cpp
     crypter.cpp
     fees.cpp
-    $<$<BOOL:${USE_SQLITE}>:${CMAKE_CURRENT_LIST_DIR}/notifications.cpp>
-    $<$<BOOL:${USE_SQLITE}>:${CMAKE_CURRENT_LIST_DIR}/scriptpubkeyman.cpp>
+    notifications.cpp
+    scriptpubkeyman.cpp
     spend.cpp
     wallet_bdb_parser.cpp
 )

src/wallet/test/util.h

@@ -27,9 +27,7 @@ class WalletDatabase;
 struct WalletContext;
 
 static const DatabaseFormat DATABASE_FORMATS[] = {
-#ifdef USE_SQLITE
        DatabaseFormat::SQLITE,
-#endif
 #ifdef USE_BDB
        DatabaseFormat::BERKELEY,
 #endif

src/wallet/walletdb.cpp

@@ -22,9 +22,7 @@
 #include <wallet/bdb.h>
 #endif
 #include <wallet/migrate.h>
-#ifdef USE_SQLITE
 #include <wallet/sqlite.h>
-#endif
 #include <wallet/wallet.h>
 
 #include <atomic>
@@ -1464,25 +1462,14 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas
 
     // 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
         format = DatabaseFormat::SQLITE;
-#endif
 #ifdef USE_BDB
         format = DatabaseFormat::BERKELEY;
 #endif
     }
 
     if (format == DatabaseFormat::SQLITE) {
-#ifdef USE_SQLITE
-        if constexpr (true) {
-            return MakeSQLiteDatabase(path, options, status, error);
-        } else
-#endif
-        {
-            error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support SQLite database format.", fs::PathToString(path)));
-            status = DatabaseStatus::FAILED_BAD_FORMAT;
-            return nullptr;
-        }
+        return MakeSQLiteDatabase(path, options, status, error);
     }
 
     if (format == DatabaseFormat::BERKELEY_RO) {

test/CMakeLists.txt

@@ -16,7 +16,6 @@ function(create_test_config)
   endmacro()
 
   set_configure_variable(ENABLE_WALLET ENABLE_WALLET)
-  set_configure_variable(WITH_SQLITE USE_SQLITE)
   set_configure_variable(WITH_BDB USE_BDB)
   set_configure_variable(BUILD_CLI BUILD_BITCOIN_CLI)
   set_configure_variable(BUILD_UTIL BUILD_BITCOIN_UTIL)

test/config.ini.in

@@ -16,7 +16,6 @@ RPCAUTH=@abs_top_srcdir@/share/rpcauth/rpcauth.py
 [components]
 # Which components are enabled. These are commented out by `configure` if they were disabled when running config.
 @ENABLE_WALLET_TRUE@ENABLE_WALLET=true
-@USE_SQLITE_TRUE@USE_SQLITE=true
 @USE_BDB_TRUE@USE_BDB=true
 @BUILD_BITCOIN_CLI_TRUE@ENABLE_CLI=true
 @BUILD_BITCOIN_UTIL_TRUE@ENABLE_BITCOIN_UTIL=true

test/functional/feature_filelock.py

@@ -59,8 +59,7 @@ def check_wallet_filelock(descriptors):
 
             if self.is_bdb_compiled():
                 check_wallet_filelock(False)
-            if self.is_sqlite_compiled():
-                check_wallet_filelock(True)
+            check_wallet_filelock(True)
 
 if __name__ == '__main__':
     FilelockTest(__file__).main()