[WIP] Allow pregenerating nonces for future MuSig2 signing - #552
Draft
bigspider wants to merge 5 commits into
Draft
[WIP] Allow pregenerating nonces for future MuSig2 signing#552bigspider wants to merge 5 commits into
bigspider wants to merge 5 commits into
Conversation
Contributor
|
elf sizes
Stack consumption summary
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #552 +/- ##
===========================================
+ Coverage 76.36% 76.46% +0.09%
===========================================
Files 96 96
Lines 10887 10882 -5
Branches 2529 2531 +2
===========================================
+ Hits 8314 8321 +7
+ Misses 2244 2232 -12
Partials 329 329
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Round 1 and round 2 each had their own inline call to musig_nonce_gen, with their own handling of the synthetic randomness. They must derive exactly the same nonce: if the two ever diverged, the partial signatures of round 2 would not match the pubnonces published in round 1, and the aggregate signature would be invalid. Move the derivation into musig_derive_nonce(), which owns the lifetime of rand_i_j and zeroes the secnonce on failure. This is a pure refactor; the derivation itself is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `aggpk` argument passed to NonceGen was the aggregate key after all the tweaks, which depends on the input's (change, address_index) and on the taptree. That made each pubnonce usable for one input of one transaction only, forcing a client to involve the signing device twice for a MuSig2 signature: once to learn the pubnonces, and once to sign. Pass the aggregate key of the musig() key expression before any tweak instead. It is a per-account value, so the nonce now depends only on the session randomness, the input index, the key expression index and the wallet policy. In BIP-0327 `aggpk` is an optional argument of NonceGen, and (like `msg`, which this app already omits) it only serves to add entropy as a defense in depth against a faulty RNG. The uniqueness of the nonce does not rely on it: `rand_root` comes from the hardware RNG, and compute_rand_i_j provides the (input_index, keyexpr_index) domain separation. The two rounds change together, so pubnonces and partial signatures stay consistent; the aggregate key, and therefore the PSBT field keys, are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
doc/musig.md prescribes that round 2 verifies that the pubnonce contained in the psbt matches the one synthetically recomputed, but the check was never implemented: my_pubnonce was read from the psbt only to detect its presence, and its value was never compared to the regenerated pubnonce. Without the check, a psbt whose pubnonces do not belong to the session in storage still produces a partial signature, computed with a secnonce that is not the one committed to in the aggregate nonce. The result is an invalid aggregate signature, and the other cosigners rightly identify this signer as the disruptive one. The new test asserts the clean failure; it fails with "DID NOT RAISE" without this commit. This check also replaces the guarantee that used to come from having the transaction in the psbt_session_id: rather than approximating "same transaction", it directly verifies "these are the nonces of the session I am about to use", which is the property that actually matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The psbt_session_id used to commit to the transaction hashes, so round 2
could only find the session that round 1 had stored if it was given the same
transaction. Together with the nonce derivation, that forced a client to
involve the signing device twice per MuSig2 signature.
Derive the id from the wallet policy alone. A client can now execute round 1
whenever it likes -- for example while the device happens to be connected --
and use the resulting pubnonces for whichever transaction it later builds,
as long as its inputs sit at the same indexes.
The tradeoff is that at most one round-1 batch can be pending per wallet
policy, since a new round 1 replaces the session in storage. When that
happens, round 2 for the older batch now fails cleanly on the pubnonce
check of the previous commit instead of silently producing a partial
signature for a nonce that was never published.
The id is versioned in the tagged hash ("PsbtSessionId2"), so that a
session stored by an older version of the app can never be matched.
Since tx_hashes was its only use, compute_musig_per_input_info() no longer
takes the signing state, and the id moves out of musig_per_input_info_t: it
is a property of the account, not of the input.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Now that the psbt_session_id does not depend on the transaction, the id is no longer a second line of defense for the volatile copy of the session that round 2 pops from storage: a _round2 that survived into a later signing flow would be _accepted_ by musigsession_round2_initialize() instead of being rejected for having a different id. The same secnonce would then sign two different transactions, which is enough to recover the private key. Today the invariant holds, because handler_sign_psbt keeps the signing state in a zeroed stack local. But the comment above G_sign_psbt_cache explicitly contemplates moving such state to globals to stay within the stack budget, so make the requirement explicit instead of incidental: - document in musig_sessions.h that the state must never outlive the flow, and never become a global; - assert in musigsession_initialize_signing_state() that _round2 is zeroed on entry, which fails loudly if the state ever starts being reused; - zero the whole state in musigsession_commit(), so that a completed flow leaves neither a usable session nor a rand_root in RAM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bigspider
force-pushed
the
musig-pregen-nonces
branch
from
September 4, 2026 10:18
26dcc77 to
385711a
Compare
Contributor
Code coverage reportPer-file coverage
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exploring how to address #551