From 080fd78fb4a8b0fcaade2f16bc50d9aad6777138 Mon Sep 17 00:00:00 2001 From: ziggie Date: Fri, 14 Nov 2025 18:20:59 +0100 Subject: [PATCH] paymentsdb: introduce a harness interface The design of the sql and kv db are a bit different. A harness interface is introduced which allows us to unit most of the test and keep the backend specific tests at a minimum. --- payments/db/kv_store_test.go | 78 ------------------------------------ payments/db/payment_test.go | 42 +++++++++++-------- payments/db/test_harness.go | 26 ++++++++++++ payments/db/test_kvdb.go | 73 ++++++++++++++++++++++++++++++++- payments/db/test_postgres.go | 22 +++++++++- payments/db/test_sqlite.go | 22 +++++++++- 6 files changed, 162 insertions(+), 101 deletions(-) create mode 100644 payments/db/test_harness.go diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index fbc8478d0..ee28e12e0 100644 --- a/payments/db/kv_store_test.go +++ b/payments/db/kv_store_test.go @@ -18,7 +18,6 @@ import ( "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" "github.com/lightningnetwork/lnd/tlv" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -252,83 +251,6 @@ func TestKVStoreDeleteNonInFlight(t *testing.T) { require.Equal(t, 1, indexCount) } -type htlcStatus struct { - *HTLCAttemptInfo - settle *lntypes.Preimage - failure *HTLCFailReason -} - -// fetchPaymentIndexEntry gets the payment hash for the sequence number provided -// from our payment indexes bucket. -func fetchPaymentIndexEntry(t *testing.T, p *KVStore, - sequenceNumber uint64) (*lntypes.Hash, error) { - - t.Helper() - - var hash lntypes.Hash - - if err := kvdb.View(p.db, func(tx walletdb.ReadTx) error { - indexBucket := tx.ReadBucket(paymentsIndexBucket) - key := make([]byte, 8) - byteOrder.PutUint64(key, sequenceNumber) - - indexValue := indexBucket.Get(key) - if indexValue == nil { - return ErrNoSequenceNrIndex - } - - r := bytes.NewReader(indexValue) - - var err error - hash, err = deserializePaymentIndex(r) - - return err - }, func() { - hash = lntypes.Hash{} - }); err != nil { - return nil, err - } - - return &hash, nil -} - -// assertPaymentIndex looks up the index for a payment in the db and checks -// that its payment hash matches the expected hash passed in. -func assertPaymentIndex(t *testing.T, p DB, expectedHash lntypes.Hash) { - t.Helper() - - // Only the kv implementation uses the index so we exit early if the - // payment db is not a kv implementation. This helps us to reuse the - // same test for both implementations. - kvPaymentDB, ok := p.(*KVStore) - if !ok { - return - } - - // Lookup the payment so that we have its sequence number and check - // that is has correctly been indexed in the payment indexes bucket. - pmt, err := kvPaymentDB.FetchPayment(expectedHash) - require.NoError(t, err) - - hash, err := fetchPaymentIndexEntry(t, kvPaymentDB, pmt.SequenceNum) - require.NoError(t, err) - assert.Equal(t, expectedHash, *hash) -} - -// assertNoIndex checks that an index for the sequence number provided does not -// exist. -func assertNoIndex(t *testing.T, p DB, seqNr uint64) { - t.Helper() - - kvPaymentDB, ok := p.(*KVStore) - if !ok { - return - } - - _, err := fetchPaymentIndexEntry(t, kvPaymentDB, seqNr) - require.Equal(t, ErrNoSequenceNrIndex, err) -} - func makeFakeInfo(t *testing.T) (*PaymentCreationInfo, *HTLCAttemptInfo) { diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go index df922a455..aa42b4eca 100644 --- a/payments/db/payment_test.go +++ b/payments/db/payment_test.go @@ -103,6 +103,14 @@ var ( } ) +// htlcStatus is a helper structure used in tests to track the status of an HTLC +// attempt, including whether it was settled or failed. +type htlcStatus struct { + *HTLCAttemptInfo + settle *lntypes.Preimage + failure *HTLCFailReason +} + // payment is a helper structure that holds basic information on a test payment, // such as the payment id, the status and the total number of HTLCs attempted. type payment struct { @@ -446,7 +454,7 @@ func TestDeleteFailedAttempts(t *testing.T) { // testDeleteFailedAttempts tests the DeleteFailedAttempts method with the // given keepFailedPaymentAttempts flag as argument. func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { - paymentDB := NewTestDB( + paymentDB, _ := NewTestDB( t, WithKeepFailedPaymentAttempts(keepFailedPaymentAttempts), ) @@ -537,7 +545,7 @@ func testDeleteFailedAttempts(t *testing.T, keepFailedPaymentAttempts bool) { func TestMPPRecordValidation(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB, _ := NewTestDB(t) preimg, err := genPreimage(t) require.NoError(t, err) @@ -638,7 +646,7 @@ func TestMPPRecordValidation(t *testing.T) { func TestDeleteSinglePayment(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB, _ := NewTestDB(t) // Register four payments: // All payments will have one failed HTLC attempt and one HTLC attempt @@ -1581,7 +1589,7 @@ func TestEmptyRoutesGenerateSphinxPacket(t *testing.T) { func TestSuccessesWithoutInFlight(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB, _ := NewTestDB(t) preimg, err := genPreimage(t) require.NoError(t, err) @@ -1604,7 +1612,7 @@ func TestSuccessesWithoutInFlight(t *testing.T) { func TestFailsWithoutInFlight(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB, _ := NewTestDB(t) preimg, err := genPreimage(t) require.NoError(t, err) @@ -1624,7 +1632,7 @@ func TestFailsWithoutInFlight(t *testing.T) { func TestDeletePayments(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB, _ := NewTestDB(t) // Register three payments: // 1. A payment with two failed attempts. @@ -1682,7 +1690,7 @@ func TestDeletePayments(t *testing.T) { func TestSwitchDoubleSend(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB, harness := NewTestDB(t) preimg, err := genPreimage(t) require.NoError(t, err) @@ -1697,7 +1705,7 @@ func TestSwitchDoubleSend(t *testing.T) { err = paymentDB.InitPayment(info.PaymentIdentifier, info) require.NoError(t, err, "unable to send htlc message") - assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) + harness.AssertPaymentIndex(t, info.PaymentIdentifier) assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) @@ -1760,7 +1768,7 @@ func TestSwitchDoubleSend(t *testing.T) { func TestSwitchFail(t *testing.T) { t.Parallel() - paymentDB := NewTestDB(t) + paymentDB, harness := NewTestDB(t) preimg, err := genPreimage(t) require.NoError(t, err) @@ -1774,7 +1782,7 @@ func TestSwitchFail(t *testing.T) { err = paymentDB.InitPayment(info.PaymentIdentifier, info) require.NoError(t, err, "unable to send htlc message") - assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) + harness.AssertPaymentIndex(t, info.PaymentIdentifier) assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) @@ -1808,8 +1816,8 @@ func TestSwitchFail(t *testing.T) { // Check that our index has been updated, and the old index has been // removed. - assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) - assertNoIndex(t, paymentDB, payment.SequenceNum) + harness.AssertPaymentIndex(t, info.PaymentIdentifier) + harness.AssertNoIndex(t, payment.SequenceNum) assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, @@ -1926,7 +1934,7 @@ func TestMultiShard(t *testing.T) { } runSubTest := func(t *testing.T, test testCase) { - paymentDB := NewTestDB(t) + paymentDB, harness := NewTestDB(t) preimg, err := genPreimage(t) require.NoError(t, err) @@ -1938,7 +1946,7 @@ func TestMultiShard(t *testing.T) { err = paymentDB.InitPayment(info.PaymentIdentifier, info) require.NoError(t, err) - assertPaymentIndex(t, paymentDB, info.PaymentIdentifier) + harness.AssertPaymentIndex(t, info.PaymentIdentifier) assertDBPaymentstatus( t, paymentDB, info.PaymentIdentifier, StatusInitiated, ) @@ -2533,7 +2541,7 @@ func TestQueryPayments(t *testing.T) { ctx := t.Context() - paymentDB := NewTestDB(t) + paymentDB, harness := NewTestDB(t) // Make a preliminary query to make sure it's ok to // query when we have no payments. @@ -2592,8 +2600,8 @@ func TestQueryPayments(t *testing.T) { ) // Verify the index is removed (KV store only). - assertNoIndex( - t, paymentDB, pmt.SequenceNum, + harness.AssertNoIndex( + t, pmt.SequenceNum, ) // For the last payment, settle it so we have at least diff --git a/payments/db/test_harness.go b/payments/db/test_harness.go new file mode 100644 index 000000000..11f88c3f8 --- /dev/null +++ b/payments/db/test_harness.go @@ -0,0 +1,26 @@ +package paymentsdb + +import ( + "testing" + + "github.com/lightningnetwork/lnd/lntypes" +) + +// TestHarness provides implementation-specific test utilities for the payments +// database. Different database backends (KV, SQL) have different internal +// structures and indexing mechanisms, so this interface allows tests to verify +// implementation-specific behavior without coupling the test logic to a +// particular backend. +type TestHarness interface { + // AssertPaymentIndex checks that a payment is correctly indexed. + // For KV: verifies the payment index bucket entry exists and points + // to the correct payment hash. + // For SQL: no-op (SQL doesn't use a separate index bucket). + AssertPaymentIndex(t *testing.T, expectedHash lntypes.Hash) + + // AssertNoIndex checks that an index for a sequence number doesn't + // exist. + // For KV: verifies the index bucket entry is deleted. + // For SQL: no-op. + AssertNoIndex(t *testing.T, seqNr uint64) +} diff --git a/payments/db/test_kvdb.go b/payments/db/test_kvdb.go index a4bbfccbd..ed1710b14 100644 --- a/payments/db/test_kvdb.go +++ b/payments/db/test_kvdb.go @@ -3,14 +3,18 @@ package paymentsdb import ( + "bytes" "testing" + "github.com/btcsuite/btcwallet/walletdb" "github.com/lightningnetwork/lnd/kvdb" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) // NewTestDB is a helper function that creates an BBolt database for testing. -func NewTestDB(t *testing.T, opts ...OptionModifier) DB { +func NewTestDB(t *testing.T, opts ...OptionModifier) (DB, TestHarness) { backend, backendCleanup, err := kvdb.GetTestBackend( t.TempDir(), "paymentsDB", ) @@ -21,7 +25,7 @@ func NewTestDB(t *testing.T, opts ...OptionModifier) DB { paymentDB, err := NewKVStore(backend, opts...) require.NoError(t, err) - return paymentDB + return paymentDB, &kvTestHarness{db: paymentDB} } // NewKVTestDB is a helper function that creates an BBolt database for testing @@ -40,3 +44,68 @@ func NewKVTestDB(t *testing.T, opts ...OptionModifier) *KVStore { return paymentDB } + +// kvTestHarness is the KV-specific test harness implementation. +type kvTestHarness struct { + db *KVStore +} + +// AssertPaymentIndex looks up the index for a payment in the db and checks +// that its payment hash matches the expected hash passed in. +func (h *kvTestHarness) AssertPaymentIndex(t *testing.T, + expectedHash lntypes.Hash) { + + t.Helper() + + // Lookup the payment so that we have its sequence number and check + // that it has correctly been indexed in the payment indexes bucket. + pmt, err := h.db.FetchPayment(expectedHash) + require.NoError(t, err) + + hash, err := h.fetchPaymentIndexEntry(t, pmt.SequenceNum) + require.NoError(t, err) + assert.Equal(t, expectedHash, *hash) +} + +// AssertNoIndex checks that an index for the sequence number provided does not +// exist. +func (h *kvTestHarness) AssertNoIndex(t *testing.T, seqNr uint64) { + t.Helper() + + _, err := h.fetchPaymentIndexEntry(t, seqNr) + require.Equal(t, ErrNoSequenceNrIndex, err) +} + +// fetchPaymentIndexEntry gets the payment hash for the sequence number +// provided from the payment indexes bucket. +func (h *kvTestHarness) fetchPaymentIndexEntry(t *testing.T, + sequenceNumber uint64) (*lntypes.Hash, error) { + + t.Helper() + + var hash lntypes.Hash + + if err := kvdb.View(h.db.db, func(tx walletdb.ReadTx) error { + indexBucket := tx.ReadBucket(paymentsIndexBucket) + key := make([]byte, 8) + byteOrder.PutUint64(key, sequenceNumber) + + indexValue := indexBucket.Get(key) + if indexValue == nil { + return ErrNoSequenceNrIndex + } + + r := bytes.NewReader(indexValue) + + var err error + hash, err = deserializePaymentIndex(r) + + return err + }, func() { + hash = lntypes.Hash{} + }); err != nil { + return nil, err + } + + return &hash, nil +} diff --git a/payments/db/test_postgres.go b/payments/db/test_postgres.go index b4f00f9b0..bd22703f1 100644 --- a/payments/db/test_postgres.go +++ b/payments/db/test_postgres.go @@ -6,14 +6,16 @@ import ( "database/sql" "testing" + "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/sqldb" "github.com/stretchr/testify/require" ) // NewTestDB is a helper function that creates a SQLStore backed by a SQL // database for testing. -func NewTestDB(t testing.TB, opts ...OptionModifier) DB { - return NewTestDBWithFixture(t, nil, opts...) +func NewTestDB(t testing.TB, opts ...OptionModifier) (DB, TestHarness) { + db := NewTestDBWithFixture(t, nil, opts...) + return db, &noopTestHarness{} } // NewTestDBFixture creates a new sqldb.TestPgFixture for testing purposes. @@ -75,3 +77,19 @@ func newBatchQuerierWithFixture(t testing.TB, }, ) } + +// noopTestHarness is the SQL test harness implementation. Since SQL doesn't +// use a separate payment index bucket like KV, these assertions are no-ops. +type noopTestHarness struct{} + +// AssertPaymentIndex is a no-op for SQL implementations. +func (h *noopTestHarness) AssertPaymentIndex(t *testing.T, + expectedHash lntypes.Hash) { + + // No-op: SQL doesn't use a separate index bucket. +} + +// AssertNoIndex is a no-op for SQL implementations. +func (h *noopTestHarness) AssertNoIndex(t *testing.T, seqNr uint64) { + // No-op: SQL doesn't use a separate index bucket. +} diff --git a/payments/db/test_sqlite.go b/payments/db/test_sqlite.go index 8664db484..99d104780 100644 --- a/payments/db/test_sqlite.go +++ b/payments/db/test_sqlite.go @@ -6,14 +6,16 @@ import ( "database/sql" "testing" + "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/sqldb" "github.com/stretchr/testify/require" ) // NewTestDB is a helper function that creates a SQLStore backed by a SQL // database for testing. -func NewTestDB(t testing.TB, opts ...OptionModifier) DB { - return NewTestDBWithFixture(t, nil, opts...) +func NewTestDB(t testing.TB, opts ...OptionModifier) (DB, TestHarness) { + db := NewTestDBWithFixture(t, nil, opts...) + return db, &noopTestHarness{} } // NewTestDBFixture is a no-op for the sqlite build. @@ -54,3 +56,19 @@ func newBatchQuerierWithFixture(t testing.TB, }, ) } + +// noopTestHarness is the SQL test harness implementation. Since SQL doesn't +// use a separate payment index bucket like KV, these assertions are no-ops. +type noopTestHarness struct{} + +// AssertPaymentIndex is a no-op for SQL implementations. +func (h *noopTestHarness) AssertPaymentIndex(t *testing.T, + expectedHash lntypes.Hash) { + + // No-op: SQL doesn't use a separate index bucket. +} + +// AssertNoIndex is a no-op for SQL implementations. +func (h *noopTestHarness) AssertNoIndex(t *testing.T, seqNr uint64) { + // No-op: SQL doesn't use a separate index bucket. +}