💬 fanquake commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2435706570)
Rebased, and dropped the `[WIP]` from the CI commit.

💬 theStack commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1815309496)
nit, if you retouch:
```suggestion
auto coin = (!mempool || !mempool->isSpent(vOutPoint)) ? view.GetCoin(vOutPoint) : std::nullopt;
```
(slightly easier to read imho; it's no big deal though here, maybe i'm just post-traumatized by the [serialized hash calculation bug](https://github.com/bitcoin/bitcoin/pull/28685#pullrequestreview-1692828827) where missing parantheses in a ternary expression were an issue :P)

💬 theStack commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1815321190)
nit: the `assert` is lost here, but since `std::optional`'s `value` method throws an exception if no value is available, it seems to be fine to not have it (I assume that was intentional for that reason)

👍 theStack approved a pull request: "refactor: migrate `bool GetCoin` to return `optional<Coin>`"
(https://github.com/bitcoin/bitcoin/pull/30849#pullrequestreview-2393139880)
Code-review ACK 4feaa28728442b0fd29a677d2b170a05fdf967c0

👍 hebasto approved a pull request: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881#pullrequestreview-2393167239)
re-ACK a4e17239ca54ccaa237fa6c824b968ca067228e6, verified with:
```
git range-diff master f9e0010de394ad6ae5eb6a22ead7ebc116a16fd4 a4e17239ca54ccaa237fa6c824b968ca067228e6
```

Only rebased and amended the commit message since my recent [review](https://github.com/bitcoin/bitcoin/pull/29881#pullrequestreview-2363408909).

👍 hebasto approved a pull request: "util: Remove RandAddSeedPerfmon"
(https://github.com/bitcoin/bitcoin/pull/31124#pullrequestreview-2393187592)
ACK 9bb92c0e7ffe2426b4afb80ddd4b4c96968316b5, I have reviewed the code and it looks OK.

💬 darosior commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2435736744)
Rebased after #31148 merge.

👍 pablomartin4btc approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2393191955)
re-ACK 5757fdf0dc74ec9d6bbefba937d6e23b09652605

A small improvement since my last review, adding (also) benchmark name/ test name to the test common dir when `-testdatadir` is not specified.

💬 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