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.

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

Allow to set maximum total fees during routing #2339

Closed
tnull opened this issue Jun 5, 2023 · 3 comments · Fixed by #2417
Closed

Allow to set maximum total fees during routing #2339

tnull opened this issue Jun 5, 2023 · 3 comments · Fixed by #2417
Assignees

Comments

@tnull
Copy link
Contributor

tnull commented Jun 5, 2023

We should give users the capability of setting a hard cap for the routing fees, e.g, through PaymentParameters::max_total_routing_fee or similar.

This might also go hand-in-hand with #495, as such an upper bound might give us some indication of a privacy budget we're allowed to spend on finding more reliable or more random routes.

@tnull tnull changed the title Allow to set maximum total during routing Allow to set maximum total fees during routing Jun 5, 2023
@TheBlueMatt
Copy link
Collaborator

We definitely should, but applying this is a bit complicated, at least on the outbound_payment end - we'll need to figure out how much of our budget is left every time we go to retry, and will run the risk of "over-spending" on one path and then running out entirely on another retry. At least on the routing end its probably not that hard to apply, though there's no way to set a total limit on a secondary metric in dijkstras without spuriously failing, but hopefully its low enough risk it doesn't matter.

@TheBlueMatt TheBlueMatt modified the milestones: 0.1, 0.1.1 Jun 5, 2023
@tnull
Copy link
Contributor Author

tnull commented Jul 13, 2023

As stated in #2409:

Also we should rip out the 10% check in the retry code which was really just a buggy code check, but that should happen later.

@tnull tnull modified the milestones: 0.1.1, 0.0.116 Jul 13, 2023
@tnull tnull self-assigned this Jul 13, 2023
@TheBlueMatt TheBlueMatt modified the milestones: 0.0.116, 0.0.117 Jul 21, 2023
@TheBlueMatt
Copy link
Collaborator

Turned out to be a bit bigger of a refactor than we could fit inside 116, but definitely need to get it in in 117.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants