💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2156780094)
nit: requires `#include <utility>`

💬 fanquake commented on pull request "doc: Remove build instruction for running `clang-tidy`":
(https://github.com/bitcoin/bitcoin/pull/32662#issuecomment-2988172883)
> Also, would be good to mention that this will now print errors about the missing auto-generated files.

Yea, I think we could actually revert this. It mostly *works*, but as you say, now produces errors due to missing files, i.e:
```bash
/root/ci_scratch/src/test/sighash_tests.cpp:13:10: error: 'test/data/sighash.json.h' file not found [clang-diagnostic-error]
13 | #include <test/data/sighash.json.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
694 warnings and 1 error generated.
```
...

💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#issuecomment-3021687570)
@SomberNight

Compilation of potential fixes for PR #32844:

## 1. Witness nonce issue
The witness commitment wasn't being calculated correctly. Added in `src/rpc/txoutproof.cpp`:

First, add required includes at the top:
```cpp
#include <hash.h>
#include <logging.h>
```

Then after line 220:

```cpp
const auto wtxid_root = merkleBlock.m_wtxid_tree.ExtractMatches(wtxid_matches, wtxid_indexes);

// Extract witness nonce from coinbase input
if (merkleBlock.m_gentx->vin.empty() || merkleBlock.m_ge
...

💬 fanquake commented on pull request "refactor: Drop unused `#include <boost/operators.hpp>`":
(https://github.com/bitcoin/bitcoin/pull/32668#issuecomment-3027713936)
Yea, I'm not sure what is the best approach here. However I don't think a good way forward is PR's like this, based on IWYU output, which simultaneously ignore other IWYU output (and no explanation for devs that might otherwise try to make changes based on the output). I think any changes should be a part of #31308.

💬 maflcko commented on pull request "refactor: Drop unused `#include <boost/operators.hpp>`":
(https://github.com/bitcoin/bitcoin/pull/32668#issuecomment-3027737893)
>I think any changes should be a part of #31308.

I'd say it is also fine to do it in smaller steps, but no strong opinion.

✅ hebasto closed a pull request: "refactor: Drop unused `#include <boost/operators.hpp>`"
(https://github.com/bitcoin/bitcoin/pull/32668)

💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2179755821)
In 7c4eb58fb0:

I don't really see the point of using `util::Overloaded` here when we're always doing a `Wtxid` query?

This looks more straightforward to me:

<details>
<summary>git diff on 7c4eb58fb0</summary>

```diff
diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp
index a16490ceb9..87a47b30ee 100644
--- a/src/node/txdownloadman_impl.cpp
+++ b/src/node/txdownloadman_impl.cpp
@@ -9,7 +9,6 @@
#include <consensus/validation.h>
#include <logging.h>
...

💬 fjahr commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#discussion_r2216982231)
nit: could do IWYU I guess, even though this wasn't followed that well in the index cpp files.

```
#include <dbwrapper.h>
#include <interfaces/types.h>
```

⚠️ totdking opened an issue: "A missing import to the src/chainparamsbase.h"
(https://github.com/bitcoin/bitcoin/issues/33019)
Tried running the make -j $CORES as the bitcoin repo is important to what I'm working with.

The import that is missing is the ```#include <cstdint>```, it gave a plethora of errors.

This is not a bug but i just wanted the community to have a peaceful and seamless integration while building the repo

📝 hebasto opened a pull request: "refactor: Move `FreespaceChecker` class into its own module"
(https://github.com/bitcoin-core/gui/pull/881)
The MOC compiler in older versions of Qt 6 fails to parse `qt/intro.cpp`, as noted in [this comment](https://github.com/bitcoin/bitcoin/pull/32998#issuecomment-3082011233).

This PR proposes a move-only refactoring to simplify the source structure by eliminating the need for the inline `#include <qt/intro.moc>`, thereby effectively working around the issue.

💬 ajtowns commented on pull request "refactor: Move `FreespaceChecker` class into its own module":
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3130725962)
ACK dd392a64bb0608847f771f8b1f09c2fcae146923

Shouldn't the copyright dates in these files be updated? `git blame` has some lines from 2022 in intro.h and 2024/25 in intro.cpp.

You could avoid the circularity with something like:

```diff
diff --git a/src/qt/freespacechecker.h b/src/qt/freespacechecker.h
index d3a61a11571..214324be7c8 100644
--- a/src/qt/freespacechecker.h
+++ b/src/qt/freespacechecker.h
@@ -9,8 +9,6 @@
#include <QString>
#include <QtGlobal>

-class Intro;
-

...

💬 hodlinator commented on pull request "test,refactor: extract script template helpers & widen sigop count coverage":
(https://github.com/bitcoin/bitcoin/pull/32729#discussion_r2260374450)
Since we're adding the *secp256k1.h* `#include` in commit 17922e88153b07f6146f5740dde69e8a3587220a, why not use `SECP256K1_TAG_PUBKEY_EVEN`/`..._ODD` directly instead of changing to them in 90df1ccfca9c0adfd1c61c1b07d8911ab632bc6a?

💬 stickies-v commented on pull request "refactor: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/33116#discussion_r2262703752)
nit: could modernize

<details>
<summary>git diff on a20724d926</summary>

```diff
diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp
index c26355d832..a1c2edcb7b 100644
--- a/src/node/mini_miner.cpp
+++ b/src/node/mini_miner.cpp
@@ -16,6 +16,7 @@

#include <algorithm>
#include <numeric>
+#include <ranges>
#include <utility>

namespace node {
@@ -61,12 +62,8 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& ou
if (m_request
...

💬 ryanofsky commented on issue "depends: `native_libmultiprocess` fails to build on OpenBSD":
(https://github.com/bitcoin/bitcoin/issues/33219#issuecomment-3202305628)
Thanks for the report and clear reproduction steps. Following change fixes the problem for me, and I guess this was caused by the IWYU commit in https://github.com/bitcoin-core/libmultiprocess/pull/184,

```
--- a/src/ipc/libmultiprocess/src/mp/util.cpp
+++ b/src/ipc/libmultiprocess/src/mp/util.cpp
@@ -16,6 +16,7 @@
#include <sys/socket.h>
#include <sys/wait.h>
#include <system_error>
+#include <thread>
#include <unistd.h>
#include <utility>
#include <vector>
```

An alternate change also
...

🤔 l0rinc requested changes to a pull request: "bench: Add more realistic Coin Selection Bench"
(https://github.com/bitcoin/bitcoin/pull/33160#pullrequestreview-3157264436)
Concept ACK, the existing test were all quite trivial compared to this new one.
I left a ton of comments, for simplicity here's the final code that I'm suggesting, feel free to pick and choose from it.

<details>
<summary>Details</summary>

```C++
// Copyright (c) 2012-2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <bench/bench.h>
#include <consensus/
...