-
Notifications
You must be signed in to change notification settings - Fork 493
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
BOLT04: Atomic Multi-path Payments [feature 30/31] #658
base: master
Are you sure you want to change the base?
Conversation
I think that makes sense, as you can consider this to be a super set of the basic mpp in a sense. If we go in this direction, then should we switch to more of an "open coded" format for the AMP payload? This direction allows different use protocols to share common fields, but then create a blurry distinction between the allocated onion space amongst the various protocols. The other down side is that it consumes more type space, but with 64-bits available, that really isn't a concern IMO. |
If we go this route, then we may need to allocate an additional tag to indicate the context, so receivers know how to interpret this field. In the MPP case, it's just to be matched against the invoice, while in the AMP case, it may need to be passed to the receiver via some sort of hook for additional validation (the account ID had actually authorized a deposit, etc). |
|
||
The writer: | ||
- MUST NOT include `option_amp` for any non-final node. | ||
- if the sender has an invoice and `option_amp` feature was not set in the invoice: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should specify how this is to be communicated after #656 is merged.
04-onion-routing.md
Outdated
- if it does include `option_amp`: | ||
- MUST generate a random `stream_id` to be used on all HTLCs in the set. | ||
- MAY send more than one HTLC using the same `stream_id`. | ||
- MUST set the `share` values of all HTLCs such that their xor is a random |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MUST ensure all share
values are unique?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added SHOULD
since it's not a hard requirement, and receiver will accept it all the same. I don't have a strong opinion tho, if others think we should use MUST
s_n = s_1 ^ ... ^ s_n-1 ^ r | ||
``` | ||
|
||
If a partial payment fails, this process can be applied recursively to generate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Def an underrated feature of this scheme!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a problem with the receiver reading the share value, but then canceling the htlc with for example an invalid onion key error? To the sender is looks like a failure that may even have been caused by the second last node, but in reality the receiver has already obtained the root seed.
Could a proof of payment for atomic multi-path payments be achieved by locking the htlcs to two hashes (logical AND in the bitcoin script)? In order to settle such an htlc, the receiver needs to reveal both the preimage of the payment hash in the invoice and the preimage that was generated by the sender and embedded in the final onion payload. |
s_n = s_1 ^ ... ^ s_n-1 ^ r | ||
``` | ||
|
||
If a partial payment fails, this process can be applied recursively to generate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a problem with the receiver reading the share value, but then canceling the htlc with for example an invalid onion key error? To the sender is looks like a failure that may even have been caused by the second last node, but in reality the receiver has already obtained the root seed.
04-onion-routing.md
Outdated
The `amt_to_forward` value will be the amount for this partial payment only. The | ||
`option_amp` flag flag is a promise by the sender that the rest of the payment | ||
will follow in succeeding HTLCs with the same `stream_id`; we call these HTLCs, | ||
which that the same `stream_id`, an "HTLC set". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found set_id
more descriptive than stream_id
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this confusion is mostly because of the payment_id
being different from our usual invoice-based payment_id
. I think that stream_id
should be renamed payment_id
(it identifies this particular payment, which is sent in multiple parts), and payment_id
should be renamed something else (subscription_id
or something that makes sense for identifying a recurring payment?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found set_id more descriptive than stream_id.
I did too, reverted back to set_id
for now.
I think that this confusion is mostly because of the payment_id being different from our usual invoice-based payment_id. I think that stream_id should be renamed payment_id (it identifies this particular payment, which is sent in multiple parts), and payment_id should be renamed something else (subscription_id or something that makes sense for identifying a recurring payment?).
The intention behind calling it payment_id
was to keep some overlap with payment_hash
, since they reuse the same field in the invoice. I'm not totally convinced on subscription_id
, since that is only one of many use cases. Perhaps payment_addr
is better than payment_id
? In many ways it does behave more like an addr than an id
- MUST set the `payment_id` of each HTLC to the `payment_hash` in the | ||
invoice. | ||
- otherwise: | ||
- MUST set the `payment_id` of each HTLC to zero. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or define it as a separate tlv type? There may be some overlap with the random identifier generated by the receiver that was discussed before for regular payments and mpp. Both are ids generated by the receiver and both are not used to lock the htlc onto.
04-onion-routing.md
Outdated
- otherwise: | ||
- MAY fulfill the `i-th` HTLC in the set using `p_i`. | ||
- otherwise: | ||
- MUST fail an HTLC in set if its `cltv_expiry` elapses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be failed earlier than that? In case of an (amp) hodl invoice, an application assumes that when the invoice is marked as accepted (we know the root seed, but haven't pulled yet), the invoice cltv expiry condition holds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure exactly what you meant here, can you elaborate on "failed earlier"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example:
- current height: 100
- (hodl) invoice cltv delta: 40, amt: 100
- first htlc for 50 sats comes in with expiry 140 -> accepted as partial payment
- new block comes in
- second htlc for 50 sats comes in with expiry 141 -> accepted as partial payment
The hodl invoice should now move to the accepted state, because enough has been paid. We'd send the accepted
event to rpc subscribers. But at that point, there are only 39 blocks left before the first htlc expires. The subscriber will probably assume that when the event comes in, the final cltv delta of 40 is still met.
04-onion-routing.md
Outdated
The `amt_to_forward` value will be the amount for this partial payment only. The | ||
`option_amp` flag flag is a promise by the sender that the rest of the payment | ||
will follow in succeeding HTLCs with the same `stream_id`; we call these HTLCs, | ||
which that the same `stream_id`, an "HTLC set". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this confusion is mostly because of the payment_id
being different from our usual invoice-based payment_id
. I think that stream_id
should be renamed payment_id
(it identifies this particular payment, which is sent in multiple parts), and payment_id
should be renamed something else (subscription_id
or something that makes sense for identifying a recurring payment?).
@@ -261,6 +268,197 @@ The reader: | |||
|
|||
The requirements for the contents of these fields are specified [above](#legacy-hop_data-payload-format). | |||
|
|||
## Atomic Multi-path Payments | |||
|
|||
If the final node receives an onion packet with `option_amp` field, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having both option_amp
and option_mpp
is very confusing (but hopefully the two proposals merge at least partially). I think that the main feature this proposal adds to rusty's MPP proposal is the spontaneous part (because option_mpp
is currently also atomic - controlled by the recipient). Maybe the naming should reflect that (option_spontaneous_mpp
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might even make sense that this PR mutates in a specification of spontaneous payments (not invoice-based), encompassing both the multi-part aspect and the non multi-part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the main feature this proposal adds to rusty's MPP proposal is the spontaneous part
I disagree here, the primary goal here is not reuse payment hashes for better privacy:
- intermediaries can't correlate subpayments
- forwarding a payment doesn't leak anything about the invoice being paid. since all identifiable information is enclosed only for the receiver's eyes (if the invoice is public)
- eliminates known probing vectors (of the payment hash) since it is never exposed directly over the network
The spontaneous + recurring pieces are useful side-effects of the sender generating the required randomness.
- SHOULD choose a unique child_index_i for each HTLC. | ||
- MUST derive the `payment_hash` for an HTLC using `amp_child(r, child_index_i)`. | ||
- if the invoice specifies a non-zero `amount`: | ||
- MUST set `total_msat` to `amount`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this allow over-payment (as specified for standard invoice payments)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes probably, i think it should mirror whatever is finalized in #643
payment. | ||
|
||
None of the requirements enforce that more than one HTLC is sent, permitting the | ||
base case of 1 HTLC to function as a standalone spontaneous payment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏 I think this is important and this is why I view this work more as a "spontaneous payments" overall feature
Great stuff @cfromknecht I think this adds very valuable features. One quick thought about share failures. Not sure it can be exploited, but just putting it out there to make sure it's obvious to everyone. |
Okay I tried to respond to all of the immediately actionable comments (lmk if i've missed anything). The major things that still seem in flux:
Yes I think it is possible, but also requires the entire path to be upgraded before it can be used, unlike the current proposal which only requires sender/receiver to upgrade. If one really wants proof of payment tho, it's probably better to just use #643, since by adding the payment hash in all scripts along the path you lose decorrelation (and reveal that you're using AMP).
If the receiver gets a subpayment and then cancels it back it can learn the root seed, but also won't have a payment to settle. At worst, the sender may try another shard (or split further into subshards), but the total amounts should never exceed what was already presented. In this sense it behaves like base amp, except that only the receiver can initiate the settlement (if they aren't colluding w/ intermediaries), instead of any intermediary who already knows the preimage to a subpayment.
Related to the above, if the receiver is actually in control of an intermediate node then it can perform a wormhole attack. But idt it is any different from what can already be done (for regular payments or base amp) when using payment hashes, and will eventually be fixed by moving to DLOG challenges :) |
OTOH I would propose that we plan to transition all payment types over to mpp or amp, and deprecate the existing single-shot payments. Both proposals support sending 1 shard, and both schemes address known issues in the current single-shot payment flow. In that case a reasonable split of the tlv records might be:
|
Based on discussion in #643, mpp will include a receiver-generated nonce rather than a sender-generated one. Therefore the prior split would be better defined as:
|
@@ -248,6 +248,13 @@ This is a more flexible format, which avoids the redundant `short_channel_id` fi | |||
1. type: 6 (`short_channel_id`) | |||
2. data: | |||
* [`short_channel_id`:`short_channel_id`] | |||
1. type: 10 (`option_amp`) | |||
2. data: | |||
* [`32*byte`:`payment_id`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For recurring payments, the idea with AMP is that payment_id
can be used as a type of 'deposit box', similar to a traditional bank account number.
But this field is also used to prevent multiple people trying to pay to the same invoice simultaneously from disturbing each others sets.
It seems that both uses of the same field are mutually exclusive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, clarified offline. That is what set_id
is for
Hi all, decided to submit this draft concurrently with @rustyrussell's Base AMP since there is likely a bit of overlap, and comments can likely be shared between the two. The structure of the proposal and requirements is based on #643 to make that process simpler. The chosen feature bits are just temporary.
Background
This proposal outlines the requirements for Atomic Multi-path Payments originally outlined in @Roasbeef and I's email to lightning-dev. The proposal has also been referred to as OG AMP, or Moon AMP due to the moon-math involved :)
The core of the proposal is mostly unchanged, though I will highlight some distinctions:
hop_payload
for the sphinx onion #619total_msat
(as in Base AMP) instead of a transmitting the number of shares. This is more flexible when sending partial payments adaptively, since the required number of partial payments may not be known upfront, while the total amount being paid should be known. Committing to the total amount being paid also eliminates weaknesses in zero-value invoices, or any scenario involving overpayment.set_id
Summary
One of the biggest distinctions (some would argue drawback) from Base AMP is that the sender does not receive a preimage as a result of a successful payment. However, the lack of a "proof" of payment unlocks a range of novel features:
set_id
. Multiple, concurrent payments can be made to the same invoice and properly reconstructed using the enclosedset_id
.Open Questions
option_amp
are to be included in other proposals as well. Should we reuse thetotal_msat
field fromoption_mpp
, which can make use of the truncated encoding? Similarly, theset_id
is analogous to the proposal to generically include a nonce in all payments, should that be it's own record? The signaling of which feature you are attempting to use may be more complex, but it's RISC-ier in some sense.Feedback appreciated!