💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1836793220)
Fixed.

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#issuecomment-2468354278)
Fixed the test-each-commit failure and addressed comment.

💬 fanquake commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2468376483)
Guix Build
```bash
b526b46943f39780cb2a204cd64159f7dd4f502e482256ba78371c82d717a591 guix-build-5a96767e3f53/output/aarch64-linux-gnu/SHA256SUMS.part
77714a2034b96c572598342da2147a638abd626b29e4ccecfe786f53dbf72a4e guix-build-5a96767e3f53/output/aarch64-linux-gnu/bitcoin-5a96767e3f53-aarch64-linux-gnu-debug.tar.gz
a32ce8d85863ec00f75b37ab7b10bfc37e85efd3a1016ed66ce6cbd0024d018a guix-build-5a96767e3f53/output/aarch64-linux-gnu/bitcoin-5a96767e3f53-aarch64-linux-gnu.tar.gz
751eba9ba4c9c2432
...

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1836809045)
Fixed.

👍 fanquake approved a pull request: "cmake: Revamp `FindLibevent` module"
(https://github.com/bitcoin/bitcoin/pull/31181#pullrequestreview-2427529805)
ACK 5a96767e3f531ba9e8a676eec47727421f9f589f

💬 hodlinator commented on pull request "util: Narrow scope of args (-color, -version, -conf, -datadir)":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2468421728)
> > Disallow -noconf since a config file is required
>
> But it's not. Seems like `-noconf` should work to ignore the config file.

As mentioned in the commit message for 2d65e6ae0756b87e82dbafd032308e8687ec7941:

> ArgsManager::GetConfigFilePath() asserts that a path is always set. Explicitly negating it is therefore invalid."
>
> Previously, -noconf would result in an ifstream being opened to the ".bitcoin"-directory (not a file) in ArgsManager::ReadConfigFiles(), also circumventing t
...

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836842757)
> I don't think it is right to use mockable time here. Also, GetTime is deprecated, so what about TicksSinceEpoch<std::chrono::nanoseconds>(SystemClock::now())?

Yeah sure. This is usually called before mocking the time, but agree to change it.

> Also, I think it would be better to switch test_name / rand_str to rand_str / test_name, so that all unit tests from the same time are bundled together. This would also be identical to the functional test runner.

That won’t work as you expect. T
...

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2468440155)
Rebased for renaming `TRACE5` -> `TRACEPOINT`.

💬 sipa commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2468443487)
Concept ACK. If it's possible to have differently-compiled compilation units without needing separate libraries for each, then this does seem like a cleaner solution. I suspect we perhaps ended up in this state because in autotools separate libraries were the only way to achieve this?

💬 l0rinc commented on pull request "util: Narrow scope of args (-color, -version, -conf, -datadir)":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2468446882)
Concept ACK

👍 rkrux approved a pull request: "doc: correct typos"
(https://github.com/bitcoin/bitcoin/pull/31271#pullrequestreview-2427587276)
ACK 726cbee9553b25bedfef70cfd5be9f1eeec8a30d

🚀 fanquake merged a pull request: "cmake: Revamp `FindLibevent` module"
(https://github.com/bitcoin/bitcoin/pull/31181)

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2468455953)
re-ACK 6c7b3bf0ce7a6238457dd44ee1d5d744b3663a52

💬 furszy commented on pull request "test: report detailed msg during utf8 response decoding error":
(https://github.com/bitcoin/bitcoin/pull/31251#discussion_r1836872860)
sure, done as suggested. Thanks

💬 brunoerg commented on pull request "fuzz: fix bad alloc in connman target":
(https://github.com/bitcoin/bitcoin/pull/31235#issuecomment-2468489703)
Force-pushed squashing and addressing https://github.com/bitcoin/bitcoin/pull/31235#discussion_r1836765925.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2468489763)
re-crACK 9edbe5a48fe9a2c0d364882fcb7d44ae6aac6d24

💬 brunoerg commented on pull request "fuzz: fix bad alloc in connman target":
(https://github.com/bitcoin/bitcoin/pull/31235#discussion_r1836877155)
Done.

💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1836877971)
minor nit: I don't think this one needed to be renamed to `pair`. `self` still makes sense here.

💬 hodlinator commented on pull request "util: Narrow scope of args (-color, -version, -conf, -datadir)":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2468495898)
Added commit which now guards against passing directory paths as `-conf` values as per https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2468421728.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1836880690)
yeah, but it's more consistent this way (and method name already mentions self)