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

QueryRoutes always returns same route #3206

Closed
C-Otto opened this issue Jun 15, 2019 · 13 comments
Closed

QueryRoutes always returns same route #3206

C-Otto opened this issue Jun 15, 2019 · 13 comments

Comments

@C-Otto
Copy link
Contributor

C-Otto commented Jun 15, 2019

I'm using 0.7.0-beta-rc1 with the latest GRPC files. For my rebalance script at https://github.com/C-Otto/rebalance-lnd I need to try a second route if the first does not work out due to technical reasons (not enough funds, most likely). It's also possible that a route looks fine, but is considered too expensive (so no payment is attempted).

With #2497 and 7a5bd29 it's not possible to request more than a single route.

However, in my experiments QueryRoutes always returns the same route, even if sending a payment fails. I'd prefer lnd to give another route when running the query a second time.

A crucial contributor to this might be that the requested/returned route is modified before sending the payment (by adding a lost hop routing back to sending node, to rebalance a channel). If there are heuristics that (temporarily) block hops, these might have to be tuned.

The discussion in #2497 mentions "a follow up PR will add any source to destination queries to QueryRoutes", which sounds promising. Please point me to any information regarding this, if it exists.

@bitromortac
Copy link
Collaborator

In my rebalancing script (https://github.com/bitromortac/lndmanage), I'm checking the error code for the failed channel id and then put it on the channel blacklist using the "ignored_edges" field. In that way lnd returns an alternative route.

@willcl-ark
Copy link

I know this is not a solution, but it is my understanding that the rationale for this change is that you are encouraged to use the failure as something useful which you can feed to your routing algorithm, to get "better" queries in the future, similar to what @bitromortac says he is doing above.

@C-Otto
Copy link
Contributor Author

C-Otto commented Jun 16, 2019

@willcl-ark I don't have a routing algorithm, I'd like lnd to figure out the details of how to reach the destination.

@alexbosworth
Copy link
Contributor

Currently queryRoutes doesn't use the new mission control system, which would otherwise inform its subsequent routes

The old result was a partial solution but it also wouldn't necessarily return substantially different routes, even if you saw 100 routes they could all potentially be failing in the same way

@C-Otto
Copy link
Contributor Author

C-Otto commented Jun 16, 2019

I think using the ignored_nodes and ignored_edges feature of QueryRoutes is the best option. @alexbosworth is there an issue that connects QueryRoutes and the mission control system? If not, I'd like to keep this open (or create a more specific issue).

@alexbosworth
Copy link
Contributor

I don't know of such an issue

sendtoroute itself is connected to mission control, so the errors are being recorded to inform the pathfinding of normal payments

I would definitely recommend using ignored_nodes and ignored_edges in conjunction with the getRoutes API as a workaround. I would recommend using the routerrpc sendtoroute RPC to get structured errors which can identify the failing hop and the failing reason.

On intermediate node failures, add the failed hop as an ignored edge to subsequent calls to queryRoutes. In addition you could try adding more ignores for other errors that are not related to the specific hop. For FeeInsufficient, IncorrectCltvExpiry failures, add the forwarding nodes to ignore nodes. On TemporaryNodeFailure errors add the hop forward direction node to ignore nodes, and on the UnknownNextPeer error add the next hop after the failed hop to ignore edges.

@C-Otto
Copy link
Contributor Author

C-Otto commented Jun 16, 2019

Thank you, I'll close this. Please have a look at the new issue.

@C-Otto C-Otto closed this as completed Jun 16, 2019
@C-Otto
Copy link
Contributor Author

C-Otto commented Jun 16, 2019

@alexbosworth could you point me to the structured error information you mentioned? I'm only able to find a string that I'd need to parse. https://github.com/lightningnetwork/lnd/blob/master/lnrpc/routerrpc/router.proto#L145

@alexbosworth
Copy link
Contributor

The Failure message is the structured error, it shouldn't require string parsing

@C-Otto
Copy link
Contributor Author

C-Otto commented Jun 16, 2019

Oh. Thanks :)

@joostjager
Copy link
Contributor

If the goal is to replicate the previously available num_routes parameter, there is no need for mission control or sendtoroute.

It would involve implementing the k-shortest path algorithm client-side by using a custom route source and the ignore lists. It should be possible to port over the now removed k-shortest code from lnd.

@C-Otto
Copy link
Contributor Author

C-Otto commented Jun 18, 2019

I see your point. Instead of dealing with graph theory in the small script, I'd prefer to use as much of lnd's internal logic as possible. Ignoring failing edges (or having that feature in QueryRoutes) seems to be easier.

@joostjager
Copy link
Contributor

It is a question of who does what. K-shortest proved to be quite intensive in maintenance, at the expense of development of features that we consider more important. The available time is fixed, so we sometimes need to make choices that won't be ideal for everyone.

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

No branches or pull requests

5 participants