💬 sipa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#issuecomment-3013244578)
The reason why `FeeFrac` supports negative fees and negative sizes is because that allows it to be used on "set differences" (*), which was at some point expected to be used inside the cluster linearization algorithm. It isn't in the current version in master, however, but it ended up being very useful in PR #32545 (see [this line](https://github.com/bitcoin/bitcoin/pull/32545/commits/68ce630abd669b86d9e2ff270aa2ba1e7332f95a#diff-1433c1fc4926a466291656ba67cf6b029523e4bd5da177ade812f25edf07343cR9
...

🤔 furszy reviewed a pull request: "rpc: use CScheduler for HTTPRPCTimer"
(https://github.com/bitcoin/bitcoin/pull/32796#pullrequestreview-2966805286)
ACK d06942c6731d5db7326bc565655b33a379a5d9b0

> I think it makes more sense to move the validation signals to their own scheduler thread if we are really concerned scheduled events will slow down block validation. walletpassphrase is currently the only RPC that schedules a new event, so all this PR will add to the queue are calls to CWallet::Lock() after the user needs a private key

Sounds fair to me.

💬 Sjors commented on pull request "rpc: Handle -named argument parsing where Base64 encoding is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3013274110)
> > If possible it would be good to add python tests to check the new behavior
>
> Sure, I'm also thinking of adding a python test, but the main problem is, this issue is only reproducible when there exists `=` padding or in between, so while creating a PSBT, I need to look at how I can achieve this.

You could just manually generate a random PSBT on regtest, hardcode it into a test and call `analyzepsbt`. It doesn't matter if it's unspendable.

Or you could even just call these methods w
...

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2172171331)
I think it applies to `BOOST_REQUIRE_GT(result.pow_validated_headers.size(), 0);` as well

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2172173668)
I don't like the new repetition - will have to debug it locally to understand why we run the condition outside of the loop in the first place 🤔

💬 l0rinc commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3013338371)
Code review re-ACK 9adf381d810f11b997ea036da6e2ce7dbad74cf6

Someone else with more experience should also review it, I have only lightly tested it locally.

💬 l0rinc commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2172225961)
you could store it as a multiplication instead of the comment:
```suggestion
HEADERS_DOWNLOAD_TIMEOUT_BASE_SEC = 15 * 60
```

