Skip to content

In-flight amounts get calculated as a part of the per-hop-per-amount penalty when scoring a channel #3271

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.

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

Closed
TheBlueMatt opened this issue Aug 23, 2024 · 2 comments · Fixed by #3356
Assignees
Labels
Take a Friday Leave a Friday Stomp the Bugs, Without Much Commitment

Comments

@TheBlueMatt
Copy link
Collaborator

@jkczyz and @wvanlint pointed out that in df52da7 we started applying the in-flight amounts as a part of the per-hop-per-amount penalty which can lead to us strongly avoiding channels that have in-flight funds already. This doesn't make sense as a part of our model, though - the per-hop-per-amount penalty is intended to just provide a per-hop fixed penalty (as there is some per-hop failure probability). It makes sense in the success probability part of our model, though, so we may need to pass the in-flight amount through to the scorer, finally.

@TheBlueMatt TheBlueMatt added the Take a Friday Leave a Friday Stomp the Bugs, Without Much Commitment label Aug 23, 2024
arik-so added a commit to arik-so/rust-lightning that referenced this issue Sep 20, 2024
@jkczyz jkczyz self-assigned this Oct 9, 2024
@jkczyz
Copy link
Contributor

jkczyz commented Oct 9, 2024

Looking into this, I see that:

  • base_penalty_amount_multiplier_msat is applied to the payment amount even though the documentation was updated to say to the total amount
  • liquidity_penalty_amount_multiplier_msat and historical_liquidity_penalty_amount_multiplier_msat are applied to the total amount as the documentation states

IIUC, the first just requires reverting the documentation change. The second, however, should only apply to the payment amount. Though the total amount should be used in the success_probability calculation.

@TheBlueMatt Does all that sound right?

@jkczyz
Copy link
Contributor

jkczyz commented Oct 10, 2024

IIUC, the first just requires reverting the documentation change. The second, however, should only apply to the payment amount. Though the total amount should be used in the success_probability calculation.

@TheBlueMatt Does all that sound right?

See #3356 for the proposed fix mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Take a Friday Leave a Friday Stomp the Bugs, Without Much Commitment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants