✅ fanquake closed an issue: "guix: Windows build is non-deterministic across build architectures"
(https://github.com/bitcoin/bitcoin/issues/32923)

🚀 fanquake merged a pull request: "Resolve guix non-determinism with emplace_back instead of push_back"
(https://github.com/bitcoin/bitcoin/pull/32930)

💬 fanquake commented on pull request "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`":
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3061514214)
This is going to need backporting, to fix the 29.x branch.

💬 Sjors commented on pull request "Enable `-Werror=dev` in CI & Guix":
(https://github.com/bitcoin/bitcoin/pull/32937#issuecomment-3061521322)
ACK 35e0331493e97f8c9a79e40449fe2787dc798f3c

Test by deliberately triggering one with `message(AUTHOR_WARNING`. By default for `cmake -B build` it's just a warning, but with `--preset dev-mode` it's an error.

Guix build on x86_64:

```
c7870f6c402ac407d0de4edc21d42f5040d19374deaba50b496e4f99a421c8e8 guix-build-35e0331493e9/output/aarch64-linux-gnu/SHA256SUMS.part
d64532e1bd83568f2386a6b67c49e0843f08c0d486828b1cd0ce955a179b07bb guix-build-35e0331493e9/output/aarch64-linux-gnu/bitcoin-
...

💬 Sjors commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2200231328)
Although I agree that's a better name, renaming would introduce (even) more churn.

👍 dergoegge approved a pull request: "fuzz: Add missing calls to `SetMockTime` for determinism"
(https://github.com/bitcoin/bitcoin/pull/32927#pullrequestreview-3009530556)
utACK fa8862723c14cdd471bbffc7a4897ece2e6d8a7f

👍 dergoegge approved a pull request: "fuzz: Make process_message(s) more deterministic"
(https://github.com/bitcoin/bitcoin/pull/32822#pullrequestreview-3009550765)
utACK fa501ea5ed874ffcba2b606806460e61a1204bd2

As a follow up (if someone is interested), we could add the block hashes for the blocks with mature coinbases (as well as the mature coinbase prevouts) to the [dictionary](https://github.com/bitcoin-core/qa-assets/blob/main/fuzz_dicts/net_processing.dict) in qa-assets.

💬 Sjors commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2200273610)
Done

💬 Sjors commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2200273799)
Every call to `FillPSBT(` sets `bip32derivs` explicitly so this change shouldn't matter.

Except in `src/wallet/feebumper.cpp` where I originally dropped `/*bip32derivs=*/true`. I brought that back for clarity.

💬 Sjors commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#issuecomment-3061603729)
Rebased (just in case after ##32930) and addressed comments, mainly:
- using `SIGHASH_ALL` instead of `1`
- making sure `bip32derivs` is explicitly set everywhere (added `SignTransaction` in `fee_bumper.cpp`) because the default changed

🤔 rkrux reviewed a pull request: "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively"
(https://github.com/bitcoin/bitcoin/pull/28333#pullrequestreview-3009605243)
I seem to be getting onboard with this idea as I can notice readability improvements on a cursory glance. Do you anticipate any performance improvements as well?

The last sentence related to legacy SPKMs of the last paragraph in the PR description can be removed now.

Will do full review soon.

👍 dergoegge approved a pull request: "refactor: Convert GenTxid to `std::variant`"
(https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-3009607187)
Code review ACK a60f863d3e276534444571282f432b913d3967db

💬 dergoegge commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2200291621)
nit: Imo, function parameters should reflect what a function needs to do its job, as opposed to passing it what you have available at the call sites.

`CInv` can be more than a tx announcement, and it's only used here to be passed to `ToGenTxid` (which crashes if its not a tx inv). Distinguishing tx from other announcements is not the job of `FindTxForGetData`. It only needs a wtxid/txid to do its job and should take a `GenTxid` (or be templated).

🚀 fanquake merged a pull request: "fuzz: Add missing calls to `SetMockTime` for determinism"
(https://github.com/bitcoin/bitcoin/pull/32927)

💬 hebasto commented on pull request "cmake: Use newer signature of `qt6_add_lrelease` when available":
(https://github.com/bitcoin/bitcoin/pull/32940#issuecomment-3061672234)
My Guix build:
```
aarch64
9a9cd30b66d0c150c6e843a37b89cdbae284e0e7c2829d7d6c64076453d4be1f guix-build-94931656b52f/output/aarch64-linux-gnu/SHA256SUMS.part
e7525f1c1cf720f53a292b404ccd63ca792631c14587a1a34978b33d35c4fece guix-build-94931656b52f/output/aarch64-linux-gnu/bitcoin-94931656b52f-aarch64-linux-gnu-debug.tar.gz
231c4e16929291d5d6edf1d3bf0c0ba2f9fda5a11586941a92f256f6bd1067c7 guix-build-94931656b52f/output/aarch64-linux-gnu/bitcoin-94931656b52f-aarch64-linux-gnu.tar.gz
f965bfc1
...

👍 dergoegge approved a pull request: "Fuzz: extend CConnman tests"
(https://github.com/bitcoin/bitcoin/pull/28584#pullrequestreview-3009670249)
Code review ACK 0802398e749c5e16fa7085cd87c91a31bbe043bd

💬 dergoegge commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r2200334391)
nit: it might make sense to name this differently, just to avoid someone confusing this function with a smart ptr's `reset()`.

💬 fanquake commented on pull request "depends: fix libevent `_WIN32_WINNT` usage":
(https://github.com/bitcoin/bitcoin/pull/32837#issuecomment-3061722462)
Guix Build (aarch64):
```bash
54f1ec86a1e595a5529f115a333c5f6cf575ff35f0b87ed501f36f83e28d9063 guix-build-f5647c6c5ae8/output/aarch64-linux-gnu/SHA256SUMS.part
e9db7e23661bac03b87e38dfc974d27b72ce8d684d9cff7e7105c735b8bab7af guix-build-f5647c6c5ae8/output/aarch64-linux-gnu/bitcoin-f5647c6c5ae8-aarch64-linux-gnu-debug.tar.gz
55c5b8685af798bbe36db8d1bc6a4fa90a10f83c68c8793d08512af4d7a9e326 guix-build-f5647c6c5ae8/output/aarch64-linux-gnu/bitcoin-f5647c6c5ae8-aarch64-linux-gnu.tar.gz
c67cf3
...

💬 hebasto commented on pull request "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`":
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3061725053)
> Could we also set `CMAKE_FIND_USE_PACKAGE_REGISTRY` to false in the toolchain file as a belt-and-suspenders in case some future package manually opts-in using `CMAKE_EXPORT_PACKAGE_REGISTRY` ?

I don't think it prevents issues like https://github.com/bitcoin/bitcoin/issues/32938, which occur when the toolchain file is not used.

💬 hebasto commented on pull request "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`":
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3061753523)
> This is going to need backporting, to fix the 29.x branch.

I think it should be also backported to the 28.x branch, considering https://github.com/bitcoin/bitcoin/commit/f59e9057e2aa596b54cf9e85bab35c3ead137547.

Additionally, we might want to advise users to delete the `~/.cmake/packages/libevent` directory on their machines.