Schnorrkel uprev #368
Schnorrkel uprev #368
Conversation
crypto/sig/src/lib.rs
Outdated
| let mut hasher = Blake2b256::new(); | ||
| hasher.input(context_tag); |
There was a problem hiding this comment.
I think it would be a good idea to add as a prefix, the length of the context_tag as well, since it is variable length
that prevents situations where "", private_key1 "privatekey1 foo" has the same nonce as a signature from another context"privatekey1", privatekey1, "foo")
There was a problem hiding this comment.
(i dont think we need framing around the message because it is the last thing in the sequence)
There was a problem hiding this comment.
i think this kind of framing is what merlin does: https://merlin.cool/transcript/ops.html#appending-messages
There was a problem hiding this comment.
I agree that including the lengths of any variable-length tags/context/etc. is a good idea. For simplicity's sake, you probably just want to use merlin again for the nonce generation here, using Transcript.build_rng(), so that you're getting identical behaviour w.r.t. domain separation as you are in the Schnorrkel signature (or, at least, no worse, since Schnorrkel is also using merlin).
|
LGTM otherwise! |
crypto/sig/src/lib.rs
Outdated
| let mut hasher = Blake2b256::new(); | ||
| hasher.input(context_tag); |
There was a problem hiding this comment.
I agree that including the lengths of any variable-length tags/context/etc. is a good idea. For simplicity's sake, you probably just want to use merlin again for the nonce generation here, using Transcript.build_rng(), so that you're getting identical behaviour w.r.t. domain separation as you are in the Schnorrkel signature (or, at least, no worse, since Schnorrkel is also using merlin).
Soundtrack of this PR: DJ Fauci
Motivation
We we were previously carrying a patch of Schnorrkel in order to pass in our own rng, which enabled us to build in
no_std. However, as mentioned in w3f/schnorrkel#55, we can use attach_rng instead. This requires us to use the Merlin Transcript directly, as opposed to the signing_context, which is a wrapper around a Transcript. This is fine, as the signing context functionality is preserved in this revision.In this PR
MC-1619, MC-1761