Commit a73024d
committed
Merge #6594: refactor: polish CoinJoin interface, use
68cec8d refactor: use `interfaces::WalletLoader` in Dash-specific wallet init (Kittywhiskers Van Gogh)
c9c5275 refactor: get rid of `DashPostChainstateSetup()` (Kittywhiskers Van Gogh)
6502e54 refactor: use `CoinJoin::Loader` in `InitCoinJoinSettings` (Kittywhiskers Van Gogh)
0d8de73 refactor: make `WalletContext::coinjoin_loader` a pointer (Kittywhiskers Van Gogh)
0760ae0 fix: resolve undefined reference linking error in `bench_dash` (Kittywhiskers Van Gogh)
846dc67 refactor: initialize CoinJoin loader in `WalletInit::Construct()` (Kittywhiskers Van Gogh)
b14e55f chore: fix `MakeWallet` argument list in dummy wallet (Kittywhiskers Van Gogh)
b3ac5cf refactor: place `ArgsManager` before `interfaces::CoinJoin::Loader` (Kittywhiskers Van Gogh)
73671dd refactor: fold `InitCoinJoinSettings` calls into `CoinJoin::Loader` (Kittywhiskers Van Gogh)
27f8d9c refactor: defer accessing `CoinJoinWalletManager` to interface call (Kittywhiskers Van Gogh)
398e9c3 refactor: stop exposing `CoinJoinWalletManager` from `CoinJoin::Loader` (Kittywhiskers Van Gogh)
48fccdd refactor: introduce `coinjoin status` for information on mix sessions (Kittywhiskers Van Gogh)
Pull request description:
## Motivation
Since [bitcoin#22219](bitcoin#22219) (introduced in [dash#6568](#6568)), the workarounds implemented for allowing the backport of [bitcoin#19101](bitcoin#19101) (a part of [dash#6529](#6529)) no longer work. A major reason for this is the initialization order assumed in [dash#6529](#6529) is no longer holds true, requiring a rework of CoinJoin interfaces. This pull request aims to do that.
## Additional Information
* Dependency for #6529
* To allow for dropping `walletman()` from `CoinJoin::Loader`, both `CoinJoin::Loader` and `CoinJoin::Client` (accessible through `CoinJoin::Loader::GetClient()`) need to account for all potential usage.
To that end, `CoinJoin::Client::get{JsonInfo, SessionStatuses}()` have been implemented.
* Though some invocations of `CCoinJoinClientSession` functions cannot (or rather, should not) be available through the interface (like `DoAutomaticDenominating()`).
* To take care of one such invocation (in `coinjoin start`, [source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/rpc/coinjoin.cpp#L135-L136)), it has been removed altogether. `coinjoin start` will now behave like the "Start CoinJoin" button in Dash Qt and won't try to get ahead of the scheduler and start denomination.
* The loss of status information in fail cases due to this, has been made up for with a new RPC, `coinjoin status`, that will report status information during any active mix session (as opposed to earlier behavior where such information was only made available when starting a mix session through `coinjoin start` and encountering a fail state).
* `CoinJoin::Loader` and `WalletLoader` rely on each other as
* `CoinJoin::Loader::{Add,Remove}Wallet()` needs to be able to run `WalletInitInterface::InitCoinJoinSettings()`, which in turn needs to be able enumerate through all available wallets ([source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/wallet/init.cpp#L207)), currently doing so through the global wallet context but eventually through `WalletLoader` when said globals are no longer available (post-[bitcoin#19101](bitcoin#19101))
* `WalletLoader` needs `CoinJoin::Loader` to load ([source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/wallet/interfaces.cpp#601)) and restore ([source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/wallet/interfaces.cpp#L607)) wallets as `CWallet` needs access to `CoinJoin::Loader` in order to call `CoinJoin::Loader::{Add,Remove}Wallet()` ([source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/wallet/wallet.cpp#L3516), [source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/wallet/wallet.cpp#L148))
This interdependent relationship has currently been floating along by passing a const ref of the smart pointer ([source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/interfaces/init.h#L37)), which makes for cumbersome but workable code but in combination with [bitcoin#22219](bitcoin#22219), it breaks [bitcoin#19101](bitcoin#19101) as it forces us to definitively initialize both `WalletLoader` and `CoinJoin::Loader` _before_ we get to initialize CoinJoin logic.
To work around this, instead of requiring a fully initialized `CoinJoinWalletManager` to create a `CoinJoin::Loader`, we instead take `NodeContext` and defer resolving `CoinJoinWalletManager` to when interface calls are made.
* By moving the initialization of the CoinJoin loader from `AppInitMain()` to `WalletInit::Construct()`, the linker was unhappy with trying to emit `bench_dash` (see error below). This was remedied by linking `libbitcoin_wallet` with `libtest_util`.
<details>
<summary>Linker error:</summary>
```
AR libbitcoin_server.a
CXXLD dashd
CXXLD test/test_dash
CXXLD bench/bench_dash
CXXLD qt/dash-qt
CXXLD qt/test/test_dash-qt
/usr/bin/ld: libtest_util.a(libtest_util_a-setup_common.o): in function `DashPostChainstateSetup(NodeContext&)':
/src/dash/src/test/util/setup_common.cpp:127:(.text+0x1a8e): undefined reference to `interfaces::MakeCoinJoinLoader(NodeContext&)'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:7354: bench/bench_dash] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory '/src/dash/src'
make[1]: *** [Makefile:20648: all-recursive] Error 1
make[1]: Leaving directory '/src/dash/src'
make: *** [Makefile:797: all-recursive] Error 1
```
</details>
* [bitcoin#22219](bitcoin#22219) removes a `WalletInit::Construct()` call from `setup_common.cpp` ([source](029572d#diff-6a8ef76c60f30a6ca67d9f0e478fd02989c4b7fbc4c3116f80e13d873d5775e6L195-L198)), which is necessary since `WalletInit::Construct()` now relies on `node.init` ([source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/wallet/init.cpp#L191)).
This isn't so much a problem for tests that use `WalletTestingSetup` or `InitWalletDirTestingSetup` but creates problems for tests based on `TestChain100Setup` ([source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/wallet/test/coinjoin_tests.cpp#L128), [source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/wallet/test/wallet_tests.cpp#L205)), which is a specialization of `TestingSetup` that in turn, used to initialize `coinjoin_loader` ([source](https://github.com/dashpay/dash/blob/5c1a6270a73ea7025879ee0485be5095b3bc142b/src/test/util/setup_common.cpp#L127)) but since initialization has been moved to `WalletInit::Construct()` in line with `Init::makeWalletLoader()`, it is not done automatically anymore.
To work around this, `WalletInit::Construct()`-like code has been introduced to initialize both `coinjoin_loader` and `wallet_loader` ([source](https://github.com/kwvg/dash/blob/038619d284c457610ebbf833991298469c3e9a3d/src/test/util/setup_common.cpp#L320-L329)).
## Breaking Changes
* `coinjoin start` will no longer report errors from mix sessions. To see the status of mix sessions, a new RPC, `coinjoin status`, is available in its place.
## 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
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 68cec8d
PastaPastaPasta:
utACK 68cec8d
Tree-SHA512: 17ffc8f79ff19ffee9874787bf9cf982c8059c566475ed78634c3870506a24020f969c35ed6d23748fea0f993d9e646929d523c9c441fad27ea5eaf4b9438303WalletLoader in Dash wallet initialization code, introduce new RPC coinjoin status
File tree
30 files changed
+249
-150
lines changed- doc
- src
- coinjoin
- init
- interfaces
- node
- qt/test
- rpc
- test
- util
- wallet
- test
- test/functional
30 files changed
+249
-150
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
350 | 350 | | |
351 | 351 | | |
352 | 352 | | |
353 | | - | |
| 353 | + | |
354 | 354 | | |
355 | | - | |
356 | | - | |
357 | | - | |
358 | 355 | | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
359 | 360 | | |
360 | 361 | | |
361 | | - | |
| 362 | + | |
362 | 363 | | |
363 | | - | |
| 364 | + | |
364 | 365 | | |
365 | 366 | | |
366 | 367 | | |
| |||
1914 | 1915 | | |
1915 | 1916 | | |
1916 | 1917 | | |
1917 | | - | |
1918 | | - | |
1919 | | - | |
1920 | | - | |
1921 | | - | |
1922 | | - | |
1923 | | - | |
1924 | | - | |
| 1918 | + | |
| 1919 | + | |
| 1920 | + | |
| 1921 | + | |
| 1922 | + | |
1925 | 1923 | | |
1926 | 1924 | | |
1927 | 1925 | | |
| |||
1933 | 1931 | | |
1934 | 1932 | | |
1935 | 1933 | | |
1936 | | - | |
1937 | | - | |
1938 | | - | |
1939 | | - | |
1940 | | - | |
| 1934 | + | |
| 1935 | + | |
1941 | 1936 | | |
1942 | 1937 | | |
1943 | 1938 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
327 | 327 | | |
328 | 328 | | |
329 | 329 | | |
330 | | - | |
| 330 | + | |
331 | 331 | | |
332 | 332 | | |
333 | 333 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
8 | 12 | | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
9 | 16 | | |
10 | 17 | | |
11 | 18 | | |
| |||
37 | 44 | | |
38 | 45 | | |
39 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
40 | 51 | | |
41 | 52 | | |
42 | 53 | | |
43 | 54 | | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
44 | 59 | | |
45 | 60 | | |
46 | 61 | | |
| |||
61 | 76 | | |
62 | 77 | | |
63 | 78 | | |
64 | | - | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
65 | 89 | | |
66 | 90 | | |
67 | | - | |
68 | | - | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
69 | 97 | | |
70 | | - | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
71 | 103 | | |
72 | 104 | | |
73 | | - | |
| 105 | + | |
| 106 | + | |
74 | 107 | | |
75 | 108 | | |
76 | 109 | | |
77 | | - | |
| 110 | + | |
78 | 111 | | |
79 | 112 | | |
80 | 113 | | |
81 | | - | |
| 114 | + | |
82 | 115 | | |
83 | 116 | | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
| 117 | + | |
| 118 | + | |
88 | 119 | | |
89 | 120 | | |
90 | 121 | | |
91 | 122 | | |
92 | 123 | | |
93 | 124 | | |
94 | | - | |
| 125 | + | |
95 | 126 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
17 | 18 | | |
18 | 19 | | |
19 | 20 | | |
| |||
28 | 29 | | |
29 | 30 | | |
30 | 31 | | |
31 | | - | |
32 | | - | |
| 32 | + | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| |||
80 | 81 | | |
81 | 82 | | |
82 | 83 | | |
83 | | - | |
| 84 | + | |
84 | 85 | | |
85 | 86 | | |
86 | 87 | | |
87 | 88 | | |
88 | | - | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
89 | 95 | | |
90 | 96 | | |
91 | 97 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2028 | 2028 | | |
2029 | 2029 | | |
2030 | 2030 | | |
2031 | | - | |
2032 | | - | |
2033 | | - | |
2034 | | - | |
2035 | | - | |
2036 | 2031 | | |
2037 | 2032 | | |
2038 | 2033 | | |
| |||
2207 | 2202 | | |
2208 | 2203 | | |
2209 | 2204 | | |
| 2205 | + | |
| 2206 | + | |
| 2207 | + | |
| 2208 | + | |
| 2209 | + | |
2210 | 2210 | | |
2211 | 2211 | | |
2212 | 2212 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| 13 | + | |
12 | 14 | | |
13 | 15 | | |
14 | 16 | | |
| |||
29 | 31 | | |
30 | 32 | | |
31 | 33 | | |
32 | | - | |
| 34 | + | |
33 | 35 | | |
34 | | - | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
35 | 41 | | |
36 | 42 | | |
37 | 43 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
| 12 | + | |
11 | 13 | | |
12 | 14 | | |
13 | 15 | | |
| |||
24 | 26 | | |
25 | 27 | | |
26 | 28 | | |
27 | | - | |
| 29 | + | |
28 | 30 | | |
29 | | - | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
30 | 36 | | |
31 | 37 | | |
32 | 38 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | | - | |
12 | 12 | | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
13 | 16 | | |
14 | 17 | | |
15 | 18 | | |
| |||
21 | 24 | | |
22 | 25 | | |
23 | 26 | | |
| 27 | + | |
| 28 | + | |
24 | 29 | | |
25 | 30 | | |
26 | 31 | | |
| |||
38 | 43 | | |
39 | 44 | | |
40 | 45 | | |
41 | | - | |
42 | 46 | | |
43 | 47 | | |
44 | 48 | | |
45 | | - | |
| 49 | + | |
46 | 50 | | |
47 | 51 | | |
48 | 52 | | |
| |||
0 commit comments