Skip to content

Conversation

@f321x
Copy link
Member

@f321x f321x commented Jan 21, 2025

Related to #9433 (comment). Adds a bool flag to NoPathFound exception that can be used to determine if the exception could be caused by a too low fee or if it has another cause when catching it. This information is then be used to send either OnionFailureCode.TRAMPOLINE_FEE_INSUFFICIENT or OnionFailureCode.UNKNOWN_NEXT_PEER when failing to forward a trampoline payment, so we don't always return TRAMPOLINE_FEE_INSUFFICIENT even if maybe there is no channel available at all (so raising fees would be useless).
The flag could also be used client side, for example to show the user that a payment maybe failed because of a too low fee instead of just showing that it failed without reason as we use the NoPathFound exception too.
However, maybe there is a more elegant version than a bool flag, like an additional exception (NoPathTooLowFee)?

@f321x f321x changed the title Seperate between fee related NoPathFound and other causes when doing lightning payments/forwards Separate between fee related NoPathFound and other causes when doing lightning payments/forwards Jan 21, 2025
@ecdsa
Copy link
Member

ecdsa commented Jan 22, 2025

I think it would be more elegant to raise another exception, rather than adding this flag

f321x added 2 commits January 28, 2025 15:58
store exception in variable instead of using a bool flag

add default str to routing exceptions

Add separate exception class to handle fee related payment errors
@f321x f321x force-pushed the improved_trampoline_error branch from 118b156 to 5eb9aa0 Compare January 28, 2025 15:16
@f321x f321x marked this pull request as ready for review February 3, 2025 11:17
@ecdsa ecdsa merged commit c725a29 into spesmilo:master Feb 3, 2025
14 checks passed
@f321x f321x deleted the improved_trampoline_error branch February 10, 2025 13:39
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 this pull request may close these issues.

2 participants