Skip to content
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.

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

MuSig2 support #294

Draft
wants to merge 56 commits into
base: develop
Choose a base branch
from
Draft

MuSig2 support #294

wants to merge 56 commits into from

Conversation

bigspider
Copy link
Collaborator

@bigspider bigspider commented Oct 9, 2024

Add full support for musig() in wallet policies.

Also improves the signing flow to only sign for spending paths that are filled in in the PSBT (that is, the corresponding PSBT_IN_BIP32_DERIVATION or PSBT_IN_TAP_BIP32_DERIVATION fields are present), optimizing certain use cases for wallet policies with multiple alternative spending paths.

Closes: #208
Closes: #177

src/handler/sign_psbt.c Fixed Show fixed Hide fixed
@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 84.21053% with 12 lines in your changes missing coverage. Please review.

Project coverage is 84.67%. Comparing base (bc81ed0) to head (daa79a3).

Files with missing lines Patch % Lines
src/common/wallet.c 83.56% 12 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #294      +/-   ##
===========================================
- Coverage    84.76%   84.67%   -0.10%     
===========================================
  Files           17       17              
  Lines         2186     2231      +45     
===========================================
+ Hits          1853     1889      +36     
- Misses         333      342       +9     
Flag Coverage Δ
unittests 84.67% <84.21%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bigspider bigspider changed the title Musig2 MuSig2 support Oct 9, 2024
Copy link

sonarcloud bot commented Oct 29, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
6.0% Coverage on New Code (required ≥ 80%)
11.1% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

…ed some comments.

Generalizing to key expressions containing musig() makes it necessary to distinguish
the key expressions in the wallet policy from the actual key placeholders that are
just indexes to the list of key informations (@num in the descriptor template),
whereas the two concepts were often not clearly separated in the code base.

Renaming to "key expressions" makes the distinction more clear.
…on type is used; generalized some parts of the code that are not generalized to musig key expressions, and annotated some others.
@bigspider bigspider force-pushed the musig2 branch 3 times, most recently from 256f790 to 4616d67 Compare November 6, 2024 09:26
@bigspider bigspider force-pushed the musig2 branch 7 times, most recently from aae5a24 to 261d7d6 Compare November 7, 2024 11:55
It will be needed in order to enable silently participating to Round 1
of the MuSig2 protocol, which should only happen if no such fields are
found (which would indicate that Round 1 was already executed).
int sign_result = sign_transaction(dc, &st, cache, &signing_state, internal_inputs);

if (!G_swap_state.called_from_swap) {
ui_post_processing_confirm_transaction(dc, sign_result);

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect (because
ui_post_processing_confirm_transaction
has no external side effects).
…onfirmation.

The first round of MuSig does not involve access to the private keys and does not
complete authorizing a transaction. Therefore, it is safe to do it without user
confirmation, which allows software wallets to possibly do it in background.

In cases when the other cosigners are online, this allows to get a single-sig
user experience, as the user would only have to plug the device once, and confirm
a single action.
src/handler/sign_psbt.c Fixed Show fixed Hide fixed
src/handler/sign_psbt/musig_signing.c Fixed Show fixed Hide fixed
In the past, while the app was deployed on Nano S, we preferred
avoiding the use of Nano S in order to reduce the binary size.
Now, using qsort makes the code more readable and succinct.
This is redundant with the current implementation. However, the
musigsession module is written in such a way that the calling code
has no knowledge about its internal working. Therefore, it should
not assume that zeroing out is the correct way of initializing it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MuSig2 support Allow the client to only sign for a subset of key placeholders
2 participants