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

Two different versions of hash_g2 being used #26

Open
iancoleman opened this issue Aug 26, 2021 · 3 comments
Open

Two different versions of hash_g2 being used #26

iancoleman opened this issue Aug 26, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@iancoleman
Copy link
Contributor

There are two different hash_g2 being used. This issue is to track whether we should move to just a single one.

The first one is used for encrypt and decrypt, which is hash_g2 in lib.rs from when we forked threshold_crypto. The team at poanetwork/threshold_crypto wrote this version of hash_g2.

As the code comment says, this Returns a hash of the given message in G2.

It's a kind of preprocessing to convert any arbitrary bytes (eg a message to sign or encrypt) into a G2, which can then be used for signing/verifying or encrypting/decrypting.

The second one is used for sign and verify, which is within blst (contained within the call to blst_sk.sign and blst_sig.verify).

The simplest way to see this second version of hash_g2 is in blsbs which has a util function wrapping it all up pretty neatly: hash_g2_with_dst. Note this requires use of unsafe.

These two different ways of converting arbitrary bytes to G2 are incompatible (same input bytes produces a different output G2). Maybe we should consolidate.

The blst version follows an Internet Research Task Force standard, see blst/bindings/vectors/hash_to_curve and draft-irtf-cfrg-hash-to-curve. The threshold_crypto/blsttc version does not.

There's also an irtf/blst compatible implementation of hash_g2 in the javascript library noble-bls12-381, see hashToCurve. Sometimes the javascript can be easier to understand than other lower level languages and is a good alternative to know about.

We could potentially keep encrypt/decrypt using the non-standard hash_g2 since no other library implements bls encrypt/decrypt, all other libs only implement sign/verify (which we are compatible with because we use blst for those operations). This means we're free to do what we like with encrypt/decrypt, nobody is following us and there is nobody else to follow.

But it seems to make sense to me that we follow standards as best we can and should avoid doubling up with two incompatible implementations.

So I propose we remove the non-standard hash_g2 and only use the irtf standard hash_g2_with_dst

Two downsides to this change are

  1. the blst version requires us to use unsafe in blsttc. We may be able to avoid this by adding it as a helper method to blst itself, like uniq. This keeps all unsafe in blst.

  2. this is a breaking change so any old ciphertexts won't decrypt with the new hash_g2. Should we rename the non-standard hash_g2 to hash_g2_nonstandard or _legacy or something like that? I personally feel we should remove the non-standard hash_g2 since we have no other users of blsttc (to my knowledge, see stats chart at the bottom of crates.io/blsttc and dependents which are all maidsafe).

@iancoleman iancoleman added the enhancement New feature or request label Aug 26, 2021
@dan-da
Copy link
Contributor

dan-da commented Aug 26, 2021

So I propose we remove the non-standard hash_g2 and only use the irtf standard hash_g2_with_dst

this would make us incompatible with threshold_crypto for encrypt/decrypt, would it not?

Perhaps another approach (though slower) would be to enter into dialogue with threshold_crypto (and maybe blst) about using a common impl/standard. It seems best if threshold_crypto could be pursuaded to use hash_g2_with_dst.

edit: or I guess we could just make the change and let them know we did it, so they can follow suit if they wish.

@iancoleman
Copy link
Contributor Author

this would make us incompatible with threshold_crypto for encrypt/decrypt, would it not?

Yes it will. But we already are incompatible for sign and verify. If I sign a message with blsttc it won't verify with threshold_crypto (and vice versa). I prefer our crate to be internally consistent and follow the irtf standard for hash_g2, even if it means we lose compatibility with non-standard threshold_crypto.

To put it another way, all {py,js,go...} bls libs will have the blst hash_g2. No {py,js,go...} bls libs will have the threshold_crypto hash_g2. So I reckon we should only use the blst hash_g2 and remove the threshold_crypto hash_g2.

enter into dialogue with threshold_crypto (and maybe blst) about using a common impl/standard

blst already uses the standard (irtf). threshold_crypto will not move to the standard, see poanetwork/threshold_crypto#110 (comment)

To my mind, threshold_crypto crate is now a is legacy / unmaintained crate and blsttc is the modern and maintained fork of it. Currently the only other consumer of threshold_crypto is solana (see dependents), so all other projects that I'm aware of using bls are using the standard irtf hash_g2.

Two possible ways to progress (if we want to):

  1. update our hash_g2 to use the existing blst version the way blsbs does (can be done now, but we have to use unsafe in that function in our crate)

  2. submit a pr to blst making hash_g2 public, then update our hash_g2 to use that (will take time, but keeps unsafe all in blst)

My preference is the first one, and also do the second one at the same time. If the second option bears fruit we can replace the unsafe code in our crate later.

@dan-da
Copy link
Contributor

dan-da commented Aug 28, 2021

well stated. I agree with your rationale and approach. +1.

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

No branches or pull requests

2 participants