Skip to content

Commit d5e3d46

Browse files
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
2 parents 379c935 + e2eea71 commit d5e3d46

40 files changed

+1556
-1079
lines changed

src/bench/coin_selection.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ static void CoinSelection(benchmark::Bench& bench)
3232
NodeContext node;
3333
auto chain = interfaces::MakeChain(node);
3434
CWallet wallet(chain.get(), /*coinjoin_loader=*/ nullptr, "", CreateDummyWalletDatabase());
35-
wallet.SetupLegacyScriptPubKeyMan();
3635
std::vector<std::unique_ptr<CWalletTx>> wtxs;
3736
LOCK(wallet.cs_wallet);
3837

@@ -55,7 +54,7 @@ static void CoinSelection(benchmark::Bench& bench)
5554
bench.run([&] {
5655
std::set<CInputCoin> setCoinsRet;
5756
CAmount nValueRet;
58-
bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params);
57+
bool success = wallet.AttemptSelection(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params);
5958
assert(success);
6059
assert(nValueRet == 1003 * COIN);
6160
assert(setCoinsRet.size() == 2);

src/bench/wallet_balance.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,21 @@
1313

1414
#include <optional>
1515

16-
static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_watchonly, const bool add_mine, const uint32_t epoch_iters)
16+
static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_mine, const uint32_t epoch_iters)
1717
{
1818
const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
1919
const auto& ADDRESS_WATCHONLY = ADDRESS_B58T_UNSPENDABLE;
2020

2121
CWallet wallet{test_setup->m_node.chain.get(), test_setup->m_node.coinjoin_loader.get(), "", CreateMockWalletDatabase()};
2222
{
23-
wallet.SetupLegacyScriptPubKeyMan();
23+
LOCK(wallet.cs_wallet);
24+
wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
25+
wallet.SetupDescriptorScriptPubKeyMans();
2426
if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false);
2527
}
2628
auto handler = test_setup->m_node.chain->handleNotifications({&wallet, [](CWallet*) {}});
2729

2830
const std::optional<std::string> address_mine{add_mine ? std::optional<std::string>{getnewaddress(wallet)} : std::nullopt};
29-
if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY);
3031

3132
for (int i = 0; i < 100; ++i) {
3233
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
4041
if (set_dirty) wallet.MarkDirty();
4142
bal = wallet.GetBalance();
4243
if (add_mine) assert(bal.m_mine_trusted > 0);
43-
if (add_watchonly) assert(bal.m_watchonly_trusted > 0);
4444
});
4545
}
4646

47-
static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ true, /* add_watchonly */ true, /* add_mine */ true, 2500); }
48-
static void WalletBalanceClean(benchmark::Bench& bench) {WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ true, /* add_mine */ true, 8000); }
49-
static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ false, /* add_mine */ true, 16000); }
50-
static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_watchonly */ true, /* add_mine */ false, 8000); }
47+
static void WalletBalanceDirty(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ true, /* add_mine */ true, 2500); }
48+
static void WalletBalanceClean(benchmark::Bench& bench) {WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true, 8000); }
49+
static void WalletBalanceMine(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ true, 16000); }
50+
static void WalletBalanceWatch(benchmark::Bench& bench) { WalletBalance(bench, /* set_dirty */ false, /* add_mine */ false, 8000); }
5151

5252
BENCHMARK(WalletBalanceDirty);
5353
BENCHMARK(WalletBalanceClean);

src/bitcoin-cli.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,7 @@ static void GetWalletBalances(UniValue& result)
950950
}
951951

