Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Misc routing optimization #2803
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
Misc routing optimization #2803
Changes from all commits
df9c15d
3e90240
98f9e8b
9566c27
bed1fb0
f689e01
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
#[allow(unused_comparisons)]
attribute is used to suppress warnings about comparisons that are always true or false. This is a code smell and may indicate that the logic could be simplified or that the comparisons are unnecessary.Review the logic involving
amount_to_transfer_over_msat
,htlc_minimum_msat
, and$next_hops_path_htlc_minimum_msat
to determine if the comparisons are necessary and remove them if they are not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The handling of first hop targets within the routing algorithm is complex and should be carefully reviewed to ensure correctness.
Review the logic for handling first hop targets to ensure correctness. Consider adding more comments to explain how first hop targets are processed in the routing algorithm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth adding
debug_assert
s or similar checks to make sure we don't deviate from this assumption?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it would be, but I'm not sure I know how to write such an assertion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we could check that if one field is updated, all others are too? But maybe not worth it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm not sure how to check on a per-field basis. The likely failure case is we add another field and fail to update it when relevant, but I'm not aware of a way to iterate over the fields of a struct?