💬 fanquake commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3491494704)
> Would be possible relatively easily, I think, if there's interest.

I'm wondering what problem we are trying to solve. Externallising this would just lead to random PRs to this repo, to "fix" includes, which can't be verified here? If developers aren't actively engaged in doing that, and no CI will ever fail, that sounds like endless churn.

💬 TheCharlatan commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3491515263)
Maybe a better approach would be to run the enforced sections in a separate, faster job? Some of the linters are already a bit annoying to invoke locally, so I usually just run the lint job. Doing the same for the includes seems fine to me.

💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3491554129)
Guix Build (x86_64):
```bash
6c31a0f19daec94c3d629e50d6bc2461ceb6e0fbb0823cb10a40bc735e37f9b5 guix-build-25ede968bada/output/aarch64-linux-gnu/SHA256SUMS.part
f3ba2f0c5f1d99d377ea11ea2b9983187192d733c7db444060d112d1be48ecd2 guix-build-25ede968bada/output/aarch64-linux-gnu/bitcoin-25ede968bada-aarch64-linux-gnu-debug.tar.gz
321db43f3a4327287813274b7f9874fb76799643fdc9cbbc9ee13bce10b6e004 guix-build-25ede968bada/output/aarch64-linux-gnu/bitcoin-25ede968bada-aarch64-linux-gnu.tar.gz
58f4490
...

💬 willcl-ark commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3491572687)
```
src/core/bitcoin on  pr-33775 [$?] via △ v3.31.6 via 🐍 v3.13.5 via ❄️ impure (nix-shell-env)
❯ uname -m; find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
x86_64
6c31a0f19daec94c3d629e50d6bc2461ceb6e0fbb0823cb10a40bc735e37f9b5 guix-build-25ede968bada/output/aarch64-linux-gnu/SHA256SUMS.part
f3ba2f0c5f1d99d377ea11ea2b9983187192d733c7db444060d112d1be48ecd2 guix-build-25ede968bada/output/aarch64-linux-gnu/bitcoin-25ed
...

💬 stickies-v commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2494809479)
It's a nit, so not a blocker either way. The debug value would come from knowing which block was rolled back/forward before some other event happened. Sure it's noisy when the roll{back/forward} happens but those events are rare so shouldn't be an issue in general, and when debug is enabled during these events, I think it's quite likely they're relevant information?

If not adopted here, I'm not sure it's worth opening a follow-up until people actually want the behaviour changed. Just seems li
...

