What changed, and why it matters
This commit only reorganizes test code for the payments database. It introduces a small test helper interface so the same unit tests can run against both the older key-value (KV/Bolt) backend and the newer SQL backend. No production code is changed, and nothing about how real user payments are handled is modified.
No security action needed; this is a test-only refactoring change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change refactors payments/db tests: it moves KV-specific index assertions (assertPaymentIndex/assertNoIndex) from kv_store_test.go and payment_test.go into a new TestHarness interface with a KV implementation (kvTestHarness) and no-op SQL implementations (noopTestHarness). NewTestDB now returns both a DB and a TestHarness. The production DB code remains untouched.
Changed components
payments/db test files onlyInspect captured patch +162 / −101
diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go
index fbc8478..ee28e12 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 df922a4..aa42b4e 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 0000000..11f88c3
--- /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 a4bbfcc..ed1710b 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 b4f00f9..bd22703 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 8664db4..99d1047 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.
+}
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.