💬 hebasto commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2832107817)
> Using master @ [d73f37d](https://github.com/bitcoin/bitcoin/commit/d73f37dda221835b5109ede1b84db2dc7c4b74a1).
>
> The following seems incorrect, because
> ```
> make -C depends/ NO_QT=1 NO_WALLET=1 NO_ZMQ=1 -j18 CFLAGS="-O3" CXXFLAGS="-O3"
> cmake -B build --toolchain /root/ci_scratch/depends/aarch64-unknown-linux-gnu/toolchain.cmake
> <snip>
> C++ compiler flags .................... -O3 -O2 -g
> Linker flags .......................... -O3 -O2 -g
> ```
> is overriding the user-provided `-O3`.
...

👋 Bue-von-hon's pull request is ready for review: "rpc: Support v3 raw transactions creation"
(https://github.com/bitcoin/bitcoin/pull/31936)

💬 Bue-von-hon commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2832113486)
@luke-jr @instagibbs @maflcko @rkrux @glozow @adamandrews1 @w0xlt @1440000bytes
PTAL 🙏

💬 hebasto commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2832116385)
> Is making `m_nodes_mutex` non-recursive a goal? That sounds nice to me and I'm not opposed, but I'd rather not make changes just to get halfway there.

I'm curious how far we are from a non-recursive `m_nodes_mutex`?

👍 hebasto approved a pull request: "Crash fix, disconnect numBlocksChanged() signal during shutdown"
(https://github.com/bitcoin-core/gui/pull/864#pullrequestreview-2795975241)
ACK 71656bdfaa6bfe08ce9651246a3ef606f923351b, I have reviewed the code and it looks OK.

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