📝 l0rinc opened a pull request: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786)
Found while reviewing #32840

The `nullptr` witness path was dead in normal code paths: replacing it with reference enables us deleting unreachable logic.

Code coverage proof:
https://maflcko.github.io/b-c-cov/total.coverage/src/script/interpreter.cpp.gcov.html#L2135

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492188555)
Took the formats, thanks!

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492189914)
I removed the abort early logic, so we keep going if we don't find an input. It makes the logic much simpler, but we will do some more work if we get a block mined that is double spending.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492191544)
I've updated this to be an atomic_bool instead of this enum. So, since there is only a false value that is set to true, we have the main thread call `input.ready.wait(false, std::memory_order_acquire);`. This way we don't spin.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492194122)
@TheCharlatan thanks for fuzzing, and the diff for the fuzzer! I have taken it, and added you as a co-author :heart_hands:.

@l0rinc it is concerning that you are getting malloc errors. Are there any other details you can share about this?

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2492195430)
Done, this `Status` enum has been replaced. It is now just an `atomic_bool` `ready`.

💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488313837)
Reworked per feedback from @trevarj.

💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2492270996)
Thanks! [Reworked](https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488313837).

💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3488354051)
My Guix build:
```
aarch64
4e80325f9cba1c286a999eeaa6fb5f06bb56ba03f118cfb6eaa86c8443318ccc guix-build-dbb835956abf/output/dist-archive/bitcoin-dbb835956abf.tar.gz
7557b85092d69ac4f6ebeb4881a41fe808ee684b1b2f87efde97a616f1d1b213 guix-build-dbb835956abf/output/x86_64-w64-mingw32/SHA256SUMS.part
3cf561ff53f83f822fd451b2f83161908daad89598d286de916a641640abe747 guix-build-dbb835956abf/output/x86_64-w64-mingw32/bitcoin-dbb835956abf-win64-codesigning.tar.gz
f607587780890278ffcc9c4b346bcecc999
...

👍 darosior approved a pull request: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786#pullrequestreview-3418942549)
Neat. utACK 24bcad3d4df59690f30c9df8ebb62f0bddd0f1c7.

The need for a pointer was removed when `CTxInWitness` was moved into `CTxIn` in f6fb7acda4aefd01b8ef6cd77063bfc0c4f4ab36.

👋 l0rinc's pull request is ready for review: "script: remove dead code in `CountWitnessSigOps`"
(https://github.com/bitcoin/bitcoin/pull/33786)

👋 hebasto's pull request is ready for review: "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors"
(https://github.com/bitcoin/bitcoin/pull/33779)

💬 hebasto commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3488404139)
There are no conflicts with other contributors' PRs.

Friendly ping @l0rinc @maflcko @ryanofsky @willcl-ark who reviewed https://github.com/bitcoin/bitcoin/pull/31308, and @TheCharlatan, the kernel expert :)

🤔 l0rinc requested changes to a pull request: "test: Enhance GetTxSigOpCost tests for coinbase transactions"
(https://github.com/bitcoin/bitcoin/pull/32840#pullrequestreview-3418794264)
I have gone through the cases, I think we should take this opportunity and unify the test to use BOOST_ checkers for better error messages, to split the big test into smaller self-contained tests (otherwise the first failure will break the remaining ones - though this will result in some setup-repetition, but we can add helpers for those).

I have applied the cleanup that I would like to see here, if you decide to accept any of it, please do it in multiple focused commits.

<details>
<summary>De
...

💬 l0rinc commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2492222322)
this is already a coinbase tx, basically equivalent to the above, let's put this before mutating the spending tx, it's not related to it

💬 l0rinc commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2492225315)
After we make this a coinbase, it's confusing to refer to it as a "spending" transaction...
We could add another helper that copies it and makes it a coinbase

💬 l0rinc commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2492179386)
it seems to me `scriptWitness` is actually empty here, so the comment and code are a bit confusing here.
I think a `BOOST_REQUIRE(!spendingTx.vin[0].scriptWitness.IsNull());` would document the requirements here better than a comment.
`BOOST_CHECK_EQUAL(GetTransactionSigOpCost(CTransaction(creationTx), coins, flags), 0);` already checks that coinbases don't have sigops

💬 l0rinc commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#discussion_r2492229011)
I know you didn't introduce them, but these are basically self-contained tests that could be split out to separate `BOOST_AUTO_TEST_CASE`
* GetSigOpCount
* GetTxSigOpCost_Multisig
* GetTxSigOpCost_MultisigP2SH
* GetTxSigOpCost_P2WPKH
* GetTxSigOpCost_P2WPKH_P2SH
* GetTxSigOpCost_P2WSH
* GetTxSigOpCost_P2WSH_P2SH

💬 l0rinc commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3488414728)
code review ACK a8a33bc0c0a11093418debc36db8ac63bf90e687

💬 Ianilfy commented on something "":
(https://github.com/bitcoin/bitcoin/commit/1c7e820ded0846ef6ab4be9616b0de452336ef64#commitcomment-169646520)
I need help on my phrase