🤔 hebasto reviewed a pull request: "build: replace header checks with `__has_include`"
(https://github.com/bitcoin/bitcoin/pull/32405#pullrequestreview-2812387753)
Concept ACK. This improves code locality.

🤔 marcofleon reviewed a pull request: "fuzz: Remove unused TimeoutExpired catch in fuzz runner"
(https://github.com/bitcoin/bitcoin/pull/32388#pullrequestreview-2812454130)
crACK fa4804009ceba96926edd7f55ea22442ebdc665d

💬 l0rinc commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071924816)
`m_was_written = true`, since `AutoFile::write_buffer` also dirties (which is the term we use in other cases, if you think it describes the behavior better, consider renaming)

👍 hebasto approved a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086#pullrequestreview-2812537498)
re-ACK 7238f242d78f3d71834764031d7904d19525bab3.

💬 l0rinc commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071926224)
I find returning error codes for boolean values a lot more confusing.

🤔 mabu44 reviewed a pull request: "test: remove Boost SIGCHLD workaround."
(https://github.com/bitcoin/bitcoin/pull/32403#pullrequestreview-2812561636)
ACK 3add6ab9adcd722d57c6d488581358ae9b377f1a

<details>
<summary>Testing procedure</summary>

Reviewed the changes and compiled the code, then run
```bash
./build_dev/bin/test_bitcoin --log_level=all --run_test=util_tests/test_LockDirectory
```
The output of the test looks good to me.
</details>

📝 AlnetharyT opened a pull request: "النظاري (#1)"
(https://github.com/bitcoin/bitcoin/pull/32407)
* versionbits: Use std::array instead of C-style arrays

* versionbits: Remove params from AbstractThresholdConditionChecker

For an abstract class, specifying parameters in detail serves no point; and for the concrete implementation, changing the consensus parameters between invocations doesn't make sense. So simplify the class by removing the consensus params from the method arguments, and just make it a member variable in the concrete object where needed. This also allows dropping dummy p
...

✅ fanquake closed a pull request: "النظاري (#1)"
(https://github.com/bitcoin/bitcoin/pull/32407)

💬 monlovesmango commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2071988479)
The summary at the end should probably be removed now?

💬 monlovesmango commented on pull request "tests: Added progress tracker when running fuzz test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/32354#discussion_r2071991488)
Was there a reason you prefer to use list arguments rather than integers that store the counts? Doesn't really matter since this is just a fuzz test runner, but seems likes an unnecessary use of lists.

🤔 pinheadmz reviewed a pull request: "Extend signetchallenge to set target block spacing"
(https://github.com/bitcoin/bitcoin/pull/29365#pullrequestreview-2812313762)
code reviewed at 41728b516e

Built and tested on arm64/macos. Played with the feature locally using 10 and 30 second block times. A few comments below, my main concern being a mismatch between the python miner and the custom consensus rules. Didn't have time to try and mine 2, 016 signet blocks to check that difficulty adjusted as expected, but otherwise everything seems to work as expected.

Consensus code is touched, but only for signet ;-)

💬 pinheadmz commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r2071792992)
> currently the only supported parameter is target spacing, the format of <params> to set it is 01<8 bytes value of target spacing, seconds, little endian>

I think you can remove this, it could get carried away if more parameters are added in the future. You could just link to verbose documentation in signet/README.md or reccomend using signet/miner, which leads me to a second suggestion:

Add a new command to signet/miner like `generate`, `solvepsbt` etc... maybe `makechallenge` that accep
...

💬 pinheadmz commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r2071904374)
Why not pass `options` and `args` to one function `ParseSignetChallenge()` that calls the two functions you have, and fills in the `SigNetOptions` members where applicable? In other words, why write data to these vectors?

💬 pinheadmz commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r2071915516)
I would expect to see a loop here that parses the byte vector:

1. read byte -- if its unknown (not `01`, currently) throw an InitError
2. for byte == `01` read the next 8 bytes to set the value
3. repeat for all future extensions

💬 pinheadmz commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r2071913713)
Don't we have a serializer helper method for this kind of thing?

🤔 l0rinc requested changes to a pull request: "util: explicitly close all AutoFiles that have been written"
(https://github.com/bitcoin/bitcoin/pull/29307#pullrequestreview-2812640885)
Code review fe2d1955f6333c57f54aed2a2a8b3cdaa6bf9d71

* blocker: the new `AutoFile::write_buffer` should also mark the file as dirty.
* the current `fclose` returns a fake error code, but is actually a boolean - may make sense to rename it (e.g. `Close`, similarly to how we've wrapped `ftruncate` with `TruncateFile`) and migrate to returning a boolean to avoid the current confusion.

<details>
<summary>Diff since last review</summary>

```patch
diff --git a/src/index/blockfilterindex.cp
...

💬 l0rinc commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2071988064)
> because here from the destructor

Still not sure what this means exactly, would this maybe work better?

```suggestion
// This is because here in the destructor we have no way to signal
// errors from close(), which, after write, could mean the file is
```

💬 l0rinc commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2072005944)
do we need both messages? How will this be displayed in the logs, isn't it redundant?

💬 hebasto commented on issue "ctest -C Debug fails on vs2022 (miniscript_tests (SEGFAULT))":
(https://github.com/bitcoin/bitcoin/issues/32341#issuecomment-2847867432)
> > A third question would be, why the fuzz output is https://github.com/hebasto/bitcoin/actions/runs/14650416249/job/41114604643#step:14:60: `subprocess timed out: Currently only libFuzzer is supported`. A timeout should not happen for this process call, and the expected outcome should be an immediate stderr of `test/fuzz/fuzz.cpp:269 main: Assertion 'read_file(input_path, buffer)' failed. Error processing input "-help=1"` and an ignored exit code.
>
> Looks like the assertions fails, but the
...

📝 pinheadmz opened a pull request: "tests: Expand HTTP coverage to assert libevent behavior"
(https://github.com/bitcoin/bitcoin/pull/32408)
These commits are cherry-picked from #32061 and part of a project to [remove libevent](https://github.com/bitcoin/bitcoin/issues/31194).

This PR only adds functional tests to `interface_http` to cover some HTTP server behaviors we inherit from libevent, in order to maintain those behaviors when we replace libevent with our own HTTP server.

1. Pipelining: The server must respond to requests from a client in the order in which they were received [RFC 7230 6.3.2](https://www.rfc-editor.org/r
...