Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't interpret decayed data as we've failed to send tiny values #3368

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Don't interpret decayed data as we've failed to send tiny values #3368

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions lightning/src/routing/scoring.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1818,15 +1818,27 @@ mod bucketed_history {
// values, which will result in us thinking we have some nontrivial probability of
// routing up to that amount.
if min_liquidity_offset_history_buckets[0] != 0 {
let mut highest_max_bucket_with_points = 0; // The highest max-bucket with any data
// Track the highest max-buckets with any data at all, as well as the highest
// max-bucket with at least BUCKET_FIXED_POINT_ONE.
let mut highest_max_bucket_with_points = 0;
let mut highest_max_bucket_with_full_points = None;
let mut total_max_points = 0; // Total points in max-buckets to consider
for (max_idx, max_bucket) in max_liquidity_offset_history_buckets.iter().enumerate() {
if *max_bucket >= BUCKET_FIXED_POINT_ONE {
highest_max_bucket_with_full_points = Some(cmp::max(highest_max_bucket_with_full_points.unwrap_or(0), max_idx));
}
if *max_bucket != 0 {
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;
// Use the highest max-bucket with at least BUCKET_FIXED_POINT_ONE, but if none is
// available use the highest max-bucket with any non-zero value. This ensures that
// if we have substantially decayed data we don't end up thinking the highest
// max-bucket is zero even though we have no points in the 0th max-bucket and do
// have points elsewhere.
let selected_max = highest_max_bucket_with_full_points.unwrap_or(highest_max_bucket_with_points);
let max_bucket_end_pos = BUCKET_START_POS[32 - selected_max] - 1;
Comment on lines +1840 to +1841
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slightly confused, mostly i am missing something.
iiuc, we used to "ignore max-buckets which have a value below BUCKET_FIXED_POINT_ONE"
and we no longer want to do that,
so why not just track max_bucket instead of tracking two max buckets? if they are valid regardless of the value they hold (as long as they're not zero)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're picking the highest max-bucket which has some value to use as the max-bucket for calculation, but we don't really want to be looking at a really high max-bucket if its fairly far decayed and there's some other max-bucket that isn't (as) decayed. We could have some smarter heuristic here for that, of course, but CPU cycles in this code are very expensive so the naive over-under-one is picked.

if payment_pos < max_bucket_end_pos {
let (numerator, denominator) = success_probability(payment_pos as u64, 0,
max_bucket_end_pos as u64, POSITION_TICKS as u64 - 1, params, true);
Expand Down
Loading