This was previously done implicitly in boost::optional by BOOST_ASSERT.
Also, it was checked at runtime by valgrind if for some reason the
assert was disabled.
std::optional dereference won't assert, so add the Assert here
explicitly.
The explicit Assert also helps to document the code better.
a0a771843f contrib: Changes to checks for PowerPC64 (Luke Dashjr)
634f6ec4eb contrib: Parse ELF directly for symbol and security checks (Wladimir J. van der Laan)
Pull request description:
Instead of the ever-messier text parsing of the output of the readelf tool (which is clearly meant for human consumption not to be machine parseable), parse the ELF binaries directly.
Add a small dependency-less ELF parser specific to the checks.
This is slightly more secure, too, because it removes potential ambiguity due to misparsing and changes in the output format of `elfread`. It also allows for stricter and more specific ELF format checks in the future.
This removes the build-time dependency for `readelf`.
It passes the test-security-check for me locally, ~~though I haven't checked on all platforms~~. I've checked that this works on the cross-compile output for all ELF platforms supported by Bitcoin Core at the moment, as well as PPC64 LE and BE.
Top commit has no ACKs.
Tree-SHA512: 7f9241fec83ee512642fecf5afd90546964561efd8c8c0f99826dcf6660604a4db2b7255e1afb1e9bb0211fd06f5dbad18a6175dfc03e39761a40025118e7bfc
6d1e85f475 Clean up logging of outbound connection type (Suhas Daftuar)
Pull request description:
We have a function that converts `ConnectionType` enums to strings, so use it.
Suggested by ajtowns in https://github.com/bitcoin/bitcoin/pull/19858#discussion_r540791588
ACKs for top commit:
amitiuttarwar:
ACK 6d1e85f475
naumenkogs:
ACK 6d1e85f
Tree-SHA512: f5084d8b5257380696d9fde86a8873e190cd4553feb07fa49df39bbd9510bf5832d190a3bca1571c48370d16a17c7a34900857b21b27bec0777bfa710211d7bb
faeb40bee3 ci: Install missing lint packages (MarcoFalke)
Pull request description:
The cirrus container is vanilla ubuntu, so we need to install the needed packages
ACKs for top commit:
hebasto:
ACK faeb40bee3, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: e56198108e26ea0ba2a344b1b74bc294652f34e9866cca053a25fb1b83bbd87ea40254c340e5e169fdfcbd4dcb39fdc2078b5157ca729a22a9a1792ec514a33e
e71b656f31 qt: Align layout of checkboxes (Hennadii Stepanov)
Pull request description:
From https://github.com/bitcoin/bitcoin/issues/20555#issuecomment-747742627:
> 0.21.0rc3 compiled from source, Ubuntu 20.04 + i3: tested mainnet, testnet, and signet. created a descriptor wallet in qt, tested torv3 with addnode, and checked if anchors.dat is created on shutdown and removed on startup.
>
> I've noticed the "Advanced options" label being cut-off in the wallet creation dialog.
This PR should fix this issue.
ACKs for top commit:
MarcoFalke:
review ACK e71b656f31
Tree-SHA512: dcc997a440a96fc936296323afaf9f4040ac06cf5ad32724a88c2e2e16c726c35b1b2bacbb3e081839d8a69b5a6086b2570d6b2e98976f29b5427db2d434eee9
fae32f295c wallet: Add missing check for -descriptors wallet tool option (MarcoFalke)
faf8f61368 test: Add missing check for is_sqlite_compiled (MarcoFalke)
fa7dde1c41 wallet: Pass ArgsManager into ExecuteWalletToolFunc instead of using global (MarcoFalke)
Pull request description:
Also, fix a test failure when compiled without sqlite
ACKs for top commit:
ryanofsky:
Code review ACK fae32f295c. Thanks for implementing the -descriptors check and dealing with the test failure!
jonatack:
Code review utACK fae32f295c
Tree-SHA512: 3d7710694085822739a8316e4abc6db270799ca6ff6b0f9e5563ae240da65ae6a9cab7ba2647feae6ba540dac40b55b38ed41c8f6ed0bf02a3d1536284448927
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
a4118c6e20 Add patch to make codesign_allocate compatible with Apple's (Pieter Wuille)
Pull request description:
This is an alternative to #20638.
The problem is that Apple's codesign(_allocate) apparently rounds the "vmsize" attribute on the __LINKEDIT section to a multiple of 0x2000 on x86_64 rather than 0x1000 (as their published source code does). This divergence means that the binary signed by codesign is slightly different from the one recreated by our reattach-sig-to-gitian-output process, and the signature being invalid.
This fixes it by patching our codesign_allocate source code to also use 0x2000. In tests, this appears to result in matching binaries.
ACKs for top commit:
jonasschnelli:
Tested ACK a4118c6e20 - removed the osx cache, built commit a4118c6e20 for osx in gitian (dependency where built, patch was applied), signed on my signing mac (detach-sig-create), ran gitian osx signer with the produces signature and the a4118c6e20 build (detach-sig-apply), signature then was successful verified on my Mac (codesign -v /Volumes/Bitcoin-Core/Bitcoin-Qt.app)
MarcoFalke:
Concept ACK a4118c6e20
Tree-SHA512: 07b8cdf8216249ddfe4bd38b39f2b48b2e190d4002b84d8981e62197bbbc9f25ac5c137bcc32057b23fbf38cbb2889ef95101ce008edfbf608cd170b88b3acbc
23d8f34689 fuzz: replace CNode code with fuzz/util.h::ConsumeNode() (Jon Atack)
Pull request description:
Noticed this while updating the CNode fuzzing in #20210.
- cc26fab48d created `test/fuzz/net.cpp` in May 2020
- 79ef8324d4 created a CNode factory utility `test/fuzz/util.h::ConsumeNode()` in October 2020
This PR updates `fuzz/net.cpp` from the first commit to use `ConsumeNode()` from the second commit.
ACKs for top commit:
MarcoFalke:
ACK 23d8f34689
Tree-SHA512: 26f7685395b3d48fcf40dde0d479d5c2fb4e953ec9371940b19eee16bb30aee4840b081e1a918b924a9704c1bef484302ea3e8fe63819a3bba73e7eb805164f1
7b6887e75a doc: Convert depends options list from html to markdown (Wladimir J. van der Laan)
Pull request description:
This makes it easier to read in `less`, which is important for install instructions.
Rendered: [before](7ef6b1c51d/depends (dependency-options)) - [after](d97042406f/depends/README.md (dependency-options))
ACKs for top commit:
jonatack:
Code review re-ACK 7b6887e75a per `git diff d970424 7b6887e`
hebasto:
re-ACK 7b6887e75a
Tree-SHA512: 02970b2bb97d2e8fb2d66470f6d70662653fda176bf6f4861742823b361fdc7ab6a2b44143480ac1a525b8d7808b6a068e8b3677dbba16cd783b4cab90470af5
23cac24dd3 tests: Test bitcoin-wallet dump and createfromdump (Andrew Chow)
a88c320041 wallettool: Add createfromdump command (Andrew Chow)
e1e7a90d5f wallettool: Add dump command (Andrew Chow)
Pull request description:
Adds two commands to the `bitcoin-wallet` tool: `dump` and `createfromdump`. These commands will be useful for a wallet storage migration in the future. It is also generally useful to have a storage agnostic dump like this. These commands are similar to BDB's `db_dump` and `db_load` tools. This can also be useful for manual construction of a wallet file for tests.
`dump` outputs every key-value pair from the wallet as comma separated hex. Each key-value pair is on its own line with the key and value in hex separated by a comma. This is output to the file specified by the new `-dumpfile` option.
`createfromdump` takes a file produced by `dump` and creates a new wallet file with exactly the records specified in that file.
A new option `-dumpfile` is added to the wallet tool. When used with `dump`, the records will be written to the specified file. When used with `createfromdump`, the file is read and the key-value pairs constructed from it. `createfromdump` requires `-dumpfile`.
A simple round-trip test is added to the `tool_wallet.py`.
This PR is based on #19334,
ACKs for top commit:
Sjors:
re-utACK 23cac24
MarcoFalke:
re review ACK 23cac24dd3 only change is rebase and removing useless shared_ptr wrapper 🎼
ryanofsky:
Code review ACK 23cac24dd3. Only changes since last review rebase and changing a pointer to a reference
Tree-SHA512: 2d63cf62baca3d16495aa698dc02f7d889c81b41015e9c92c23c275bb4a690fc176d351c3fd7f310bd6b17f5a936cc9be694cbecd702af741b96c0f530e72fa2
cc3044ccdb fix misleading comment about call to non-existing function (pox)
Pull request description:
The comment seems to be describing the subsequent call to `SyncTransaction` but refers to it as `SyncNotifications`, which is not any function currently in the codebase.
It's best to just remove the "what" aspect of the comment and focus on the "why", which also reduces the risk of similar documentation errors in the future, in case the function ever gets renamed, for example.
ACKs for top commit:
laanwj:
ACK cc3044ccdb
Tree-SHA512: 882ff17836ef585a603dc504f3dd21f56f682e49b28a0998f23fd16025826fbb083b7978db3ee70d0e0ff2c86fd6c3fd99a2361e5d45c765fdc5822c5f14c0a7
010eed3ce0 doc: warn that incoming conns are unlikely when not using default ports (Adam Jonas)
Pull request description:
Closes#5150.
This was mostly copied from #5285 by sulks, who has since quit GitHub.
The issue has remained open for 6 years, but the extra explanation still seems useful.
ACKs for top commit:
laanwj:
re-ACK 010eed3ce0
Tree-SHA512: d240fb06bba41ad8898ced59356c10adefc09f3abb33e277f8e2c5980b40678f2d237f286b476451bb29d2b94032a7dee2ada3b2efe004ed1c2509e70b48e40f
0c41c10830 doc: Remove shouty enums in net_processing comments (Suhas Daftuar)
Pull request description:
This uses the `CNode::ConnectionTypeAsString()` strings in place of the all-caps enums in a couple of comments in `net_processing`, as suggested by ajtowns in https://github.com/bitcoin/bitcoin/pull/19858#discussion_r540821050.
ACKs for top commit:
practicalswift:
ACK 0c41c10830
jnewbery:
ACK 0c41c10830
laanwj:
ACK 0c41c10830
Tree-SHA512: c8ab905e151ebb144c3f878277dc59d77591e4b39632658407b69b80b80d65825d5a391b01e2aea6af2fdf174c143dfe7d2f3eba84a020a58d7926458fdcd0a5
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
198fff88f3 GUI: Define MAX_DIGITS_BTC for magic number in BitcoinUnits::format (Luke Dashjr)
Pull request description:
A magic number snuck in with https://github.com/bitcoin/bitcoin/pull/16432
ACKs for top commit:
hebasto:
ACK 198fff88f3, I have reviewed the code and it looks OK, I agree it can be merged.
kristapsk:
utACK 198fff88f3
Tree-SHA512: 78dc23c2ae61bac41e5e34eebf57274599cb2ebb0b18d46e8a3228d42b256a1bc9bb17091c748f0f692ef1c4c241cfbd3e30a12bcd12222a234c1a9547ebe786
facf5e37f6 ci: Only use credits for pull requests to the main repo (MarcoFalke)
Pull request description:
No need to spend credits for the `master` branch, because the build shouldn't fail there anyway and it is not time-critical to get a fast feedback.
Some other changes:
* Disable `stateful` for faster scheduling
* Reduce lint memory from 8G to 1G for faster scheduling
ACKs for top commit:
hebasto:
ACK facf5e37f6, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 4d052e68217086574b9ea3d603cde1b585833c289d47dfed5308ff001d02964dc75ec3b3ebf5b233ccd09c47ad4ff5ba0bef639bf6362d984e7c49fca8fec24b
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
4045a6722c ci: Use cpu=1 for linter (Dhruv Mehta)
739d39022d ci: Move linter task to cirrus (Dhruv Mehta)
Pull request description:
Solves #20467: Move linter to Cirrus-CI as Travis-CI.org is shutting down
ACKs for top commit:
MarcoFalke:
ACK 4045a6722c
Tree-SHA512: 9aa7487ac86c91fc68bb584d29134e304dbd46702514a5d47d1ef0de6b877d96d42b7589870fc67ad9a31f5d3a789728446da4418688f336111a9ba0f8de5feb
5021810650 Make CanFlushToDisk a const member function (practicalswift)
281cf99554 Do not run functions with necessary side-effects in assert() (practicalswift)
Pull request description:
Do not run functions with necessary side-effects in `assert()`.
ACKs for top commit:
laanwj:
Code review ACK 5021810650
sipa:
utACK 5021810650
theStack:
Code Review ACK 5021810650🟢
Tree-SHA512: 38b7faccc2f16a499f9b7b1b962b49eb58580b2a2bbf63ea49dcc418a5ecc8f21a0972fa953f66db9509c7239af67cfa2f9266423fd220963d091034d7332b96
7587d11ec9 build: remove cdrkit package from depends (fanquake)
0df9819126 build: Replace genisoimage with xorriso (fanquake)
22437fc72e build: Run libdmg-hfsplus's DMG tool in make deploy (Carl Dong)
Pull request description:
This is a redo of fanquake's https://github.com/bitcoin/bitcoin/pull/18151, which, aside from switching us from the deprecated `genisoimage` to the maintained `xorriso`, is also necessary for Guix to achieve determinism without using faketime.
> xorriso and its mkisofs/genisoimage emulation alter-ego xorrisofs are
> more maintained, and has the right toggles for us to achieve output
> determinism without using blunt tools like faketime.
>
> In this commit, we use xorrisofs from the build environment rather than
> building it ourselves using depends. This is not necessary and can be
> changed in the future.
>
> From wiki.debian.org/genisoimage?action=recall&rev=11 :
>
> > The classical command line interface for production of ISO 9660
> > filesystem images is the option set established by program mkisofs.
> > For reasons of licensing and other problems with its author, Debian
> > ships a fork of mkisofs, called genisoimage, which was split off in
> > 2006 and then developed independently.
> >
> > Meanwhile, genisoimage gets no new features and not even bug fixes. It
> > is first choice only if its options -udf or -hfs are needed.
> >
> > Replacement in most uses cases, especially for bootable ISO 9660
> > filesystems, archiving, and backup, is xorrisofs which starts the -as
> > mkisofs emulation mode of program xorriso.
ACKs for top commit:
laanwj:
ACK 7587d11ec9
Tree-SHA512: 62f3aad08fa8bf21192e951d7dd33b24975586d76834cfa3498f4b8cdb586cefec8cab2c073d1951a0884b5e182fd71ef2cf3accad98f84455016776ad3c5422
