Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SWUMap and WBMap, fix MapToCurve semantics #629

Closed
burdges opened this issue Mar 22, 2023 · 2 comments · May be fixed by #643
Closed

Remove SWUMap and WBMap, fix MapToCurve semantics #629

burdges opened this issue Mar 22, 2023 · 2 comments · May be fixed by #643

Comments

@burdges
Copy link
Contributor

burdges commented Mar 22, 2023

Afaik, there is no reason for WB hash-to-curve when SWU hash-to-curve works, right? Also, There is only one suite listed for each curve in https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-hash-to-curve-16#name-suites-for-hashing As an aside, they refer to both WB and SWU as SSWU.

There are reasons to expose the underlying map-to-curve as their hash-to-field favors expand_message_xmd for sha256 when other likely prefer expand_message_xof for blake2x, shake, merlin, etc. and even PRNGs

It seemingly follows we should remove the SWUMap and WBMap types, replace their MapToCurve methods by free methods, like

pub fn map_to_curve<C>(point: P::BaseField) -> Result<C, HashToCurveError> 
where
    C: CurveGroup,
    <C as CurveGroup>::Config: SWUConfig,
{ ... }

and

pub fn map_to_curve<C>(point: P::BaseField) -> Result<C, HashToCurveError> 
where
    C: CurveGroup,
    <C as CurveGroup>::Config: WBConfig,
{ ... }

We finally replace the MapToCurve trait by

pub trait MapToCurve: CurveGroup  {
    fn map_to_curve(<Self as CurveGroup>::BaseField) -> Self;
}

Individual curves then impl MapToCurve by invoking hashing::swu::map_to_curve or hashing::wb::map_to_curve as appropriate.

Afaik, we've no reason to output an affine point here like we currently do. In fact, we pointlessly normalize the point both before and after a cofactor multiplication right now.

@burdges
Copy link
Contributor Author

burdges commented Mar 22, 2023

In principle, we could merge MapToCurve into CurveGroup itself, but we should not define maps without some consensus, which maybe the sage script provides, or maybe not. Interestingly, polymorphic free fns can be monomorphized as associated constants of traits like https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=8e02c2aea90434583aca18e0bae73459 so you could've an optional method like

pub trait CurveGroup {
    ...
    const MAP_TO_CURVE: Option<fn(&Self::BaseField) -> Self>;
    ...
}

Ain't so clear if this plays nicely with const fn work though, so probably best doing a MapToCurve trait, or else have a const MAP_TO_CURVE_IS_STABLE : bool = false

@burdges
Copy link
Contributor Author

burdges commented Apr 25, 2023

I think the orphan rules obstruct many approaches here, so #643 leaves these types and traits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant