channeldb+invoices: fix what pending invoice means in ChannelDB

This commits builds on top of PR #3694 to further clarify invoice
state by defining pending invoices as the ones which are not
settled or canceled. Automatic cancellation of expired invoices
makes this possbile. While this change only directly affects
ChannelDB, users of the listinvoices RPC will receive actual
pending invoices when pending_only flag is set.

channeldb/invoice_test.go

@@ -52,6 +52,29 @@ func randInvoice(value lnwire.MilliSatoshi) (*Invoice, error) {
 	return i, nil
 }
 
+// Tests that pending invoices are those which are either in ContractOpen or
+// in ContractAccepted state.
+func TestInvoiceIsPending(t *testing.T) {
+	contractStates := []ContractState{
+		ContractOpen, ContractSettled, ContractCanceled, ContractAccepted,
+	}
+
+	for _, state := range contractStates {
+		invoice := Invoice{
+			State: state,
+		}
+
+		// We expect that an invoice is pending if it's either in ContractOpen
+		// or ContractAccepted state.
+		pending := (state == ContractOpen || state == ContractAccepted)
+
+		if invoice.IsPending() != pending {
+			t.Fatalf("expected pending: %v, got: %v, invoice: %v",
+				pending, invoice.IsPending(), invoice)
+		}
+	}
+}
+
 func TestInvoiceWorkflow(t *testing.T) {
 	t.Parallel()
 
@@ -436,7 +459,7 @@ func TestFetchAllInvoicesWithPaymentHash(t *testing.T) {
 		invoice.State = states[i%len(states)]
 		paymentHash := invoice.Terms.PaymentPreimage.Hash()
 
-		if invoice.State != ContractSettled && invoice.State != ContractCanceled {
+		if invoice.IsPending() {
 			testPendingInvoices[paymentHash] = invoice
 		}
 
@@ -579,6 +602,69 @@ func TestDuplicateSettleInvoice(t *testing.T) {
 	}
 }
 
+// TestFetchAllInvoices tests that FetchAllInvoices works as expected.
+func TestFetchAllInvoices(t *testing.T) {
+	t.Parallel()
+
+	db, cleanUp, err := makeTestDB()
+	defer cleanUp()
+	if err != nil {
+		t.Fatalf("unable to make test db: %v", err)
+	}
+
+	contractStates := []ContractState{
+		ContractOpen, ContractSettled, ContractCanceled, ContractAccepted,
+	}
+
+	numInvoices := len(contractStates) * 2
+
+	var expectedPendingInvoices []Invoice
+	var expectedAllInvoices []Invoice
+
+	for i := 1; i <= numInvoices; i++ {
+		invoice, err := randInvoice(lnwire.MilliSatoshi(i))
+
+		if err != nil {
+			t.Fatalf("unable to create invoice: %v", err)
+		}
+
+		invoice.AddIndex = uint64(i)
+		// Set the contract state of the next invoice such that there's an equal
+		// number for all possbile states.
+		invoice.State = contractStates[i%len(contractStates)]
+
+		paymentHash := invoice.Terms.PaymentPreimage.Hash()
+		if invoice.IsPending() {
+			expectedPendingInvoices = append(expectedPendingInvoices, *invoice)
+		}
+		expectedAllInvoices = append(expectedAllInvoices, *invoice)
+
+		if _, err := db.AddInvoice(invoice, paymentHash); err != nil {
+			t.Fatalf("unable to add invoice: %v", err)
+		}
+	}
+
+	pendingInvoices, err := db.FetchAllInvoices(true)
+	if err != nil {
+		t.Fatalf("unable to fetch all pending invoices: %v", err)
+	}
+
+	allInvoices, err := db.FetchAllInvoices(false)
+	if err != nil {
+		t.Fatalf("unable to fetch all non pending invoices: %v", err)
+	}
+
+	if !reflect.DeepEqual(pendingInvoices, expectedPendingInvoices) {
+		t.Fatalf("pending invoices: %v\n != \n expected einvoices: %v",
+			spew.Sdump(pendingInvoices), spew.Sdump(expectedPendingInvoices))
+	}
+
+	if !reflect.DeepEqual(allInvoices, expectedAllInvoices) {
+		t.Fatalf("pending + non pending: %v\n != \n expected: %v",
+			spew.Sdump(allInvoices), spew.Sdump(expectedAllInvoices))
+	}
+}
+
 // TestQueryInvoices ensures that we can properly query the invoice database for
 // invoices using different types of queries.
 func TestQueryInvoices(t *testing.T) {

channeldb/invoices.go

@@ -149,15 +149,13 @@ const (
 	// ContractOpen means the invoice has only been created.
 	ContractOpen ContractState = 0
 
-	// ContractSettled means the htlc is settled and the invoice has been
+	// ContractSettled means the htlc is settled and the invoice has been paid.
-	// paid.
 	ContractSettled ContractState = 1
 
 	// ContractCanceled means the invoice has been canceled.
 	ContractCanceled ContractState = 2
 
-	// ContractAccepted means the HTLC has been accepted but not settled
+	// ContractAccepted means the HTLC has been accepted but not settled yet.
-	// yet.
 	ContractAccepted ContractState = 3
 )
 
@@ -385,6 +383,11 @@ func validateInvoice(i *Invoice) error {
 	return nil
 }
 
+// IsPending returns ture if the invoice is in ContractOpen state.
+func (i *Invoice) IsPending() bool {
+	return i.State == ContractOpen || i.State == ContractAccepted
+}
+
 // AddInvoice inserts the targeted invoice into the database. If the invoice has
 // *any* payment hashes which already exists within the database, then the
 // insertion will be aborted and rejected due to the strict policy banning any
@@ -578,9 +581,9 @@ type InvoiceWithPaymentHash struct {
 
 // FetchAllInvoicesWithPaymentHash returns all invoices and their payment hashes
 // currently stored within the database. If the pendingOnly param is true, then
-// only unsettled invoices and their payment hashes will be returned, skipping
+// only open or accepted invoices and their payment hashes will be returned,
-// all invoices that are fully settled or canceled. Note that the returned
+// skipping all invoices that are fully settled or canceled. Note that the
-// array is not ordered by add index.
+// returned array is not ordered by add index.
 func (d *DB) FetchAllInvoicesWithPaymentHash(pendingOnly bool) (
 	[]InvoiceWithPaymentHash, error) {
 
@@ -617,10 +620,7 @@ func (d *DB) FetchAllInvoicesWithPaymentHash(pendingOnly bool) (
 				return err
 			}
 
-			if pendingOnly &&
+			if pendingOnly && !invoice.IsPending() {
-				(invoice.State == ContractSettled ||
-					invoice.State == ContractCanceled) {
-
 				return nil
 			}
 
@@ -643,8 +643,9 @@ func (d *DB) FetchAllInvoicesWithPaymentHash(pendingOnly bool) (
 }
 
 // FetchAllInvoices returns all invoices currently stored within the database.
-// If the pendingOnly param is true, then only unsettled invoices will be
+// If the pendingOnly param is set to true, then only invoices in open or
-// returned, skipping all invoices that are fully settled.
+// accepted state will be returned, skipping all invoices that are fully
+// settled or canceled.
 func (d *DB) FetchAllInvoices(pendingOnly bool) ([]Invoice, error) {
 	var invoices []Invoice
 
@@ -668,9 +669,7 @@ func (d *DB) FetchAllInvoices(pendingOnly bool) ([]Invoice, error) {
 				return err
 			}
 
-			if pendingOnly &&
+			if pendingOnly && !invoice.IsPending() {
-				invoice.State == ContractSettled {
-
 				return nil
 			}
 
@@ -816,11 +815,9 @@ func (d *DB) QueryInvoices(q InvoiceQuery) (InvoiceSlice, error) {
 				return err
 			}
 
-			// Skip any settled invoices if the caller is only
+			// Skip any settled or canceled invoices if the caller is
-			// interested in unsettled.
+			// only interested in pending ones.
-			if q.PendingOnly &&
+			if q.PendingOnly && !invoice.IsPending() {
-				invoice.State == ContractSettled {
-
 				continue
 			}
 

lnrpc/rpc.pb.go

@@ -7556,7 +7556,9 @@ func (m *PaymentHash) GetRHash() []byte {
 }
 
 type ListInvoiceRequest struct {
-	/// If set, only unsettled invoices will be returned in the response.
+	//*
+	//If set, only invoices that are not settled and not canceled will be returned
+	//in the response.
 	PendingOnly bool `protobuf:"varint,1,opt,name=pending_only,proto3" json:"pending_only,omitempty"`
 	//*
 	//The index of an invoice that will be used as either the start or end of a

lnrpc/rpc.proto

@@ -2450,7 +2450,10 @@ message PaymentHash {
 }
 
 message ListInvoiceRequest {
-    /// If set, only unsettled invoices will be returned in the response.
+    /**
+    If set, only invoices that are not settled and not canceled will be returned
+    in the response.
+    */
     bool pending_only = 1 [json_name = "pending_only"];
 
     /**

lnrpc/rpc.swagger.json

@@ -896,7 +896,7 @@
         "parameters": [
           {
             "name": "pending_only",
-            "description": "/ If set, only unsettled invoices will be returned in the response.",
+            "description": "*\nIf set, only invoices that are not settled and not canceled will be returned\nin the response.",
             "in": "query",
             "required": false,
             "type": "boolean",