… 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)_
## 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
Once Platform is live, there could be an edge case where the CL could
arrive to an EvoNode faster through Platform quorum than regular P2P
propagation.
## What was done?
This PR introduces a new RPC `submitchainlock` with the following 3
mandatory parameters:
- `blockHash`, `signature` and `height`.
Besides some basic tests:
- If the block is unknown then the RPC returns an error (could happen if
the node is stucked)
- If the signature is not verified then the RPC return an error.
- If the node already has this CL, the RPC returns true.
- If the node doesn't have this CL, it inserts it, broadcast it through
the inv system and return true.
## How Has This Been Tested?
`feature_llmq_chainlocks.py` was modified with the following scenario:
1. node0 is isolated from the rest of the network
2. node1 mines a new block and waits for CL
3. Make sure node0 doesn't know the new block/CL (by checking
`getbestchainlock()`)
4. CL is submitted via the new RPC on node0
5. checking `getbestchainlock()` and make sure the CL was processed +
'known_block' is false
6. reconnect node0
## 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
- [ ] 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: thephez <thephez@users.noreply.github.com>
1ea11e10acd60807a06adea5ecf553974a1b0346 doc: link to managing-wallets from doc readme (fanquake)
Pull request description:
This was forgotten in #22523.
ACKs for top commit:
achow101:
ACK 1ea11e10acd60807a06adea5ecf553974a1b0346
jarolrod:
ACK 1ea11e10acd60807a06adea5ecf553974a1b0346
Tree-SHA512: b82664b282cc0fe733b752c011621593df0f846d2188f12dbc5fedb7ffed2bd161293ce2a369ca973926030795b5f7acde7a1cbf5e337042a6f665906069c656
ce4e90629ed70f4d45906d7e79563b2f7ad6493e Document about wallet backup and restoration (lsilva01)
Pull request description:
This PR adds a document about backing up and restoring the Bitcoin Core wallet as suggested in the issue https://github.com/bitcoin/bitcoin/issues/20149 .
ACKs for top commit:
achow101:
re-ACK ce4e90629ed70f4d45906d7e79563b2f7ad6493e
prayank23:
reACK ce4e90629e
meshcollider:
re-ACK ce4e90629ed70f4d45906d7e79563b2f7ad6493e
Tree-SHA512: 68881fc1d81ff27cb59b891e6d422e303844a751afd9f4699f7ae505f204452afe9496c9be915ba94a7045f3cf3eaeb2af0e42ff2a12b4c77ef1f71a9de4faad
