Special-case the 0th minimum bucket in historical scoring

Points in the 0th minimum bucket either indicate we sent a payment
which is < 1/16,384th of the channel's capacity or, more likely,
we failed to send a payment. In either case, averaging the success
probability across the full range of upper-bounds doesn't make a
whole lot of sense - if we've never managed to send a "real"
payment over a channel, we should be considering it quite poor.

To address this, we special-case the 0th minimum bucket and only
look at the largest-offset max bucket when calculating the success
probability.

Author: TheBlueMatt

lightning/src/routing/scoring.rs

@@ -1758,7 +1758,32 @@ mod bucketed_history {
 			}
 
 			let mut cumulative_success_prob_times_billion = 0;
-			for (min_idx, min_bucket) in self.min_liquidity_offset_history.buckets.iter().enumerate() {
+			// Special-case the 0th min bucket - it generally means we failed a payment, so only
+			// consider the highest (i.e. largest-offset-from-max-capacity) max bucket for all
+			// points against the 0th min bucket!
+			if self.min_liquidity_offset_history.buckets[0] != 0 {
+				let mut highest_max_bucket_with_points = 0;
+				let mut total_max_points = 0;
+				for (max_idx, max_bucket) in self.max_liquidity_offset_history.buckets.iter().enumerate() {
+					if *max_bucket >= 32 {
+						highest_max_bucket_with_points = cmp::max(highest_max_bucket_with_points, max_idx);
+					}
+					total_max_points += *max_bucket as u64;
+				}
+				let max_bucket_end_pos = BUCKET_START_POS[32 - highest_max_bucket_with_points] - 1;
+				if payment_pos < max_bucket_end_pos {
+					let bucket_prob_times_billion =
+						(self.min_liquidity_offset_history.buckets[0] as u64) * total_max_points
+							* 1024 * 1024 * 1024 / total_valid_points_tracked;
+					cumulative_success_prob_times_billion += bucket_prob_times_billion *
+						((max_bucket_end_pos - payment_pos) as u64) /
+						// Add an additional one in the divisor as the payment bucket has been
+						// rounded down.
+						(max_bucket_end_pos + 1) as u64;
+				}
+			}
+
+			for (min_idx, min_bucket) in self.min_liquidity_offset_history.buckets.iter().enumerate().skip(1) {
 				let min_bucket_start_pos = BUCKET_START_POS[min_idx];
 				for (max_idx, max_bucket) in self.max_liquidity_offset_history.buckets.iter().enumerate().take(32 - min_idx) {
 					let max_bucket_end_pos = BUCKET_START_POS[32 - max_idx] - 1;
@@ -2987,7 +3012,7 @@ mod tests {
 		// Even after we tell the scorer we definitely have enough available liquidity, it will
 		// still remember that there was some failure in the past, and assign a non-0 penalty.
 		scorer.payment_path_failed(&payment_path_for_amount(1000).iter().collect::<Vec<_>>(), 43);
-		assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 198);
+		assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 32);
 		// The first points should be decayed just slightly and the last bucket has a new point.
 		assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
 			Some(([31, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 32, 0, 0, 0, 0, 0],
@@ -2997,8 +3022,8 @@ mod tests {
 		// simply check bounds here.
 		let five_hundred_prob =
 			scorer.historical_estimated_payment_success_probability(42, &target, 500).unwrap();
-		assert!(five_hundred_prob > 512 * 1024 * 1024); // 0.5
-		assert!(five_hundred_prob < 532 * 1024 * 1024); // ~ 0.52
+		assert!(five_hundred_prob > 680 * 1024 * 1024); // ~0.66
+		assert!(five_hundred_prob < 690 * 1024 * 1024); // ~0.67
 		let one_prob =
 			scorer.historical_estimated_payment_success_probability(42, &target, 1).unwrap();
 		assert!(one_prob < 1024 * 1024 * 1024);
@@ -3147,16 +3172,14 @@ mod tests {
 		assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat),
 			Some(512 * 1024 * 1024));
 
-		// ...but once we see a failure, we consider the payment to be substantially less likely,
-		// even though not a probability of zero as we still look at the second max bucket which
-		// now shows 31.
+		// Once we see a second failure for the lower amount, we'll again consider the success
+		// probability zero, as we only look at the lowest-value maximum bucket when all previous
+		// attempts at using a channel have failed.
 		scorer.payment_path_failed(&payment_path_for_amount(amount_msat).iter().collect::<Vec<_>>(), 42);
 		assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
 			Some(([63, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
 				[32, 31, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0])));
-		assert!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat)
-			.unwrap() > 251 * 1024 * 1024);
-		assert!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat)
-			.unwrap() < 256 * 1024 * 1024);
+		assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat),
+			Some(0));
 	}
 }