👍 pablomartin4btc approved a pull request: "cmake: Exclude generated sources from translation"
(https://github.com/bitcoin/bitcoin/pull/31899#pullrequestreview-2627202385)
tACK ff4ddd3d2e3b08156fd0a0aeb954fde0a5f4cb03

Tested in both Ubuntu and macOS.

💬 mzumsande commented on issue "wallet: wrong balance and crash after reorg and unclean shutdown":
(https://github.com/bitcoin/bitcoin/issues/31824#issuecomment-2669011985)
> Is this not where the chainstate related changes are flushed to disk?
> https://github.com/bitcoin/bitcoin/blob/28.x/src/validation.cpp#L3069

The call uses `FlushStateMode::IF_NEEDED`, which means it usually won't do anything - it will only flush if it's necessary because the cache has grown to a critical size.

💬 darosior commented on pull request "random: move VerifyRNDRRS above InitHardwareRand":
(https://github.com/bitcoin/bitcoin/pull/31902#issuecomment-2669021440)
I think for now it's safer to just revert the original PR https://github.com/bitcoin/bitcoin/pull/31908. The changes can be re-considered and re-reviewed after the 29.0 branch-off.

✅ tnull closed a pull request: "Fix field name styling in `submitpackage` RPC results"
(https://github.com/bitcoin/bitcoin/pull/31900)

💬 tnull commented on pull request "Fix field name styling in `submitpackage` RPC results":
(https://github.com/bitcoin/bitcoin/pull/31900#issuecomment-2669032754)
> Ok... even though it's marked as experimental, I think it's too late to break `submitpackage` purely for style reasons. Fwiw I did think this was worthwhile before (and I pinged @tnull after seeing [comment](https://github.com/spesmilo/electrum-protocol/issues/4#issuecomment-2665116596)), but I'm leaning towards nack now. My apologies!

Alright, no worries! Going ahead and closing this.

💬 glozow commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2669053075)
Up to reviewers to decide whether this or #31902 is more appropriate, but added to milestone so we do this before branch off.

💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2669069081)
So the issue is internal to the functional tests themselves. See https://cirrus-ci.com/task/5346278168592384?logs=ci#L4358 (commit https://github.com/bitcoin/bitcoin/commit/fa780b31ab54febd7e56a0fc165683e056fc2500) which prints:

```
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
python3 50589 root 11u IPv4 179599797 0t0 TCP localhost:60000->localhost:19216 (ESTABLISHED)
bitcoind 50594 root 21u IPv4 179704903 0t0 TCP localhost:19216->localhost:60000 (ESTABLISHED)
...

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1961955650)
`clang-format` doesn't seem to touch this. However once I added some line breaks myself, it suddenly had an opinion. So I'll commit that.

💬 0xB10C commented on pull request "ci: Switch to gcr.io mirror to avoid rate limits":
(https://github.com/bitcoin/bitcoin/pull/31906#issuecomment-2669078279)
An alternative would probably be to configure the docker deamons of the self-hosted runners to use the mirror?

in e.g. `/etc/docker/daemon.json`

```
{
"registry-mirrors": ["https://mirror.gcr.io"]
}
```

or in `/etc/containers/registries.conf.d/gcr.conf` for podman

```
[[registry]]
location = "https://mirror.gcr.io"
prefix = "docker.io"
```

💬 sipa commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2669082332)
ACK 3e9b12b3e0f039a8760410afed74c7e4d15afbe6

We shouldn't have merged #31826 if it didn't compile, and apparently nobody actually tested it. It's one thing to make changes for an unsupported platform that don't hurt others, but clearly the bar for testing was too low if it actually doesn't compile at all.

💬 0xB10C commented on pull request "ci: Switch to gcr.io mirror to avoid rate limits":
(https://github.com/bitcoin/bitcoin/pull/31906#discussion_r1961960848)
the ASAN job (and others) currently runs on a GitHub hosted runner and these are exempt from the rate-limiting: https://github.com/actions/runner-images/issues/1445#issuecomment-714832439

💬 dergoegge commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2669090900)
> neither apparently reviewers do (nor does the author?)

It was actually tested on the merged PR: see https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2652823871 and https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2653350018 but a later refactoring suggestion (https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1955192539) introduced the compilation issue and no re-testing was done before merge.

I think if @laanwj and @giahuy98 are willing to re-test on #31902 we
...

👍 TheCharlatan approved a pull request: "Revert merge of PR #31826"
(https://github.com/bitcoin/bitcoin/pull/31908#pullrequestreview-2627300756)
ACK 3e9b12b3e0f039a8760410afed74c7e4d15afbe6

🤔 eval-exec reviewed a pull request: "Revert merge of PR #31826"
(https://github.com/bitcoin/bitcoin/pull/31908#pullrequestreview-2627309977)
ACK

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1961976727)
Done both.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1961979095)
I'll make it "during this iteration".

💬 maflcko commented on pull request "test, refactor: Add TestNode.binaries to hold binary paths":
(https://github.com/bitcoin/bitcoin/pull/31866#discussion_r1961985647)
Just a nit, anything is fine here. Further clean-ups can trivially done as a follow-up, if there is need to.

💬 maflcko commented on pull request "test, refactor: Add TestNode.binaries to hold binary paths":
(https://github.com/bitcoin/bitcoin/pull/31866#issuecomment-2669124512)
review ACK 8f5228d9239464ece06f3a56372aeec29685801c 😸

<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}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 8f5228d92394
...

👍 TheCharlatan approved a pull request: "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally"
(https://github.com/bitcoin/bitcoin/pull/31662#pullrequestreview-2627267258)
ACK 9e7eb06a24b670e8e72e0233a63bbc67733d4adb

Can you add the note about `FUZZ_LIBS` usage to the PR description?

💬 TheCharlatan commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1961983762)
In commit 9e7eb06a24b670e8e72e0233a63bbc67733d4adb
Nit: Is this change related to the actual change introducing `FUZZ_LIBS` in this commit?