💬 HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3138628456)
> > Hi furszy, I thought the test I put already demonstrate the issue, though it is not proper to be included to the codebase.
>
> I agree that a test that copies / amends the actual production code is not great for inclusion, but why not include the test [furszy@f2b8a06](https://github.com/furszy/bitcoin-core/commit/f2b8a06060e2a313f35101454c7669d46e0edc38) from above that doesn't have this problem?

I see, sorry for somehow I missed that part. I thought the test furszy mentioned is my tes
...

💬 nervana21 commented on pull request "qa: test that we do not disconnect a peer for submitting an invalid compact block":
(https://github.com/bitcoin/bitcoin/pull/33083#discussion_r2241408531)
Small nit

```suggestion
# The failure above was cached. Submitting the compact block again will return a cached
```

✅ rkrux closed a pull request: "Musig2 fields followups"
(https://github.com/bitcoin/bitcoin/pull/32412)

💬 rkrux commented on pull request "Musig2 fields followups":
(https://github.com/bitcoin/bitcoin/pull/32412#issuecomment-3138649267)
Closing this one now.

💬 HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3138661911)
> @HowHsu, are you going to include the test or a patch to reproduce the issue? I’m happy to ACK it either way.

Hi @furszy , sorry for the delay, I've picked your commit of the test, which I missed earlier.

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2244450698)
Ah yes, in that case:

```
@param[out] has_hardened whether hardened derivation was found
```

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2244453674)
The version in https://github.com/bitcoin/bitcoin/pull/29675/commits/24bcdd27fd6c32646eb87eb8bea6f539bbc54e42 is fine.

💬 maflcko commented on pull request "refactor: remove unused `ser_writedata16be` and `ser_readdata16be`":
(https://github.com/bitcoin/bitcoin/pull/33093#issuecomment-3138711573)
lgtm ACK 0431a690c3a498a1e728c9df34a132ac16177a04

💬 luke-jr commented on pull request "doc: move `cmake -B build -LH` up in Unix build docs":
(https://github.com/bitcoin/bitcoin/pull/33088#issuecomment-3138752685)
(might make sense to mention `ccmake` too?)

💬 instagibbs commented on pull request "qa: test that we do not disconnect a peer for submitting an invalid compact block":
(https://github.com/bitcoin/bitcoin/pull/33083#issuecomment-3138794818)
crACK c1574381168573c561ebddf1945d2debefb340f7

💬 instagibbs commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#issuecomment-3138821243)
ACK ea17a9423fb431a86d36927b02d3624f654fd867

I think the doc update gives a decent explanation how this can be used. Code cleanup is nice.

💬 instagibbs commented on issue "doc: Mempool Policy documentation Outdated since TRUC":
(https://github.com/bitcoin/bitcoin/issues/32067#issuecomment-3138855557)
You are correct that this no longer holds with TRUC. The caveat can be added.

💬 fanquake commented on pull request "[28.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33076#issuecomment-3138907944)
Guix Build (aarch64):
```bash
9ef8edcd797ec28b927c50bcab2bb1ab31f5990ff08671081e6a62391c22c089 guix-build-5492e1be3b40/output/aarch64-linux-gnu/SHA256SUMS.part
d269e0b8610943a4eb061088c139f9840ebbdeb3a1884a2d1f2d13967b01ddf6 guix-build-5492e1be3b40/output/aarch64-linux-gnu/bitcoin-5492e1be3b40-aarch64-linux-gnu-debug.tar.gz
4fc9874cd91344348602a91b6ab680293f6aa1afb5bab54147bdcfe3c69c8777 guix-build-5492e1be3b40/output/aarch64-linux-gnu/bitcoin-5492e1be3b40-aarch64-linux-gnu.tar.gz
899d2d
...

💬 marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2244676459)
Second commit was a bit sloppy it seems😔 I'll clean up this function in the next PR

🤔 Sjors reviewed a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3074175438)
Reviewed the non-base commits up to e6889565af3afed567e08976b4bc0f0381d0bf8 _sign: Add CreateMuSig2Nonce_, but I skipped over 25c4fd4e50a6d230c959fb0072df04ef7093aa0a _sign: Add CreateMuSig2AggregateSig_.

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2244536549)
In 1d4fdeb63792988b4fb93cf8e84142a3a5b34a39 _sign: Include taproot output key's KeyOriginInfo in sigdata_: can you update the description of `taproot_misc_pubkeys` to mention that this can now also include the output key?

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2244607495)
In f4b4c704e40cc11663dbdaa156b1775be668a15f _signingprovider: Add musig2 secnonces_: maybe add an `Assume` here?

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2244604441)
In f4b4c704e40cc11663dbdaa156b1775be668a15f _signingprovider: Add musig2 secnonces_: needs guard?

```cpp
if (!musig2_secnonces) return std::nullopt;
```

Ditto for `DeleteMuSig2Session`

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2244500941)
In 5ff19c37af53f6ebcd69a9973ededcbadd69d3bd _pubkey: Return tweaks from BIP32 derivation_: in BIP32 this is I_L which isn't given a name. In libsecp it's called tweak, so I guess that's fine.

Although maybe `bip32_tweak` is better, so as not to confuse it with the tweaks in taproot.
https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#public-parent-key--public-child-key

💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2244611331)
In f4b4c704e40cc11663dbdaa156b1775be668a15f _signingprovider: Add musig2 secnonces_: I guess there's no realistic scenario where two different sessions actually need to be merged, but maybe add a comment to point this out.