psbt: add new fields for MuSig2 - #2198
Conversation
Pull Request Test Coverage Report for Build 9513655947Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9519109524Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9528732501Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9528834149Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9528865705Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9528900206Details
💛 - Coveralls |
63f97ed to
8e879e1
Compare
Pull Request Test Coverage Report for Build 14964403415Details
💛 - Coveralls |
e4bb95c to
04ec8d7
Compare
|
The BIP-0373 PR was merged a while ago. I also rebased on latest master that has the Golang module fixes and addressed all TODOs. This is ready for review IMO. |
Allows other packages to use the ComputeSigningNonce function.
1e599b1 to
6fc877f
Compare
6fc877f to
c9f6407
Compare
|
@TechLateef thank you very much for the thorough review! I've addressed all your comments. |
sputn1ck
left a comment
sputn1ck
left a comment
There was a problem hiding this comment.
BIP-373 implementation review. Inline findings below.
| keyAggOpts []musig2.KeyAggOption, | ||
| combineOpts []musig2.CombineOption) (*schnorr.Signature, error) { | ||
|
|
||
| aggKey, _, _, err := musig2.AggregateKeys(set.keys, true, keyAggOpts...) |
There was a problem hiding this comment.
[P1] Preserve participant aggregation order
The signing set is reconstructed from nonce-map order, then AggregateKeys is always called with sorting enabled. BIP 373 stores participants in the required KeyAgg order and BIP 327 makes sorting optional. I reproduced this with a valid explicitly unsorted session; MaybeFinalizeAll rejects it. Please select the matching participant record, map nonces and signatures into its Keys order, and aggregate with sort=false.
There was a problem hiding this comment.
Fixed and added tests for!
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| if xpub == nil { |
There was a problem hiding this comment.
[P1] Implement the BIP 328 fallback
Derived aggregate finalization requires PSBT_GLOBAL_XPUB and explicitly rejects its absence, but BIP 373 says to assume BIP 328's fixed synthetic chain code when no matching global xpub exists. The official case 4c partial signatures therefore cannot be aggregated after removing the preaggregated key signature.
There was a problem hiding this comment.
Addressed and unit tested.
| var sigHashMsg [32]byte | ||
| copy(sigHashMsg[:], sigHash) | ||
|
|
||
| // No taproot tweak: the aggregate key is the key in the script and is |
There was a problem hiding this comment.
[P1] Apply derivation tweaks to script spends
The tapscript finalizer always combines with no tweaks. BIP 373 permits the key inside a leaf to be derived from a parent MuSig2 aggregate, in which case the BIP32 derivation tweaks must also be applied here. Otherwise this method installs an invalid witness.
| return nil, fmt.Errorf("no MuSig2 partial signatures on input") | ||
| } | ||
|
|
||
| if len(pInput.MuSig2PubNonces) != numSigs { |
There was a problem hiding this comment.
[P1] Group independent signing contexts
This requires the total nonce and signature counts to match and later requires every record to use one aggregate and leaf. BIP 373 keys these records so several aggregates or spend paths can coexist. A complete key-path session plus one nonce for another leaf becomes ErrNotFinalizable. Please group by aggregate key and tapleaf hash, then select a complete context.
There was a problem hiding this comment.
Makes sense, fixed.
c9f6407 to
0541026
Compare
4 participants
Implements the new input and output fields to the PSBT library according to BIP-0373.