feat: ed25519 digital signature scheme

[mrdaybird]

closes #78

• Implement keygen, sign, verify for ed25519 scheme
• add doc + better comments
• add benchmark
• added sha512

Note:

1. Even though I did not want to, I have had to add a dependency(crypto-bigint) to deal with large numbers (256, 512-bit) used in the code.
2. I have also added a dev-dependency, hex_literal, which is used for compile-time hex string to bytes conversion, which is not important but nice to have since there are a lot of such conversion throughout the repo.

[mrdaybird]

I think this is ready for review!

@0xJepsen @Autoparallel @brunny-eth

[Autoparallel]

I don't mind having either dependency -- especially dev-deps.

At some point it would be kinda cool to make our own bigint implementation. I honestly don't know the intricacies of how the popular libraries do it (i just imagine you'd do some generic ripple carry or something).

Let me review!

[Autoparallel]

I think we need semver CI running on PRs into main... I can add this elsewhere and rebase here.

[Autoparallel]

This is looking great! I left comments throughout.

Mostly, I think my concern here is that this specific implementation moves further away from "first principles" than I'd like it to. Perhaps we can work this a bit to use more of ronk's native types we have (e.g., our SHA impl, PrimeField, and Curves). There would be a good amount of work to make that possible I'd imagine. I roughly see what it is, but it would be good to work towards this.

So some options are to:

• See how much we can reduce external dependency usage now with minimal pain
• Make issues for what is needed so that we move towards only using our internal types in the future.

[Autoparallel]

Don't we already have a finite field sqrt we can use?

[mrdaybird]

If you mean the one for PrimeField, then I am not sure how, because this one is specialized for BaseField that is a type specialization of ConstMontyForm(from crypto_bigint) type

[mrdaybird]

Thanks for the review! @Autoparallel

Mostly, I think my concern here is that this specific implementation moves further away from "first principles" than I'd like it to. Perhaps we can work this a bit to use more of ronk's native types we have (e.g., our SHA impl, PrimeField, and Curves). There would be a good amount of work to make that possible I'd imagine. I roughly see what it is, but it would be good to work towards this.

I agree with you completely! That was my concern as well. There are few things I would to point out.
Firstly, the issue with PrimeField is that it is not designed to work with large numbers, that is not just having bigint support but also having support for fast modular arithmetic using Montgomery form or Barrett reduction. (like you said, it will be cool to have our own implementation)

Secondly, the issue with ronkation's curve implementation is that Edwards curve is a different form of elliptic curve (different from the Weierstrass from that is used in ronkathon), so RFC mentions different parameters. But I think the main difference is point representation that we have used. I have had used extended homogeneous coordinates (which is the also recommended as per the RFC) because using affine coordinates was at least an order of magnitude slower. So they use different point addition formula.

Lastly, with SHA, we had SHA256 not SHA512. So I had to add a dependency. But, like you said, it will be better to implement sha512. (maybe not a lot of work, i guess)

[mrdaybird]

Added a sha512 implementation. I have reused most of the code from sha256.rs.

[mrdaybird]

So some options are to:
- See how much we can reduce external dependency usage now with minimal pain
- Make issues for what is needed so that we move towards only using our internal types in the future.

I think we can start with adding support for large FiniteField(and friends) which uses Montgomery reduction or Barrett reduction. I will open an issue in this regard.

[lonerapier]

Awesome work sir. I have few questions that I mentioned on the respective files. I think we can generalise and reduce code dedeuplication.

Adding Edwards curve support will be hugee (if not, we should create an issue)

[lonerapier]

want to create a trait for DSA?

[mrdaybird]

sure!

[lonerapier]

This is really nice, but I'd like us to add support for Edwards curve in the curve module. will leave the decision for adding the support for this in current PR to you (i support adding it), but we should definitely create an issue, and remove this file from dsa module entirely.

[lonerapier]

I feel like we can combine sha256 and sha512 using const generics easily? most of the logic is duplicated

[mrdaybird]

The main issue is the different definition of sigma functions for sha512 and sha256, also the different paddings

[mrdaybird]

I am a beginner in rust, so if you want you can go ahead and do that!

[mrdaybird]

After a bit of thought, there are two phases to going dependency free, first add bigint library and then refactor exisiting code.
I will start by creating the initial scaffolding of bigint and a new bigint based field and curve. After that, major part will be refactoring the existing code to support bigint, for which I will create an issue.

[Autoparallel]

Adding Edwards curve would maybe not be that bad? Though it would be monumental for this PR to contain it. It's a different way of writing the equation (actually a slightly different equation at that) and homogeneous coordinates are just a different coordinate system.

It would be possible to make our curve trait generic over coordinate system and type. E.g., have something like:

EllipticCurve<Edwards, Homogeneous>
EllipticCurve<Weierstrass, Standard>

The main differences come down to how you implement the group operation.

---

As for the PrimeField concern. I'm curious if we could also put a generic in here too to handle cases for large primes. E.g.,

PrimeField<BigInt, 21888242871839275222246405745257275088548364400416034343698204186575808495617>

or something. In this case, we could implement the optimizations on the BigInt version of the PrimeField type. Though, I am not sure the optimizations are actually necessary as speed isn't exactly what we're going for (but we also don't want this to be incredibly slow either). I do actually think implementing optimizations is a good thing because they're a real part of most codebases. With that said, I'd be in favor of special implementations for BigInt type PrimeField elements. Specialization on structs in Rust works well at least (can't say the same for traits).

---

Also, nice work with the SHA-512. That was a good call and I appreciate you doing it. I think at this point moving forward to merge this and create issues to come back and solve these propositions would be my advice!

[mrdaybird]

At some point, we have to implement our own stuff and get rid of the deps, so better now than later.
I have already started implementing BigInt and BigInt based PrimeField in separate modules.
I am focusing on a "good enough" and simple implementation rather than the most optimal. This will give us a good start and later on we can improve on it.

Another motivation for implementing it here is that I found an excellent material on it in the Handbook of Applied Cryptography (See Chapter 14).

[Autoparallel]

@mrdaybird I'd be very curious to review. What's your current plan for these implementations?

[mrdaybird]

@Autoparallel

At the moment, PrimeField struct looks something like this:

pub trait Modulus<T> {
    const M: T;
    fn reduce(x: Concat<T>) -> T;
}

pub struct PrimeField<T, P: Modulus<T>> {
    pub(crate) value: T,
    phantom: PhantomData<P>,
}

(perhaps is PhantomData not needed...)

Then we provide macros to create struct that implements Modulus trait, like:

create_large_modulus!(P0, BigInt<4>, /* some large hex string*/);

This creates a struct name P0 that implement Modulus<BigInt<4>>.
We could have other macros like create_modulus that implements a usize based modulus.

[lonerapier]

while i like the direction you're going, i think it will be overkill for this PR to have a BigInt implementation. We can discuss this in a separate issue.

what's your view on choosing crypto-bigint as the default bigint dep, and move current Field, Curve implementation to that (maybe in a separate PR)?

@Autoparallel would like your take as well.