From fa14ff67adf017cef27a9ce0bbc190e81883cfc8 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Mon, 2 Dec 2019 13:24:26 +0100 Subject: [PATCH] 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 | 88 ++++++++++++++++++++++++++++++++++++++- channeldb/invoices.go | 39 ++++++++--------- lnrpc/rpc.pb.go | 4 +- lnrpc/rpc.proto | 5 ++- lnrpc/rpc.swagger.json | 2 +- 5 files changed, 113 insertions(+), 25 deletions(-) diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index 060ca5dd..00de1326 100644 --- a/channeldb/invoice_test.go +++ b/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) { diff --git a/channeldb/invoices.go b/channeldb/invoices.go index 14e6c451..2be860a9 100644 --- a/channeldb/invoices.go +++ b/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 - // paid. + // ContractSettled means the htlc is settled and the invoice has been paid. ContractSettled ContractState = 1 // ContractCanceled means the invoice has been canceled. ContractCanceled ContractState = 2 - // ContractAccepted means the HTLC has been accepted but not settled - // yet. + // ContractAccepted means the HTLC has been accepted but not settled 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 -// all invoices that are fully settled or canceled. Note that the returned -// array is not ordered by add index. +// only open or accepted invoices and their payment hashes will be returned, +// skipping all invoices that are fully settled or canceled. Note that the +// 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 && - (invoice.State == ContractSettled || - invoice.State == ContractCanceled) { - + if pendingOnly && !invoice.IsPending() { 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 -// returned, skipping all invoices that are fully settled. +// If the pendingOnly param is set to true, then only invoices in open or +// 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 && - invoice.State == ContractSettled { - + if pendingOnly && !invoice.IsPending() { 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 - // interested in unsettled. - if q.PendingOnly && - invoice.State == ContractSettled { - + // Skip any settled or canceled invoices if the caller is + // only interested in pending ones. + if q.PendingOnly && !invoice.IsPending() { continue } diff --git a/lnrpc/rpc.pb.go b/lnrpc/rpc.pb.go index 52845deb..b0d73a2f 100644 --- a/lnrpc/rpc.pb.go +++ b/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 diff --git a/lnrpc/rpc.proto b/lnrpc/rpc.proto index f2765307..1b800a25 100644 --- a/lnrpc/rpc.proto +++ b/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"]; /** diff --git a/lnrpc/rpc.swagger.json b/lnrpc/rpc.swagger.json index 71516f78..78b130be 100644 --- a/lnrpc/rpc.swagger.json +++ b/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",