From 509998db861bb79be3942f1d633adbda5f76cec8 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 17 Jul 2018 16:27:04 -0700 Subject: [PATCH 1/2] channeldb: add new test for duplicate invoice settles In this commit, we add a new test to ensure that duplicate invoice settles work as expected. At the present time, this test will fail as the second to last assertion fails as we'll return a nil invoice the second time around. --- channeldb/invoice_test.go | 61 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index 4f3ee5dda..e752c30b5 100644 --- a/channeldb/invoice_test.go +++ b/channeldb/invoice_test.go @@ -314,3 +314,64 @@ func TestInvoiceAddTimeSeries(t *testing.T) { } } } + +// TestDuplicateSettleInvoice tests that if we add a new invoice and settle it +// twice, then the second time we also receive the invoice that we settled as a +// return argument. +func TestDuplicateSettleInvoice(t *testing.T) { + t.Parallel() + + db, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test db: %v", err) + } + + // We'll start out by creating an invoice and writing it to the DB. + amt := lnwire.NewMSatFromSatoshis(1000) + invoice, err := randInvoice(amt) + if err != nil { + t.Fatalf("unable to create invoice: %v", err) + } + + if _, err := db.AddInvoice(invoice); err != nil { + t.Fatalf("unable to add invoice %v", err) + } + + // With the invoice in the DB, we'll now attempt to settle the invoice. + payHash := sha256.Sum256(invoice.Terms.PaymentPreimage[:]) + dbInvoice, err := db.SettleInvoice(payHash, amt) + if err != nil { + t.Fatalf("unable to settle invoice: %v", err) + } + + // We'll update what we expect the settle invoice to be so that our + // comparison below has the correct assumption. + invoice.SettleIndex = 1 + invoice.Terms.Settled = true + invoice.AmtPaid = amt + invoice.SettleDate = dbInvoice.SettleDate + + // We should get back the exact same invoice that we just inserted. + if !reflect.DeepEqual(dbInvoice, invoice) { + t.Fatalf("wrong invoice after settle, expected %v got %v", + spew.Sdump(invoice), spew.Sdump(dbInvoice)) + } + + // If we try to settle the invoice again, then we should get the very + // same invoice back. + dbInvoice, err = db.SettleInvoice(payHash, amt) + if err != nil { + t.Fatalf("unable to settle invoice: %v", err) + } + + if dbInvoice == nil { + t.Fatalf("invoice from db is nil after settle!") + } + + invoice.SettleDate = dbInvoice.SettleDate + if !reflect.DeepEqual(dbInvoice, invoice) { + t.Fatalf("wrong invoice after second settle, expected %v got %v", + spew.Sdump(invoice), spew.Sdump(dbInvoice)) + } +} From 5ee368f0f9f911c6e7032dcde2365359648200ca Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 17 Jul 2018 16:28:08 -0700 Subject: [PATCH 2/2] channeldb: ensure that we return an invoice on duplicate settles In this commit, we fix an existing bug related to duplicate invoice settle.s Before this commit, the second (and later) times an invoice was settled we would return a nil pointer. This would result in the new invoiceRegistry panicing as it would go to attempt to notify with a nil invoice. We fix this by returning the invoice on disk (unmodified) for each settle after the initial one. Fixes #1568. --- channeldb/invoices.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/channeldb/invoices.go b/channeldb/invoices.go index d315c4a07..feb074420 100644 --- a/channeldb/invoices.go +++ b/channeldb/invoices.go @@ -8,9 +8,9 @@ import ( "io" "time" + "github.com/btcsuite/btcd/wire" "github.com/coreos/bbolt" "github.com/lightningnetwork/lnd/lnwire" - "github.com/btcsuite/btcd/wire" ) var ( @@ -704,7 +704,7 @@ func settleInvoice(invoices, settleIndex *bolt.Bucket, invoiceNum []byte, // Add idempotency to duplicate settles, return here to avoid // overwriting the previous info. if invoice.Terms.Settled { - return nil, nil + return &invoice, nil } // Now that we know the invoice hasn't already been settled, we'll