Why do you use a fork of `curve25519-dalek`?

[tarcieri]

We get a pretty steady stream of accidental PRs from people's forks of curve25519-dalek. That makes me wonder if there are things we could put in a hazmat API which would allow such use cases to consume the code as a crate/library rather than a fork.

I assume access to the field element implementation is a big part of it, but also one of the most problematic to expose, not only due to massive footgun potential, but because there is currently no common FieldElement type suitable for public exposure. #787 adds such a type, but it has its own problems and deliberately doesn't expose lazy reductions, which I expect many users attempting to consume the field element implementations wants for performance. If you're interested in a public "hazmat" FieldElement type, please leave a note about whether #787 fits your use cases (and I suppose we could open a separate tracking issue about that).

Are there any other things we could put under a hazmat API that would eliminate the need to fork curve25519-dalek and allow using it as a crate instead?

[burdges]

I always use this crate myself now that its maintained again, so I won't clutter this thread, but I've posted #812 for discussion about converting to other's field representations

[kayabaNerve]

Edit: The following is most of my original response, yet having spent some time reviewing where things stand now, 90% of my use-cases would be resolved by #816 and zkcrypto/group#68.

---

I maintain and 'promote' (via my usage of and requirements on) dalek-ff-group. It isn't a fork yet a wrapper. dalek-ff-group was originally created for two explicit reasons:

1. The ff, group APIs which are now present. If I recall correctly, I tried to use the upstream implementations once added but was blocked by lack of some helpers I had added along the way.

2. The FieldElement API. For what it's worth, performance isn't a concern of mine. Yes, performance is great, but I shipped a crypto-bigint-based module from... crypto-bigint 0.3? Before Residue was a thing? And it always performed reductions. Lazy reduction is a nice-to-have for performance but I just want the functionality, and I want it correct.

I also made the repository to bump the version of digest used (when curve25519-dalek hadn't published an update in a long time), and still have a couple extensions I'm unsure alternatives are present for within curve25519-dalek.

1. FieldElement, which I of course have an open PR for (ff FieldElement #787)
2. impl FieldElement { const fn from_u256() -> Self }, and I still use impl Scalar { const fn from_bits() -> Self for this purpose (despite its deprecation and unsafety, something something "I know what I'm doing").
3. Ciphersuite. Three years ago, I needed more functionality than offered via the existing ff/group traits (at least, per my knowledge of them), so I wrote a bespoke trait. Ideally, it'd simply be a marker trait, but I can't yet so deprecate it. https://github.com/serai-dex/serai/blob/c0e48867e13f8a325cf860403a27eaa8da94fcbc/crypto/ciphersuite/src/lib.rs#L40

Though I don't need my own curve25519-dalek to define Ciphersuite. I define a Ciphersuite impl around k256, p256, simply be defining my own struct and pointing to types from it (https://github.com/serai-dex/serai/blob/c0e48867e13f8a325cf860403a27eaa8da94fcbc/crypto/ciphersuite/kp256/src/lib.rs#L37). I'd just like to take a moment to hijack this issue to discuss Ciphersuite as it states my generic requirements on the elliptic curves I use and I would like to replace it with upstream components as possible (see how we use the digest 0.11 pre-release for hybrid-array to achieve use of FromUniformBytes).

The main blocker with removing Ciphersuite would actually be the read_G helper. I can't define it in an extension trait because I need curves to be able to override it. The provided method assumes encodings are canonical, so by re-encoding it can check if it read a canonically-encoded point. The issue is that re-encoding check is very expensive. That's why, in all the elliptic curves I implement, I require points be canonically encoded and override it with an impl which just says 'if it decodes, it is canonical'. A canonical point decode would have to be upstreamed to group or elliptic-curve (and implemented for all curves I support). Reference to #380.

I also would much prefer upstreaming to group, as while I love elliptic-curve, it's a much larger dependency. I've accepted it for k256, p256, which 'always had it', but I'd be hesistant to so increase the scope of curve25519-dalek.

The rest of Ciphersuite, besides marker traits and helpers which could be extensions, would be:

• The ID field, which is pleasant when I'm building transcripts but unnecessary/could be a much smaller trait

• The dedicated generator function. It's a couple hundred lines of code to wrap a Group impl into one with a distinct generator. It's ~20 lines to define a Ciphersuite. This could itself be its own minimal trait though. See Consider promoting Group::{identity, generator} to associated constants zkcrypto/group#32 Decide on a generator API zkcrypto/group#45. In hindsight, I don't see why we couldn't:

  • Move generator to its own trait
  • Within its own trait, have it be a constant

Which was somewhat proposed back in the day, but that discussion unfortunately didn't go too far.

For a real-world use-case, I can point to how the next Monero protocol upgrade moves commitments to private keys from being over G to being over a new generator, T. It isn't that the generator fn is bad, and it isn't that it isn't used in libs. It's that for half our keys (all modern ones) we use a distinct generator, which under the current Group trait, is a massive pain. While we can simply not use the generator API, we'd also need all libraries not to, when again, providing and using a default generator makes sense. It just needs to also be easy to define alternates.

• The associated hash function. Again, could be its own much smaller trait, yet it's sometimes pleasant to have a hash 'with the curve'. That way, if I write generic code (like a DLEq), I don't mandate usage of Blake2b-512. Ed25519 is frequently used with SHA2-512 and it telling me 'please use SHA2-512' keeps the dependency tree straightforward. It's also a requirement to ensure a hash is used with a long enough output to achieve an negligibly biased output, without piping to an XOF and resampling using a canonical decoding function.

[kayabaNerve]

@Slixe who maintains a curve25519-dalek fork, I believe due to shipping an accelerated solver for small discrete logarithms in theirs.

https://github.com/xelis-project/curve25519-dalek

[Slixe]

I guess once we can use FieldElement as a dependency, a RistrettoPoint::coset4 publicly accessible and a FieldElement::is_zero_not_ct, thats all we need now.

Our ECDLP module could be converted into a crate itself