💬 ryanofsky commented on issue "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`":
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2400559220)
Looking into windows timeout issue #30950 and the workaround #31028 disabling a subset of fuzz tests when BUILD_FUZZ_BINARY is false, I wonder if we can't just go with a simpler solution of fuzz testing normal libraries instead of building all the libraries specially for fuzz testing. Would a fix like the following not work?
<details><summary>diff</summary>
<p>
```diff
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -240,11 +240,6 @@ if(BUILD_FOR_FUZZING)
set(BUILD_GUI_TESTS OFF)
   
...
  (https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2400559220)
Looking into windows timeout issue #30950 and the workaround #31028 disabling a subset of fuzz tests when BUILD_FUZZ_BINARY is false, I wonder if we can't just go with a simpler solution of fuzz testing normal libraries instead of building all the libraries specially for fuzz testing. Would a fix like the following not work?
<details><summary>diff</summary>
<p>
```diff
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -240,11 +240,6 @@ if(BUILD_FOR_FUZZING)
set(BUILD_GUI_TESTS OFF)
...
💬 maflcko commented on pull request "ci: Approximate MAKEJOBS in image build phase":
(https://github.com/bitcoin/bitcoin/pull/30935#issuecomment-2400583836)
rebased
  (https://github.com/bitcoin/bitcoin/pull/30935#issuecomment-2400583836)
rebased
💬 achow101 commented on pull request "scripted-diff: Modernize nLocalServices naming":
(https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2400584380)
ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca
  (https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2400584380)
ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2400726577)
Reworked after receiving a bunch of out-of-band feedback. In short:
* Got rid of the `void *` option handling. Options are now set through dedicated functions instead of a single setter for all options.
* Got rid of the `kernel_TaskRunner`. The context now holds an immediate task runner internally on which a user can register various validation interfaces. It is now the user's responsibility to process the validation callbacks in a non-blocking fashion with their own infrastructure.
* Got r
...
  (https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2400726577)
Reworked after receiving a bunch of out-of-band feedback. In short:
* Got rid of the `void *` option handling. Options are now set through dedicated functions instead of a single setter for all options.
* Got rid of the `kernel_TaskRunner`. The context now holds an immediate task runner internally on which a user can register various validation interfaces. It is now the user's responsibility to process the validation callbacks in a non-blocking fashion with their own infrastructure.
* Got r
...
👍 TheCharlatan approved a pull request: "refactor: include the proper header rather than forward-declaring RemovalReasonToString"
(https://github.com/bitcoin/bitcoin/pull/31058#pullrequestreview-2355432332)
ACK ca2e4ba352cb0a3b5983c002b09ce15ebbf95177
  (https://github.com/bitcoin/bitcoin/pull/31058#pullrequestreview-2355432332)
ACK ca2e4ba352cb0a3b5983c002b09ce15ebbf95177
💬 fanquake commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1792453358)
I think it'd be good to just get this finished so the docs are updated. So that we don't end up with even more PRs to fix the same docs.
  (https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1792453358)
I think it'd be good to just get this finished so the docs are updated. So that we don't end up with even more PRs to fix the same docs.
💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2400748950)
Rebased 60ade81516d42d275c1143f49f47e142d32c45fc -> 1c1df66cebbe825a24cb2f66a24aab78709db7db ([blockmanDB_2](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_2) -> [blockmanDB_3](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockmanDB_2..blockmanDB_3))
* Fixed conflict with #30967
  (https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2400748950)
Rebased 60ade81516d42d275c1143f49f47e142d32c45fc -> 1c1df66cebbe825a24cb2f66a24aab78709db7db ([blockmanDB_2](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_2) -> [blockmanDB_3](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockmanDB_2..blockmanDB_3))
* Fixed conflict with #30967
💬 ryanofsky commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2400758677)
re: https://github.com/bitcoin/bitcoin/pull/31003#pullrequestreview-2349619672
> I did, however, run into a repeatable segfault that appears to be occurring with bitcoin_node when running bitcoin-mine (which calls `waitFeesChanged()`), interrupting bitcoin-mine (CTRL-C while it waits for change), then generating a block (with `bitcoin-cli`). Feels a bit DoSsy. Could be something I'm overlooking/messed up on my end. Not sure. Please request more details if needed.
@tdb3 Thanks for reporting
...
  (https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2400758677)
