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

Added feature route to self #1685

Closed
wants to merge 3 commits into from

Conversation

ZapUser77
Copy link

  • Added ability to find paths to self

  • Added unit tests for self routing

This allows queryroutes to find a route to self for manual balancing of channels.
And will lays the grounds for rebalance command, as well as auto-balancing channels.

* Added ability to find paths to self

* Added unit tests for self routing

This allows queryroutes to find a route to self for manual balancing of channels.
And will lays the grounds for rebalance command, as well as auto-balancing channels.
* Fixed infinite loop and formatting

Fixed infinite loop between FindPath and FindPaths
@ZapUser77
Copy link
Author

Fixed bad interaction between FindPath and FindPaths that wasn't previously detected in unit testing, which caused an infinite loop between them.

@halseth
Copy link
Contributor

halseth commented Aug 9, 2018

Blocked by #1321.

@joostjager
Copy link
Collaborator

In my opinion, the implementation as it is now contains assumptions that may not be true leading to suboptimal results. In particular the determination of the "middle node".

I am not sure either whether this is optimal performance-wise with the recursive findPath call.

I think the plan with a virtual self node as outlined in #1652 is more solid. Don't want to reopen the discussion, but just drop the reference for any reviewers to consider.

@ZapUser77
Copy link
Author

You don't have to find the "optimum" path on the first try, that's what "FindPaths" does.
FindPaths finds more than one path, then provides them in ascending order.
Virtual self simply does not work, as it doesn't look down paths where it encounters a node that's closer, which is must do to function properly.

@halseth halseth added needs review PR needs review by regular contributors and removed blocked labels Aug 10, 2018
@ZapUser77
Copy link
Author

Needs to have the code pieces moved around to compensate for the reverse pathfinding introduced in 1321.

@ZapUser77
Copy link
Author

Had to re-write to be compatible with new pathfind algo.
Closing this PR and opened #1751

@ZapUser77 ZapUser77 closed this Aug 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review PR needs review by regular contributors routing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants