ExpandMsg improvements

[daxpedda]

Follow-up for #400.

• Prevent overflows and add zero check.

[daxpedda]

So should we make hash_from_bytes and encode_from_bytes infallible?
#871 (comment)

[daxpedda]

Some outstanding issues:

• Updating to digest 0.10.
• Make hash_to_field publicly accessible.

We can merge this and make separate PR's for each or handle it here. Are there any other unresolved questions @tarcieri@mikelodder7?

[tarcieri]

Updating to digest 0.10.

This is going to be a pretty big effort and needs to happen in a coordinated way across not only elliptic-curve but also ecdsa and k256/p256. It will require a major version bump of all of these crates and I'm still working on getting all of the downstream dependencies of these crates I maintain updated to the latest releases.

I did the first step of cutting a new signature release which bumps to digest v0.10 but updating everything else is going to be a lot of work.

[daxpedda]

That's alright, we will do this later then. The remaining issue is hash_to_field then. Any suggestions on what to do? I was actually thinking of adding it to GroupDigest and letting it return a Scalar, which should solve the problem.

[tarcieri]

Isn't hash_to_field primarily intended to hash an input message to elements of the base field?

Are there valid use cases (interop?) for hash-to-scalar that make it a better choice than the existing Reduce trait?

[daxpedda]

Implementing VOPRF requires calling hash_to_field:
https://www.ietf.org/archive/id/draft-irtf-cfrg-voprf-08.html#section-4.3-1.1.2.2

[tarcieri]

Okay, well if it's needed for interop I'd be fine with a public hash_to_scalar function, but I think it should be called that and not hash_to_field, to avoid confusion with the base field

[tarcieri]

Also VOPRF seems like the sort of thing that probably belongs in the respective RustCrypto/elliptic-curves crates?

[daxpedda]

... but I think it should be called that and not hash_to_field, to avoid confusion with the base field

Sounds good to me.

Also VOPRF seems like the sort of thing that probably belongs in the respective RustCrypto/elliptic-curves crates?

Yes, we just need a trait we can be generic over.

[mikelodder7]

hash_to_scalar would be just a convenience wrapper for Digest output to 64 bytes then calling Reduce. I wouldn't usehash_to_field for this purpose since it does a more complex mixing of bytes than is necessary for that purpose.

[daxpedda]

Well I have to name it something 😄. Preferable I would like to stick with hash_to_field as this is what it's called in the spec, but I understand @tarcieri concern. I will just name it hash_to_scalar for now and let you guys hash it out 😉.

[mikelodder7]

Well I have to name it something 😄. Preferable I would like to stick with hash_to_field as this is what it's called in the spec, but I understand @tarcieri concern. I will just name it hash_to_scalar for now and let you guys hash it out 😉.

Dalek uses the name hash_from_bytes. That could work.

[tarcieri]

hash_to_scalar would be just a convenience wrapper for Digest output to 64 bytes then calling Reduce. I wouldn't usehash_to_field for this purpose since it does a more complex mixing of bytes than is necessary for that purpose.

And that's what's already provided by these methods on Reduce (added in #869):

[daxpedda]

Maybe it would be better to actually codify that comment?

I left the comment to explain why this can't fail nonetheless.

Dalek uses the name hash_from_bytes. That could work.

I agree with @tarcieri, it's not the same, and already provided.

[mikelodder7]

Yep that's just fine the way it is

[tarcieri]

Looks good now!

@mikelodder7 WDYT?

[daxpedda]

I'm a bit busy with something else right now, still adding the other test vectors. But feel free to merge if you like to.

[tarcieri]

Not in a hurry. Feel free to push some more changes.

[mikelodder7]

Looks good now!

@mikelodder7 WDYT?

I think it works for now once all the checks are cleared. Some minor nits that I can make after this merge.

[daxpedda]

This is good to go.

It seems I am out of my depth when it comes to how to implement hash_to_scalar, any guidance is welcome, in the meantime I will do more research.

[tarcieri]

Looks good!