-
Notifications
You must be signed in to change notification settings - Fork 20
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
Comments
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. |
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.
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):
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. |
well stated. I agree with your rationale and approach. +1. |
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 standardhash_g2_with_dst
Two downsides to this change are
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 allunsafe
in blst.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
tohash_g2_nonstandard
or_legacy
or something like that? I personally feel we should remove the non-standardhash_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).The text was updated successfully, but these errors were encountered: