search

lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.17k stars 368 forks source link

Add `probing_diversity_penalty_msat` to the scorer parameters #3422

Open TheBlueMatt opened 6 days ago

TheBlueMatt commented 6 days ago

When doing background probing, its important to get a diverse view of the network graph to ensure you have as many options when pathfinding as possible.

Sadly, the naive probing we currently recommend users do does not accomplish that - using the same success-optimized pathfinder when sending as when probing results in always testing the same (good) path over and over and over again.

Instead, here, we add a probing_diversity_penalty_msat parameter to the scorer, allowing us to penalize channels for which we already have recent data. It applies a penalty which scales with the square of the inverse time since the channel was last updated, up to one day.

TheBlueMatt commented 6 days ago

Needs a test, should otherwise work, plan to start using this asap tho.

tnull commented 6 days ago

Instead, here, we add a probing_diversity_penalty_msat parameter to the scorer, allowing us to penalize channels for which we already have recent data. It applies a penalty which scales with the square of the inverse time since the channel was last updated, up to one day.

Hmm. I think we had previously discussed this and I expressed the need for path diversity in background probing to explore the wider network. However, I'm not sure why this should be a parameter on our regular scorer we use to send payments.

Shouldn't this rather be handled by target/path selection of the background probing software?

TheBlueMatt commented 6 days ago

Hmm. I think we had previously discussed this and I expressed the need for path diversity in background probing to explore the wider network. However, I'm not sure why this should be a parameter on our regular scorer we use to send payments.

I'm not sure exactly how we'd build it as a separate router. I think "just do the normal thing but try to explore backup options" seems like a pretty good strategy. Doing that as a separate router/scorer would be kinda hard - it really needs access to the internal scorer fields and scorer support.

Shouldn't this rather be handled by target/path selection of the background probing software?

It still would be. No sensible use of the scorer would ever set this parameter, you'd only ever set it when background probing.

codecov[bot] commented 6 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.22%. Comparing base (2d6720e) to head (957f93f).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3422 +/- ## ========================================== - Coverage 89.24% 89.22% -0.02% ========================================== Files 130 130 Lines 106959 107035 +76 Branches 106959 107035 +76 ========================================== + Hits 95452 95501 +49 - Misses 8718 8732 +14 - Partials 2789 2802 +13 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚨 Try these New Features: