Fix log_int implementation, removing floats

The cast from f32::MAX to u32 was undefined behaviour, and the use of floating
point logarithms would yield incorrect results due to rounding and truncation,
e.g. for the integer 16777206
This commit is contained in:
Michael Sproul 2019-03-02 15:52:33 +11:00
parent 8671c5790c
commit 6795aa42b2
2 changed files with 15 additions and 15 deletions

View File

@ -8,7 +8,6 @@ edition = "2018"
db = { path = "../../beacon_node/db" } db = { path = "../../beacon_node/db" }
ssz = { path = "../utils/ssz" } ssz = { path = "../utils/ssz" }
types = { path = "../types" } types = { path = "../types" }
fast-math = "0.1.1"
log = "0.4.6" log = "0.4.6"
bit-vec = "0.5.0" bit-vec = "0.5.0"

View File

@ -1,5 +1,5 @@
//! The optimised bitwise LMD-GHOST fork choice rule.
extern crate bit_vec; extern crate bit_vec;
extern crate fast_math;
use crate::{ForkChoice, ForkChoiceError}; use crate::{ForkChoice, ForkChoiceError};
use bit_vec::BitVec; use bit_vec::BitVec;
@ -7,7 +7,6 @@ use db::{
stores::{BeaconBlockStore, BeaconStateStore}, stores::{BeaconBlockStore, BeaconStateStore},
ClientDB, ClientDB,
}; };
use fast_math::log2_raw;
use log::{debug, trace}; use log::{debug, trace};
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::Arc; use std::sync::Arc;
@ -19,22 +18,17 @@ use types::{
//TODO: Pruning - Children //TODO: Pruning - Children
//TODO: Handle Syncing //TODO: Handle Syncing
/// The optimised bitwise LMD-GHOST fork choice rule. // NOTE: This uses u32 to represent difference between block heights. Thus this is only
/// NOTE: This uses u32 to represent difference between block heights. Thus this is only // applicable for block height differences in the range of a u32.
/// applicable for block height differences in the range of a u32. // This can potentially be parallelized in some parts.
/// This can potentially be parallelized in some parts.
// we use fast log2, a log2 lookup table is implemented in Vitaliks code, potentially do /// Compute the base-2 logarithm of an integer, floored (rounded down)
// the comparison. Log2_raw takes 2ns according to the documentation.
#[inline] #[inline]
fn log2_int(x: u32) -> u32 { fn log2_int(x: u32) -> u32 {
if x == 0 { if x == 0 {
return 0; return 0;
} }
assert!( 31 - x.leading_zeros()
x <= std::f32::MAX as u32,
"Height too large for fast log in bitwise fork choice"
);
log2_raw(x as f32) as u32
} }
fn power_of_2_below(x: u32) -> u32 { fn power_of_2_below(x: u32) -> u32 {
@ -469,7 +463,6 @@ mod tests {
#[test] #[test]
pub fn test_power_of_2_below() { pub fn test_power_of_2_below() {
println!("{:?}", std::f32::MAX);
assert_eq!(power_of_2_below(4), 4); assert_eq!(power_of_2_below(4), 4);
assert_eq!(power_of_2_below(5), 4); assert_eq!(power_of_2_below(5), 4);
assert_eq!(power_of_2_below(7), 4); assert_eq!(power_of_2_below(7), 4);
@ -478,4 +471,12 @@ mod tests {
assert_eq!(power_of_2_below(33), 32); assert_eq!(power_of_2_below(33), 32);
assert_eq!(power_of_2_below(63), 32); assert_eq!(power_of_2_below(63), 32);
} }
#[test]
pub fn test_power_of_2_below_large() {
let pow: u32 = 1 << 24;
for x in (pow - 20)..(pow + 20) {
assert!(power_of_2_below(x) <= x, "{}", x);
}
}
} }