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

Improve logging of pathfinding behavior #1646

Closed
tnull opened this issue Aug 2, 2022 · 11 comments · Fixed by #2436
Closed

Improve logging of pathfinding behavior #1646

tnull opened this issue Aug 2, 2022 · 11 comments · Fixed by #2436
Labels
good first issue Good for newcomers

Comments

@tnull
Copy link
Contributor

tnull commented Aug 2, 2022

The behavior of sending a payment can be quite confusing to users, in particular if an otherwise obvious channel choice is avoided without giving any feedback as to why.

We therefore should try to improve the logging in get_route without getting too spammy. In particular, we should find a way to give users feedback when for example a direct channel is avoided due to lacking channel reserves, its value contribution, or similar.

@tnull tnull added the good first issue Good for newcomers label Aug 2, 2022
@Sharmalm
Copy link
Contributor

Sharmalm commented Jan 7, 2023

Hey i,d like to try this issue.

@Sharmalm
Copy link
Contributor

Sharmalm commented Jan 7, 2023

And i am new to opensource, so i may need guidance.

@tnull
Copy link
Contributor Author

tnull commented Jan 9, 2023

Welcome! The relevant parts of the code can be found in the get_route method which is part of lightning/routing/router.rs. I'd recommend first reading this method in its entirety to understand how it works (it's kind of a beast, featuring multiple sub-macros, etc).

Then you'll have to figure out for which cases it makes sense to add logging lines (probably in TRACE level). It's important to strike a balance here, as a lot of hops/paths are excluded trivially, which shouldn't spam our logs. However, it would be nice to have logging for edge cases, in particular if they result in no path being returned at all. So maybe to start you could, instead of just logging any candidate exclusion right away, remember the last failure reason and only log it when it results in no path being returned.

@Sharmalm
Copy link
Contributor

Sharmalm commented Jan 9, 2023

Thanks

@Sharmalm
Copy link
Contributor

Welcome! The relevant parts of the code can be found in the get_route method which is part of lightning/routing/router.rs. I'd recommend first reading this method in its entirety to understand how it works (it's kind of a beast, featuring multiple sub-macros, etc).

Then you'll have to figure out for which cases it makes sense to add logging lines (probably in TRACE level). It's important to strike a balance here, as a lot of hops/paths are excluded trivially, which shouldn't spam our logs. However, it would be nice to have logging for edge cases, in particular if they result in no path being returned at all. So maybe to start you could, instead of just logging any candidate exclusion right away, remember the last failure reason and only log it when it results in no path being returned.

Hey @tnull, thanks for the explanation, but I still find it hard to understand this issue. Please provide some material to understand the code.

@tnull
Copy link
Contributor Author

tnull commented Jan 10, 2023

Hey @tnull, thanks for the explanation, but I still find it hard to understand this issue. Please provide some material to understand the code.

Mh, could you be a bit more specific on what you're having trouble understanding? Also, feel free to find me on LDK Discord and we can discuss this a bit more interactively.

@Sharmalm
Copy link
Contributor

Yeah, I am finding it hard to understand code in lightning/routing/router.rs .
and tnull#9465 is this your discord username?

@tnull
Copy link
Contributor Author

tnull commented Jan 10, 2023

Yeah, I am finding it hard to understand code in lightning/routing/router.rs . and tnull#9465 is this your discord username?

Yep, feel free to reach out.

@Sharmalm
Copy link
Contributor

Yeah, I am finding it hard to understand code in lightning/routing/router.rs . and tnull#9465 is this your discord username?

Yep, feel free to reach out.

Ok

@Aankirz
Copy link

Aankirz commented Mar 7, 2023

Hello @tnull, I have spent some time in understanding LDK and I would like to work on this issue can you pls assign this to me ?

@tnull
Copy link
Contributor Author

tnull commented Mar 7, 2023

Hello @tnull, I have spent some time in understanding LDK and I would like to work on this issue can you pls assign this to me ?

I think @BOLT-12 is already working on this in #2011. So please pick another "good first issue". :)

tnull added a commit to tnull/rust-lightning that referenced this issue Jul 18, 2023
Previously, we barely gave any hints why we excluded certain hops during
pathfinding. Here, we introduce more verbose logging by a) accounting
how much candidates we ignored for which reasons and b) logging any
first/last/blinded hops we end up ignoring.

Fixes lightningdevkit#1646.
tnull added a commit to tnull/rust-lightning that referenced this issue Jul 18, 2023
Previously, we barely gave any hints why we excluded certain hops during
pathfinding. Here, we introduce more verbose logging by a) accounting
how much candidates we ignored for which reasons and b) logging any
first/last/blinded hops we end up ignoring.

Fixes lightningdevkit#1646.
tnull added a commit to tnull/rust-lightning that referenced this issue Jul 20, 2023
Previously, we barely gave any hints why we excluded certain hops during
pathfinding. Here, we introduce more verbose logging by a) accounting
how much candidates we ignored for which reasons and b) logging any
first/last/blinded hops we end up ignoring.

Fixes lightningdevkit#1646.
tnull added a commit to tnull/rust-lightning that referenced this issue Jul 21, 2023
Previously, we barely gave any hints why we excluded certain hops during
pathfinding. Here, we introduce more verbose logging by a) accounting
how much candidates we ignored for which reasons and b) logging any
first/last/blinded hops we end up ignoring.

Fixes lightningdevkit#1646.
tnull added a commit to tnull/rust-lightning that referenced this issue Jul 21, 2023
Previously, we barely gave any hints why we excluded certain hops during
pathfinding. Here, we introduce more verbose logging by a) accounting
how much candidates we ignored for which reasons and b) logging any
first/last/blinded hops we end up ignoring.

Fixes lightningdevkit#1646.
tnull added a commit to tnull/rust-lightning that referenced this issue Jul 21, 2023
Previously, we barely gave any hints why we excluded certain hops during
pathfinding. Here, we introduce more verbose logging by a) accounting
how much candidates we ignored for which reasons and b) logging any
first/last/blinded hops we end up ignoring.

Fixes lightningdevkit#1646.
waterson pushed a commit to waterson/rust-lightning that referenced this issue Jul 31, 2023
Previously, we barely gave any hints why we excluded certain hops during
pathfinding. Here, we introduce more verbose logging by a) accounting
how much candidates we ignored for which reasons and b) logging any
first/last/blinded hops we end up ignoring.

Fixes lightningdevkit#1646.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants