routing+routerrpc+lnrpc: add option to use mc in queryroutes

[joostjager]

A boolean flag is added to the QueryRoutes rpc that allows feeding mission control probabilities into path finding.

This pr builds on #3298

[joostjager]

I just gave it a try. The first route had a temporary channel failure. Immediately afterwards the same route was returned by QueryRoutes, so I didn't see any change by mission control. Please tell me how to debug this, I'd be happy to help.

What I think happened is: It did work, but the reason the same route was returned was because it was still the route with the highest probability (although very low).

In SendPayment we apply a minimum probability. If the best route is below that, we give up.

Modified this PR to also apply that minimum probability to QueryRoutes with UseMissionControl is true.

[C-Otto]

Same issue. With the code from this PR I get a route, resulting in a temporary channel failure. Running QueryRoutes again returns the same route.

With my code, ignoring based on the channel failure, I get a different route when running QueryRoutes the second time.

[joostjager]

I failed to reproduce in the following set up on regtest: A -> B -> C. The B has 100k sat balance on the B->C channel.

• queryroutes for 200k sat from A -> C.
• sendtoroute using that route -> fails because TempChannelFailure
• queryroutes again for 200k from A -> C: unable to find path to destination.

This is the behavior that I'd expect.

Do you have more detailed steps? One thing about TempChannelFailure is that it only takes out the channel only for amounts that are at least the failing amount. So if it fails for 200k sat, it is only blacklisted for values >=200 k sat.

[joostjager]

Note to self: use_mc doesn't work fully until all mission control state is globalized. Currently the second try logic still resides in the payment session.

Needs to be rebased onto #3164

[joostjager]

Rebased onto first part of #3164

[cfromknecht]

Last 5 commits LGTM, just needs to rebased on final version of #3298 🚀

[Roasbeef]

Needs a rebase.

[joostjager]

rebased

[halseth]

couldn't we define this interface in routing?

[joostjager]

We could, but these are routerrpc's specific requirements for a mission control.

[cfromknecht]

yeah i think it makes sense to do it in routerrpc, so it can minimally define what it requires to operate

[halseth]

Yeah, makes sense. Could move it later if we need to mock it in the router package.

…

On Wed, Jul 17, 2019, 19:30 Joost Jager ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lnrpc/routerrpc/router_backend.go <#3221 (comment)>: > @@ -55,6 +55,22 @@ type RouterBackend struct { Tower routing.ControlTower } +// MissionControl defines the mission control dependencies of routerrpc. +type MissionControl interface { We could, but these are routerrpc's specific requirements for a mission control. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3221?email_source=notifications&email_token=AA4XLZEKFV44HUDGG3T3WDLP75JKHA5CNFSM4HZGJXA2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB6YDL7I#discussion_r304554381>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA4XLZG5OUEMZB5BRHLAXFLP75JKHANCNFSM4HZGJXAQ> .

[cfromknecht]

LGTM 🌋

[halseth]

LGTM

[halseth]

Needs rebase

[joostjager]

rebased