musig2: Add adaptor signature support to MuSig2 protocol #2325
Conversation
This commit adds comprehensive support for adaptor signatures in the MuSig2 protocol, including: - New methods for generating and setting adaptor points - Support for creating partial adaptor signatures - Methods to adapt final signatures and recover adaptor secrets - Updated signature verification and combination logic - Added test cases for adaptor signature functionality across different signer configurations
| } | ||
|
|
||
| // The adaptor point added to the combined nonce must be even. | ||
| // We keep generating random secrets until we find one that works. |
There was a problem hiding this comment.
Do we need to do this in a loop? Can't we just take the inverse of the secret to get an even y coordinate if the first draw produces an odd one?
There was a problem hiding this comment.
A loop is needed because we want the adaptor point added to the combined nonce to be even, not just for the adaptor point to be even.
There was a problem hiding this comment.
Yeah, I understand that. But the evenness should be deterministic. So wouldn't it be possible to just invert the secret if we find out that (after adding the adaptor point to the nonce) it's not even, we can invert it instead of drawing a new value?
It's just that a for loop with no defined exit condition feels suboptimal.
btcec/schnorr/musig2/context.go
Outdated
| // the validity of the adaptor secret key depends on the combined nonce. It | ||
| // must also be called before any partial signatures are generated or provided, | ||
| // because the validation of the signatures depends on the adaptor point. | ||
| func (s *Session) GenerateAdaptor(msg [32]byte) (*btcec.ModNScalar, *btcec.JacobianPoint, error) { |
There was a problem hiding this comment.
nit: doc was added after this PR was opened, but we try to wrap before 80 characters to help with review and readability in narrow/multi-column environments: https://github.com/btcsuite/btcd/blob/master/docs/code_formatting_rules.md#80-character-line-length
btcec/schnorr/musig2/context.go
Outdated
| @@ -548,6 +573,129 @@ func (s *Session) RegisterPubNonce(nonce [PubNonceSize]byte) (bool, error) { | |||
| return haveAllNonces, nil | |||
| } | |||
|
|
|||
| // GenerateAdaptorSecret generates an adaptor secret key. This must be called | |||
There was a problem hiding this comment.
nit: Godoc comment/method name mismatch.
| return nil, nil, err | ||
| } | ||
|
|
||
| var secret btcec.ModNScalar |
There was a problem hiding this comment.
This seems duplicated with SetAdaptor. Couldn't we instead just call into that method until we don't get ErrInvalidAdaptorPoint? Should reduce the size of the diff quite a bit.
btcec/schnorr/musig2/sign.go
Outdated
| @@ -320,6 +332,19 @@ func Sign(secNonce [SecNonceSize]byte, privKey *btcec.PrivateKey, | |||
| return nil, err | |||
| } | |||
|
|
|||
| nonce.ToAffine() | |||
|
|
|||
| var adaptedNonce *btcec.JacobianPoint | |||
There was a problem hiding this comment.
How about adaptedNonce := nonce to avoid else case? Same for verifyPartialSig.
btcec/schnorr/musig2/sign_test.go
Outdated
| @@ -389,3 +390,119 @@ func TestMusig2SignCombine(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| func TestMusig2SignCombineAdaptor(t *testing.T) { | |||
| // Pre-generate 10 deterministic private keys | |||
There was a problem hiding this comment.
nit: comments should always be full sentences, including punctuation.
btcec/schnorr/musig2/sign.go
Outdated
| @@ -606,6 +635,8 @@ type combineOptions struct { | |||
| combinedKey *btcec.PublicKey | |||
|
|
|||
| tweakAcc *btcec.ModNScalar | |||
|
|
|||
| adaptorPub *btcec.JacobianPoint | |||
There was a problem hiding this comment.
Where is this actually used? It seems like you can set it with WithAdaptorCombine, but don't see it being read anywhere. Which indicates to me that we probably want a unit test case for this as well.
There was a problem hiding this comment.
You're right this is unnecessary. I've removed it along with WithAdaptorCombine.
This commit adds comprehensive support for adaptor signatures in the MuSig2 protocol, including: