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.

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.
+		// corresponding invoice if we were the original destination. As
-		err = h.Registry.SettleInvoice(h.payHash, h.htlcAmt)
+		// 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.
+	// invoice if we were the original destination. As the htlc is already
-	err = h.Registry.SettleInvoice(h.payHash, h.htlcAmt)
+	// 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)

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
+	// NotifyExitHopHtlc attempts to mark an invoice as settled. If the
-	// passed payment hash as fully settled.
+	// invoice is a debug invoice, then this method is a noop as debug
-	SettleInvoice(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi) error
+	// 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.

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
+	// Notify the invoiceRegistry of the exit hop htlc. If we crash right
-	// amount accepted at settle time) with this latest commitment update.
+	// after this, this code will be re-executed after restart. We will
-	//
+	// receive back a resolution event.
-	// If we crash right after this, this code will be re-executed after
+	event, err := l.cfg.Registry.NotifyExitHopHtlc(
-	// restart and the HTLC fulfill message will be sent out then.
+		invoiceHash, pd.Amount,
-	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.
 	if err != nil {
-		return false, fmt.Errorf("unable to settle invoice: %v", err)
+		return false, err
 	}
 
-	preimage := invoice.Terms.PaymentPreimage
+	// Process the received resolution.
-	err = l.settleHTLC(preimage, pd.HtlcIndex, pd.SourceRef)
+	htlc := hodlHtlc{
+		pd:         pd,
+		obfuscator: obfuscator,
+	}
+	err = l.processHodlEvent(*event, htlc)
 	if err != nil {
 		return false, err
 	}

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,
+func (i *mockInvoiceRegistry) NotifyExitHopHtlc(rhash lntypes.Hash,
-	amt lnwire.MilliSatoshi) error {
+	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 {

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.
+// settled. The return value describes how the htlc should be resolved.
-func (i *InvoiceRegistry) SettleInvoice(rHash lntypes.Hash,
+func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
-	amtPaid lnwire.MilliSatoshi) error {
+	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 {
+	if invoice, ok := i.debugInvoices[rHash]; ok {
-		// Debug invoices are never fully settled, so we simply return
+		// Debug invoices are never fully settled, so we just settle the
-		// immediately in this case.
+		// htlc in this case.
-		return nil
+		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
+	// If invoice is already settled, settle htlc. This means we accept more
-	// settled.
+	// payments to the same invoice hash.
-	if err == channeldb.ErrInvoiceAlreadySettled {
+	case channeldb.ErrInvoiceAlreadySettled:
-		log.Debugf("Invoice %v already settled", rHash)
+		return createEvent(&invoice.Terms.PaymentPreimage), nil
-		return 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 {
+	// If another error occurred, return that.
-		return err
+	return nil, err
-	}
-
-	log.Infof("Payment received: %v", spew.Sdump(invoice))
-
-	i.notifyClients(rHash, invoice, channeldb.ContractSettled)
-
-	return nil
 }
 
 // CancelInvoice attempts to cancel the invoice corresponding to the passed

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.
+var (
-func TestSettleInvoice(t *testing.T) {
+	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{
+	addIdx, err := registry.AddInvoice(testInvoice, hash)
-		Terms: channeldb.ContractTerm{
-			PaymentPreimage: preimage,
-			Value:           lnwire.MilliSatoshi(100000),
-		},
-		PaymentRequest: []byte(testPayReq),
-	}
-
-	addIdx, err := registry.AddInvoice(invoice, 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()
+	registry, cleanup := newTestContext(t)
-	if err != nil {
-		t.Fatal(err)
-	}
 	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{
+	_, err = registry.AddInvoice(testInvoice, hash)
-		Terms: channeldb.ContractTerm{
-			PaymentPreimage: preimage,
-			Value:           amt,
-		},
-	}
-
-	_, err = registry.AddInvoice(invoice, 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.
+	// Notify arrival of a new htlc paying to this invoice. This should
-	err = registry.SettleInvoice(hash, amt)
+	// succeed.
-	if err != channeldb.ErrInvoiceAlreadyCanceled {
+	event, err := registry.NotifyExitHopHtlc(hash, amt)
-		t.Fatal("expected settlement of a canceled invoice to fail")
+	if err != nil {
+		t.Fatal("expected settlement of a canceled invoice to succeed")
+	}
+
+	if event.Preimage != nil {
+		t.Fatal("expected cancel hodl event")
 	}
 }