From 7d56d5385d6ce56c8c9a2457257da4ef850bcfd7 Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Thu, 26 Oct 2023 11:35:31 +0200 Subject: [PATCH] invoices: parametrize invoice(registry) tests with InvoiceDB constructor This commit extracts the InvoiceDB construction from all invoice and registry tests such that we can later on run subtests with multiple backends without needing to use tags. --- invoices/invoiceregistry_test.go | 310 ++++++++++++++++++------- invoices/invoices_test.go | 374 +++++++++++++++++++++---------- invoices/test_utils_test.go | 38 +--- 3 files changed, 479 insertions(+), 243 deletions(-) diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index a10842556..04fe310b5 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -11,6 +11,7 @@ import ( "github.com/lightningnetwork/lnd/amp" "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/clock" invpkg "github.com/lightningnetwork/lnd/invoices" "github.com/lightningnetwork/lnd/lntest/wait" @@ -20,11 +21,115 @@ import ( "github.com/stretchr/testify/require" ) -// TestSettleInvoice tests settling of an invoice and related notifications. -func TestSettleInvoice(t *testing.T) { +// TestInvoiceRegistry is a master test which encompasses all tests using an +// InvoiceDB instance. The purpose of this test is to be able to run all tests +// with a custom DB instance, so that we can test the same logic with different +// DB implementations. +func TestInvoiceRegistry(t *testing.T) { + testList := []struct { + name string + test func(t *testing.T, + makeDB func(t *testing.T) ( + invpkg.InvoiceDB, *clock.TestClock)) + }{ + { + name: "SettleInvoice", + test: testSettleInvoice, + }, + { + name: "CancelInvoice", + test: testCancelInvoice, + }, + { + name: "SettleHoldInvoice", + test: testSettleHoldInvoice, + }, + { + name: "CancelHoldInvoice", + test: testCancelHoldInvoice, + }, + { + name: "UnknownInvoice", + test: testUnknownInvoice, + }, + { + name: "KeySend", + test: testKeySend, + }, + { + name: "HoldKeysend", + test: testHoldKeysend, + }, + { + name: "MppPayment", + test: testMppPayment, + }, + { + name: "MppPaymentWithOverpayment", + test: testMppPaymentWithOverpayment, + }, + { + name: "InvoiceExpiryWithRegistry", + test: testInvoiceExpiryWithRegistry, + }, + { + name: "OldInvoiceRemovalOnStart", + test: testOldInvoiceRemovalOnStart, + }, + { + name: "HeightExpiryWithRegistry", + test: testHeightExpiryWithRegistry, + }, + { + name: "MultipleSetHeightExpiry", + test: testMultipleSetHeightExpiry, + }, + { + name: "SettleInvoicePaymentAddrRequired", + test: testSettleInvoicePaymentAddrRequired, + }, + { + name: "SettleInvoicePaymentAddrRequiredOptionalGrace", + test: testSettleInvoicePaymentAddrRequiredOptionalGrace, + }, + { + name: "AMPWithoutMPPPayload", + test: testAMPWithoutMPPPayload, + }, + { + name: "SpontaneousAmpPayment", + test: testSpontaneousAmpPayment, + }, + } + + makeKeyValueDB := func(t *testing.T) (invpkg.InvoiceDB, + *clock.TestClock) { + + testClock := clock.NewTestClock(testNow) + db, err := channeldb.MakeTestInvoiceDB( + t, channeldb.OptionClock(testClock), + ) + require.NoError(t, err, "unable to make test db") + + return db, testClock + } + + for _, test := range testList { + test := test + + t.Run(test.name, func(t *testing.T) { + test.test(t, makeKeyValueDB) + }) + } +} + +// testSettleInvoice tests settling of an invoice and related notifications. +func testSettleInvoice(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() - ctx := newTestContext(t, nil) + ctx := newTestContext(t, nil, makeDB) ctxb := context.Background() allSubscriptions, err := ctx.registry.SubscribeNotifications(ctxb, 0, 0) @@ -199,7 +304,9 @@ func TestSettleInvoice(t *testing.T) { } } -func testCancelInvoice(t *testing.T, gc bool) { +func testCancelInvoiceImpl(t *testing.T, gc bool, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() cfg := defaultRegistryConfig() @@ -207,7 +314,7 @@ func testCancelInvoice(t *testing.T, gc bool) { // If set to true, then also delete the invoice from the DB after // cancellation. cfg.GcCanceledInvoicesOnTheFly = gc - ctx := newTestContext(t, &cfg) + ctx := newTestContext(t, &cfg, makeDB) ctxb := context.Background() allSubscriptions, err := ctx.registry.SubscribeNotifications(ctxb, 0, 0) @@ -329,36 +436,37 @@ func testCancelInvoice(t *testing.T, gc bool) { require.Equal(t, testCurrentHeight, failResolution.AcceptHeight) } -// TestCancelInvoice tests cancellation of an invoice and related notifications. -func TestCancelInvoice(t *testing.T) { +// testCancelInvoice tests cancellation of an invoice and related notifications. +func testCancelInvoice(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() // Test cancellation both with garbage collection (meaning that canceled // invoice will be deleted) and without (meaning it'll be kept). t.Run("garbage collect", func(t *testing.T) { - testCancelInvoice(t, true) + testCancelInvoiceImpl(t, true, makeDB) }) t.Run("no garbage collect", func(t *testing.T) { - testCancelInvoice(t, false) + testCancelInvoiceImpl(t, false, makeDB) }) } -// TestSettleHoldInvoice tests settling of a hold invoice and related +// testSettleHoldInvoice tests settling of a hold invoice and related // notifications. -func TestSettleHoldInvoice(t *testing.T) { +func testSettleHoldInvoice(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() defer timeout()() - idb, err := newTestChannelDB(t, clock.NewTestClock(time.Time{})) - if err != nil { - t.Fatal(err) - } + idb, clock := makeDB(t) // Instantiate and start the invoice ctx.registry. cfg := invpkg.RegistryConfig{ FinalCltvRejectDelta: testFinalCltvRejectDelta, - Clock: clock.NewTestClock(testTime), + Clock: clock, } expiryWatcher := invpkg.NewInvoiceExpiryWatcher( @@ -366,7 +474,7 @@ func TestSettleHoldInvoice(t *testing.T) { ) registry := invpkg.NewRegistry(idb, expiryWatcher, &cfg) - err = registry.Start() + err := registry.Start() require.NoError(t, err) defer registry.Stop() @@ -511,15 +619,15 @@ func TestSettleHoldInvoice(t *testing.T) { ) } -// TestCancelHoldInvoice tests canceling of a hold invoice and related +// testCancelHoldInvoice tests canceling of a hold invoice and related // notifications. -func TestCancelHoldInvoice(t *testing.T) { +func testCancelHoldInvoice(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() defer timeout()() - testClock := clock.NewTestClock(testTime) - idb, err := newTestChannelDB(t, testClock) - require.NoError(t, err) + idb, testClock := makeDB(t) // Instantiate and start the invoice ctx.registry. cfg := invpkg.RegistryConfig{ @@ -531,7 +639,7 @@ func TestCancelHoldInvoice(t *testing.T) { ) registry := invpkg.NewRegistry(idb, expiryWatcher, &cfg) - err = registry.Start() + err := registry.Start() if err != nil { t.Fatal(err) } @@ -587,14 +695,16 @@ func TestCancelHoldInvoice(t *testing.T) { require.Equal(t, testCurrentHeight, failResolution.AcceptHeight) } -// TestUnknownInvoice tests that invoice registry returns an error when the +// testUnknownInvoice tests that invoice registry returns an error when the // invoice is unknown. This is to guard against returning a cancel htlc // resolution for forwarded htlcs. In the link, NotifyExitHopHtlc is only called // if we are the exit hop, but in htlcIncomingContestResolver it is called with // forwarded htlc hashes as well. -func TestUnknownInvoice(t *testing.T) { +func testUnknownInvoice(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() - ctx := newTestContext(t, nil) + ctx := newTestContext(t, nil, makeDB) // Notify arrival of a new htlc paying to this invoice. This should // succeed. @@ -611,28 +721,32 @@ func TestUnknownInvoice(t *testing.T) { checkFailResolution(t, resolution, invpkg.ResultInvoiceNotFound) } -// TestKeySend tests receiving a spontaneous payment with and without keysend +// testKeySend tests receiving a spontaneous payment with and without keysend // enabled. -func TestKeySend(t *testing.T) { +func testKeySend(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() t.Run("enabled", func(t *testing.T) { - testKeySend(t, true) + testKeySendImpl(t, true, makeDB) }) t.Run("disabled", func(t *testing.T) { - testKeySend(t, false) + testKeySendImpl(t, false, makeDB) }) } -// testKeySend is the inner test function that tests keysend for a particular -// enabled state on the receiver end. -func testKeySend(t *testing.T, keySendEnabled bool) { +// testKeySendImpl is the inner test function that tests keysend for a +// particular enabled state on the receiver end. +func testKeySendImpl(t *testing.T, keySendEnabled bool, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() defer timeout()() cfg := defaultRegistryConfig() cfg.AcceptKeySend = keySendEnabled - ctx := newTestContext(t, &cfg) + ctx := newTestContext(t, &cfg, makeDB) allSubscriptions, err := ctx.registry.SubscribeNotifications( context.Background(), 0, 0, @@ -742,20 +856,24 @@ func testKeySend(t *testing.T, keySendEnabled bool) { checkSubscription() } -// TestHoldKeysend tests receiving a spontaneous payment that is held. -func TestHoldKeysend(t *testing.T) { +// testHoldKeysend tests receiving a spontaneous payment that is held. +func testHoldKeysend(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() t.Run("settle", func(t *testing.T) { - testHoldKeysend(t, false) + testHoldKeysendImpl(t, false, makeDB) }) t.Run("timeout", func(t *testing.T) { - testHoldKeysend(t, true) + testHoldKeysendImpl(t, true, makeDB) }) } -// testHoldKeysend is the inner test function that tests hold-keysend. -func testHoldKeysend(t *testing.T, timeoutKeysend bool) { +// testHoldKeysendImpl is the inner test function that tests hold-keysend. +func testHoldKeysendImpl(t *testing.T, timeoutKeysend bool, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() defer timeout()() @@ -764,7 +882,7 @@ func testHoldKeysend(t *testing.T, timeoutKeysend bool) { cfg := defaultRegistryConfig() cfg.AcceptKeySend = true cfg.KeysendHoldTime = holdDuration - ctx := newTestContext(t, &cfg) + ctx := newTestContext(t, &cfg, makeDB) ctxb := context.Background() allSubscriptions, err := ctx.registry.SubscribeNotifications(ctxb, 0, 0) @@ -844,14 +962,16 @@ func testHoldKeysend(t *testing.T, timeoutKeysend bool) { require.Equal(t, settledInvoice.State, invpkg.ContractSettled) } -// TestMppPayment tests settling of an invoice with multiple partial payments. +// testMppPayment tests settling of an invoice with multiple partial payments. // It covers the case where there is a mpp timeout before the whole invoice is // paid and the case where the invoice is settled in time. -func TestMppPayment(t *testing.T) { +func testMppPayment(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() defer timeout()() - ctx := newTestContext(t, nil) + ctx := newTestContext(t, nil, makeDB) ctxb := context.Background() // Add the invoice. @@ -940,15 +1060,17 @@ func TestMppPayment(t *testing.T) { } } -// TestMppPaymentWithOverpayment tests settling of an invoice with multiple +// testMppPaymentWithOverpayment tests settling of an invoice with multiple // partial payments. It covers the case where the mpp overpays what is in the // invoice. -func TestMppPaymentWithOverpayment(t *testing.T) { +func testMppPaymentWithOverpayment(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() ctxb := context.Background() f := func(overpaymentRand uint64) bool { - ctx := newTestContext(t, nil) + ctx := newTestContext(t, nil, makeDB) // Add the invoice. testInvoice := newInvoice(t, false) @@ -1017,13 +1139,13 @@ func TestMppPaymentWithOverpayment(t *testing.T) { } } -// Tests that invoices are canceled after expiration. -func TestInvoiceExpiryWithRegistry(t *testing.T) { - t.Parallel() +// testInvoiceExpiryWithRegistry tests that invoices are canceled after +// expiration. +func testInvoiceExpiryWithRegistry(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { - testClock := clock.NewTestClock(testTime) - idb, err := newTestChannelDB(t, testClock) - require.NoError(t, err) + t.Parallel() + idb, testClock := makeDB(t) cfg := invpkg.RegistryConfig{ FinalCltvRejectDelta: testFinalCltvRejectDelta, @@ -1119,21 +1241,20 @@ func TestInvoiceExpiryWithRegistry(t *testing.T) { // Retrospectively check that all invoices that were expected to be // canceled are indeed canceled. - err = wait.NoError(canceled, testTimeout) + err := wait.NoError(canceled, testTimeout) require.NoError(t, err, "timeout checking invoice state") // Finally stop the registry. require.NoError(t, registry.Stop(), "failed to stop invoice registry") } -// TestOldInvoiceRemovalOnStart tests that we'll attempt to remove old canceled +// testOldInvoiceRemovalOnStart tests that we'll attempt to remove old canceled // invoices upon start while keeping all settled ones. -func TestOldInvoiceRemovalOnStart(t *testing.T) { - t.Parallel() +func testOldInvoiceRemovalOnStart(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { - testClock := clock.NewTestClock(testTime) - idb, err := newTestChannelDB(t, testClock) - require.NoError(t, err) + t.Parallel() + idb, testClock := makeDB(t) cfg := invpkg.RegistryConfig{ FinalCltvRejectDelta: testFinalCltvRejectDelta, @@ -1203,35 +1324,39 @@ func TestOldInvoiceRemovalOnStart(t *testing.T) { require.Equal(t, expected, response.Invoices) } -// TestHeightExpiryWithRegistry tests our height-based invoice expiry for +// testHeightExpiryWithRegistry tests our height-based invoice expiry for // invoices paid with single and multiple htlcs, testing the case where the // invoice is settled before expiry (and thus not canceled), and the case // where the invoice is expired. -func TestHeightExpiryWithRegistry(t *testing.T) { +func testHeightExpiryWithRegistry(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() t.Run("single shot settled before expiry", func(t *testing.T) { - testHeightExpiryWithRegistry(t, 1, true) + testHeightExpiryWithRegistryImpl(t, 1, true, makeDB) }) t.Run("single shot expires", func(t *testing.T) { - testHeightExpiryWithRegistry(t, 1, false) + testHeightExpiryWithRegistryImpl(t, 1, false, makeDB) }) t.Run("mpp settled before expiry", func(t *testing.T) { - testHeightExpiryWithRegistry(t, 2, true) + testHeightExpiryWithRegistryImpl(t, 2, true, makeDB) }) t.Run("mpp expires", func(t *testing.T) { - testHeightExpiryWithRegistry(t, 2, false) + testHeightExpiryWithRegistryImpl(t, 2, false, makeDB) }) } -func testHeightExpiryWithRegistry(t *testing.T, numParts int, settle bool) { +func testHeightExpiryWithRegistryImpl(t *testing.T, numParts int, settle bool, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() defer timeout()() - ctx := newTestContext(t, nil) + ctx := newTestContext(t, nil, makeDB) require.Greater(t, numParts, 0, "test requires at least one part") @@ -1337,15 +1462,17 @@ func testHeightExpiryWithRegistry(t *testing.T, numParts int, settle bool) { "hold invoice: %v, got: %v", expectedState, inv.State) } -// TestMultipleSetHeightExpiry pays a hold invoice with two mpp sets, testing +// testMultipleSetHeightExpiry pays a hold invoice with two mpp sets, testing // that the invoice expiry watcher only uses the expiry height of the second, // successful set to cancel the invoice, and does not cancel early using the // expiry height of the first set that was canceled back due to mpp timeout. -func TestMultipleSetHeightExpiry(t *testing.T) { +func testMultipleSetHeightExpiry(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() defer timeout()() - ctx := newTestContext(t, nil) + ctx := newTestContext(t, nil, makeDB) // Add a hold invoice. testInvoice := newInvoice(t, true) @@ -1431,13 +1558,15 @@ func TestMultipleSetHeightExpiry(t *testing.T) { }, testTimeout, time.Millisecond*100, "invoice not canceled") } -// TestSettleInvoicePaymentAddrRequired tests that if an incoming payment has +// testSettleInvoicePaymentAddrRequired tests that if an incoming payment has // an invoice that requires the payment addr bit to be set, and the incoming // payment doesn't include an mpp payload, then the payment is rejected. -func TestSettleInvoicePaymentAddrRequired(t *testing.T) { +func testSettleInvoicePaymentAddrRequired(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() - ctx := newTestContext(t, nil) + ctx := newTestContext(t, nil, makeDB) ctxb := context.Background() allSubscriptions, err := ctx.registry.SubscribeNotifications(ctxb, 0, 0) @@ -1519,14 +1648,16 @@ func TestSettleInvoicePaymentAddrRequired(t *testing.T) { require.Equal(t, failResolution.Outcome, invpkg.ResultAddressMismatch) } -// TestSettleInvoicePaymentAddrRequiredOptionalGrace tests that if an invoice +// testSettleInvoicePaymentAddrRequiredOptionalGrace tests that if an invoice // in the database has an optional payment addr required bit set, then we'll // still allow it to be paid by an incoming HTLC that doesn't include the MPP // payload. This ensures we don't break payment for any invoices in the wild. -func TestSettleInvoicePaymentAddrRequiredOptionalGrace(t *testing.T) { +func testSettleInvoicePaymentAddrRequiredOptionalGrace(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() - ctx := newTestContext(t, nil) + ctx := newTestContext(t, nil, makeDB) ctxb := context.Background() allSubscriptions, err := ctx.registry.SubscribeNotifications(ctxb, 0, 0) @@ -1633,15 +1764,17 @@ func TestSettleInvoicePaymentAddrRequiredOptionalGrace(t *testing.T) { } } -// TestAMPWithoutMPPPayload asserts that we correctly reject an AMP HTLC that +// testAMPWithoutMPPPayload asserts that we correctly reject an AMP HTLC that // does not include an MPP record. -func TestAMPWithoutMPPPayload(t *testing.T) { +func testAMPWithoutMPPPayload(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() defer timeout()() cfg := defaultRegistryConfig() cfg.AcceptAMP = true - ctx := newTestContext(t, &cfg) + ctx := newTestContext(t, &cfg, makeDB) const ( shardAmt = lnwire.MilliSatoshi(10) @@ -1666,9 +1799,11 @@ func TestAMPWithoutMPPPayload(t *testing.T) { checkFailResolution(t, resolution, invpkg.ResultAmpError) } -// TestSpontaneousAmpPayment tests receiving a spontaneous AMP payment with both +// testSpontaneousAmpPayment tests receiving a spontaneous AMP payment with both // valid and invalid reconstructions. -func TestSpontaneousAmpPayment(t *testing.T) { +func testSpontaneousAmpPayment(t *testing.T, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { + t.Parallel() tests := []struct { @@ -1712,24 +1847,25 @@ func TestSpontaneousAmpPayment(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - testSpontaneousAmpPayment( + testSpontaneousAmpPaymentImpl( t, test.ampEnabled, test.failReconstruction, - test.numShards, + test.numShards, makeDB, ) }) } } // testSpontaneousAmpPayment runs a specific spontaneous AMP test case. -func testSpontaneousAmpPayment( - t *testing.T, ampEnabled, failReconstruction bool, numShards int) { +func testSpontaneousAmpPaymentImpl( + t *testing.T, ampEnabled, failReconstruction bool, numShards int, + makeDB func(t *testing.T) (invpkg.InvoiceDB, *clock.TestClock)) { t.Parallel() defer timeout()() cfg := defaultRegistryConfig() cfg.AcceptAMP = ampEnabled - ctx := newTestContext(t, &cfg) + ctx := newTestContext(t, &cfg, makeDB) ctxb := context.Background() allSubscriptions, err := ctx.registry.SubscribeNotifications(ctxb, 0, 0) diff --git a/invoices/invoices_test.go b/invoices/invoices_test.go index 259a977d4..caf58f13a 100644 --- a/invoices/invoices_test.go +++ b/invoices/invoices_test.go @@ -32,8 +32,6 @@ var ( ) testNow = time.Unix(1, 0) - - testClock = clock.NewTestClock(testNow) ) func randInvoice(value lnwire.MilliSatoshi) (*invpkg.Invoice, error) { @@ -93,9 +91,125 @@ func settleTestInvoice(invoice *invpkg.Invoice, settleIndex uint64) { invoice.SettleIndex = settleIndex } -// Tests that pending invoices are those which are either in ContractOpen or -// in ContractAccepted state. +// TestInvoices is a master test which encompasses all tests using an InvoiceDB +// instance. The purpose of this test is to be able to run all tests with a +// custom DB instance, so that we can test the same logic with different DB +// implementations. +func TestInvoices(t *testing.T) { + testList := []struct { + name string + test func(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) + }{ + { + name: "InvoiceWorkflow", + test: testInvoiceWorkflow, + }, + { + name: "AddDuplicatePayAddr", + test: testAddDuplicatePayAddr, + }, + { + name: "AddDuplicateKeysendPayAddr", + test: testAddDuplicateKeysendPayAddr, + }, + { + name: "FailInvoiceLookupMPPPayAddrOnly", + test: testFailInvoiceLookupMPPPayAddrOnly, + }, + { + name: "InvRefEquivocation", + test: testInvRefEquivocation, + }, + { + name: "InvoiceCancelSingleHtlc", + test: testInvoiceCancelSingleHtlc, + }, + { + name: "InvoiceCancelSingleHtlcAMP", + test: testInvoiceCancelSingleHtlcAMP, + }, + { + name: "InvoiceAddTimeSeries", + test: testInvoiceAddTimeSeries, + }, + { + name: "SettleIndexAmpPayments", + test: testSettleIndexAmpPayments, + }, + { + name: "FetchPendingInvoices", + test: testFetchPendingInvoices, + }, + { + name: "DuplicateSettleInvoice", + test: testDuplicateSettleInvoice, + }, + { + name: "QueryInvoices", + test: testQueryInvoices, + }, + { + name: "CustomRecords", + test: testCustomRecords, + }, + { + name: "InvoiceHtlcAMPFields", + test: testInvoiceHtlcAMPFields, + }, + { + name: "AddInvoiceWithHTLCs", + test: testAddInvoiceWithHTLCs, + }, + { + name: "SetIDIndex", + test: testSetIDIndex, + }, + { + name: "UnexpectedInvoicePreimage", + test: testUnexpectedInvoicePreimage, + }, + { + name: "UpdateHTLCPreimages", + test: testUpdateHTLCPreimages, + }, + { + name: "DeleteInvoices", + test: testDeleteInvoices, + }, + { + name: "DeleteCanceledInvoices", + test: testDeleteCanceledInvoices, + }, + { + name: "AddInvoiceInvalidFeatureDeps", + test: testAddInvoiceInvalidFeatureDeps, + }, + } + + makeKeyValueDB := func(t *testing.T) invpkg.InvoiceDB { + db, err := channeldb.MakeTestInvoiceDB( + t, channeldb.OptionClock(clock.NewTestClock(testNow)), + ) + require.NoError(t, err, "unable to make test db") + + return db + } + + for _, test := range testList { + test := test + + t.Run(test.name, func(t *testing.T) { + test.test(t, makeKeyValueDB) + }) + } +} + +// TestInvoiceIsPending tests that pending invoices are those which are either +// in ContractOpen or in ContractAccepted state. func TestInvoiceIsPending(t *testing.T) { + t.Parallel() + contractStates := []invpkg.ContractState{ invpkg.ContractOpen, invpkg.ContractSettled, invpkg.ContractCanceled, invpkg.ContractAccepted, @@ -143,20 +257,24 @@ var invWorkflowTests = []invWorkflowTest{ // TestInvoiceWorkflow asserts the basic process of inserting, fetching, and // updating an invoice. We assert that the flow is successful using when // querying with various combinations of payment hash and payment address. -func TestInvoiceWorkflow(t *testing.T) { +func testInvoiceWorkflow(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + t.Parallel() for _, test := range invWorkflowTests { test := test t.Run(test.name, func(t *testing.T) { - testInvoiceWorkflow(t, test) + t.Parallel() + testInvoiceWorkflowImpl(t, test, makeDB) }) } } -func testInvoiceWorkflow(t *testing.T, test invWorkflowTest) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err, "unable to make test db") +func testInvoiceWorkflowImpl(t *testing.T, test invWorkflowTest, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + + db := makeDB(t) // Create a fake invoice which we'll use several times in the tests // below. @@ -290,11 +408,13 @@ func testInvoiceWorkflow(t *testing.T, test invWorkflowTest) { } } -// TestAddDuplicatePayAddr asserts that the payment addresses of inserted +// testAddDuplicatePayAddr asserts that the payment addresses of inserted // invoices are unique. -func TestAddDuplicatePayAddr(t *testing.T) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err) +func testAddDuplicatePayAddr(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + + t.Parallel() + db := makeDB(t) // Create two invoices with the same payment addr. invoice1, err := randInvoice(1000) @@ -317,12 +437,14 @@ func TestAddDuplicatePayAddr(t *testing.T) { require.Error(t, err, invpkg.ErrDuplicatePayAddr) } -// TestAddDuplicateKeysendPayAddr asserts that we permit duplicate payment +// testAddDuplicateKeysendPayAddr asserts that we permit duplicate payment // addresses to be inserted if they are blank to support JIT legacy keysend // invoices. -func TestAddDuplicateKeysendPayAddr(t *testing.T) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err) +func testAddDuplicateKeysendPayAddr(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + + t.Parallel() + db := makeDB(t) // Create two invoices with the same _blank_ payment addr. invoice1, err := randInvoice(1000) @@ -360,13 +482,15 @@ func TestAddDuplicateKeysendPayAddr(t *testing.T) { require.Equal(t, invoice2, &dbInv2) } -// TestFailInvoiceLookupMPPPayAddrOnly asserts that looking up a MPP invoice +// testFailInvoiceLookupMPPPayAddrOnly asserts that looking up a MPP invoice // that matches _only_ by payment address fails with ErrInvoiceNotFound. This // ensures that the HTLC's payment hash always matches the payment hash in the // returned invoice. -func TestFailInvoiceLookupMPPPayAddrOnly(t *testing.T) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err) +func testFailInvoiceLookupMPPPayAddrOnly(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + + t.Parallel() + db := makeDB(t) // Create and insert a random invoice. invoice, err := randInvoice(1000) @@ -391,11 +515,13 @@ func TestFailInvoiceLookupMPPPayAddrOnly(t *testing.T) { require.Equal(t, invpkg.ErrInvoiceNotFound, err) } -// TestInvRefEquivocation asserts that retrieving or updating an invoice using +// testInvRefEquivocation asserts that retrieving or updating an invoice using // an equivocating InvoiceRef results in ErrInvRefEquivocation. -func TestInvRefEquivocation(t *testing.T) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err) +func testInvRefEquivocation(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + + t.Parallel() + db := makeDB(t) // Add two random invoices. invoice1, err := randInvoice(1000) @@ -431,13 +557,13 @@ func TestInvRefEquivocation(t *testing.T) { require.Error(t, err, invpkg.ErrInvRefEquivocation) } -// TestInvoiceCancelSingleHtlc tests that a single htlc can be canceled on the +// testInvoiceCancelSingleHtlc tests that a single htlc can be canceled on the // invoice. -func TestInvoiceCancelSingleHtlc(t *testing.T) { - t.Parallel() +func testInvoiceCancelSingleHtlc(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) preimage := lntypes.Preimage{1} paymentHash := preimage.Hash() @@ -511,16 +637,14 @@ func TestInvoiceCancelSingleHtlc(t *testing.T) { } } -// TestInvoiceCancelSingleHtlcAMP tests that it's possible to cancel a single +// testInvoiceCancelSingleHtlcAMP tests that it's possible to cancel a single // invoice of an AMP HTLC across multiple set IDs, and also have that update // the amount paid and other related fields as well. -func TestInvoiceCancelSingleHtlcAMP(t *testing.T) { - t.Parallel() +func testInvoiceCancelSingleHtlcAMP(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB( - t, channeldb.OptionClock(testClock), - ) - require.NoError(t, err, "unable to make test db: %v", err) + t.Parallel() + db := makeDB(t) // We'll start out by creating an invoice and writing it to the DB. amt := lnwire.NewMSatFromSatoshis(1000) @@ -691,19 +815,17 @@ func TestInvoiceCancelSingleHtlcAMP(t *testing.T) { require.Equal(t, invoice, dbInvoice) } -// TestInvoiceTimeSeries tests that newly added invoices invoices, as well as +// testInvoiceTimeSeries tests that newly added invoices invoices, as well as // settled invoices are added to the database are properly placed in the add // or settle index which serves as an event time series. -func TestInvoiceAddTimeSeries(t *testing.T) { - t.Parallel() +func testInvoiceAddTimeSeries(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB( - t, channeldb.OptionClock(testClock), - ) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) ctxb := context.Background() - _, err = db.InvoicesAddedSince(ctxb, 0) + _, err := db.InvoicesAddedSince(ctxb, 0) require.NoError(t, err) // We'll start off by creating 20 random invoices, and inserting them @@ -849,18 +971,15 @@ func TestInvoiceAddTimeSeries(t *testing.T) { } } -// TestSettleIndexAmpPayments tests that repeated settles of the same invoice +// testSettleIndexAmpPayments tests that repeated settles of the same invoice // end up properly adding entries to the settle index, and the // InvoicesSettledSince will emit a "projected" version of the invoice w/ // _just_ that HTLC information. -func TestSettleIndexAmpPayments(t *testing.T) { - t.Parallel() +func testSettleIndexAmpPayments(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - testClock := clock.NewTestClock(testNow) - db, err := channeldb.MakeTestInvoiceDB( - t, channeldb.OptionClock(testClock), - ) - require.Nil(t, err) + t.Parallel() + db := makeDB(t) // First, we'll make a sample invoice that'll be paid to several times // below. @@ -1022,15 +1141,13 @@ func TestSettleIndexAmpPayments(t *testing.T) { require.Nil(t, err) } -// TestFetchPendingInvoices tests that we can fetch all pending invoices from +// testFetchPendingInvoices tests that we can fetch all pending invoices from // the database using the FetchPendingInvoices method. -func TestFetchPendingInvoices(t *testing.T) { - t.Parallel() +func testFetchPendingInvoices(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB( - t, channeldb.OptionClock(testClock), - ) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) ctxb := context.Background() @@ -1078,16 +1195,14 @@ func TestFetchPendingInvoices(t *testing.T) { require.Equal(t, pendingInvoices, pending) } -// TestDuplicateSettleInvoice tests that if we add a new invoice and settle it +// 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() +func testDuplicateSettleInvoice(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB( - t, channeldb.OptionClock(testClock), - ) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) // We'll start out by creating an invoice and writing it to the DB. amt := lnwire.NewMSatFromSatoshis(1000) @@ -1142,15 +1257,13 @@ func TestDuplicateSettleInvoice(t *testing.T) { ) } -// TestQueryInvoices ensures that we can properly query the invoice database for +// testQueryInvoices ensures that we can properly query the invoice database for // invoices using different types of queries. -func TestQueryInvoices(t *testing.T) { - t.Parallel() +func testQueryInvoices(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB( - t, channeldb.OptionClock(testClock), - ) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) // To begin the test, we'll add 50 invoices to the database. We'll // assume that the index of the invoice within the database is the same @@ -1577,13 +1690,13 @@ func getUpdateInvoice(amt lnwire.MilliSatoshi) invpkg.InvoiceUpdateCallback { } } -// TestCustomRecords tests that custom records are properly recorded in the +// testCustomRecords tests that custom records are properly recorded in the // invoice database. -func TestCustomRecords(t *testing.T) { - t.Parallel() +func testCustomRecords(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) preimage := lntypes.Preimage{1} paymentHash := preimage.Hash() @@ -1630,7 +1743,7 @@ func TestCustomRecords(t *testing.T) { }, nil } - _, err = db.UpdateInvoice(ctxb, ref, nil, callback) + _, err := db.UpdateInvoice(ctxb, ref, nil, callback) require.NoError(t, err, "unable to add invoice htlc") // Retrieve the invoice from that database and verify that the custom @@ -1648,20 +1761,27 @@ func TestCustomRecords(t *testing.T) { ) } -// TestInvoiceHtlcAMPFields asserts that the set id and preimage fields are +// testInvoiceHtlcAMPFields asserts that the set id and preimage fields are // properly recorded when updating an invoice. -func TestInvoiceHtlcAMPFields(t *testing.T) { +func testInvoiceHtlcAMPFields(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + + t.Parallel() + t.Run("amp", func(t *testing.T) { - testInvoiceHtlcAMPFields(t, true) + t.Parallel() + testInvoiceHtlcAMPFieldsImpl(t, true, makeDB) }) t.Run("no amp", func(t *testing.T) { - testInvoiceHtlcAMPFields(t, false) + t.Parallel() + testInvoiceHtlcAMPFieldsImpl(t, false, makeDB) }) } -func testInvoiceHtlcAMPFields(t *testing.T, isAMP bool) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.Nil(t, err) +func testInvoiceHtlcAMPFieldsImpl(t *testing.T, isAMP bool, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + + db := makeDB(t) testInvoice, err := randInvoice(1000) require.Nil(t, err) @@ -1727,6 +1847,8 @@ func testInvoiceHtlcAMPFields(t *testing.T, isAMP bool) { // TestInvoiceRef asserts that the proper identifiers are returned from an // InvoiceRef depending on the constructor used. func TestInvoiceRef(t *testing.T) { + t.Parallel() + payHash := lntypes.Hash{0x01} payAddr := [32]byte{0x02} setID := [32]byte{0x03} @@ -1765,6 +1887,8 @@ func TestInvoiceRef(t *testing.T) { // not comingle, and also that HTLCs with disjoint set ids appear in different // sets. func TestHTLCSet(t *testing.T) { + t.Parallel() + inv := &invpkg.Invoice{ Htlcs: make(map[models.CircuitKey]*invpkg.InvoiceHTLC), } @@ -1859,11 +1983,13 @@ func TestHTLCSet(t *testing.T) { } } -// TestAddInvoiceWithHTLCs asserts that you can't insert an invoice that already +// testAddInvoiceWithHTLCs asserts that you can't insert an invoice that already // has HTLCs. -func TestAddInvoiceWithHTLCs(t *testing.T) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.Nil(t, err) +func testAddInvoiceWithHTLCs(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + + t.Parallel() + db := makeDB(t) testInvoice, err := randInvoice(1000) require.Nil(t, err) @@ -1876,15 +2002,12 @@ func TestAddInvoiceWithHTLCs(t *testing.T) { require.Equal(t, invpkg.ErrInvoiceHasHtlcs, err) } -// TestSetIDIndex asserts that the set id index properly adds new invoices as we +// testSetIDIndex asserts that the set id index properly adds new invoices as we // accept HTLCs, that they can be queried by their set id after accepting, and // that invoices with duplicate set ids are disallowed. -func TestSetIDIndex(t *testing.T) { - testClock := clock.NewTestClock(testNow) - db, err := channeldb.MakeTestInvoiceDB( - t, channeldb.OptionClock(testClock), - ) - require.Nil(t, err) +func testSetIDIndex(t *testing.T, makeDB func(t *testing.T) invpkg.InvoiceDB) { + t.Parallel() + db := makeDB(t) // We'll start out by creating an invoice and writing it to the DB. amt := lnwire.NewMSatFromSatoshis(1000) @@ -2217,16 +2340,16 @@ func getUpdateInvoiceAMPSettle(setID *[32]byte, preimage [32]byte, } } -// TestUnexpectedInvoicePreimage asserts that legacy or MPP invoices cannot be +// testUnexpectedInvoicePreimage asserts that legacy or MPP invoices cannot be // settled when referenced by payment address only. Since regular or MPP // payments do not store the payment hash explicitly (it is stored in the // index), this enforces that they can only be updated using a InvoiceRefByHash // or InvoiceRefByHashOrAddr. -func TestUnexpectedInvoicePreimage(t *testing.T) { - t.Parallel() +func testUnexpectedInvoicePreimage(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) invoice, err := randInvoice(lnwire.MilliSatoshi(100)) require.NoError(t, err) @@ -2256,9 +2379,11 @@ type updateHTLCPreimageTestCase struct { expError error } -// TestUpdateHTLCPreimages asserts various properties of setting HTLC-level +// testUpdateHTLCPreimages asserts various properties of setting HTLC-level // preimages on invoice state transitions. -func TestUpdateHTLCPreimages(t *testing.T) { +func testUpdateHTLCPreimages(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + t.Parallel() tests := []updateHTLCPreimageTestCase{ @@ -2277,15 +2402,16 @@ func TestUpdateHTLCPreimages(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - testUpdateHTLCPreimages(t, test) + t.Parallel() + testUpdateHTLCPreimagesImpl(t, test, makeDB) }) } } -func testUpdateHTLCPreimages(t *testing.T, test updateHTLCPreimageTestCase) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err, "unable to make test db") +func testUpdateHTLCPreimagesImpl(t *testing.T, test updateHTLCPreimageTestCase, + makeDB func(t *testing.T) invpkg.InvoiceDB) { + db := makeDB(t) // We'll start out by creating an invoice and writing it to the DB. amt := lnwire.NewMSatFromSatoshis(1000) invoice, err := randInvoice(amt) @@ -2350,13 +2476,13 @@ func testUpdateHTLCPreimages(t *testing.T, test updateHTLCPreimageTestCase) { require.Equal(t, test.expError, err) } -// TestDeleteInvoices tests that deleting a list of invoices will succeed +// testDeleteInvoices tests that deleting a list of invoices will succeed // if all delete references are valid, or will fail otherwise. -func TestDeleteInvoices(t *testing.T) { - t.Parallel() +func testDeleteInvoices(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) // Add some invoices to the test db. numInvoices := 3 @@ -2434,13 +2560,13 @@ func TestDeleteInvoices(t *testing.T) { assertInvoiceCount(0) } -// TestDeleteCanceledInvoices tests that deleting canceled invoices with the +// testDeleteCanceledInvoices tests that deleting canceled invoices with the // specific DeleteCanceledInvoices method works correctly. -func TestDeleteCanceledInvoices(t *testing.T) { - t.Parallel() +func testDeleteCanceledInvoices(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) // Updatefunc is used to cancel an invoice. updateFunc := func(invoice *invpkg.Invoice) ( @@ -2493,13 +2619,13 @@ func TestDeleteCanceledInvoices(t *testing.T) { require.Equal(t, invoices, dbInvoices.Invoices) } -// TestAddInvoiceInvalidFeatureDeps asserts that inserting an invoice with +// testAddInvoiceInvalidFeatureDeps asserts that inserting an invoice with // invalid transitive feature dependencies fails with the appropriate error. -func TestAddInvoiceInvalidFeatureDeps(t *testing.T) { - t.Parallel() +func testAddInvoiceInvalidFeatureDeps(t *testing.T, + makeDB func(t *testing.T) invpkg.InvoiceDB) { - db, err := channeldb.MakeTestInvoiceDB(t) - require.NoError(t, err, "unable to make test db") + t.Parallel() + db := makeDB(t) invoice, err := randInvoice(500) require.NoError(t, err) diff --git a/invoices/test_utils_test.go b/invoices/test_utils_test.go index 13cd11184..dd2638408 100644 --- a/invoices/test_utils_test.go +++ b/invoices/test_utils_test.go @@ -16,7 +16,6 @@ import ( "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/lightningnetwork/lnd/chainntnfs" - "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/clock" invpkg "github.com/lightningnetwork/lnd/invoices" "github.com/lightningnetwork/lnd/lntypes" @@ -131,26 +130,8 @@ var ( testInvoiceCreationDate = testTime ) -func newTestChannelDB(t *testing.T, clock clock.Clock) (*channeldb.DB, error) { - t.Helper() - - // Create channeldb for the first time. - cdb, err := channeldb.Open( - t.TempDir(), channeldb.OptionClock(clock), - ) - if err != nil { - return nil, err - } - - t.Cleanup(func() { - cdb.Close() - }) - - return cdb, nil -} - type testContext struct { - idb *channeldb.DB + idb invpkg.InvoiceDB registry *invpkg.InvoiceRegistry notifier *mockChainNotifier clock *clock.TestClock @@ -166,17 +147,13 @@ func defaultRegistryConfig() invpkg.RegistryConfig { } func newTestContext(t *testing.T, - registryCfg *invpkg.RegistryConfig) *testContext { + registryCfg *invpkg.RegistryConfig, + makeDB func(t *testing.T) (invpkg.InvoiceDB, + *clock.TestClock)) *testContext { t.Helper() - clock := clock.NewTestClock(testTime) - - idb, err := newTestChannelDB(t, clock) - if err != nil { - t.Fatal(err) - } - + idb, clock := makeDB(t) notifier := newMockNotifier() expiryWatcher := invpkg.NewInvoiceExpiryWatcher( @@ -192,10 +169,7 @@ func newTestContext(t *testing.T, // Instantiate and start the invoice ctx.registry. registry := invpkg.NewRegistry(idb, expiryWatcher, &cfg) - err = registry.Start() - if err != nil { - t.Fatal(err) - } + require.NoError(t, registry.Start()) t.Cleanup(func() { require.NoError(t, registry.Stop()) })