👍 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.

💬 hebasto commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3061802573)
> > Why?
>
> Padding (not margin) has been removed because it is unnecessary and visually disruptive. Padding serves no purpose and only worsens the layout.

Not being a designer, these claims seem like a matter of taste to me. It would be helpful to hear other designers' opinions.

> > Bitcoin Core has its own logo for years. I don't see your reference as a justification for this change.
>
> I discussed this PR with @jonasschnelli on IRC a few days ago. As the creator of the original
...

💬 hebasto commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2200427146)
Since https://codeberg.org/guix/guix/pulls/353 uses the newer `pyproject-build-system` and `python-scikit-build-core`, it no longer requires an extra build step to set the `PYTHONPATH` environment variable.

⚠️ cyjseagull opened an issue: "Why bitcoin use so many RecursiveMutex"
(https://github.com/bitcoin/bitcoin/issues/32946)
I have noticed that in the Bitcoin source code, almost all thread-safe sections use `RecursiveGuard`.
As we all know, `RecursiveGuard` is **exclusive**, which can **impact system performance and concurrency**.

At the same time, I have also noticed [another issue](https://github.com/bitcoin/bitcoin/issues/19303) specifically tracking the replacement of `RecursiveMutex` with `Mutex`, which would help reduce some of the locking overhead.

However, I think `Mutex` is still **exclusive**. For modul
...

✅ cyjseagull closed an issue: "Why bitcoin use so many RecursiveMutex"
(https://github.com/bitcoin/bitcoin/issues/32934)

🤔 marcofleon reviewed a pull request: "fuzz: Add missing calls to `SetMockTime` for determinism"
(https://github.com/bitcoin/bitcoin/pull/32927#pullrequestreview-3009814486)
post merge ACK fa8862723c14cdd471bbffc7a4897ece2e6d8a7f