-
Notifications
You must be signed in to change notification settings - Fork 31
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
test(VDF): add bench tests for VDF in rsa group #42
base: master
Are you sure you want to change the base?
Conversation
is this PR ready to be merged? |
@omershlo Yes I think so! Would you mind taking a look again? |
benches/vdf/rsa_group.rs
Outdated
for _ in 0..t { | ||
r2 = r.clone() * two.clone(); | ||
b = r2.clone().div_rem_floor(l.clone()).0; | ||
r = r2.clone().div_rem_floor(l.clone()).1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to run r2.clone().div_rem_floor(l.clone())
only once. (instead of twice)
benches/vdf/rsa_group.rs
Outdated
// inverse, to get enough security bits | ||
let inverse = match hashed.invert(&modulus.clone()) { | ||
Ok(inverse) => inverse, | ||
Err(unchanged) => unchanged, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually not needed in this function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the same trick as #42 (comment) here?
benches/vdf/rsa_group.rs
Outdated
hasher.update(seed.to_digits::<u8>(Order::Lsf)); | ||
let hashed = Integer::from_digits(&hasher.finalize(), Order::Lsf); | ||
|
||
// inverse, to get enough security bits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I double checked and we can actually cannot do the inverse trick as it do not provide a random element in the group.
Instead you should concat results of nine sha256 and take the result modulo N
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I run recursive hashing only on the imtermediate hashing result itself, then at the end use them to reconstruct the randomness?
Or should I hash->construct->hash->construct repeatedly?
The latter seems can offer more entropy?
i.e., should I
let mut part = h_g_inner(seed);
let mut result = part.clone();
let mut ent = HASH_ENT;
while ent < GROUP_ENT {
part = h_g_inner(&part);
result = (result << HASH_ENT) + part.clone();
ent += HASH_ENT;
}
result.div_rem_floor(modulus.clone()).1
or
let mut result = h_g_inner(seed);
let mut ent = HASH_ENT;
while ent < GROUP_ENT {
result = h_g_inner(&result);
result = (result.clone() << HASH_ENT) + result.clone();
ent += HASH_ENT;
}
result.div_rem_floor(modulus.clone()).1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also come up with another idea.
We can actually
sha256("part1"||seed) || sha256("part2"||seed) || ... || sha256("part8"||seed)
i.e., we keep using the same seed for a part of the input, but introduces "partXXX" to provide different randomness.
We then concat then all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all of the methods above works and we should take the fastest
4bfe5f5
to
5f440ce
Compare
BTW, h_g_inner now converts between bytes and Integer just for better demostrating the logic. This is indeed tedious and unnecessay. We can actually run hashing and concat only using bytes, and convert it to Integer at the end. I will refactor it in this way once we decide which hash_and_concat way (we mentioned 3 ways above) we want to use. |
let me know when ready to merge |
updated bechmark results:
|
This PR adds an exmaple of benchmarking VDF in rsa_group (using RSA-2048 modulus).
environment
how to test it
test result