💬 l0rinc commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1815354171)
I remember this being asked before, but I can't find it - yes, `.value()` already validates the optional.

💬 theuni commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2435747523)
Concept ACK

💬 marcofleon commented on pull request "fuzz: speed up addrman":
(https://github.com/bitcoin/bitcoin/pull/30688#issuecomment-2435764406)
@Eunovo I'm not seeing that reduction in coverage when I run the target. What sanitizers are you using? Also, what does the coverage say if you do `-runs=1`? (instead of letting it run for a while)

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1815386949)
I'm working on a test case for master for that case

🤔 pablomartin4btc reviewed a pull request: "build: Rename `PACKAGE_*` variables to `CLIENT_*`"
(https://github.com/bitcoin/bitcoin/pull/31042#pullrequestreview-2393258996)
re-ACK 36a3de0af9a9120aa98a07ca4abbb77ee6e58ef9

Since my previous review: [extended renaming](https://github.com/bitcoin/bitcoin/pull/31042#discussion_r1795498723) to the rest of sources where `PACKAGE_` namespace existed.

💬 achow101 commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#issuecomment-2435847417)
ACK 4feaa28728442b0fd29a677d2b170a05fdf967c0

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2435868627)
Needs rebase

🤔 hebasto reviewed a pull request: "Update libmultiprocess library"
(https://github.com/bitcoin/bitcoin/pull/31105#pullrequestreview-2393300526)
Post-merge ACK 90b405516f7f3be522ced3e0c4d23b3892df0661.

💬 maflcko commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#issuecomment-2435910975)
Only change since previous review (https://github.com/bitcoin/bitcoin/pull/30928#pullrequestreview-2355129479) is dropping one commit.

re-ACK 4cf12216951e91550c81db807fe0ecfafe6834c9 🔡

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTM
...

🚀 achow101 merged a pull request: "wallet: optimize migration process, batch db transactions"
(https://github.com/bitcoin/bitcoin/pull/28574)

💬 laanwj commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2435926822)
> It is not entirely clear whether it should be called after every RegQueryValueExA or if it's fine as-is, after the loop.

FWIW the idea that i got from reading that was that you should close the key when done with the API. Not at any specific point. This will unload the performance collection DLLs. The next time you query the key again they will be loaded again.

i concluded that we do the right thing (never close it) because, we're never done with it, we keep queryiing them time after tim
...

👍 laanwj approved a pull request: "util: Remove RandAddSeedPerfmon"
(https://github.com/bitcoin/bitcoin/pull/31124#pullrequestreview-2393333397)
Code review ACK 9bb92c0e7ffe2426b4afb80ddd4b4c96968316b5

📝 maflcko converted_to_draft a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061)
All translatable format strings are fixed. This change surfaces errors in them at compile-time.

The implementation achieves this by allowing to delay the translation (or `std::string` construction) that previously happened in `Untranslated()` or `_()` by returning a new type from those functions. The new type can be converted to `bilingual_str` where needed.

This can be tested by adding a format string error in an original string literal and observing a new compile-time failure.

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1815481584)
I don't like this suggestion. If you disagree, you can continue discussion in https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1801093580

🚀 achow101 merged a pull request: "refactor: migrate `bool GetCoin` to return `optional<Coin>`"
(https://github.com/bitcoin/bitcoin/pull/30849)

💬 laanwj commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2436025444)
Complete build on RPi5 with NVME hat, 8GB memory:
```
$ cmake -B build -G Ninja --preset=dev-mode -DWITH_CCACHE=OFF --toolchain /data/tmp/bitcoin/depends/aarch64-unknown-linux-gnu/toolchain.cmake
$ time ninja -C build
```

(PR with pch)
real 33m33.235s
user 127m24.712s
sys 4m22.753s

(base commit)
real 37m39.927s
user 144m17.888s
sys 3m26.133s

About 10% faster (in real time), not as impressive as some results on faster machines, but still nice to have.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-2436046062)
Finished benching on a HDD until 860k on Intel(R) Core(TM) i7-7700 CPU @ 3.60GHz, CPU 8:

Summary
'COMMIT=f278ca4ec3f0a90c285e640f1a270869ca594d20 ./build/src/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=860000 -dbcache=10000 -printtoconsole=0' ran
1.02 times faster than 'COMMIT=e9e23b59f8eedb8dfae75aa660328299fba92b50 ./build/src/bitcoind -datadir=/mnt/my_storage/BitcoinData -stopatheight=860000 -dbcache=10000 -printtoconsole=
0'

f278ca4ec3 coins: allow emplacing no
...

👍 maflcko approved a pull request: "Introduce `g_fuzzing` global for fuzzing checks"
(https://github.com/bitcoin/bitcoin/pull/31093#pullrequestreview-2393422314)
lgtm

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#issuecomment-2436080216)
> So likely on HDD we shouldn't use so many threads, apparently it slows down IBD.

I'm not sure we can conclude that from your benchmark. It used a very high dbcache setting, which makes the effect of this change less important. It also is syncing from untrusted network peers, so there is some variance which could also account for the 2% difference.

🤔 nymius reviewed a pull request: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532#pullrequestreview-2393479212)
nit: I have read the test and I don't have any code related comment yet, but I have noticed that some of the commits are structured with some not written conventions and beside the actual comments about commit message quality in the `CONTRIBUTIONS.md` file, there are some other conventions used by other contributors too, in a similar fashion to [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/) with (`wallet:`, `ci:`, `rpc:`). In that spirit, wouldn't be a good idea to, in ca
...