This pr makes the macOS build docs more informative and adds in the following information:
- Proper descriptions and delineation of required/optional dependencies
- walk-through of optional dependencies
- configuration walk-through
- various other tid-bits of information
4899fa3abd doc: Update build instructions for Fedora (Hennadii Stepanov)
Pull request description:
This PR updates build instructions for Fedora, as Fedora 33 has no `libdb4-devel` and `libdb4-cxx-devel` packages in its default repos.
ACKs for top commit:
practicalswift:
ACK 4899fa3abd
Tree-SHA512: 0c23e2ce0ea690ed5eeaa56514d4246e0057a77b7c71f28af4ee1e480521d465122f81cea37cc773ce2db4fc189d5ab3c8f8ffdd65f150cc006390aa1e2a4ac8
Brew has updated such that qt now refers to Qt 6.0.1. If builders
install this, configure will not work pick up qt. For now, install
qt@5 (5.15.2), until required build system and likely source changes
are made.
Introduce two new options to reach the I2P network:
* `-i2psam=<ip:port>` point to the I2P SAM proxy. If this is set then
the I2P network is considered reachable and we can make outgoing
connections to I2P peers via that proxy. We listen for and accept
incoming connections from I2P peers if the below is set in addition to
`-i2psam=<ip:port>`
* `-i2pacceptincoming` if this is set together with `-i2psam=<ip:port>`
then we accept incoming I2P connections via the I2P SAM proxy.
faa06ecc9c build: Bump minimum Qt version to 5.9.5 (Hennadii Stepanov)
Pull request description:
Close#20104.
ACKs for top commit:
laanwj:
Code review ACK faa06ecc9c
jarolrod:
ACK faa06ecc9c
fanquake:
ACK faa06ecc9c - this should be ok to do now.
Tree-SHA512: 7295472b5fd37ffb30f044e88c39d375a5a5187d3f2d44d4e73d0eb0c7fd923cf9949c2ddab6cddd8c5da7e375fff38112b6ea9779da4fecce6f024d05ba9c08
f75e0c1edd doc: add external-signer.md (Sjors Provoost)
d4b0107d68 rpc: send: support external signer (Sjors Provoost)
245b4457cf rpc: signerdisplayaddress (Sjors Provoost)
7ebc7c0215 wallet: ExternalSigner: add GetDescriptors method (Sjors Provoost)
fc5da520f5 wallet: add GetExternalSigner() (Sjors Provoost)
259f52cc33 test: external_signer wallet flag is immutable (Sjors Provoost)
2655197e1c rpc: add external_signer option to createwallet (Sjors Provoost)
2700f09c41 rpc: signer: add enumeratesigners to list external signers (Sjors Provoost)
07b7c940a7 rpc: add external signer RPC files (Sjors Provoost)
8ce7767071 wallet: add ExternalSignerScriptPubKeyMan (Sjors Provoost)
157ea7c614 wallet: add external_signer flag (Sjors Provoost)
f3e6ce78fb test: add external signer test (Sjors Provoost)
8cf543f96d wallet: add -signer argument for external signer command (Sjors Provoost)
f7eb7ecc67 test: framework: add skip_if_no_external_signer (Sjors Provoost)
87a97941f6 configure: add --enable-external-signer (Sjors Provoost)
Pull request description:
Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).
This PR lets `bitcoind` call an arbitrary command `-signer=<cmd>`, e.g. a hardware wallet driver, where it can fetch public keys, ask to display an address, and sign a transaction (using PSBT under the hood).
It's design to work with https://github.com/bitcoin-core/HWI, which supports multiple hardware wallets. Any command with the same arguments and return values will work. It simplifies the manual procedure described [here](https://github.com/bitcoin-core/HWI/blob/master/docs/bitcoin-core-usage.md).
Usage is documented in [doc/external-signer.md](
https://github.com/Sjors/bitcoin/blob/2019/08/hww-box2/doc/external-signer.md), which also describes what protocol a different signer binary should conform to.
Use `--enable-external-signer` to opt in, requires Boost::Process:
```
Options used to compile and link:
with wallet = yes
with gui / qt = no
external signer = yes
```
It adds the following RPC methods:
* `enumeratesigners`: asks <cmd> for a list of signers (e.g. devices) and their master key fingerprint
* `signerdisplayaddress <address>`: asks <cmd> to display an address
It enhances the following RPC methods:
* `createwallet`: takes an additional `external_signer` argument and fetches keys from device
* `send`: automatically sends transaction to device and waits
Usage TL&DR:
* clone HWI repo somewhere and launch `bitcoind -signer=../HWI/hwi.py`
* check if you can see your hardware device: `bitcoin-cli enumeratesigners`
* create wallet and auto import keys `bitcoin-cli createwallet "hww" true true "" true true true`
* display address on device: `bitcoin-cli signerdisplayaddress ...`
* to spend, use `send` RPC and approve transaction on device
Prerequisites:
- [x] #21127 load wallet flags before everything else
- [x] #21182 remove mostly pointless BOOST_PROCESS macro
Potentially useful followups:
- GUI support: bitcoin-core/gui#4
- bumpfee support
- (automatically) verify (a subset of) keys on the device after import, through message signing
ACKs for top commit:
laanwj:
re-ACK f75e0c1edd
Tree-SHA512: 7db8afd54762295c1424c3f01d8c587ec256a72f34bd5256e04b21832dabd5dc212be8ab975ae3b67de75259fd569a561491945750492f417111dc7b6641e77f
25c57d6409 [doc] Add a note about where lock annotations should go. (Amiti Uttarwar)
ad5f01b960 [validation] Move the lock annotation from function definition to declaration (Amiti Uttarwar)
Pull request description:
Based on reviewing #21188
the first commit switches the lock annotations on `CheckInputScripts` to be on the function declaration instead of on the function definition. this ensures that all call sites are checked, not just ones that come after the definition.
the second commit adds a note to the developer-notes section to clarify where the annotations should be applied.
ACKs for top commit:
MarcoFalke:
ACK 25c57d6409🥘
promag:
Code review ACK 25c57d6409.
Tree-SHA512: 61b6ef856bf6c6016d535fbdd19daf57b9e59fe54a1f30d47282a071b9b9d60b2466b044ee57929e0320cb1bdef52e7a1687cacaa27031bbc43d058ffffe22ba
This option replaces --with-boost-process
This prepares external signer support to be disabled by default.
It adds a configure option to enable this feature and to check
if Boost::Process is present.
This also exposes ENABLE_EXTERNAL_SIGNER to the test suite via test/config.ini
c5da2749e2 build: actually stop configure if Boost isn't available (fanquake)
cad8b527ea build: explicitly install libboost-dev package (fanquake)
Pull request description:
If Boost is not found via AX_BOOST_BASE, we don't actually stop
configuring, only a warning is emitted:
```bash
checking for boostlib >= 1.58.0 (105800)... configure: We could not detect the boost libraries (version MINIMUM_REQUIRED_BOOST or higher). If you have a staged boost library (still not installed) please specify $BOOST_ROOT in your environment and do not give a PATH to --with-boost option. If you are sure you have boost installed, then check your version number looking in <boost/version.hpp>. See http://randspringer.de/boost for more documentation.
```
Instead we usually fail when one of the other AX_BOOST_* macros fails to find a library. These macros are slowly being
removed, and in any case, it makes more sense to fail earlier if Boost is missing.
If Boost is unavailable, the failure now looks like:
```bash
checking for boostlib >= 1.58.0 (105800)... configure: We could not detect the boost libraries (version 1.58.0 or higher). If you have a staged boost library (still not installed) please specify $BOOST_ROOT in your environment and do not give a PATH to --with-boost option. If you are sure you have boost installed, then check your version number looking in <boost/version.hpp>. See http://randspringer.de/boost for more documentation.
configure: error: Boost is not available!
```
Note that we now just pass the version into AX_BOOST_BASE, which fixes it's display in the output (rather than showing `MINIMUM_REQUIRED_BOOST`).
This PR also has a commit that adds `libboost-dev` to our install instructions and CI. This package is currently installed as a side-effect of installing our other libboost-*-dev packages. However as those continue to disappear, it makes sense to install boost-dev explicitly.
ACKs for top commit:
laanwj:
Code review ACK c5da2749e2
MarcoFalke:
Concept ACK c5da2749e2
Tree-SHA512: f866062f9d7d3a2316b6c887f17c664b9cfff41fdc0cb99ca79d641240fb01a5ae0d34140e515bc465219e1b43d5ca84f7c55f48b9c5b45a80ff2795dafd072b
fd0be92cff 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
This package is currently installed as a side-effect of installing our
other libboost-*-dev packages. However as those continue to dissapear,
it makes sense to install boost dev explicitly.
aa929abf8d [docs] Update developer notes to discourage very long lines (John Newbery)
Pull request description:
Mandatory rules on line lengths are bad - there will always be cases where a longer line is more readable than the alternative.
However, very long lines for no good reason _do_ hurt readability. For example, this declaration in validation.h is 274 chars:
```c++
bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs);
```
That won't fit on one line without wrapping on my 27" monitor with a comfortable font size. Much easier to read is something like:
```c++
bool ConnectTip(BlockValidationState& state, const CChainParams& chainparams,
CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock,
ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool)
EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs);
```
Therefore, _discourage_ (don't forbid) line lengths greater than 100 characters in our developer style guide.
100 chars is somewhat arbitrary. The old standard was 80, but that seems very limiting with modern displays.
ACKs for top commit:
fanquake:
ACK aa929abf8d - this is basically just something to point too when a PR has unreasonably long lines for no particularly reason.
practicalswift:
ACK aa929abf8d
amitiuttarwar:
ACK aa929abf8d
theStack:
ACK aa929abf8d
glozow:
ACK aa929abf8d
Tree-SHA512: 17f1b11f811137497ede8851ede93fa612dc622922b5ad7ac8f065ea026d9a718db5b92325754b74d24012b4d45c4e2cd5cd439a6a8d34bbabf5da927d783970
c943326d3c 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
060a2a64d4 ci: remove boost thread installation (fanquake)
06e1d7d81d build: don't build or use Boost Thread (fanquake)
7097add83c refactor: replace Boost shared_mutex with std shared_mutex in sigcache (fanquake)
8e55981ef8 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 060a2a64d4
vasild:
ACK 060a2a64d4
Tree-SHA512: 572d14d8c9de20bc434511f20d3f431836393ff915b2fe9de5a47a02dca76805ad5c3fc4cceecb4cd43f3ba939a0508178c4e60e62abdbaaa6b3e8db20b75b03
e1604b3d50 doc: Replace tabs for spaces (Gunar C. Gessner)
98db48d349 doc: Fix markdown formatting (Gunar Gessner)
Pull request description:
Lines were being joined making it hard to read.
ACKs for top commit:
RandyMcMillan:
ACK e1604b3d50
Tree-SHA512: fd5a7c5e9a1cbbf0fbb13b5c30b87853c84751da7f0fad08151bda07f1933872ab51cad29a0c0a70ced48e60df6d83bff3f84c2f77d00d22723fae9a8c3534fc
fa362064e3 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 fa362064e3
glozow:
ACK fa362064e3🧸
jnewbery:
ACK fa362064e3
Tree-SHA512: e2fa1664df39c9e187f9229fc35764ccf436f6f75889c5a206d34fff473fc21efbf2bb143f4ca7895c27659218c22884d0ec4195e7a536a5a96973fc9dd82d08
To make release tags the `make-tag.py` script from the maintainer tools
should be used. This ensures that all the various occurences of the
version in different files match the tagged version before proceeding.
Also replace other "ping wumpus" references.
fc726e0138 doc, rpc: add missing signet mentions in network name lists (Sebastian Falbesoner)
Pull request description:
This small PR adds a few missing mentions of signet w.r.t. chain enumerations:
- RPC `getblockchaininfo`: result description for `"chain"`
- RPC `getmininginfo`: result description for `"chain"`
- REST interface documentation:
- default ports listing for each chain
- `"chain"` description for `chaininfo` endpoint result
The instances were identified via `git grep -i "main.*test.*reg"`.
ACKs for top commit:
ajtowns:
ACK fc726e0138 -- quick code review only
benthecarman:
ACK fc726e0138
Tree-SHA512: 62cdc6ef74fa10db75cc04b9eaf7367183f726b3fee3d21fdf741b3816669dd21508735e89da389ddac980f49773ab229263748d1399553375fefe4526361846
570e43fe72 guix: Print build params inside/outside of container (Carl Dong)
2f9d1fdde6 guix: Move DISTSRC determination to guix-build.sh (Carl Dong)
0b7cd07bb5 guix: Move OUTDIR determination+creation to guix-build.sh (Carl Dong)
d27ff8b86a guix: Add more sanity checks to guix-build.sh (Carl Dong)
57f9533146 guix: Add section headings to guix-build.sh (Carl Dong)
38b7b2ed72 genbuild: Specify rev-parse length (Carl Dong)
036dc740da docs: Point to contrib/guix/README.md in doc/guix.md (Carl Dong)
34f0fda2d3 guix: Small updates to README wording (Carl Dong)
402e3a5b1e guix: Update HOSTS README entry for new architectures (Carl Dong)
cfa7ceb21b guix: Remove README development environment section (Carl Dong)
93b6a8544a guix: Add ADDITIONAL_GUIX_{COMMON,TIMEMACHINE}_FLAGS options (Carl Dong)
0f31e24703 guix: Add SUBSTITUTE_URLS option (Carl Dong)
444fcfca90 guix: Make guix honor MAX_JOBS setting (Carl Dong)
Pull request description:
After live-demo-ing a Guix build (which completed successfully!) on achow101's stream, I realized there were a few quality of life improvements which can be made to improve the user experience of our Guix build process. Here are a few of them.
Notable changes:
1. When `MAX_JOBS` is specified, both `guix time-machine` and `guix environment` will now build up to `MAX_JOBS` packages at a time when creating the build environment
2. The instructions for using substitutes were incorrect, and has now been replaced with a `SUBSTITUTE_URLS` environment variable, which works well with shell's IFS splitting rules
3. New `ADDITIONAL_GUIX_{COMMON,TIMEMACHINE}_FLAGS` options, for more granular customization of the build process.
4. README cleanup
ACKs for top commit:
fanquake:
ACK 570e43fe72 - lets move this forward.
Tree-SHA512: 4e8ab560522ade5efb5e8736aec0fb1a3f19ae9deb586c1ab87020816876f3f466a950b3f8c04d9fa1d072ae5ee780038c5c9063577049bdd9db17978e11c328
3e61b8c800 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 3e61b8c800
Tree-SHA512: d177139ee142d47cb27ad878d721cafcd03403ef861965ff532d712da461416380ec5878f70accf223a552a1f1e65eedb1e0ad72cb7a96791f8a55536ce28645
95487b0553 doc: Drop mentions of Travis CI as it is no longer used (Hennadii Stepanov)
09d105ef0f 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 95487b0553
Tree-SHA512: 2e259bb8b1e37bcefc1251737bb2716f06ddb57c490010b373825c4e70f42ca38efae69a2f63f21f577d7cee3725b94097bdddbd313f8ebf499281cf97c53cef
a34eceb4cc doc: update -externalip documentation in tor.md (Jon Atack)
dc8a591222 doc: add tor.md section on how to get tor info via bitcoind (Jon Atack)
e1765d8b04 doc: update tor.md address examples from onion v2 to v3 (Jon Atack)
Pull request description:
It looks like `doc/tor.md` could use some updates and improvements, not only for Tor v3, but also for setting multiple addresses with `-externalip` (see the conversation from http://www.erisian.com.au/bitcoin-core-dev/log-2020-09-16.html#l-39), how to see information about your Tor config via Bitcoin Core, and other improvements.
Closes#19924.
ACKs for top commit:
laanwj:
ACK a34eceb4cc
Tree-SHA512: 3197cdca1188dbd645c8f9e6ed7c023da5ad9bcf246a6bcbfbe6078f40c01c563032b4906736cde253a2daf71aaed28a659121628891a5d0bf6e89f821a17a28
5c3eaf9983 doc: Add warnings for http interfaces limitations (Fabian Jahr)
Pull request description:
`libevent`, which is used for our rest interface, can use up all of the available file descriptors in a system if too many connections are opened at once. If a new block is connected at the same time and can not be written to disk because there are no file descriptors available, the node crashes. Based on my investigation so far the issue is best solved upstream which means we have to wait for the next release (2.2). In the meantime it would be good if we would warn users of this limitation.
See #11368 for more background.
ACKs for top commit:
MarcoFalke:
ACK 5c3eaf9983
Tree-SHA512: 73914538588477ead19068f5832fdcc8e0eb736e51f73b3aca501c93165e5ad634c2511a3fcffff251adcd3efda23a742b48211ad9d3b2a29cdeac17201d06a1
c175690561 doc: Update for FreeBSD 12.2, add GUI Build Instructions (Jarol Rodriguez)
Pull request description:
The current FreeBSD Build documentation is a little outdated and underwhelming. This PR intends to keep the build-freebsd.md doc up to date. Here are the main improvements:
- Introduce dependency information
- New instructions for building the GUI
- Instructions for supporting descriptor wallets
- Various notes on the build and compile process
**Before/Master:** [render](https://github.com/bitcoin/bitcoin/blob/master/doc/build-freebsd.md)
**After/PR:** [render](2e8b9a5aac/doc/build-freebsd.md)
ACKs for top commit:
laanwj:
utACK c175690561
Tree-SHA512: 64aeb743c7f4ed167451454564b53d13c5c30d82bd2423799655c7b5b465a75733072fb0c574927c2588a46e89a68c6c57b008220acfd25336d445c9dc309f3b
fa13e1b0c5 build: Add option --enable-danger-fuzz-link-all (MarcoFalke)
44444ba759 fuzz: Link all targets once (MarcoFalke)
Pull request description:
Currently the linker is invoked more than 150 times when compiling with `--enable-fuzz`. This is problematic for several reasons:
* It wastes disk space north of 20 GB, as all libraries and sanitizers are linked more than 150 times
* It wastes CPU time, as the link step can practically not be cached (similar to ccache for object files)
* It makes it a blocker to compile the fuzz tests by default for non-fuzz builds #19388, for the aforementioned reasons
* The build file is several thousand lines of code, without doing anything meaningful except listing each fuzz target in a highly verbose manner
* It makes writing new fuzz tests unnecessarily hard, as build system knowledge is required; Compare that to boost unit tests, which can be added by simply editing an existing cpp file
* It encourages fuzz tests that re-use the `buffer` or assume the `buffer` to be concatenations of seeds, which increases complexity of seeds and complexity for the fuzz engine to explore; Thus reducing the effectiveness of the affected fuzz targets
Fixes#20088
ACKs for top commit:
practicalswift:
Tested ACK fa13e1b0c5
sipa:
ACK fa13e1b0c5. Reviewed the code changes, and tested the 3 different test_runner.py modes (run once, merge, generate). I also tested building with the new --enable-danger-fuzz-link-all
Tree-SHA512: 962ab33269ebd51810924c51266ecc62edd6ddf2fcd9a8c359ed906766f58c3f73c223f8d3cc49f2c60f0053f65e8bdd86ce9c19e673f8c2b3cd676e913f2642
The current FreeBSD Build documentation is a little outdated and underwhelming. This PR updates the doc to
be more informative. It also adds new instructions for building the GUI and adding support for descriptor
wallets.
on vasild's recommendation: it is ok to point the user to download db5 which has an active port package
instead of building db4 from the provided script.
Co-authored-by: Vasil Dimov <vd@freebsd.org>
32045bbfd5 [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 32045bbfd5
laanwj:
Review ACK 32045bbfd5
RiccardoMasutti:
ACK 32045bb
Tree-SHA512: c623387b76d68845c0fa47f67a6f8ef70c9c560e3f8f8754e45a4f51e43198c2092be789588acd4ada607f42fbe62d51a4b1888d81b225df19b6557a081835c0
c932e0d67e doc: Update wallet database installation guide for macOS (Hennadii Stepanov)
ee7b84e63c build: Use Homebrew's sqlite package if it is available (Hennadii Stepanov)
c96d1f65a5 build, refactor: Check that Homebrew's qt5 package is actually installed (Hennadii Stepanov)
Pull request description:
On master (7ae86b3c68) installed Homebrew `sqlite` package is ignored during build on macOS.
This PR fixes this issue and update macOS build docs.
Closes#20498.
ACKs for top commit:
willcl-ark:
> > That said, another tACK of [c932e0d](c932e0d67e)
hebasto:
> That said, another tACK of [c932e0d](c932e0d67e)
laanwj:
Code review ACK c932e0d67e
jonasschnelli:
code review re-ACK c932e0d67e
Tree-SHA512: 2563f25534d065556b17ee8c0fca957aea61b5ae288a2aa72743e77607843a45c39f209321e0f05b34283a74d2edcf961cf1dc54a35ed0cc21182304bb961505
343dc4760f test: add test for high-bandwidth mode states in getpeerinfo (Sebastian Falbesoner)
dab6583307 doc: release note for new getpeerinfo fields "bip152_hb_{from,to}" (Sebastian Falbesoner)
a7ed00f8bb rpc: expose high-bandwidth mode states via getpeerinfo (Sebastian Falbesoner)
30bc8fab68 net: save high-bandwidth mode states in CNodeStats (Sebastian Falbesoner)
Pull request description:
Fixes#19676, "_For every peer expose through getpeerinfo RPC whether or not we selected them as HB peers, and whether or not they selected us as HB peers._" See [BIP152](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki), in particular the [protocol flow diagram](https://github.com/bitcoin/bips/raw/master/bip-0152/protocol-flow.png). The newly introduced states are changed on the following places in the code:
* on reception of a `SENDCMPCT` message with valid version, the field `m_highbandwidth_from` is changed depending on the first integer parameter in the message (1=high bandwidth, 0=low bandwidth), i.e. it just mirrors the field `CNodeState.fPreferHeaderAndIDs`.
* after adding a `SENDCMPCT` message to the send queue, the field `m_highbandwidth_to` is changed depending on how the first integer parameter is set (same as above)
Note that after receiving `VERACK`, the node also sends `SENDCMPCT`, but that is only to announce the preferred version and never selects high-bandwidth mode, hence there is no need to change the state variables there, which are initialized to `false` anyways.
ACKs for top commit:
naumenkogs:
reACK 343dc4760f
jonatack:
re-ACK 343dc4760f per `git range-diff 7ea6499 4df1d12 343dc47`
Tree-SHA512: f4999e6a935266812c2259a9b5dc459710037d3c9e938006d282557cc225e56128f72965faffb207fc60c6531fab1206db976dd8729a69e8ca29d4835317b99f
This is a follow up to https://github.com/bitcoin/bitcoin/pull/19638 that left some deprectaed "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.
faa05854f8 util: Remove probably misleading TODO (MarcoFalke)
fac5efe730 util: Add Assume() identity function (MarcoFalke)
fa861569dc util: Allow Assert(...) to be used in all contexts (practicalswift)
Pull request description:
This is needed for #20138. Please refer to the added documentation for motivation.
ACKs for top commit:
practicalswift:
cr ACK faa05854f8
jnewbery:
utACK faa05854f8
hebasto:
ACK faa05854f8, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 72165fbd898b92ab9a79b070993fa1faa86c2e3545b6645e72c652bda295d5107bc298d0482bf3aaf0926fc0c3e6418a445c0e073b08568c44231f547f76a688
053b4fbad8 doc: Release note regarding -rpcauth validation (João Barbosa)
46001323b1 rpc: Validate -rpcauth arguments (João Barbosa)
d37c813a43 rpc: Refactor to process -rpcauth once (João Barbosa)
Pull request description:
Invalid `-rpcauth` arguments are currently silently ignored. This make server initialization fail if any `-rpcauth` is invalid.
ACKs for top commit:
MarcoFalke:
review ACK 053b4fbad8
jonatack:
ACK 053b4fbad8
ryanofsky:
Code review ACK 053b4fbad8. Only changes since last review are moving a variable declaration and adding a comment, release notes, and a `const`.
Tree-SHA512: c99923d4a121f0c9f882b07f5402ea53e9b2d9455ad34468a094ffab1d64df26c82e1279734c0d42bc2e113eae7b581fbc3be52f3ed4a2d7450d11793afcf406
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.
a52ecc936a build: set minimum supported macOS to 10.14 (fanquake)
Pull request description:
This is a requirement for C++17 support. See my comments [here](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-643722538):
> You cannot use std::get with std::variant on macOS < 10.14, because Apples libc++ doesn't support the std::bad_variant_access exception. [Relevant comment](https://github.com/bitcoin/bitcoin/pull/19183#discussion_r439794318) in #19183.
> While we could work around this in our own code, using std::get_if, this would still be a problem for 3rd-party dependencies.
> I've been testing Qt 5.15LTS (we'll have to enable C++17 in qt, and may upgrade to a newer version at the same time), and you can't enable -std c++17, while targeting a macOS deployment version < 10.14, configuring will fail. They are making use of std::get with std::variant throughout their cocoa code.
We would have to had to have bumped to at least 10.13 in any case, as Qt 5.15 (#19716) [requires 10.13+](https://doc.qt.io/qt-5/supported-platforms.html).
ACKs for top commit:
hebasto:
ACK a52ecc936a, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: f669b2fc777aeea1e9afdbbc7bd9afe3997418211db6ba53c934cae0e62a9b999603da539518c229f34961d275c9e2f315c7b022cf5fb97bd201a69c85d470cc
d52f502b1e Fix mock SQLiteDatabases (Andrew Chow)
99309ab3e9 Allow disabling BDB in configure with --without-bdb (Andrew Chow)
ee47f11f73 GUI: Force descriptor wallets when BDB is not compiled (Andrew Chow)
71e40b33bd RPC: Require descriptors=True for createwallet when BDB is not compiled (Andrew Chow)
6ebc41bf9c Enforce salvage is only for BDB wallets (Andrew Chow)
a58b719cf7 Do not compile BDB things when USE_BDB is defined (Andrew Chow)
b33af48210 Include wallet/bdb.h where it is actually being used (Andrew Chow)
Pull request description:
Adds a `--without-bdb` option to `configure` which disables the compilation of the BDB stuff. Legacy wallets will not be created when BDB is not compiled. A legacy-sqlite wallet can be loaded, but we will not create them.
Based on #20156 to resolve the situation where both `--without-sqlite` and `--without-bdb` are provided. In that case, the wallet is disabled and `--disable-wallet` is effectively set.
ACKs for top commit:
laanwj:
Code review ACK d52f502b1e
Tree-SHA512: 5a92ba7a542acc2e27003e9d4e5940e0d02d5c1f110db06cdcab831372bfd83e8d89c269caff31dd5bff062c1cf5f04683becff12bd23a33be731676f346553d
8f7b930475 Drop the leading 0 from the version number (Andrew Chow)
Pull request description:
Removes the leading 0 from the version number. The minor version, which we had been using as the major version, is now the major version. The revision, which we had been using as the minor version, is now the minor version. The revision number is dropped. The build number is promoted to being part of the version number. This also avoids issues where it was accidentally not included in the version number.
The CLIENT_VERSION remains the same format as previous as previously, as the Major version was 0 so it never actually got included in it.
The user agent string formatter is updated to follow this new versioning.
***
Honestly I'm just tired of all of the people asking for "1.0" that maybe this'll shut them up. Skip the whole 1.0 thing and go straight to version 22.0!
Also, this means that the terminology we commonly use lines up with how the variables are named. So major versions are actually bumping the major version number, etc.
ACKs for top commit:
jnewbery:
Code review ACK 8f7b930475
MarcoFalke:
review ACK 8f7b930475🎻
Tree-SHA512: b5c3fae14d4c0a9c0ab3b1db7c949ecc0ac3537646306b13d98dd0efc17c489cdd16d43f0a24aaa28e9c4a92ea360500e05480a335b03f9fb308010cdd93a436
dc80a7d0b0 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
fac7198728 Use std::make_unique (MarcoFalke)
faaee810e6 build: Require C++17 compiler (MarcoFalke)
Pull request description:
Developers have been compiling with C++17 for a few months now (fuzz tests and the msvc build have it even enabled by default). According to #16684, the 22.0 release shall be compiled with C++17 enabled.
This only sets the build flag, any other changes need more discussion and can be done later.
ACKs for top commit:
elichai:
utACK fac7198728
hebasto:
ACK fac7198728, I've locally compiled on ARM 32bit SBC without GUI.
fanquake:
ACK fac7198728
Tree-SHA512: 53eb40ba5d496376a2d2cf16e2000bef36616cc2a3696c3ec59a5366e41fa8b872817a7ca21751f030f9c1efb339dfa63cc655eaa5199b9a3d2e52c2de0ccb29
Removes the leading 0 from the version number. The minor version, which
we had been using as the major version, is now the major version. The
revision, which we had been using as the minor version, is now the minor
version. The revision number is dropped. The build number is promoted to
being part of the version number. This also avoids issues where it was
accidentally not included in the version number.
The CLIENT_VERSION remains the same format as previous as previously,
the Major version was 0 so that was never a factor in CLIENT_VERSION.
daf1ebf0b1 doc: Remove generated manual pages from master branch (Wladimir J. van der Laan)
Pull request description:
Replace the generated manual pages on the master branch with placeholders with instructions. The master branch is too much in flux for anything generated from command output to be kept up to date, and having pages from versions ago looks silly.
We can't remove them completely because `make dist` relies on the files being present.
Resolves#20062.
ACKs for top commit:
MarcoFalke:
review ACK daf1ebf0b1
RiccardoMasutti:
ACK daf1ebf
Tree-SHA512: fdd0bb777ddf66c663e4f6b00d206b3f6283ae89028e00b38d16e68a224a781e9245a25edf6f3326f9252ae2ef7c2950c591667c56d4e9d10a16fc50a6c358f4
Replace the generated manual pages on the master branch with
placeholders with instructions. The master branch is too much in flux
for anything generated from command output to be kept up to date,
and having pages from versions ago looks silly.
We can't remove them completely because `make dist` relies
on the files being present.
Resolves#20031.
97c738ff1b [tests] Recommend f-strings for formatting, update feature_block to use them (Anthony Towns)
8ae9d314e9 Bump minimum python version to 3.6 (Anthony Towns)
Pull request description:
Python 3.5 has reached [end-of-life](https://devguide.python.org/#status-of-python-branches) as of September 2020, and 3.6 has some moderately nice [features](https://docs.python.org/3/whatsnew/3.6.html):
- `f'x = {x}'` as an alternative to `'x = {}'.format(x)` format strings (cf https://github.com/bitcoin/bitcoin/pull/13718#issuecomment-406591027)
- underscore separators for large numbers, like `1_234_567`
- improvements to async
- improvements to typing module
Note that 3.6 is not available in xenial (16.04), but is available in bionic (18.04), while focal (20.04) has 3.8. CentOS 7 and 8 have 3.6.8, Debian stable has 3.7.3, and [gentoo and arch already had 3.6 and 3.7 in 2018](https://github.com/bitcoin/bitcoin/pull/14954#issuecomment-447118707).
ACKs for top commit:
MarcoFalke:
re-ACK 97c738ff1b
Tree-SHA512: ec7fce68845edde4d61a42de12c065fd49e5217311a6fda1323206f091a0afd50f293645dffc27d420127e4e5deb864e953f1b67eff735a0dfbbedd7899a9d60
38ada892ed addrman: ensure old versions don't parse peers.dat (Vasil Dimov)
Pull request description:
Even though the format of `peers.dat` was changed in a backwards
incompatible way, it is not guaranteed that old versions will fail to
parse it. There is a chance that old versions parse its contents as
garbage and use it.
Old versions expect the "key size" field to be 32 and fail the parsing
if it is not. Thus, we put something other than 32 in it. This will make
versions between 0.11.0 and 0.20.1 deterministically fail on the new
format. Versions prior to https://github.com/bitcoin/bitcoin/pull/5941
will still parse it as garbage.
Also, introduce a way to increment the `peers.dat` format in a way that
does not necessary make older versions refuse to read it.
ACKs for top commit:
jnewbery:
ACK 38ada892ed
laanwj:
Code review ACK 38ada892ed
MarcoFalke:
re-ACK 38ada892ed🥐
Tree-SHA512: 550bd660c5019dba0f9c334aca8a11c4a0463cfddf11efe7a4a5585ffb05549c82b95066fba5d073ae37893e0eccc158a7ffea9b33ea031d9be4a39e44f6face
Even though the format of `peers.dat` was changed in an incompatible
way (old software versions <0.21 cannot understand the new file format),
it is not guaranteed that old versions will fail to parse it. There is a
chance that old versions parse its contents as garbage and use it.
Old versions expect the "key size" field to be 32 and fail the parsing
if it is not. Thus, we put something other than 32 in it. This will make
versions between 0.11.0 and 0.20.1 deterministically fail on the new
format. Versions prior to https://github.com/bitcoin/bitcoin/pull/5941
(<0.11.0) will still parse it as garbage.
Also, introduce a way to increment the `peers.dat` format in a way that
does not necessary make older versions refuse to read it.