952952
/**
953-
* GetProgressBar contructs a progress bar with 5% intervals.
953+
* GetProgressBar constructs a progress bar with 5% intervals.
954954
*
955955
* @param[in] progress The proportion of the progress bar to be filled between 0 and 1.
956956
* @param[out] progress_bar String representation of the progress bar.

src/dummywallet.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const
3838
{
3939
argsman.AddHiddenArgs({
4040
"-avoidpartialspends",
41+
"-consolidatefeerate=<amt>",
4142
"-createwalletbackups=<n>",
4243
"-disablewallet",
4344
"-instantsendnotify=<cmd>",
@@ -91,7 +92,8 @@ std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet)
9192
throw std::logic_error("Wallet function called in non-wallet build.");
9293
}
9394

94-
std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, ArgsManager& args, interfaces::CoinJoin::Loader& coinjoin_loader)
95+
std::unique_ptr<WalletClient> MakeWalletLoader(Chain& chain, ArgsManager& args, NodeContext& node_context,
96+
interfaces::CoinJoin::Loader& coinjoin_loader)
9597
{
9698
throw std::logic_error("Wallet function called in non-wallet build.");
9799
}

src/init.cpp

+18
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include <index/txindex.h>
3232
#include <interfaces/init.h>
3333
#include <interfaces/node.h>
34+
#include <interfaces/wallet.h>
3435
#include <mapport.h>
3536
#include <node/miner.h>
3637
#include <net.h>
@@ -1516,6 +1517,23 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
15161517
for (const auto& client : node.chain_clients) {
15171518
client->registerRpcs();
15181519
}
1520+
#ifdef ENABLE_WALLET
1521+
// Register non-core wallet-only RPC commands. These are commands that
1522+
// aren't a part of the wallet library but heavily rely on wallet logic.
1523+
// TODO: Move them to chain client interfaces so they can be called
1524+
// with registerRpcs()
1525+
if (!args.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) {
1526+
for (const auto& commands : {
1527+
GetWalletCoinJoinRPCCommands(),
1528+
GetWalletEvoRPCCommands(),
1529+
GetWalletGovernanceRPCCommands(),
1530+
GetWalletMasternodeRPCCommands(),
1531+
}) {
1532+
node.wallet_loader->registerOtherRpcs(commands);
1533+
}
1534+
}
1535+
#endif // ENABLE_WALLET
1536+
15191537
#if ENABLE_ZMQ
15201538
RegisterZMQRPCCommands(tableRPC);
15211539
#endif

src/init/bitcoin-node.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class BitcoinNodeInit : public interfaces::Init
3737
}
3838
std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
3939
{
40-
return MakeWalletLoader(chain, *Assert(m_node.args), coinjoin_loader);
40+
return MakeWalletLoader(chain, *Assert(m_node.args), m_node, coinjoin_loader);
4141
}
4242
std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
4343
interfaces::Ipc* ipc() override { return m_ipc.get(); }

src/init/bitcoind.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class BitcoindInit : public interfaces::Init
3232
}
3333
std::unique_ptr<interfaces::WalletLoader> makeWalletLoader(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader) override
3434
{
35-
return MakeWalletLoader(chain, *Assert(m_node.args), coinjoin_loader);
35+
return MakeWalletLoader(chain, *Assert(m_node.args), m_node, coinjoin_loader);
3636
}
3737
std::unique_ptr<interfaces::Echo> makeEcho() override { return interfaces::MakeEcho(); }
3838
NodeContext& m_node;

src/interfaces/wallet.h

+16-5
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,22 @@
2828
class CCoinControl;
2929
class CFeeRate;
3030
class CKey;
31+
class CRPCCommand;
3132
class CWallet;
3233
class UniValue;
3334
enum class FeeReason;
3435
enum class TransactionError;
3536
enum isminetype : unsigned int;
36-
struct bilingual_str;
3737
struct CRecipient;
38+
struct NodeContext;
3839
struct PartiallySignedTransaction;
3940
struct WalletContext;
4041
struct bilingual_str;
4142
using isminefilter = std::underlying_type<isminetype>::type;
4243

44+
template <typename T>
45+
class Span;
46+
4347
namespace interfaces {
4448

4549
class Handler;
@@ -335,8 +339,11 @@ class Wallet
335339
class WalletLoader : public ChainClient
336340
{
337341
public:
338-
//! Create new wallet.
339-
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;
342+
//! Register non-core wallet RPCs
343+
virtual void registerOtherRpcs(const Span<const CRPCCommand>& commands) = 0;
344+
345+
//! Create new wallet.
346+
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;
340347

341348
//! Load existing wallet.
342349
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
358365
//! loaded at startup or by RPC.
359366
using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>;
360367
virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0;
368+
369+
//! Return pointer to internal context, useful for testing.
370+
virtual WalletContext* context() { return nullptr; }
361371
};
362372

363373
//! Information about one wallet address.
@@ -443,11 +453,12 @@ struct WalletTxOut
443453

444454
//! Return implementation of Wallet interface. This function is defined in
445455
//! dummywallet.cpp and throws if the wallet component is not compiled.
446-
std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet);
456+
std::unique_ptr<Wallet> MakeWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet);
447457

448458
//! Return implementation of ChainClient interface for a wallet loader. This
449459
//! function will be undefined in builds where ENABLE_WALLET is false.
450-
std::unique_ptr<WalletLoader> MakeWalletLoader(Chain& chain, ArgsManager& args, CoinJoin::Loader& coinjoin_loader);
460+
std::unique_ptr<WalletLoader> MakeWalletLoader(Chain& chain, ArgsManager& args, NodeContext& node_context,
461+
CoinJoin::Loader& coinjoin_loader);
451462

452463
} // namespace interfaces
453464

src/qt/test/addressbooktests.cpp

+9-4
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,12 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
6565
TestChain100Setup test;
6666
node.setContext(&test.m_node);
6767
const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase());
68-
wallet->SetupLegacyScriptPubKeyMan();
6968
wallet->LoadWallet();
69+
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
70+
{
71+
LOCK(wallet->cs_wallet);
72+
wallet->SetupDescriptorScriptPubKeyMans();
73+
}
7074

7175
auto build_address = [wallet]() {
7276
CKey key;
@@ -110,9 +114,10 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
110114
// Initialize relevant QT models.
111115
OptionsModel optionsModel;
112116
ClientModel clientModel(node, &optionsModel);
113-
AddWallet(wallet);
114-
WalletModel walletModel(interfaces::MakeWallet(wallet), clientModel);
115-
RemoveWallet(wallet, std::nullopt);
117+
WalletContext& context = *node.walletLoader().context();
118+
AddWallet(context, wallet);
119+
WalletModel walletModel(interfaces::MakeWallet(context, wallet), clientModel);
120+
RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt);
116121
EditAddressDialog editAddressDialog(EditAddressDialog::NewSendingAddress);
117122
editAddressDialog.setModel(walletModel.getAddressTableModel());
118123

src/qt/test/wallettests.cpp

+17-7
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,24 @@ void TestGUI(interfaces::Node& node)
109109
test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
110110
}
111111
node.setContext(&test.m_node);
112+
WalletContext& context = *node.walletLoader().context();
112113
const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), node.context()->coinjoin_loader.get(), "", CreateMockWalletDatabase());
113-
AddWallet(wallet);
114+
AddWallet(context, wallet);
114115
wallet->LoadWallet();
116+
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
115117
{
116-
auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan();
117-
LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore);
118-
wallet->SetAddressBook(PKHash(test.coinbaseKey.GetPubKey()), "", "receive");
119-
spk_man->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey());
118+
LOCK(wallet->cs_wallet);
119+
wallet->SetupDescriptorScriptPubKeyMans();
120+
121+
// Add the coinbase key
122+
FlatSigningProvider provider;
123+
std::string error;
124+
std::unique_ptr<Descriptor> desc = Parse("combo(" + EncodeSecret(test.coinbaseKey) + ")", provider, error, /* require_checksum=*/ false);
125+
assert(desc);
126+
WalletDescriptor w_desc(std::move(desc), 0, 0, 1, 1);
127+
if (!wallet->AddWalletDescriptor(w_desc, provider, "", false)) assert(false);
128+
CTxDestination dest = PKHash(test.coinbaseKey.GetPubKey());
129+
wallet->SetAddressBook(dest, "", "receive");
120130
wallet->SetLastBlockProcessed(105, node.context()->chainman->ActiveChain().Tip()->GetBlockHash());
121131
}
122132
{
@@ -134,7 +144,7 @@ void TestGUI(interfaces::Node& node)
134144
TransactionView transactionView;
135145
OptionsModel optionsModel;
136146
ClientModel clientModel(node, &optionsModel);
137-
WalletModel walletModel(interfaces::MakeWallet(wallet), clientModel);
147+
WalletModel walletModel(interfaces::MakeWallet(context, wallet), clientModel);
138148
sendCoinsDialog.setModel(&walletModel);
139149
transactionView.setModel(&walletModel);
140150

@@ -246,7 +256,7 @@ void TestGUI(interfaces::Node& node)
246256
QPushButton* removeRequestButton = receiveCoinsDialog.findChild<QPushButton*>("removeRequestButton");
247257
removeRequestButton->click();
248258
QCOMPARE(requestTableModel->rowCount({}), currentRowCount-1);
249-
RemoveWallet(wallet, std::nullopt);
259+
RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt);
250260

