test vectors: mention that plain after x-only tweaking is optional #68
Conversation
This file contains 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
This gives better test coverage for implementations that do not allow plain tweaking after x-only tweaking.
|
See #32 (comment) |
|
ACK |
|
Concept ACK Though we should still do mention somewhere in the text that not all implementations support every feature of the BIP seems fine. Otherwise this note in the test vector comes a little bit out of the blue. (Can be a different PR) |
The section names were potentially confusing because the section that was called "Description of Signing" is actually part of the specification.
bip-musig2.mediawiki
Outdated
| This specification is written with a focus on clarity. | ||
| As a result, the specified algorithms are not always optimal in terms of computation and space. | ||
| In particular, some values are recomputed but can be cached in actual implementations (see [[#signing-flow|Signing Flow]]). | ||
|
|
||
| == Description of Signing Flow == | ||
| Depending on the application scenario, some features of the BIP are unnecessary, and implementors can decide not to support them. |
There was a problem hiding this comment.
Suggested change
| Depending on the application scenario, some features of the BIP are unnecessary, and implementors can decide not to support them. | |
| The goal of this BIP is to provide a standard that supports a wide range of possible application scenarios. | |
| Given a specific application scenario, some features supported by this standard may be unnecessary or not desirable, and implementations can choose not to support them. |
Comment on lines
+67
to
+72
| Such features include: | ||
| * Applying plain tweaks after x-only tweaks. | ||
| * Applying tweaks at all. | ||
| * Dealing with messages that are not exactly 32 bytes. |
There was a problem hiding this comment.
Suggested change
| Such features include: | |
| * Applying plain tweaks after x-only tweaks. | |
| * Applying tweaks at all. | |
| * Dealing with messages that are not exactly 32 bytes. | |
| Such features include: | |
| * Applying plain tweaks after x-only tweaks. | |
| * Applying tweaks at all. | |
| * Dealing with messages that are not exactly 32 bytes. | |
| * Identifying a disruptive signer after aborting (aborting itself remains mandatory). | |
| * Dealing with duplicate public keys in key aggregation. |
bip-musig2.mediawiki
Outdated
| * Applying plain tweaks after x-only tweaks. | ||
| * Applying tweaks at all. | ||
| * Dealing with messages that are not exactly 32 bytes. | ||
| The corresponding test vectors can be skipped or can result in an error. |
There was a problem hiding this comment.
Suggested change
| The corresponding test vectors can be skipped or can result in an error. | |
| If applicable, the corresponding algorithms should simply fail when encountering inputs unsupported by a particular implementation. (For example, the signing algorithm may fail when given a message which is not 32 bytes.) | |
| Similarly, the test vectors that exercise the unimplemented features should be re-interpreted to expect an error, or be skipped if appropriate. |
Is this what you meant?
bip-musig2.mediawiki
Outdated
| @@ -55,13 +55,22 @@ The MuSig2 variant in this specification stands out by combining all of the foll | |||
| * '''MuSig2* optimization''': The specification uses an optimization that allows saving a point multiplication in key aggregation. The MuSig2 scheme with this optimization is called MuSig2* and proven secure in the appendix of the [https://eprint.iacr.org/2020/1261 MuSig2 paper]. The optimization is that the second distinct key in the list of public keys given to the key aggregation algorithm (as well as any keys identical to this key) gets the constant key aggregation coefficient ''1''. | |||
| * '''Parameterization of MuSig2 and security''': In this specification, each signer's nonce consists of two elliptic curve points. The [https://eprint.iacr.org/2020/1261 MuSig2 paper] gives distinct security proofs depending on the number of points that constitute a nonce. See section [[#choosing-the-size-of-the-nonce|Choosing the Size of the Nonce]] for a discussion. | |||
|
|
|||
| == Overview == | |||
|
|
|||
| Implementors of the specification must make sure to understand this section thoroughly to avoid subtle mistakes that may lead to catastrophic failure. | |||
There was a problem hiding this comment.
micro nit:
Suggested change
| Implementors of the specification must make sure to understand this section thoroughly to avoid subtle mistakes that may lead to catastrophic failure. | |
| Implementers of the specification must make sure to understand this section thoroughly to avoid subtle mistakes that may lead to catastrophic failure. |
(seems more common https://english.stackexchange.com/questions/195699/implementor-vs-implementer )
There was a problem hiding this comment.
also changed this at another location
Also move "This specification is written [...]" sentence to the overview section which is a better place.
jonasnick
force-pushed
the
tweak-order
branch
from
670277d
to
19e1421
Compare
real-or-random
approved these changes
Sep 15, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.