re: https://github.com/bitcoin/bitcoin/pull/31003#pullrequestreview-2349619672
> I did, however, run into a repeatable segfault that appears to be occurring with bitcoin_node when running bitcoin-mine (which calls `waitFeesChanged()`), interrupting bitcoin-mine (CTRL-C while it waits for change), then generating a block (with `bitcoin-cli`). Feels a bit DoSsy. Could be something I'm overlooking/messed up on my end. Not sure. Please request more details if needed.
@tdb3 Thanks for reporting
...
💬 theuni commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2400782077)
Combining with #30911 produces even more of a speedup (with Make, ninja is about the same).
Config: `-DCMAKE_BUILD_TYPE=Debug -DWITH_CCACHE=OFF -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_BENCH=ON`
Using: `make -j24`
Master: 2m7.311s
This PR: 1m34.495s
This PR + #30911: 1m19.046s
Result: Completes in 62% of the time compared to master.
  (https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2400782077)
Combining with #30911 produces even more of a speedup (with Make, ninja is about the same).
Config: `-DCMAKE_BUILD_TYPE=Debug -DWITH_CCACHE=OFF -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_BENCH=ON`
Using: `make -j24`
Master: 2m7.311s
This PR: 1m34.495s
This PR + #30911: 1m19.046s
Result: Completes in 62% of the time compared to master.
💬 TheCharlatan commented on pull request "init: Some small chainstate load improvements":
(https://github.com/bitcoin/bitcoin/pull/31046#issuecomment-2400797758)
Thank you for the reviews @mzumsande and @stickies-v,
Updated 1bec418e77482a4c53c8f1e7c89a151fdeae0f8e -> a7a02b09fafe9d03e5db1fb9d9600f6a13aaa852 ([chainstate_init_followup_0](https://github.com/TheCharlatan/bitcoin/tree/chainstate_init_followup_0) -> [chainstate_init_followup_1](https://github.com/TheCharlatan/bitcoin/tree/chainstate_init_followup_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstate_init_followup_0..chainstate_init_followup_1))
* Took @stickies-v's [s
...
  (https://github.com/bitcoin/bitcoin/pull/31046#issuecomment-2400797758)
Thank you for the reviews @mzumsande and @stickies-v,
Updated 1bec418e77482a4c53c8f1e7c89a151fdeae0f8e -> a7a02b09fafe9d03e5db1fb9d9600f6a13aaa852 ([chainstate_init_followup_0](https://github.com/TheCharlatan/bitcoin/tree/chainstate_init_followup_0) -> [chainstate_init_followup_1](https://github.com/TheCharlatan/bitcoin/tree/chainstate_init_followup_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstate_init_followup_0..chainstate_init_followup_1))
* Took @stickies-v's [s
...
💬 sdaftuar commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2400943410)
reACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825
  (https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2400943410)
reACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825
💬 mzumsande commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2400951708)
> I'm still not sure why it failed, tried to bump a mocktime with the last push, but poking in the dark since it doesn't fail locally for me...
Hopefullly fixed now, I believe that there was a missing sync in the added test.
> Rather than the crude 10 minute timeout - why not actually check if the block is being downloaded from the chosen peer - i.e. work out the throughput and make a decision based on this (compared to typical throughput from other peers)?
I'm not sure what exactly you
...
  (https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2400951708)
> I'm still not sure why it failed, tried to bump a mocktime with the last push, but poking in the dark since it doesn't fail locally for me...
Hopefullly fixed now, I believe that there was a missing sync in the added test.
> Rather than the crude 10 minute timeout - why not actually check if the block is being downloaded from the chosen peer - i.e. work out the throughput and make a decision based on this (compared to typical throughput from other peers)?
I'm not sure what exactly you
...
💬 gburkhow commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#issuecomment-2401046057)
contrib/guix/INSTALL.md maflcko:2409-less-boost
  (https://github.com/bitcoin/bitcoin/pull/30967#issuecomment-2401046057)
contrib/guix/INSTALL.md maflcko:2409-less-boost
💬 gburkhow commented on pull request "refactor: include the proper header rather than forward-declaring RemovalReasonToString":
(https://github.com/bitcoin/bitcoin/pull/31058#issuecomment-2401047233)
theuni:fix-pch-forward-declare
  (https://github.com/bitcoin/bitcoin/pull/31058#issuecomment-2401047233)
theuni:fix-pch-forward-declare
🚀 achow101 merged a pull request: "scripted-diff: Modernize nLocalServices naming"
(https://github.com/bitcoin/bitcoin/pull/30885)
  (https://github.com/bitcoin/bitcoin/pull/30885)
🤔 pablomartin4btc reviewed a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2355854106)
tACK ded1a6cc498ffde0695eb3ee6828b7c513ccc277
<details>
<summary>Tested on Ubuntu 22.04 locally and using an external USB.</summary>
```
./build/src/bench/bench_bitcoin -filter=Xor -testdatadir=/tmp/btc
Warning, results might be unstable:
* DEBUG defined
* CPU frequency scaling enabled: CPU 0 between 400.0 and 4,700.0 MHz
* CPU governor is 'powersave' but should be 'performance'
* Turbo is enabled, CPU frequency will fluctuate
Recommendations
* Make sure you compile for Release
...
  (https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2355854106)
tACK ded1a6cc498ffde0695eb3ee6828b7c513ccc277
<details>
<summary>Tested on Ubuntu 22.04 locally and using an external USB.</summary>
```
./build/src/bench/bench_bitcoin -filter=Xor -testdatadir=/tmp/btc
Warning, results might be unstable:
* DEBUG defined
* CPU frequency scaling enabled: CPU 0 between 400.0 and 4,700.0 MHz
* CPU governor is 'powersave' but should be 'performance'
* Turbo is enabled, CPU frequency will fluctuate
Recommendations
* Make sure you compile for Release
...
💬 VivaRado commented on issue "support BIP39 mnemonic in descriptors":
(https://github.com/bitcoin/bitcoin/issues/19151#issuecomment-2401402365)
Excuse the spam, but here is our implementation of bip39 with UI in JS, that we use for account access control, client and server side with checksum recalculation. https://github.com/VivaRado/BIP39UI
  (https://github.com/bitcoin/bitcoin/issues/19151#issuecomment-2401402365)
Excuse the spam, but here is our implementation of bip39 with UI in JS, that we use for account access control, client and server side with checksum recalculation. https://github.com/VivaRado/BIP39UI
🤔 Imebeez reviewed a pull request: "[28.x] backports and finalize"
(https://github.com/bitcoin/bitcoin/pull/30959#pullrequestreview-2356142508)
N-2
  (https://github.com/bitcoin/bitcoin/pull/30959#pullrequestreview-2356142508)
N-2
🤔 Imebeez reviewed a pull request: "[26.0] Finalize or rc4"
(https://github.com/bitcoin/bitcoin/pull/28959#pullrequestreview-2356143151)
N-2
  (https://github.com/bitcoin/bitcoin/pull/28959#pullrequestreview-2356143151)
N-2
🤔 stratospher reviewed a pull request: "Stratum v2 Noise Protocol"
(https://github.com/bitcoin/bitcoin/pull/29346#pullrequestreview-2054783141)
ACK b588ff8. went through the noise protocol spec. also sharing a [pictorial represantion of the NX handshake](https://github.com/stratospher/blogosphere/blob/main/noise.pdf) if it's useful to other reviewers.
  (https://github.com/bitcoin/bitcoin/pull/29346#pullrequestreview-2054783141)
ACK b588ff8. went through the noise protocol spec. also sharing a [pictorial represantion of the NX handshake](https://github.com/stratospher/blogosphere/blob/main/noise.pdf) if it's useful to other reviewers.
