curve25519-dalek: add batch montgomery conversion

[apoelstra]

This is useful for vanitygenning curve25519 keys. Unsure of other usecases.

[tarcieri]

Seems like this could be implemented in an alloc-free manner using const generic arrays, although that would limit the input type to an array

[apoelstra]

Seems like this could be implemented in an alloc-free manner using const generic arrays, although that would limit the input type to an array

Agreed. It seems like it but it's a bit tricky. In the libsecp code we take an outptr for the result and during computation we abuse that as scratch space.

But here our output is a bunch of MontgomeryPoints which I think are big enough to abuse as scratch space, but internally they have a [u8; 32] and our scratch space consists of Scalars and I was worried that the conversions might cost some time.

I can give it a shot later today.

[apoelstra]

Oh, I realize that in your suggestion I don't need to reuse space; I can have multiple on-stack arrays of the appropriate length.

But if I do reuse space, then I could implement this by taking a &[Edwards] and &mut [Montgomery] and assert_eqing that their lengths match. This is un-Rustic but lets the user reuse a buffer and avoids any allocations.

[tarcieri]

Using const generics would allow you to return a same-sized array and avoids the runtime panic conditions

[apoelstra]

If I use const generics I see a slight slowdown with a batch size of 128, as well as blowing up the user's stack space (and limiting the API). I don't know that this is worth making the function work in a no-alloc context.

If I try to implement the outptr solution where the user passes a &mut [MontgomeryPoint], with safe code I see a massive slowdown due to the conversion costs when temporarily storing FieldElements in the slice.

[tarcieri]

The alloc dependency is fine, I guess. Maybe it'd be nice to offer both that and an alloc-free const generic version in the future.

Not sure how batch_to_montgomery jives in terms of API consistency. The other public APIs put batch at the end. Some bikeshedding/nit, but perhaps convert_batch_to_montgomery would be better?

Would be nice if @rozbb could take a look and/or weigh in before merging

[rozbb]

Thank you @apoelstra ! I took the liberty of simplifying the routine. We already have the Montgomery trick for batch field element inversion implemented, so I opted to just reuse that. Let me know if there's something I missed.

Re naming, it's actually mixed 😞 :

ed25519-dalek/src/batch.rs
136:pub fn verify_batch(

curve25519-dalek/src/ristretto.rs
553:    pub fn double_and_compress_batch<'a, I>(points: I) -> Vec<CompressedRistretto>

curve25519-dalek/src/edwards.rs
569:    pub fn batch_to_montgomery(eds: &[Self]) -> Vec<MontgomeryPoint> {
599:    pub fn compress_batch(inputs: &[EdwardsPoint]) -> Vec<CompressedEdwardsY> {

curve25519-dalek/src/scalar.rs
788:    pub fn batch_invert(inputs: &mut [Scalar]) -> Scalar {

That said, it's mostly *_batch, so I think to_montgomery_batch is appropriate. I'll go ahead and make that change.

I also added a reference in the function doc to point to to_montgomery, since it has specific behavior around identity points. Lmk if that looks alright.

If all is good, I'm happy to merge

[apoelstra]

@rozbb nice, thanks for the cleanup! I didn't see the existing field batch-inversion method when I wrote this.

I'm also happy with whatever names you all think are best. Happy to see this moving forward!