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

@@ -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) {
 

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)
+	harness.AssertPaymentIndex(t, info.PaymentIdentifier)
-	assertNoIndex(t, paymentDB, payment.SequenceNum)
+	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(
+			harness.AssertNoIndex(
-				t, paymentDB, pmt.SequenceNum,
+				t, pmt.SequenceNum,
 			)
 
 			// For the last payment, settle it so we have at least

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)
+}

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
+}

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 {
+func NewTestDB(t testing.TB, opts ...OptionModifier) (DB, TestHarness) {
-	return NewTestDBWithFixture(t, nil, opts...)
+	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.
+}

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 {
+func NewTestDB(t testing.TB, opts ...OptionModifier) (DB, TestHarness) {
-	return NewTestDBWithFixture(t, nil, opts...)
+	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.
+}