Skip to content

musig2: add WithExternalCombinedNonce option to Sign #2443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 27, 2025
Merged

musig2: add WithExternalCombinedNonce option to Sign #2443

merged 2 commits into from
Oct 27, 2025

Conversation

@sputn1ck
Copy link
Collaborator

@sputn1ck sputn1ck commented Oct 7, 2025 •
edited
Loading

This PR adds a new functional option to the Session.Sign function which allows specifying an external combined Nonce. This is useful when a central coordinator aggregates all nonces.

Externally Aggregated Nonces are part of the spec:

Third-party nonce and partial signature aggregation: Instead of every signer sending their nonce and partial signature to every 
other signer, it is possible to use an untrusted third-party aggregator in order to reduce the communication complexity from 
quadratic to linear in the number of signers. In each of the two rounds, the aggregator collects all signers' contributions 
(nonces or partial signatures), aggregates them, and broadcasts the aggregate back to the signers. A malicious aggregator can 
force the signing session to fail to produce a valid Schnorr signature but cannot negatively affect the unforgeability of the
 scheme.

https://github.com/bitcoin/bips/blob/master/bip-0327.mediawiki#user-content-Nonce_Aggregation

Roasbeef
Roasbeef reviewed Oct 7, 2025
View reviewed changes
@coveralls
Copy link

coveralls commented Oct 7, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 18458584969

Details

  • 23 of 27 (85.19%) changed or added relevant lines in 1 file are covered.
  • 8 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.02%) to 54.864%
Changes Missing Coverage Covered Lines Changed/Added Lines %
btcec/schnorr/musig2/context.go 23 27 85.19%
Files with Coverage Reduction New Missed Lines %
btcec/v2/schnorr/signature.go 1 79.46%
btcutil/gcs/gcs.go 3 81.25%
database/ffldb/blockio.go 4 88.81%
Totals Coverage Status
Change from base Build 17994535365: 0.02%
Covered Lines: 31147
Relevant Lines: 56771
💛 - Coveralls

@sputn1ck sputn1ck force-pushed the musig2_sign_with_agg_nonce branch from 37e656c to b2594f2 Compare October 7, 2025 18:25
@sputn1ck sputn1ck requested a review from Roasbeef October 7, 2025 18:26
bhandras
bhandras approved these changes Oct 7, 2025
View reviewed changes
Copy link
Contributor

@bhandras bhandras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you! 🎉

@sputn1ck sputn1ck requested a review from yyforyongyu October 7, 2025 18:56
Roasbeef
Roasbeef requested changes Oct 7, 2025
View reviewed changes
@sputn1ck sputn1ck force-pushed the musig2_sign_with_agg_nonce branch from b2594f2 to 824e867 Compare October 8, 2025 07:59
@sputn1ck sputn1ck requested a review from Roasbeef October 8, 2025 07:59
sputn1ck
sputn1ck commented Oct 8, 2025
View reviewed changes
btcec/schnorr/musig2/context.go
// - A combined nonce has been registered via RegisterCombinedNonce
//
// If the combined nonce is not yet available, this method returns an error.
func (s *Session) CombinedNonce() ([PubNonceSize]byte, error) {
Copy link
Collaborator Author

@sputn1ck sputn1ck Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this approach is good(allows for more missuse of the nonces i guess) or if we should just return the combined nonce one time only if we register the last pubnonce? (changing RegisterPubNonce sig to return the aggregated nonce, instead of a havAllNonces bool)

Copy link
Member

@Roasbeef Roasbeef Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each time we sign, we blank out localNonces, which prevents us from signing again directly after unless a new set of local nonces are generated.

Roasbeef
Roasbeef requested changes Oct 11, 2025
View reviewed changes
@sputn1ck
musig2: add Session.RegisterCombinedNonce
8f54cc6 
This commit adds a new function to musig2.Session, which allows the caller
to add an external aggregated nonce to the session.
@sputn1ck
musig2: add combinedNonce getter
21eb99e
@sputn1ck sputn1ck force-pushed the musig2_sign_with_agg_nonce branch from 3099b54 to 21eb99e Compare October 13, 2025 07:37
Roasbeef
Roasbeef approved these changes Oct 27, 2025
View reviewed changes
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎷

@Roasbeef Roasbeef merged commit 442ef28 into btcsuite:master Oct 27, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Roasbeef Roasbeef Roasbeef approved these changes

@yyforyongyu yyforyongyu Awaiting requested review from yyforyongyu

+1 more reviewer

@bhandras bhandras bhandras approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sputn1ck @coveralls @bhandras @Roasbeef