6777ab73a2 Merge bitcoin-core/gui#682: Don't directly delete abandoned txes from GUI (Hennadii Stepanov)
6e1a8c1fdc Merge bitcoin/bitcoin#26628: RPC: Reject RPC requests with same named parameter specified multiple times (MarcoFalke)
7c28b01c78 Merge bitcoin/bitcoin#26666: refactor: Deleted unreachable code in httpserver.cpp (MarcoFalke)
478fe51ead Merge bitcoin/bitcoin#26100: doc: clarify that NetPermissionFlags::Implicit is only about whitelists (MarcoFalke)
69b19cbfc0 Merge bitcoin/bitcoin#26546: test: remove unused class `NodePongAdd1` (fanquake)
245df942c4 Merge bitcoin/bitcoin#26380: Revert "test: check importing wallets when blocks are pruned throw an error" (MacroFake)
3db3bd0d31 Merge bitcoin/bitcoin#24269: test: add functional test for `-discover` (Andrew Chow)
c72ef299da Merge bitcoin/bitcoin#25896: wallet: Log when Wallet::SetMinVersion sets a different minversion (Andrew Chow)
12b438cb46 Merge bitcoin/bitcoin#25925: doc: add `{import,list}descriptors` to list of descriptor RPCs (Andrew Chow)
73d64c48a9 Merge bitcoin/bitcoin#25738: depends: use a patch instead of sed in libxcb (fanquake)
1fae0c2bbb Merge bitcoin/bitcoin#25333: test: Fix out-of-range port collisions (MacroFake)
b75089667c Merge bitcoin/bitcoin#25231: ci: Install documented packages for "Win64" CI task (fanquake)
Pull request description:
## Issue being fixed or feature implemented
Batch of trivial backports
## What was done?
Trivial backports
## How Has This Been Tested?
Built; ran tests locally
## Breaking Changes
None
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] 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 6777ab73a2
Tree-SHA512: 60d68c8d0fb9875d0b2421cad97b46a9d4deb79e03ca011d66cc0595ed68bb7ad207f2fd95973a3b7a0b9bca2c2f0deaf0e1116f0312a4d9d315f009228fe485
8c3ff7d52ae3314959e1e66da8718a3f0d30abaa test: Suggested cleanups for rpc_namedparams test (Ryan Ofsky)
d1ca56382512df3084fce7353bf1e8b66cae61bc bitcoin-cli: Make it an error to specify the "args" parameter two different ways (Ryan Ofsky)
6bd1d20b8cf27aa72ec2907342787e6fc9f94c50 rpc: Make it an error server-side to specify same named parameter multiple times (Ryan Ofsky)
e2c3b18e671e347e422d696d1cbdd9f82b2ce468 test: Add RPC tests for same named parameter specified more than once (Ryan Ofsky)
Pull request description:
Make the JSON-RPC server reject requests with the same named parameter specified multiple times, instead of silently overwriting earlier parameter values with later ones.
Generally JSON keys are supposed to unique, and their order isn't supposed to be significant, so having the server silently discard duplicate keys is error-prone. Most likely if an RPC client is sending a request with duplicate keys it means something is wrong with the request and there should be an error.
After this change, named parameters are still allowed to specified multiple times on the `bitcoin-cli` command line, since `bitcoin-cli` automatically replaces earlier values with later values before sending the JSON-RPC request. This makes sense, since it's not unusual for the order of command line options to be significant or for later command line options to override earlier ones.
ACKs for top commit:
MarcoFalke:
review ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa 🗂
kristapsk:
ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa
stickies-v:
ACK 8c3ff7d52
Tree-SHA512: 2d1357dcc2c171da287aeefc7b333ba4e67babfb64fc14d7fa0940256e18010a2a65054f3bf7fa1571b144d2de8b82d53076111b5f97ba29320cfe84b6ed986f
32f8fda7d6 merge bitcoin#24991: allow startup with -onlynet=onion -listenonion=1 (Kittywhiskers Van Gogh)
e67ed92d3d merge bitcoin#25173: add coverage for unknown network in -onlynet (Kittywhiskers Van Gogh)
77efd36112 merge bitcoin#24687: Check an invalid -i2psam will raise an init error (Kittywhiskers Van Gogh)
fb1416f7cb merge bitcoin#24205: improve network reachability test coverage and safety (Kittywhiskers Van Gogh)
7cb7479829 merge bitcoin#24663: add links to doc/cjdns.md (Kittywhiskers Van Gogh)
c736ebf566 merge bitcoin#24555: create initial doc/cjdns.md for CJDNS how-to documentation (Kittywhiskers Van Gogh)
554bd24186 partial bitcoin#24468: improve -onlynet help and related tor/i2p documentation (Kittywhiskers Van Gogh)
5436b6a82d merge bitcoin#24165: extend inbound eviction protection by network to CJDNS peers (Kittywhiskers Van Gogh)
d52724d039 merge bitcoin#22834: respect -onlynet= when making outbound connections (Kittywhiskers Van Gogh)
f9d1a9a00d merge bitcoin#23077: Full CJDNS support (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Depends on https://github.com/dashpay/dash/pull/6034
* Depends on https://github.com/dashpay/dash/pull/6035
* If `-proxy=` is given together with `-noonion` then the provided proxy will not be set as a proxy for reaching the Tor network. So it will not be possible to open manual connections to the Tor network for example with the `addnode` RPC. To mimic the old behavior use `-proxy=` together with `-onlynet=` listing all relevant networks except `onion`.
* [bitcoin#24165](https://github.com/bitcoin/bitcoin/pull/24165) has been backported _before_ [bitcoin#23758](https://github.com/bitcoin/bitcoin/pull/23758) and to account for this, minor changes were made in `src/test/net_peer_eviction_tests.cpp` (using `nTimeConnected` instead of `m_connected`). When backporting [bitcoin#23758](https://github.com/bitcoin/bitcoin/pull/23758), these changes will have to be reversed as they won't be covered by the cherry-pick diff.
* CJDNS support has been labelled as being introduced in Dash Core 21.0, in line with the milestone designation of the PR. Should `develop` be used for a new minor/patch release, `doc/cjdns.md` will have to be modified to reflect the correct version number.
## Breaking changes
No expected protocol or consensus changes.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [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:
PastaPastaPasta:
utACK 32f8fda7d6
Tree-SHA512: e23b22ca5edbe4c4abeab0bc07780303e68e7c4cc46b7697300b0837c5acd3a98649b6b03bd07a23c827bd85f64210173027b0b0eea31872c031fa4ed04eeb0c
6242314ba8a0ebf036e782e75c80d1b1ad6251ca doc: add `{import,list}descriptors` to list of descriptor RPCs (Sebastian Falbesoner)
Pull request description:
This PR adds the missing RPCs `importlistdescriptors` ([since v0.21](1420547ec3/doc/release-notes/release-notes-0.21.0.md (L405))) and `listdescriptors` ([since v22](1420547ec3/doc/release-notes/release-notes-22.0.md (L175))) to the list of RPCs supporting descriptors in descriptors.md. Also changes the description of `importmulti` slightly to point out that it only works for legacy wallets.
ACKs for top commit:
S3RK:
ACK 6242314ba8a0ebf036e782e75c80d1b1ad6251ca
achow101:
ACK 6242314ba8a0ebf036e782e75c80d1b1ad6251ca
aureleoules:
ACK 6242314ba8a0ebf036e782e75c80d1b1ad6251ca.
brunoerg:
ACK 6242314ba8a0ebf036e782e75c80d1b1ad6251ca
Tree-SHA512: e8905c800b0c9a760e3380efebe2fb015c321a891dd4bf283039486d9d3b382b2c76901fcc8413acf435ed9832f76d9828efd70ba5ce62d4be65e87672bbd0a2
86a4a15bdcc96eb565ab80166642d71d542061a9 Highlight DNS request part (Prayank)
Pull request description:
_What?_
Highlight DNS requests part in Proxy section
_Why?_
1. DNS requests are very important while considering privacy
2. Lot of users might skip reading it because of the way it is mixed with everything else in the doc right now
3. I have seen lot of users ignoring DNS requests or unaware of such things while using privacy tools
_How?_
Initially I had tried keeping these lines separate from code block but [Jonatack didn't agree with the changes](https://github.com/bitcoin/bitcoin/pull/21157#discussion_r618177116). Harding suggested using [bold/italic in `<pre></pre>`](https://github.com/bitcoin/bitcoin/pull/21157#discussion_r644364143). I have used the suggestions from previous PR and added `---`
This is a part of alternative described in https://github.com/bitcoin/bitcoin/pull/22316
ACKs for top commit:
jonatack:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
Rspigler:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
achow101:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
RiccardoMasutti:
ACK 86a4a15
lsilva01:
ACK 86a4a15bdc
kristapsk:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
theStack:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
Tree-SHA512: a4fe0e8c08df330e5ca78ce19ce74be7034c653f4374469d928908847a6debf385283e3a6da66de600566c7bab6290ccd35df26864aef94cbb3f294123391437
34c80473a8 Merge #19877: [test] clarify rpc_net & p2p_disconnect_ban functional tests (Wladimir J. van der Laan)
e42412924f Merge #19770: RPC: getpeerinfo: Deprecate "whitelisted" field (replaced by "permissions") (MarcoFalke)
f96966b7ea Merge #20043: doc: Add 19501 release notes (fanquake)
6a164eaea9 Merge #19501: send* RPCs in the wallet returns the "fee reason" (MarcoFalke)
b6c8d852e3 Merge #19725: [RPC] Add connection type to getpeerinfo, improve logs (MarcoFalke)
f86263b180 Merge #18202: refactor: consolidate sendmany and sendtoaddress code (Samuel Dobson)
fab41fd3c5 partial Merge #18878: test: Add test for conflicted wallet tx notifications (Wladimir J. van der Laan)
db5bd34ee8 Merge #19202: log: remove deprecated `db` log category (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Regular backports from bitcoin v21
## What was done?
- bitcoin/bitcoin#19202
- partial bitcoin/bitcoin#18878
- bitcoin/bitcoin#18202
- bitcoin/bitcoin#19725
- bitcoin/bitcoin#19501
- bitcoin/bitcoin#20043
- bitcoin/bitcoin#19770
- bitcoin/bitcoin#19877
## How Has This Been Tested?
Run unit/functional tests
## Breaking Changes
- (RPC) The `getpeerinfo` RPC no longer returns the `addnode` field by default. This
field will be fully removed in the next major release. It can be accessed
with the configuration option `-deprecatedrpc=getpeerinfo_addnode`. However,
it is recommended to instead use the `connection_type` field (it will return
`manual` when addnode is true)
- (Settings) The `sendtoaddress` and `sendmany` RPCs accept an optional `verbose=True`
argument to also return the fee reason about the sent tx.
- (Settings) The `-debug=db` logging category, which was deprecated in v0.18 and replaced by
`-debug=walletdb` to distinguish it from `coindb`, has been removed.
- (RPC) To make RPC `sendtoaddress` more consistent with `sendmany` the following error
`sendtoaddress` codes were changed from `-4` to `-6`:
- Insufficient funds
- Fee estimation failed
- Transaction has too long of a mempool chain
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 34c80473a8
Tree-SHA512: 725a103e04c9c7d44a79da6f3f54e7745c7fb98ec906e7228ae16f7662d568e48c015c855902ff8485f2908f0f71815e769ca394cf6c3ca2e5fd920dd39cca74
fa710a6d67b2de64bde90def77c70d0a052f9030 doc: Add 19501 release notes (MarcoFalke)
faf60dee34ae3dbe8e103a2c1b0679f13df6a921 doc: Remove double-whitespace from help string, other whitespace fixups (MarcoFalke)
Pull request description:
Adds release notes and fixes up some whitespace nits for the touched RPCs
ACKs for top commit:
fanquake:
ACK fa710a6d67b2de64bde90def77c70d0a052f9030
laanwj:
Code review ACK fa710a6d67b2de64bde90def77c70d0a052f9030
Tree-SHA512: b84a96386a9a8ed69f464c7dffdd600cf9a8b33a06120798b141b300991baed369ab91ae48df6446e89e1d62534ccd8ae721454e7a19b48900b317e9192afc47
a512925e19a70d7f6b80ac530a169f45ffaafa1c [doc] Release notes (Amiti Uttarwar)
50f94b34a33c954f6e207f509c93d33267a5c3e2 [rpc] Deprecate getpeerinfo addnode field (Amiti Uttarwar)
df091b9b509f0b10e4315c0bfa2da0cc0c31c22f [refactor] Rename test file to allow any getpeerinfo deprecations. (Amiti Uttarwar)
395acfa83a5436790c1a722a5609ac9d48df235f [rpc] Add connection type to getpeerinfo RPC, update tests (Amiti Uttarwar)
49c10a9ca40967d28ae16dfea9cccc6f3a6624a1 [log] Add connection type to log statement (Amiti Uttarwar)
Pull request description:
After #19316, we can more directly expose information about the connection type on the `getpeerinfo` RPC. Doing so also makes the existing addnode field redundant, so this PR begins the process of deprecating this field.
This PR also includes one commit that improves a log message, as both use a shared function to return the connection type as a string.
Suggested by sdaftuar- https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468001604 & https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468018093
ACKs for top commit:
jnewbery:
Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c.
sipa:
utACK a512925e19a70d7f6b80ac530a169f45ffaafa1c
guggero:
Tested and code review ACK a512925e.
MarcoFalke:
cr ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c 🌇
promag:
Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c.
Tree-SHA512: 601a7a38aee235ee59aca690784f886dc2ae4e418b2e6422c4b58cd597376c00f74910f66920b08a08a0bec28bf8022e71a1435785ff6ba8a188954261aba78e
08fc6f6cfc3b06fd170452a766696d7b833113fa [rpc] refactor: consolidate sendmany and sendtoaddress code (Sjors Provoost)
Pull request description:
I consolidated code between these two RPC calls, since `sendtoaddress` is essentially `sendmany` with 1 destination.
Unless I overlooked something, the only behaviour change is that some `sendtoaddress` error codes changed from `-4` to `-6`. The release note mentions this.
Salvaged from #18201.
ACKs for top commit:
fjahr:
Code review ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
jonatack:
ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
meshcollider:
Code review & functional test run ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
Tree-SHA512: 7b66c52fa0444a4d02fc3f81d9c2a386794d447616026a30111eda35fb46510475eea6506a9ceda00bb4e0230ebb758da5d236b3ac05c954c044fa68a1e3e909
c514a4f59a7430f05dbe20465ddf4ca323329f1e doc: release note for `db` log category removal (Jon Atack)
4c0c89307dabbf51a32551471c54966ddf7c5bc3 log: remove deprecated `db` log category (Jon Atack)
Pull request description:
The `db` log category was renamed to `walletdb` (like `coindb`) in #17410 and its upcoming removal announced in the 0.20 release notes.
```
- The `-debug=db` logging category has been renamed to
`-debug=walletdb` to distinguish it from `coindb`. The `-debug=db`
option has been deprecated and will be removed in the next major
release. (#17410)
```
This PR removes the warning and reverts to the usual behavior for an unrecognised log category.
```
$ bitcoin-cli logging '["db"]'
error code: -8
error message:
unknown logging category db
```
```
$ ./src/bitcoind -debug=db
Warning: Unsupported logging category -debug=db.
2020-06-07T15:30:45Z Bitcoin Core version v0.20.99.0-4c0c89307d (debug build)
2020-06-07T15:30:45Z Warning: Unsupported logging category -debug=db.
2020-06-07T15:30:45Z Assuming ancestors of block 0000000000000000000f2adce67e49b0b6bdeb9de8b7c3d7e93b21e7fc1e819d have valid signatures.
2020-06-07T15:30:45Z Setting nMinimumChainWork=00000000000000000000000000000000000000000e1ab5ec9348e9f4b8eb8154
2020-06-07T15:30:45Z Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation
2020-06-07T15:30:45Z Using RdSeed as additional entropy source
```
ACKs for top commit:
MarcoFalke:
ACK c514a4f59a7430f05dbe20465ddf4ca323329f1e 🔄
Tree-SHA512: fd62fd7ae0dc65446ba4401d75b4047e055396a33f7f1b176e79a7753250aec2a474ae604163d3f7e68710443c0ed2f45e44435d15f35612d794807e2142d5a3
8047bb6feaa9ee5d6c1edb7640baaf228450bc6b build: Update `qt` package up to 5.15.11 (Hennadii Stepanov)
Pull request description:
In the light of https://github.com/bitcoin/bitcoin/pull/28622, we probably have to patch Qt. It seems reasonable to update it up to the latest available version before doing that.
ACKs for top commit:
TheCharlatan:
ACK 8047bb6feaa9ee5d6c1edb7640baaf228450bc6b
Tree-SHA512: b4d7df2ff059b8f58c3202d913237c0d39a962748658f1ce853884dca095fbda5f56d4d68f73a1bc8da2f295e96a20927306e148b41a9f4afc42c8edb11c3729
06c43201a714b0426cc68b2fd5c681e5df10af99 cli: use C++17 std::array class template argument deduction (CTAD) (Jon Atack)
edf3167151f7a6d08cf733b4e230e2d745819ac8 addrinfo: raise helpfully on server error or incompatible server version (Jon Atack)
bb85cbc4f7638a85049658ed951a0e06e7959cd4 doc: add cli -addrinfo release note (Jon Atack)
5056a37624b64588b277419f7ed8c325477a8ec7 cli: add -addrinfo command (Jon Atack)
db4d2c282afd46709792aaf2d36ffbfc1745b776 cli: create AddrinfoRequestHandler class (Jon Atack)
Pull request description:
While looking at issue #21351, it turned out that the problem was a lack of tor v3 addresses known to the node. It became clear (e.g. https://github.com/bitcoin/bitcoin/issues/21351#issuecomment-811004779) that a CLI command returning the number of addresses the node knows per network (with a tor v2 / v3 breakdown) would be very helpful. This patch adds that.
`-addrinfo` is useful to see if your node knows enough addresses in a network to use options like `-onlynet=<network>`, or to upgrade to the upcoming tor release that no longer supports tor v2, for which you'll need to be sure your node knows enough tor v3 peers.
```
$ bitcoin-cli --help | grep -A1 addrinfo
-addrinfo
Get the number of addresses known to the node, per network and total.
$ bitcoin-cli -addrinfo
{
"addresses_known": {
"ipv4": 14406,
"ipv6": 2511,
"torv2": 5563,
"torv3": 2842,
"i2p": 8,
"total": 25330
}
}
$ bitcoin-cli -addrinfo 1
error: -addrinfo takes no arguments
```
This can be manually tested, for example, with commands like this:
```
$ bitcoin-cli getnodeaddresses 0 | jq '.[] | (select(.address | contains(".onion")) | select(.address | length <= 22)) | .address' | wc -l
5563
$ bitcoin-cli getnodeaddresses 0 | jq '.[] | (select(.address | contains(".onion")) | select(.address | length > 22)) | .address' | wc -l
2842
$ bitcoin-cli getnodeaddresses 0 | jq '.[] | .address' | wc -l
25330
```
ACKs for top commit:
laanwj:
Tested ACK 06c43201a714b0426cc68b2fd5c681e5df10af99
Tree-SHA512: b668b47718a4ce052aff218789f3da629bca730592c18fcce9a51034d95a0a65f8e6da33dd47443cdd8f60c056c02696db175b0fe09a688e4385a76c1d8b7aeb
5f2be6e71e6130b58ebfbf81aaf48ce90dd9d179 Remove no longer used contrib/bitcoin-qt.pro from the repo (Hennadii Stepanov)
Pull request description:
From [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2021-04-15.html#l-209):
> \<hebasto> wumpus: I cannot see any way how the `contrib/bitcoin-qt.pro` is used in the translation process, neither in the main repo nor in https://github.com/bitcoin-core/bitcoin-maintainer-tools. Besides it looks outdated and unmaintained. May I ask you to confirm/deny my assumption?
> \<wumpus> hebasto: it is not used for anything, it exists to be able to edit the qt forms in qt designer nothing more
> \<wumpus> i'm not sure if it is even *necessary* for that, but it is why it is there
> \<hebasto> wumpus: thanks, qt designer does not need *.pro file at all
> \<hebasto> maybe qt creator does
> \<wumpus> feel free to create a PR to remove it, best way to find out if someone wants to keep it, you are right it hasn't been updated in a long time
> \<hebasto> ok
> \<wumpus> fwiw, the only question i get about it ever is why it exists
> \<hebasto> it was in use with `qmake` years ago (what I found digging into the repo history)
> \<wumpus> yes, that was the original reason, but when we switched to automake it was kept around for use w/ qt's GUI tools
> \<hebasto> I've noticed it in https://github.com/bitcoin/bitcoin/blame/master/doc/translation_process.md#L25
> \<wumpus> what it says there is definitely not true anymore
ACKs for top commit:
laanwj:
ACK 5f2be6e71e6130b58ebfbf81aaf48ce90dd9d179
jarolrod:
ACK 5f2be6e71e6130b58ebfbf81aaf48ce90dd9d179
Tree-SHA512: 7c105612f28185097fee9e4108b162b4c8b07cc527f4438bdf5bcab08c65421ea301de8584d58770cd113fa871f6781daa8145bd6463278523449e28bfc49d06
ceefab5226 fix: feature_backwards compatible works now with as expected if no bdb compiled (Konstantin Akimov)
b20f812674 fix: follow-up fixes for functional tests used protx (Konstantin Akimov)
655146d5e7 Merge #21302: wallet: createwallet examples for descriptor wallets (W. J. van der Laan)
99a8b60393 Merge #21063: wallet, rpc: update listdescriptors response format (fanquake)
6ee2c7cc59 Merge #21277: wallet: listdescriptors uses normalized descriptor form (Wladimir J. van der Laan)
8bacdbf71f Merge #19136: wallet: add parent_desc to getaddressinfo (Samuel Dobson)
f567de007a chore: release notes for 5965 with wallet tool improvements (Konstantin Akimov)
0daf360edf chore: add TODO to implement mnemonic for descriptor wallets (Konstantin Akimov)
5016294307 chore: move functional test wallet_multiwallet from category "slow 5 minutes" to "fast test" (Konstantin Akimov)
ef7ce87c1b fix: remove workarounds introduced due to missing bitcoin#20267 (bdb is not compiled) (Konstantin Akimov)
06b2d85bb4 partial Merge #20267: Disable and fix tests for when BDB is not compiled (Wladimir J. van der Laan)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/59
## Extra notes
This commit `chore: move functional test wallet_multiwallet from category "slow 5 minutes" to "fast test"` is not directly connected to descriptor wallets, but added to this PR due to conflicts with 20267
## What was done?
It steadily improves support of descriptor wallets in Dash core.
Done backports and related fixes:
- partial bitcoin/bitcoin#20267
- bitcoin/bitcoin#19136
- bitcoin/bitcoin#21277
- bitcoin/bitcoin#21063
- bitcoin/bitcoin#21302
Beside backports and related fixes, this PR includes release notes for previous batch of backports for descriptor wallets support #5965
## How Has This Been Tested?
Run unit functional tests
## Breaking Changes
N/A
## 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
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
Top commit has no ACKs.
Tree-SHA512: f4b2033f8c4fa1d0f72cfc31378909703b3ae8f44748989ff00c3e71311ac80ac37837137133c7e4a166823a941ed7df10efa09c89f5b213f3c8ede7d3d6e8f4
1fedf470cd test: add type annotation for `ADDRS` in `p2p_addrv2_relay` (Kittywhiskers Van Gogh)
022b76f20b merge bitcoin#23218: Use mocktime for ping timeout (Kittywhiskers Van Gogh)
45d9e58023 merge bitcoin#22960: Set peertimeout in write_config (Kittywhiskers Van Gogh)
06e909b737 merge bitcoin#22604: address rate-limiting follow-ups (Kittywhiskers Van Gogh)
60b3e08ed1 merge bitcoin#22616: address relay fixups (Kittywhiskers Van Gogh)
8b8fbc5226 merge bitcoin#22618: Small follow-ups to 21528 (Kittywhiskers Van Gogh)
18fe765988 merge bitcoin#21528: Reduce addr blackholes (Kittywhiskers Van Gogh)
c1874c6615 net_processing: gate `m_tx_relay` access behind `!IsBlockOnlyConn()` (Kittywhiskers Van Gogh)
602d13d2a2 merge bitcoin#22387: Rate limit the processing of rumoured addresses (Kittywhiskers Van Gogh)
fe66202c05 merge bitcoin#22211: relay I2P addresses even if not reachable (by us) (Kittywhiskers Van Gogh)
7e08db55fe merge bitcoin#22306: Improvements to p2p_addr_relay.py (Kittywhiskers Van Gogh)
ff3497c18b merge bitcoin#21843: enable GetAddr, GetAddresses, and getnodeaddresses by network (Kittywhiskers Van Gogh)
51edeb082c merge bitcoin#21594: add network field to getnodeaddresses (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependency for https://github.com/dashpay/dash/pull/5982
* Population of `ADDRS` in `p2p_addr`(`v2`)`_relay` in Dash is done in the test object ([source](0a62b9f985/test/functional/p2p_addrv2_relay.py (L42-L49))) as opposed to upstream, where it is done in the global state ([source](d930c7f5b0/test/functional/p2p_addrv2_relay.py (L23-L35))). This is because Dash specifically relies on `self.mocktime` instead of Bitcoin, which will work with simply sampling current time (`time.time()`).
* [bitcoin#22211](https://github.com/bitcoin/bitcoin/pull/22211) adds changes ([source](https://github.com/bitcoin/bitcoin/pull/22211/files#diff-d3d7b1bb23f25a96c9c7444a79159ad1799895565f99efebf1618e41e886bd53R44-R46)) that add usage of `ADDRS` outside the test object. That, alongside with other considerations, resulted in [dash#5967](https://github.com/dashpay/dash/pull/5967) and a discussion ([source](https://github.com/dashpay/dash/pull/5967/files#r1548101561))
* Eventually, following the footsteps of [dash#5967](https://github.com/dashpay/dash/pull/5967), `ADDRS` was defined outside but setup within the test object. This worked just fine ([build](https://gitlab.com/dashpay/dash/-/jobs/6594036014)) but displeased the linter ([build](https://gitlab.com/dashpay/dash/-/jobs/6594035886)) because `ADDRS` type could not be implicitly determined solely on usage in the global scope.
* An attempt to correct this was done by realignment with upstream ([commit](262d00682c)), which pleased the linter ([build](https://gitlab.com/dashpay/dash/-/jobs/6597322521)) but broken the test ([build](https://gitlab.com/dashpay/dash/-/jobs/6597322548)) for the reasons as mentioned above.
* Therefore, to keep the linter happy, `ADDRS` has been annotated as a `List[CAddress]` (which involved importing `List` but that's fine) ([commit](cb6d36df7d))
* Working on [bitcoin#21528](https://github.com/bitcoin/bitcoin/pull/21528) proved challenging due to differences in Dash's and Bitcoin's approach to relaying and the workarounds used to accommodate for that.
* Bitcoin conditionally initializes `m_tx_relay` ([source](3f7250b328/src/net.cpp (L2989-L2991))) and can always check if transaction relaying is permitted by checking if it's initialized ([source](3f7250b328/src/net_processing.cpp (L1820-L1826))).
* Dash unconditionally initializes it ([source](0a62b9f985/src/net.h (L605-L607))). Earlier, Dash used to check if it's _appropriate_ to relay transactions by checking if it can relay addresses ([source](dc6f52ac99/src/net_processing.cpp (L2134-L2140))), which at the time, simply meant, it wasn't a block-only connection ([source](dc6f52ac99/src/net.h (L568-L572))).
* This mutual exclusivity no longer held true in [dash#5964](https://github.com/dashpay/dash/pull/5964) and therefore, some transaction relay decisions were bound to **not** being a block-only connection ([commit](26c39f5b92)) but some were left behind, adopting `RelayAddrsWithPeer()` ([source](0a62b9f985/src/net_processing.cpp (L2215-L2221))), which, to be noted, is determined by the initialization status of `Peer::m_addr_known` ([source](0a62b9f985/src/net_processing.cpp (L839-L842))), which, so far, was pegged to **not** block-relay connection status ([source](0a62b9f985/src/net_processing.cpp (L1319))).
* [bitcoin#21528](https://github.com/bitcoin/bitcoin/pull/21528) got rid of `RelayAddrsWithPeer()` and replaced it with `Peer::m_addr_relay_enabled` ([source](3f7250b328/src/net_processing.cpp (L237-L251))), which is setup using `Peer::SetupAddressRelay()` ([source](3f7250b328/src/net_processing.cpp (L637-L643))). This means, rather than defining the address relay status during construction, it is setup during the first address-related message (i.e. `ADDR`, `ADDRV2`, `GETADDR`) ([source](3f7250b328/src/net_processing.cpp (L227-L236))).
* Meaning, until the first addr-related message happens, the state is has not been determined and defaults to `false`. Because some `m_tx_relay` usage still piggybacked on addr-relay permission to determine tx-relay, if a transaction message is processed before an address message is processed, there will be a false-negative condition.
The transaction relay logic won't run since it's expecting that if transactions can be relayed, so can addresses and checks for address relaying but believes that it cannot do address relaying, borrowing that state for transaction relaying, despite address relaying permissions actually being indeterminate since it hasn't had a chance to validate its eligibility.
* There were two approaches, run `SetupAddressRelay()` as early in the connection as possible to substitute for the "determine at construction" behaviour and change no other conditional statements... and break address-related tests _or_ move the remaining conditional transaction relay logic to use **not** block-only connection checks instead.
* We've gone with the latter, resulting in some changes where the condition only changes form but is the same (`RelayAddrsWithPeer()` > `Peer::m_addr_relay_enabled`) ([source](109c5a9383 (diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3L2131-L2134))) but other changes where the condition itself has been changed (`RelayAddrsWithPeer()` > `!CNode::IsBlockOnlyConn()`) ([source](109c5a9383 (diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R2256-R2259)))
* This does mean that in [dash#5982](https://github.com/dashpay/dash/pull/5982), `Peer::m_block_relay_only` is introduced to be the counterpart to `Peer::m_addr_relay_enabled` ([source](45b48dae0a/src/net_processing.cpp (L321-L322))) to account for some `CConnman` logic being moved into `PeerManager` ([source](45b48dae0a/src/net_processing.cpp (L2186-L2195))), which, in a way, reverts [dash#5339](https://github.com/dashpay/dash/pull/5339) but also, doesn't, since it moves the information into `Peer` instead of reinstating it into `CNode`.
* This was eventual since the underlying presumption that `CNode::IsAddrRelayPeer() == !CNode::IsBlockOnlyConn()` no longer holds true (also because `CNode::IsAddrRelayPeer()` doesn't exist anymore).
Special thanks to @UdjinM6 for help with understanding Dash-specifics with respect to functional tests through help on [dash#5964](https://github.com/dashpay/dash/pull/5964) and [dash#5967](https://github.com/dashpay/dash/pull/5967)
## Breaking Changes
None expected.
RPC changes have been introduced in `getnodeaddresses`, where a new input `network`, can filter addresses based on desired network and a new output, also `network`, will associate the address with the origin network. This change is expected to be backwards-compatible.
## 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:
PastaPastaPasta:
utACK 1fedf470cd
Tree-SHA512: 533d33f79a0d9fd730073b3b9a58baf1dd3b0c95823e765c88a43cc974970ed3609bf1863c63ac7fc5586d1437e5250b0a2d3005468da09e407110a412bd0264
21ad71c578 Merge #21676: test: Use mocktime to avoid intermittent failure in rpc_tests (MarcoFalke)
76a41eb245 Merge #21602: rpc: add additional ban time fields to listbanned (MarcoFalke)
adea52a5fe Merge bitcoin-core/gui#260: Handle exceptions instead of crash (W. J. van der Laan)
7e023c394f Merge #17934: doc: Use CONFIG_SITE variable instead of --prefix option (fanquake)
bc6e3ed6e4 Merge #21606: fuzz: Extend psbt fuzz target a bit (MarcoFalke)
233fb245f7 Merge #21445: cirrus: Use SSD cluster for speedup (fanquake)
a224b800e4 Merge #21609: ci: increase CPU count of sanitizer job to increase memory limit (MarcoFalke)
ad947099a0 test: remove exception for util::Ref which doesn't exist more (Konstantin Akimov)
6674ee85ab Merge #21390: test: Test improvements for UTXO set hash tests (MarcoFalke)
e10eec249b Merge #21338: test: add functional test for anchors.dat (MarcoFalke)
d9c31d6817 Merge #21411: test: add logging, reduce blocks, move sync_all in wallet_ groups (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Regular backports from bitcoin v22
## Note for reviewers:
PRs bitcoin#17934 and bitcoin#21606 have been backported partially in past.
## What was done?
Removed unused sanitizer rules (see bitcoin#21366 and dashpay/dash#5055)
- bitcoin/bitcoin#21338
- bitcoin/bitcoin#21390
- bitcoin/bitcoin#21609
- bitcoin/bitcoin#21445
- bitcoin/bitcoin#21606
- bitcoin/bitcoin#17934
- bitcoin-core/gui#260
- bitcoin/bitcoin#21602
- bitcoin/bitcoin#21676
## How Has This Been Tested?
Run unit/functional tests
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 21ad71c578
Tree-SHA512: 94276d56255300d7d8c056d15b468720ba028d83cc585b16396e8bad90157e9e010490be239e09cccd4362f575f8df2d56dde3fb505745376c7790d70e3635cd
d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec doc: release notes for new listbanned fields (Jarol Rodriguez)
60290d3f5ec8e7e3b8cb1ebae02d5d72f6005184 test: increase listbanned unit test coverage (Jon Atack)
3e978d1a5dbd43f85bd03e759984ab1f209d6e34 rpc: add time_remaining field to listbanned (Jarol Rodriguez)
5456b345312857981cb426712f0665800c682e09 rpc: add ban_duration field to listbanned (Jarol Rodriguez)
c95c61657afd058b46549fb3d65633d7c736f5fc doc: improve listbanned help (Jarol Rodriguez)
dd3c8eaa3399b28dc78a883ff78cbe7cc5c31b5b rpc: swap position of banned_until and ban_created fields (Jarol Rodriguez)
Pull request description:
This PR adds a `ban_duration` and `time_remaining` field to the `listbanned` RPC command. Thanks to jonatack, this PR also expands the `listbanned` test coverage to include these new fields
It's useful to keep track of `ban_duration` as this is another data point on which to sort banned peers. I found this helpful in adding additional context columns to the GUI `bantablemodel` as part of a follow-up PR. As [suggested](https://github.com/bitcoin/bitcoin/pull/21602#issuecomment-813486134) by jonatack, `time_remaining` is another useful user-centric data point.
Since a ban always expires after its created, the `ban_created` field is now placed before the `banned_until` field. This new ordering is more logical.
This PR also improves the `help listbanned` output by providing additional context to the descriptions of the `address`, `ban_created`, and `banned_until` fields.
**Master: listbanned**
```
[
{
"address": "1.2.3.4/32",
"banned_until": 1617691101,
"ban_created": 1617604701
},
{
"address": "135.181.41.129/32",
"banned_until": 1649140716,
"ban_created": 1617604716
}
]
```
**PR: listbanned**
```
[
{
"address": "1.2.3.4/32",
"ban_created": 1617775773,
"banned_until": 1617862173,
"ban_duration": 86400,
"time_remaining": 86392
},
{
"address": "3.114.211.172/32",
"ban_created": 1617753165,
"banned_until": 1618357965,
"ban_duration": 604800,
"time_remaining": 582184
}
]
```
ACKs for top commit:
jonatack:
re-ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec
hebasto:
ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec, tested on Linux Mint 20.1 (x86_64).
MarcoFalke:
review ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec 🕙
Tree-SHA512: 5b83ed2483344e546d57e43adc8a1ed7a1fff292124b14c86ca3a1aa2aec8b0f7198212fabff2c5145e7f726ca04ae567fe667b141254c7519df290cf63774e5
223b1ba7d90509a47ea07af46f4b9c3b8efbc9f8 doc: Use CONFIG_SITE instead of --prefix (Hennadii Stepanov)
Pull request description:
The current examples of `--prefix=...` option usage to point `configure` script to appropriate `depends` directory is not [standard](https://www.gnu.org/prep/standards/html_node/Directory-Variables.html). This causes some [confusion](https://github.com/bitcoin/bitcoin/pull/16691) and a bit of inconvenience.
Consider a CentOS 7 32 bit system. Packages `libdb4-devel`, `libdb4-cxx-devel`, `miniupnpc-devel` and `zeromq-devel` are unavailable from repos. After recommended build with depends:
```
cd depends
make
cd ..
./autogen.sh
./configure --prefix=$PWD/depends/i686-pc-linux-gnu
make
```
a user is unable to `make install` compiled binaries neither locally (to `~/.local`) nor system-wide (to `/usr/local`) as `--prefix` is set already.
Meanwhile, the standard approach with using [`config.site`](https://www.gnu.org/software/automake/manual/html_node/config_002esite.html) files allows both possibilities:
```
cd depends
make
cd ..
./autogen.sh
CONFIG_SITE=$PWD/depends/i686-pc-linux-gnu/share/config.site ./configure --prefix ~/.local
make
make install
```
or
```
CONFIG_SITE=$PWD/depends/i686-pc-linux-gnu/share/config.site ./configure
make
sudo make install # install to /usr/local
```
Moreover, this approach is used in [Gitian descriptors](https://github.com/bitcoin/bitcoin/tree/master/contrib/gitian-descriptors) already.
ACKs for top commit:
practicalswift:
ACK 223b1ba7d90509a47ea07af46f4b9c3b8efbc9f8: patch looks correct
fanquake:
ACK 223b1ba7d90509a47ea07af46f4b9c3b8efbc9f8
Tree-SHA512: 46d97924f0fc7e95ee4566737cf7c2ae805ca500e5c49af9aa99ecc3acede4b00329bc727a110aa1b62618dfbf5d1ca2234e736f16fbdf96d6ece5f821712f54
e1604b3d50dca3291a432be59cfd03c0e846e7b2 doc: Replace tabs for spaces (Gunar C. Gessner)
98db48d3490e5863b4d89e03cebeece9bd1f91ae doc: Fix markdown formatting (Gunar Gessner)
Pull request description:
Lines were being joined making it hard to read.
ACKs for top commit:
RandyMcMillan:
ACK e1604b3d50dca3291a432be59cfd03c0e846e7b2
Tree-SHA512: fd5a7c5e9a1cbbf0fbb13b5c30b87853c84751da7f0fad08151bda07f1933872ab51cad29a0c0a70ced48e60df6d83bff3f84c2f77d00d22723fae9a8c3534fc
7f57516d04 docs: add v20.0.4 release notes (pasta)
Pull request description:
## Issue being fixed or feature implemented
We didn't archive v20.0.4 release notes somehow in #5910
## What was done?
Archive 20.0.4 release notes
## How Has This Been Tested?
## Breaking Changes
None
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] 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)_
Top commit has no ACKs.
Tree-SHA512: d703dedf1aad2afb5eb8af291f89e38fa2dfb4185ef9e25194c273bb9f04ae8075ea87318552b468b1a487b39b95cf1dee794d3d3c5b8a863dbfd6d458ee8e72
32045bbfd5d77513efc162be8d4e24ea67539e27 [doc] Tidy up Tor doc (more stringent) (wodry)
Pull request description:
This is a follow up to https://github.com/bitcoin/bitcoin/pull/19638 that left two deprecated "hidden service/server" naming occurences.
It also shall make the chapter titles regarding creation of onion services stringent and easy to read and distinguish.
It removes the one and only reference to the testnet (here the testnet onion service port), as it is not explained that it references to the testnet and I do not know why it is mentioned there. It is only confusing. Also, as said, the testnet is not referenced at any other place in this document.
ACKs for top commit:
practicalswift:
ACK 32045bbfd5d77513efc162be8d4e24ea67539e27
laanwj:
Review ACK 32045bbfd5d77513efc162be8d4e24ea67539e27
RiccardoMasutti:
ACK 32045bb
Tree-SHA512: c623387b76d68845c0fa47f67a6f8ef70c9c560e3f8f8754e45a4f51e43198c2092be789588acd4ada607f42fbe62d51a4b1888d81b225df19b6557a081835c0
1d578c078f0ce00cb032d3c6c689fd199b8d2f35 doc: Add bash as an OpenBSD dependency (Emil Engler)
Pull request description:
If we require Python for the test framework, we should also require
bash. It is required for the linters and other scripts and does not
comes in a default OpenBSD installation.
ACKs for top commit:
practicalswift:
ACK 1d578c078f0ce00cb032d3c6c689fd199b8d2f35
RiccardoMasutti:
ACK 1d578c0
theStack:
ACK 1d578c078f0ce00cb032d3c6c689fd199b8d2f35
Tree-SHA512: ef14e801983093a99d4c28d134adbc589ca06e5437bf1ff34f8e593968c59793e9d99e8a48f577f847acdfc5185e193276526894b4c632c59d66d87939977910
dc80a7d0b0817b43d41af3a08b5800c425ebd5d8 docs/descriptors.md: Remove hardened marker in the path after xpub (Dmitry Petukhov)
Pull request description:
As described in "Key origin identification" section, a descriptor
that has hardened derivation after xpub does not let you compute scripts
without access to the corresponding private keys. Such a descriptor is
practically useless.
The text after the descriptor said "with child key *1'/2* of the
specified xpub", and clearly an xpub cannot have "child key" with
hardened derivation. Therefore it makes sense to fix this inconsistency
to not confuse the reader of the doc
Top commit has no ACKs.
Tree-SHA512: 9f35951d90868585303681c27ea2ef9d36d4036181e337cfbd48730de0c346320b08dd089350b116d4c8542f3b506868cf318796bb713e5f80600ccbd96f9970
95487b055328b590ba83f258de9637ab0f9a2f17 doc: Drop mentions of Travis CI as it is no longer used (Hennadii Stepanov)
09d105ef0f8b4b06bf248721a1209c9e16e9db75 ci: Drop travis_fold feature as Travis CI is no longer used (Hennadii Stepanov)
Pull request description:
As Travis CI is no longer used, this PR:
- drops `travis_fold` feature
- drops mentions of Travis CI in docs
ACKs for top commit:
MarcoFalke:
ACK 95487b055328b590ba83f258de9637ab0f9a2f17
Tree-SHA512: 2e259bb8b1e37bcefc1251737bb2716f06ddb57c490010b373825c4e70f42ca38efae69a2f63f21f577d7cee3725b94097bdddbd313f8ebf499281cf97c53cef
41d55d30579358c805036201664ad6a1c1d48681 doc: getpeerinfo banscore deprecation release note (Jon Atack)
dd54e3796e633cfdf6954af306afd26eadc25116 test: getpeerinfo banscore deprecation test (Jon Atack)
8c7647b3fbbab03ea84071cf3cd2d0d2bf8be255 rpc: deprecate banscore field in rpc getpeerinfo (Jon Atack)
Pull request description:
Per https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592, this PR deprecates returning the `banscore` field in the `getpeerinfo` RPC, updates the help, adds a test, and updates the release notes. Related to #19464.
ACKs for top commit:
fanquake:
ACK 41d55d30579358c805036201664ad6a1c1d48681
Tree-SHA512: 8eca08332581e2fe191a2aafff6ba89ce39413f0491ed0de8b86577739f0ec430b1a8fbff2914b0f3138a229563dfcc1981c0cf5b7dd6061b5c48680a28423bc
b9e76f1bf08c52fcd402b2314e00db4ad247ebc8 rpc: Add test for -rpcwaittimeout (Christian Decker)
f76cb10d7dc9a7b0c55d28011161606399417664 rpc: Prefix rpcwaittimeout error with details on its nature (Christian Decker)
c490e17ef698a1695050f82ef6567b3b87a21861 doc: Add release notes for the `-rpcwaittimeout` cli parameter (Christian Decker)
a7fcc8eb59fe51473571661316214156fbdbdcae rpc: Add a `-rpcwaittimeout` parameter to limit time spent waiting (Christian Decker)
Pull request description:
Adds a new numeric `-rpcwaittimeout` that can be used to limit the
time we spend waiting on the RPC server to appear. This is used by
downstream projects to provide a bit of slack when `bitcoind`s RPC
interface is not available right away.
This makes the `-rpcwait` argument more useful, since we can now limit
how long we'll ultimately wait, before potentially giving up and reporting
an error to the caller. It was discussed in the context of the BTCPayServer
wanting to have c-lightning wait for the RPC interface to become available
but still have the option of giving up eventually ([4355]).
I checked with laanwj whether this is already possible ([comment]), and
whether this would be a welcome change. Initially I intended to repurpose
the (optional) argument to `-rpcwait`, however I decided against it since it
would potentially break existing configurations, using things like `rpcwait=1`,
or `rpcwait=true` (the former would have an unintended short timeout, when
old behavior was to wait indefinitely).
~Due to its simplicity I didn't implement a test for it yet, but if that's desired I
can provide one.~ Test was added during reviews.
[4355]: https://github.com/ElementsProject/lightning/issues/4355
[comment]: https://github.com/ElementsProject/lightning/issues/4355#issuecomment-768288261
ACKs for top commit:
laanwj:
Code review ACK b9e76f1bf08c52fcd402b2314e00db4ad247ebc8
promag:
ACK b9e76f1bf08c52fcd402b2314e00db4ad247ebc8.
Tree-SHA512: 3cd6728038ec7ca7c35c2e7ccb213bfbe963f99a49bb48bbc1e511c4dd23d9957c04f9af1f8ec57120e47b26eaf580b46817b099d5fc5083c98da7aa92db8638
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
