btcec/schnorr/musig2: update to musig 1.0.0 #1894
Conversation
|
Correctness tests work now, next step is to update all the test vectors. |
The two _concrete_ changes between version 0.4.0 (where we are before
this commit), and version 0.7.0 are:
1. Variable length messages are now allowed, this comes with a new 8
byte prefix for the messages.
* Our implementation was already using a `[]byte` for the
message/hash, so no extra API changes are needed here.
2. The serialization for a blank message and a normal message (for
nonce gen) is now distinct. A single byte is added (either 0 or 1)
to indicate if a message was passed into nonce generation.
The major change in musig 1.0.0 is that plain public keys are used as input to key aggregation.
|
Ok, this is ready for review now: all test vectors pass! |
Pull Request Test Coverage Report for Build 3325072371
💛 - Coveralls |
| // If the message isn't present, then we'll just write out a single | ||
| // uint8 of a zero byte: m_prefixed = bytes(1, 0). | ||
| case 0: | ||
| case opts.msg == nil: |
There was a problem hiding this comment.
nit: squash this commit with previous one that updates the message encoding logic?
There was a problem hiding this comment.
LGTM!
In order to mitigate https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021000.html we could add in the public key to GenNonces, as in jonasnick/bips#74.
Another fix would be to make our WithSecretKey option mandatory.
If you use the btcd musig session, we are already safe, as GenNonces is called with the privkey option.
btcd/btcec/schnorr/musig2/context.go
Line 483 in 323871f
I think there is no reason to not add the privkey to the other use of
GenNonces here btcd/btcec/schnorr/musig2/context.go
Line 242 in 323871f
However adding the pubkey to GenNonces or making WithSecretKey mandatory would make outside usage of GenNonces less user error prone.
Great point, added in a new commit tacked onto the end.
So this is already available as a cutom option via |
This helps mitigate an issue discovered in musig2 under certain scenarios: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021000.html.
The two concrete changes between version 0.4.0 (where we are before
this commit), and version 0.7.0 are:
Variable length messages are now allowed, this comes with a new 8
byte prefix for the messages.
[]bytefor themessage/hash, so no extra API changes are needed here.
The serialization for a blank message and a normal message (for
nonce gen) is now distinct. A single byte is added (either 0 or 1)
to indicate if a message was passed into nonce generation.
The major change in musig 1.0.0 is that plain public keys are used as
input to key aggregation.
Some correctness tests are failing, so leaving this in draft until I fix those
and also update to the latest set of test vectors.