routing: multi-part mpp send #3967

joostjager · 2020-01-28T16:24:38Z

This PR enables multi-part payment sending. If lnd isn't able to find a single route that can carry the full payment amount, it will recursively split into smaller partial payments that are all collected by the receiver. Once the receiver has htlcs pending that together add up to the full amount, they will settle them all and thereby reveal the preimage to the sender.

The minimum partial payment size is currently fixed at 10k sats (unless the remainder is less than that).

Multi-part sending is controlled via a new payment rpc parameter max_parts. This parameter indicates what the maximum number of partial payments is that may be send. If set to one, there will be no splitting.

The parameter is also exposed on lncli. To send a multi-part payment, use:

lncli payinvoice --max_parts 3 <payreq>

The output from sendpayment/payinvoice/trackpayment/listpayments can be difficult to interpret for mpp payments. A visualization of the payment process can be created using https://github.com/joostjager/lnd-tools/tree/master/timingdiagram

Fixes #3940.

joostjager · 2020-01-28T18:58:14Z

Currently running into an endless loop because of #3691

This is probably more important to fix with mpp because channels are spent down further.

joostjager · 2020-02-15T16:36:30Z

Todo: track fee limit on the payment level

Roasbeef · 2020-03-13T00:52:45Z

Rebase to remove the first merged PR?

Roasbeef · 2020-03-13T00:52:57Z

Also to rebase on top of the latest version of the MPP shard send PR.

joostjager · 2020-03-13T09:03:30Z

Rebased. But this PR isn't ready for review, except to observe the general idea for mpp splitting.

joostjager · 2020-03-17T20:36:30Z

Gets into endless loop if receiver has not enough inbound capacity. Will repro in the integrated routing test. Thanks for bringing up the scenario @Roasbeef

With mpp it isn't possible anymore for findPath to determine that there isn't enough local bandwidth. The full payment amount isn't known at that point. In a follow-up, this payment outcome can be reintroduced on a higher level (payment lifecycle).

Preparation for more test coverage of payment session. The function findPath now has the call signature of the former findPathInternal function.

Cover the payment session in the integrated routing test as a preparation for executing the upcoming mpp split logic as part of the test.

…oute

Modifies the payment session to launch additional pathfinding attempts for lower amounts. If a single shot payment isn't possible, the goal is to try to complete the payment using multiple htlcs. In previous commits, the payment lifecycle has been prepared to deal with partial-amount routes returned from the payment session. It will query for additional shards if needed. Additionally a new rpc payment parameter is added that controls the maximum number of shards that will be used for the payment.

It can happen that the receiver times out some htlcs of the set if it took to long to complete. But because the sender's mission control is now updated, it is worth to keep trying to send those shards again.

Co-authored-by: Johan T. Halseth <johanth@gmail.com>

joostjager · 2020-04-09T06:27:52Z

Rebased

halseth · 2020-04-09T09:13:16Z

Itest flake is caused by async hand-off of the htlc to the link and its interaction with the next pathfinding round.

Should we create an issue to track this? The sync handoff is also assumed by the payment loop, so we should probably fix this at one point...

joostjager · 2020-04-09T09:19:47Z

routing/payment_session.go

+		// don't have enough bandwidth to carry the payment. New
+		// bandwidth hints are queried for every new path finding
+		// attempt, because concurrent payments may change balances.
+		bandwidthHints, err := p.getBandwidthHints()


Moving the comment here, to prevent a very long list of "top-level" comments in github.

Should we create an issue to track this? The sync handoff is also assumed by the payment loop, so we should probably fix this at one point...

I don't know if it is really an issue or merely an optimization. Although the boundary between the two is vague. I did think a bit about solutions. Maybe it is possible to hand off the htlc to the link directly, cut out the switch and mailbox. Or does the switch serve an essential purpose for locally initiated payments?

If it is something we don't want to fix, then the whole launch then collect logic in the payment loop can be greatly simplified I think.

I agree it is an optimization, since the pathfinding will just be retried, but would be nice to move the assumptions to either extreme: assume the bandwidth hints tell you nothing about in-flight shards (and we could pass in a list of active shards, that can also help pathfinding on non-local channels) or assume they tell you everything you need to know (the current assumption).

Maybe it is possible to hand off the htlc to the link directly, cut out the switch and mailbox. Or does the switch serve an essential purpose for locally initiated payments?

Yeah, didn't look into how to actually solve it, but I was think something like an ACK channel to wait on.

Issue created: #4181

halseth

LGTM 🚂 Only lingering question is the htlcs vs shards naming, but I don't have a strong opinion about it.

halseth · 2020-04-09T09:19:48Z

routing/router.go

