💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2054477414)
Fixed

👍 hebasto approved a pull request: "bench: close wallets after migration"
(https://github.com/bitcoin/bitcoin/pull/32309#pullrequestreview-2784709452)
ACK cad39f86fb5a81f0e3b5116e8e989bab8af89718, tested on Ubuntu 25.04.

🚀 hebasto merged a pull request: "bench: close wallets after migration"
(https://github.com/bitcoin/bitcoin/pull/32309)

💬 hebasto commented on pull request "test: Run all benchmarks in the sanity check":
(https://github.com/bitcoin/bitcoin/pull/32310#issuecomment-2821907878)
> > Why is the CI failing?
>
> Thx, excluded msvc for now, with the same approach as the temporary win-cross exclusion.

This can now be reverted after rebase.

🤔 jonatack reviewed a pull request: "[IBD] flush UTXO set in batches proportional to `dbcache` size"
(https://github.com/bitcoin/bitcoin/pull/31645#pullrequestreview-2784731899)
ACK 8fd522b223fc1405e70ca93c7e2d5a39f3f826fb

A few nits, feel free to pick/choose/ignore.

💬 jonatack commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2054494446)
This seems to be the most frequently seen order:

```diff
#include <coins.h>
+#include <node/coins_view_args.h>
#include <streams.h>
@@ -20,7 +21,6 @@
#include <vector>

#include <boost/test/unit_test.hpp>
-#include <node/coins_view_args.h>

using namespace util::hex_literals;
```

💬 jonatack commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2054492337)
Might be good to add a comment for this magic value, i.e. (from the PR description):

`Capped at 256 MiB, as gains are barely measurable for bigger batches (see PR 31645)`

also, clang-format

```diff
- /*hi=*/256_MiB
- );
+ /*hi=*/256_MiB);
```

and unneeded braces line 18

```diff
- (dbcache_bytes / DEFAULT_KERNEL_CACHE) * DEFAULT_DB_CACHE_BATCH,
+ dbcache_bytes / DEFAULT_KERNEL_CACHE * DEFAULT_DB_CACHE_BATCH,
```

💬 jonatack commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2054497017)
Feel free to ignore, but the following would be more readable and follow the most frequent convention in this codebase:

```diff
- if (const auto value = args.GetIntArg("-dbbatchsize")) options.batch_write_bytes = *value;
- else options.batch_write_bytes = GetDbBatchSize(args.GetIntArg("-dbcache", DEFAULT_KERNEL_CACHE));
+ if (const auto value = args.GetIntArg("-dbbatchsize")) {
+ options.batch_write_bytes = *value;
+ } else {
+ options.batch_write_bytes = GetDb
...

💬 jonatack commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2054511132)
lgtm (thanks for updating)

```
$ ./build/bin/bitcoind -help-debug | grep -A2 dbbatchsize
-dbbatchsize
Maximum database write batch size in bytes (default: calculated from the
`-dbcache` value or 16777216 if not set)
```

💬 achow101 commented on pull request "test: Test that migration automatically repairs corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r2054516517)
Done

💬 achow101 commented on pull request "test: Test that migration automatically repairs corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r2054516784)
Oops yes, fixed.

💬 achow101 commented on pull request "test: Test that migration automatically repairs corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r2054517349)
Done

💬 maflcko commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2054552472)
std::in → stdin

💬 jonatack commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2822019017)
ACK cc8f239663a0874c307efa30815b40d9ef40badb

👋 maflcko's pull request is ready for review: "refactor: Avoid copies by using const references or by move-construction"
(https://github.com/bitcoin/bitcoin/pull/31650)

💬 maflcko commented on pull request "refactor: Avoid copies by using const references or by move-construction":
(https://github.com/bitcoin/bitcoin/pull/31650#discussion_r2054580426)
Ok, restored initial version of this pull request. Should be easy to re-review.

💬 achow101 commented on pull request "docs: remove passing CI section in guidelines for PR merging as it's common sense":
(https://github.com/bitcoin/bitcoin/pull/32006#issuecomment-2822066744)
NACK, agree with @pinheadmz

👍 ryanofsky approved a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2784838614)
Code review ACK ece168ecef6e57e3a994ec5f7bd341f9c7525721. Changes since last review were squashing and rebasing, doing a better job of passing seed, timeouts, and tmpdirs to child processes. Previously dropped workaround for None errno is also back, this time with link to upstream python bug.

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2054592532)
re: https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2053031058

I think you could drop all unnecessary encoding and decoding by just dropping `encoding="utf-8"` parameter to `subprocess.run` and printing the bytes directly with something like `sys.stderr.buffer.write(b"<CHILD OUTPUT BEGIN>\n%s\n<CHILD OUTPUT END>" % e.output)`

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-2822100971)
Rebased e219f15976f50e0a703bc696b6445feb9950e65d -> eec7c49cf777cdcc53aeb20e31fbd5881c2b008b ([chainman_flush_destructor_4](https://github.com/TheCharlatan/bitcoin/tree/chainman_flush_destructor_4) -> [chainman_flush_destructor_5](https://github.com/TheCharlatan/bitcoin/tree/chainman_flush_destructor_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainman_flush_destructor_4..chainman_flush_destructor_5))

* Rebased to get a CI fix for windows cross