💬 l0rinc commented on pull request "cmake: Do not override flags set by the user when replacing them":
(https://github.com/bitcoin/bitcoin/pull/32356#issuecomment-2832127762)
Concept ACK

Nit: the title is a bit ambiguous, not sure what "replacing them" mean in this context

✅ hebasto closed an issue: "Segfault during shutdown in SendCoinsDialog::updateCoinControlState"
(https://github.com/bitcoin-core/gui/issues/862)

🚀 hebasto merged a pull request: "Crash fix, disconnect numBlocksChanged() signal during shutdown"
(https://github.com/bitcoin-core/gui/pull/864)

💬 hebasto commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/32292#issuecomment-2832134070)
> Could also include [bitcoin-core/gui#864](https://github.com/bitcoin-core/gui/pull/864), possibly?

+1

💬 hebasto commented on pull request "cmake: Do not override flags set by the user when replacing them":
(https://github.com/bitcoin/bitcoin/pull/32356#issuecomment-2832136978)
> Concept ACK
>
> Nit: the title is a bit ambiguous, not sure what "replacing them" mean in this context

I meant "we try to replace `-O3` with `-O2` but only when `CMAKE_CXX_FLAGS_RELEASE` is not specified by the user explicitly". I'll be happy to adjust wording for a better suggestion.

💬 Eunovo commented on pull request "Wallet: Fix Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor":
(https://github.com/bitcoin/bitcoin/pull/32344#discussion_r2061300483)
Calling AddWalletDescriptor with a descriptor that's already in wallet, calls UpdateWalletDescriptor which calls CanUpdateWalletDescriptor. This is what happens on the second call to AddWalletDescriptor.

💬 l0rinc commented on pull request "cmake: Do not override flags set by the user when replacing them":
(https://github.com/bitcoin/bitcoin/pull/32356#issuecomment-2832203043)
How about:
```
cmake: Respect user-provided compiler optimization level
```

💬 jamesob commented on pull request "BIP-348 (OP_CHECKSIGFROMSTACK) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/32247#issuecomment-2832242691)
Thanks @instagibbs; I've compressed your test changes into a single commit and added that here.

🤔 theStack reviewed a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-2796051128)
Concept ACK

Seems like the commit message for 2e6dcdbc8055660a2e20ba81b62b7d26ae0ccb05 ("Add MuSig2 Keyagg Cache class and functions") is out-of-sync, as there is no such class added and also the mentioned `MuSig2KeyAggCacheImpl` doesn't exist.

For the newly introduced parameters of the `Const` and `Split` functions (commits fe02d7cb237c00de6abe1776b3101342ffddf757 and 4a1eeee27a64c4be7293740f8fff839879b88d86), it would be nice to have unit test coverage, but this can be also done in a fol
...

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2061332697)
nit: could use `LookupHelper` here (for slightly better readability imho)
```suggestion
std::vector<CPubKey> participant_pubkeys;
LookupHelper(aggregate_pubkeys, pubkey, participant_pubkeys);
return participant_pubkeys;
```

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2061338625)
nit: should also include ` <optional>`, since it's used for the return types below
```suggestion
#include <optional>
#include <vector>
```

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2061339360)
nit: could turn this into a one-liner by using `std::any_of`

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2061339636)
nit: could turn this into a one-liner by using `std::all_of`

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2832298968)
Restored the deserialization validation in `Obfuscation::Unserialize`, we can't assert, but we can safely throw an `std::logic_error`, since during mempool fuzzing https://github.com/bitcoin/bitcoin/blob/master/src/node/mempool_persist.cpp#L141 catches and ignored these errors safely (managed to reproduce it on Linux, not sure why it's not reproducible on Mac).
I've also split out all renamed to a single commit before any other refactor or optimization to simplify the higher-risk changes.
PR i
...

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2832401037)
ACK 0302eefd9a826e1d8e81b3684ff8ebc6de9d4217

Latest push fixed dead code concerns, uses counts instead of find for non-regex cases consistently and uses less encoding rountrips.

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2061444289)
Thanks for fixing this!

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2061411267)
What's the exact purpose of this, why would someone set `--tmr=x` or `--tmdi=y`?
And `if not tmpdir_arg` is basically the same as the previous loop which inserts to existing location, it just appends and uses a different root folder, right? We could use those as defaults I think, and slice it in, something like:

```suggestion
i, path = next(((i, m[1]) for i, arg in enumerate(parent_args) if (m := re.match(r'--tmpdir=(.+)', arg))),
(len(parent_args), self.opti
...

💬 l0rinc commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2061435588)
I haven't fully given up on this :D. It's not very important, just had to find a solution, feel free to ignore.
Since empty strings are also falsy, the 🦭 operator can come to the rescue again.
```suggestion
assert not (msg := '\n'.join(errors)), f"Child test didn't contain (only) expected errors:\n{msg}\n<CHILD OUTPUT BEGIN>\n{output}\n<CHILD OUTPUT END>\n"
```

👍 darosior approved a pull request: "Bugfix: Miner: Don't reuse block_reserved_weight for "block is full enough to give up" weight delta"
(https://github.com/bitcoin/bitcoin/pull/32355#pullrequestreview-2796264751)
utACK 524f981bb87319fdd6ff2ab4a932c4b4e31a7398

Makes sense and code looks good to me.

📝 hebasto opened a pull request: "depends: Fix cross-compiling `qt` package from macOS to Windows"
(https://github.com/bitcoin/bitcoin/pull/32357)
Native packages cannot be used during cross-compiling. However, Qt still unconditionally tries to find them, which causes issues in some cases, such as when [cross-compiling from macOS to Windows](https://github.com/bitcoin/bitcoin/issues/32346).

This PR explicitly disables this unnecessary Qt behaviour.

Fixes https://github.com/bitcoin/bitcoin/issues/32346.

Here is a full workflow on my macOS Sequoia 15.4.1 (Intel):
```
% brew install make cmake ninja mingw-w64 nsis
% gmake -C depen
...