@@ -1624,6 +1624,10 @@ type LightningPayment struct {
 	// understand this new onion payload format, then the payment will
 	// fail.
 	DestCustomRecords record.CustomSet
+
+	// MaxHtlcs is the maximum number of partial payments that may be use to


nit: s/use/used

Roasbeef · 2020-04-10T01:50:55Z

Only lingering question is the htlcs vs shards naming, but I don't have a strong opinion about it.

Yeah even during offline discussion I find myself having to say "channel update max htlc" in order to disambiguate.

Roasbeef

LGTM 💍

I think this is ready to land, we can iron out the terminology (max htlc vs max splits vs max shards, etc) in the RC phase. I'll rebase my follow up PR (to raise the invoice limit).

Roasbeef · 2020-04-10T01:51:57Z

cmd/lncli/cmd_pay.go

@@ -46,6 +46,13 @@ var (
 			"<hex_value>,.. For example: --data 3438382=0a21ff. " +
 			"Custom record ids start from 65536.",
 	}
+
+	maxHtlcsFlag = cli.UintFlag{
+		Name: "max_htlcs",


max_splits could also be another option here.

In this commit, we remove the restriction surrounding the largest invoices that we'll allow a user to create. After lightningnetwork#3967 has landed, users will be able to send in _aggregate_ a payment larger than the current max HTLC size limit in the network. As a result, we can just treat that value as the system's MTU, and allow users to request payments it multiples of that MTU value. A follow up to this PR at a later time will also allow wumbo _channels_. However, that requires us to tweak the way we scale CSV values, as post wumbo, there is no true channel size limit, only the _local_ limit of a given node. We also need to implement a way for nodes to signal to other nodes their accepted max channel size.

joostjager force-pushed the mpp-send branch from 24a2511 to f6ba9cc Compare January 28, 2020 18:39

joostjager force-pushed the mpp-send branch from f6ba9cc to c526414 Compare January 29, 2020 15:07

Roasbeef added mpp payments Related to invoices/payments routing labels Feb 3, 2020

joostjager mentioned this pull request Feb 26, 2020

routing+channeldb: mpp bucket structure #4000

Merged

Roasbeef added this to the 0.10.0 milestone Feb 26, 2020

joostjager force-pushed the mpp-send branch 2 times, most recently from 160da22 to af386f9 Compare March 9, 2020 07:29

Roasbeef mentioned this pull request Mar 13, 2020

MPP: Enable MultiPathPayments for payment lifecycle #3970

Merged

2 tasks

Roasbeef mentioned this pull request Mar 13, 2020

rpc: allow wumbo invoices #4075

Merged

joostjager force-pushed the mpp-send branch from af386f9 to 13b04ea Compare March 13, 2020 08:59

joostjager force-pushed the mpp-send branch 7 times, most recently from e6dddd4 to 8ecc22b Compare March 17, 2020 20:29

joostjager and others added 13 commits April 9, 2020 08:20

routing: use routingGraph interface in payment session

47f9c1c

Preparation for more test coverage of payment session. The function findPath now has the call signature of the former findPathInternal function.

routing/test: use payment session in integrated routing test

b13616a

Cover the payment session in the integrated routing test as a preparation for executing the upcoming mpp split logic as part of the test.

routing/test: expose full list of htlc attempts

3c6e461

routing/test: use fixed identifiers in test graph

0f8eb80

routing/test: return pathfinding error

e5c7e9a

routing: distinguish between receiver amount and total amount in newR…

46f5fc7

…oute

lncli: expose max payment htlcs

f5c3f93

routing: continue trying after mpp timeout

110c99f

It can happen that the receiver times out some htlcs of the set if it took to long to complete. But because the sender's mission control is now updated, it is worth to keep trying to send those shards again.

itest: create mpp test context

35ebfcd

itest: add multi-part payment test

af14f2e

Co-authored-by: Johan T. Halseth <johanth@gmail.com>

itest: log failure time

ef38f8f

joostjager force-pushed the mpp-send branch from 9025e6e to ef38f8f Compare April 9, 2020 06:23

joostjager commented Apr 9, 2020

View reviewed changes

halseth approved these changes Apr 9, 2020

View reviewed changes

Roasbeef approved these changes Apr 10, 2020

View reviewed changes

Roasbeef merged commit 1354a46 into lightningnetwork:master Apr 10, 2020

joostjager changed the title ~~routing: multi-shard mpp send~~ routing: multi-part mpp send Apr 27, 2020

Pantamis mentioned this pull request Apr 30, 2020

Multi-Part MPP send support joule-labs/joule-extension#251

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

routing: multi-part mpp send #3967

routing: multi-part mpp send #3967

joostjager commented Jan 28, 2020 •

edited

Loading

joostjager commented Jan 28, 2020

joostjager commented Feb 15, 2020

Roasbeef commented Mar 13, 2020

Roasbeef commented Mar 13, 2020

joostjager commented Mar 13, 2020

joostjager commented Mar 17, 2020

joostjager commented Apr 9, 2020

halseth commented Apr 9, 2020

joostjager Apr 9, 2020

halseth Apr 9, 2020

halseth Apr 9, 2020

joostjager Apr 13, 2020

halseth left a comment

halseth Apr 9, 2020

Roasbeef commented Apr 10, 2020

Roasbeef left a comment

Roasbeef Apr 10, 2020

routing: multi-part mpp send #3967

routing: multi-part mpp send #3967

Conversation

joostjager commented Jan 28, 2020 • edited Loading

joostjager commented Jan 28, 2020

joostjager commented Feb 15, 2020

Roasbeef commented Mar 13, 2020

Roasbeef commented Mar 13, 2020

joostjager commented Mar 13, 2020

joostjager commented Mar 17, 2020

joostjager commented Apr 9, 2020

halseth commented Apr 9, 2020

joostjager Apr 9, 2020

Choose a reason for hiding this comment

halseth Apr 9, 2020

Choose a reason for hiding this comment

halseth Apr 9, 2020

Choose a reason for hiding this comment

joostjager Apr 13, 2020

Choose a reason for hiding this comment

halseth left a comment

Choose a reason for hiding this comment

halseth Apr 9, 2020

Choose a reason for hiding this comment

Roasbeef commented Apr 10, 2020

Roasbeef left a comment

Choose a reason for hiding this comment

Roasbeef Apr 10, 2020

Choose a reason for hiding this comment

joostjager commented Jan 28, 2020 •

edited

Loading