📝 fanquake opened a pull request: "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job"
(https://github.com/bitcoin/bitcoin/pull/30519)
Add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to one of the libc++ CI jobs, to catch missing includes, that are otherwise hidden by transitive includes inside libc++. A more appropriate place for this might be the tidy job, but that does not use libc++.

See https://libcxx.llvm.org/DesignDocs/HeaderRemovalPolicy.html for more information.

🤔 danielabrozzoni reviewed a pull request: "refactor: Add FlatFileSeq member variables in BlockManager"
(https://github.com/bitcoin/bitcoin/pull/30517#pullrequestreview-2196466308)
ACK 7aa8994c6fceae5cf8fb7e661371cdb19d2cb482

💬 glozow commented on pull request "m_tx_download_mutex followups":
(https://github.com/bitcoin/bitcoin/pull/30507#discussion_r1689635561)
done

💬 glozow commented on pull request "m_tx_download_mutex followups":
(https://github.com/bitcoin/bitcoin/pull/30507#discussion_r1689635405)
Hm, added a `ACQUIRED_BEFORE`, but compiler didn't complain when I added a `LOCK2(m_mempool.cs, m_tx_download_mutex)`

💬 glozow commented on pull request "m_tx_download_mutex followups":
(https://github.com/bitcoin/bitcoin/pull/30507#discussion_r1689635780)
done

👋 glozow's pull request is ready for review: "m_tx_download_mutex followups"
(https://github.com/bitcoin/bitcoin/pull/30507)

💬 maflcko commented on pull request "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job":
(https://github.com/bitcoin/bitcoin/pull/30519#discussion_r1689634589)
nit: The same can be achieved, by setting C++23, according to the docs.

This may be beneficial for other reasons as well, even if it is just turning existing compile warnings into errors (https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1675775175)

So adding `-std=c++23` here, and a comment that `libc++` is required for the feature seems better. But this is fine either way.

💬 maflcko commented on pull request "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job":
(https://github.com/bitcoin/bitcoin/pull/30519#discussion_r1689636199)
nit: If it compiles, while touching, could fix all headers?

```
init.h should add these lines:
#include <atomic> // for atomic

init.h should remove these lines:
- #include <any> // lines 9-9
- #include <memory> // lines 10-10
- #include <string> // lines 11-11

👍 maflcko approved a pull request: "ci: add `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` to TSAN (libc++) job"
(https://github.com/bitcoin/bitcoin/pull/30519#pullrequestreview-2196512417)
lgtm

This is useful to catch build failures before they hit users that compile from source.

💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1689639934)
Matches my diagrams. Thanks for the additional context! :+1:

👍 tdb3 approved a pull request: "refactor: Add FlatFileSeq member variables in BlockManager"
(https://github.com/bitcoin/bitcoin/pull/30517#pullrequestreview-2196603604)
ACK 7aa8994c6fceae5cf8fb7e661371cdb19d2cb482

👍 TheCharlatan approved a pull request: "Early logging improvements"
(https://github.com/bitcoin/bitcoin/pull/30386#pullrequestreview-2196253338)
Nice, ACK b4dd7ab43e8cfc2c171f67588e4e1ec2705393c2

💬 TheCharlatan commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1689628041)
Nit: Why is this being set again? I'm guessing it is not declared as `const` to future-proof it against introducing an option controlling its size, but it is not clear why disconnecting should clear such a value again.

💬 TheCharlatan commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1689526333)
Nit: Unneeded newline.

💬 hebasto commented on pull request "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)":
(https://github.com/bitcoin/bitcoin/pull/28923#issuecomment-2247822717)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/280.

💬 hebasto commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#issuecomment-2247823778)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/280.

💬 hebasto commented on pull request "depends: Bump `libmultiprocess` for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30513#issuecomment-2247862187)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/277.

💬 hebasto commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2247865660)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/278.

🤔 stratospher reviewed a pull request: "net: Log accepted connection after m_nodes.push_back; Fix intermittent test issue"
(https://github.com/bitcoin/bitcoin/pull/30512#pullrequestreview-2196772985)
tested ACK fa3ea3b.

💬 fanquake commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#discussion_r1689861943)
Added.

💬 fanquake commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2248024050)
> i'd be okay with skipping the check for bitcoin-util: it's the least relevant binary for fortification (no network access, not even file format access). Could reconsider it later if it actually gains some useful functionality 😄

I think I agree, and I've added that skipping now.