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

Reproduce pay route crash #3633

Merged

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Apr 8, 2020

Reproduces the crash in #3607 which @niftynei fixed in #3630

@rustyrussell
Copy link
Contributor Author

Rebase on master, which contains the fix this tests.

@niftynei
Copy link
Collaborator

niftynei commented Apr 9, 2020

seems like you've got some whitespace issues in the python and a timing issue. lgtm otherwise.

>           raise RpcError(method, payload, resp['error'])
E           pyln.client.lightning.RpcError: RPC call failed: method: invoice, payload: {'msatoshi': 1000, 'label': 'lbl', 'description': 'desc', 'exposeprivatechannels': '103x2x1'}, error: {'code': 902, 'message': 'None of those hints were suitable local channels'}

Copy link
Collaborator

@niftynei niftynei left a comment

Choose a reason for hiding this comment

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

spelling nit

tests/test_pay.py Outdated Show resolved Hide resolved
@rustyrussell rustyrussell force-pushed the reproduce-pay-route-crash branch 3 times, most recently from 1e8d77f to bd61291 Compare April 14, 2020 05:21
@rustyrussell
Copy link
Contributor Author

Trivial rebase

This makes testing easier, and makes sense: lightningd might not
*know* about other connected channels, depending on gossip, but if the
user specifies it we should obey it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON: `invoice` `exposeprivatechannels` now includes explicitly named channels even if they seem like dead-ends.
When route returns a result which is too expensive, we try to figure out which
hop is most expensive to exclude it for next time.

If it's a single-hop route, we don't count it, since the first hop is free.
That's not usually a problem, since single-hop routes can't exceed our limits
(they're always "free"!).

But if we are using a routehint, the total cost could exceed our limits,
even if the start of the routehint is a single hop away.

This reproduces that test case.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
Copy link
Contributor Author

... and added Changelog into commit msg for deadend heuristic and expostprivatechannels.

@niftynei
Copy link
Collaborator

ACK 79c5e0a

@niftynei niftynei merged commit 1dc281c into ElementsProject:master Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants