-
Notifications
You must be signed in to change notification settings - Fork 990
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
Fix reading POW for edge bits 59 and higher #3069
Conversation
It requires reading more than 8 bytes
@@ -391,21 +391,39 @@ impl Proof { | |||
} | |||
} | |||
|
|||
#[inline(always)] | |||
fn extract_bits(bits: &Vec<u8>, bit_start: usize, bit_count: usize, read_from: usize) -> u64 { | |||
let mut buf: [u8; 8] = [0; 8]; |
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.
Is it possible in rust to directly assign
let bits64 = u64::from_le_bytes(&bits[read_from..read_from + 8]);
?
if so, that would be much preferable over the extra buffer and copying....
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.
Other than that, this PR looks fine to me.
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.
Array and slice are different types in Rust, so we can't directly use bits
in from_le_bytes
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.
and there's no function or method we can call on the slice to convert to array?
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.
btw, remaining comments are just about aesthetics (can always be reconsidered later) and should not hold up this PR...
let mut v = Vec::with_capacity(42); | ||
for _ in 0..42 { | ||
v.push(rng.gen_range( | ||
u64::pow(2, bits - 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.
btw, isn't it easier to write 1 << (bits - 1) than u64::pow(2, bits - 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 think it's easier to read u64::pow(2, bits - 1), so personally prefer it iff there's no performance hit.
It requires reading more than 8 bytes