251261
// Check removal from wallet
252262
QCOMPARE(walletModel.wallet().getAddressReceiveRequests().size(), size_t{0});

src/rpc/coinjoin.cpp

+35-10
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,18 @@
22
// Distributed under the MIT/X11 software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

5-
#include <node/context.h>
6-
#include <validation.h>
75
#include <coinjoin/context.h>
86
#include <coinjoin/server.h>
7+
#include <node/context.h>
98
#include <rpc/blockchain.h>
109
#include <rpc/server.h>
1110
#include <rpc/server_util.h>
12-
#include <util/check.h>
1311
#include <rpc/util.h>
12+
#include <util/check.h>
1413
#include <util/strencodings.h>
14+
#include <validation.h>
1515
#include <wallet/rpc/util.h>
16+
#include <walletinitinterface.h>
1617

1718
#ifdef ENABLE_WALLET
1819
#include <coinjoin/client.h>
@@ -485,14 +486,13 @@ static RPCHelpMan getcoinjoininfo()
485486
};
486487
}
487488

488-
void RegisterCoinJoinRPCCommands(CRPCTable &t)
489+
#ifdef ENABLE_WALLET
490+
Span<const CRPCCommand> GetWalletCoinJoinRPCCommands()
489491
{
490492
// clang-format off
491493
static const CRPCCommand commands[] =
492-
{ // category actor (function)
493-
// --------------------- -----------------------
494-
{ "dash", &getcoinjoininfo, },
495-
#ifdef ENABLE_WALLET
494+
{ // category actor (function)
495+
// --------------------- -----------------------
496496
{ "dash", &coinjoin, },
497497
{ "dash", &coinjoin_reset, },
498498
{ "dash", &coinjoin_start, },
@@ -502,10 +502,35 @@ static const CRPCCommand commands[] =
502502
{ "dash", &coinjoinsalt_generate, },
503503
{ "dash", &coinjoinsalt_get, },
504504
{ "dash", &coinjoinsalt_set, },
505+
{ "dash", &getcoinjoininfo, },
506+
};
507+
// clang-format on
508+
return commands;
509+
}
505510
#endif // ENABLE_WALLET
511+
512+
void RegisterCoinJoinRPCCommands(CRPCTable& t)
513+
{
514+
// clang-format off
515+
static const CRPCCommand commands_wallet[] =
516+
{ // category actor (function)
517+
// --------------------- -----------------------
518+
{ "dash", &getcoinjoininfo, },
506519
};
507520
// clang-format on
508-
for (const auto& command : commands) {
509-
t.appendCommand(command.name, &command);
521+
// If we aren't compiling with wallet support, we still need to register RPCs that are
522+
// capable of working without wallet support. We have to do this even if wallet support
523+
// is compiled in but is disabled at runtime because runtime disablement prohibits
524+
// registering wallet RPCs. We still want the reduced functionality RPC to be registered.
525+
// TODO: Spin off these hybrid RPCs into dedicated wallet-only and/or wallet-free RPCs
526+
// and get rid of this workaround.
527+
if (!g_wallet_init_interface.HasWalletSupport()
528+
#ifdef ENABLE_WALLET
529+
|| gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)
530+
#endif // ENABLE_WALLET
531+
) {
532+
for (const auto& command : commands_wallet) {
533+
tableRPC.appendCommand(command.name, &command);
534+
}
510535
}
511536
}

0 commit comments

Comments
 (0)