Merge #6529: backport: merge bitcoin#21207, bitcoin#22008, bitcoin#22686

, bitcoin#22742, bitcoin#19101, bitcoin#22183, bitcoin#22009, bitcoin#22938, bitcoin#23288, bitcoin#24592 (wallet backports: part 2)

e2eea71 merge bitcoin#24592: Delete old line of code that was commented out (Kittywhiskers Van Gogh)
cf8d5a3 merge bitcoin#23288: remove usage of LegacyScriptPubKeyMan from some wallet tests (Kittywhiskers Van Gogh)
e99d065 merge bitcoin#22938: Add remaining scenarios of 0 waste, in wallet waste_test (Kittywhiskers Van Gogh)
31f8f9e merge bitcoin#22009: Decide which coin selection solution to use based on waste metric (Kittywhiskers Van Gogh)
24695e1 merge bitcoin#22183: Remove `gArgs` from `wallet.h` and `wallet.cpp` (Kittywhiskers Van Gogh)
14aa05d merge bitcoin#19101: remove ::vpwallets and related global variables (Kittywhiskers Van Gogh)
0d64290 refactor: separate out Dash-specific RPCs that rely on wallet logic (Kittywhiskers Van Gogh)
1548058 partial bitcoin#19101: remove ::vpwallets and related global variables (Kittywhiskers Van Gogh)
1fcdc96 test: remove unneeded code from some `wallet_tests` (Kittywhiskers Van Gogh)
5dc5090 refactor: move tests to match upstream order, Dash tests at the tail (Kittywhiskers Van Gogh)
2c24c63 merge bitcoin#22742: Use proper target in do_fund_send (Kittywhiskers Van Gogh)
2f3ceba merge bitcoin#22686: Use GetSelectionAmount in ApproximateBestSubset (Kittywhiskers Van Gogh)
f94993c merge bitcoin#22008: Cleanup and refactor CreateTransactionInternal (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on #6517

  * Dependent on #6594

  * [bitcoin#19101](bitcoin#19101) has been split into two due to it performing two refactors in one commit, the first half deals with utilizing interfaces and managers as defined in `WalletContext` (as opposed to either passing them in separately as arguments or accessing them through globals) and the second half deals with deglobalizing wallet variables like `::vpwallets` and `cs_wallets`.

    * `WalletContext` still needs to be initialized with a constructor as it stores a const-ref `std::unique_ptr` of `interfaces::CoinJoin::Loader`. This requirement hasn't been done away with. Tests that don't have access to `coinjoin_loader` will still need to pass `nullptr`.

    * Bitcoin registers wallet RPCs through `WalletClient::registerRpcs()` ([source](https://github.com/bitcoin/bitcoin/blob/62a09a30772141ef4add2f10d29927211abf57eb/src/wallet/interfaces.cpp#L512-L522)), which a part of the `ChainClient` interface ([source](https://github.com/bitcoin/bitcoin/blob/62a09a30772141ef4add2f10d29927211abf57eb/src/interfaces/chain.h#L292-L293)). They are enumerated ([source](https://github.com/bitcoin/bitcoin/blob/62a09a30772141ef4add2f10d29927211abf57eb/src/wallet/rpcwallet.cpp#L4702-L4776)) differently from non-wallet RPCs ([source](https://github.com/bitcoin/bitcoin/blob/62a09a30772141ef4add2f10d29927211abf57eb/src/rpc/blockchain.cpp#L2638-L2682)).

      Wallet RPCs aren't supposed to have knowledge of `NodeContext` and likewise non-wallet RPCs aren't supposed to have knowledge of `WalletContext`. So far, Bitcoin has reworked their RPCs to maintain this separation and upstream RPCs are a part of the `libbitcoin_wallet` library.

        This isn't the case for some Dash RPCs, many of which reside in `libbitcoin_server`, some of which fit into two categories.

      * Requires knowledge of both `NodeContext` and `WalletContext`
        * Knowledge of `WalletContext` wasn't mandatory before deglobalization as wallet information could be accessed through the global context courtesy of `::vpwallets` and friends but now that it is, many RPCs need to be able to access `WalletContext` when otherwise they wouldn't need to.
        * Due to the mutual exclusivity mentioned above, RPCs that access `WalletContext` _cannot_ access `NodeContext` as their RPC registration logic doesn't give them access to `NodeContext` ([source](https://github.com/bitcoin/bitcoin/blob/62a09a30772141ef4add2f10d29927211abf57eb/src/wallet/interfaces.cpp#L516-L517), `m_context` here is `WalletContext`) but in order to give those RPCs `WalletContext`, we need to register them as wallet RPCs, which prevent us from accessing `NodeContext`.
        * This has been **tentatively** worked around by including a pointer to `NodeContext` in `WalletContext` ([source](https://github.com/dashpay/dash/blob/eea8e42e631575de2968b9c1205c453bad88b135/src/wallet/context.h#L47-L52)) and modifying `EnsureAnyNodeContext()` to fetch from `WalletContext` if regular behavior doesn't yield a `NodeContext` ([source](https://github.com/dashpay/dash/blob/eea8e42e631575de2968b9c1205c453bad88b135/src/rpc/server_util.cpp#L28-L37)).

      * Are RPCs that possess both wallet and non-wallet functionality (i.e. have "reduced" capabilities if wallet support isn't compiled in as opposed to being absent altogether).
        * To enable wallet functionality, the RPCs need to be _registered_ as wallet RPCs. If wallet functionality is disabled, those RPCs are not registered. Simple enough, unless you have an RPC that can _partially_ work if wallet functionality as disabled, as the lack of registration would mean those RPCs are _entirely_ inaccessible.
        * This is also **tentatively** worked around by registering them as non-wallet RPCs in the cases where wallet RPCs aren't registered (i.e. if wallet support isn't compiled in **or** if wallet support is disabled at runtime).
          * Bitcoin doesn't use `#ifndef ENABLE_WALLET` for checking if support is absent, we're expected to use `!g_wallet_init_interface.HasWalletSupport()` (which will always evaluate `false` if support isn't compiled in, [source](https://github.com/bitcoin/bitcoin/blob/62a09a30772141ef4add2f10d29927211abf57eb/src/dummywallet.cpp#L19), as the stub wallet would be utilized to initialize the global in those cases, [source](https://github.com/bitcoin/bitcoin/blob/62a09a30772141ef4add2f10d29927211abf57eb/src/dummywallet.cpp#L57)). This has been done in the PR.
          * A notable change in this PR is that a lot of behavior that would be enabled if support was compiled in but disabled at runtime would now be disabled if support was disabled at runtime as `wallet_loader` won't be initialized if runtime disablement occurs ([source](https://github.com/bitcoin/bitcoin/blob/62a09a30772141ef4add2f10d29927211abf57eb/src/wallet/init.cpp#L128-L131)), which in turns means that anything that relies on `wallet_loader` wouldn't work either, such as `coinjoin_loader`.

             This means that we also need to register the RPC as a non-wallet RPC if support is compiled in but disabled at runtime ([source](https://github.com/dashpay/dash/blob/eea8e42e631575de2968b9c1205c453bad88b135/src/rpc/evo.cpp#L1841-L1843)).

    * To register RPCs as wallet RPCs, generally they're done through `WalletClient::registerRpcs()`, which iterates through `GetWalletRPCCommands()`. This is perfectly fine as both reside in `libbitcoin_wallet`. Unfortunately, our Dash RPCs reside in `libbitcoin_server` and `registerRpcs()` cannot be extended to enumerate through RPCs not included in its library.

      We cannot simply move the Dash RPCs either (at least in its current state) as they rely on non-wallet logic, so moving the source files for those RPCs into `libbitcoin_wallet` would pull more or less, all of `libbitcoin_server` along with it.
      * To **tentatively** get around this, a new method has been defined, `WalletLoader::registerOtherRpcs()`, which will accept any set of RPCs for registration ([source](https://github.com/dashpay/dash/blob/eea8e42e631575de2968b9c1205c453bad88b135/src/wallet/interfaces.cpp#L603-L606)) and we use it in RPC init logic ([source](https://github.com/dashpay/dash/blob/eea8e42e631575de2968b9c1205c453bad88b135/src/init.cpp#L1513-L1528)).

    * Some usage of `CoreContext` had to be removed ([source](b27d2dc#diff-157f588a09ff1da05b8c74acfcfb4da21917b6a97ed5d2e702228d621d23e66dL1024)) as without those removals, the test suite would crash.

      <details>

      <summary>Crash:</summary>

      ```
      dash@01fd4f6cfa52:/src/dash$ lldb ./src/test/test_dash
      (lldb) target create "./src/test/test_dash"
      Current executable set to '/src/dash/src/test/test_dash' (x86_64).
      (lldb) r -t wallet_tests
      Process 653232 launched: '/src/dash/src/test/test_dash' (x86_64)
      Running 17 test cases...
      unknown location(0): fatal error: in "wallet_tests/CreateTransactionTest": unknown type
      wallet/test/wallet_tests.cpp(1052): last checkpoint

      *** 1 failure is detected in the test module "Dash Core Test Suite"
      Process 653232 exited with status = 201 (0x000000c9)
      ```

      </details>

  * The assertion introduced in [bitcoin#22686](bitcoin#22686) ([source](https://github.com/achow101/bitcoin/blob/92885c4f69a5e6fc4989677d6e5be8a666fbff0d/src/wallet/spend.cpp#L781-L783)) has not been backported as this assertion is downgraded to an error in [bitcoin#26611](bitcoin#26611) and then further modified in [bitcoin#26643](bitcoin#26643) ([source](bitcoin@c1a84f1#diff-6e06b309cd494ef5da4e78aa0929a980767edd12342137f268b9219167064d13R1016-R1020)), portions of which are already included in [dash#6517](#6517) ([source](https://github.com/dashpay/dash/blob/012298acb071a45fa943653197ccc4f2c48a75a4/src/wallet/wallet.cpp#L3832-L3835)).

  * [bitcoin#23288](bitcoin#23288) finished what e3687f7 ([dash#6078](#6078)) started and `coinselection_tests` has now been realigned with upstream.
    * Dash-specific tests in `wallet_tests` (as introduced in [dash#3367](#3667)) have not been moved over to descriptor wallets and are still based on legacy wallets (to this effect, the old `AddKey` logic has been retained as `AddLegacyKey`, [source](https://github.com/dashpay/dash/blob/d4e8d1f1628933d9d9159fc261b22fc4427a1c7a/src/wallet/test/wallet_tests.cpp#L873-L879)).

      * To prevent merge conflicts, Dash-specific tests have been moved to the end of the source file and demarcated by a comment ([source](https://github.com/dashpay/dash/blob/d4e8d1f1628933d9d9159fc261b22fc4427a1c7a/src/wallet/test/wallet_tests.cpp#L868)).

  ## How Has This Been Tested?

  6813863 was tested on Debian 12 (`bookworm`) mixing ~1 tDASH on default settings.

  ![CoinJoin run](https://github.com/user-attachments/assets/bdc6a20b-a2f5-4826-96b2-a1954e579e00)

  ## Breaking Changes

  * The RPCs `coinjoin`, `coinjoinsalt`, `getpoolinfo`, `gobject list-prepared`, `gobject prepare`, `gobject vote-alias`, `gobject vote-many`, `masternode outputs`, `protx update*`, `protx register*` and `protx revoke` will no longer be available if wallet support is disabled at runtime. This is not a change in **functionality** as they were already inoperative with disabled wallets but is a change in **reporting** as they would not be available at all.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    LGTM, utACK e2eea71
  PastaPastaPasta:
    utACK e2eea71

Tree-SHA512: b3237670c2c861a353ce2486a1f0932b0bfcc6df488592cff28b85a49ed96c4612fc88866d0808e6d2cf0d409e4b8e8120e605acd7367b4bf554009672d3e9ea

src/bench/coin_selection.cpp

@@ -32,7 +32,6 @@ static void CoinSelection(benchmark::Bench& bench)
     NodeContext node;
     auto chain = interfaces::MakeChain(node);
     CWallet wallet(chain.get(), /*coinjoin_loader=*/ nullptr, "", CreateDummyWalletDatabase());
-    wallet.SetupLegacyScriptPubKeyMan();
     std::vector<std::unique_ptr<CWalletTx>> wtxs;
     LOCK(wallet.cs_wallet);
 
@@ -55,7 +54,7 @@ static void CoinSelection(benchmark::Bench& bench)
     bench.run([&] {
         std::set<CInputCoin> setCoinsRet;
         CAmount nValueRet;
-        bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params);
+        bool success = wallet.AttemptSelection(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params);
         assert(success);
         assert(nValueRet == 1003 * COIN);
         assert(setCoinsRet.size() == 2);

src/bench/wallet_balance.cpp

@@ -13,20 +13,21 @@
 
 #include <optional>
 
-static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_watchonly, const bool add_mine, const uint32_t epoch_iters)
+static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_mine, const uint32_t epoch_iters)
 {
     const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
     const auto& ADDRESS_WATCHONLY = ADDRESS_B58T_UNSPENDABLE;
 
     CWallet wallet{test_setup->m_node.chain.get(), test_setup->m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()};
     {
-        wallet.SetupLegacyScriptPubKeyMan();
+        LOCK(wallet.cs_wallet);
+        wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+        wallet.SetupDescriptorScriptPubKeyMans();
         if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false);
     }
     auto handler = test_setup->m_node.chain->handleNotifications({&wallet, [](CWallet*) {}});
 
     const std::optional<std::string> address_mine{add_mine ? std::optional<std::string>{getnewaddress(wallet)} : std::nullopt};
-    if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY);
 
     for (int i = 0; i < 100; ++i) {
         generatetoaddress(test_setup->m_node, address_mine.value_or(ADDRESS_WATCHONLY));
@@ -40,14 +41,13 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b
         if (set_dirty) wallet.MarkDirty();
         bal = wallet.GetBalance();
         if (add_mine) assert(bal.m_mine_trusted > 0);
-        if (add_watchonly) assert(bal.m_watchonly_trusted > 0);
     });
 }
 
-static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ true, /* add_watchonly */ true, /* add_mine */ true, 2500); }
-static void WalletBalanceClean(benchmark::Bench& bench) {WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ true, /* add_mine */ true, 8000); }
-static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ false, /* add_mine */ true, 16000); }
-static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ true, /* add_mine */ false, 8000); }
+static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ true, /* add_mine */ true, 2500); }
+static void WalletBalanceClean(benchmark::Bench& bench) {WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true, 8000); }
+static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true, 16000); }
+static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ false, 8000); }
 
 BENCHMARK(WalletBalanceDirty);
 BENCHMARK(WalletBalanceClean);

src/bitcoin-cli.cpp

@@ -950,7 +950,7 @@ static void GetWalletBalances(UniValue& result)
 }
 
 /**
- * GetProgressBar contructs a progress bar with 5% intervals.
+ * GetProgressBar constructs a progress bar with 5% intervals.
  *
  * @param[in]   progress      The proportion of the progress bar to be filled between 0 and 1.
  * @param[out]  progress_bar  String representation of the progress bar.

src/dummywallet.cpp

@@ -38,6 +38,7 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const
 {
     argsman.AddHiddenArgs({
         "-avoidpartialspends",
+        "-consolidatefeerate=<amt>",
         "-createwalletbackups=<n>",
         "-disablewallet",
         "-instantsendnotify=<cmd>",
@@ -91,7 +92,8 @@ std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet)
     throw std::logic_error("Wallet function called in non-wallet build.");
 }
 
-std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, ArgsManager& args, interfaces::CoinJoin::Loader& coinjoin_loader)
+std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, ArgsManager& args, NodeContext& node_context,
+                                               interfaces::CoinJoin::Loader& coinjoin_loader)
 {
     throw std::logic_error("Wallet function called in non-wallet build.");
 }

src/init.cpp

@@ -31,6 +31,7 @@
 #include <index/txindex.h>
 #include <interfaces/init.h>
 #include <interfaces/node.h>
+#include <interfaces/wallet.h>
 #include <mapport.h>
 #include <node/miner.h>
 #include <net.h>
@@ -1516,6 +1517,23 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     for (const auto& client : node.chain_clients) {
         client->registerRpcs();
     }
+#ifdef ENABLE_WALLET
+    // Register non-core wallet-only RPC commands. These are commands that
+    // aren't a part of the wallet library but heavily rely on wallet logic.
+    // TODO: Move them to chain client interfaces so they can be called
+    //       with registerRpcs()
+    if (!args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) {
+        for (const auto& commands : {
+            GetWalletCoinJoinRPCCommands(),
+            GetWalletEvoRPCCommands(),
+            GetWalletGovernanceRPCCommands(),
+            GetWalletMasternodeRPCCommands(),
+        }) {
+            node.wallet_loader->registerOtherRpcs(commands);
+        }
+    }
+#endif // ENABLE_WALLET
+
 #if ENABLE_ZMQ
     RegisterZMQRPCCommands(tableRPC);
 #endif

src/init/bitcoin-node.cpp

@@ -37,7 +37,7 @@ class BitcoinNodeInit : public interfaces::Init
     }
     std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
     {
-        return MakeWalletLoader(chain, *Assert(m_node.args), coinjoin_loader);
+        return MakeWalletLoader(chain, *Assert(m_node.args), m_node, coinjoin_loader);
     }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     interfaces::Ipc* ipc() override { return m_ipc.get(); }

src/init/bitcoind.cpp

@@ -32,7 +32,7 @@ class BitcoindInit : public interfaces::Init
     }
     std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
     {
-        return MakeWalletLoader(chain, *Assert(m_node.args), coinjoin_loader);
+        return MakeWalletLoader(chain, *Assert(m_node.args), m_node, coinjoin_loader);
     }
     std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
     NodeContext& m_node;

src/interfaces/wallet.h

@@ -28,18 +28,22 @@
 class CCoinControl;
 class CFeeRate;
 class CKey;
+class CRPCCommand;
 class CWallet;
 class UniValue;
 enum class FeeReason;
 enum class TransactionError;
 enum isminetype : unsigned int;
-struct bilingual_str;
 struct CRecipient;
+struct NodeContext;
 struct PartiallySignedTransaction;
 struct WalletContext;
 struct bilingual_str;
 using isminefilter = std::underlying_type<isminetype>::type;
 
+template <typename T>
+class Span;
+
 namespace interfaces {
 
 class Handler;
@@ -335,8 +339,11 @@ class Wallet
 class WalletLoader : public ChainClient
 {
 public:
-    //! Create new wallet.
-    virtual std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
+   //! Register non-core wallet RPCs
+   virtual void registerOtherRpcs(const Span<const CRPCCommand>& commands) = 0;
+
+   //! Create new wallet.
+   virtual std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
 
    //! Load existing wallet.
    virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
@@ -358,6 +365,9 @@ class WalletLoader : public ChainClient
    //! loaded at startup or by RPC.
    using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>;
    virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0;
+
+   //! Return pointer to internal context, useful for testing.
+   virtual WalletContext* context() { return nullptr; }
 };
 
 //! Information about one wallet address.
@@ -443,11 +453,12 @@ struct WalletTxOut
 
 //! Return implementation of Wallet interface. This function is defined in
 //! dummywallet.cpp and throws if the wallet component is not compiled.
-std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet);
+std::unique_ptr<Wallet> MakeWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet);
 
 //! Return implementation of ChainClient interface for a wallet loader. This
 //! function will be undefined in builds where ENABLE_WALLET is false.
-std::unique_ptr<WalletLoader> MakeWalletLoader(Chain& chain, ArgsManager& args, CoinJoin::Loader& coinjoin_loader);
+std::unique_ptr<WalletLoader> MakeWalletLoader(Chain& chain, ArgsManager& args, NodeContext& node_context,
+                                               CoinJoin::Loader& coinjoin_loader);
 
 } // namespace interfaces
 

src/qt/test/addressbooktests.cpp

@@ -65,8 +65,12 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
     TestChain100Setup test;
     node.setContext(&test.m_node);
     const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase());
-    wallet->SetupLegacyScriptPubKeyMan();
     wallet->LoadWallet();
+    wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
+    {
+        LOCK(wallet->cs_wallet);
+        wallet->SetupDescriptorScriptPubKeyMans();
+    }
 
     auto build_address = [wallet]() {
         CKey key;
@@ -110,9 +114,10 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
     // Initialize relevant QT models.
     OptionsModel optionsModel;
     ClientModel clientModel(node, &optionsModel);
-    AddWallet(wallet);
-    WalletModel walletModel(interfaces::MakeWallet(wallet), clientModel);
-    RemoveWallet(wallet, std::nullopt);
+    WalletContext& context = *node.walletLoader().context();
+    AddWallet(context, wallet);
+    WalletModel walletModel(interfaces::MakeWallet(context, wallet), clientModel);
+    RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt);
     EditAddressDialog editAddressDialog(EditAddressDialog::NewSendingAddress);
     editAddressDialog.setModel(walletModel.getAddressTableModel());
 

src/qt/test/wallettests.cpp

@@ -109,14 +109,24 @@ void TestGUI(interfaces::Node& node)
         test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
     }
     node.setContext(&test.m_node);
+    WalletContext& context = *node.walletLoader().context();
     const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase());
-    AddWallet(wallet);
+    AddWallet(context, wallet);
     wallet->LoadWallet();
+    wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
     {
-        auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan();
-        LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore);
-        wallet->SetAddressBook(PKHash(test.coinbaseKey.GetPubKey()), "", "receive");
-        spk_man->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey());
+        LOCK(wallet->cs_wallet);
+        wallet->SetupDescriptorScriptPubKeyMans();
+
+        // Add the coinbase key
+        FlatSigningProvider provider;
+        std::string error;
+        std::unique_ptr<Descriptor> desc = Parse("combo(" + EncodeSecret(test.coinbaseKey) + ")", provider, error, /* require_checksum=*/ false);
+        assert(desc);
+        WalletDescriptor w_desc(std::move(desc), 0, 0, 1, 1);
+        if (!wallet->AddWalletDescriptor(w_desc, provider, "", false)) assert(false);
+        CTxDestination dest = PKHash(test.coinbaseKey.GetPubKey());
+        wallet->SetAddressBook(dest, "", "receive");
         wallet->SetLastBlockProcessed(105, node.context()->chainman->ActiveChain().Tip()->GetBlockHash());
     }
     {
@@ -134,7 +144,7 @@ void TestGUI(interfaces::Node& node)
     TransactionView transactionView;
     OptionsModel optionsModel;
     ClientModel clientModel(node, &optionsModel);
-    WalletModel walletModel(interfaces::MakeWallet(wallet), clientModel);
+    WalletModel walletModel(interfaces::MakeWallet(context, wallet), clientModel);
     sendCoinsDialog.setModel(&walletModel);
     transactionView.setModel(&walletModel);
 
@@ -246,7 +256,7 @@ void TestGUI(interfaces::Node& node)
     QPushButton* removeRequestButton = receiveCoinsDialog.findChild<QPushButton*>("removeRequestButton");
     removeRequestButton->click();
     QCOMPARE(requestTableModel->rowCount({}), currentRowCount-1);
-    RemoveWallet(wallet, std::nullopt);
+    RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt);
 
     // Check removal from wallet
     QCOMPARE(walletModel.wallet().getAddressReceiveRequests().size(), size_t{0});