💬 ryanofsky commented on pull request "Add read-only mode to sqlite db and use in `bitcoin-wallet`":
(https://github.com/bitcoin/bitcoin/pull/32818#issuecomment-3013382781)
Note IIRC, the SQLITE_OPEN_READONLY flag only guarantees that sqlite won't try to write to the main database file, not any wal or jrn files associated with the database. So if the goal is being able to access wallets on read-only filesystems this may not always work and the [immutable](https://www.sqlite.org/uri.html#uriimmutable) mode might need to be used instead.

💬 stringintech commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#issuecomment-3013414743)
[8d257ed](https://github.com/bitcoin/bitcoin/commit/8d257ed5937a47e1a60b7fe085f2e984eb60a956) to [1154618](https://github.com/bitcoin/bitcoin/commit/11546183c70def6c0aa539642fd1c9ada3d46840) ([compare](https://github.com/bitcoin/bitcoin/compare/8d257ed5937a47e1a60b7fe085f2e984eb60a956..11546183c70def6c0aa539642fd1c9ada3d46840)) - Addresses review comments from @maflcko and @l0rinc. Thanks!

📝 maflcko opened a pull request: " fuzz: Make process_message(s) more deterministic "
(https://github.com/bitcoin/bitcoin/pull/32822)
`process_message(s)` are the least stable fuzz targets, according to OSS-Fuzz.

Tracking issue: https://github.com/bitcoin/bitcoin/issues/29018.

# Testing

Needs coverage compilation, as explained in `./contrib/devtools/README.md`. And then, using 32 threads:

```
cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../b-c-qa-assets/fuzz_corpora/ process_messages 32
```

Each commit can be reverted to see more non-determinism
...

💬 maflcko commented on pull request "fuzz: Make process_message(s) more deterministic":
(https://github.com/bitcoin/bitcoin/pull/32822#issuecomment-3013497916)
for testing, the single-check doesn't hit here and can be quite expensive, so it could be disabled via:

```diff
diff --git a/contrib/devtools/deterministic-fuzz-coverage/src/main.rs b/contrib/devtools/deterministic-fuzz-coverage/src/main.rs
index 3eeb121db0..f729740a3a 100644
--- a/contrib/devtools/deterministic-fuzz-coverage/src/main.rs
+++ b/contrib/devtools/deterministic-fuzz-coverage/src/main.rs
@@ -219,13 +219,6 @@ The coverage was not deterministic between runs.
if !entry
...

🤔 stickies-v reviewed a pull request: "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError"
(https://github.com/bitcoin/bitcoin/pull/32604#pullrequestreview-2958036547)
This [thread](https://github.com/bitcoin/bitcoin/pull/32604/files#r2164992810) nerdsniped me into reimplementing this PR with a different approach, leaving the scheduled resetting to `CScheduler`. This feels like a much more natural fit to me, and allows a couple of other improvements. See branch [here](https://github.com/stickies-v/bitcoin/commits/2025-06/schedule-ratelimit-reset/), or diff [here](https://github.com/Crypt-iQ/bitcoin/compare/2ac8696b53e455dd27c8341828404a23b5cb68a9..9fb36ba46bcc
...

💬 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_r2172385738)
Sorry for the wait, branch is ready: https://github.com/stickies-v/bitcoin/commits/2025-06/schedule-ratelimit-reset/ . I added the "fixup" comments as commits that should be squashable into the current branch as-is (requiring rebase for the commits above, though). Hopefully the isolation of the changes makes this easier to review and potentially adopt into this PR.

The main change is that I've carved out scheduling from `LogRateLimiter` (leaving that to `CScheduler`), which allows quite a bit
...

🤔 hebasto reviewed a pull request: "doc: add alpine build instructions"
(https://github.com/bitcoin/bitcoin/pull/32559#pullrequestreview-2967102495)
Tested 64986e1a44e108628b245ca977d2b8eb69ed5580 on Alpine Linux v3.22.

💬 hebasto commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2172347865)
FWIW, `linux-headers` is a dependency of `boost-dev` on my system.

💬 hebasto commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2172354607)
not even a nit: `gmake`, which explicitly refers to GNU Make, also works.

💬 hebasto commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2172363200)
However, depends build the `systemtap` package. The subsequent configuration results in:
```sh
# cmake -B build --toolchain depends/x86_64-pc-linux-musl/toolchain.cmake
<snip>
USDT tracing ........................ ON
<snip>
```
And the build succeeds.

cc @0xB10C

💬 hebasto commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2172422391)
1. I think we should consider a note from `apk`:
```
Executing ninja-build-1.12.1-r1.post-install
* this only installs ninja to /usr/lib/ninja-build/bin/ninja
* add that to your path to use it, or invoke it directly.
* for most uses, you want samurai instead:
* $ apk add samurai
* which has a "ninja" executable compatible with ninja.
```
and install `samurai`. Otherwise, Qt still complains:
```
CMake Warning at qtbase/cmake/QtBuildHelpers.cmake:12 (message):
The officially support
...

💬 hebasto commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#issuecomment-3013738127)
Additionally,
```
# ldd ./build-dep/bin/bitcoin-qt
/lib/ld-musl-x86_64.so.1 (0x77d3de93a000)
Error loading shared library libfontconfig.so.1: No such file or directory (needed by ./build-dep/bin/bitcoin-qt)
Error loading shared library libfreetype.so.6: No such file or directory (needed by ./build-dep/bin/bitcoin-qt)
Error loading shared library libxkbcommon.so.0: No such file or directory (needed by ./build-dep/bin/bitcoin-qt)
Error loading shared library libxkbcommon-x11.so.0: No such
...

📝 pablomartin4btc opened a pull request: "test: Fix wait_for_getheaders() call in test_outbound_eviction_blocks_relay_only()"
(https://github.com/bitcoin/bitcoin/pull/32823)
This change avoids relying on `tip_header.hash`, which is `None` when the header is deserialized from hex during `CBlockHeader()` construction.
Instead, `tip_header.rehash()` explicitly computes the hash, making the test behavior more robust.

Using the explicit `rehash()` avoids depending on `wait_for_getheaders()` falling back to any received message, thus making the test more deterministic.

This is a follow-up to #32742.

Also, as noted in a previous review [comment](https://github.co
...