Schnorr signature #6
Conversation
src/main/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/FixedGenerators.kt
Outdated
Show resolved
Hide resolved
| import java.security.SecureRandom | ||
| import kotlin.random.Random.Default.nextBytes | ||
|
|
||
| enum class Hash{ |
There was a problem hiding this comment.
is this not already defined somewhere in the lib? because Alexei did one of this hashes.
There was a problem hiding this comment.
It's not, afaik. But I removed this outside of this class and defined as an enum with other supported hash functions. Because, in the future, Mimc or Poseidon can be used here.
src/main/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrSignature.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrSignature.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrSignature.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrTest.kt
Outdated
Show resolved
Hide resolved
|
Please see In general, very usefu, looking forward to embedd this in into the |
There was a problem hiding this comment.
First of all: nice work! really cool to have this.
I have only two minor comments for now.
One question: I see some of Victor's comments marked as resolved, but no code changed for it. Is that the result of agreement between you outside GitHub comments? I have +1'd the ones I feel still need to be addressed.
Finally, I agree with Victor that this would be a good time to 'upgrade' the build of this project a bit with both Junit5 and Spotless. Alternatively do it directly after in a separate PR. It is easily copied from the ZK Notary project. I can help with that.
src/test/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrTest.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/ing/dlt/zkkrypto/ecc/schnorr/SchnorrSignature.kt
Outdated
Show resolved
Hide resolved
|
|
||
| return Signature(rPoint, s) | ||
| } | ||
|
|
||
| fun signHashedMessage(msgBytes: ByteArray) : Signature { | ||
| fun signHashedMessage(msgBytes: ByteArray): Signature { |
There was a problem hiding this comment.
This now also only supports messages of MAX_MESSAGE_SIZE. What is the added value above the raw version then? Why don't we make it support any length? That should be possible with Blake2s, right?
| import org.junit.jupiter.api.Test | ||
| import java.math.BigInteger | ||
|
|
||
| import kotlin.system.measureNanoTime | ||
|
|
||
| class SchnorrSignatureTest { |
There was a problem hiding this comment.
I would like to see more tests (preferably one parametrized test) that use test vectors that prove that this is really a conforming implementation. Are there test vectors we can get from somewhere? Possibly not for this specific curve, but I think vectors exist for Secp256k1?
There was a problem hiding this comment.
I added tests with test vectors. I used the message values from Zinc and from Corda EdDSA, which are smaller than 32 bytes. I also needed to add a setter function for keys and remove seed generation out of the signing function to be able to compare the generated values with the test vectors.
| /** | ||
| * Implementation of Schnorr signatures | ||
| * This implementation uses AltBabyJubJub curve and Blake2 hash | ||
| */ | ||
|
|
||
| class SchnorrSignature ( | ||
| class SchnorrSignature( |
There was a problem hiding this comment.
Have you already looked at how to integrate this into Corda through/as a Security Provider? I thought to be able to do that, the Signature implementation has to be a SignatureSpi:
* This class defines the <i>Service Provider Interface</i> (<b>SPI</b>)
* for the {@code Signature} class, which is used to provide the
* functionality of a digital signature algorithm. Digital signatures are used
* for authentication and integrity assurance of digital data.
*.
* <p> All the abstract methods in this class must be implemented by each
* cryptographic service provider who wishes to supply the implementation
* of a particular signature algorithm.
And this would also require you to make the keys as standard keys: java.security.PublicKey and java.security.PrivateKey.
That said, I am not a specialist in this area, I just looked at where Corda gets its supported signature schemes from and how they are used. Perhaps you have found another, easier way in which the current API you designed can be used.
Warning: 'Expected performance impact from inlining is insignificant. Inlining works best for functions with parameters of functional types'. So removed inline prefix.
mvdbos
force-pushed
the
schnorr-signature
branch
from
c4e0e1e to
4283283
Compare
|
Maybe also update readme file? |
Updated it now! |
PR for the implementation of Schnorr signatures. The code implements a variant of Schnorr signatures, EdDSA on AltBabyJubJub curve using Blake2 hash functions.
The signing algorithm has two variants: One with the raw message, i.e., the message is not hashed before signing, and one with message hashing.