Skip to content

Commit 10cfe5c

Browse files
jkczyzjkczyz
committed
Fix scorer panic when available capacity is zero
ProbabilisticScorer takes a ChannelUsage when computing a penalty for a channel. The formula for calculating the liquidity penalty reduces the maximum capacity by the amount of in-flight HTLCs (available capacity) and adds one to prevent division by zero. However, since the available capacity is passed to DirectedChannelLiquidity as the capacity, other penalty formulas may use the available (i.e., reduced) capacity inadvertently. In practice, this has two ramifications for the historical liquidity penalty computation: 1. The bucket formula doesn't have a consistent denominator for a given channel. 2. The bucket formula may divide by zero when the in-flight HTLC amount equals or exceeds the effective capacity. Fixing this involves only using the available capacity when appropriate.
1 parent 8311581 commit 10cfe5c

File tree

1 file changed

+71
-44
lines changed

1 file changed

+71
-44
lines changed

‎lightning/src/routing/scoring.rs

Lines changed: 71 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,7 @@ struct DirectedChannelLiquidity<'a, L: Deref<Target = u64>, BRT: Deref<Target =
702702
max_liquidity_offset_msat: L,
703703
min_liquidity_offset_history: BRT,
704704
max_liquidity_offset_history: BRT,
705+
inflight_htlc_msat: u64,
705706
capacity_msat: u64,
706707
last_updated: U,
707708
now: T,
@@ -739,7 +740,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
739740
let log_direction = |source, target| {
740741
if let Some((directed_info, _)) = chan_debug.as_directed_to(target) {
741742
let amt = directed_info.effective_capacity().as_msat();
742-
let dir_liq = liq.as_directed(source, target, amt, &self.params);
743+
let dir_liq = liq.as_directed(source, target, 0, amt, &self.params);
743744

744745
let buckets = HistoricalMinMaxBuckets {
745746
min_liquidity_offset_history: &dir_liq.min_liquidity_offset_history,
@@ -781,7 +782,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
781782
if let Some(liq) = self.channel_liquidities.get(&scid) {
782783
if let Some((directed_info, source)) = chan.as_directed_to(target) {
783784
let amt = directed_info.effective_capacity().as_msat();
784-
let dir_liq = liq.as_directed(source, target, amt, &self.params);
785+
let dir_liq = liq.as_directed(source, target, 0, amt, &self.params);
785786
return Some((dir_liq.min_liquidity_msat(), dir_liq.max_liquidity_msat()));
786787
}
787788
}
@@ -818,7 +819,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
818819
if let Some(liq) = self.channel_liquidities.get(&scid) {
819820
if let Some((directed_info, source)) = chan.as_directed_to(target) {
820821
let amt = directed_info.effective_capacity().as_msat();
821-
let dir_liq = liq.as_directed(source, target, amt, &self.params);
822+
let dir_liq = liq.as_directed(source, target, 0, amt, &self.params);
822823

823824
let buckets = HistoricalMinMaxBuckets {
824825
min_liquidity_offset_history: &dir_liq.min_liquidity_offset_history,
@@ -923,7 +924,8 @@ impl<T: Time> ChannelLiquidity<T> {
923924
/// Returns a view of the channel liquidity directed from `source` to `target` assuming
924925
/// `capacity_msat`.
925926
fn as_directed<'a>(
926-
&self, source: &NodeId, target: &NodeId, capacity_msat: u64, params: &'a ProbabilisticScoringParameters
927+
&self, source: &NodeId, target: &NodeId, inflight_htlc_msat: u64, capacity_msat: u64,
928+
params: &'a ProbabilisticScoringParameters
927929
) -> DirectedChannelLiquidity<'a, &u64, &HistoricalBucketRangeTracker, T, &T> {
928930
let (min_liquidity_offset_msat, max_liquidity_offset_msat, min_liquidity_offset_history, max_liquidity_offset_history) =
929931
if source < target {
@@ -939,6 +941,7 @@ impl<T: Time> ChannelLiquidity<T> {
939941
max_liquidity_offset_msat,
940942
min_liquidity_offset_history,
941943
max_liquidity_offset_history,
944+
inflight_htlc_msat,
942945
capacity_msat,
943946
last_updated: &self.last_updated,
944947
now: T::now(),
@@ -949,7 +952,8 @@ impl<T: Time> ChannelLiquidity<T> {
949952
/// Returns a mutable view of the channel liquidity directed from `source` to `target` assuming
950953
/// `capacity_msat`.
951954
fn as_directed_mut<'a>(
952-
&mut self, source: &NodeId, target: &NodeId, capacity_msat: u64, params: &'a ProbabilisticScoringParameters
955+
&mut self, source: &NodeId, target: &NodeId, inflight_htlc_msat: u64, capacity_msat: u64,
956+
params: &'a ProbabilisticScoringParameters
953957
) -> DirectedChannelLiquidity<'a, &mut u64, &mut HistoricalBucketRangeTracker, T, &mut T> {
954958
let (min_liquidity_offset_msat, max_liquidity_offset_msat, min_liquidity_offset_history, max_liquidity_offset_history) =
955959
if source < target {
@@ -965,6 +969,7 @@ impl<T: Time> ChannelLiquidity<T> {
965969
max_liquidity_offset_msat,
966970
min_liquidity_offset_history,
967971
max_liquidity_offset_history,
972+
inflight_htlc_msat,
968973
capacity_msat,
969974
last_updated: &mut self.last_updated,
970975
now: T::now(),
@@ -1022,7 +1027,16 @@ impl<L: Deref<Target = u64>, BRT: Deref<Target = HistoricalBucketRangeTracker>,
10221027

10231028
if params.historical_liquidity_penalty_multiplier_msat != 0 ||
10241029
params.historical_liquidity_penalty_amount_multiplier_msat != 0 {
1025-
let payment_amt_64th_bucket = amount_msat * 64 / self.capacity_msat;
1030+
let payment_amt_64th_bucket = if amount_msat < u64::max_value() / 64 {
1031+
amount_msat * 64 / self.capacity_msat
1032+
} else {
1033+
// Only use 128-bit arithmetic when multiplication will overflow to avoid 128-bit
1034+
// division. This branch should only be hit in fuzz testing since the amount would
1035+
// need to be over 2.88 million BTC in practice.
1036+
((amount_msat as u128) * 64 / (self.capacity_msat as u128))
1037+
.try_into().unwrap_or(65)
1038+
};
1039+
#[cfg(not(fuzzing))]
10261040
debug_assert!(payment_amt_64th_bucket <= 64);
10271041
if payment_amt_64th_bucket > 64 { return res; }
10281042

@@ -1042,13 +1056,14 @@ impl<L: Deref<Target = u64>, BRT: Deref<Target = HistoricalBucketRangeTracker>,
10421056
// If we don't have any valid points (or, once decayed, we have less than a full
10431057
// point), redo the non-historical calculation with no liquidity bounds tracked and
10441058
// the historical penalty multipliers.
1045-
let max_capacity = self.capacity_msat.saturating_sub(amount_msat).saturating_add(1);
1059+
let available_capacity = self.available_capacity();
1060+
let numerator = available_capacity.saturating_sub(amount_msat).saturating_add(1);
1061+
let denominator = available_capacity.saturating_add(1);
10461062
let negative_log10_times_2048 =
1047-
approx::negative_log10_times_2048(max_capacity, self.capacity_msat.saturating_add(1));
1063+
approx::negative_log10_times_2048(numerator, denominator);
10481064
res = res.saturating_add(Self::combined_penalty_msat(amount_msat, negative_log10_times_2048,
10491065
params.historical_liquidity_penalty_multiplier_msat,
10501066
params.historical_liquidity_penalty_amount_multiplier_msat));
1051-
return res;
10521067
}
10531068
}
10541069

@@ -1080,9 +1095,13 @@ impl<L: Deref<Target = u64>, BRT: Deref<Target = HistoricalBucketRangeTracker>,
10801095

10811096
/// Returns the upper bound of the channel liquidity balance in this direction.
10821097
fn max_liquidity_msat(&self) -> u64 {
1083-
self.capacity_msat
1084-
.checked_sub(self.decayed_offset_msat(*self.max_liquidity_offset_msat))
1085-
.unwrap_or(0)
1098+
self.available_capacity()
1099+
.saturating_sub(self.decayed_offset_msat(*self.max_liquidity_offset_msat))
1100+
}
1101+
1102+
/// Returns the capacity minus the in-flight HTLCs in this direction.
1103+
fn available_capacity(&self) -> u64 {
1104+
self.capacity_msat.saturating_sub(self.inflight_htlc_msat)
10861105
}
10871106

10881107
fn decayed_offset_msat(&self, offset_msat: u64) -> u64 {
@@ -1201,12 +1220,12 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> Score for Probabilis
12011220
}
12021221

12031222
let amount_msat = usage.amount_msat;
1204-
let capacity_msat = usage.effective_capacity.as_msat()
1205-
.saturating_sub(usage.inflight_htlc_msat);
1223+
let capacity_msat = usage.effective_capacity.as_msat();
1224+
let inflight_htlc_msat = usage.inflight_htlc_msat;
12061225
self.channel_liquidities
12071226
.get(&short_channel_id)
12081227
.unwrap_or(&ChannelLiquidity::new())
1209-
.as_directed(source, target, capacity_msat, &self.params)
1228+
.as_directed(source, target, inflight_htlc_msat, capacity_msat, &self.params)
12101229
.penalty_msat(amount_msat, &self.params)
12111230
.saturating_add(anti_probing_penalty_msat)
12121231
.saturating_add(base_penalty_msat)
@@ -1234,13 +1253,13 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> Score for Probabilis
12341253
self.channel_liquidities
12351254
.entry(hop.short_channel_id)
12361255
.or_insert_with(ChannelLiquidity::new)
1237-
.as_directed_mut(source, &target, capacity_msat, &self.params)
1256+
.as_directed_mut(source, &target, 0, capacity_msat, &self.params)
12381257
.failed_at_channel(amount_msat, format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
12391258
} else {
12401259
self.channel_liquidities
12411260
.entry(hop.short_channel_id)
12421261
.or_insert_with(ChannelLiquidity::new)
1243-
.as_directed_mut(source, &target, capacity_msat, &self.params)
1262+
.as_directed_mut(source, &target, 0, capacity_msat, &self.params)
12441263
.failed_downstream(amount_msat, format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
12451264
}
12461265
} else {
@@ -1268,7 +1287,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> Score for Probabilis
12681287
self.channel_liquidities
12691288
.entry(hop.short_channel_id)
12701289
.or_insert_with(ChannelLiquidity::new)
1271-
.as_directed_mut(source, &target, capacity_msat, &self.params)
1290+
.as_directed_mut(source, &target, 0, capacity_msat, &self.params)
12721291
.successful(amount_msat, format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
12731292
} else {
12741293
log_debug!(self.logger, "Not able to learn for channel with SCID {} as we do not have graph info for it (likely a route-hint last-hop).",
@@ -1873,52 +1892,52 @@ mod tests {
18731892
// Update minimum liquidity.
18741893

18751894
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1876-
.as_directed(&source, &target, 1_000, &scorer.params);
1895+
.as_directed(&source, &target, 0, 1_000, &scorer.params);
18771896
assert_eq!(liquidity.min_liquidity_msat(), 100);
18781897
assert_eq!(liquidity.max_liquidity_msat(), 300);
18791898

18801899
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1881-
.as_directed(&target, &source, 1_000, &scorer.params);
1900+
.as_directed(&target, &source, 0, 1_000, &scorer.params);
18821901
assert_eq!(liquidity.min_liquidity_msat(), 700);
18831902
assert_eq!(liquidity.max_liquidity_msat(), 900);
18841903

18851904
scorer.channel_liquidities.get_mut(&42).unwrap()
1886-
.as_directed_mut(&source, &target, 1_000, &scorer.params)
1905+
.as_directed_mut(&source, &target, 0, 1_000, &scorer.params)
18871906
.set_min_liquidity_msat(200);
18881907

18891908
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1890-
.as_directed(&source, &target, 1_000, &scorer.params);
1909+
.as_directed(&source, &target, 0, 1_000, &scorer.params);
18911910
assert_eq!(liquidity.min_liquidity_msat(), 200);
18921911
assert_eq!(liquidity.max_liquidity_msat(), 300);
18931912

18941913
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1895-
.as_directed(&target, &source, 1_000, &scorer.params);
1914+
.as_directed(&target, &source, 0, 1_000, &scorer.params);
18961915
assert_eq!(liquidity.min_liquidity_msat(), 700);
18971916
assert_eq!(liquidity.max_liquidity_msat(), 800);
18981917

18991918
// Update maximum liquidity.
19001919

19011920
let liquidity = scorer.channel_liquidities.get(&43).unwrap()
1902-
.as_directed(&target, &recipient, 1_000, &scorer.params);
1921+
.as_directed(&target, &recipient, 0, 1_000, &scorer.params);
19031922
assert_eq!(liquidity.min_liquidity_msat(), 700);
19041923
assert_eq!(liquidity.max_liquidity_msat(), 900);
19051924

19061925
let liquidity = scorer.channel_liquidities.get(&43).unwrap()
1907-
.as_directed(&recipient, &target, 1_000, &scorer.params);
1926+
.as_directed(&recipient, &target, 0, 1_000, &scorer.params);
19081927
assert_eq!(liquidity.min_liquidity_msat(), 100);
19091928
assert_eq!(liquidity.max_liquidity_msat(), 300);
19101929

19111930
scorer.channel_liquidities.get_mut(&43).unwrap()
1912-
.as_directed_mut(&target, &recipient, 1_000, &scorer.params)
1931+
.as_directed_mut(&target, &recipient, 0, 1_000, &scorer.params)
19131932
.set_max_liquidity_msat(200);
19141933

19151934
let liquidity = scorer.channel_liquidities.get(&43).unwrap()
1916-
.as_directed(&target, &recipient, 1_000, &scorer.params);
1935+
.as_directed(&target, &recipient, 0, 1_000, &scorer.params);
19171936
assert_eq!(liquidity.min_liquidity_msat(), 0);
19181937
assert_eq!(liquidity.max_liquidity_msat(), 200);
19191938

19201939
let liquidity = scorer.channel_liquidities.get(&43).unwrap()
1921-
.as_directed(&recipient, &target, 1_000, &scorer.params);
1940+
.as_directed(&recipient, &target, 0, 1_000, &scorer.params);
19221941
assert_eq!(liquidity.min_liquidity_msat(), 800);
19231942
assert_eq!(liquidity.max_liquidity_msat(), 1000);
19241943
}
@@ -1942,42 +1961,42 @@ mod tests {
19421961

19431962
// Check initial bounds.
19441963
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1945-
.as_directed(&source, &target, 1_000, &scorer.params);
1964+
.as_directed(&source, &target, 0, 1_000, &scorer.params);
19461965
assert_eq!(liquidity.min_liquidity_msat(), 400);
19471966
assert_eq!(liquidity.max_liquidity_msat(), 800);
19481967

19491968
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1950-
.as_directed(&target, &source, 1_000, &scorer.params);
1969+
.as_directed(&target, &source, 0, 1_000, &scorer.params);
19511970
assert_eq!(liquidity.min_liquidity_msat(), 200);
19521971
assert_eq!(liquidity.max_liquidity_msat(), 600);
19531972

19541973
// Reset from source to target.
19551974
scorer.channel_liquidities.get_mut(&42).unwrap()
1956-
.as_directed_mut(&source, &target, 1_000, &scorer.params)
1975+
.as_directed_mut(&source, &target, 0, 1_000, &scorer.params)
19571976
.set_min_liquidity_msat(900);
19581977

19591978
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1960-
.as_directed(&source, &target, 1_000, &scorer.params);
1979+
.as_directed(&source, &target, 0, 1_000, &scorer.params);
19611980
assert_eq!(liquidity.min_liquidity_msat(), 900);
19621981
assert_eq!(liquidity.max_liquidity_msat(), 1_000);
19631982

19641983
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1965-
.as_directed(&target, &source, 1_000, &scorer.params);
1984+
.as_directed(&target, &source, 0, 1_000, &scorer.params);
19661985
assert_eq!(liquidity.min_liquidity_msat(), 0);
19671986
assert_eq!(liquidity.max_liquidity_msat(), 100);
19681987

19691988
// Reset from target to source.
19701989
scorer.channel_liquidities.get_mut(&42).unwrap()
1971-
.as_directed_mut(&target, &source, 1_000, &scorer.params)
1990+
.as_directed_mut(&target, &source, 0, 1_000, &scorer.params)
19721991
.set_min_liquidity_msat(400);
19731992

19741993
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1975-
.as_directed(&source, &target, 1_000, &scorer.params);
1994+
.as_directed(&source, &target, 0, 1_000, &scorer.params);
19761995
assert_eq!(liquidity.min_liquidity_msat(), 0);
19771996
assert_eq!(liquidity.max_liquidity_msat(), 600);
19781997

19791998
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
1980-
.as_directed(&target, &source, 1_000, &scorer.params);
1999+
.as_directed(&target, &source, 0, 1_000, &scorer.params);
19812000
assert_eq!(liquidity.min_liquidity_msat(), 400);
19822001
assert_eq!(liquidity.max_liquidity_msat(), 1_000);
19832002
}
@@ -2001,42 +2020,42 @@ mod tests {
20012020

20022021
// Check initial bounds.
20032022
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2004-
.as_directed(&source, &target, 1_000, &scorer.params);
2023+
.as_directed(&source, &target, 0, 1_000, &scorer.params);
20052024
assert_eq!(liquidity.min_liquidity_msat(), 400);
20062025
assert_eq!(liquidity.max_liquidity_msat(), 800);
20072026

20082027
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2009-
.as_directed(&target, &source, 1_000, &scorer.params);
2028+
.as_directed(&target, &source, 0, 1_000, &scorer.params);
20102029
assert_eq!(liquidity.min_liquidity_msat(), 200);
20112030
assert_eq!(liquidity.max_liquidity_msat(), 600);
20122031

20132032
// Reset from source to target.
20142033
scorer.channel_liquidities.get_mut(&42).unwrap()
2015-
.as_directed_mut(&source, &target, 1_000, &scorer.params)
2034+
.as_directed_mut(&source, &target, 0, 1_000, &scorer.params)
20162035
.set_max_liquidity_msat(300);
20172036

20182037
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2019-
.as_directed(&source, &target, 1_000, &scorer.params);
2038+
.as_directed(&source, &target, 0, 1_000, &scorer.params);
20202039
assert_eq!(liquidity.min_liquidity_msat(), 0);
20212040
assert_eq!(liquidity.max_liquidity_msat(), 300);
20222041

20232042
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2024-
.as_directed(&target, &source, 1_000, &scorer.params);
2043+
.as_directed(&target, &source, 0, 1_000, &scorer.params);
20252044
assert_eq!(liquidity.min_liquidity_msat(), 700);
20262045
assert_eq!(liquidity.max_liquidity_msat(), 1_000);
20272046

20282047
// Reset from target to source.
20292048
scorer.channel_liquidities.get_mut(&42).unwrap()
2030-
.as_directed_mut(&target, &source, 1_000, &scorer.params)
2049+
.as_directed_mut(&target, &source, 0, 1_000, &scorer.params)
20312050
.set_max_liquidity_msat(600);
20322051

20332052
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2034-
.as_directed(&source, &target, 1_000, &scorer.params);
2053+
.as_directed(&source, &target, 0, 1_000, &scorer.params);
20352054
assert_eq!(liquidity.min_liquidity_msat(), 400);
20362055
assert_eq!(liquidity.max_liquidity_msat(), 1_000);
20372056

20382057
let liquidity = scorer.channel_liquidities.get(&42).unwrap()
2039-
.as_directed(&target, &source, 1_000, &scorer.params);
2058+
.as_directed(&target, &source, 0, 1_000, &scorer.params);
20402059
assert_eq!(liquidity.min_liquidity_msat(), 0);
20412060
assert_eq!(liquidity.max_liquidity_msat(), 600);
20422061
}
@@ -2774,6 +2793,14 @@ mod tests {
27742793
// data entirely instead.
27752794
assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
27762795
Some(([0; 8], [0; 8])));
2796+
2797+
let usage = ChannelUsage {
2798+
amount_msat: 100,
2799+
inflight_htlc_msat: 1024,
2800+
effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_024 },
2801+
};
2802+
scorer.payment_path_failed(&payment_path_for_amount(1).iter().collect::<Vec<_>>(), 42);
2803+
assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 2048);
27772804
}
27782805

27792806
#[test]

0 commit comments

Comments
 (0)