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.
## Issue being fixed or feature implemented
We did not previously ship any onion seeds. This results in people
needing to use `addnode` in order to actually get connected
## What was done?
Modified seed creation process to handle a list of onion seeds.
## How Has This Been Tested?
Running with and without onlynet=onion and with dnsseed=0 and deleting
peers.dat
## Breaking Changes
None
## 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 _(for repository
code-owners and collaborators only)_
## Issue being fixed or feature implemented
HD wallets are old-existsing feature, appeared in Dash years ago, but
enabling HD wallets is not trivial task that requires multiple steps and
command line/rpc calls.
Let's have them enabled by default.
## What was done?
- HD wallets are enabled by default. Currently behavior `dashd`,
`dash-qt` are similar to run with option `-usehd=1`
- the rpc `upgradewallet` do not let to upgrade from non-HD wallet to HD
wallet to don't encourage user use non-crypted wallets (postponed till
v21)
- the initialization of ScriptPubKey is updated to be sure that encypted
HD seed is never written on disk (if passphrase is provided)
- enabled and dashified a script `wallet_upgradewallet.py` which test
compatibility between different versions of wallet
## What is not done?
- wallet tool still does not support passhprase, HD seed can appear on
disk
- there's no dialog that show user a mnemonic phrase and encourage him
to make a paper backup
Before removing a command line 'usehd' (backport bitcoin#11250) need to
make at least one major release for fail-over option (if someone wish to
use non-HD wallets only).
## How Has This Been Tested?
Run unit and functional tests.
Enabled new functional test `wallet_upgradewallet.py` that has been
backported long time ago but waited this PR to be enabled.
## Breaking Changes
HD wallets are created by default.
## 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
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
9b313dfef18792fcc36e78ef3caa693fafcce04e guix: Ensure EPOCH_SOURCE_DATE does not include GPG information (Andrew Chow)
43225f0a2a517ccd79dc49279b979ffd2eca6b85 guix: Remove extra \r from all.SHA256SUMS line ending (Andrew Chow)
d080c27066449f76bc8709fc50e422757971d2cf guix, doc: Add a note that codesigners need to rebuild after tagging (Andrew Chow)
4a466388a0092fbdf5f8969c6bfb65bf8cc962e1 guix: Allow changing the base manifest in guix-verify (Andrew Chow)
33455c76964b9e27b33e970d9722cc47657b291b guix: Make all.SHA256SUMS rather than codesigned.SHA256SUMS (Andrew Chow)
Pull request description:
`guix-verify` expects `all.SHA256SUMS` but `guix-attest` produces `codesigned.SHA256SUMS`. Since `all.SHA256SUMS` makes more sense (as the file contains all the sha256sums, not just the codesigned ones), `guix-attest` has been changed to output a file of that name.
As a quality of life improvement, `guix-verify` can take `SIGNER` and use the signer's manifest as the base to compare against. This makes it easier to compare a single person's attestations with everyone else's and can make it more obvious when one builder is clearly mismatching with everyone else.
Lastly `release-process.md` is updated with a note about a gotcha that can cause a mismatch in the codesigned attestation.
ACKs for top commit:
fanquake:
ACK 9b313dfef18792fcc36e78ef3caa693fafcce04e
Tree-SHA512: 0d60627def38288dbd3059ad1e72cad224f9205da11b1a561c082ef28250a074df5cc5f2797c91a7be027bc486a3fda3319c2e496a8724e5b539337236c6f990
… best height
## Issue being fixed or feature implemented
Platform wants to know the height of the bestchainlock when they call
submitchainlock; sooo we change the API of submitchainlock to also
return the height
## What was done?
Adjust API and tests
## How Has This Been Tested?
New tests added for this behavior
## Breaking Changes
Not really any; I **guess** that return value could be considered
breaking change; but going from nothing -> something feels unlikely to
break anything although it in theory could.
## 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)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0 [util] add RunCommandParseJSON (Sjors Provoost)
c17f54ee535faaedf9033717403e1f775b5f1530 [ci] use boost::process (Sjors Provoost)
32128ba682033560d6eb2e4848a9f77a842016d2 [doc] include Doxygen comments for HAVE_BOOST_PROCESS (Sjors Provoost)
3c84d85f7d218fa27e9343c5cd1a55e519218980 [build] msvc: add boost::process (Sjors Provoost)
c47e4bbf0b44f2de1278f9538124ec98ee0815bb [build] make boost-process opt-in (Sjors Provoost)
929cda5470f98d1ef85c05b1cad4e2fb9227e3b0 configure: add ax_boost_process (Sjors Provoost)
8314c23d7b39fc36dde8b40b03b6efbe96f85698 [depends] boost: patch unused variable in boost_process (Sjors Provoost)
Pull request description:
Prerequisite for external signer support in #16546. Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).
This adds a new dependency [boost process](https://github.com/boostorg/process/tree/boost-1.64.0). This is part of Boost since 1.64 which is part of `depends`. Because the minimum Boost version is 1.47, this functionality is skipped for older versions of Boost.
Use `./configure --with-boost-process` to opt in, which checks for the presence of Boost::Process.
We add `UniValue runCommandParseJSON(const std::string& strCommand)` to `system.{h,cpp}` which calls an arbitrary command and processes the JSON returned by it. This is currently only called by the test suite.
~For testing purposes this adds a new regtest-only RPC method `runcommand`, as well as `test/mocks/command.py` used by functional tests.~ (this is no longer the case)
TODO:
- [ ] review boost process in #15440
ACKs for top commit:
achow101:
ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0
hebasto:
re-ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, only rebased (verified with `git range-diff`) and removed an unintentional tab character since the [previous](https://github.com/bitcoin/bitcoin/pull/15382#pullrequestreview-458371035) review.
meshcollider:
Very light utACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, although I am not very confident with build stuff.
promag:
Code review ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0, don't mind the nit.
ryanofsky:
Code review ACK 31cf68a3ad1f0a5537c8419e2912b55fbfb88fa0. I left some comments below that could be ignored or followed up later. The current change is clean and comprehensive.
Tree-SHA512: c506e747014b263606e1f538ed4624a8ad7bcf4e025cb700c12cc5739964e254dc04a2bbb848996b170e2ccec3fbfa4fe9e2b3976b191222cfb82fc3e6ab182d
## Issue being fixed or feature implemented
Dashmate wanted a way to know if it is safe to restart the masternode.
This new RPC indicates the number of active DKG sessions, and the number
of blocks until next potential DKG.
## What was done?
Examples of responses:
`{'active_dkgs': 0, 'next_dkg': 22}`
## How Has This Been Tested?
`feature_llmq_rotation.py` was updated
## Breaking Changes
no
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] 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)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
RPC `getassetunlockstatuses` is now accepting an extra optional
parameter `height`.
When a valid `height` is passed, then the RPC returns the status of
AssetUnlock indexes up to this specific block. (Requested by Platform
team)
## What was done?
Note that in order to avoid cases that can lead to deterministic result,
when `height` is passed, then the only `chainlocked` and `unknown`
outcomes are possible.
## How Has This Been Tested?
`feature_asset_locks.py` was updated.
## 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 _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
5da96210fc2fda9fbd79531f42f91262fd7a9257 doc: release note for getpeerinfo last_block/last_transaction (Jon Atack)
cfef5a2c98b9563392a4a258fedb8bdc869c9749 test: rpc_net.py logging and test naming improvements (Jon Atack)
21c57bacda766a4f56ee75a2872f5d0f94e3901e test: getpeerinfo last_block and last_transaction tests (Jon Atack)
8a560a7d57cbd9f473d6a3782893a0e2243c55bd rpc: expose nLastBlockTime/TXTime as getpeerinfo last_block/transaction (Jon Atack)
02fbe3ae0bd91cbab2828cb7aa46f6493c82f026 net: add nLastBlockTime/TXTime to CNodeStats, CNode::copyStats (Jon Atack)
Pull request description:
This PR adds inbound peer eviction criteria `nLastBlockTime` and `nLastTXTime` to `CNodeStats` and `CNode::copyStats`, which then allows exposing them in the next commit as `last_transaction` and `last_block` Unix Epoch Time fields in RPC `getpeerinfo`.
This may be useful for writing missing eviction tests. I'd also like to add `lasttx` and `lastblk` columns to the `-netinfo` dashboard as described in https://github.com/bitcoin/bitcoin/pull/19643#issuecomment-671093420.
Relevant discussion at the p2p irc meeting http://www.erisian.com.au/bitcoin-core-dev/log-2020-08-11.html#l-549:
```text
<jonatack> i was specifically trying to observe and figure out how to test https://github.com/bitcoin/bitcoin/issues/19500
<jonatack> which made me realise that i didn't know what was going on with my peer conns in enough detail
<jonatack> i'm running bitcoin locally with nLastBlockTime and nLastTXTime added to getpeerinfo for my peer connections dashboard
<jonatack> sipa: is there a good reason why that (eviction criteria) data is not exposed through getpeerinfo currently?
<sipa> jonatack: nope; i suspect just nobody ever added it
<jonatack> sipa: thanks. will propose.
```
The last commit is optional, but I think it would be good to have logging in `rpc_net.py`.
ACKs for top commit:
jnewbery:
Code review ACK 5da96210fc2fda9fbd79531f42f91262fd7a9257
theStack:
Code Review ACK 5da96210fc2fda9fbd79531f42f91262fd7a9257
darosior:
ACK 5da96210fc2fda9fbd79531f42f91262fd7a9257
Tree-SHA512: 2db164bc979c014837a676e890869a128beb7cf40114853239e7280f57e768bcb43bff6c1ea76a61556212135281863b5290b50ff9d24fce16c5b89b55d4cd70
2dc79c4264d608ebe48c980f0ead54274ab3ee4f doc: Update and improve files.md (Hennadii Stepanov)
Pull request description:
This PR adds to the `files.md`:
- the `signet` subdirectory
- the `ip_asn.map` file
- some small improvements
ACKs for top commit:
practicalswift:
ACK 2dc79c4264d608ebe48c980f0ead54274ab3ee4f
MarcoFalke:
ACK 2dc79c4264d608ebe48c980f0ead54274ab3ee4f
Tree-SHA512: f645486a26293e91eda826dee46e5798af9a81be410d48d07c2714f416da19b85e7e75b1a638b0e03a3e6dc486a8bb65c4be811eb2ff51b66f5817aecf89416d
bec7f2caf76901a33dcdd2c3bf976f3954131666 doc: install qt5 when building on macOS (fanquake)
Pull request description:
Brew has updated such that qt now refers to [Qt 6.0.1](https://github.com/Homebrew/homebrew-core/blob/master/Formula/qt.rb). If builders
install this, configure will not pick up qt. For now, install
[qt@5 (5.15.2)](https://github.com/Homebrew/homebrew-core/blob/master/Formula/qt@5.rb), until required build system and likely source changes
are made.
ACKs for top commit:
hebasto:
ACK bec7f2caf76901a33dcdd2c3bf976f3954131666, tested on Tested on macOS Big Sur 11.2.2 (20D80).
Tree-SHA512: 86663cfbc68c8c6f5d608d60cd59b37d3faf1e7f33ae17ec2e1a7c076e835eb8200181a17575f121929ea6ecded74b1619096fe5a763106f56de0bdbea9ae4fa
56f9dba015c592b8925795012e3061a710070a27 Only relay IPv4, IPv6, Tor addresses (Pieter Wuille)
79f3d9b932bf62b90995bce1cf4b0b1f0152d26d Mention BIP155 in doc/bips.md (Pieter Wuille)
Pull request description:
This:
* Documents BIP155 support in doc/bips.md
* Restricts addrv2 relay to IPv4, IPv6, and Tor addresses. Relaying addresses in ranges that no network software has support for seems like a gratuitous spam vector.
ACKs for top commit:
jonatack:
ACK 56f9dba015c592b8925795012e3061a710070a27
naumenkogs:
ACK 56f9dba
hebasto:
ACK 56f9dba015c592b8925795012e3061a710070a27, verified both links.
Tree-SHA512: f0a2072b3d84a05cdbc7b961c18d7322a2e7260517f5306599ff52d8c728f9167de0a59a6d66cb95d84d69f3028680ce8bd05dab0db8c4f97938a287e5ce9631
BACKPORT NOTICE
There's some extra changes that has been forgotten due to skipped bitcoin#17002 (which is DNM)
-------------
e5f3e95a8e277acc54bc377a6b116d60d8c4eb5c doc: fix getchaintxstats fields in release-process.md (Jon Atack)
Pull request description:
ISTM the getchaintxstats fields should be `window_final_block_hash` rather than `window_last_block_hash`. While here, replace getblockchaininfo with getblockheader (and getblockhash) instead of getblockchaininfo for updating the nMinimumChainWork and defaultAssumeValid consensus params, update the example PR, and improve a link with a named anchor tag.
Markdown rendering here: https://github.com/jonatack/bitcoin/blob/release-process-getchaintxstats-fix/doc/release-process.md
ACKs for top commit:
theStack:
re-ACK e5f3e95a8e277acc54bc377a6b116d60d8c4eb5c
Tree-SHA512: 48c9c65f10d65e461da8d4935af56b6c67e6faca94e4593237f754d8c48f03bef2b9b4a71e5d1009b215a415ba7c4c4218aca6dce97238101ca1c81f5d098bdb
fa3a7331160d1a460b1c15fca1810e98070d629c chainparams: Bump assumed chain params (MarcoFalke)
Pull request description:
As every year, reviewers get extra point when their node is running:
* `assumevalid=0`
* `checkpoints=0`
* on non-x86_64 hardware
See https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-every-major-and-minor-release for the process.
ACKs for top commit:
laanwj:
ACK fa3a7331160d1a460b1c15fca1810e98070d629c
Sjors:
ACK fa3a7331160d1a460b1c15fca1810e98070d629c for mainnet on macOS 10.14.6.
jamesob:
ACK fa3a733116
fanquake:
ACK fa3a7331160d1a460b1c15fca1810e98070d629c - checked the mainnet values. I have notes on reviewing `assumevalid` updates in [core-review](https://github.com/fanquake/core-review/blob/master/update-assumevalid.md).
Tree-SHA512: fc545ba0a7056908040b47076b393d028c1c022967c25a2074752f76f0386ef099a64445da6125117a04418bd7eb0655121bfc94e6f60b7bc2666947491b5228
6d1f51343cf11b07cd401fbd0c5bc3603e185a0e [rpc] fundrawtransaction, walletcreatefundedpsbt lock manually selected coins (Sjors Provoost)
Pull request description:
When using `fundrawtransaction` and `walletcreatefundedpsbt` with `lockUnspents`, it would only lock automatically selected coins, not manually selected coins. That doesn't make much sense to me if the goal is to prevent accidentally double-spending yourself before you broadcast a transaction.
Note that when creating a transaction, manually selected coins are automatic "unlocked" (or more accurately: the lock is ignored). Earlier versions of this PR introduced an error when a locked coin is manually selected, but this idea was abandoned after some discussion. An application that uses this RPC should either rely on automatic coin selection (with `lockUnspents`) or handle lock concurrency itself with manual coin selection. In particular it needs to make sure to avoid/pause calls with automatic coin selection between calling `lockunspent` and the subsequent spending RPC.
See #7518 for historical background.
ACKs for top commit:
meshcollider:
Code review ACK 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e
fjahr:
Code review ACK 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e
Tree-SHA512: 8773c788d92f2656952e1beac147ba9956b8c5132d474e0880e4c89ff53642928b4cbfcd1cb3d17798b9284f02618a8830c93a9f7a4733e5bded96adff1d5d4d
10b7a6d532148f880568c529e61a6d7edc7c91a9 refactor: make txmempool interface use GenTxid (Pieter Wuille)
5c124e17407a5b5824fec062b73a03a1030fa28c refactor: make FindTxForGetData use GenTxid (Pieter Wuille)
a2bfac893549e2d62708d8cda7071b4fe9750a2d refactor: use GenTxid in tx request functions (Pieter Wuille)
e65d115b725640eefb3bfa09786447816f7ca9cc test: request parents of orphan from wtxid relay peer (Anthony Towns)
900d7f6c075fd78e63503f31d267dbc16b3983d9 p2p: enable fetching of orphans from wtxid peers (Pieter Wuille)
9efd86a908cf09d9ddbadd3195f202635117d505 refactor: add GenTxid (=txid or wtxid) type and use it for tx request logic (Pieter Wuille)
d362f19355b36531a4a82094e0259f7f3db500a7 doc: list support for BIP 339 in doc/bips.md (Pieter Wuille)
Pull request description:
This is based on https://github.com/bitcoin/bitcoin/pull/18044#discussion_r450687076.
A new type `GenTxid` is added to protocol.h, which represents a tagged txid-or-wtxid. The tx request logic is updated to use these instead of uint256s, permitting per-announcement distinguishing of txid/wtxid (instead of assuming that everything we want to request from a wtxid peer is wtx). Then the restriction of orphan-parent requesting to non-wtxid peers is lifted.
Also document BIP339 in doc/bips.md.
ACKs for top commit:
jnewbery:
Code review ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9
jonatack:
ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9
ajtowns:
ACK 10b7a6d532148f880568c529e61a6d7edc7c91a9 -- code review. Using gtxid to replace the is_txid_or_wtxid flag for the mempool functions is nice.
naumenkogs:
utACK 10b7a6d
Tree-SHA512: d518d13ffd71f8d2b3c175dc905362a7259689e6022a97a0b4f14f1f9fdd87475cf5af70cb12338d1e5d31b52c12e4faaea436114056a2ae9669cb506240758b
## Issue being fixed or feature implemented
For platform needs `getrawtransactionmulti` will help to reduce amount
of rpc calls for sake of performance improvement.
## What was done?
Implemented new RPC, basic functional test, release note.
## How Has This Been Tested?
On testnet:
```
> getrawtransactionmulti '{"000000abbe61a4d9b9356cb1d7deb1132d0b444a62869e71c2f3aa8ce2361359":["6e3ef19a3f955ac75a1f84dae60d42bbe11548ef54e37033ff2d91b3c4a09e9c", "415d5fafd5ee24ada8b99c36df339785a3066170c0dca6bb1aa6a5b96cf51e35"], "0":["ec7090f01c0e9b6e29d3be8810b12c780d2fb34372a53b231ce18bb7d2f1e8b0"]}'
> getrawtransactionmulti '{"000000abbe61a4d9b9356cb1d7deb1132d0b444a62869e71c2f3aa8ce2361359":["6e3ef19a3f955ac75a1f84dae60d42bbe11548ef54e37033ff2d91b3c4a09e9c", "415d5fafd5ee24ada8b99c36df339785a3066170c0dca6bb1aa6a5b96cf51e35"], "0":["ec7090f01c0e9b6e29d3be8810b12c780d2fb34372a53b231ce18bb7d2f1e8b0"]}' true
```
## 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
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: pasta <pasta@dashboost.org>
52a797bfe5ced4329f2272be417c35730ec8839f doc: Avoid ADL for function calls (Hennadii Stepanov)
Pull request description:
It happened two times recently, when [ADL](https://en.cppreference.com/w/cpp/language/adl) popped up unexpectedly and brought some confusion:
- https://github.com/bitcoin/bitcoin/pull/24338/files#r805989994
> Any idea why this even compiles?
- https://www.erisian.com.au/bitcoin-core-dev/log-2022-02-18.html#l-51:
> 2022-02-18T03:24:14 \<dongcarl\> Does anyone know why this compiles? 6d3d2caa37
> 2022-02-18T03:24:14 \<dongcarl\> GetUTXOStatsWithHasher and MakeUTXOHasher are both in the `kernel::` namespace and I never added a `using` declaration on top...
> 2022-02-18T03:25:53 \<sipa\> https://en.cppreference.com/w/cpp/language/adl ?
Let's document our intention to avoid similar cases in the future.
ACKs for top commit:
laanwj:
Anyhow, ACK 52a797bfe5ced4329f2272be417c35730ec8839f, there is no need to hold merge up on this, documenting it is a step forward.
Tree-SHA512: f52688b5d8f6130302185206ec6ea4731b099a75294ea2d477901a52d6d58473e3427e658aea408c140c2824c37a0399ec7376aded2a91197895ea52d51f0018
01cd24c22606408d5c0ac74c9a2c5d85eff77846 doc: set CC_FOR_BUILD when building on OpenBSD (fanquake)
Pull request description:
Closes: #19559
While #19559 has been fixed upstream, it makes sense to not only
recommend using `CC_FOR_BUILD`here until the fix is pulled in as
part of our next libsecp update, but after discussing with Cory,
he suggested we should be setting this on OpenBSD (which still has
the an ancient GCC) regardless.
ACKs for top commit:
real-or-random:
ACK 01cd24c22606408d5c0ac74c9a2c5d85eff77846 I looked at the diff (but can't test the instructions on OpenBSD)
laanwj:
Code review ACK 01cd24c22606408d5c0ac74c9a2c5d85eff77846
Tree-SHA512: 322802b9303771f1be2ad9628f268dfa71dc7ee77948fa2a34f21eceb19b2d8efdd8876c8f0778adbfcde48fa0f88cd4e698ae425428159abca38e8c7980da1d
308caf326db5619141f0c224fa48410293d59330 doc: remove version number from bips.md (fanquake)
Pull request description:
This always just needs "bumping" (see previous rc type pulls), and the version number is already whichever version of the code you acquired bips.md with.
ACKs for top commit:
MarcoFalke:
lgtm ACK 308caf326db5619141f0c224fa48410293d59330
achow101:
ACK 308caf326db5619141f0c224fa48410293d59330
theStack:
ACK 308caf326db5619141f0c224fa48410293d59330
hebasto:
ACK 308caf326db5619141f0c224fa48410293d59330
Tree-SHA512: fcb98e7cdc0c1f8960bfba86be09c2badb36b613060fae394a56e1561c69d28f433434f573c8b1ae1d71ae326277dea2a4841d5c08ad39f8e8848300743146e7
e2bab2aa162ae38b2bf8195b577c982402fbee9d multiprocess: add multiprocess travis configuration (Russell Yanofsky)
603fd6a2e708c04ef6c9880f89d0a4cbaa6fc7c5 depends: add MULTIPROCESS depends option (Russell Yanofsky)
5d1377b52bfcd4edf8553aaf332bfeb92fc554cc build: multiprocess autotools changes (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
---
This PR consists of build changes only. It adds an `--enable-multiprocess` autoconf option (off by default and marked experimental), that builds new `bitcoin-node` and `bitcoin-gui` binaries. These currently function the same as existing `bitcoind` and `bitcoin-qt` binaries, but are extended in #10102 with IPC features to execute node, wallet, and gui functions in separate processes.
In addition to adding the `--enable-multiprocess` config flag, it also adds a depends package and autoconf rules to build with the [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) library, and it adds new travis configuration to exercise the build code and run functional tests with the new binaries.
The changes in this PR were originally part of #10102 but were moved into #16367 to be able to develop and review the multiprocess build changes independently of the code changes. #16367 was briefly merged and then reverted in #18588. Only change since #16367 has been dropping the `native_boost.mk` depends package which was pointed out to be no longer necessary in https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-596484337 and https://github.com/bitcoin/bitcoin/pull/18588#pullrequestreview-391765649
ACKs for top commit:
practicalswift:
ACK e2bab2aa162ae38b2bf8195b577c982402fbee9d
Sjors:
tACK e2bab2aa162ae38b2bf8195b577c982402fbee9d on macOS 10.15.4
hebasto:
ACK e2bab2aa162ae38b2bf8195b577c982402fbee9d, tested on Linux Mint 19.3 (x86_64):
Tree-SHA512: b5a76eab5abf63d9d8b6d628cbdff4cc1888eef15cafa0a5d56369e2f9d02595fed623f4b74b2cf2830c42c05a774f0943e700f9c768a82d9d348cad199e135c
c943326d3c06e481c142b112c7e7a0c6ff5a76b3 doc/bips: Add BIPs 43, 44, 49, and 84 (Luke Dashjr)
Pull request description:
If you don't like what they say, please suggest alternatives ;)
ACKs for top commit:
prusnak:
ACK c943326
Tree-SHA512: 7db93f8491289657ec45df30e557eb8572b35201eb29aed1b11bf3949924fce56b4e2d71e1f0acf5d24a01278c0dec99790d632f04c15117010c4ac564368d6b
060a2a64d40d75fecb60b7d2b9946a67e46aa6fc ci: remove boost thread installation (fanquake)
06e1d7d81d5a56d136c6fc88f09a2b0654a164f9 build: don't build or use Boost Thread (fanquake)
7097add83c8596f81be9edd66971ffd2486357eb refactor: replace Boost shared_mutex with std shared_mutex in sigcache (fanquake)
8e55981ef834490c438436719f95cbaf888c4914 refactor: replace Boost shared_mutex with std shared_mutex in cuckoocache tests (fanquake)
Pull request description:
This replaces `boost::shared_mutex` and `boost::unique_lock` with [`std::shared_mutex`](https://en.cppreference.com/w/cpp/thread/shared_mutex) & [`std::unique_lock`](https://en.cppreference.com/w/cpp/thread/unique_lock).
Even though [some concerns were raised](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-726214696) in #16684 with regard to `std::shared_mutex` being unsafe to use across some glibc versions, I still think this change is an improvement. As I mentioned in #21022, I also think trying to restrict standard library feature usage based on bugs in glibc is not only hard to do, but it's not currently clear exactly how we do that in practice (does it also extend to patching out use in our dependencies, should we be implementing more runtime checks for features we are using, when do we consider an affected glibc "old enough" not to worry about? etc). If you take a look through the [glibc bug tracker](https://sourceware.org/bugzilla/describecomponents.cgi?product=glibc) you'll no doubt find plenty of (active) bug reports for standard library code we already using. Obviously not to say we shouldn't try and avoid buggy code where possible.
Two other points:
[Cory mentioned in #21022](https://github.com/bitcoin/bitcoin/pull/21022#issuecomment-769274179):
> It also seems reasonable to me to worry that boost hits the same underlying glibc bug, and we've just not happened to trigger the right conditions yet.
Moving away from Boost to the standard library also removes the potential for differences related to Boosts configuration. Boost has multiple versions of `shared_mutex`, and what you end up using, and what it's backed by depends on:
* The version of Boost.
* The platform you're building for.
* Which version of `BOOST_THREAD_VERSION` is defined: (2,3,4 or 5) default=2. (see [here](https://www.boost.org/doc/libs/1_70_0/doc/html/thread/build.html#thread.build.configuration) for some of the differences).
* Is `BOOST_THREAD_V2_SHARED_MUTEX` defined? (not by default). If so, you might get the ["less performant, but more robust"](https://github.com/boostorg/thread/issues/230#issuecomment-475937761) version of `shared_mutex`.
A lot of these factors are eliminated by our use of depends, but users will have varying configurations. It's also not inconceivable to think that a distro, or some package manager might start defining something like `BOOST_THREAD_VERSION=3`. Boost tried to change the default from 2 to 3 at one point.
With this change, we no longer use Boost Thread, so this PR also removes it from depends, the build system, CI etc.
Previous similar PRs were #19183 & #20922. The authors are included in the commits here.
Also related to #21022 - pthread sanity checking.
ACKs for top commit:
laanwj:
Code review ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc
vasild:
ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc
Tree-SHA512: 572d14d8c9de20bc434511f20d3f431836393ff915b2fe9de5a47a02dca76805ad5c3fc4cceecb4cd43f3ba939a0508178c4e60e62abdbaaa6b3e8db20b75b03
fd0be92cff6a4b5e343e6ddae7481868354b9869 doc: Add instructions on how to fuzz the P2P layer using Honggfuzz NetDriver (practicalswift)
Pull request description:
Add instructions on how to fuzz the P2P layer using [Honggfuzz NetDriver](http://blog.swiecki.net/2018/01/fuzzing-tcp-servers.html).
Honggfuzz NetDriver allows for very easy fuzzing of TCP servers such as Bitcoin Core without having to write any custom fuzzing harness. The `bitcoind` server process is largely fuzzed without modification.
This makes the fuzzing highly realistic: a bug reachable by the fuzzer is likely also remotely triggerable by an untrusted peer.
Top commit has no ACKs.
Tree-SHA512: 9e98cb30f00664c00c8ff9fd224ff9822bff3fd849652172df48dbaeade1dd1a5fc67ae53203f1966a1d4210671b35656009a2d8b84affccf3ddf1fd86124f6e
fa362064e383163a2585ffbc71ac1ea3bcc92663 rpc: Return total fee in mempool (MarcoFalke)
Pull request description:
This avoids having to loop over the whole mempool to query each entry's fee
ACKs for top commit:
achow101:
ACK fa362064e383163a2585ffbc71ac1ea3bcc92663
glozow:
ACK fa362064e3🧸
jnewbery:
ACK fa362064e383163a2585ffbc71ac1ea3bcc92663
Tree-SHA512: e2fa1664df39c9e187f9229fc35764ccf436f6f75889c5a206d34fff473fc21efbf2bb143f4ca7895c27659218c22884d0ec4195e7a536a5a96973fc9dd82d08
3e61b8c800180d350621cedda7ec46a48047ff04 doc: Add explicit macdeployqtplus dependencies install step (Hennadii Stepanov)
Pull request description:
This PR adds to macOS docs an explicit step to install `macdeployqtplus` script dependencies that are not part of the [Python Standard Library](https://docs.python.org/3/library/index.html):
- https://pypi.org/project/ds-store/
- https://pypi.org/project/mac-alias/
This change is required on macOS 11 Big Sur:
- #20371
- #20878Close#20878.
ACKs for top commit:
fanquake:
ACK 3e61b8c800180d350621cedda7ec46a48047ff04
Tree-SHA512: d177139ee142d47cb27ad878d721cafcd03403ef861965ff532d712da461416380ec5878f70accf223a552a1f1e65eedb1e0ad72cb7a96791f8a55536ce28645
cb0b7125c14bf97394bd8b43bf2abfb943bb1cf9 doc: libbitcoinconsensus: add missing error code description, fix NBitcoin link (Sebastian Falbesoner)
Pull request description:
This PR improves the libbitcoinconsensus description in `shared-libraries.md` in two ways:
* adds the missing error code description for `bitcoinconsensus_ERR_INVALID_FLAGS` (introduced by commit 5ca8ef299a, PR #8976)
* updates and fixes the link to the NBitcoin implementation (introduced by commit 3361edd010, PR #6430)
* the owner of the `NBitcoin` github repository changed from `NicolasDorier` to `MetacoSA` (redirection still worked though)
* instead of dynamically referring to a file in master with a fixed line number (which is obviously always quickly outdated), use a permalink with a file numbers area
ACKs for top commit:
MarcoFalke:
cr ACK cb0b7125c14bf97394bd8b43bf2abfb943bb1cf9
harding:
Code (documentation) review ACK cb0b7125c14bf97394bd8b43bf2abfb943bb1cf9. Text is clear and seems accurate, and the link checks out.
Tree-SHA512: 9840458db6fb40e71c9852104aefcec5abbaf5054c6123701181dd477cea8c81d3647f376b67692159adf577c9b6305b05b784728bf9f14a753fab5898075a4e
174f58c1857468252a8b9214abd187fa296e883c Add link to NetBSD release (Marnix)
Pull request description:
For consistency with other Build Guides, like `doc/build-freebsd.md` & `doc/build-openbsd.md`
ACKs for top commit:
theStack:
Code-review ACK 174f58c1857468252a8b9214abd187fa296e883c
vincenzopalazzo:
ACK 174f58c185
Tree-SHA512: 08297aac82ee8fab2bbcb486b13b9c6ca12fb4fba573e9979e94204bd7340c2d13f1e54e07b314498603c291c25e29f2e89141ee150478951f699772538b709c
bef61496ab5e12e38ac5794cd0836723af070ab5 test: compare `/mempool/contents` response with `getrawmempool` RPC (brunoerg)
5bc5cbaf310f60e89c72e8ecf3f6187c85499027 doc: add reference to `getrawmempool` RPC in `/mempool/contents` REST doc (brunoerg)
Pull request description:
This PR is similar to #24797, it compares `/mempool/contents` REST response with `getrawmempool` RPC (verbose=True) since they use the same `MempoolToJSON` function.
Also, adds a reference to `getrawmempool` RPC help to get details about the fields from `/mempool/contents`.
ACKs for top commit:
0xB10C:
ACK bef6149
Tree-SHA512: b7e9e9c765ee837986ba167b9234a9b95c9ef0a9ebcc2a03d50f6be6d3aba1480bd77c78111d95df1e4023cde6dfc64bf1e7908d9e5b6f96ca46b76611a4a9b4
## Issue being fixed or feature implemented
Client version string is inconsistent. Building `v20.0.0-beta.8` tag
locally produces binaries that report `v20.0.0-beta.8` version but
binaries built in guix would report
`v20.0.0rc1-g3e732a952226a20505f907e4fd9b3fdbb14ea5ee` instead. Building
any commit after `v20.0.0-beta.8` locally would result in versions like
`v20.0.0rc1-8c94153d2497` which is close but it's still yet another
format. And both versions with `rc1` in their names are confusing cause
you'd expect them to mention `beta.8` instead maybe (or is it just me?
:D ).
## What was done?
Change it so that the version string would look like this:
on tag: ~`v20.0.0-beta.8-dev` or `v20.0.0-beta.8-gitarc`~
`v20.0.0-beta.8`
post-tag: ~`v20.0.0-beta.8-1-gb837e08164-gitarc`~
`v20.0.0-beta.8-1-gb837e08164`
post-tag format is
`recent tag`-`commits since that tag`-`g+12 chars of commit hash`-`dirty
(optional)` ~-`dev or gitarc`~
~`dev`/`gitarc` suffixes should help avoiding confusion with the release
versions and they also indicate the way non-release binaries were
built.~
Note that release binaries do not use any of this, they still use
`PACKAGE_VERSION` from `configure` like before.
Also, `CLIENT_VERSION_RC` is no longer used in this setup so it was
removed.
Few things aren't clear to me yet:
1. Version bump in `configure.ac` no longer affects the reported version
(unless it's an actual release). Are there any downsides I might be
missing?
2. Which tag should we use on `develop` once we bump version in
configure? `v21.0.0-init`? `v21.0.0-alpha1`?
3. How is it going to behave once `merge master back into develop` kind
of PR is merged? E.g. say `develop` branch is on `v21.0.0-alpha1` tag
and we merge v20.1.0 from `master` back into it. Will this bring
`v20.1.0` release tag into `develop`? Will it become the one that will
be used from that moment? If so we will probably need another tag on
`develop` every time such PR is merged e.g. `v21.0.0-alpha2` (or
whatever the next number is).
Don't think these are blockers but would like to hear thoughts from
others.
## How Has This Been Tested?
Built binaries locally, built them using guix at a specific tag and at
some commit on top of it.
## 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
- [ ] 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)_
d11020019a0c93dcc56859cdfcd9f0c6a777424f Add OpenBSD instructions for building the Qt GUI (grubles)
Pull request description:
Using OpenBSD as a desktop OS is prevalent enough IMO to warrant updating the documentation for building the GUI.
ACKs for top commit:
fanquake:
ACK d11020019a0c93dcc56859cdfcd9f0c6a777424f - looks fine. Have not tested.
Tree-SHA512: a8078334fdd35438bcf87c3f5eae851c2a1ce961eb48ae50770bf2c556489da86b6ee198fe9fb732dcaddb2e0f2f4f55a3126971aae8f7d4e2e320dbb024e204
7eaf86d3bfc83f2beb3ef449707d5156853126fb trivial: Suggested cleanups to surrounding code (Russell Yanofsky)
b604c5c8b5892842f13dee89ae31812a28ab25d1 wallet: Minimal fix to restore conflicted transaction notifications (Russell Yanofsky)
Pull request description:
This fix is a based on the fix by Antoine Riard (ariard) in https://github.com/bitcoin/bitcoin/pull/18600.
Unlike that PR, which implements some new behavior, this just restores previous wallet notification and status behavior for transactions removed from the mempool because they conflict with transactions in a block. The behavior was accidentally changed in two `CWallet::BlockConnected` updates: a31be09bfd77eed497a8e251d31358e16e2f2eb1 and 7e89994133725125dddbfa8d45484e3b9ed51c6e from https://github.com/bitcoin/bitcoin/pull/16624, causing issue https://github.com/bitcoin/bitcoin/issues/18325.
The change here could be improved and replaced with a more comprehensive cleanup, so it includes a detailed comment explaining future considerations.
Fixes#18325
Co-authored-by: Antoine Riard (ariard)
ACKs for top commit:
jonatack:
Re-ACK 7eaf86d3bfc83f
ariard:
ACK 7eaf86d, reviewed, built and ran tests.
MarcoFalke:
ACK 7eaf86d3bfc83f2beb3ef449707d5156853126fb 🍡
Tree-SHA512: 9a1efe975969bb522a9dd73c41064a9348887cb67883cd92c6571fd2df4321b9f4568363891abdaae14a3b9b168ef8142e95c373fc04677e46289b251fb84689
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
00124713912ead4ce610d519bb3ebab7e31cbea7 build: turn on --enable-c++17 by --enable-fuzz (Vasil Dimov)
Pull request description:
Fuzzing code uses C++17 specific code (e.g. std::optional), so it is not
possible to compile with --enable-fuzz and without --enable-c++17.
Thus, turn on --enable-c++17 whenever --enable-fuzz is used.
ACKs for top commit:
hebasto:
ACK 00124713912ead4ce610d519bb3ebab7e31cbea7, tested on Linux Mint 19.3 (x86_64); verified that it fails to compile with `--enable-fuzz` and without `--enable-c++17` on master.
Tree-SHA512: 290531ea8d79de3b9251ea4ad21e793478b18150cc0124eea1e50c3a4ed92bab89c3e70ed0aa526906f8723ea952cdba4268f1560ae4be9bd25b9e4f9b97436c
8b3f1e30f0f7bcd1a58efe29f57015ce03f64c50 Update RPC argument and field naming guideline in developer notes (Jon Atack)
Pull request description:
Clarify the doc per the IRC discussion today at https://www.erisian.com.au/bitcoin-core-dev/log-2022-04-08.html#l-229.
ACKs for top commit:
mzumsande:
Code Review ACK 8b3f1e30f0f7bcd1a58efe29f57015ce03f64c50 - I agree with the added guideline.
Tree-SHA512: d0d06bc8d9587c0dc72545843097e48a4e27a9437ceca03c71d0aa4a9b8434971014687d8d2dd012b71e92b26d4ad116697365be3f2a8ed14daecfdb1d0982ef
38a1b0b1967272cbb984f37bcc86b6640930319f doc: remove unneeded documentation on basic package management on FreeBSD (jessebarton)
Pull request description:
In reference to #24618
ACKs for top commit:
fanquake:
ACK 38a1b0b1967272cbb984f37bcc86b6640930319f - Thanks. In future, please re-use existing PRs, so that discussion and changes are kept together.
Tree-SHA512: ece5b85bca7f11e11d47c0674a6b96a72c3bb65dd02ab25553db511a001a9fc682c0ff8276e39d979fdd1f57a64137f586cfa548aab5c08cd9341455217b9181
## Issue being fixed or feature implemented
`instantlock` and `chainlock` are broken in `getspecialtxes`
kudos to @thephez for finding the issue
## What was done?
pass the hash and also rename the variable to self-describing
## How Has This Been Tested?
run `getspecialtxes` on a node with and without the patch
## Breaking Changes
`instantlock` and `chainlock` will show actual values and not just
`false` all the time now (not sure if that qualifies for "breaking"
though)
## 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 _(for repository
code-owners and collaborators only)_
## Issue being fixed or feature implemented
Platform (in the scope of Withdrawals) need to be aware if a tx isn't in
mempool when requesting status of a tx using RPC `gettxchainlocks`.
cc @markin-io
## What was done?
- mempool is passed to `GetTransaction` and saving the result for
checking latter.
- If the returned tx_ref is nullptr, then the RPC returns null for the
corresponding tx in the array.
Example:
`tx1` is mined and chainlocked, `tx2` is in mempool and `tx3` doesn't
exist.
The result now is:
`[
{
"height": 830,
"chainlock": false,
"mempool": true
},
{
"height": -1,
"chainlock": false,
"mempool": true
},
{
"height": -1,
"chainlock": false,
"mempool": false
}
]`
## How Has This Been Tested?
## Breaking Changes
## 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 _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Platform in the scope of credit withdrawals, need a way to get the
status of an Asset Unlock by index.
## What was done?
A new RPC was created `getassetunlockchainlocks` that accepts Asset
Unlock indexes array as parameter and return corresponding status for
each index.
The possible outcomes per each index are:
- `chainlocked`: If the Asset Unlock index is mined on a Chainlocked
block.
- `mined`: If no Chainlock information is available, and the Asset
Unlock index is mined.
- `mempooled`: If the Asset Unlock index is in the mempool.
- `unknown`: If none of the above are valid.
Note: This RPC is whitelisted for the Platform RPC user.
## How Has This Been Tested?
Inserted on `feature_asset_locks.py` covering cases where Asset Unlock
txs are in mempool, mined and not present.
## Breaking Changes
no
## 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)_
---------
Co-authored-by: thephez <thephez@users.noreply.github.com>
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
## Issue being fixed or feature implemented
Remove dash-qt from docker images; save ~41MB
## What was done?
## How Has This Been Tested?
Hasn't
## Breaking Changes
I guess in theory someone could've been relying on dash-qt from docker 🤷
## 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)_
