invoiceregistry: promote update closure to method

This commit moves the update code into its own function as a preparation
for extending the logic further for mpp.

In order to make this change cleanly, structured result codes are
introduced. This also prepares for a future htlc notifier rpc hook that
reports htlc settle decisions to external applications.

Furthermore the awkward use of errNoUpdate as a way to signal no update
is removed.

channeldb/invoices.go

@@ -1241,6 +1241,11 @@ func (d *DB) updateInvoice(hash lntypes.Hash, invoices, settleIndex *bbolt.Bucke
 		return &invoice, err
 	}
 
+	// If there is nothing to update, return early.
+	if update == nil {
+		return &invoice, nil
+	}
+
 	// Update invoice state.
 	invoice.State = update.State
 

invoices/invoiceregistry.go

@@ -24,9 +24,6 @@ var (
 	// ErrShuttingDown is returned when an operation failed because the
 	// invoice registry is shutting down.
 	ErrShuttingDown = errors.New("invoice registry shutting down")
-
-	// errNoUpdate is returned when no invoice updated is required.
-	errNoUpdate = errors.New("no update needed")
 )
 
 // HodlEvent describes how an htlc should be resolved. If HodlEvent.Preimage is
@@ -439,111 +436,48 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 			rHash[:], s, amtPaid, expiry, circuitKey)
 	}
 