src/rpc/coinjoin.cpp

@@ -2,17 +2,18 @@
 // Distributed under the MIT/X11 software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <node/context.h>
-#include <validation.h>
 #include <coinjoin/context.h>
 #include <coinjoin/server.h>
+#include <node/context.h>
 #include <rpc/blockchain.h>
 #include <rpc/server.h>
 #include <rpc/server_util.h>
-#include <util/check.h>
 #include <rpc/util.h>
+#include <util/check.h>
 #include <util/strencodings.h>
+#include <validation.h>
 #include <wallet/rpc/util.h>
+#include <walletinitinterface.h>
 
 #ifdef ENABLE_WALLET
 #include <coinjoin/client.h>
@@ -485,14 +486,13 @@ static RPCHelpMan getcoinjoininfo()
     };
 }
 
-void RegisterCoinJoinRPCCommands(CRPCTable &t)
+#ifdef ENABLE_WALLET
+Span<const CRPCCommand> GetWalletCoinJoinRPCCommands()
 {
 // clang-format off
 static const CRPCCommand commands[] =
-{ //  category               actor (function)
-  //  ---------------------  -----------------------
-    { "dash",                &getcoinjoininfo,        },
-#ifdef ENABLE_WALLET
+{ //  category              actor (function)
+  //  --------------------- -----------------------
     { "dash",                &coinjoin,               },
     { "dash",                &coinjoin_reset,         },
     { "dash",                &coinjoin_start,         },
@@ -502,10 +502,35 @@ static const CRPCCommand commands[] =
     { "dash",                &coinjoinsalt_generate,  },
     { "dash",                &coinjoinsalt_get,       },
     { "dash",                &coinjoinsalt_set,       },
+    { "dash",                &getcoinjoininfo,        },
+};
+// clang-format on
+    return commands;
+}
 #endif // ENABLE_WALLET
+
+void RegisterCoinJoinRPCCommands(CRPCTable& t)
+{
+// clang-format off
+static const CRPCCommand commands_wallet[] =
+{ //  category               actor (function)
+  //  ---------------------  -----------------------
+    { "dash",                &getcoinjoininfo,        },
 };
 // clang-format on
-    for (const auto& command : commands) {
-        t.appendCommand(command.name, &command);
+    // If we aren't compiling with wallet support, we still need to register RPCs that are
+    // capable of working without wallet support. We have to do this even if wallet support
+    // is compiled in but is disabled at runtime because runtime disablement prohibits
+    // registering wallet RPCs. We still want the reduced functionality RPC to be registered.
+    // TODO: Spin off these hybrid RPCs into dedicated wallet-only and/or wallet-free RPCs
+    //       and get rid of this workaround.
+    if (!g_wallet_init_interface.HasWalletSupport()
+#ifdef ENABLE_WALLET
+        || gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)
+#endif // ENABLE_WALLET
+    ) {
+        for (const auto& command : commands_wallet) {
+            tableRPC.appendCommand(command.name, &command);
+        }
     }
 }