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

Derive additional traits for CompressedRistretto / CompressedEdwardsY #221

Closed
hdevalence opened this issue Dec 27, 2018 · 2 comments
Closed

Comments

@hdevalence
Copy link
Contributor

Branching off of #220, specifically 3e91ba0 , where @burdges suggested adding

#[derive(Ord, PartialOrd, Hash)]

to CompressedRistretto. This allows using a CompressedRistretto as the key to a BTreeMap, which requires Ord (hence PartialOrd), and with HashMap, which requires Hash.

This seems like a useful use-case, for instance looking up information by a public key.

Some questions that would be good to cross off answers to:

  • Does it make sense to derive Ord for encodings of points? Unlike scalars, there's no meaningful ordering on points, so we could only do something like ordering the bytestrings.

  • Does it make sense to do this also for CompressedEdwardsY as well as CompressedRistretto? In particular, is there any potential for ambiguity/non-uniqueness?

  • Is there a problem with using the derived implementations? For instance, if we derive Hash, will it do something like hash one byte at a time?

  • What's the stability impact of adding new derives? I think this is probably fine (it's just adding new traits) but we should check.

@burdges
Copy link
Contributor

burdges commented Dec 28, 2018

I never suggested this for CompressedEdwardsY out of anti-cofactor prejudice, but actually the cofactor could be managed reasonably for at least multi-signatures, so sure whatever.

I think these derives produce very thin wrappers, so the question boils down to [u8;32]:

In this case, impl Hash for [T; 32] delegates to impl Hash for [T] which uses <T as Hash>::hash_slice, for which u8 does the right thing.

Also impl PartialOrd for [T; 32] delegates to impl PartialOrd for [T], which used SlicePartialOrd and SliceOrd

@hdevalence hdevalence added this to the 1.1 milestone Dec 29, 2018
@hdevalence hdevalence modified the milestones: 1.1, 1.2 Jan 20, 2019
@hdevalence hdevalence modified the milestones: 1.2, 1.3 Jun 4, 2019
@hdevalence
Copy link
Contributor Author

Closed by #318.

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

No branches or pull requests

3 participants