-	// Default is to not update subscribers after the invoice update.
+	// Create the update context containing the relevant details of the
-	updateSubscribers := false
+	// incoming htlc.
-
+	updateCtx := invoiceUpdateCtx{
-	updateInvoice := func(inv *channeldb.Invoice) (
+		circuitKey:           circuitKey,
-		*channeldb.InvoiceUpdateDesc, error) {
+		amtPaid:              amtPaid,
-
+		expiry:               expiry,
-		// Don't update the invoice when this is a replayed htlc.
+		currentHeight:        currentHeight,
-		htlc, ok := inv.Htlcs[circuitKey]
+		finalCltvRejectDelta: i.finalCltvRejectDelta,
-		if ok {
-			switch htlc.State {
-			case channeldb.HtlcStateCanceled:
-				debugLog("replayed htlc to canceled invoice")
-
-			case channeldb.HtlcStateAccepted:
-				debugLog("replayed htlc to accepted invoice")
-
-			case channeldb.HtlcStateSettled:
-				debugLog("replayed htlc to settled invoice")
-
-			default:
-				return nil, errors.New("unexpected htlc state")
-			}
-
-			return nil, errNoUpdate
-		}
-
-		// If the invoice is already canceled, there is no further
-		// checking to do.
-		if inv.State == channeldb.ContractCanceled {
-			debugLog("invoice already canceled")
-			return nil, errNoUpdate
-		}
-
-		// If an invoice amount is specified, check that enough
-		// is paid. Also check this for duplicate payments if
-		// the invoice is already settled or accepted.
-		if inv.Terms.Value > 0 && amtPaid < inv.Terms.Value {
-			debugLog("amount too low")
-			return nil, errNoUpdate
-		}
-
-		// The invoice is still open. Check the expiry.
-		if expiry < uint32(currentHeight+i.finalCltvRejectDelta) {
-			debugLog("expiry too soon")
-			return nil, errNoUpdate
-		}
-
-		if expiry < uint32(currentHeight+inv.Terms.FinalCltvDelta) {
-			debugLog("expiry too soon")
-			return nil, errNoUpdate
-		}
-
-		// Record HTLC in the invoice database.
-		newHtlcs := map[channeldb.CircuitKey]*channeldb.HtlcAcceptDesc{
-			circuitKey: {
-				Amt:          amtPaid,
-				Expiry:       expiry,
-				AcceptHeight: currentHeight,
-			},
-		}
-
-		update := channeldb.InvoiceUpdateDesc{
-			Htlcs: newHtlcs,
-		}
-
-		// Don't update invoice state if we are accepting a duplicate
-		// payment. We do accept or settle the HTLC.
-		switch inv.State {
-		case channeldb.ContractAccepted:
-			debugLog("accepting duplicate payment to accepted invoice")
-			update.State = channeldb.ContractAccepted
-			return &update, nil
-
-		case channeldb.ContractSettled:
-			debugLog("accepting duplicate payment to settled invoice")
-			update.State = channeldb.ContractSettled
-			return &update, nil
-		}
-
-		// Check to see if we can settle or this is an hold invoice and
-		// we need to wait for the preimage.
-		holdInvoice := inv.Terms.PaymentPreimage == channeldb.UnknownPreimage
-		if holdInvoice {
-			debugLog("accepted")
-			update.State = channeldb.ContractAccepted
-		} else {
-			debugLog("settled")
-			update.Preimage = inv.Terms.PaymentPreimage
-			update.State = channeldb.ContractSettled
-		}
-
-		updateSubscribers = true
-
-		return &update, nil
 	}
 
 	// We'll attempt to settle an invoice matching this rHash on disk (if
-	// one exists). The callback will set the resolution action that is
+	// one exists). The callback will update the invoice state and/or htlcs.
-	// returned to the link or contract resolver.
+	var (
-	invoice, err := i.cdb.UpdateInvoice(rHash, updateInvoice)
+		result            updateResult
-	if err != nil && err != errNoUpdate {
+		updateSubscribers bool
+	)
+	invoice, err := i.cdb.UpdateInvoice(
+		rHash,
+		func(inv *channeldb.Invoice) (
+			*channeldb.InvoiceUpdateDesc, error) {
+
+			updateDesc, res, err := updateInvoice(&updateCtx, inv)
+			if err != nil {
+				return nil, err
+			}
+
+			// Only send an update if the invoice state was changed.
+			updateSubscribers = updateDesc != nil &&
+				inv.State != updateDesc.State
+
+			// Assign result to outer scope variable.
+			result = res
+
+			return updateDesc, nil
+		},
+	)
+	if err != nil {
 		debugLog(err.Error())
 
 		return nil, err
 	}
+	debugLog(result.String())
 
 	if updateSubscribers {
 		i.notifyClients(rHash, invoice, invoice.State)
@@ -1043,3 +977,154 @@ func (i *InvoiceRegistry) HodlUnsubscribeAll(subscriber chan<- interface{}) {
 
 	delete(i.hodlReverseSubscriptions, subscriber)
 }
+
+// updateResult is the result of the invoice update call.
+type updateResult uint8
+
+const (
+	resultInvalid updateResult = iota
+	resultReplayToCanceled
+	resultReplayToAccepted
+	resultReplayToSettled
+	resultInvoiceAlreadyCanceled
+	resultAmountTooLow
+	resultExpiryTooSoon
+	resultDuplicateToAccepted
+	resultDuplicateToSettled
+	resultAccepted
+	resultSettled
+)
+
+// String returns a human-readable representation of the invoice update result.
+func (u updateResult) String() string {
+	switch u {
+
+	case resultInvalid:
+		return "invalid"
+
+	case resultReplayToCanceled:
+		return "replayed htlc to canceled invoice"
+
+	case resultReplayToAccepted:
+		return "replayed htlc to accepted invoice"
+
+	case resultReplayToSettled:
+		return "replayed htlc to settled invoice"
+
+	case resultInvoiceAlreadyCanceled:
+		return "invoice already canceled"
+
+	case resultAmountTooLow:
+		return "amount too low"
+
+	case resultExpiryTooSoon:
+		return "expiry too soon"
+
+	case resultDuplicateToAccepted:
+		return "accepting duplicate payment to accepted invoice"
+
+	case resultDuplicateToSettled:
+		return "accepting duplicate payment to settled invoice"
+
+	case resultAccepted:
+		return "accepted"
+
+	case resultSettled:
+		return "settled"
+
+	default:
+		return "unknown"
+	}
+}
+
+// invoiceUpdateCtx is an object that describes the context for the invoice
+// update to be carried out.
+type invoiceUpdateCtx struct {
+	circuitKey           channeldb.CircuitKey
+	amtPaid              lnwire.MilliSatoshi
+	expiry               uint32
+	currentHeight        int32
+	finalCltvRejectDelta int32
+}
+
+// updateInvoice is a callback for DB.UpdateInvoice that contains the invoice
+// settlement logic.
+func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
+	*channeldb.InvoiceUpdateDesc, updateResult, error) {
+
+	// Don't update the invoice when this is a replayed htlc.
+	htlc, ok := inv.Htlcs[ctx.circuitKey]
+	if ok {
+		switch htlc.State {
+		case channeldb.HtlcStateCanceled:
+			return nil, resultReplayToCanceled, nil
+
+		case channeldb.HtlcStateAccepted:
+			return nil, resultReplayToAccepted, nil
+
+		case channeldb.HtlcStateSettled:
+			return nil, resultReplayToSettled, nil
+
+		default:
+			return nil, 0, errors.New("unknown htlc state")
+		}
+	}
+
+	// If the invoice is already canceled, there is no further checking to
+	// do.
+	if inv.State == channeldb.ContractCanceled {
+		return nil, resultInvoiceAlreadyCanceled, nil
+	}
+
+	// If an invoice amount is specified, check that enough is paid. Also
+	// check this for duplicate payments if the invoice is already settled
+	// or accepted.
+	if inv.Terms.Value > 0 && ctx.amtPaid < inv.Terms.Value {
+		return nil, resultAmountTooLow, nil
+	}
+
+	// The invoice is still open. Check the expiry.
+	if ctx.expiry < uint32(ctx.currentHeight+ctx.finalCltvRejectDelta) {
+		return nil, resultExpiryTooSoon, nil
+	}
+
+	if ctx.expiry < uint32(ctx.currentHeight+inv.Terms.FinalCltvDelta) {
+		return nil, resultExpiryTooSoon, nil
+	}
+
+	// Record HTLC in the invoice database.
+	newHtlcs := map[channeldb.CircuitKey]*channeldb.HtlcAcceptDesc{
+		ctx.circuitKey: {
+			Amt:          ctx.amtPaid,
+			Expiry:       ctx.expiry,
+			AcceptHeight: ctx.currentHeight,
+		},
+	}
+
+	update := channeldb.InvoiceUpdateDesc{Htlcs: newHtlcs}
+
+	// Don't update invoice state if we are accepting a duplicate payment.
+	// We do accept or settle the HTLC.
+	switch inv.State {
+	case channeldb.ContractAccepted:
+		update.State = channeldb.ContractAccepted
+		return &update, resultDuplicateToAccepted, nil
+
+	case channeldb.ContractSettled:
+		update.State = channeldb.ContractSettled
+		return &update, resultDuplicateToSettled, nil
+	}
+
+	// Check to see if we can settle or this is an hold invoice and we need
+	// to wait for the preimage.
+	holdInvoice := inv.Terms.PaymentPreimage == channeldb.UnknownPreimage
+	if holdInvoice {
+		update.State = channeldb.ContractAccepted
+		return &update, resultAccepted, nil
+	}
+
+	update.Preimage = inv.Terms.PaymentPreimage
+	update.State = channeldb.ContractSettled
+
+	return &update, resultSettled, nil
+}