Skip to content

Commit 43d0964

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.
1 parent c9a7bfe commit 43d0964

File tree

1 file changed

+29
-7
lines changed

1 file changed

+29
-7
lines changed

lightning/src/routing/scoring.rs

+29-7
Original file line numberDiff line numberDiff line change
@@ -2060,15 +2060,17 @@ mod bucketed_history {
20602060
}
20612061

20622062
fn recalculate_valid_point_count(&mut self) {
2063-
let mut total_valid_points_tracked = 0;
2063+
let mut total_valid_points_tracked = 0u128;
20642064
for (min_idx, min_bucket) in self.min_liquidity_offset_history.buckets.iter().enumerate() {
20652065
for max_bucket in self.max_liquidity_offset_history.buckets.iter().take(32 - min_idx) {
20662066
// In testing, raising the weights of buckets to a high power led to better
20672067
// scoring results. Thus, we raise the bucket weights to the 4th power here (by
2068-
// squaring the result of multiplying the weights).
2068+
// squaring the result of multiplying the weights). This results in
2069+
// bucket_weight having at max 64 bits, which means we have to do our summation
2070+
// in 128-bit math.
20692071
let mut bucket_weight = (*min_bucket as u64) * (*max_bucket as u64);
20702072
bucket_weight *= bucket_weight;
2071-
total_valid_points_tracked += bucket_weight;
2073+
total_valid_points_tracked += bucket_weight as u128;
20722074
}
20732075
}
20742076
self.total_valid_points_tracked = total_valid_points_tracked as f64;
@@ -2161,12 +2163,12 @@ mod bucketed_history {
21612163

21622164
let total_valid_points_tracked = self.tracker.total_valid_points_tracked;
21632165
#[cfg(debug_assertions)] {
2164-
let mut actual_valid_points_tracked = 0;
2166+
let mut actual_valid_points_tracked = 0u128;
21652167
for (min_idx, min_bucket) in min_liquidity_offset_history_buckets.iter().enumerate() {
21662168
for max_bucket in max_liquidity_offset_history_buckets.iter().take(32 - min_idx) {
21672169
let mut bucket_weight = (*min_bucket as u64) * (*max_bucket as u64);
21682170
bucket_weight *= bucket_weight;
2169-
actual_valid_points_tracked += bucket_weight;
2171+
actual_valid_points_tracked += bucket_weight as u128;
21702172
}
21712173
}
21722174
assert_eq!(total_valid_points_tracked, actual_valid_points_tracked as f64);
@@ -2193,7 +2195,7 @@ mod bucketed_history {
21932195
// max-bucket with at least BUCKET_FIXED_POINT_ONE.
21942196
let mut highest_max_bucket_with_points = 0;
21952197
let mut highest_max_bucket_with_full_points = None;
2196-
let mut total_weight = 0;
2198+
let mut total_weight = 0u128;
21972199
for (max_idx, max_bucket) in max_liquidity_offset_history_buckets.iter().enumerate() {
21982200
if *max_bucket >= BUCKET_FIXED_POINT_ONE {
21992201
highest_max_bucket_with_full_points = Some(cmp::max(highest_max_bucket_with_full_points.unwrap_or(0), max_idx));
@@ -2206,7 +2208,7 @@ mod bucketed_history {
22062208
// squaring the result of multiplying the weights), matching the logic in
22072209
// `recalculate_valid_point_count`.
22082210
let bucket_weight = (*max_bucket as u64) * (min_liquidity_offset_history_buckets[0] as u64);
2209-
total_weight += bucket_weight * bucket_weight;
2211+
total_weight += (bucket_weight * bucket_weight) as u128;
22102212
}
22112213
debug_assert!(total_weight as f64 <= total_valid_points_tracked);
22122214
// Use the highest max-bucket with at least BUCKET_FIXED_POINT_ONE, but if none is
@@ -2343,6 +2345,26 @@ mod bucketed_history {
23432345

23442346
assert_ne!(probability1, probability);
23452347
}
2348+
2349+
#[test]
2350+
fn historical_heavy_buckets_operations() {
2351+
// Checks that we don't hit overflows when working with tons of data (even an
2352+
// impossible-to-reach amount of data).
2353+
let mut tracker = HistoricalLiquidityTracker::new();
2354+
tracker.min_liquidity_offset_history.buckets = [0xffff; 32];
2355+
tracker.max_liquidity_offset_history.buckets = [0xffff; 32];
2356+
tracker.recalculate_valid_point_count();
2357+
tracker.merge(&tracker.clone());
2358+
assert_eq!(tracker.min_liquidity_offset_history.buckets, [0xffff; 32]);
2359+
assert_eq!(tracker.max_liquidity_offset_history.buckets, [0xffff; 32]);
2360+
2361+
let mut directed = tracker.as_directed_mut(true);
2362+
let default_params = ProbabilisticScoringFeeParameters::default();
2363+
directed.calculate_success_probability_times_billion(&default_params, 42, 1000);
2364+
directed.track_datapoint(42, 52, 1000);
2365+
2366+
tracker.decay_buckets(1.0);
2367+
}
23462368
}
23472369
}
23482370

0 commit comments

Comments
 (0)