55696a0ac30bcfbd555f71cbc8eac23b725f7dcf wallet: remove `mempool_sequence` from `transactionRemovedFromMempool` (w0xlt)
bf19069c53501231a2f3ba59afa067913ec4d3b2 wallet: remove `mempool_sequence` from `transactionAddedToMempool` (w0xlt)
Pull request description:
This PR removes `mempool_sequence` from `transactionRemovedFromMempool` and `transactionAddedToMempool`.
`mempool_sequence` is not used in these methods, only in ZMQ notifications.
ACKs for top commit:
instagibbs:
ACK 55696a0ac3
Tree-SHA512: 621e89230bcb6edfed83e2758601a2b093822fc2dc4e9bfb00487e340f2bc4c5ac3bf6df3ca00b7fe55bb3df15858820f2bf698f403d2e48b915dd9eb47b63e0
b942c94d153f83b77ef5d603211252d9abadde95 test: Change coinselection parameter location to make tests independent (yancy)
Pull request description:
the `subtract_fee_outputs` param is expected to be `true` for all subsequent tests. It should be defined outside of a single test so that if it's removed or changed, all subsequent tests won't fail. Currently if you remove this [test](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L304:L325) the following [test](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L327:L345) fails. This change makes the tests independent.
ACKs for top commit:
achow101:
ACK b942c94d153f83b77ef5d603211252d9abadde95
aureleoules:
ACK b942c94d153f83b77ef5d603211252d9abadde95.
rajarshimaitra:
tACK b942c94d153f83b77ef5d603211252d9abadde95
theStack:
ACK b942c94d153f83b77ef5d603211252d9abadde95
Tree-SHA512: 461e19d15351318102ef9f96c68442365d8ca238c48ad7aefe23e8532b33b91dadf6c7840c7894574bccede6da162a55ad7a6f6a330d61a11ce804e68ddc5e9c
3a4f8bc24271d05765e9bf1e26558a28ab2e6b81 bench: add benchmark for wallet 'AvailableCoins' function. (furszy)
Pull request description:
#### Rationale
`AvailableCoins` is part of several important flows for the wallet; from RPC commands that create transactions like `fundrawtransaction`, `send`, `walletcreatefundedpsbt`, get the available balance, list the available coins with `listunspent` etc. to GUI connected processes that perform the same or similar actions: tx creation, available balance calculation, present the spendable coins in the coin control dialog.
As we are improving this process in #24699, #25005 and there are more structural changes coming on the way. This benchmark aims to ensure us that, at least, there are no regressions (obviously performance improvements are great but, at least for me, this heads into the direction of having a base metric to compare future structural changes).
#### Implementation Notes
There are 5 new benchmarks, one per wallet supported output type (LEGACY, P2SH_SEGWIT, BECH32, BECH32M), plus a multi-output-type wallet benchmark which contains outputs from all the descriptor types.
The test, by default, fills-up the wallet with 1k transactions, 2k outputs. Mainly to not consume much time if the user just want to verify that no substantial regressions were introduced. But, my expectation for those who are focused on this process is to use a much higher number locally to really note the differences across commits.
ACKs for top commit:
achow101:
ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81
hernanmarino:
ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81
aureleoules:
ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81
Tree-SHA512: d0bb4c165f1efa181b47cb31561e6217eff9135bcd1b6761a7292f9018e456d13d18a1b886c2e2268d35c52f9e1fd8e0f252972424e5c5f00c280620b79c5a1b
fa5cbf2290dab08296cabbd973e9adefc883f3fc ci: Properly set COMMIT_RANGE in lint task (MarcoFalke)
Pull request description:
Currently the variable holds (apart from the commits in the pull request) all commits to master since the pull was opened.
This is problematic, because already merged commits are linted in unrelated pulls, leading to:
* Wasted resources. For example, currently the lint task may take 9 minutes, when it should take 1. See https://cirrus-ci.com/task/6032782770569216?logs=lint#L1449
* False failures. For example, when a "wrong" commit is in master it can lead to some pulls failing unrelatedly, and others not.
Now that the CI has the `/merge` commit (since commit fad7281d7842f337932cf44e703fdd631230ddd6), `COMMIT_RANGE` can simply be set to `HEAD~..HEAD` to only hold the changes in the pull.
ACKs for top commit:
fanquake:
ACK fa5cbf2290dab08296cabbd973e9adefc883f3fc
Tree-SHA512: e85fca4ca9d2615ddd2544403485e06885769a3f70bca297e23eefda2a1d28f47c5271f6adfa6ce0e5e972335c78098b76e0db4b109f59d0986bf508cef7528f
f6eadaa413f20c1853cf1bea5141560cd5e4b15a Use same Python executable for subprocesses as for all-lint.py (Kristaps Kaupe)
Pull request description:
Before this all linters were ran by `/usr/bin/env python3`, no matter what was used to run `test/lint/all-lint.py`. This change allows to use non-default Python executable for `test/lint/all-lint.py` and then all subprocesses will also use same Python interpreter (for example, `python3.10 ./test/lint/all-lint.py`). See https://github.com/bitcoin/bitcoin/issues/26792#issuecomment-1369558866 as use case.
ACKs for top commit:
fanquake:
ACK f6eadaa413f20c1853cf1bea5141560cd5e4b15a - did not test
Tree-SHA512: 4da3b5581a0dd8ab9a6387829495019091a93a7ceaf2135d65d40a1983fd11a0b92b20891ef30d2a132abb0a690cd9b2f7eb5fcc38df06a340394ef449d640af
22e9afe40d987f4f90bc8469f9475df138fe6261 use sha256 command instead of sha256sum on FreeBSD (Murray Nesbitt)
Pull request description:
The FreeBSD version of `sha256sum` takes different arguments than the GNU version.
The `sha256_check` function in `contrib/install_db4.sh` has code specific to FreeBSD, however it doesn't get reached because while the `sha256sum` command does exist on FreeBSD, it is incompatible and results in an error:
```
sha256sum: option requires an argument -- c
usage: sha256sum [-pqrtx] [-c file] [-s string] [files ...]
```
This change moves the FreeBSD-specific code before the check for the `sha256sum` command.
Fixes: #26774
Top commit has no ACKs.
Tree-SHA512: 2485e2e7d8fdca3b072b29fb22bbdfd69e520740537b331b33c64cc645b63da712cfa63a23bdf039bbc92a6558fc7bf03323a51784bf601ff360ff0ef59506c8
927b8d4e0cddd89e1f71093c10bd697c25b7a7d8 rpc: Correct RPCHelpMan for fundrawtransaction's input_weights field (jdjkelly@gmail.com)
Pull request description:
`input_weights` is incorrectly documented as a fixed length JSON array, but it is actually a JSON array of JSON objects - this commit changes `input_weights` to use `RPCArg::Type::OBJ`
The behavior of `input_weights` as an object exists as a functional test in [wallet_fundrawtransaction.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_fundrawtransaction.py).
ACKs for top commit:
achow101:
ACK 927b8d4e0cddd89e1f71093c10bd697c25b7a7d8
Tree-SHA512: 384f5e16be36dba670d64d96f16f1fde2d0d51357e1094ae13eb71d004af0f4dc8bac965b4d2d724ccf64fb671faad37b73055152a9882af24f65dfceaf1e5fb
fa818e103c0ddb515f29ae9ce8de44931e12e69e txmempool: Remove unused clear() member function (MarcoFalke)
Pull request description:
Seems odd to have code in Bitcoin Core that is unused.
Moreover the function was broken (see https://github.com/bitcoin/bitcoin/pull/24145) and is brittle, as there is nothing that prevents similar bugs from re-appearing.
Fix both issues by replacing it with C++11 member initializers.
ACKs for top commit:
glozow:
ACK fa818e103c0ddb515f29ae9ce8de44931e12e69e
Tree-SHA512: e79e44cac7d5a84d9ecc8e3f3b0b9a50e1e3ebec358b20ba5dac175ef07d1fbe338a20f83ee80f746f7c726c79e77f8be49e14bca57a41063da8a5302123c3a9
76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 gui: create tx, launch error dialog if backend throws runtime_error (furszy)
f4d79477ff0946b0bd340ade9251fa38e3b95dd7 wallet: coin selection, add duplicated inputs checks (furszy)
0aa065b14e67592d5be8f46ebbe5d59a083ff0a5 wallet: return accurate error messages from Coin Selection (furszy)
7e8340ab1a970a14e180b1fcf420b46a5657b062 wallet: make SelectCoins flow return util::Result (furszy)
e5e147fe97f706e82bc51358f8bdc355f355be57 wallet: refactor eight consecutive 'AttemptSelection' calls into a loop (furszy)
Pull request description:
Work decoupled from #25806, which cleanup and improves the Coin Selection flow further.
Adding the capability to propagate specific error messages from the Coin Selection process to the user.
Instead of always returning the general "Insufficient funds" message which is not always accurate to what happened internally.
Letting us instruct the user how to proceed under certain circumstances.
The following error messages were added:
1) If the selection result exceeds the maximum transaction weight,
we now will return:
-> "The inputs size exceeds the maximum weight. Please try sending
a smaller amount or manually consolidating your wallet's UTXOs".
2) If the user pre-selected inputs and disallowed the automatic coin
selection process (no other inputs are allowed), we now will
return:
-> "The preselected coins total amount does not cover the transaction
target. Please allow other inputs to be automatically selected or include
more coins manually".
3) The double-counted preset inputs during Coin Selection error will now
throw an "internal bug detected" message instead of crashing the node.
The essence of this work comes from several comments:
1. https://github.com/bitcoin/bitcoin/pull/26560#discussion_r1037395665
2. https://github.com/bitcoin/bitcoin/pull/25729#discussion_r940619491
3. https://github.com/bitcoin/bitcoin/pull/25269#pullrequestreview-1135240825
4. https://github.com/bitcoin/bitcoin/issues/23144 (which is connected to #24845)
ACKs for top commit:
ishaanam:
crACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
achow101:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
aureleoules:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
theStack:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 🌇
Tree-SHA512: 9de30792d7a5849cae77747aa978e70390b66ee9d082779a56088a024f82e725b0af050e6603aece0ac8229f6d73bc471ba97b4ab69dc7eddf419f5f56ae89a5
47c4b1f52ab8d95d7deef83050bad49d1e3e5990 mempool: log/halt when CalculateMemPoolAncestors fails unexpectedly (stickies-v)
5481f65849313ff947f38433b1ac28285a7f7694 mempool: add AssumeCalculateMemPoolAncestors helper function (stickies-v)
f911bdfff95eba3793fffaf71a31cc8bfc6f80c9 mempool: use util::Result for CalculateMemPoolAncestors (stickies-v)
66e028f7399b6511f9b73b1cef54b6a6ac38a024 mempool: use util::Result for CalculateAncestorsAndCheckLimits (stickies-v)
Pull request description:
Upon reviewing the documentation for `CTxMemPool::CalculateMemPoolAncestors`, I noticed `setAncestors` was meant to be an `out` parameter but actually is an `in,out` parameter, as can be observed by adding `assert(setAncestors.empty());` as the first line in the function and running `make check`. This PR fixes this unexpected behaviour and introduces refactoring improvements to make intents and effects of the code more clear.
## Unexpected behaviour
This behaviour occurs only in the package acceptance path, currently only triggered by `testmempoolaccept` and `submitpackage` RPCs.
In `MemPoolAccept::AcceptMultipleTransactions()`, we first call `PreChecks()` and then `SubmitPackage()` with the same `Workspace ws` reference. `PreChecks` leaves `ws.m_ancestors` in a potentially non-empty state, before it is passed on to `MemPoolAccept::SubmitPackage`. `SubmitPackage` is the only place where `setAncestors` isn't guaranteed to be empty before calling `CalculateMemPoolAncestors`. The most straightforward fix is to just forcefully clear `setAncestors` at the beginning of CalculateMemPoolAncestors, which is done in the first bugfix commit.
## Improvements
### Return value instead of out-parameters
This PR updates the function signatures for `CTxMemPool::CalculateMemPoolAncestors` and `CTxMemPool::CalculateAncestorsAndCheckLimits` to use a `util::Result` return type and eliminate both the `setAncestors` `in,out`-parameter as well as the error string. It simplifies the code and makes the intent and effects more explicit.
### Observability
There are 7 instances where we currently call `CalculateMemPoolAncestors` without actually checking if the function succeeded because we assume that it can't fail, such as in [miner.cpp](69b10212ea/src/node/miner.cpp (L399)). This PR adds a new wrapper `AssumeCalculateMemPoolAncestors` function that logs such unexpected failures, or in case of debug builds even halts the program. It's not crucial to the objective, more of an observability improvement that seems sensible to add on here.
ACKs for top commit:
achow101:
ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990
w0xlt:
ACK 47c4b1f52a
glozow:
ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990
furszy:
light code review ACK 47c4b1f5
aureleoules:
ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990
Tree-SHA512: d908dad00d1a5645eb865c4877cc0bae74b9cd3332a3641eb4a285431aef119f9fc78172d38b55c592168a73dae83242e6af3348815f7b37cbe2d448a3a58648
1e5e87cec32260a6dfd03f11040aed8b7bb4064e script: update python linter dependencies (Jon Atack)
459cb637aca80f744a8399e84bc78fab60de0b5c script, test: fix python linter E275 errors with flake8 5.0.4 (Jon Atack)
Pull request description:
It is helpful to be able to run the python linter locally to review PRs and check local diffs and work. Fix the errors raised by `./test/lint/lint-python.py` when run locally with flake8 5.0.4, which enforces rule E275 more strictly than previous versions, and update our python linter CI dependencies.
ACKs for top commit:
kristapsk:
ACK 1e5e87cec32260a6dfd03f11040aed8b7bb4064e
Tree-SHA512: c7da09396144b9fed4ce6405c0763b2e3e5651d49a5220b053da465aceca09f754fb70a8ab9647278ce2028dde803bea461a3cd93fb39868ead39d06187cd8af
04609284ad5e0b72651f2d4b43263461ada40816 rpc: Improve error when wallet is already loaded (Aurèle Oulès)
Pull request description:
Currently, trying to load a descriptor (sqlite) wallet that is already loaded throws the following error:
> error code: -4
> error message:
> Wallet file verification failed. SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of Bitcoin Core?
I don't think it is very clear what it means for a user.
While a legacy wallet would throw:
> error code: -35
> error message:
> Wallet file verification failed. Refusing to load database. Data file '/home/user/.bitcoin/signet/wallets/test_wallet/wallet.dat' is already loaded.
This PR changes the error message for both types of wallet to:
> error code: -35
> error message:
> Wallet file verification failed. Wallet "test_wallet" is already loaded.
ACKs for top commit:
achow101:
ACK 04609284ad5e0b72651f2d4b43263461ada40816
hernanmarino:
ACK 0460928
theStack:
Tested ACK 04609284ad5e0b72651f2d4b43263461ada40816
Tree-SHA512: a8f3d5133bfaef7417a6c05d160910ea08f32ac62bfdf7f5ec305ff5b62e9113b55f385abab4d5a4ad711aabcb1eb7ef746eb41f841b196e8fb5393ab3ccc01e
9622fe64b8785430c71d4abc8637075026dc690c test: move coins result test to wallet_tests.cpp (furszy)
f69347d0588647ff9a4e986c7be987827a0417f4 test: extend and simplify availablecoins_tests (furszy)
212ccdf2c2b70d973b18ae78f0158ec5f0c3bbb4 wallet: AvailableCoins, add arg to include/skip locked coins (furszy)
Pull request description:
Negative PR with extended test coverage :).
1) Cleaned duplicated code and added coverage for the 'AvailableCoins' incremental result.
2) The class `AvailableCoinsTestingSetup` inside `availablecoins_tests.cpp` is a plain copy
of `ListCoinsTestingSetup` that is inside `wallet_tests.cpp`.
So, deleted the file and moved the `BasicOutputTypesTest` test case to `wallet_tests.cpp`.
3) Added arg to include/skip locked coins from the `AvailableCoins` result. This is needed for point (1) as otherwise the wallet will spend the coins that we recently created due its closeness to the recipient amount.
Note: this last point comes from #25659 where I'm using the same functionality to clean/speedup another flow as well.
ACKs for top commit:
achow101:
ACK 9622fe64b8785430c71d4abc8637075026dc690c
theStack:
ACK 9622fe64b8785430c71d4abc8637075026dc690c
aureleoules:
reACK 9622fe64b8785430c71d4abc8637075026dc690c, nice cleanup!
Tree-SHA512: 1ed9133120bfe8815455d1ad317bb0ff96e11a0cc34ee8098716ab9b001749168fa649212b2fa14b330c1686cb1f29039ff1f88ae306db68881b0428c038f388
81d4a2b14ff65fe07085ef2a967a466015370ce3 refactor: Move feerate comparison invariant outside of the loop (yancy)
365aca40453995163bbd17231251512f9f9a103b refactor: Simplify feerate comparison statement (yancy)
Pull request description:
This is a small nit, however I think it's more understandable to write:
`utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee`
vs
`(utxo_pool.at(0).fee - utxo_pool.at(0).long_term_fee) > 0`
ACKs for top commit:
Xekyo:
ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3
achow101:
ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3
aureleoules:
ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3
Tree-SHA512: 3e89377989c36716b53114fe40178261671dde5688075fab1c21ec173ac310f8c84ed6af90354d7c329176cb7262dfcaa7191fd19847d3b7147a9a10c3e31176
f496528556a67107d3d75d9c2ae345f7f4565d77 walletdb: refactor: drop unused `FindWalletTx` parameter and rename (Sebastian Falbesoner)
Pull request description:
Since commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b ("Remove -zapwallettxes"), the `FindWalletTx` helper is only needed to read tx hashes, so drop the other parameter and rename the method accordingly.
ACKs for top commit:
S3RK:
code review ACK f496528556a67107d3d75d9c2ae345f7f4565d77
achow101:
ACK f496528556a67107d3d75d9c2ae345f7f4565d77
vincenzopalazzo:
ACK f496528556
Tree-SHA512: ead85bc724462f9e920f9d7fe89679931361187579ffd6e63427c8bf5305cd5f71da24ed84f3b1bd22a12be46b5abec13f11822e71a3e1a63bf6cf49de950ab5
input_weights is incorrectly documented as a fixed length JSON array,
but it is actually a JSON array of JSON objects - this commit changes
input_weights to use RPCArg::Type::OBJ
fabb6af8508a9c0d05d2e9f0006b0b1d3f38119a ci: Remove duplicate CC and CXX from tsan task (MarcoFalke)
fa5d9a0e246e94eb0d0e9a75f63ff1b09ec93c03 Revert "ci: Use clang-15 in tsan task" (MarcoFalke)
faa835e7e564536fb0589ec94d3f85042a1f9cd8 Revert "test: Drop no longer needed `race:epoll_ctl` TSan suppression" (MarcoFalke)
Pull request description:
Looks like there are still bugs in clang-15, so we need to roll back all the way to the previously used version (clang-13).
ACKs for top commit:
hebasto:
ACK fabb6af8508a9c0d05d2e9f0006b0b1d3f38119a, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: d62203049847ab9095ee3fc89e18bdd721d1d9d5a7ef7a9f524c80e6be58d1d9f6aa2f14533df1ea77eb59597fba6fa9b987b17eb03b2c3f7cb577ab59cd59c0
090ad51c80fe02a73f8988c1d5b867d2e90ab2f3 rpc: Remove duplicate field in RPCHelpMan for gettransactions (Joshua Kelly)
Pull request description:
The field 'comment' appears twice in `TransactionDescriptionString`, incorrectly - this commit removes the instance of the comment field without a description, preserving the one with a description.
On master, the duplicate fields can be be viewed here: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpc/transactions.cpp#L419-L423
`TransactionDescriptionString` is included in RPC calls such as `listtransactions` which have functional tests.
ACKs for top commit:
w0xlt:
ACK 090ad51c80
Tree-SHA512: 4bacdafdb517dda2af6d1c193f331b634ae74bd62ac6289c0c288957f39f98a73d07aeab72fbe5bf1ece5532406d4a40a5b8a2277be50115f76c92bb938e21fa
The field 'comment' appears twice in TransactionDescriptionString,
incorrectly - this commit removes the instance of the comment field
without a description, preserving the one with a description
faa00ca78e27a40cbafe701348e6b3674e3ddeed ci: Use clang-15 in tsan task (MarcoFalke)
Pull request description:
Generally it is best to use the latest clang version for sanitizers, because it comes with the most features and bugfixes.
So bump to clang-15, the latest release, for the tsan task.
The task was using clang-13 (instead of 14) due to a bug, see https://github.com/bitcoin/bitcoin/pull/24572#issue-1169970859. Bumping to 15 will hopefully fix this bug, as well as https://github.com/bitcoin/bitcoin/pull/26759#issuecomment-1367360491
ACKs for top commit:
hebasto:
ACK faa00ca78e27a40cbafe701348e6b3674e3ddeed
Tree-SHA512: adb2386bb9615a3e1185e0624b0b68cd2738309530185819714a26e63bdf1c79461c4b4d3aa9cbe2fe08cc412349d7453f192abbbe9fb5adca74cf4b148ae7b7
This change reverts cc7335edc87c6ef34429b4df94f53973db520aac partially.
C++20 has introduced some new headers, and it is premature to consider
them when using the IWYU tool.
3666a06730b2229e5af24124e6ef0403e4a6792e test: add coverage for unknown wallet flag in `setwalletflag` (brunoerg)
Pull request description:
This PR adds test coverage for the following error:
6d40a1a7e7/src/wallet/rpc/wallet.cpp (L275-L277)https://marcofalke.github.io/btc_cov/total.coverage/src/wallet/rpc/wallet.cpp.gcov.html
ACKs for top commit:
aureleoules:
ACK 3666a06730b2229e5af24124e6ef0403e4a6792e
Tree-SHA512: b6b2415442dfbc2404aed9720cc899995686007d6ba222dae461d064e4454d5af1326d3d527770b51b1005721ac42a49972f1eabf21108f656c30d3584790747
f1e89597c803001ab9d5afd7e173184fe6886d1d test: Drop no longer required bench output redirection (Hennadii Stepanov)
4dbcdf26a301f54c60c85ceab1aaa4dae43f6aeb bench: Suppress output when running with `-sanity-check` option (Hennadii Stepanov)
Pull request description:
This change allows to simplify CI tests, and makes it easier to integrate the `bench_bitcoin` binary into CMake custom [targets](https://cmake.org/cmake/help/latest/command/add_custom_target.html) or [commands](https://cmake.org/cmake/help/latest/command/add_custom_command.html), as `COMMAND` does not support output redirection.
ACKs for top commit:
aureleoules:
tACK f1e89597c803001ab9d5afd7e173184fe6886d1d. Ran as expected and is more practical than using an output redirection.
Tree-SHA512: 29086d428cccedcfd031c0b4514213cbc1670e35f955e8fd35cee212bc6f9616cf9f20d0cb984495390c4ae2c50788ace616aea907d44e0d6a905b9dda1685d8
a3f5e541523a843e834df1858e16f89188fe19a2 test: Drop no longer needed `race:epoll_ctl` TSan suppression (Hennadii Stepanov)
Pull request description:
The removed suppression seems no needed.
I cannot point the exact commit/PR which makes this change possible.
Top commit has no ACKs.
Tree-SHA512: 8ee79cbdb2bc62796d72c69be4a818379132eae47be33951e8b9d224b049ff77e867004801c7cb0cc564a5374f318dafd9142b5231e9bd428f80acc75253933e
Currently `migratewallet` migrates the address book (i.e. labels and
purposes) for watchonly and solvable wallets only in RAM, but doesn't
persist them on disk. Fix this by adding another loop for both of the
special wallet types after which writes the corresponding NAME and
PURPOSE entries to the database in a single batch.
