Update probes_for_diversity to test datapoint time updating
#3713
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
lightning/src/routing/scoring.rs
Outdated
| scorer.payment_path_failed(&payment_path_for_amount(500), 42, Duration::ZERO); | ||
|
|
||
| // Apply an update to set the last-update time to 1 second | ||
| scorer.payment_path_failed(&payment_path_for_amount(490), 42, Duration::from_secs(1)); |
There was a problem hiding this comment.
If we don't update the bounds, didn't we still learn that the bounds are still valid? And maybe the data point time needs to be updated in that case?
There was a problem hiding this comment.
Maybe there's still something about the probe amount. Probing with 1 sat gives less information than 20% chan capacity. But I am assuming for this that constant amount probing is used.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
It turns out `probes_for_diversity` wasn't actually testing that the datapoint update time logic worked as it was indicating a payment failed while trying to fully saturate a channel (which teaches us nothing). Instead, we need to "send" less over the channel and update twice to get the channel last-update-time logic tested.
When we see a failure or success and it doesn't result in a bounds update, we previously were'nt updating the `last_datapoint_time` field as we were updating it in the bounds-update logic. This is wrong for the purpose of the `probing_diversity_penalty` because the whole point is to avoid repeatedly probing the same channel, but because of this we'll probe the same channel again and again as long as we don't learn any new information from the probes which causes a bounds update.
TheBlueMatt
force-pushed
the
2025-04-probing-diversity-test
branch
from
b895da6 to
c22a032
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3713 +/- ##
==========================================
+ Coverage 90.00% 91.23% +1.23%
==========================================
Files 151 157 +6
Lines 130296 145803 +15507
Branches 130296 145803 +15507
==========================================
+ Hits 117273 133029 +15756
+ Misses 10463 10375 -88
+ Partials 2560 2399 -161 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
It turns out
probes_for_diversitywasn't actually testing that the datapoint update time logic worked as it was indicating a payment failed while trying to fully saturate a channel (which teaches us nothing).Instead, we need to "send" less over the channel and update twice to get the channel last-update-time logic tested.