🤔 rkrux reviewed a pull request: "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC"
(https://github.com/bitcoin/bitcoin/pull/32593#pullrequestreview-2892695590)
ACK 6135e05

```
git range-diff 11d0513...6135e05
```

The PR is a refactor that improves code readability and robustness upto some extent.

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2935594462)
> Are your e2e tests running against the latest version of this branch?

The container is rebuilt every time a PR is merged, and the musig2 test never failed over the last 6+ months since they were added.

I rebuilt the container today and confirmed it works on the tagged commit for 2.4.0 release ([this](https://github.com/LedgerHQ/app-bitcoin-new/actions/runs/15418931868/job/43388642661) is the CI run).

> But when I pass a PSBT without the Bitcoin Core public nonce, it goes ahead and doe
...

💬 rkrux commented on pull request "rpc: Use type-safe HelpException":
(https://github.com/bitcoin/bitcoin/pull/32660#discussion_r2124061046)
TIL, ty!

💬 instagibbs commented on pull request "p2p: Add mutation check inside compact block's FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#discussion_r2124068316)
done

💬 instagibbs commented on pull request "p2p: Add mutation check inside compact block's FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#discussion_r2124083652)
Actually, this strategy is unneeded. Both merkle checks can fail due to collisions, and either may need full blocks fetched if they're from the first peer. Pushed a change that is simpler and cleaned up READ_STATUS_CHECKBLOCK_FAILED which is no longer necessary.

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2935730736)
> If only some nonces are present, then the device assumes Round 1 already happened, but it will fail to run Round 2.

Ok, that explains issue (2).

But not why it failed with (1), because the policy variant with `older` fallback triggers the error without a public nonce from Bitcoin Core for the key path.

> The workflow I'm expecting is that the coordinator would give the same PSBT in parallel to all the signers, which execute round 1, then all nonces are added to the PSBT, and eah signe
...

💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2124158984)
@stickies-v, my mistake, I quickly wanted to make sure this doesn't affect my IBD benchmarks, didn't have time to review it in detail yet

💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2124163227)
Thanks for clarifying

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2124177620)
> Same here?

Please see https://github.com/include-what-you-use/include-what-you-use/issues/1764.

💬 theStack commented on pull request "[Draft/POC] Add secp256k1-based HPKE (Hybrid Public Key Encryption) For Payjoin v2":
(https://github.com/bitcoin/bitcoin/pull/32617#issuecomment-2935923079)
Fore more context to reviewers, could add a link to [BIP 77](https://github.com/bitcoin/bips/blob/master/bip-0077.md) in the PR description, particularly to the cryptography part: https://github.com/bitcoin/bips/blob/master/bip-0077.md#secp256k1-hybrid-public-key-encryption

> Full HPKE Modes: All four HPKE modes are supported – Base, PSK, Auth, and AuthPSK.

Do we need all of the four modes for Payjoin v2 support? Didn't look in-depth yet, but at least BIP77 contains only the two modes "Bas
...

💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2935928425)
> But not why it failed with (1), because the policy variant with `older` fallback triggers the error without a public nonce from Bitcoin Core for the key path.

Please feel free to [open an issue](https://github.com/LedgerHQ/app-bitcoin-new/issues) and I'll investigate.

> > The workflow I'm expecting is that the coordinator would give the same PSBT in parallel to all the signers, which execute round 1, then all nonces are added to the PSBT, and eah signer is invoked again and produces the
...

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2936000601)
> > But not why it failed with (1), because the policy variant with `older` fallback triggers the error without a public nonce from Bitcoin Core for the key path.
>
> Please feel free to [open an issue](https://github.com/LedgerHQ/app-bitcoin-new/issues) and I'll investigate.

I'll test it once more and will open an issue if needed.

> Verifying if 'my nonce is present' is a substantial amount of work, while checking if 'some nonces' are present is trivial by iterating once through the PS
...

👍 l0rinc approved a pull request: "clang-tidy: Apply modernize-deprecated-headers"
(https://github.com/bitcoin/bitcoin/pull/32673#pullrequestreview-2892956742)
ACK facb152697b8d7b75a9e6108f8896f774b06b35f

I like that it's more consistent now, thanks for fixing it.
Since a few header names have changed now, the only suggestion I have is to keep the simplest ordering that would minimize merge and review conflicts in the future, e.g. a new commit on top with alphabetical ordering (or removal) where we also usually separate internal and external dependencies:

<details>
<summary>Suggested sorting</summary>

```
diff --git a/src/bech32.h b/src/bec
...

💬 l0rinc commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124179503)
should we re-sort these after the change?

💬 l0rinc commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124186698)
this should likely go the the bottom include stack now, right?

💬 l0rinc commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124193602)
looks to me like we might as well remove this

💬 l0rinc commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124231739)
Q: is there any reason the header include guard is after the includes here?

💬 Muniru0 commented on issue "doc: references to unshipped documentation":
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2936112247)
@fanquake why not just updating the the docs to point to the online versions. e.g:

**Replace:**
`see doc/external-signer.md`

With:
`see https://github.com/bitcoin/bitcoin/blob/master/doc/cjdns.md`

This make's sure that:
1. **Accessibility**: Users can easily access the documentation regardless of how they obtained the binaries.

2. **Up-to-Date Information**: Linking to the online repository ensures that users are directed to the most current version of the documentation.

3. **Reduced Confu
...

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2124341111)
I've tested the pre-CMake master branch @ 80f00cafdeef0600fa1a5e906686720786c2336c, and this line wasn't necessary there.

💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2124354681)
@achow101 @maflcko do you or do you know of any use of using the `get_previous_releases.py` script to build from source? Am I being a bit eager in dropping this feature?