🤔 BrandonOdiwuor reviewed a pull request: "coins: warn on shutdown for big UTXO set flushes"
(https://github.com/bitcoin/bitcoin/pull/31534#pullrequestreview-2514035020)
Concept ACK

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2553418201)
Thanks for the suggestion @stickies-v, I think it is the right call.

Updated 20eec64b5e417cac8c68100826c0adf2152a49eb -> f157b0cbc7d90075858a6522d13a7bc4f0b25a5f ([kernelApi_13](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_13) -> [kernelApi_14](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_14), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_13..kernelApi_14))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/30595#discussion
...

💬 maflcko commented on pull request "Do not print "Reducing -maxconnections..." warning in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2553420053)
This also seems unrelated to this pull request? If I increase `-j` sufficiently, I get errors before and after this pull request.

For reference, the error is:


```
./bld-cmake/test/functional/test_runner.py -j 40
Temporary test directory at /tmp/test_runner_₿_🏃_20241219_105517
1/315 - wallet_conflicts.py --legacy-wallet skipped (BDB has not been compiled.)
2/315 - rpc_psbt.py --legacy-wallet skipped (BDB has not been compiled.)
3/315 - wallet_fundrawtransaction.py --legacy-wallet s
...

💬 hodlinator commented on pull request "fuzz: Fix misplaced SeedRand::ZEROS":
(https://github.com/bitcoin/bitcoin/pull/31521#issuecomment-2553435131)
I was worried that `g_rng_temp_path` had somehow stopped working again, but I see you needed to replace it with `m_rng` in your diff to make paths deterministic.

It is a dilemma regarding the hash salts, one would like to exercise many values for them, but still get determinism. What would you think about having the fuzz input affect the random seed?

📝 l0rinc opened a pull request: "optimization: buffer reads/writes in [undo]blocks"
(https://github.com/bitcoin/bitcoin/pull/31539)
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/31490 (first few commits here, hence a draft) and a precursor to https://github.com/bitcoin/bitcoin/pull/31144.

Currently, obfuscation operations are performed byte-by-byte during serialization. Buffering the reads allows batching these operations (implemented in https://github.com/bitcoin/bitcoin/pull/31144) and improves file access efficiency by reducing fread calls and associated locking overhead. Testing with various buffer si
...

💬 Sjors commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#issuecomment-2553441321)
@A-Manning thanks, can you squash the commits?

cc @maflcko

💬 hebasto commented on pull request "Do not print "Reducing -maxconnections..." warning in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2553446026)
> This also seems unrelated to this pull request? If I increase `-j` sufficiently, I get errors before and after this pull request.

That diff causes `test_runner.py` to fail even with the default `-j` (at least on my Ubuntu 24.04).

💬 A-Manning commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#issuecomment-2553452773)
@Sjors
>thanks, can you squash the commits?

Squashed in ccb41ce0e782470a5e6e9d95a8bd337a1d07adcf

💬 Sjors commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#discussion_r1891647010)
nit: wrong indentation and unnecessary blank line

👍 hodlinator approved a pull request: "Remove unused variable assignment"
(https://github.com/bitcoin/bitcoin/pull/31497#pullrequestreview-2514136983)
crACK b9766c9977e58a9ebc358d9879576376e76a72b1

This dead store was a leftover from the introduction of CoinGrinder in 6cc9a46cd0f4ed80d4523bbef1508142e0c80d27 / #27877.

Checked that there are no shenanigans with the variable being captured into a lambda further up which is then called further down.

💬 Sjors commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#discussion_r1891651120)
I don't understand why this was duplicated in the original version. @kallewoof do you remember?

💬 Sjors commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#discussion_r1891655005)
Maybe also add a `check_getmininginfo` helper?

💬 maflcko commented on pull request "Do not print "Reducing -maxconnections..." warning in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2553487975)
> > This also seems unrelated to this pull request? If I increase `-j` sufficiently, I get errors before and after this pull request.
>
> That diff causes `test_runner.py` to fail even with the default `-j` (at least on my Ubuntu 24.04). And I assume that the CI will fail as well.

You supplied `-j40` in your command, which is not the default. As for the patch, `-maxconnections` controls the number of inbound connections (assuming that it is above the number of minimum outbound connections)
...

💬 maflcko commented on pull request "fuzz: Fix misplaced SeedRand::ZEROS":
(https://github.com/bitcoin/bitcoin/pull/31521#issuecomment-2553495150)
> It is a dilemma regarding the hash salts, one would like to exercise many values for them, but still get determinism. What would you think about having the fuzz input affect the random seed?

Certainly, this is possible and can be done on a case-by-case basis for fuzz targets where it would be useful.

💬 hebasto commented on pull request "Do not print "Reducing -maxconnections..." warning in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2553497647)
> You supplied `-j40` in your command, which is not the default.

By "default", I mean not providing `-j` at all. This is another run, not the first one with `-j 40`. I apologies for any confusion.

💬 hebasto commented on pull request "Do not print "Reducing -maxconnections..." warning in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2553501435)
> So the failure is expected and my recommendation would be to set the value to the max inbound count (maybe double or triple that).

Do we have a named constant for that in the test framework?

💬 maflcko commented on pull request "Do not print "Reducing -maxconnections..." warning in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2553506832)
No, the number of inbound connections depends on the test at runtime, depending of how many inbound connections are created.

💬 hebasto commented on pull request "Do not print "Reducing -maxconnections..." warning in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2553646587)
> the goal should be to not have any test-only hooks

Reworked. Only python code is modified now.

💬 A-Manning commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#discussion_r1891837715)
Added in edc34044362628f2994cb5fa5de74ca49fce0fd1

💬 maflcko commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1891893833)
This test and the commit 6d43aad742c7ea28303cf2799528188938e7ce32 that introduced it actually have many issues:

* `std::vector<bool>::data` never returned `void*`, only `void`, see https://godbolt.org/z/ePqGxjovv
* The reason why `std::vector<bool>` is not spannable is because the iterator is not contiguous, not due to the type of the data pointer
* `std::span` is more correct in that it does all the checks, so my recommendation would be to just remove the test and all related code, as expl
...