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

htlcswitch: hodl invoice #2022

Merged
merged 22 commits into from Mar 15, 2019

Conversation

Projects
None yet
6 participants
@joostjager
Copy link
Collaborator

commented Oct 9, 2018

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 #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, lnd needs 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 SubscribeInvoices rpc. 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 new SubscribeSingleInvoice rpc can be used.

SubscribeSingleInvoice is not available through lncli. For testing, the invoice state can be observed through lncli 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 through lncli:

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 expiry property). This does not only apply to hodl invoices, but to regular invoices as well. Currently expiry is 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 amtPaid does 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.

@joostjager joostjager referenced this pull request Oct 9, 2018

Closed

Hold invoice #6

@joostjager joostjager force-pushed the joostjager:holdinvoice branch 8 times, most recently from 32fe441 to df51265 Oct 15, 2018

Show resolved Hide resolved htlcswitch/iterator.go Outdated

@halseth halseth modified the milestones: 0.5.2, 0.6 Oct 29, 2018

@joostjager joostjager force-pushed the joostjager:holdinvoice branch 3 times, most recently from 7ceb180 to 36ef752 Nov 26, 2018

preimage := invoice.Terms.PaymentPreimage
if !bytes.Equal(preimage[:], unknownPreimage[:]) {
err = l.cfg.Registry.AcceptInvoice(

This comment has been minimized.

Copy link
@joostjager

joostjager Nov 26, 2018

Author Collaborator

@cfromknecht, I worked your PoC into this PR. You wrote:

one thing we may consider is logically groupingswitchSettledPackets from 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?

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Dec 5, 2018

Collaborator

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

preimage := invoice.Terms.PaymentPreimage
if !bytes.Equal(preimage[:], unknownPreimage[:]) {
err = l.cfg.Registry.AcceptInvoice(

This comment has been minimized.

Copy link
@joostjager

joostjager Nov 26, 2018

Author Collaborator

And do we need a if fwdPkg.State == channeldb.FwdStateLockedIn check here?

@joostjager joostjager force-pushed the joostjager:holdinvoice branch 2 times, most recently from aa92f9a to e2ad744 Nov 26, 2018

@joostjager joostjager force-pushed the joostjager:holdinvoice branch from e2ad744 to a792776 Dec 4, 2018

@cfromknecht
Copy link
Collaborator

left a 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

Show resolved Hide resolved invoiceregistry.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{})

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Dec 5, 2018

Collaborator

should probably pass nil instead here. resolutions should never go through the htlcPlex, so we shouldn't ever hit the packet.isResolution case.

Show resolved Hide resolved htlcswitch/link.go Outdated
Show resolved Hide resolved cmd/lncli/commands.go Outdated
Show resolved Hide resolved rpcserver.go Outdated
Show resolved Hide resolved channeldb/invoices.go Outdated
return err
}

if err := invoices.Put(invoiceNum[:], buf.Bytes()); err != nil {

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Dec 5, 2018

Collaborator

similar comment here wrt returning Put

This comment has been minimized.

Copy link
@joostjager

joostjager Jan 15, 2019

Author Collaborator

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.

Show resolved Hide resolved channeldb/invoices.go Outdated
Show resolved Hide resolved channeldb/invoices.go Outdated
preimage := invoice.Terms.PaymentPreimage
if !bytes.Equal(preimage[:], unknownPreimage[:]) {
err = l.cfg.Registry.AcceptInvoice(

This comment has been minimized.

Copy link
@cfromknecht

cfromknecht Dec 5, 2018

Collaborator

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

@joostjager

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 5, 2018

@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 joostjager force-pushed the joostjager:holdinvoice branch from a792776 to 354be91 Dec 10, 2018

@joostjager joostjager force-pushed the joostjager:holdinvoice branch 4 times, most recently from 0cad4a6 to 7fe10e7 Dec 18, 2018

@joostjager joostjager force-pushed the joostjager:holdinvoice branch from 7fe10e7 to ce048c8 Dec 27, 2018

joostjager added some commits Feb 9, 2019

contractcourt: add access to full invoice registry from resolvers
Previously a function pointer was passed to chain arbitrator to avoid a
circular dependency. Now that the routetypes package exists, we can pass
the full invoice registry to chain arbitrator.

This is a preparation to be able to use other invoice registry methods
in contract resolvers.
lntypes: return a value from constructors
Returning pointers proved inconvenient in almost all cases. This commmit
converts the constructors to returning values.
htlcswitch/test: move preimage cache to server level
In this commit the preimage cache is instantiated on the mock server
level where it belongs. Previously it was a cache shared across all mock
servers.
htlcswitch: resolve invoice cancelation race condition
Previously it could happen that an invoice was open at the time of the
LookupInvoice call, the htlc was settled because of that, but when the
SettleInvoice call was made eventually, it would fail because the
invoice was canceled in the mean time. The htlc would then be settled,
but the invoice not marked as such.
invoices: extract invoice decoding from registry
Previously it was difficult to use the invoice registry in unit tests,
because it used zpay32 to decode the invoice. For that to succeed, a
valid signature is required on the payment request.

This commit injects the decode dependency on a different level so that
it is easier to mock.
htlcswitch/test: make unknown hash test independent of mock
In the TestChannelLinkMultiHopUnknownPaymentHash test, a preimage was
modified to trigger an unknown payment hash failure. The way the mock is
implemented, it would take the hash of that modified preimage and store
it. It basically stores a completely different invoice. For this test,
it is just as good to store no invoice at all.
htlcswitch/test: use real invoice registry with temp db as mock
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.
htlcswitch: abtract invoice from link
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.
htlcswitch: hodl invoice
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.

@joostjager joostjager force-pushed the joostjager:holdinvoice branch from c3759b8 to f6d6794 Mar 15, 2019

@joostjager

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 15, 2019

@cfromknecht
Copy link
Collaborator

left a comment

@joostjager excellent work on this feature, excited that this also positions us nicely for integrating multi-path payments in the near term! LGTM 💰

@Roasbeef Roasbeef merged commit aa1cd04 into lightningnetwork:master Mar 15, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 60.39%
Details
@alevchuk

This comment has been minimized.

Copy link

commented on f6d6794 Mar 16, 2019

what's the difference between payinvoice and settleinvoice from use-case perspective?

This comment has been minimized.

Copy link
Collaborator

replied Mar 19, 2019

@alevchuk payinvoice is used by the sender of a payment, settleinvoice by the receiver.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.