Fix musig2 counter-based nonce generation API #153
Conversation
We used kotlin's ULong type for the counter arguments which mangles method names when used from Scala. We also add an alternate method for nonce generation that includes new paramaters (message and additional data) and deprecate (but keep) the old one.
Can you detail what you mean by that? What was the failure?
Why are we keeping the old function? Isn't it confusing? Since nobody uses it for now, isn't it better to just remove it? |
It was just unusable from Scala (name was mangled to
It's used in |
|
Kotlin's |
| * @param extraInput (optional) additional random data. | ||
| */ | ||
| @JvmStatic | ||
| public fun generateNonce(sessionId: ByteVector32, privateKey: PrivateKey?, publicKey: PublicKey, publicKeys: List<PublicKey>, message: ByteVector32?, extraInput: ByteVector32?): Pair<SecretNonce, IndividualNonce> { |
There was a problem hiding this comment.
Would it make sense to have a default null value for the optional parameters to simplify callers (message: ByteVector32? = null, extraInput: ByteVector32? = null)? Or do we rather want to strongly encourage providing those arguments and thus force callers to explicitly set them to null?
There was a problem hiding this comment.
Optional parameters are not understood by Scala so in Eclair we would have to provide them all anyway, but we could have another generateNonce() methods without these parameters.
There was a problem hiding this comment.
Optional parameters are not understood by Scala
Damn, I didn't realize that! It's probably not worth having explicit methods without those, let's just keep the existing code to encourage people to provide as much randomness as possible 👍
We used kotlin's ULong type for the counter arguments which mangles method names when used from Scala. We also add an alternate method for nonce generation that includes new paramaters (message and additional data) and deprecate (but keep) the old one.