💬 achow101 commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/32683#issuecomment-2945644819)
> Import a P2SH publickey

Can you be more specific as to what you imported? P2SH is, by definition, more than just a public key nor do P2SH scripts necessarily have to have a public key.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129795111)
I changed the commit message based on your suggestion. I dropped the "make the code less complicated" part -
it's a bit subjective and anyway not my main motivation for this PR:
I think that in an area as central as validation we should avoid having structures that are populated on a "best-effort" basis (and therefore cannot be subjected to rigorous checks as in `CheckBlockIndex`) and trust that no one will ever rely on them for anything too serious in the future.

💬 enirox001 commented on issue "wallet: render BIP388 wallet policies":
(https://github.com/bitcoin/bitcoin/issues/32659#issuecomment-2945652066)
Thanks for the clarification! That's a crucial point about listdescriptors and the /** handling.

I agree completely that getdescriptorinfo is the right place to start. My plan is to focus on implementing the policy and keys fields there, by adding logic to parse the input descriptor into a BIP 388 policy if it's compatible.

I'll keep the idea of a dedicated listwalletpolicies RPC in mind for a follow-up contribution, as that seems like the cleanest way to expose stored wallet policies without
...

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129806417)
> which is referring to the loop in Chainstate::SetBlockFailureFlags

It's referring to `SetBlockFailureFlags` and `RecalculateBestHeader` which both loop over the block index.

> unnecessary database writes

I agree that this is a potential follow-up (to #31835, pinging @stratospher). It's probably not performance-critical, because it's also guarded by PoW (creating multiple PoW-valid headers that build on a failed block is very costly).

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129810763)
I rewrote the commit message - hoped it's clearer now.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2945690965)
[ed172d3](https://github.com/bitcoin/bitcoin/commit/ed172d3002da68a3ddbd5d13e7d3f0618c1378d4) to [1f09e37](https://github.com/bitcoin/bitcoin/commit/1f09e3754363382806cc1e3d6bed25cd235ac5f4):
Addressed feedback by @stickies-v and @ryanofsky - thanks!

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129101313)
IIUC, I think we should explicitly pass a dummy signing provider here instead of `arg` because there can't be a hardened derivation at this level. Also, better to not pass the signing provider if we know for certain that it will not be used.

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129134556)
Interesting, I now understand why it's written in a double negation way.

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129128674)
Slightly confused in this `else` block: for the case of ranged participants and non ranged musig derivation that has a `m_path` such as `/0/1/2`, it doesn't seem to do the derivation after getting the aggregate pubkey because I don't see `m_path` being used here like it is used in `m_aggregate_provider` above.

I don't see an explicit mention of this in BIP 390 as well, is this not a valid case to consider?

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129133487)
```diff
- Cannot have hardened hardened derivation
+ Cannot have hardened derivation
```

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129163516)
Aren't the participant pubkeys already derived in the loop on line 659? Here it seems to only aggregate the derived keys.

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129819178)
I'm sensing that this block of derivation of participants' keys should reside later in the `else` block starting on line 674. Reason being that in case of non ranged participants that leads to the presence of `m_aggregate_provider`, we don't really need to derive the participants' keys again and `pos` is effectively unused.

After moving this block down there, either we can derive the keys in the `if` block below with 0 `pos`, or preferably cache the participants' derived keys along with `m_ag
...

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129857969)
> Good point, these 2 structs in PSBT would need to be updated:

I believe there won't need to be any changes to PSBT. If a participant pubkey is duplicated, it is still valid to for each time the pubkey appears, to use the same pubnonce and the same partial sig for that nonce for each instance the pubkey appears. The only complication with this is if the duplicate pubkeys are different signers and produce different nonces and partial sigs as they may overwrite each other resulting a PSBT wit
...

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129889627)
It's disallowed

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129925030)
Even with aggregate derivation, `out` needs to be filled with participant pukey info. This loop is present in order to do that for both aggregate derivation and participant derivation.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2129925291)
Added a test that has interleaved worst peers in a LimitOrphans call

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2129927509)
Implemented the general idea, i.e. keep trimming until this peer wouldn't be the next thing we pop from the heap. There was some iterator invalidation going on here

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2129928431)
Good idea, done

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129933928)
Done

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2129934262)
Done