55554463c15e3c75a64d26209dd3f8396e508cc2 fuzz: Use ConsumeWeakEnum in addrman for service flags (MarcoFalke)
Pull request description:
This has minimally better performance. Reported by me in https://github.com/bitcoin/bitcoin/pull/20228#discussion_r598081787
ACKs for top commit:
practicalswift:
Tested ACK 55554463c15e3c75a64d26209dd3f8396e508cc2
vasild:
ACK 55554463c15e3c75a64d26209dd3f8396e508cc2
Tree-SHA512: 4e5f51fe4f2bd7b2f37d0690e41203341ba45c0c9bc9247449cd26cfb5f77dc2ec61df3e4963276f68694e4b3ca3d0a76367a51c4d775501edeb3224d305a261
7b3434f8002d1a8cf0dbd0a0caef28e783b1efd8 build: don't try and use -fstack-clash-protection on Windows (fanquake)
Pull request description:
This has never worked with any of the mingw-w64 compilers we use, and the `-O0` is causing issues for builders applying spectre mitigations (see [IRC logs](http://www.erisian.com.au/bitcoin-core-dev/log-2021-03-12.html#l-15)).
Recent discussion on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458 would also indicate that this should just not be used on Windows.
ACKs for top commit:
laanwj:
Concept and code review ACK (but untested) 7b3434f8002d1a8cf0dbd0a0caef28e783b1efd8
hebasto:
ACK 7b3434f8002d1a8cf0dbd0a0caef28e783b1efd8, I've verified that this change does not affect builds for `HOST=x86_64-w64-mingw32` by comparing sizes of the output `*.exe` files.
Tree-SHA512: 72b582321ddff8db3201460fa42a53304e70f141ae078d76a4d4eeb1ca27c8dd567ccb468cc8616179c8df784bd8ca038dcb9a277f9e29f9d98c3cc842916b18
663f6cd9ddadeec30b27ec12f0f5ed49f3146cc9 contrib: Use -daemonwait in systemd init script (Wladimir J. van der Laan)
Pull request description:
Make systemd invoke dependencies only when ready by using `-daemonwait` in the service file instead of `-daemon`.
Closes#21322 by making bitcoind conform to behavior specified for `type=forking`.
This may need some tuning of timeouts.
ACKs for top commit:
darosior:
ACK 663f6cd
hebasto:
re-ACK 663f6cd9ddadeec30b27ec12f0f5ed49f3146cc9
Tree-SHA512: 890005852b632a202caa578e6c796ebdc9da0b2379a9157a4f56f7db9d193c0ffbb78d120bbf112ab2f273855f2a08c3da000b1f7a9fb5222a3b94dcdb16b878
fa4cebadcffd9112da4b13c7cc7ccf21e2bee887 util: Make Assume() usable as unary expression (MarcoFalke)
Pull request description:
Assume shouldn't behave different at the call site depending on build flags. Currently compilation fails if it is used as expression. Fix that by using the lambda approach from `Assert()` without the `assert()`.
ACKs for top commit:
jnewbery:
ACK fa4cebadcffd9112da4b13c7cc7ccf21e2bee887
practicalswift:
cr ACK fa4cebadcffd9112da4b13c7cc7ccf21e2bee887: patch looks correct and commit hash starts with `fa`
Tree-SHA512: 9ec9ac8d410cdaf5e4e28df571a89e3d23d38e05a7027bb726cae3da6e9314734277e5a218e9e090cc17e10db763da71052c229ad642077ca5824ee42022f3ed
5294f0d5a94cc7beaf692131fba0cad8beec9f13 refactor: return std::nullopt instead of {} (fanquake)
Pull request description:
In #21415 [we decided](https://github.com/bitcoin/bitcoin/pull/21415#issuecomment-800236640) to return `std::optional` rather than `{}` for
uninitialized values. This PR replaces the two remaining usages of `{}`
with `std::nullopt`.
As a side-effect, this also quells the spurious GCC 10.2.x warning that
we've had reported quite a few times. i.e #21318, #21248, #20797.
```bash
txmempool.cpp: In member function ‘CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>&) const’:
txmempool.cpp:898:13: warning: ‘<anonymous>’ may be used uninitialized in this function [-Wmaybe-uninitialized]
898 | return {};
| ^
```
ACKs for top commit:
hebasto:
ACK 5294f0d5a94cc7beaf692131fba0cad8beec9f13, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 5b776be79ab26e5a3a5fc2b463b394ea5ce6797ed5558424873fa4ecee2898170eff76d6da9d69394d28f8f98974117fc63b922a3e19c52f5294c83073e79bb0
In #21415 we decided to return `std::optional` rather than `{}` for
uninitialized values. This PR repalces the two remaining usages of `{}`
with `std::nullopt`.
As a side-effect, this also quells the spurious GCC 10.2.x warning that
we've had reported quite a few times. i.e #21318, #21248, #20797.
```bash
txmempool.cpp: In member function ‘CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>&) const’:
txmempool.cpp:898:13: warning: ‘<anonymous>’ may be used uninitialized in this function [-Wmaybe-uninitialized]
898 | return {};
| ^
```
Introduce an android directory under qt and allow one to package bitcoin-qt for Android by running make apk.
Add bitcoin-qt Android build instructions.
3d086f42ab58f0f5984101d5285d5a707e3dc8e7 test: add ParseUInt16() test coverage (Jon Atack)
Pull request description:
`ParseUInt16()` was just added in #21328.
ACKs for top commit:
practicalswift:
cr ACK 3d086f42ab58f0f5984101d5285d5a707e3dc8e7: patch looks correct & more coverage is better than less coverage
Tree-SHA512: bf7f96deb7c1531419565907f0ea8a8e32b368d4b823a3e80928b2c118edbf643ea06e357b4b5504a89f855caeed289daa9f823c740231ed6ad1b8ed00285ce8
52a43b0c7dac33bff65dfcefa98cd30169a6b38f build: Fix fuzz-cuckoocache cross-compiling for Windows with DEBUG=1 (Hennadii Stepanov)
Pull request description:
Fix#21348.
ACKs for top commit:
practicalswift:
Tested ACK 52a43b0c7dac33bff65dfcefa98cd30169a6b38f
Tree-SHA512: 6592f829edfb740a1e9d0691acf04b2372e91b0a53ca395b08350cb0b80031d3b55fa7331bdaddf857d450eb30b605af9fe8fe02559cda19374a48f9634fae70
787df19b09babf50dd8124b3ac990b29c33cfe93 validation: don't try to invalidate genesis block (Sebastian Falbesoner)
Pull request description:
In the block invalidation method (`CChainState::InvalidateBlock`), the code for creating the candidate block map assumes that the passed block's previous block (`pindex->pprev`) is available and otherwise segfaults due to null-pointer deference in `CBlockIndexWorkComparator()` (see analysis by practicalswift in #20914), i.e. it doesn't work with the genesis block. Rather than analyzing all possible code paths and implications for this corner case, simply fail early if the genesis block is passed.
Fixes#20914.
ACKs for top commit:
sipa:
ACK 787df19b09babf50dd8124b3ac990b29c33cfe93. Tested invalidation of generic on regtest.
practicalswift:
Tested ACK 787df19b09babf50dd8124b3ac990b29c33cfe93
Tree-SHA512: 978be7cf2bd1c1faebfe945d191ac77dea72791bea826459abd308f77c74c5991efee495a38817c306e488ecd5208b5c888df7d9d044132dd9a06bbbdb256b6c
8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a add functional test (Larry Ruane)
b5a80fa7e487c37b7ac0e3874a2fabade41b9ca8 util: Handle HTTP_SERVICE_UNAVAILABLE in bitcoin-cli (Hennadii Stepanov)
Pull request description:
If `bitcoind` is processing 16 RPC requests, attempting to submit another request using `bitcoin-cli` produces this less-than-helpful error message: `error: couldn't parse reply from server`. This PR changes the error to: `error: server response: Work queue depth exceeded`.
ACKs for top commit:
fjahr:
tACK 8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a
luke-jr:
utACK 8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a (no changes since previous utACK)
hebasto:
re-ACK 8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a, only suggested changes since my [previous](https://github.com/bitcoin/bitcoin/pull/18335#pullrequestreview-460621350) review.
darosior:
ACK 8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a
Tree-SHA512: 33e25f6ff05d9b56fae2bdb68b132557bb8e995f5438ac4fbbc53c304c5152a98aa43c43600c31d8a6a2830cbd48bf8ec7d89dce50190b29ec00a43830126913
52dd40a9febec1f4e70d777821b6764830bdec61 test: add missing netaddress include headers (Jon Atack)
6f09c0f6b57ac01a473c587a3e51e9d477866bb0 util: add missing braces and apply clang format to SplitHostPort() (Jon Atack)
2875a764f7d8b1503c7bdb2f262964f7a0cb5fc3 util: add ParseUInt16(), use it in SplitHostPort() (Jon Atack)
6423c8175fad3163c10ffdb49e0df48e4e4931f1 p2p, refactor: pass and use uint16_t CService::port as uint16_t (Jon Atack)
Pull request description:
As noticed during review today in https://github.com/bitcoin/bitcoin/pull/20685#discussion_r584873708 of the upcoming I2P network support, `CService::port` is `uint16_t` but is passed around the codebase and into the ctors as `int`, which causes uneeded conversions and casts. We can avoid these (including in the incoming I2P code without further changes to it) by using ports with the correct type. The remaining conversions are pushed out to the user input boundaries where they can be range-checked and raise with user feedback in the next patch.
ACKs for top commit:
practicalswift:
cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61: patch looks correct
MarcoFalke:
cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61
vasild:
ACK 52dd40a9febec1f4e70d777821b6764830bdec61
Tree-SHA512: 203c1cab3189a206c55ecada77b9548b810281cdc533252b8e3330ae0606b467731c75f730ce9deb07cbaab66facf97e1ffd2051084ff9077cba6750366b0432
Now that we have a reliable way to detect inbound onion peers, this commit
updates our existing eviction protection of 1/4 localhost peers to instead
protect up to 1/4 onion peers (connected via our tor control service), sorted by
longest uptime. Any remaining slots of the 1/4 are then allocated to protect
localhost peers, or 2 localhost peers if no slots remain and 2 or more onion
peers are protected, sorted by longest uptime.
The goal is to avoid penalizing onion peers, due to their higher min ping times
relative to IPv4 and IPv6 peers, and improve our diversity of peer connections.
Thank you to Gregory Maxwell, Suhas Daftuar, Vasil Dimov and Pieter Wuille
for valuable review feedback that shaped the direction.
and an `m_is_onion` struct member to NodeEvictionCandidate and tests.
We'll use these in the peer eviction logic to protect inbound onion peers
in addition to the existing protection of localhost peers.
An unordered set can tell if an element is present in ~O(1) time (constant on
average, worst case linear to the size of the container), which speeds up and
simplifies the lookup in IsEvicted().
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
out of net_tests, because the eviction tests:
- are a different domain of test coverage, with different dependencies
- run more slowly than the net tests
- will be growing in size, in this PR branch and in the future, as eviction
test coverage is improved
to allow deterministic unit testing of the ratio-based peer eviction protection
logic, which protects peers having longer connection times and those connected
via higher-latency networks.
Add documentation.
ea76f4ac7d6e8c268d301d7ae6c8d4d8d804d55f Doc: Tell howto install clang-format on Debian/Ubuntu (wodry)
Pull request description:
Because only macOS wasy mentioned, I was unsure if this would be a macOS specific tool. I guess Linux is more used than Mac, so Linux guide should be there, too.
ACKs for top commit:
hebasto:
ACK ea76f4ac7d6e8c268d301d7ae6c8d4d8d804d55f, every system upgrade via clean installation I do the same.
Tree-SHA512: 75c28540e8815cb41f4cf92784b6349978988b679e4deef9ae77ede951f93516ca13ec7b313ab72865b01273e115b49ed2b67cdcd68015af1b643a6186b190dd
Because only macOS wasy mentioned, I was unsure if this would be a macOS specific tool. I guess Linux is more used than Mac, so Linux guide should be there, too.
fa8177324392c923c6ce39056cfd870af55ab673 style-only: Remove whitespace (MarcoFalke)
fae77b9e6dc9e59b355d56df49c4d9685b6f40a4 net: Simplify ProcessGetBlockData execution by removing send flag. (Patrick Strateman)
fae7c0429f96e08bcac944f6fa30264636dfda8c log: Clarify that block request below NODE_NETWORK_LIMITED_MIN_BLOCKS disconnects (MarcoFalke)
Pull request description:
* Clarify that "ignoring" really means "disconnect" in the log
* Revive a refactor I took from #13670
ACKs for top commit:
jnewbery:
utACK fa8177324392c923c6ce39056cfd870af55ab673
sipa:
utACK fa8177324392c923c6ce39056cfd870af55ab673
Tree-SHA512: 0a4fcb979cb82c4e26012881eeaf903c38dfbb85d461476c01e35294760744746a79c48ffad827fe31c1b830f40c6e4240529c71e375146e4d0313c3b7d784ca
