Skip to content

Allow pay request and hop hints as input to queryroutes#2186

Closed
Bluetegu wants to merge 4 commits into
lightningnetwork:masterfrom
Bluetegu:hophints-queryroutes-2056
Closed

Allow pay request and hop hints as input to queryroutes#2186
Bluetegu wants to merge 4 commits into
lightningnetwork:masterfrom
Bluetegu:hophints-queryroutes-2056

Conversation

@Bluetegu
Copy link
Copy Markdown
Contributor

This addresses issue #2056

Two additional optional input flags were added to queryroutes cli command:
payment request (invoice)
hints (in json format)

This is useful mostly to compute routes to private destinations.

The current implementation assumes that the hints provide the last few steps towards the destination, i.e. that their final hop connects to the last node on the path. In particular, hints to cross a private edge in the middle of the network are not supported. This limitation can be lifted in future.

For testing a setup of A->B->C where B->C is private was used.

To test the use of an invoice as input to queryroutes, an invoice was generated on C. The invoice was paid using sendtoroute, with queryroutes feeding it with routes. The invoice was used as input to queryroutes. Here is a log of that command:

ubuntu@btc:~/go/dev/alice$ lncli-a queryroutes --pay_req="lnsb53220n1pd746evpp58njslvvsjfrwljr49aj4qtq8xcnghvppmv03jgmledgmlvlegdvqdzz2pexjanpw3jjq6twwehkjcm9yp6x7grzv5s8qctfvssxy7fqwdjkuer5daex7at5v5cqzysxqyjw5qrzjqf4996kagt4tku8fkpyp4acqr55e7ltmgypsjaamx7304trmhcqpyqq8gvqqqqgqqqqqqfcsqqqqqqqqjql7ga8cm06x68w5uw3glyy37xmqzpncqajprrnan8wes0fx0q8mf4s6x58dqys4fee6xmarm72f3ulhqdg6rg3aumxyg9x8pu78zwwccqnyvlyz" | lncli-a sendtoroute "3ce50fb1909246efc8752f65502c0736268bb021db1f19237fcb51bfb3f94358" -
{
"payment_error": "",
"payment_preimage": "94192fedd60bd0e31db8160aeaafac48cf79de5196af77e3bc58a6d2bde9734d",
"payment_route": {
"total_time_lock": 2016,
"total_fees": 10,
"total_amt": 5332,
"hops": [
{
"chan_id": 1540415790579712,
"chan_capacity": 1000000,
"amt_to_forward": 5322,
"fee": 10,
"expiry": 1872,
"amt_to_forward_msat": 5322000,
"fee_msat": 10000,
"pub_key": "026a52eadd42eabb70e9b0481af7001d299f7d7b41030977bb37a2faac7bbe0012"
},
{
"chan_id": 2043992116101120,
"chan_capacity": 5322,
"amt_to_forward": 5322,
"expiry": 1872,
"amt_to_forward_msat": 5322000,
"pub_key": "034a15c028970d4070c60f532018cfdb943b584ba267c83fe333ef1ad70b10feaf"
}
],
"total_fees_msat": 10000,
"total_amt_msat": 5332000
}
}

To test input of hints to queryroutes, a hint that includes the last B->C hop was used as input. This hint is identical to the hint generated when creating the invoice on C.

ubuntu@btc:~/go/dev/alice$ lncli-a queryroutes --hints='{"route_hints": [
{
"hop_hints": [
{
"node_id": "026a52eadd42eabb70e9b0481af7001d299f7d7b41030977bb37a2faac7bbe0012",
"chan_id": "2043992116101120",
"fee_base_msat": 10000,
"fee_proportional_millionths": 0,
"cltv_expiry_delta": 144
}
]
}
]}' "034a15c028970d4070c60f532018cfdb943b584ba267c83fe333ef1ad70b10feaf" 333
{
"routes": [
{
"total_time_lock": 2016,
"total_fees": "10",
"total_amt": "343",
"hops": [
{
"chan_id": "1540415790579712",
"chan_capacity": "1000000",
"amt_to_forward": "333",
"fee": "10",
"expiry": 1872,
"amt_to_forward_msat": "333000",
"fee_msat": "10000",
"pub_key": "026a52eadd42eabb70e9b0481af7001d299f7d7b41030977bb37a2faac7bbe0012"
},
{
"chan_id": "2043992116101120",
"chan_capacity": "333",
"amt_to_forward": "333",
"fee": "0",
"expiry": 1872,
"amt_to_forward_msat": "333000",
"fee_msat": "0",
"pub_key": "034a15c028970d4070c60f532018cfdb943b584ba267c83fe333ef1ad70b10feaf"
}
],
"total_fees_msat": "10000",
"total_amt_msat": "343000"
}
]
}

@Bluetegu Bluetegu force-pushed the hophints-queryroutes-2056 branch from aa0a9ab to 9f3d021 Compare November 15, 2018 14:49
@wpaulino wpaulino added rpc Related to the RPC interface routing payments Related to invoices/payments lncli P3 might get fixed, nice to have needs review PR needs review by regular contributors needs testing PR hasn't yet been actively tested on testnet/mainnet labels Nov 16, 2018
@Roasbeef Roasbeef requested a review from halseth December 11, 2018 01:22
Copy link
Copy Markdown
Contributor

@halseth halseth left a comment

Choose a reason for hiding this comment

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

Nice, I imagine this will be a very useful addition for users doing their own custom path finding! :D

Mostly style suggestions for now, looks pretty good 👍

Comment thread lnrpc/rpc.proto Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why 4?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Lol. Fixed

Comment thread routing/pathfind.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

needs a rebase to work with new method signature! 😀

Comment thread rpcserver.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should check if PaymentRequest is present before attempting to parse it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea. I was taking advantage of Decode handling correctly null inputs. Anyhow, I agree that this is convoluted. Rearranged code to make it more readable.

Comment thread rpcserver.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you can do this check before parsing the hop hints?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. Moved up (need parsing of payment first though)

Comment thread rpcserver.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this if is not necessary

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

Comment thread rpcserver.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if this is not set, shouldn't that be an error?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. Fixed. Now returning custom errors, to ensure the source of the error is known (i.e. that its from malformed hints).

@Bluetegu Bluetegu force-pushed the hophints-queryroutes-2056 branch 2 times, most recently from 153dedc to fa5d643 Compare December 12, 2018 06:12
@Bluetegu
Copy link
Copy Markdown
Contributor Author

Thanks for the review!
Rebased && Fixed issues
Please see inline

@Bluetegu Bluetegu force-pushed the hophints-queryroutes-2056 branch from fa5d643 to fd71fb0 Compare December 17, 2018 09:37
@Bluetegu Bluetegu force-pushed the hophints-queryroutes-2056 branch from fd71fb0 to 06b9522 Compare February 10, 2019 09:20
@halseth
Copy link
Copy Markdown
Contributor

halseth commented May 9, 2019

Needs a rebase once #3054 is in.

@Roasbeef
Copy link
Copy Markdown
Member

Superseded by #3911.

@Roasbeef Roasbeef closed this Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lncli needs review PR needs review by regular contributors needs testing PR hasn't yet been actively tested on testnet/mainnet P3 might get fixed, nice to have payments Related to invoices/payments routing rpc Related to the RPC interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants