Skip to content

Commit 18b4231

Browse files
TheBlueMattTheBlueMatt
committed
Drop half-life-based bucket decay in update_history_buckets
Because we decay the bucket information in the background, there's not much reason to try to decay them immediately prior to updating, and in removing that we can also clean up a good bit of dead code, which we do here.
1 parent 21facd0 commit 18b4231

File tree

1 file changed

+38
-98
lines changed

1 file changed

+38
-98
lines changed

‎lightning/src/routing/scoring.rs‎

Lines changed: 38 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ struct ChannelLiquidity {
789789
min_liquidity_offset_history: HistoricalBucketRangeTracker,
790790
max_liquidity_offset_history: HistoricalBucketRangeTracker,
791791

792-
/// Time when the liquidity bounds were last modified as an offset since the unix epoch.
792+
/// Time when either liquidity bound was last modified as an offset since the unix epoch.
793793
last_updated: Duration,
794794

795795
/// Time when the historical liquidity bounds were last modified as an offset against the unix
@@ -805,7 +805,6 @@ struct DirectedChannelLiquidity<L: Deref<Target = u64>, BRT: Deref<Target = Hist
805805
capacity_msat: u64,
806806
last_updated: T,
807807
offset_history_last_updated: T,
808-
decay_params: ProbabilisticScoringDecayParameters,
809808
}
810809

811810
impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ProbabilisticScorer<G, L> where L::Target: Logger {
@@ -837,7 +836,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ProbabilisticScorer<G, L> whe
837836
let log_direction = |source, target| {
838837
if let Some((directed_info, _)) = chan_debug.as_directed_to(target) {
839838
let amt = directed_info.effective_capacity().as_msat();
840-
let dir_liq = liq.as_directed(source, target, amt, self.decay_params);
839+
let dir_liq = liq.as_directed(source, target, amt);
841840

842841
let min_buckets = &dir_liq.liquidity_history.min_liquidity_offset_history.buckets;
843842
let max_buckets = &dir_liq.liquidity_history.max_liquidity_offset_history.buckets;
@@ -889,7 +888,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ProbabilisticScorer<G, L> whe
889888
if let Some(liq) = self.channel_liquidities.get(&scid) {
890889
if let Some((directed_info, source)) = chan.as_directed_to(target) {
891890
let amt = directed_info.effective_capacity().as_msat();
892-
let dir_liq = liq.as_directed(source, target, amt, self.decay_params);
891+
let dir_liq = liq.as_directed(source, target, amt);
893892
return Some((dir_liq.min_liquidity_msat(), dir_liq.max_liquidity_msat()));
894893
}
895894
}
@@ -931,7 +930,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ProbabilisticScorer<G, L> whe
931930
if let Some(liq) = self.channel_liquidities.get(&scid) {
932931
if let Some((directed_info, source)) = chan.as_directed_to(target) {
933932
let amt = directed_info.effective_capacity().as_msat();
934-
let dir_liq = liq.as_directed(source, target, amt, self.decay_params);
933+
let dir_liq = liq.as_directed(source, target, amt);
935934

936935
let min_buckets = dir_liq.liquidity_history.min_liquidity_offset_history.buckets;
937936
let mut max_buckets = dir_liq.liquidity_history.max_liquidity_offset_history.buckets;
@@ -962,7 +961,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ProbabilisticScorer<G, L> whe
962961
if let Some(liq) = self.channel_liquidities.get(&scid) {
963962
if let Some((directed_info, source)) = chan.as_directed_to(target) {
964963
let capacity_msat = directed_info.effective_capacity().as_msat();
965-
let dir_liq = liq.as_directed(source, target, capacity_msat, self.decay_params);
964+
let dir_liq = liq.as_directed(source, target, capacity_msat);
966965

967966
return dir_liq.liquidity_history.calculate_success_probability_times_billion(
968967
&params, amount_msat, capacity_msat
@@ -989,7 +988,7 @@ impl ChannelLiquidity {
989988
/// Returns a view of the channel liquidity directed from `source` to `target` assuming
990989
/// `capacity_msat`.
991990
fn as_directed(
992-
&self, source: &NodeId, target: &NodeId, capacity_msat: u64, decay_params: ProbabilisticScoringDecayParameters
991+
&self, source: &NodeId, target: &NodeId, capacity_msat: u64,
993992
) -> DirectedChannelLiquidity<&u64, &HistoricalBucketRangeTracker, &Duration> {
994993
let (min_liquidity_offset_msat, max_liquidity_offset_msat, min_liquidity_offset_history, max_liquidity_offset_history) =
995994
if source < target {
@@ -1010,14 +1009,13 @@ impl ChannelLiquidity {
10101009
capacity_msat,
10111010
last_updated: &self.last_updated,
10121011
offset_history_last_updated: &self.offset_history_last_updated,
1013-
decay_params: decay_params,
10141012
}
10151013
}
10161014

10171015
/// Returns a mutable view of the channel liquidity directed from `source` to `target` assuming
10181016
/// `capacity_msat`.
10191017
fn as_directed_mut(
1020-
&mut self, source: &NodeId, target: &NodeId, capacity_msat: u64, decay_params: ProbabilisticScoringDecayParameters
1018+
&mut self, source: &NodeId, target: &NodeId, capacity_msat: u64,
10211019
) -> DirectedChannelLiquidity<&mut u64, &mut HistoricalBucketRangeTracker, &mut Duration> {
10221020
let (min_liquidity_offset_msat, max_liquidity_offset_msat, min_liquidity_offset_history, max_liquidity_offset_history) =
10231021
if source < target {
@@ -1038,7 +1036,6 @@ impl ChannelLiquidity {
10381036
capacity_msat,
10391037
last_updated: &mut self.last_updated,
10401038
offset_history_last_updated: &mut self.offset_history_last_updated,
1041-
decay_params: decay_params,
10421039
}
10431040
}
10441041

@@ -1289,14 +1286,6 @@ DirectedChannelLiquidity<L, BRT, T> {
12891286
/// state"), we allow the caller to set an offset applied to our liquidity bounds which
12901287
/// represents the amount of the successful payment we just made.
12911288
fn update_history_buckets(&mut self, bucket_offset_msat: u64, duration_since_epoch: Duration) {
1292-
let half_lives =
1293-
duration_since_epoch.checked_sub(*self.offset_history_last_updated)
1294-
.unwrap_or(Duration::ZERO).as_secs()
1295-
.checked_div(self.decay_params.historical_no_updates_half_life.as_secs())
1296-
.map(|v| v.try_into().unwrap_or(u32::max_value())).unwrap_or(u32::max_value());
1297-
self.liquidity_history.min_liquidity_offset_history.time_decay_data(half_lives);
1298-
self.liquidity_history.max_liquidity_offset_history.time_decay_data(half_lives);
1299-
13001289
self.liquidity_history.min_liquidity_offset_history.track_datapoint(
13011290
*self.min_liquidity_offset_msat + bucket_offset_msat, self.capacity_msat
13021291
);
@@ -1369,7 +1358,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ScoreLookUp for Probabilistic
13691358
self.channel_liquidities
13701359
.get(&scid)
13711360
.unwrap_or(&ChannelLiquidity::new(Duration::ZERO))
1372-
.as_directed(&source, &target, capacity_msat, self.decay_params)
1361+
.as_directed(&source, &target, capacity_msat)
13731362
.penalty_msat(amount_msat, score_params)
13741363
.saturating_add(anti_probing_penalty_msat)
13751364
.saturating_add(base_penalty_msat)
@@ -1399,14 +1388,14 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ScoreUpdate for Probabilistic
13991388
self.channel_liquidities
14001389
.entry(hop.short_channel_id)
14011390
.or_insert_with(|| ChannelLiquidity::new(duration_since_epoch))
1402-
.as_directed_mut(source, &target, capacity_msat, self.decay_params)
1391+
.as_directed_mut(source, &target, capacity_msat)
14031392
.failed_at_channel(amount_msat, duration_since_epoch,
14041393
format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
14051394
} else {
14061395
self.channel_liquidities
14071396
.entry(hop.short_channel_id)
14081397
.or_insert_with(|| ChannelLiquidity::new(duration_since_epoch))
1409-
.as_directed_mut(source, &target, capacity_msat, self.decay_params)
1398+
.as_directed_mut(source, &target, capacity_msat)
14101399
.failed_downstream(amount_msat, duration_since_epoch,
14111400
format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
14121401
}
@@ -1435,7 +1424,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ScoreUpdate for Probabilistic
14351424
self.channel_liquidities
14361425
.entry(hop.short_channel_id)
14371426
.or_insert_with(|| ChannelLiquidity::new(duration_since_epoch))
1438-
.as_directed_mut(source, &target, capacity_msat, self.decay_params)
1427+
.as_directed_mut(source, &target, capacity_msat)
14391428
.successful(amount_msat, duration_since_epoch,
14401429
format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
14411430
} else {
@@ -1959,14 +1948,6 @@ mod bucketed_history {
19591948
self.buckets[bucket] = self.buckets[bucket].saturating_add(BUCKET_FIXED_POINT_ONE);
19601949
}
19611950
}
1962-
/// Decay all buckets by the given number of half-lives. Used to more aggressively remove old
1963-
/// datapoints as we receive newer information.
1964-
#[inline]
1965-
pub(super) fn time_decay_data(&mut self, half_lives: u32) {
1966-
for e in self.buckets.iter_mut() {
1967-
*e = e.checked_shr(half_lives).unwrap_or(0);
1968-
}
1969-
}
19701951
}
19711952

19721953
impl_writeable_tlv_based!(HistoricalBucketRangeTracker, { (0, buckets, required) });
@@ -2359,52 +2340,52 @@ mod tests {
23592340
// Update minimum liquidity.
23602341

23612342
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2362-
.as_directed(&source, &target, 1_000, decay_params);
2343+
.as_directed(&source, &target, 1_000);
23632344
assert_eq!(liquidity.min_liquidity_msat(), 100);
23642345
assert_eq!(liquidity.max_liquidity_msat(), 300);
23652346

23662347
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2367-
.as_directed(&target, &source, 1_000, decay_params);
2348+
.as_directed(&target, &source, 1_000);
23682349
assert_eq!(liquidity.min_liquidity_msat(), 700);
23692350
assert_eq!(liquidity.max_liquidity_msat(), 900);
23702351

23712352
scorer.channel_liquidities.get_mut(&42).unwrap()
2372-
.as_directed_mut(&source, &target, 1_000, decay_params)
2353+
.as_directed_mut(&source, &target, 1_000)
23732354
.set_min_liquidity_msat(200, Duration::ZERO);
23742355

23752356
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2376-
.as_directed(&source, &target, 1_000, decay_params);
2357+
.as_directed(&source, &target, 1_000);
23772358
assert_eq!(liquidity.min_liquidity_msat(), 200);
23782359
assert_eq!(liquidity.max_liquidity_msat(), 300);
23792360

23802361
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2381-
.as_directed(&target, &source, 1_000, decay_params);
2362+
.as_directed(&target, &source, 1_000);
23822363
assert_eq!(liquidity.min_liquidity_msat(), 700);
23832364
assert_eq!(liquidity.max_liquidity_msat(), 800);
23842365

23852366
// Update maximum liquidity.
23862367

23872368
let liquidity = scorer.channel_liquidities.get(&43).unwrap()
2388-
.as_directed(&target, &recipient, 1_000, decay_params);
2369+
.as_directed(&target, &recipient, 1_000);
23892370
assert_eq!(liquidity.min_liquidity_msat(), 700);
23902371
assert_eq!(liquidity.max_liquidity_msat(), 900);
23912372

23922373
let liquidity = scorer.channel_liquidities.get(&43).unwrap()
2393-
.as_directed(&recipient, &target, 1_000, decay_params);
2374+
.as_directed(&recipient, &target, 1_000);
23942375
assert_eq!(liquidity.min_liquidity_msat(), 100);
23952376
assert_eq!(liquidity.max_liquidity_msat(), 300);
23962377

23972378
scorer.channel_liquidities.get_mut(&43).unwrap()
2398-
.as_directed_mut(&target, &recipient, 1_000, decay_params)
2379+
.as_directed_mut(&target, &recipient, 1_000)
23992380
.set_max_liquidity_msat(200, Duration::ZERO);
24002381

24012382
let liquidity = scorer.channel_liquidities.get(&43).unwrap()
2402-
.as_directed(&target, &recipient, 1_000, decay_params);
2383+
.as_directed(&target, &recipient, 1_000);
24032384
assert_eq!(liquidity.min_liquidity_msat(), 0);
24042385
assert_eq!(liquidity.max_liquidity_msat(), 200);
24052386

24062387
let liquidity = scorer.channel_liquidities.get(&43).unwrap()
2407-
.as_directed(&recipient, &target, 1_000, decay_params);
2388+
.as_directed(&recipient, &target, 1_000);
24082389
assert_eq!(liquidity.min_liquidity_msat(), 800);
24092390
assert_eq!(liquidity.max_liquidity_msat(), 1000);
24102391
}
@@ -2430,42 +2411,42 @@ mod tests {
24302411

24312412
// Check initial bounds.
24322413
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2433-
.as_directed(&source, &target, 1_000, decay_params);
2414+
.as_directed(&source, &target, 1_000);
24342415
assert_eq!(liquidity.min_liquidity_msat(), 400);
24352416
assert_eq!(liquidity.max_liquidity_msat(), 800);
24362417

24372418
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2438-
.as_directed(&target, &source, 1_000, decay_params);
2419+
.as_directed(&target, &source, 1_000);
24392420
assert_eq!(liquidity.min_liquidity_msat(), 200);
24402421
assert_eq!(liquidity.max_liquidity_msat(), 600);
24412422

24422423
// Reset from source to target.
24432424
scorer.channel_liquidities.get_mut(&42).unwrap()
2444-
.as_directed_mut(&source, &target, 1_000, decay_params)
2425+
.as_directed_mut(&source, &target, 1_000)
24452426
.set_min_liquidity_msat(900, Duration::ZERO);
24462427

24472428
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2448-
.as_directed(&source, &target, 1_000, decay_params);
2429+
.as_directed(&source, &target, 1_000);
24492430
assert_eq!(liquidity.min_liquidity_msat(), 900);
24502431
assert_eq!(liquidity.max_liquidity_msat(), 1_000);
24512432

24522433
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2453-
.as_directed(&target, &source, 1_000, decay_params);
2434+
.as_directed(&target, &source, 1_000);
24542435
assert_eq!(liquidity.min_liquidity_msat(), 0);
24552436
assert_eq!(liquidity.max_liquidity_msat(), 100);
24562437

24572438
// Reset from target to source.
24582439
scorer.channel_liquidities.get_mut(&42).unwrap()
2459-
.as_directed_mut(&target, &source, 1_000, decay_params)
2440+
.as_directed_mut(&target, &source, 1_000)
24602441
.set_min_liquidity_msat(400, Duration::ZERO);
24612442

24622443
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2463-
.as_directed(&source, &target, 1_000, decay_params);
2444+
.as_directed(&source, &target, 1_000);
24642445
assert_eq!(liquidity.min_liquidity_msat(), 0);
24652446
assert_eq!(liquidity.max_liquidity_msat(), 600);
24662447

24672448
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2468-
.as_directed(&target, &source, 1_000, decay_params);
2449+
.as_directed(&target, &source, 1_000);
24692450
assert_eq!(liquidity.min_liquidity_msat(), 400);
24702451
assert_eq!(liquidity.max_liquidity_msat(), 1_000);
24712452
}
@@ -2491,42 +2472,42 @@ mod tests {
24912472

24922473
// Check initial bounds.
24932474
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2494-
.as_directed(&source, &target, 1_000, decay_params);
2475+
.as_directed(&source, &target, 1_000);
24952476
assert_eq!(liquidity.min_liquidity_msat(), 400);
24962477
assert_eq!(liquidity.max_liquidity_msat(), 800);
24972478

24982479
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2499-
.as_directed(&target, &source, 1_000, decay_params);
2480+
.as_directed(&target, &source, 1_000);
25002481
assert_eq!(liquidity.min_liquidity_msat(), 200);
25012482
assert_eq!(liquidity.max_liquidity_msat(), 600);
25022483

25032484
// Reset from source to target.
25042485
scorer.channel_liquidities.get_mut(&42).unwrap()
2505-
.as_directed_mut(&source, &target, 1_000, decay_params)
2486+
.as_directed_mut(&source, &target, 1_000)
25062487
.set_max_liquidity_msat(300, Duration::ZERO);
25072488

25082489
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2509-
.as_directed(&source, &target, 1_000, decay_params);
2490+
.as_directed(&source, &target, 1_000);
25102491
assert_eq!(liquidity.min_liquidity_msat(), 0);
25112492
assert_eq!(liquidity.max_liquidity_msat(), 300);
25122493

25132494
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2514-
.as_directed(&target, &source, 1_000, decay_params);
2495+
.as_directed(&target, &source, 1_000);
25152496
assert_eq!(liquidity.min_liquidity_msat(), 700);
25162497
assert_eq!(liquidity.max_liquidity_msat(), 1_000);
25172498

25182499
// Reset from target to source.
25192500
scorer.channel_liquidities.get_mut(&42).unwrap()
2520-
.as_directed_mut(&target, &source, 1_000, decay_params)
2501+
.as_directed_mut(&target, &source, 1_000)
25212502
.set_max_liquidity_msat(600, Duration::ZERO);
25222503

25232504
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2524-
.as_directed(&source, &target, 1_000, decay_params);
2505+
.as_directed(&source, &target, 1_000);
25252506
assert_eq!(liquidity.min_liquidity_msat(), 400);
25262507
assert_eq!(liquidity.max_liquidity_msat(), 1_000);
25272508

25282509
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2529-
.as_directed(&target, &source, 1_000, decay_params);
2510+
.as_directed(&target, &source, 1_000);
25302511
assert_eq!(liquidity.min_liquidity_msat(), 0);
25312512
assert_eq!(liquidity.max_liquidity_msat(), 600);
25322513
}
@@ -2971,47 +2952,6 @@ mod tests {
29712952
assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), u64::max_value());
29722953
}
29732954

2974-
#[test]
2975-
fn decays_liquidity_bounds_without_shift_overflow() {
2976-
let logger = TestLogger::new();
2977-
let network_graph = network_graph(&logger);
2978-
let params = ProbabilisticScoringFeeParameters {
2979-
liquidity_penalty_multiplier_msat: 1_000,
2980-
..ProbabilisticScoringFeeParameters::zero_penalty()
2981-
};
2982-
let decay_params = ProbabilisticScoringDecayParameters {
2983-
liquidity_offset_half_life: Duration::from_secs(10),
2984-
..ProbabilisticScoringDecayParameters::default()
2985-
};
2986-
let mut scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger);
2987-
let source = source_node_id();
2988-
let usage = ChannelUsage {
2989-
amount_msat: 256,
2990-
inflight_htlc_msat: 0,
2991-
effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_000 },
2992-
};
2993-
let channel = network_graph.read_only().channel(42).unwrap().to_owned();
2994-
let (info, _) = channel.as_directed_from(&source).unwrap();
2995-
let candidate = CandidateRouteHop::PublicHop {
2996-
info,
2997-
short_channel_id: 42,
2998-
};
2999-
assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 125);
3000-
3001-
scorer.payment_path_failed(&payment_path_for_amount(512), 42, Duration::ZERO);
3002-
assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 281);
3003-
3004-
// An unchecked right shift 64 bits or more in DirectedChannelLiquidity::decayed_offset_msat
3005-
// would cause an overflow.
3006-
SinceEpoch::advance(Duration::from_secs(10 * 64));
3007-
scorer.time_passed(Duration::from_secs(10 * 64));
3008-
assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 125);
3009-
3010-
SinceEpoch::advance(Duration::from_secs(10));
3011-
scorer.time_passed(Duration::from_secs(10 * 65));
3012-
assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 125);
3013-
}
3014-
30152955
#[test]
30162956
fn restricts_liquidity_bounds_after_decay() {
30172957
let logger = TestLogger::new();
@@ -3660,7 +3600,7 @@ mod tests {
36603600
scorer.payment_path_failed(&path, 43, Duration::ZERO);
36613601

36623602
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
3663-
.as_directed(&source, &target, 1_000, decay_params);
3603+
.as_directed(&source, &target, 1_000);
36643604
assert_eq!(liquidity.min_liquidity_msat(), 256);
36653605
assert_eq!(liquidity.max_liquidity_msat(), 768);
36663606
}

0 commit comments

Comments
 (0)