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>
9c54cb16de Merge #19405: rpc, cli: add network in/out connections to `getnetworkinfo` and `-getinfo` (Wladimir J. van der Laan)
19aba38cab Merge #19200: rpc: remove deprecated getaddressinfo fields (Samuel Dobson)
f5642281cc Merge #20282: wallet: change upgradewallet return type to be an object (Samuel Dobson)
Pull request description:
## Issue being fixed or feature implemented
Bitcoin backports with breaking changes
## What was done?
- bitcoin/bitcoin#20282
- bitcoin/bitcoin#19200
- bitcoin/bitcoin#19405
## How Has This Been Tested?
Run unit/functional tests
## Breaking Changes
### RPC:
- `upgradewallet` now returns object for future extensibility (#20282)
- `getnetworkinfo` now returns fields `connections_in`, `connections_out`,
`connections_mn_in`, `connections_mn_out`, `connections_mn`
that provide the number of inbound and outbound peer
connections. These new fields are in addition to the existing `connections`
field, which returns the total number of peer connections. Old fields
`inboundconnections`, `outboundconnections`, `inboundmnconnections`,
`outboundmnconnections` and `mnconnections` are removed (#19405)
- Backwards compatibility has been dropped for two `getaddressinfo` RPC
deprecations, as notified in the 19.1.0 and 19.2.0 release notes.
The deprecated `label` field has been removed as well as the deprecated `labels` behavior of
returning a JSON object containing `name` and `purpose` key-value pairs. Since
20.1, the `labels` field returns a JSON array of label names. (#19200)
### CLI
- The `connections` field of `bitcoin-cli -getinfo` is expanded to return a JSON
object with `in`, `out` and `total` numbers of peer connections and `mn_in`,
`mn_out` and `mn_total` numbers of verified mn connections. It previously
returned a single integer value for the total number of peer connections. (#19405)
## 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 9c54cb16de
Tree-SHA512: 9710cdd062d02d64e2eebcecca1b5c2e6ccda5ca6d9bd6d1833700f4273fcfb206ae99134f71bbc8b0843cb8ebba208c72139f5a624d79ec7362bd73b117bfb2
581b343d5bf517510ab0236583ca96628751177d Add in/out connections to cli -getinfo (Jon Atack)
d9cc13e88d096c1a171159c01cbb96444f7f8d7f UNIX_EPOCH_TIME fixup in rpc getnettotals (Jon Atack)
1ab49b81cf32b6ef9e312a0a8ac45c68a3262f0d Add in/out connections to rpc getnetworkinfo (Jon Atack)
Pull request description:
This is basic info that is present in the GUI that I've been wishing to have exposed via the RPC and CLI without needing a bash workaround or script. For human users it would also be useful to have it in `-getinfo`.
`bitcoin-cli getnetworkinfo`
```
"connections": 15,
"connections_in": 6,
"connections_out": 9,
```
`bitcoin-cli -getinfo`
```
"connections": {
"in": 6,
"out": 9,
"total": 15
},
```
Update the tests, RPC help, and release notes for the changes. Also fixup the `getnettotals` timemillis help while touching `rpc/net.cpp`.
-----
Reviewers can manually test this PR by [building from source](https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests), launching bitcoind, and then running `bitcoin-cli -getinfo`, `bitcoin-cli getnetworkinfo`, `bitcoin-cli help getnetworkinfo`, and `bitcoin-cli help getnettotals` (for the UNIX epoch time change).
ACKs for top commit:
eriknylund:
> tACK [581b343](581b343d5b) on master at [a0a422c](a0a422c34c), ran unit & functional tests and and confirmed changes on an existing datadir ✌️
benthecarman:
tACK `581b343`
willcl-ark:
tACK for 581b343d5bf517510ab0236583ca96628751177d, this time rebased onto master at 862fde88be706adb20a211178253636442c3ae00.
shesek:
tACK `581b343`. This provides what I needed, thanks!
n-thumann:
tACK 581b343 on master at a0a422c, ran unit & functional tests and and confirmed changes on an existing datadir ✌️
Tree-SHA512: 08dd3ac8fefae401bd8253ff3ac027603c528eeccba53cedcb127771316173a7052fce44af8fa33ac98ebc4cf2a2b11cdefd949995d55e9b9a5942b876d00dc5
BACKPORT NOTICE:
These backports #17578 and #17585 are included to 19.1 and 19.2. That's long enough!
------------------------------------------
bc01f7ae0538d3c647ce8dfbc29f7914d5df3fbb doc: release note for rpc getaddressinfo removals (Jon Atack)
90e989390ee50633fff0e4f210a1ea23ff00e012 rpc: getaddressinfo RPCResult fixup (Jon Atack)
a8507c99da10791aa69ca277128e06753942e976 rpc: remove deprecated getaddressinfo `labels: purpose` (Jon Atack)
645a8653c895e4fc7717e9e5ac045612b5deaa60 rpc: remove deprecated getaddressinfo `label` field (Jon Atack)
Pull request description:
These were deprecated in #17578 and #17585, with expected 0.21 removal notified in the 0.20 release notes.
```
- The `getaddressinfo` RPC has had its `label` field deprecated
(re-enable for this release using the configuration parameter
`-deprecatedrpc=label`). The `labels` field is altered from returning
JSON objects to returning a JSON array of label names (re-enable
previous behavior for this release using the configuration parameter
`-deprecatedrpc=labelspurpose`). Backwards compatibility using the
deprecated configuration parameters is expected to be dropped in the
0.21 release. (#17585, #17578)
```
ACKs for top commit:
Sjors:
utACK bc01f7a
adamjonas:
utACK bc01f7a
meshcollider:
utACK bc01f7ae0538d3c647ce8dfbc29f7914d5df3fbb
Tree-SHA512: ae1af381e32c4c3bde8b061a56382838513a9a82c88767843cdeae3a2ab8aa7d8c2e66e106d2b31ea07d74bb80c191a2f842c9aaecc7c5438ad9a9bc66d1b251
2ead31fb1b17c9b183a4b81f0ae4f48e5cf67d64 [wallet] Return object from upgradewallet RPC (Sishir Giri)
Pull request description:
Change the return type of upgradewallet to be an object for future extensibility.
Also return any error string returned from the `UpgradeWallet()` function.
ACKs for top commit:
MarcoFalke:
ACK 2ead31fb1b17c9b183a4b81f0ae4f48e5cf67d64
meshcollider:
Tested ACK 2ead31fb1b17c9b183a4b81f0ae4f48e5cf67d64
Tree-SHA512: bcc7432d2f35093ec2463ea19e894fa885b698c0e8d8e4bd2f979bd4d722cbfed53ec589d6280968917893c64649dc9e40800b8d854273b0f9a1380f51afbdb1
ca2a09640fe976b1e74a33d29d9381895e71b347 Change SetType to SetInternal and remove m_address_type (Andrew Chow)
89b1ce1140535b4c902a7c5999bed335b9ddfe7c Remove unimplemented SetCrypted from DescriptorScriptPubKeyMan (Andrew Chow)
b9073c8f13fb0ba94c2ec6365666343e19fd9ddf rpc: createwallet warning that descriptor wallets are experimental (Andrew Chow)
610030d95c60ea526440d801a98ac8bd370eac48 docs: Add release notes for descriptor wallets (Andrew Chow)
Pull request description:
Some docs and cleanup following #16528.
* Added release notes to explain a bit of motivation for descriptor wallets, what was changed, and how users will be effected by it. Also mentions the caveats regarding multsigs and watchonly that we have discussed on IRC.
* Adds a warning to `createwallet` that descriptor wallets are experimental.
* Removed unused `SetCrypted` as suggestioned: https://github.com/bitcoin/bitcoin/pull/16528#discussion_r415300916
* Removed `m_address_type` as mentioned in https://github.com/bitcoin/bitcoin/pull/18782#issuecomment-620167077
ACKs for top commit:
Sjors:
tACK ca2a09640fe976b1e74a33d29d9381895e71b347
instagibbs:
utACK ca2a09640f
meshcollider:
utACK ca2a09640fe976b1e74a33d29d9381895e71b347
Tree-SHA512: 987188a912c191430e5d3f89bcef54ba6773692fc2d95b16a3ec11d9007ded210466ed980a3857e8b7196beef6422f07f9c85cc157f996c02d16f4dbde2e7b2a
2aac093a3d60e446b85eebdf170ea6bed77bec92 test: Add test coverage for -networkactive option (Hennadii Stepanov)
3c58129b1293742a49aa196cb210ff345a7339e6 net: Log network activity status change unconditionally (Hennadii Stepanov)
62fe6aa87e4cdd8b06207abc1387c68d7bfc04c1 net: Add -networkactive option (Hennadii Stepanov)
Pull request description:
Some Bitcoin Core activity is completely local (offline), e.g., reindexing.
The `setnetworkactive` RPC command is already present. This PR adds the corresponding command-line argument / config option, and allows to start the client with disabled p2p network by providing `-networkactive=0` or `-nonetworkactive`.
This was done while reviewing #16981.
ACKs for top commit:
MarcoFalke:
re-ACK 2aac093a3d60e446b85eebdf170ea6bed77bec92 🏠
LarryRuane:
ACK 2aac093a3d60e446b85eebdf170ea6bed77bec92
Tree-SHA512: 446d791b46d7b556d7694df7b1f88cd4fbc09301fe4eaf036b45cb8166ed806156353cc03788a07b633d5887d5eee30a7c02a2d4307141c8ccc75e0a88145636
c456302d4258e3abc4b8afde20fba808632771b2 doc: minor improvements in getutxos REST endpoint synopsis (Sebastian Falbesoner)
Pull request description:
Describing an optional sub-path as `<checkmempool>` in the synopsis could be misleading as the angle brackets normally indicate that the field has to be replaced a custom value. Clarify that by showing two variants instead, similar to the `block` endpoint with the `notxdetails` option:
```
#### Blocks
`GET /rest/block/<BLOCK-HASH>.<bin|hex|json>`
`GET /rest/block/notxdetails/<BLOCK-HASH>.<bin|hex|json>`
```
Further improvements:
- uppercase `<TXID>` and `<N>`, to match the description of the other endpoints
- s/getutxo command/getutxos endpoint/
- describe what the `checkmempool` option does
- s/serialisation/serialization/ (the US spelling is more dominant than the UK spelling in the project, and there is indeed no other instance of the string "serialis*" in the source tree, except once in a release note)
- link to BIP64 within the text instead of only showing bare URL
- mention that BIP64 is only relevant for `bin` and `hex` output formats
- show two endpoint formats of the block section as list
ACKs for top commit:
stickies-v:
ACK c456302d4258e3abc4b8afde20fba808632771b2 - also checked that current master (cc12b8947) doesn't have any other lines changes that would require updates as per the outlined improvement points.
Tree-SHA512: b025aac0812397f5fbf78c805c13aeb5afa6862a049d13c0b101178799cdaff1ccd3abc368a5c103ea6ebf17cdff76584c54638d0f8d303d81ade2d71443d305
0811cbfc2868ee80c522fd426f188f10b06cd421 doc: add info about status code 404 for some rest endpoints (brunoerg)
Pull request description:
This PR adds an explanation about status code 404 for 2 endpoints (`/rest/tx/ `and `/rest/blockhashbyheight/`) in`REST-interface.md`. There are other endpoints that already cover it.
ACKs for top commit:
[deleted]:
reACK 0811cbfc28
shaavan:
ACK 0811cbfc2868ee80c522fd426f188f10b06cd421
Tree-SHA512: a01ac6653f706b7a7e4a4679a2b81e448381f31460ac4bcfc179af6186401cffae7b49a82f3a52c89e556acd5c16c159ce752c7a678177900ddf2e4e5c72fe6b
14093d5d243f6eb9cfef721c80f92848d95032ee doc: add distcc to productivity notes (Sjors Provoost)
Pull request description:
If you have more than one computer at your disposal, you can use [distcc](https://www.distcc.org) to speed up compilation.
ACKs for top commit:
laanwj:
ACK 14093d5d243f6eb9cfef721c80f92848d95032ee
brunoerg:
ACK 14093d5d243f6eb9cfef721c80f92848d95032ee
w0xlt:
ACK 14093d5d24
Tree-SHA512: 2c436bdea5ab750330055778eb5817361d16b046f219d53692577439e2fd8403febf78ac8e8b20ed158c650c76252b50cfc91f4ec8375cdd522cc408068d547b
## Issue being fixed or feature implemented
On my local kubuntu linters have way too much spam
## What was done?
See each commit
## How Has This Been Tested?
Run locally. Amount of warnings decreased from thousands to fewer
amount. Excluding typos, they are:
```
src/coinjoin/client.cpp:1420:5: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/coinjoin/client.cpp:1426:5: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/coinjoin/client.cpp:655:26: warning: Consider using std::copy_if algorithm instead of a raw loop. [useStlAlgorithm]
src/coinjoin/server.cpp:593:33: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/coinjoin/server.cpp:630:106: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/governance/governance.cpp:1057:9: warning: C-style pointer casting [cstyleCast]
src/governance/governance.cpp:1068:9: warning: C-style pointer casting [cstyleCast]
src/governance/governance.cpp:1079:13: warning: C-style pointer casting [cstyleCast]
src/governance/governance.cpp:1086:9: warning: C-style pointer casting [cstyleCast]
src/governance/governance.cpp:1094:9: warning: C-style pointer casting [cstyleCast]
src/governance/governance.cpp:1099:5: warning: C-style pointer casting [cstyleCast]
src/governance/governance.cpp:1486:34: warning: Consider using std::copy_if algorithm instead of a raw loop. [useStlAlgorithm]
src/llmq/commitment.cpp:102:5: warning: Consider using std::all_of or std::none_of algorithm instead of a raw loop. [useStlAlgorithm]
src/llmq/instantsend.cpp:820:38: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/llmq/quorums.cpp:831:102: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/llmq/quorums.h:300:17: warning: C-style pointer casting [cstyleCast]
src/llmq/quorums.h:301:17: warning: C-style pointer casting [cstyleCast]
src/llmq/quorums.h:302:17: warning: C-style pointer casting [cstyleCast]
src/llmq/quorums.h:303:17: warning: C-style pointer casting [cstyleCast]
src/spork.cpp:119:58: warning: Consider using std::any_of algorithm instead of a raw loop. [useStlAlgorithm]
src/statsd_client.cpp:234:63: warning: C-style pointer casting [cstyleCast]
Advice not applicable in this specific case? Add an exception by updating
IGNORED_WARNINGS in test/lint/lint-cppcheck-dash.sh
^---- failure generated from test/lint/lint-cppcheck-dash.sh
Consider install flake8-cached for cached flake8 results.
test/functional/data/invalid_txs.py: error: Source file found twice under different module names: "invalid_txs" and "data.invalid_txs"
test/functional/data/invalid_txs.py: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info
test/functional/data/invalid_txs.py: note: Common resolutions include: a) adding `__init__.py` somewhere, b) using `--explicit-package-bases` or adjusting MYPYPATH
Found 1 error in 1 file (errors prevented further checking)
^---- failure generated from test/lint/lint-python.s
```
## 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
These changes were introduced in bitcoin#15514 (Update Transifex links),
which ordinarily do not apply to Dash as it uses its own Transifex
account but not mentioned in the name are updates to Doxygen URLs.
