1ae5b208d3 test: fix intermittent failure in p2p_compactblocks_hb.py (Martin Zumsande)
Pull request description:
Fixes#29860
As a result of node1 receiving a block, it sends out SENDCMPCT messages to some of its peers to update the high-bandwidth status. We need to wait until those are received and processed by the peers to avoid intermittent failures. Before, we'd only wait until all peers have synced with the new block (within `generate`) which is not sufficient.
I could reproduce the failure by adding a `std::this_thread::sleep_for(std::chrono::milliseconds(1000));` sleep to the [net_processing code](c7567d9223/src/net_processing.cpp (L3763)) that processes `NetMsgType::SENDCMPCT`.
ACKs for top commit:
instagibbs:
ACK 1ae5b208d3
alfonsoromanz:
Tested ACK 1ae5b208d3
glozow:
ACK 1ae5b208d3
Tree-SHA512: 47c29616e73a5e0ff966fc231e4f672c1a6892511e5c10a3905b30ad6b2a3d1267fa0a88bd8f64b523fe580199d22a43545c84e361879e5096483152065c4b9a
fa4c69669e test: Fix failing univalue float test (MarcoFalke)
Pull request description:
Currently the test may fail for some compilers, because `1e-8` may not be possible to represent exactly/consistently.
```
$ ./src/univalue/test/object
object: univalue/test/object.cpp:424: void univalue_readwrite(): Assertion `v.read("0.00000000000000000000000000000000000001e+30 ") && v.get_real() == 1e-8' failed.
Aborted (core dumped)
```
Fixes https://github.com/bitcoin/bitcoin/pull/27256#discussion_r1567356943
ACKs for top commit:
laanwj:
ACK fa4c69669e
stickies-v:
ACK fa4c69669e , thanks for fixing!
Tree-SHA512: dea4f4f843381d5e8ffaa812b2290a11e081b29f8777d041751c4aa9942e60f1f8d2d1a652d9a52b41dec470a490c9fe26ca9bc762dd593c3521b328a8af2826
05da2460db guix: remove gcc-toolchain static from Windows build (fanquake)
Pull request description:
The libs in this dir are the following:
```bash
ls /gnu/store/2vnbkrdin4rrf7ygnr80mlcglin4qqa4-gcc-toolchain-12.3.0-static/lib/
libanl.a libc.a libdl.a libm.a
libBrokenLocale.a libcrypt.a libg.a libmcheck.a
libpthread.a librt.a
libresolv.a libutil.a
```
These do not need to be propogated into the Windows build environment.
Guix Build (aarch64):
```bash
450c0c4f45f9cb7ed7fc2ef6e7557b6a23004b82c951399da3b7635e8451a076 guix-build-05da2460db89/output/dist-archive/bitcoin-05da2460db89.tar.gz
5df68ab18636090c387bc90297356d0e148b02931d3a99c0f6d33cd268aa072b guix-build-05da2460db89/output/x86_64-w64-mingw32/SHA256SUMS.part
13e979f60d9296aa11081fbbb360404da9fbb797bb4663ed2d1189d800659b4f guix-build-05da2460db89/output/x86_64-w64-mingw32/bitcoin-05da2460db89-win64-debug.zip
d1cc78437a96f012a59af7c757bef592f163559e523d45014d7804d0be29a8b8 guix-build-05da2460db89/output/x86_64-w64-mingw32/bitcoin-05da2460db89-win64-setup-unsigned.exe
33a9cfd4475677646bb32c9c45c25bd796ca5adb126590bf556d4e6f9592c676 guix-build-05da2460db89/output/x86_64-w64-mingw32/bitcoin-05da2460db89-win64-unsigned.tar.gz
5d2ee251668d3d31bf378826ab06f98542dd20926cdee2df5c3315e11222a519 guix-build-05da2460db89/output/x86_64-w64-mingw32/bitcoin-05da2460db89-win64.zip
```
Somewhat similar to #29673.
ACKs for top commit:
laanwj:
ACK 05da2460db
hebasto:
ACK 05da2460db,
Tree-SHA512: bf514a726a22e2bfae4de645b10d90a66fe090971340c4299aeb9b2ff9cf536ca6cfed274d312ea5d5a172775cbda6db0e609492ec603f5aee55c8de81462cc0
b7ba60f81a test: add coverage for -reindex and assumeutxo (Martin Zumsande)
e57f951805 init, validation: Fix -reindex option with an existing snapshot (Martin Zumsande)
Pull request description:
In c711ca186f logic was introduced that `-reindex` and `-reindex-chainstate` will delete the snapshot chainstate.
This doesn't work currently, instead of deleting the snapshot chainstate the node crashes with an assert (this can be triggered by applying the added test commit on master).
Fix this, and another bug that would prevent the new active chainstate from having a mempool after `-reindex` has deleted the snapshot (also covered by the test).
ACKs for top commit:
fjahr:
re-ACK b7ba60f81a
hernanmarino:
crACK b7ba60f81a . Good fix
BrandonOdiwuor:
re-ACK b7ba60f81a
byaye:
Tested ACK b7ba60f81a
Tree-SHA512: c168f36997d7677d590af37b10427870f5d30123abf1c76032a16661e486735373bfa7e049e6aca439526fbcb6d619f970bf9d042196c851bf058a75a32fafdc
As a result of node1 receiving a block, it sends out
SENDCMPCT messages to its peers to update the status.
We need to wait until those are received and
processed by the peers to avoid intermittent failures.
c2e0489b71 [rpc, bugfix] Enforce maximum value for setmocktime (dergoegge)
Pull request description:
The maximum value for our mocktime must be representable in nanoseconds, otherwise we end up with negative values returned from `NodeClock::now()`.
Found through fuzzing:
```
$ echo "c2V0bW9ja3RpbWVcZTptYf9w/3NldG3///////////////9p////ZP///ymL//////89////Nv9L////////LXkBAABpAA==" | base64 --decode > rpc-8cab9148ab4418ebd1923c213e9d3fe9c9b49b39.crash
$ FUZZ=rpc ./src/test/fuzz/fuzz rpc-8cab9148ab4418ebd1923c213e9d3fe9c9b49b39.crash
fuzz_libfuzzer: util/time.cpp:28: static NodeClock::time_point NodeClock::now(): Assertion `ret > 0s' failed.
```
ACKs for top commit:
maflcko:
re-ACK c2e0489b71
brunoerg:
crACK c2e0489b71
glozow:
ACK c2e0489b71
Tree-SHA512: d7e237ca37bedd74a6b085fb6e726a142705371044c77488f593f35afe70aeca756fdba86920294b1d322c7a9b2cde9ce4e1b7d410a6ccc1fd7c6f3a6e77200a
92f88a9629 doc: fixup NAT-PMP help doc (fanquake)
02395edca9 init: remove redundant upnp #ifdef (fanquake)
Pull request description:
This is a very belated followup to #26896 (which removed the configure options for setting the upnp and natpmp runtime default) and corrects the `-help` docs for `-upnp` and `-natpmp`.
ACKs for top commit:
davidgumberg:
ACK 92f88a9629
hernanmarino:
ACK 92f88a9629
Tree-SHA512: 795dc8a8703bf322b5831d845de85f2428ee0dd45d3064b48ff47d147147381af26c0a9d00c596db12009b254763844b209989daf4e7470d20e8a1753b640966
fadf7e90dc ci: Bump s390x to ubuntu:24.04 (MarcoFalke)
Pull request description:
Now that most other CI tasks are rolled to 24.04, roll this one as well.
Also, re-enable feature_init.
ACKs for top commit:
fanquake:
ACK fadf7e90dc - finished running after ~5 hours.
Tree-SHA512: d69f40227ba982eaaccc1e144ba05d3c8c4a096ac401077df3db5cf3276d1e146a438e4e0237dda876b3644fc7f0accd724a19e3c1cdb94c30c679899137280e
4722b7c715 build: remove minisketch clz check (fanquake)
1eea10a6d2 Squashed 'src/minisketch/' changes from a571ba20f9..3472e2f5ec (fanquake)
Pull request description:
https://github.com/sipa/minisketch/pull/81 will fix#29799.
Minor build cleanups after https://github.com/sipa/minisketch/pull/80.
ACKs for top commit:
dergoegge:
utACK 4722b7c715
hebasto:
ACK 4722b7c715, I have verified the subtree update and reviewed the build system changes. Both look OK.
Tree-SHA512: eabd82e5a13cc4f32155319df97368f2e8c93320a4265b6c372efcb1ea4e756f6693df7c02498c8ea989ccd376a20277fa110c66d0754cb9bca5e54d18e0a965
3472e2f5ec Merge sipa/minisketch#81: Avoid overflowing shift by special casing inverse of 1
653d8b2e26 Avoid overflowing shift by special casing inverse of 1
33b7c200b9 Merge sipa/minisketch#80: Add c++20 version of CountBits
4a48f31a37 Merge sipa/minisketch#83: ci: Fix "s390x (big-endian)" task
82b6488acb Add c++20 version of CountBits
0498084d31 ci: Fix "s390x (big-endian)" task
71709dca9e Merge sipa/minisketch#82: ci: Fix `x86_64-w64-mingw32` task
9e6127fa98 Merge sipa/minisketch#74: Avoid >> above type width in BitWriter
ed420bc170 ci: Fix `x86_64-w64-mingw32` task
fe1040f227 Drop -Wno-shift-count-overflow compile flag
154bcd43bd Avoid >> above type width in BitWriter
67b87acdb6 Merge sipa/minisketch#78: ci: Update macOS image for CI
7de7250416 ci: Update macOS image for CI
83d812ea9f Merge sipa/minisketch#73: ci: Use correct variable to designate C++ compiler
e051a7d690 ci: Install wine32 package for Windows tests
2d2c695d78 build: Drop unused `CC` variable
1810fcbd11 ci: Use correct variable to designate C++ compiler
022b959049 Merge sipa/minisketch#77: Add missing include
08443c4892 Add missing include
git-subtree-dir: src/minisketch
git-subtree-split: 3472e2f5ec75ace39ce9243af6b3fee233a67492
4ba1d0b553 fuzz: Add coverage for client_maxfeerate (Greg Sanders)
91d7d8f22a AcceptMultipleTransactions: Fix workspace client_maxfeerate (Greg Sanders)
f3aa5bd5eb fill_mempool: assertions and docsctring update (Greg Sanders)
a3da63e8fe Move fill_mempool to util function (Greg Sanders)
73b68bd8b4 fill_mempool: remove subtest-specific comment (Greg Sanders)
Pull request description:
Bug causes an `Assume()` failure due to the expectation that the individual result should be invalid when done over `submitpackage` via rpc.
Bug introduced by https://github.com/bitcoin/bitcoin/pull/28950 , and I discovered it rebasing https://github.com/bitcoin/bitcoin/pull/28984 since it's easier to hit in that test scenario.
Tests in place were only checking `AcceptSingleTransaction`-level checks due to package evaluation only triggering when minfee is too high for the parent transaction.
Added test along with fix, moving the fill_mempool utility into a common area for re-use.
ACKs for top commit:
glozow:
reACK 4ba1d0b553
theStack:
ACK 4ba1d0b553
ismaelsadeeq:
re-ACK 4ba1d0b553 via [diff](4fe7d150eb..4ba1d0b553)
Tree-SHA512: 3729bdf7f25d04e232f173ccee04ddbb2afdaafa3d04292a01cecf58fb11b3b2bc133e8490277f1a67622b62d17929c242dc980f9bb647896beea4332ee35306
95c594f4e9 depends: remove no longer needed patch for Boost::Process (Sebastian Falbesoner)
Pull request description:
As Boost::Process has been replaced by cpp-subprocess (PR #28981), this patch touches an unused code part and is hence not needed anymore.
ACKs for top commit:
hebasto:
ACK 95c594f4e9, I have reviewed the code and it looks OK.
fanquake:
ACK 95c594f4e9
Tree-SHA512: 0309b826f8c260e4180624f17302e51329fc4bd7a5431997d6d27d468dd5f7dbcd9db6a742efaba33ba30dbe361830eb1446fdbec927505ccf42412f9211934e
d5a715536e build: remove boost::process dependency for building external signer support (Sebastian Falbesoner)
70434b1c44 external_signer: replace boost::process with cpp-subprocess (Sebastian Falbesoner)
cc8b9875b1 Add `cpp-subprocess` header-only library (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/24907.
This PR is based on **theStack**'s [work](https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1466087049).
The `subprocess.hpp` header has been sourced from the [upstream repo](https://github.com/arun11299/cpp-subprocess) with the only modification being the removal of convenience functions, which are not utilized in our codebase.
Windows-related changes will be addressed in subsequent follow-ups.
ACKs for top commit:
achow101:
reACK d5a715536e
Sjors:
re-tACK d5a715536e
theStack:
Light re-ACK d5a715536e
fanquake:
ACK d5a715536e - with the expectation that this code is going to be maintained as our own. Next PRs should:
Tree-SHA512: d7fb6fecc3f5792496204190afb7d85b3e207b858fb1a75efe483c05260843b81b27d14b299323bb667c990e87a07197059afea3796cf218ed8b614086bd3611
a71eadf66b Change MAC_OSX macro to __APPLE__ in crypto package (Lőrinc)
Pull request description:
Split out from https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2044405345 to avoid the uncertainties and simplify review.
ACKs for top commit:
theuni:
ACK a71eadf66b
fanquake:
ACK a71eadf66b
Tree-SHA512: b6a7bd7ca95585dd9110cefe7c1213f4a1a72bdfc88670abf4a0d9a8bbc12e093544524adce46aa9ca714c472d417f74ca4a678af682ed2488053059434eaa02
a8203e9412 refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair<Wtxid, CTransactionRef> (AngusP)
c3c18433ae refactor: Use typesafe Wtxid in compact block encoding message, instead of ambiguous uint256. (AngusP)
Pull request description:
The first commit replaces `uint256` with typesafe `Wtxid` (or `Txid`) types introduced in #28107.
The second commit then simplifies the extra tx `vector` to just be of `CTransactionRef`s instead of a `std::pair<Wtxid, CTransactionRef>`, as it's easy to get a `Wtxid` from a transaction ref.
ACKs for top commit:
glozow:
ACK a8203e9412
dergoegge:
ACK a8203e9412
Tree-SHA512: b4ba1423a8059f9fc118782bd8a668213d229c822f22b01267de74d6ea97fe4f2aad46b5da7c0178ecc9905293e9a0eafba1d75330297c055e27fd53c8c8ebfd
47cedee776 fuzz: Introduce `BITCOINFUZZ` environment variable (Hennadii Stepanov)
1573e9a11e fuzz, refactor: Deduplicate fuzz binary path creation (Hennadii Stepanov)
Pull request description:
These changes are split from https://github.com/bitcoin/bitcoin/pull/29774 and can be beneficial on their own.
The new `BITCOINFUZZ` environment variable complements the already existing set of variables used by tests: b5d21182e5/test/functional/test_framework/test_framework.py (L238-L243)
ACKs for top commit:
maflcko:
lgtm ACK 47cedee776
davidgumberg:
utACK 47cedee776
Tree-SHA512: 45809cfd13dc4a45c44cc433184352e84726cb95bea80fd8f581c59a0b8b0a5495260ff66922f9c57c38adbdbdd102439238f370fd49d6ea27a241a5e6249895
03b87a3e64 Drop Windows Socket dependency for `randomenv.cpp` (Hennadii Stepanov)
Pull request description:
This change drops a dependency on the ws2_32 library for our libbitcoinkernel by switching to [`GetComputerName`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getcomputernamew) function.
ACKs for top commit:
sipsorcery:
utACK 03b87a3e64.
laanwj:
Code review ACK 03b87a3e64.
fanquake:
ACK 03b87a3e64
Tree-SHA512: a4abd5499176634d5f3fbf4e794a7504c40232fb73bd7f41955fbfb2cc7c44bc7ea4518c5203836e52f552c30414c6c3e1b24f0922641dbf1c8377981c0ffaf0
d4e36ae80d test: Update --tmpdir doc string to say directory must not exist (kevkevin)
Pull request description:
The error message given if passing an existing dir to --tmpdir is confusing so this makes it clear that the directory must not already exist
This change is motivated by this comment https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-1960913020
ACKs for top commit:
maflcko:
lgtm ACK d4e36ae80d
davidgumberg:
ACK d4e36ae80d
Tree-SHA512: fb31fd079767abbf94076615817943f35f5c9262fc97e65c631a18d33b3a343fe6a2d151613256e632d2b372ab2de0435f4712309b4a77ed3c663fd93a7dcdd1
5efebc0edb depends: add the new LLVM debug macro (fanquake)
Pull request description:
`LIBCXX_HARDENING_MODE` is the new macro, the previous one was removed in LLVM 18.
See https://libcxx.llvm.org/Hardening.html.
Required before https://github.com/google/oss-fuzz/pull/11725 will do anything (with the bump to 18.x).
Seems reasonable to do now that almost all our test infra is using LLVM 18.
ACKs for top commit:
theuni:
ACK 5efebc0edb
Tree-SHA512: 43078eeb5940c55ef4f95c72682f8a372dcd3eb97956b3114149c16d9f59b067a999b2aab7f34ffb57eab191524514408e2bba154ff4a6ea0cd6ec4d119c5d18
4f273ab436 Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us (Luke Dashjr)
Pull request description:
To avoid issues with DNS blacklisting, I've setup a separate domain for my DNS seed.
(This time, without a potentially alarming name)
ACKs for top commit:
kevkevinpal:
Concept ACK [4f273ab](4f273ab436), name looks good to me
petertodd:
ACK 4f273ab436
mzumsande:
ACK 4f273ab436
fanquake:
ACK 4f273ab436
Tree-SHA512: 689698e3c735df3ed0c2756a9d4adb5644bb9d8a6954e23d66bfa9d94ee10954f77fb241d9593f750054d731aa1532368a0fc8277884f6c2a98ac47cd0bdeeb7
78407b99ed [clang-tidy] Enable the misc-no-recursion check (dergoegge)
Pull request description:
Recursion is a frequent source of stack overflow bugs. Secondly, introduction of recursion can be non-obvious.
This PR proposes to use the clang-tidy `misc-no-recursion` check to make introduction of new recursion obvious. We don't make use of recursion a lot in our code base but there are a few places that need suppressions anyway (mostly the descriptor and univalue/rpc code).
ACKs for top commit:
stickies-v:
ACK 78407b99ed
TheCharlatan:
Re-ACK 78407b99ed
fanquake:
ACK 78407b99ed
Tree-SHA512: 34126d704c46086fe7371906ca852c25ced1dbd5fcfd85bf623810cd171a797569a92a33c7e26b8dc01c30c7bbf81aa326718926e8354585091411989a4edb14
The libs in this dir are the following:
```bash
ls /gnu/store/2vnbkrdin4rrf7ygnr80mlcglin4qqa4-gcc-toolchain-12.3.0-static/lib/lib
libanl.a libc.a libdl.a libm.a
libBrokenLocale.a libcrypt.a libg.a libmcheck.a
libpthread.a librt.a
libresolv.a libutil.a
```
These do not need to be propogated into the Windows build environment.
93fae5ae7c test: remove immediate tx relay workaround in wallet_groups.py (Sebastian Falbesoner)
Pull request description:
Reverts commit ab4efad51b (PR #26970). This workaround is not needed anymore, as since #27114 the test sets the noban permission for both in- and outbound connections via the `noban_tx_relay` setting, and we don't have to rely on this topology hack anymore. See commit c985eb854c (kudos to brunoerg!).
Can be tested by executing `$ time ./test/functional/wallet_groups.py` both on master and PR and verifying that the execution time is roughly equal.
ACKs for top commit:
maflcko:
lgtm ACK 93fae5ae7c
brunoerg:
utACK 93fae5ae7c
Tree-SHA512: b949fd05b4308815ba02d0ee4d1318f642b930288dd03223f46db7db745177af1c070bc7058743ac27963c5ad90564089867cc12f31fee94812a16919c353bab
