Add generate_random_point(), bump patch version. #148
Conversation
|
Can I ask you to move this function to |
|
Sure thing @survived, that is done 👍 |
|
Thanks for the PR! |
| let truncated = if bytes.len() > compressed_point_len - 1 { | ||
| &bytes[0..compressed_point_len - 1] | ||
| } else { | ||
| &bytes | ||
| }; |
There was a problem hiding this comment.
Maybe force bytes to be 32 bytes?(use an array) otherwise there can be collisions, say generate_random_point(&[1u8; 32]) == generate_random_point(&[1u8; 40])
There was a problem hiding this comment.
Good point, but we'll have to update all the places it's being called from. Actually, I checked — all the times it called in bulletproof crate, 64 bytes are passed 😄. But that can be relatively easily fixed
| use crate::{arithmetic::traits::*, BigInt}; | ||
| use generic_array::{typenum::Unsigned, GenericArray}; | ||
|
|
||
| pub fn generate_random_point(bytes: &[u8]) -> Point<Secp256k1> { |
There was a problem hiding this comment.
I'm not sure what's "random" here?
If what is the use-case here? if it's to create NUMS points then lets do that instead?(e.g keep hashing the generator to generate more and more points, or even hash some random data if you need to make sure it is a unique NUMS point)
There was a problem hiding this comment.
@elichai, the function is just copypasted from bulletproofs/centipede repos. It's purpose is to take uniformly distributed bytes and produce a point with unknown logarithm.
The reason why the function is appeared here — it's being used in two different crates (bulletproofs/centipede), but they just copypast it. So this PR basically removes code duplication in these crates.
My intention was to move that function here for now without modifications, and then replace with some proper generic hash-to-curve implementation (e.g. this one)
There was a problem hiding this comment.
Okay, we can merge it as is and change later
| let bn = BigInt::from_bytes(bytes); | ||
| let two = BigInt::from(2); | ||
| let bn_times_two = BigInt::mod_mul(&bn, &two, Scalar::<Secp256k1>::group_order()); | ||
| let bytes = BigInt::to_bytes(&bn_times_two); | ||
| generate_random_point(&bytes) |
There was a problem hiding this comment.
Is there a specific reason why multiply by 2 and reduce mod p and not hash it or something? (I'm not sure that's better but I'm curious what do you think)
There was a problem hiding this comment.
Perhaps @omershlo can answer this?
There was a problem hiding this comment.
Anyway, as I said above, I'd like to replace this function with something more generic and well-researched in near future, so it's supposed to be a temporary solution
|
@survived, I was hoping to get a patch version published at Whats the best way to proceed? |
|
@tmpfs I'm releasing v0.8.3 with your patch right now, but I'd like to update all crates to use v0.9. No changes are required (besides switching on curv's |
No description provided.