Skip to content

Commit 9d2449a

Browse files
committed
Fix overflow in historical scoring model point count summation
In adb0afc we started raising bucket weights to the power four in the historical model. This improved our model's accuracy greatly, but resulted in a much larger `total_valid_points_tracked`. In the same commit we converted `total_valid_points_tracked` to a float, but retained the 64-bit integer math to build it out of integer bucket values. Sadly, 64 bits are not enough to sum 1024 bucket pairs of 16-bit integers multiplied together and then squared (we need 16*4 + 10 = 74 bits to avoid overflow). Thus, here we replace the summation with 128-bit integers. Fairly straightforward merge conflicts (code added in 311a083 which was not included neighbored new code added plus references to new methods) fixed in: * lightning/src/routing/scoring.rs
1 parent fa8d599 commit 9d2449a

File tree

1 file changed

+27
-8
lines changed

1 file changed

+27
-8
lines changed

lightning/src/routing/scoring.rs

+27-8
Original file line numberDiff line numberDiff line change
@@ -1863,15 +1863,17 @@ mod bucketed_history {
18631863
}
18641864

18651865
fn recalculate_valid_point_count(&mut self) {
1866-
let mut total_valid_points_tracked = 0;
1866+
let mut total_valid_points_tracked = 0u128;
18671867
for (min_idx, min_bucket) in self.min_liquidity_offset_history.buckets.iter().enumerate() {
18681868
for max_bucket in self.max_liquidity_offset_history.buckets.iter().take(32 - min_idx) {
18691869
// In testing, raising the weights of buckets to a high power led to better
18701870
// scoring results. Thus, we raise the bucket weights to the 4th power here (by
1871-
// squaring the result of multiplying the weights).
1871+
// squaring the result of multiplying the weights). This results in
1872+
// bucket_weight having at max 64 bits, which means we have to do our summation
1873+
// in 128-bit math.
18721874
let mut bucket_weight = (*min_bucket as u64) * (*max_bucket as u64);
18731875
bucket_weight *= bucket_weight;
1874-
total_valid_points_tracked += bucket_weight;
1876+
total_valid_points_tracked += bucket_weight as u128;
18751877
}
18761878
}
18771879
self.total_valid_points_tracked = total_valid_points_tracked as f64;
@@ -1957,12 +1959,12 @@ mod bucketed_history {
19571959

19581960
let total_valid_points_tracked = self.tracker.total_valid_points_tracked;
19591961
#[cfg(debug_assertions)] {
1960-
let mut actual_valid_points_tracked = 0;
1962+
let mut actual_valid_points_tracked = 0u128;
19611963
for (min_idx, min_bucket) in min_liquidity_offset_history_buckets.iter().enumerate() {
19621964
for max_bucket in max_liquidity_offset_history_buckets.iter().take(32 - min_idx) {
19631965
let mut bucket_weight = (*min_bucket as u64) * (*max_bucket as u64);
19641966
bucket_weight *= bucket_weight;
1965-
actual_valid_points_tracked += bucket_weight;
1967+
actual_valid_points_tracked += bucket_weight as u128;
19661968
}
19671969
}
19681970
assert_eq!(total_valid_points_tracked, actual_valid_points_tracked as f64);
@@ -1989,7 +1991,7 @@ mod bucketed_history {
19891991
// max-bucket with at least BUCKET_FIXED_POINT_ONE.
19901992
let mut highest_max_bucket_with_points = 0;
19911993
let mut highest_max_bucket_with_full_points = None;
1992-
let mut total_weight = 0;
1994+
let mut total_weight = 0u128;
19931995
for (max_idx, max_bucket) in max_liquidity_offset_history_buckets.iter().enumerate() {
19941996
if *max_bucket >= BUCKET_FIXED_POINT_ONE {
19951997
highest_max_bucket_with_full_points = Some(cmp::max(highest_max_bucket_with_full_points.unwrap_or(0), max_idx));
@@ -2002,7 +2004,7 @@ mod bucketed_history {
20022004
// squaring the result of multiplying the weights), matching the logic in
20032005
// `recalculate_valid_point_count`.
20042006
let bucket_weight = (*max_bucket as u64) * (min_liquidity_offset_history_buckets[0] as u64);
2005-
total_weight += bucket_weight * bucket_weight;
2007+
total_weight += (bucket_weight * bucket_weight) as u128;
20062008
}
20072009
debug_assert!(total_weight as f64 <= total_valid_points_tracked);
20082010
// Use the highest max-bucket with at least BUCKET_FIXED_POINT_ONE, but if none is
@@ -2055,7 +2057,7 @@ mod bucketed_history {
20552057

20562058
#[cfg(test)]
20572059
mod tests {
2058-
use super::HistoricalBucketRangeTracker;
2060+
use super::{HistoricalBucketRangeTracker, HistoricalLiquidityTracker, ProbabilisticScoringFeeParameters};
20592061

20602062
#[test]
20612063
fn historical_liquidity_bucket_decay() {
@@ -2078,6 +2080,23 @@ mod bucketed_history {
20782080
]
20792081
);
20802082
}
2083+
2084+
#[test]
2085+
fn historical_heavy_buckets_operations() {
2086+
// Checks that we don't hit overflows when working with tons of data (even an
2087+
// impossible-to-reach amount of data).
2088+
let mut tracker = HistoricalLiquidityTracker::new();
2089+
tracker.min_liquidity_offset_history.buckets = [0xffff; 32];
2090+
tracker.max_liquidity_offset_history.buckets = [0xffff; 32];
2091+
tracker.recalculate_valid_point_count();
2092+
2093+
let mut directed = tracker.as_directed_mut(true);
2094+
let default_params = ProbabilisticScoringFeeParameters::default();
2095+
directed.calculate_success_probability_times_billion(&default_params, 42, 1000);
2096+
directed.track_datapoint(42, 52, 1000);
2097+
2098+
tracker.decay_buckets(1.0);
2099+
}
20812100
}
20822101
}
20832102
use bucketed_history::{LegacyHistoricalBucketRangeTracker, HistoricalBucketRangeTracker, DirectedHistoricalLiquidityTracker, HistoricalLiquidityTracker};

0 commit comments

Comments
 (0)