💬 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/
...

💬 maflcko commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2309281175)
It is less code this way:

```diff
diff --git a/src/test/fuzz/difference_formatter.cpp b/src/test/fuzz/difference_formatter.cpp
index 49e859ffdc..38b54352c1 100644
--- a/src/test/fuzz/difference_formatter.cpp
+++ b/src/test/fuzz/difference_formatter.cpp
@@ -4,25 +4,13 @@

#include <blockencodings.h>
#include <streams.h>
-#include <test/fuzz/FuzzedDataProvider.h>
+#include <random.h>
#include <test/fuzz/fuzz.h>

#include <vector>

namespace {
-// Test struct for VectorFo
...

🤔 stickies-v reviewed a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3189575123)
Concept ACK, but I think unconditional logging in the ctor is not the right approach, suggested alternative:

<details>
<summary>git diff on 689a321976</summary>

```diff
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
index 79e2c9688d..11d7c9029d 100644
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -19,6 +19,7 @@
#include <util/moneystr.h>
#include <util/translation.h>
#include <wallet/coincontrol.h>
+#include <wallet/sqlite.h>
#include <wallet/wallet.h>
#in
...

💬 stickies-v commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344204033)
To make this more visible than just the log line, you can use `node::warnings` to expose it through RPC (e.g. `getnetworkinfo["warnings"]` and GUI:

<details>
<summary>git diff on 080a21cb12</summary>

```diff
diff --git a/src/init.cpp b/src/init.cpp
index caf1515804..782cb7f6c4 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -56,6 +56,7 @@
#include <node/mempool_persist.h>
#include <node/mempool_persist_args.h>
#include <node/miner.h>
+#include <node/warnings.h>
#include <nod
...

💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2347458082)
If any of you feel strongly, I can change it, but it was a deliberate choice from my part.
See https://godbolt.org/z/o3sKqefT5 for an ugly error on Windows that also convinced me to just use the shorter version. I could of course hack it with:
```C++
#ifdef _WIN32
#include <windows.h>
#undef max
#else
```
or just hope that https://github.com/bitcoin/bitcoin/blob/9f744fffc39d93c9966dec1d61f113a7521983ad/CMakeLists.txt#L261 fixes it, but `SIZE_MAX` seemed simpler and shorter in this case :/

-
...

💬 w0xlt commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2347899862)
Just to make clearer what `60 * 60 * 24 * 7 * 2` means.

```cpp
constexpr int64_t TWO_WEEKS = 14 * 24 * 60 * 60;
if (GetBlockProofEquivalentTime(... ) <= TWO_WEEKS) { ... }
```

Or (not tested)

```cpp
#include <chrono>
using namespace std::chrono;

constexpr auto TWO_WEEKS = 14d; // 14 days

if (GetBlockProofEquivalentTime(...) <= duration_cast<seconds>(TWO_WEEKS).count()) {
...
}
```

💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2349820667)
Please add `#include <cstdlib>` for [std::abort()](https://en.cppreference.com/w/cpp/utility/program/abort)

💬 TheCharlatan commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2352862088)
I made a small fuzz test now. If you think it is useful, and want to add it:

```c++
// Copyright (c) 2025-present 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 <util/threadpool.h>

#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>

namespace {

struct ExpectedException : std::runtime_error {
using std::runtime_error::runtime_er
...

👍 stickies-v approved a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3284021822)
ACK fc861332b351c9390400054ff74193ce26eb0713

nit: Would still prefer doing the logging from `WalletInit::Construct`, `VerifyWallets` feels out of place to me, but no biggie.

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

```diff
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
index 79e2c9688d..4013e12967 100644
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -20,6 +20,7 @@
#include <util/translation.h>
#include <wallet/coincontrol.h>
#include <wallet/walle
...