Unecessary Vec in HashToField

[burdges]

There is an unnecessary Vec in HashToField:

    fn hash_to_field(&self, msg: &[u8], count: usize) -> Vec<F>;

This is a minor issue in the IRTF hash-to-curve spec, which extracts all fields elements at once. A hasher should not usually allocate.

If one uses rust-lang/rust#96097 then one correct formulation is

/// Trait for hashing messages to field elements.
pub unsafe trait HashToField<F: Field>: Sized {
    /// Initialises a new hash-to-field helper struct.
    ///
    /// # Arguments
    ///
    /// * `domain` - bytes that get concatenated with the `msg` during hashing, in order to separate potentially interfering instantiations of the hasher.
    fn new(domain: &[u8]) -> Self;

    /// Hash an arbitrary `msg` to fill the supplied buffer of field elements in `F`.
    ///  
    /// It is undefined behavior to not fill the buffer fully.
    fn hash_to_field(&self, msg: &[u8], out: &mut [MaybeUninit<F>]);

    /// Hash an arbitrary `msg` to #`N` elements from field `F`.
    fn hash_to_field<const N; usize>(&self, msg: &[u8]) -> [F; N] {
        let mut fs: [MaybeUninit<F>; N] = MaybeUninit::uninit_array();
        self.hash_to_field(msg, &mut fs);
        unsafe { MaybeUninit::array_assume_init(fs) }
    }
}

We could skip the unsafe bits initially by supplying only fn hash_to_field<const N; usize>(&self, msg: &[u8]) -> [F; N] as the hash-to-curve spec itself uses only N=1 or N=2.

[Pratyush]

This is fine by me, but let's avoid unsafe.

[burdges]

Around this, there are issues with the hash-to-curve traits overall, like being SNARK unfriendly. I'd suggest they be replaced with pure output traits.

HashToField becomes

pub trait HashToField<F: Field>: Sized {
    fn hash_output_to_field<const N: usize>(self) -> [F; N];
}

HashToCurve becomes

pub trait HashToCurve<T: CurveGroup>: Sized {
    fn hash_output_to_curve(self) -> Result<T::Affine, HashToCurveError>;
}

It's annoying MapToCurve has a new method too. I think polymorphic constants wind up annoying, but at least here we might bump this MapToCurve::new method to an inherent const fn on the associated config extension trait, so then it's only accessed from the one remaining methods here.

How do you add data then? digest::Update maybe? Or use merlin::Transcript? Or something snark friendly?

I'm unsure their receivers should work here. Although odd, I think self helps accidental avoid reuse. As an example, merlin could be supported by a wrapper type with optional &mut borrowing like:

pub struct ChallengeToCurve<B> {
    pub label: &'static [u8],
    pub t: B,
}

impl<B,F> HashToField<F> for ChallengeToCurve<B> 
where B: BorrowMut<merlin::Transcript>, ...

impl<B,C> HashToCurve<C> for ChallengeToCurve<B> ...
where B: BorrowMut<merlin::Transcript>, ...

As for the draft standard's cipher choices, we'd likely require two adaptors, an extender shared by Sha256, Sha384, and Sha512, and generic XoF ones for Shake256 like:

impl<'a, H: digest::XofReader, F> HashToField<F> for &'a mut H ...

impl<'a, H: digest::XofReader, C> HashToCurve<C> for &'a mut H ...

[burdges]

It's maybe subtle to do these output oriented things in hash-to-field, given the standard keeping sha2 and the domain worries, but overall the code is not structured well. I raised an issue on the source repo armfazh/h2c-rust-ref#8 but anyways but maybe a reference implementation won't care about these improprieties.

Also, arkworks does not really expose the right constants to do avoid Vecs, not even sure if const generics are really there yet either, so maybe not worth it right now.

[burdges]

Superseded by #630