💬 maflcko commented on pull request "ci, iwyu: Fix warnings in `src/kernel` and treat them as errors":
(https://github.com/bitcoin/bitcoin/pull/33779#issuecomment-3491592008)
> I'm wondering what problem we are trying to solve.

I think the problem is that it is inherently impossible to reproduce an iwyu run locally without the exact CI config. Maybe this is fine, and nothing needs to be fixed. Though, if devs think that relying exactly on the CI is too tedious, then a dashboard could seem like a nice solution.

> endless churn

I think iwyu will mean churn, regardless of how it is enforced, if it is run on more actively changed modules of the codebase.

I gu
...

👍 rkrux approved a pull request: "kernel: Use enumeration type for flags argument"
(https://github.com/bitcoin/bitcoin/pull/33791#pullrequestreview-3422514325)
lgtm ACK ed5720509f03ddd7f9158b9adf0d8fd7f56c8578 as per the mentioned review comment of the previous PR.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2494844175)
> Can we assert the behavior of the main cache as well so that the previous version doesn't pass?

Neither the temp cache or the main cache touch their backing cache in the input fetcher. So, we can't assert a failure if the backing cache is something else. We can assert that the backing view is not touched during `FetchInputs`, which is done in the fuzz harness.

👍 brunoerg approved a pull request: "test: remove obsolete `get_{key,multisig}` helpers from wallet_util.py"
(https://github.com/bitcoin/bitcoin/pull/33782#pullrequestreview-3422561866)
code review ACK ec8516ceb7568d7b09836b830023978bd37f8462

💬 alexanderwiederin commented on pull request "kernel: Use enumeration type for flags argument":
(https://github.com/bitcoin/bitcoin/pull/33791#issuecomment-3491703847)
ACK https://github.com/bitcoin/bitcoin/pull/33791/commits/ed5720509f03ddd7f9158b9adf0d8fd7f56c8578

👍 stickies-v approved a pull request: "kernel: Use enumeration type for flags argument"
(https://github.com/bitcoin/bitcoin/pull/33791#pullrequestreview-3422655653)
ACK ed5720509f03ddd7f9158b9adf0d8fd7f56c8578

🤔 pablomartin4btc reviewed a pull request: "doc: add cmake help option in Windows build docs"
(https://github.com/bitcoin/bitcoin/pull/33789#pullrequestreview-3422616007)
ACK 9577daa3b88a8fbe8c96d2848adcc88c55c55a29

_(please try to avoid mentions/ @-prefixed github names in PR descriptions as it can cause potential issues, primarily related to notification spam)_

💬 pablomartin4btc commented on pull request "doc: add cmake help option in Windows build docs":
(https://github.com/bitcoin/bitcoin/pull/33789#discussion_r2494905335)
nit:
```suggestion
cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
(run `cmake -B build -LH` to see the full list of available options)

```

📝 vasild opened a pull request: "net_processing: reorder the code that handles the VERSION message"
(https://github.com/bitcoin/bitcoin/pull/33792)
Change the order in which code snippets are executed as a result of receiving the `VERSION` message. Move the snippets that do `MakeAndPushMessage()` near the end. This makes it easier to interrupt the execution when no messages should be sent as a response to the `VERSION` messages, in private broadcast connections.

This is a non-functional change.

---

This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull
...

💬 vasild commented on pull request "net_processing: reorder the code that handles the VERSION message":
(https://github.com/bitcoin/bitcoin/pull/33792#issuecomment-3491723472)
This is just moving code around. Easier reviewed with `~/.gitconfig` like:

```gitconfig
[diff]
colorMoved = dimmed-zebra
colorMovedWS = allow-indentation-change
```

🚀 fanquake merged a pull request: "kernel: Use enumeration type for flags argument"
(https://github.com/bitcoin/bitcoin/pull/33791)

💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3491797678)
Windows is non-deterministic across x86_64 && aarch64.
Guix build (aarch64):
```bash
6c31a0f19daec94c3d629e50d6bc2461ceb6e0fbb0823cb10a40bc735e37f9b5 guix-build-25ede968bada/output/aarch64-linux-gnu/SHA256SUMS.part
f3ba2f0c5f1d99d377ea11ea2b9983187192d733c7db444060d112d1be48ecd2 guix-build-25ede968bada/output/aarch64-linux-gnu/bitcoin-25ede968bada-aarch64-linux-gnu-debug.tar.gz
321db43f3a4327287813274b7f9874fb76799643fdc9cbbc9ee13bce10b6e004 guix-build-25ede968bada/output/aarch64-linux-
...

📝 vasild opened a pull request: "test: move create_malleated_version() to messages.py for reuse"
(https://github.com/bitcoin/bitcoin/pull/33793)
Move `create_malleated_version()` from `p2p_orphan_handling.py` to `test_framework/messages.py` so that it can be reused by other tests.

---

This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull/29415). Putting it in its own PR to reduce the size of #29415 and because it does not depend on the other commits from there.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3491839226)
Extracted two more commits into separate smaller PRs for easier review:

https://github.com/bitcoin/bitcoin/pull/33792
https://github.com/bitcoin/bitcoin/pull/33793

👍 theStack approved a pull request: "Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found"
(https://github.com/bitcoin/bitcoin/pull/33433#pullrequestreview-3422901949)
Tested ACK 79b4c276e7b9b526fa8f563b1e09b2b970baece6

Verified also in a local branch rebased on master, given that the merge-base is extremely old (>7 years).