Skip to content

Commit

Permalink
Nits from PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
bigspider committed Dec 9, 2024
1 parent 6f50e7a commit 6489700
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 7 deletions.
2 changes: 1 addition & 1 deletion doc/bitcoin.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ For a registered wallet, the hmac must be correct.

For a default wallet, `hmac` must be equal to 32 bytes `0`.

The data yielded has with a `<tag_or_input_index>`, represented as a bitcoin-style varint; the rest of the message depends on the value of `<tag_or_input_index>`, as specified in the following subsections.
The data yielded begins with `<tag_or_input_index>`, represented as a bitcoin-style varint; the rest of the message depends on the value of `<tag_or_input_index>`, as specified in the following subsections.

##### If `<tag_or_input_index>` is at most 65535

Expand Down
6 changes: 3 additions & 3 deletions doc/musig.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ Storing the session in persistent memory only at the end of Phase 1, and deletin

#### Security of synthetic randomness

Generating $rand_{i, j}$ synthetically is not a problem, since the $rand\_root$ value is kept secret and never leaves the device. This ensures that all the values produced for different $i$ and $j$ not predictable for an attacker.
Generating $rand_{i, j}$ synthetically is not a problem, since the $rand\_root$ value is kept secret and never leaves the device. This ensures that all the values produced for different $i$ and $j$ are not predictable for an attacker.

#### Malleability of the PSBT
If the optional parameters are passed to the _NonceGen_ function, they will depend on the transaction data present in the PSBT. Therefore, there is no guarantee that they will be unchanged the next time the PSBT is provided.
Expand All @@ -82,6 +82,6 @@ However, that does not constitute a security risk, as those parameters are only

### Generalization to multiple PSBT signing sessions

The approach described above assumes that no attempt to sign a PSBT containing for a wallet policy containing `musig()` keys is initiated while a session is already in progress.
The approach described above assumes that no attempt to sign a PSBT for a wallet policy containing `musig()` keys is initiated while a session is already in progress.

It is possible to generalize this to an arbitrary number of parallel signing sessions. Each session could be identified by a `psbt_session_id` computed by hashing together the transaction hashes,
In order to generalize this to an arbitrary number of parallel signing sessions, one can identify each signing session with a `psbt_session_id`. Such `psbt_session_id` should deterministically depend on the transaction being signed (ignoring all the other PSBT fields), and the wallet policy being signed. In praticular, the computed `psbt_session_id` should be identical between Round 1 and Round 2 of the protocol. Note that malicious collisions of the `psbt_session_id` (for example by tampering with some details of the PSBT, like the SIGHASH flags) _are_ possible, but they do not constitute a security risk.
6 changes: 3 additions & 3 deletions src/common/wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,9 @@ static int parse_keyexpr(buffer_t *in_buf,
if (n_musig_keys < 2) {
return WITH_ERROR(-1, "musig must have at least 2 key indexes");
}
if (n_musig_keys > MAX_PUBKEYS_PER_MUSIG) {
return WITH_ERROR(-1, "Too many keys in musig");
}

// sanity check; the loop above should never exit with too many keys
LEDGER_ASSERT(n_musig_keys <= MAX_PUBKEYS_PER_MUSIG, "Too many keys in musig");

// allocate musig structures

Expand Down

0 comments on commit 6489700

Please sign in to comment.