htlcswitch: hodl invoice #2022
htlcswitch: hodl invoice #2022
Conversation
joostjager
force-pushed
the
holdinvoice
branch
8 times, most recently
from
32fe441
to
df51265
Compare
joostjager
force-pushed
the
holdinvoice
branch
3 times, most recently
from
7ceb180
to
36ef752
Compare
htlcswitch/link.go
Outdated
| preimage := invoice.Terms.PaymentPreimage | ||
| if !bytes.Equal(preimage[:], unknownPreimage[:]) { | ||
| err = l.cfg.Registry.AcceptInvoice( |
There was a problem hiding this comment.
@cfromknecht, I worked your PoC into this PR. You wrote:
one thing we may consider is logically grouping
switchSettledPacketsfrom switchPackets. both can be forwarded in a single call to forwardBatch, but we should post-process accepting the switchSettledPackets after they have been forwarded. rationale here is that AcceptInvoice will fire a notification, which if instantly acted upon, could result in the circuit not being opened when the switch tries to lookup the circuit
delaying until after the circuits have been atomically opened and committed will ensure this case doesn't arise
Is this still how we want to do this? Or is it better to make the AcceptInvoice call inside the switch?
There was a problem hiding this comment.
Thanks for the reminder on this! Yes I think we'd still want to do that. I'd prefer to have it done in the link to keep the circuit map operations as isolated as possible. It would need to be done after forwardPackets is called, and also check that the link isn't exiting by checking quit preemptively before accepting
htlcswitch/link.go
Outdated
| preimage := invoice.Terms.PaymentPreimage | ||
| if !bytes.Equal(preimage[:], unknownPreimage[:]) { | ||
| err = l.cfg.Registry.AcceptInvoice( |
There was a problem hiding this comment.
And do we need a if fwdPkg.State == channeldb.FwdStateLockedIn check here?
joostjager
force-pushed
the
holdinvoice
branch
2 times, most recently
from
aa92f9a
to
e2ad744
Compare
joostjager
force-pushed
the
holdinvoice
branch
from
e2ad744
to
a792776
Compare
There was a problem hiding this comment.
@joostjager sorry for the delay, just found a bunch of pending comments I thought I had shared.
This PR looks pretty complete to me apart from some higher level changes we've discussed elsewhere. Feel free to ignore any comments that may be irrelevant now, i'll just leave all them here
htlcswitch/switch.go
Outdated
| @@ -1587,7 +1599,8 @@ out: | |||
| // packet concretely, then either forward it along, or | |||
| // interpret a return packet to a locally initialized one. | |||
| case cmd := <-s.htlcPlex: | |||
| cmd.err <- s.handlePacketForward(cmd.pkt) | |||
| cmd.err <- s.handlePacketForward(cmd.pkt, | |||
| &lnwire.FailPermanentChannelFailure{}) | |||
There was a problem hiding this comment.
should probably pass nil instead here. resolutions should never go through the htlcPlex, so we shouldn't ever hit the packet.isResolution case.
channeldb/invoices.go
Outdated
| return err | ||
| } | ||
|
|
||
| if err := invoices.Put(invoiceNum[:], buf.Bytes()); err != nil { |
There was a problem hiding this comment.
similar comment here wrt returning Put
There was a problem hiding this comment.
Combining with the return statement would still return the nextAddSeqNo in the error case, even though it isn't committed to the db. I think it is better to keep it deterministic by returning 0.
htlcswitch/link.go
Outdated
| preimage := invoice.Terms.PaymentPreimage | ||
| if !bytes.Equal(preimage[:], unknownPreimage[:]) { | ||
| err = l.cfg.Registry.AcceptInvoice( |
There was a problem hiding this comment.
Thanks for the reminder on this! Yes I think we'd still want to do that. I'd prefer to have it done in the link to keep the circuit map operations as isolated as possible. It would need to be done after forwardPackets is called, and also check that the link isn't exiting by checking quit preemptively before accepting
|
@cfromknecht I park the link/switch comments for now. First define the rpc interface for this in #2263, implement that (new invoice states and events) and then continue with the switch/link modifications. |
joostjager
force-pushed
the
holdinvoice
branch
from
a792776
to
354be91
Compare
joostjager
force-pushed
the
holdinvoice
branch
4 times, most recently
from
0cad4a6
to
7fe10e7
Compare
joostjager
force-pushed
the
holdinvoice
branch
from
7fe10e7
to
ce048c8
Compare
In further commits the behaviour of invoice registry becomes more intrinsically connected to the link. This commit prepares for that by allowing link and registry to be tested as a single unit.
This commit detaches signaling the invoice registry that an htlc was locked in from the actually settling of the htlc. It is a preparation for hodl invoices.
This commit modifies the invoice registry to handle invoices for which the preimage is not known yet (hodl invoices). In that case, the resolution channel passed in from links and resolvers is stored until we either learn the preimage or want to cancel the htlc.
|
@cfromknecht ptal |
There was a problem hiding this comment.
@joostjager excellent work on this feature, excited that this also positions us nicely for integrating multi-path payments in the near term! LGTM 💰
|
You mention that however in the invoices.proto there is no rhash returned in the addHoldInvoiceResponse should we be expected to decode the payment_request to get this? |
|
For AddHoldInvoice, you pass in the hash yourself. Returning it in the response wouldn't be new information. |
|
oh I see so the rhash is the hash you pass, makes sense thanks |
|
what is the reason this requites a special make tag? will this be removed in the future so it is automatically part of the default LND? |
|
Yes, we will remove the build tag in the future and make it part of default |
|
Awesome thanks
…Sent from my iPhone
On Jun 13, 2019, at 15:20, Joost Jager ***@***.***> wrote:
Yes, we will remove the build tag in the future and make it part of default lnd. Eventually we want to also start removing redundant calls from the main rpc server.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Reading through this pull request and all the comments, it's hard for me to figure out if the open issue related to automatic cancelling of the accepted but unsettled HOLD invoice has been addressed. Has it? If so, how does it actually work? Is there a new htlc that is created that just reverts back to the original channel state and everyone in the route would rather do that than deal with a channel closure? At what time does the automatic cancellation occur? It seems like the default Also, has the other open issue related to the automatic cancellation of all invoices based on the |
|
auto canceling the held htlc has been implemented yes |
In this PR, a new feature called "hold invoice" is added to
lnd. A hold invoice is new type of invoice that triggers a different flow at the receiver end.Instead of immediately locking in and settling the htlc when the payment arrives, the htlc for a hold invoice is only locked in and not yet settled. At that point, it is not possible anymore for the sender to revoke the payment, but the receiver still can choose whether to settle or cancel the htlc and invoice.
From the sender perspective, a hold invoice payment request looks identical to a regular payment request. There is no way for the sender to know when a hold invoice is paid to.
Hold invoices enable several new use cases:
Receiver gains the ability to check a condition before settling. If the condition is not met, the invoice can still be canceled and a potential refund does not need to be given.
Example 1: web shop performing an inventory check. If the item is no longer in stock at the time the invoice is paid, the payment can be canceled.
Example 2: exchange accepting spontaneous LN deposits (building on top of [WIP] multi: add new draft key send mode for spontaneous payments #2455). The account to credit is specified as user data in the EOB and the exchange first checks whether this account is valid before settling the incoming htlc.
A service can use a hold invoice as an insurance against bad behaviour by its users, similar to a fidelity bond. The user gains access to the service by paying the hold invoice. When the session is over, the service cancels the invoice and the user regains its funds. The only cost for the user has been the time value of those funds. When the user misbehaves during the session, the service can settle the hold invoice as a penalty.
Example 3: prevention of dos attacks
A hold invoice may also be created if the preimage of the payment hash is not yet known to the receiver. Receiver can accept the payment and perform a follow up action to obtain the preimage. Only once the preimage is obtained, the receiver can settle the payment.
Example 4: pizza delivery service wanting to offer "no pizza no pay". A customer generates a random preimage and adds the hash of it to the pizza order. The delivery service creates a hold invoice tied to this hash and returns the invoice to the customer. Customer pays the invoice. At this point, the delivery service can accept the payment, but not settle it because it doesn't know the preimage yet. Then the driver goes out to deliver the pizza. At the door he asks the customer for the preimage, verifies that it is indeed the preimage of the hold invoice for this order and hands over the pizza. Verification can even be done offline. The delivery service can then settle the invoice when the driver returns with the preimage. Atomic pizza swap completed.
Example 5: parcel delivery service. Similar to pizza, but here the payer and recipient are different people. Payer pays the hold invoice of the delivery service and gives the preimage to the recipient of the parcel.
Example 6: taking this one step further in a three party example: a customer, a web shop and a delivery service. Customer generates preimage, customer sends hash of preimage to web shop, web shop creates hold invoice, customer pays hold invoice, web shop sends hash to delivery service, delivery service creates hold invoice for delivery cost, web shop pays delivery hold invoice, delivery service delivers to customer in exchange for preimage, delivery service settles delivery invoice with preimage, web shop settles customer invoice with preimage.
[In this example there is a risk associated with reuse of the payment hash. Intermediate nodes on both paths may shortcut the preimage and disallow the webshop from receiving any money.]
Example 7: using hold invoice to execute a trustless atomic off-chain to on-chain swap. User A creates a secret preimage and passes the hash of it to user B. User B creates a hold invoice tied to this hash. User A pays the invoice. User B publishes an on-chain htlc tied to the same hash. When user A sweeps the on-chain htlc with the preimage it knows, user B learns it too and can pull the off-chain invoice.
Usage
To enable hold invoice functionality,
lndneeds to be built with the invoices sub server enabled:make tags="invoicesrpc"Creation of a hold invoice
To add a holdinvoice, invoke:
lncli addholdinvoice <hash> --amt <amt>Waiting for acceptance
Typically an application wants to receive a notification when the htlc is accepted. This event isn't notified through the general
SubscribeInvoicesrpc. The reason for this is not to break backwards compatibility and prevent difficulties in the interaction with the add and settle indices of that call.Instead, the newSubscribeSingleInvoicerpc can be used.SubscribeSingleInvoiceis not available throughlncli. For testing, the invoice state can be observed throughlncli lookupinvoice <hash>.Settling or canceling
Once the htlc has been accepted in
lnd, it is held there until either a cancel or settle rpc is received. Both can be issued throughlncli:lncli cancelinvoice <hash>or
lncli settleinvoice <preimage>Todo
This PR leaves some issues unaddressed:
Automatic invoice cancelation when the incoming htlc is about to time out. Canceling the invoice fails back the htlc. If the invoice wouldn't be canceled, the htlc remains pending and the channel will be force closed on chain. Typical use of hodl invoices involves an application that is monitoring this timeout as well. That application would have either canceled or settled the invoice already before automatic cancelation is triggered. Automatic cancelation can be seen as a safety net to prevent channel closure.
Automatic invoice cancelation when the invoice expires (invoice
expiryproperty). This does not only apply to hodl invoices, but to regular invoices as well. Currentlyexpiryis only checked by the sender before sending the payment and can be worked around. Automatic cancelation would prevent any payment of the invoice from being accepted by the receiver, thereby preventing potential refund flows.Handling of multiple htlcs paying to the same accepted invoice. Currently
amtPaiddoes not reflect the total amount accepted. This is true for regular invoices as well. Calls for a higher level plan of when and when not to accept/settle htlcs.