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