diff --git a/contractcourt/htlc_success_resolver.go b/contractcourt/htlc_success_resolver.go index a8c774f7f..f2c04358b 100644 --- a/contractcourt/htlc_success_resolver.go +++ b/contractcourt/htlc_success_resolver.go @@ -178,8 +178,10 @@ func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) { } // With the HTLC claimed, we can attempt to settle its - // corresponding invoice if we were the original destination. - err = h.Registry.SettleInvoice(h.payHash, h.htlcAmt) + // corresponding invoice if we were the original destination. As + // the htlc is already settled at this point, we don't need to + // process the result. + _, err = h.Registry.NotifyExitHopHtlc(h.payHash, h.htlcAmt) if err != nil && err != channeldb.ErrInvoiceNotFound { log.Errorf("Unable to settle invoice with payment "+ "hash %x: %v", h.payHash, err) @@ -251,8 +253,9 @@ func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) { } // With the HTLC claimed, we can attempt to settle its corresponding - // invoice if we were the original destination. - err = h.Registry.SettleInvoice(h.payHash, h.htlcAmt) + // invoice if we were the original destination. As the htlc is already + // settled at this point, we don't need to read the result. + _, err = h.Registry.NotifyExitHopHtlc(h.payHash, h.htlcAmt) if err != nil && err != channeldb.ErrInvoiceNotFound { log.Errorf("Unable to settle invoice with payment "+ "hash %x: %v", h.payHash, err) diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index 3bb75d0a7..eb7c4099d 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -3,6 +3,7 @@ package htlcswitch import ( "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/invoices" "github.com/lightningnetwork/lnd/lnpeer" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" @@ -17,9 +18,12 @@ type InvoiceDatabase interface { // extended to us gives us enough time to settle as we prescribe. LookupInvoice(lntypes.Hash) (channeldb.Invoice, uint32, error) - // SettleInvoice attempts to mark an invoice corresponding to the - // passed payment hash as fully settled. - SettleInvoice(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi) error + // NotifyExitHopHtlc attempts to mark an invoice as settled. If the + // invoice is a debug invoice, then this method is a noop as debug + // invoices are never fully settled. The return value describes how the + // htlc should be resolved. + NotifyExitHopHtlc(rhash lntypes.Hash, amt lnwire.MilliSatoshi) ( + *invoices.HodlEvent, error) // CancelInvoice attempts to cancel the invoice corresponding to the // passed payment hash. diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 83eb51059..3ab44f25e 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -16,6 +16,7 @@ import ( "github.com/lightningnetwork/lnd/contractcourt" "github.com/lightningnetwork/lnd/htlcswitch/hodl" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/invoices" "github.com/lightningnetwork/lnd/lnpeer" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwallet" @@ -348,6 +349,12 @@ type channelLink struct { quit chan struct{} } +// hodlHtlc contains htlc data that is required for resolution. +type hodlHtlc struct { + pd *lnwallet.PaymentDescriptor + obfuscator ErrorEncrypter +} + // NewChannelLink creates a new instance of a ChannelLink given a configuration // and active channel that will be used to verify/apply updates to. func NewChannelLink(cfg ChannelLinkConfig, @@ -1064,6 +1071,50 @@ out: } } +// processHodlEvent applies a received hodl event to the provided htlc. When +// this function returns without an error, the commit tx should be updated. +func (l *channelLink) processHodlEvent(hodlEvent invoices.HodlEvent, + htlcs ...hodlHtlc) error { + + hash := hodlEvent.Hash + if hodlEvent.Preimage == nil { + l.debugf("Received hodl cancel event for %v", hash) + } else { + l.debugf("Received hodl settle event for %v", hash) + } + + // Determine required action for the resolution. + var hodlAction func(htlc hodlHtlc) error + if hodlEvent.Preimage != nil { + hodlAction = func(htlc hodlHtlc) error { + return l.settleHTLC( + *hodlEvent.Preimage, htlc.pd.HtlcIndex, + htlc.pd.SourceRef, + ) + } + } else { + hodlAction = func(htlc hodlHtlc) error { + failure := lnwire.NewFailUnknownPaymentHash( + htlc.pd.Amount, + ) + l.sendHTLCError( + htlc.pd.HtlcIndex, failure, htlc.obfuscator, + htlc.pd.SourceRef, + ) + return nil + } + } + + // Apply action for all htlcs matching this hash. + for _, htlc := range htlcs { + if err := hodlAction(htlc); err != nil { + return err + } + } + + return nil +} + // randomFeeUpdateTimeout returns a random timeout between the bounds defined // within the link's configuration that will be used to determine when the link // should propose an update to its commitment fee rate. @@ -2674,30 +2725,22 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor, return true, nil } - // Notify the invoiceRegistry of the invoices we just settled (with the - // amount accepted at settle time) with this latest commitment update. - // - // If we crash right after this, this code will be re-executed after - // restart and the HTLC fulfill message will be sent out then. - err = l.cfg.Registry.SettleInvoice(invoiceHash, pd.Amount) - - // Reject htlcs for canceled invoices. - if err == channeldb.ErrInvoiceAlreadyCanceled { - l.errorf("Rejecting htlc due to canceled invoice") - - failure := lnwire.NewFailUnknownPaymentHash(pd.Amount) - l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef) - - return true, nil - } - - // Handle unexpected errors. + // Notify the invoiceRegistry of the exit hop htlc. If we crash right + // after this, this code will be re-executed after restart. We will + // receive back a resolution event. + event, err := l.cfg.Registry.NotifyExitHopHtlc( + invoiceHash, pd.Amount, + ) if err != nil { - return false, fmt.Errorf("unable to settle invoice: %v", err) + return false, err } - preimage := invoice.Terms.PaymentPreimage - err = l.settleHTLC(preimage, pd.HtlcIndex, pd.SourceRef) + // Process the received resolution. + htlc := hodlHtlc{ + pd: pd, + obfuscator: obfuscator, + } + err = l.processHodlEvent(*event, htlc) if err != nil { return false, err } diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 3a064bc80..f695a9b6c 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -759,18 +759,18 @@ func (i *mockInvoiceRegistry) LookupInvoice(rHash lntypes.Hash) (channeldb.Invoi return i.registry.LookupInvoice(rHash) } -func (i *mockInvoiceRegistry) SettleInvoice(rhash lntypes.Hash, - amt lnwire.MilliSatoshi) error { +func (i *mockInvoiceRegistry) NotifyExitHopHtlc(rhash lntypes.Hash, + amt lnwire.MilliSatoshi) (*invoices.HodlEvent, error) { - err := i.registry.SettleInvoice(rhash, amt) + event, err := i.registry.NotifyExitHopHtlc(rhash, amt) if err != nil { - return err + return event, err } if i.settleChan != nil { i.settleChan <- rhash } - return nil + return event, err } func (i *mockInvoiceRegistry) CancelInvoice(payHash lntypes.Hash) error { diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index e44b3f4f5..c9dc4eeb6 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -27,6 +27,14 @@ var ( DebugHash = DebugPre.Hash() ) +// HodlEvent describes how an htlc should be resolved. If HodlEvent.Preimage is +// set, the event indicates a settle event. If Preimage is nil, it is a cancel +// event. +type HodlEvent struct { + Preimage *lntypes.Preimage + Hash lntypes.Hash +} + // InvoiceRegistry is a central registry of all the outstanding invoices // created by the daemon. The registry is a thin wrapper around a map in order // to ensure that all updates/reads are thread safe. @@ -163,7 +171,7 @@ func (i *InvoiceRegistry) invoiceEventNotifier() { // dispatch notifications to all registered clients. case event := <-i.invoiceEvents: // For backwards compatibility, do not notify all - // invoice subscribers of cancel events + // invoice subscribers of cancel events. if event.state != channeldb.ContractCanceled { i.dispatchToClients(event) } @@ -438,45 +446,54 @@ func (i *InvoiceRegistry) LookupInvoice(rHash lntypes.Hash) (channeldb.Invoice, return invoice, expiry, nil } -// SettleInvoice attempts to mark an invoice as settled. If the invoice is a +// NotifyExitHopHtlc attempts to mark an invoice as settled. If the invoice is a // debug invoice, then this method is a noop as debug invoices are never fully -// settled. -func (i *InvoiceRegistry) SettleInvoice(rHash lntypes.Hash, - amtPaid lnwire.MilliSatoshi) error { +// settled. The return value describes how the htlc should be resolved. +func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, + amtPaid lnwire.MilliSatoshi) (*HodlEvent, error) { i.Lock() defer i.Unlock() log.Debugf("Settling invoice %x", rHash[:]) + createEvent := func(preimage *lntypes.Preimage) *HodlEvent { + return &HodlEvent{ + Hash: rHash, + Preimage: preimage, + } + } + // First check the in-memory debug invoice index to see if this is an // existing invoice added for debugging. - if _, ok := i.debugInvoices[rHash]; ok { - // Debug invoices are never fully settled, so we simply return - // immediately in this case. - return nil + if invoice, ok := i.debugInvoices[rHash]; ok { + // Debug invoices are never fully settled, so we just settle the + // htlc in this case. + return createEvent(&invoice.Terms.PaymentPreimage), nil } // If this isn't a debug invoice, then we'll attempt to settle an // invoice matching this rHash on disk (if one exists). invoice, err := i.cdb.SettleInvoice(rHash, amtPaid) + switch err { - // Implement idempotency by returning success if the invoice was already - // settled. - if err == channeldb.ErrInvoiceAlreadySettled { - log.Debugf("Invoice %v already settled", rHash) - return nil + // If invoice is already settled, settle htlc. This means we accept more + // payments to the same invoice hash. + case channeldb.ErrInvoiceAlreadySettled: + return createEvent(&invoice.Terms.PaymentPreimage), nil + + // If invoice is already canceled, cancel htlc. + case channeldb.ErrInvoiceAlreadyCanceled: + return createEvent(nil), nil + + // If this call settled the invoice, settle the htlc. + case nil: + i.notifyClients(rHash, invoice, invoice.Terms.State) + return createEvent(&invoice.Terms.PaymentPreimage), nil } - if err != nil { - return err - } - - log.Infof("Payment received: %v", spew.Sdump(invoice)) - - i.notifyClients(rHash, invoice, channeldb.ContractSettled) - - return nil + // If another error occurred, return that. + return nil, err } // CancelInvoice attempts to cancel the invoice corresponding to the passed diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 461374024..334adaaf8 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -37,22 +37,41 @@ func decodeExpiry(payReq string) (uint32, error) { return uint32(invoice.MinFinalCLTVExpiry()), nil } -// TestSettleInvoice tests settling of an invoice and related notifications. -func TestSettleInvoice(t *testing.T) { +var ( + testInvoice = &channeldb.Invoice{ + Terms: channeldb.ContractTerm{ + PaymentPreimage: preimage, + Value: lnwire.MilliSatoshi(100000), + }, + PaymentRequest: []byte(testPayReq), + } +) + +func newTestContext(t *testing.T) (*InvoiceRegistry, func()) { cdb, cleanup, err := newDB() if err != nil { t.Fatal(err) } - defer cleanup() // Instantiate and start the invoice registry. registry := NewRegistry(cdb, decodeExpiry) err = registry.Start() if err != nil { + cleanup() t.Fatal(err) } - defer registry.Stop() + + return registry, func() { + registry.Stop() + cleanup() + } +} + +// TestSettleInvoice tests settling of an invoice and related notifications. +func TestSettleInvoice(t *testing.T) { + registry, cleanup := newTestContext(t) + defer cleanup() allSubscriptions := registry.SubscribeNotifications(0, 0) defer allSubscriptions.Cancel() @@ -66,15 +85,7 @@ func TestSettleInvoice(t *testing.T) { } // Add the invoice. - invoice := &channeldb.Invoice{ - Terms: channeldb.ContractTerm{ - PaymentPreimage: preimage, - Value: lnwire.MilliSatoshi(100000), - }, - PaymentRequest: []byte(testPayReq), - } - - addIdx, err := registry.AddInvoice(invoice, hash) + addIdx, err := registry.AddInvoice(testInvoice, hash) if err != nil { t.Fatal(err) } @@ -108,7 +119,7 @@ func TestSettleInvoice(t *testing.T) { // Settle invoice with a slightly higher amount. amtPaid := lnwire.MilliSatoshi(100500) - err = registry.SettleInvoice(hash, amtPaid) + _, err = registry.NotifyExitHopHtlc(hash, amtPaid) if err != nil { t.Fatal(err) } @@ -140,13 +151,13 @@ func TestSettleInvoice(t *testing.T) { } // Try to settle again. - err = registry.SettleInvoice(hash, amtPaid) + _, err = registry.NotifyExitHopHtlc(hash, amtPaid) if err != nil { t.Fatal("expected duplicate settle to succeed") } // Try to settle again with a different amount. - err = registry.SettleInvoice(hash, amtPaid+600) + _, err = registry.NotifyExitHopHtlc(hash, amtPaid+600) if err != nil { t.Fatal("expected duplicate settle to succeed") } @@ -169,26 +180,14 @@ func TestSettleInvoice(t *testing.T) { // TestCancelInvoice tests cancelation of an invoice and related notifications. func TestCancelInvoice(t *testing.T) { - cdb, cleanup, err := newDB() - if err != nil { - t.Fatal(err) - } + registry, cleanup := newTestContext(t) defer cleanup() - // Instantiate and start the invoice registry. - registry := NewRegistry(cdb, decodeExpiry) - - err = registry.Start() - if err != nil { - t.Fatal(err) - } - defer registry.Stop() - allSubscriptions := registry.SubscribeNotifications(0, 0) defer allSubscriptions.Cancel() // Try to cancel the not yet existing invoice. This should fail. - err = registry.CancelInvoice(hash) + err := registry.CancelInvoice(hash) if err != channeldb.ErrInvoiceNotFound { t.Fatalf("expected ErrInvoiceNotFound, but got %v", err) } @@ -203,14 +202,7 @@ func TestCancelInvoice(t *testing.T) { // Add the invoice. amt := lnwire.MilliSatoshi(100000) - invoice := &channeldb.Invoice{ - Terms: channeldb.ContractTerm{ - PaymentPreimage: preimage, - Value: amt, - }, - } - - _, err = registry.AddInvoice(invoice, hash) + _, err = registry.AddInvoice(testInvoice, hash) if err != nil { t.Fatal(err) } @@ -270,10 +262,15 @@ func TestCancelInvoice(t *testing.T) { t.Fatal("expected cancelation of a canceled invoice to succeed") } - // Try to settle. This should not be possible. - err = registry.SettleInvoice(hash, amt) - if err != channeldb.ErrInvoiceAlreadyCanceled { - t.Fatal("expected settlement of a canceled invoice to fail") + // Notify arrival of a new htlc paying to this invoice. This should + // succeed. + event, err := registry.NotifyExitHopHtlc(hash, amt) + if err != nil { + t.Fatal("expected settlement of a canceled invoice to succeed") + } + + if event.Preimage != nil { + t.Fatal("expected cancel hodl event") } }