Skip to content

Improve logging of pathfinding behavior #2011

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
wants to merge 28 commits into from
Closed
Changes from 7 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9073b8d
first commit
Sharmalm Feb 4, 2023
02dddb4
second commit
Sharmalm Feb 4, 2023
0312c2b
original
Sharmalm Feb 5, 2023
3cdb8ed
first logging
Sharmalm Feb 5, 2023
b6ac4ec
Merge remote-tracking branch 'origin/main'
Sharmalm Feb 5, 2023
cfe611a
fifth commit
Sharmalm Feb 5, 2023
5bd7708
third commit
Sharmalm Feb 5, 2023
87eef3f
fourth commit
Sharmalm Feb 9, 2023
bb6c7f6
Merge branch 'lightningdevkit:main' into main
Sharmalm Feb 9, 2023
aef2aa6
Merge branch 'lightningdevkit:main' into main
Sharmalm Feb 11, 2023
4a2c289
sixth commit
Sharmalm Feb 11, 2023
a8781ca
seventh commit
Sharmalm Feb 11, 2023
0e5e8a6
eight commit
Sharmalm Feb 14, 2023
3240053
Merge branch 'lightningdevkit:main' into main
Sharmalm Feb 14, 2023
b913098
Merge branch 'lightningdevkit:main' into main
Sharmalm Feb 15, 2023
8d2a85d
ninth
Sharmalm Feb 15, 2023
fe93ed8
Merge branch 'lightningdevkit:main' into main
Sharmalm Feb 20, 2023
6da0e06
access hashmap
Sharmalm Feb 20, 2023
78dbb77
Merge remote-tracking branch 'origin/main'
Sharmalm Feb 20, 2023
0be98f5
Merge branch 'lightningdevkit:main' into main
Sharmalm Feb 24, 2023
c830123
Merge branch 'lightningdevkit:main' into main
Sharmalm Mar 1, 2023
9bc34a1
improving hashmap conditions
Sharmalm Mar 1, 2023
1969bb7
Merge remote-tracking branch 'refs/remotes/origin/main'
Sharmalm Mar 1, 2023
7febd5b
Merge branch 'lightningdevkit:main' into main
Sharmalm Mar 4, 2023
4364e18
correction
Sharmalm Mar 4, 2023
34d21dc
Merge branch 'lightningdevkit:main' into main
Sharmalm Mar 16, 2023
592f615
test function
Sharmalm Mar 16, 2023
c633d31
Merge remote-tracking branch 'refs/remotes/origin/main'
Sharmalm Mar 16, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lightning/src/routing/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,12 @@ where L::Target: Logger {
// around again with a higher amount.
if !contributes_sufficient_value || exceeds_max_path_length ||
exceeds_cltv_delta_limit || payment_failed_on_this_channel {
if let Some(first_hops) = first_hops { // for only first hop it will log feedback
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you'd use first_hop_targets rather than first_hops, there would be no need for this unwrapping.

Copy link
Contributor Author

@Sharmalm Sharmalm Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tnull But first_hop_targets doesn't have short_channel_id field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it does. It is a HashMap from PublicKey to ChannelDetails and the latter has short_channel_id.

for hop in first_hops {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this you're logging every single first hop, independently of whether it was excluded or not. You rather should just log the current candidate if it matches one of the first hops.

Copy link
Contributor Author

@Sharmalm Sharmalm Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where to find find current candidate in this code :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where to find find current candidate in this code :)

$candidate

let _channel_id = hop.channel_id;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably want to log via the short_channel_id rather than the channel_id to maintain uniformity with other logging outputs.

}
log_trace!(logger, "first Hop is excluded because unfulfilling condition");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to know a) what hop was excluded and b) why exactly it was excluded. To this end it might make sense to break up the ORed bools above into individual else if statements.

}
// Path isn't useful, ignore it and move on.
} else if may_overpay_to_meet_path_minimum_msat {
hit_minimum_limit = true;
Expand Down