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

routing: remove newRoute #6920

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

joostjager
Copy link
Contributor

@joostjager joostjager commented Sep 15, 2022

This PR is an alternative to #6912. It removes newRoute completely and forces pathfinding and route building to construct its own routes.

The advantage of this more direct approach is that there is no chance that newRoute yields a different route than what pathfinding worked with. Potential bugs are exposed and not "smoothed over" by newRoute. Also some of the steps to get to the final route don't need to be executed twice.

The change to BuildRoute also fixes edge cases with min_htlc where a valid route wasn't expressible through newRoute's input parameters.

// Allocate a list that will contain the unified policies for this
// route.
edges := make([]*unifiedPolicy, len(hops))
var (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of removing newRoute from BuildRoute, we can also skip that and re-route BuildRoute to pathfinding: #6921

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.

1 participant