📝 fanquake opened a pull request: "build: add `-Wundef`"
(https://github.com/bitcoin/bitcoin/pull/29876)
Turn on `-Wundef`.

[> Warn if an undefined identifier is evaluated in an #if directive. Such identifiers are replaced with zero.](https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wundef).

Note that this is still beneficial with CMake, and may even be nice to have enabled prior, to catch any change in behaviour.

If we end up with this enabled, it should probably be enough to fix #16419.

📝 0xB10C opened a pull request: "tracing: cast block_connected duration to µs"
(https://github.com/bitcoin/bitcoin/pull/29877)
When the `validation:block_connected` tracepoint was introduced in 8f37f5c2a562c38c83fc40234ade9c301fc4e685, the connect block duration was passed in microseconds `µs`. By starting to use steady clock in fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 this changed to nanoseconds `ns`. As the test only checked if the duration value is `> 0` as a plausibility check, this went unnoticed. This was detected this when setting up monitoring for block validation time as part of the Great Consensus Cleanup Revi
...

💬 maflcko commented on pull request "tracing: cast block_connected duration to µs":
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2056849284)
How is is possible that this even compiled? I presume tracing just takes any type and value and passes it along as raw bytes, so `std::chrono` being type-safe does not help here?

📝 fanquake opened a pull request: "depends: build expat with CMake"
(https://github.com/bitcoin/bitcoin/pull/29878)
Bumps expat from `2.4.8` -> `2.6.2`
* Currently unreviewed, but done for now, given the amount of CMake related fixes listed in [the changelog](https://github.com/libexpat/libexpat/blob/R_2_6_2/expat/Changes).

Switch to building Expat with CMake, instead of Autotools.

🚀 fanquake merged a pull request: "ci: use Clang 16 for Valgrind"
(https://github.com/bitcoin/bitcoin/pull/29848)

💬 laanwj commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2056868984)
Concept ACK, issues with misspelled or otherwise missing defines can be really sneaky (and have caused some hard to find bugs in the past), this ia good check to have.

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-2056882691)
ready for review

💬 theStack commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565820341)
Good idea, done.

💬 theStack commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565820757)
> Nice catch! I wonder if a more wholistic way to avoid unintentionally creating tx packages is to gather all the (confirmed only) UTXOs at the beginning? e.g. [this](https://github.com/glozow/bitcoin/commits/2024-04-fill-mempool-29827/), but feel free to ignore

That seems like a good approach to avoid similar problems popping up in the future 👌 I've cherry-picked your commit (with small conflict resolutions to have it introduced before the new test commit).

Btw, other approaches I've
...

💬 theStack commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565822381)
Thanks, done.

💬 maflcko commented on pull request "build: Fix false positive `CHECK_ATOMIC` test":
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2056902062)
> On the master branch @ [0de63b8](https://github.com/bitcoin/bitcoin/commit/0de63b8b46eff5cda85b4950062703324ba65a80), a building `bitcoind` with clang-15 for `i686-pc-linux-gnu` fails to link:

Can you add exact steps to reproduce, ideally starting from a fresh install of the operating system?

💬 fjahr commented on pull request "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner":
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2056917422)
Concept NACK

As I have said before, I think there needs to be a discussion about using more Rust just for the sake of using more Rust in the project. The only motivation given so far is that @maflcko said that it could be done. There is no benefit to doing this otherwise apparently and the biggest effect I see is that there will be fewer people able to contribute to that part of the code base. And this change alone will also take up a lot of review effort so it appears to be a clear negative,
...

💬 instagibbs commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2056921899)
> Would this make things {broken, inconvenient} for potential users of TRUCs, e.g. by requiring swapping between v3 and v2?

without focusing too much on the LN use-case, anytime you wanted to use something larger, you'd have to make sure you do so, f.e. when sweeping a max size commitment tx with penalty path. Basically you'll have to check if your tx is larger than 25k and switch if so.

💬 fanquake commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2056925244)
> Want to upstream the crc32 patch to match the others we have sitting there?

Sure. Opened a PR in our crc32c subtree fork: https://github.com/bitcoin-core/crc32c-subtree/pull/7, and one in Google upstream: https://github.com/google/crc32c/pull/64.

🤔 glozow reviewed a pull request: "test: check disconnection when sending sendaddrv2 after verack"
(https://github.com/bitcoin/bitcoin/pull/29699#pullrequestreview-2001213888)
LGTM

💬 glozow commented on pull request "test: check disconnection when sending sendaddrv2 after verack":
(https://github.com/bitcoin/bitcoin/pull/29699#discussion_r1565855209)
(I think this is a good example of why we shouldn't use `assert_debug_log` when we're able to check the logic otherwise)

🚀 glozow merged a pull request: "test: check disconnection when sending sendaddrv2 after verack"
(https://github.com/bitcoin/bitcoin/pull/29699)

🚀 glozow merged a pull request: "rpc, bugfix: Enforce maximum value for setmocktime"
(https://github.com/bitcoin/bitcoin/pull/29869)

💬 maflcko commented on pull request "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner":
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2056954530)
> As I have said before, I think there needs to be a discussion about using more Rust just for the sake of using more Rust in the project. The only motivation given so far is that @maflcko said that it could be done.

Not sure where you get that from, but I never said that using more Rust for the sake of using more Rust can be done. I said something else:

* `I *don't* think moving all Python code to Rust makes sense.` https://www.github.com/bitcoin/bitcoin/pull/29744#issuecomment-2022526794
...

💬 fjahr commented on pull request "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner":
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2056968267)
> Not sure where you get that from

"an alternative might be to rewrite the 3 lint checks to rust" https://github.com/bitcoin/bitcoin/pull/29479#pullrequestreview-1957109397 That is what @davidgumberg has given as the motivation in his description. It does not matter what you have said in other places because I have not been talking about that. I only talked about your comment that the author took as the motivation for this PR.