paymentsdb: make QueryPayments test db agnostic
What changed, and why it matters
This commit only moves and refactors test code. It makes an existing payment-querying test work with any database backend and keeps the legacy key-value (KV) store duplicate-payment test separate. No production code was changed, so there is no security risk or fix here.
No security action needed. Treat as ordinary test maintenance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change refactors payments/db tests. It renames TestQueryPayments in kv_store_test.go to TestKVStoreQueryPaymentsDuplicates and trims it to only cover KV-specific legacy duplicate payment handling. It then adds a new, database-agnostic TestQueryPayments in payment_test.go that exercises QueryPayments across forward/reverse pagination, index gaps, creation-time filters, and CountTotal. The diff is entirely in *_test.go files; no implementation code is modified.
Changed components
payments/db/kv_store_test.gopayments/db/payment_test.goInspect captured patch +498 / −184
diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go
index ccd2e45..fbc8478 100644
--- a/payments/db/kv_store_test.go
+++ b/payments/db/kv_store_test.go
@@ -690,17 +690,19 @@ func putDuplicatePayment(t *testing.T, duplicateBucket kvdb.RwBucket,
require.NoError(t, err)
}
-// TestQueryPayments tests retrieval of payments with forwards and reversed
-// queries.
-//
-// TODO(ziggie): Make this test db agnostic.
-func TestQueryPayments(t *testing.T) {
- // Define table driven test for QueryPayments.
+// TestKVStoreQueryPaymentsDuplicates tests the KV store's legacy duplicate
+// payment handling. This tests the specific case where duplicate payments
+// are stored in a nested bucket within the parent payment bucket.
+func TestKVStoreQueryPaymentsDuplicates(t *testing.T) {
+ t.Parallel()
+
// Test payments have sequence indices [1, 3, 4, 5, 6, 7].
// Note that the payment with index 7 has the same payment hash as 6,
// and is stored in a nested bucket within payment 6 rather than being
- // its own entry in the payments bucket. We do this to test retrieval
- // of legacy payments.
+ // its own entry in the payments bucket. This tests retrieval of legacy
+ // duplicate payments which is KV-store specific.
+ // These test cases focus on validating that duplicate payments (seq 7,
+ // nested under payment 6) are correctly returned in queries.
tests := []struct {
name string
query Query
@@ -712,31 +714,20 @@ func TestQueryPayments(t *testing.T) {
expectedSeqNrs []uint64
}{
{
- name: "IndexOffset at the end of the payments range",
+ name: "query includes duplicate payment in forward " +
+ "order",
query: Query{
- IndexOffset: 7,
- MaxPayments: 7,
+ IndexOffset: 5,
+ MaxPayments: 3,
Reversed: false,
IncludeIncomplete: true,
},
- firstIndex: 0,
- lastIndex: 0,
- expectedSeqNrs: nil,
- },
- {
- name: "query in forwards order, start at beginning",
- query: Query{
- IndexOffset: 0,
- MaxPayments: 2,
- Reversed: false,
- IncludeIncomplete: true,
- },
- firstIndex: 1,
- lastIndex: 3,
- expectedSeqNrs: []uint64{1, 3},
+ firstIndex: 6,
+ lastIndex: 7,
+ expectedSeqNrs: []uint64{6, 7},
},
{
- name: "query in forwards order, start at end, overflow",
+ name: "query duplicate payment at end",
query: Query{
IndexOffset: 6,
MaxPayments: 2,
@@ -748,44 +739,7 @@ func TestQueryPayments(t *testing.T) {
expectedSeqNrs: []uint64{7},
},
{
- name: "start at offset index outside of payments",
- query: Query{
- IndexOffset: 20,
- MaxPayments: 2,
- Reversed: false,
- IncludeIncomplete: true,
- },
- firstIndex: 0,
- lastIndex: 0,
- expectedSeqNrs: nil,
- },
- {
- name: "overflow in forwards order",
- query: Query{
- IndexOffset: 4,
- MaxPayments: math.MaxUint64,
- Reversed: false,
- IncludeIncomplete: true,
- },
- firstIndex: 5,
- lastIndex: 7,
- expectedSeqNrs: []uint64{5, 6, 7},
- },
- {
- name: "start at offset index outside of payments, " +
- "reversed order",
- query: Query{
- IndexOffset: 9,
- MaxPayments: 2,
- Reversed: true,
- IncludeIncomplete: true,
- },
- firstIndex: 6,
- lastIndex: 7,
- expectedSeqNrs: []uint64{6, 7},
- },
- {
- name: "query in reverse order, start at end",
+ name: "query includes duplicate in reverse order",
query: Query{
IndexOffset: 0,
MaxPayments: 2,
@@ -797,36 +751,11 @@ func TestQueryPayments(t *testing.T) {
expectedSeqNrs: []uint64{6, 7},
},
{
- name: "query in reverse order, starting in middle",
- query: Query{
- IndexOffset: 4,
- MaxPayments: 2,
- Reversed: true,
- IncludeIncomplete: true,
- },
- firstIndex: 1,
- lastIndex: 3,
- expectedSeqNrs: []uint64{1, 3},
- },
- {
- name: "query in reverse order, starting in middle, " +
- "with underflow",
- query: Query{
- IndexOffset: 4,
- MaxPayments: 5,
- Reversed: true,
- IncludeIncomplete: true,
- },
- firstIndex: 1,
- lastIndex: 3,
- expectedSeqNrs: []uint64{1, 3},
- },
- {
- name: "all payments in reverse, order maintained",
+ name: "query all payments includes duplicate",
query: Query{
IndexOffset: 0,
- MaxPayments: 7,
- Reversed: true,
+ MaxPayments: math.MaxUint64,
+ Reversed: false,
IncludeIncomplete: true,
},
firstIndex: 1,
@@ -834,7 +763,7 @@ func TestQueryPayments(t *testing.T) {
expectedSeqNrs: []uint64{1, 3, 4, 5, 6, 7},
},
{
- name: "exclude incomplete payments",
+ name: "exclude incomplete includes duplicate",
query: Query{
IndexOffset: 0,
MaxPayments: 7,
@@ -845,96 +774,6 @@ func TestQueryPayments(t *testing.T) {
lastIndex: 7,
expectedSeqNrs: []uint64{7},
},
- {
- name: "query payments at index gap",
- query: Query{
- IndexOffset: 1,
- MaxPayments: 7,
- Reversed: false,
- IncludeIncomplete: true,
- },
- firstIndex: 3,
- lastIndex: 7,
- expectedSeqNrs: []uint64{3, 4, 5, 6, 7},
- },
- {
- name: "query payments reverse before index gap",
- query: Query{
- IndexOffset: 3,
- MaxPayments: 7,
- Reversed: true,
- IncludeIncomplete: true,
- },
- firstIndex: 1,
- lastIndex: 1,
- expectedSeqNrs: []uint64{1},
- },
- {
- name: "query payments reverse on index gap",
- query: Query{
- IndexOffset: 2,
- MaxPayments: 7,
- Reversed: true,
- IncludeIncomplete: true,
- },
- firstIndex: 1,
- lastIndex: 1,
- expectedSeqNrs: []uint64{1},
- },
- {
- name: "query payments forward on index gap",
- query: Query{
- IndexOffset: 2,
- MaxPayments: 2,
- Reversed: false,
- IncludeIncomplete: true,
- },
- firstIndex: 3,
- lastIndex: 4,
- expectedSeqNrs: []uint64{3, 4},
- },
- {
- name: "query in forwards order, with start creation " +
- "time",
- query: Query{
- IndexOffset: 0,
- MaxPayments: 2,
- Reversed: false,
- IncludeIncomplete: true,
- CreationDateStart: 5,
- },
- firstIndex: 5,
- lastIndex: 6,
- expectedSeqNrs: []uint64{5, 6},
- },
- {
- name: "query in forwards order, with start creation " +
- "time at end, overflow",
- query: Query{
- IndexOffset: 0,
- MaxPayments: 2,
- Reversed: false,
- IncludeIncomplete: true,
- CreationDateStart: 7,
- },
- firstIndex: 7,
- lastIndex: 7,
- expectedSeqNrs: []uint64{7},
- },
- {
- name: "query with start and end creation time",
- query: Query{
- IndexOffset: 9,
- MaxPayments: math.MaxUint64,
- Reversed: true,
- IncludeIncomplete: true,
- CreationDateStart: 3,
- CreationDateEnd: 5,
- },
- firstIndex: 3,
- lastIndex: 5,
- expectedSeqNrs: []uint64{3, 4, 5},
- },
}
for _, tt := range tests {
diff --git a/payments/db/payment_test.go b/payments/db/payment_test.go
index 22ef30f..2c2d668 100644
--- a/payments/db/payment_test.go
+++ b/payments/db/payment_test.go
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
+ "math"
"reflect"
"testing"
"time"
@@ -2214,3 +2215,477 @@ func TestMultiShard(t *testing.T) {
})
}
}
+
+// TestQueryPayments tests retrieval of payments with forwards and reversed
+// queries.
+func TestQueryPayments(t *testing.T) {
+ // Define table driven test for QueryPayments.
+ // Test payments have sequence indices [1, 3, 4, 5, 6].
+ // Note that payment with index 2 is deleted to create a gap in the
+ // sequence numbers.
+ tests := []struct {
+ name string
+ query Query
+ firstIndex uint64
+ lastIndex uint64
+
+ // expectedSeqNrs contains the set of sequence numbers we expect
+ // our query to return.
+ expectedSeqNrs []uint64
+ }{
+ {
+ name: "IndexOffset at the end of the payments range",
+ query: Query{
+ IndexOffset: 6,
+ MaxPayments: 7,
+ Reversed: false,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 0,
+ lastIndex: 0,
+ expectedSeqNrs: nil,
+ },
+ {
+ name: "query in forwards order, start at beginning",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 2,
+ Reversed: false,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 1,
+ lastIndex: 3,
+ expectedSeqNrs: []uint64{1, 3},
+ },
+ {
+ name: "query in forwards order, start at end, overflow",
+ query: Query{
+ IndexOffset: 5,
+ MaxPayments: 2,
+ Reversed: false,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 6,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{6},
+ },
+ {
+ name: "start at offset index outside of payments",
+ query: Query{
+ IndexOffset: 20,
+ MaxPayments: 2,
+ Reversed: false,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 0,
+ lastIndex: 0,
+ expectedSeqNrs: nil,
+ },
+ {
+ name: "overflow in forwards order",
+ query: Query{
+ IndexOffset: 4,
+ MaxPayments: math.MaxUint64,
+ Reversed: false,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 5,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{5, 6},
+ },
+ {
+ name: "start at offset index outside of payments, " +
+ "reversed order",
+ query: Query{
+ IndexOffset: 9,
+ MaxPayments: 2,
+ Reversed: true,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 5,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{5, 6},
+ },
+ {
+ name: "query in reverse order, start at end",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 2,
+ Reversed: true,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 5,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{5, 6},
+ },
+ {
+ name: "query in reverse order, starting in middle",
+ query: Query{
+ IndexOffset: 4,
+ MaxPayments: 2,
+ Reversed: true,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 1,
+ lastIndex: 3,
+ expectedSeqNrs: []uint64{1, 3},
+ },
+ {
+ name: "query in reverse order, starting in middle, " +
+ "with underflow",
+ query: Query{
+ IndexOffset: 4,
+ MaxPayments: 5,
+ Reversed: true,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 1,
+ lastIndex: 3,
+ expectedSeqNrs: []uint64{1, 3},
+ },
+ {
+ name: "all payments in reverse, order maintained",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 7,
+ Reversed: true,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 1,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{1, 3, 4, 5, 6},
+ },
+ {
+ name: "exclude incomplete payments",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 7,
+ Reversed: false,
+ IncludeIncomplete: false,
+ },
+ firstIndex: 6,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{6},
+ },
+ {
+ name: "query payments at index gap",
+ query: Query{
+ IndexOffset: 1,
+ MaxPayments: 7,
+ Reversed: false,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 3,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{3, 4, 5, 6},
+ },
+ {
+ name: "query payments reverse before index gap",
+ query: Query{
+ IndexOffset: 3,
+ MaxPayments: 7,
+ Reversed: true,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 1,
+ lastIndex: 1,
+ expectedSeqNrs: []uint64{1},
+ },
+ {
+ name: "query payments reverse on index gap",
+ query: Query{
+ IndexOffset: 2,
+ MaxPayments: 7,
+ Reversed: true,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 1,
+ lastIndex: 1,
+ expectedSeqNrs: []uint64{1},
+ },
+ {
+ name: "query payments forward on index gap",
+ query: Query{
+ IndexOffset: 2,
+ MaxPayments: 2,
+ Reversed: false,
+ IncludeIncomplete: true,
+ },
+ firstIndex: 3,
+ lastIndex: 4,
+ expectedSeqNrs: []uint64{3, 4},
+ },
+ {
+ name: "query in forwards order, with start creation " +
+ "time",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 2,
+ Reversed: false,
+ IncludeIncomplete: true,
+ CreationDateStart: 5,
+ },
+ firstIndex: 5,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{5, 6},
+ },
+ {
+ name: "query in forwards order, with start creation " +
+ "time at end, overflow",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 2,
+ Reversed: false,
+ IncludeIncomplete: true,
+ CreationDateStart: 6,
+ },
+ firstIndex: 6,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{6},
+ },
+ {
+ name: "query with start and end creation time",
+ query: Query{
+ IndexOffset: 9,
+ MaxPayments: math.MaxUint64,
+ Reversed: true,
+ IncludeIncomplete: true,
+ CreationDateStart: 3,
+ CreationDateEnd: 5,
+ },
+ firstIndex: 3,
+ lastIndex: 5,
+ expectedSeqNrs: []uint64{3, 4, 5},
+ },
+ {
+ name: "query with only end creation time",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: math.MaxUint64,
+ Reversed: false,
+ IncludeIncomplete: true,
+ CreationDateEnd: 4,
+ },
+ firstIndex: 1,
+ lastIndex: 4,
+ expectedSeqNrs: []uint64{1, 3, 4},
+ },
+ {
+ name: "query reversed with creation date start",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 3,
+ Reversed: true,
+ IncludeIncomplete: true,
+ CreationDateStart: 3,
+ },
+ firstIndex: 4,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{4, 5, 6},
+ },
+ {
+ name: "count total with forward pagination",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 2,
+ Reversed: false,
+ IncludeIncomplete: true,
+ CountTotal: true,
+ },
+ firstIndex: 1,
+ lastIndex: 3,
+ expectedSeqNrs: []uint64{1, 3},
+ },
+ {
+ name: "count total with reverse pagination",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: 2,
+ Reversed: true,
+ IncludeIncomplete: true,
+ CountTotal: true,
+ },
+ firstIndex: 5,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{5, 6},
+ },
+ {
+ name: "count total with filters",
+ query: Query{
+ IndexOffset: 0,
+ MaxPayments: math.MaxUint64,
+ Reversed: false,
+ IncludeIncomplete: false,
+ CountTotal: true,
+ },
+ firstIndex: 6,
+ lastIndex: 6,
+ expectedSeqNrs: []uint64{6},
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ ctx := t.Context()
+
+ paymentDB := NewTestDB(t)
+
+ // Make a preliminary query to make sure it's ok to
+ // query when we have no payments.
+ resp, err := paymentDB.QueryPayments(ctx, tt.query)
+ require.NoError(t, err)
+ require.Len(t, resp.Payments, 0)
+
+ // Populate the database with a set of test payments.
+ // We create 6 payments, deleting the payment at index
+ // 2 so that we cover the case where sequence numbers
+ // are missing.
+ numberOfPayments := 6
+
+ // Store payment info for all payments so we can delete
+ // one after all are created.
+ var paymentInfos []*PaymentCreationInfo
+
+ // First, create all payments.
+ for i := range numberOfPayments {
+ // Generate a test payment.
+ info, _, err := genInfo(t)
+ require.NoError(t, err)
+
+ // Override creation time to allow for testing
+ // of CreationDateStart and CreationDateEnd.
+ info.CreationTime = time.Unix(int64(i+1), 0)
+
+ paymentInfos = append(paymentInfos, info)
+
+ // Create a new payment entry in the database.
+ err = paymentDB.InitPayment(
+ info.PaymentIdentifier, info,
+ )
+ require.NoError(t, err)
+ }
+
+ // Now delete the payment at index 1 (the second
+ // payment).
+ pmt, err := paymentDB.FetchPayment(
+ paymentInfos[1].PaymentIdentifier,
+ )
+ require.NoError(t, err)
+
+ // We delete the whole payment.
+ err = paymentDB.DeletePayment(
+ paymentInfos[1].PaymentIdentifier, false,
+ )
+ require.NoError(t, err)
+
+ // Verify the payment is deleted.
+ _, err = paymentDB.FetchPayment(
+ paymentInfos[1].PaymentIdentifier,
+ )
+ require.ErrorIs(
+ t, err, ErrPaymentNotInitiated,
+ )
+
+ // Verify the index is removed (KV store only).
+ assertNoIndex(
+ t, paymentDB, pmt.SequenceNum,
+ )
+
+ // For the last payment, settle it so we have at least
+ // one completed payment for the "exclude incomplete"
+ // test case.
+ lastPaymentInfo := paymentInfos[numberOfPayments-1]
+ attempt, err := NewHtlcAttempt(
+ 1, priv, testRoute,
+ time.Unix(100, 0),
+ &lastPaymentInfo.PaymentIdentifier,
+ )
+ require.NoError(t, err)
+
+ _, err = paymentDB.RegisterAttempt(
+ lastPaymentInfo.PaymentIdentifier,
+ &attempt.HTLCAttemptInfo,
+ )
+ require.NoError(t, err)
+
+ var preimg lntypes.Preimage
+ copy(preimg[:], rev[:])
+
+ _, err = paymentDB.SettleAttempt(
+ lastPaymentInfo.PaymentIdentifier,
+ attempt.AttemptID,
+ &HTLCSettleInfo{
+ Preimage: preimg,
+ },
+ )
+ require.NoError(t, err)
+
+ // Fetch all payments in the database.
+ resp, err = paymentDB.QueryPayments(
+ ctx, Query{
+ IndexOffset: 0,
+ MaxPayments: math.MaxUint64,
+ IncludeIncomplete: true,
+ },
+ )
+ require.NoError(t, err)
+
+ allPayments := resp.Payments
+
+ if len(allPayments) != 5 {
+ t.Fatalf("Number of payments received does "+
+ "not match expected one. Got %v, "+
+ "want %v.", len(allPayments), 5)
+ }
+
+ querySlice, err := paymentDB.QueryPayments(
+ ctx, tt.query,
+ )
+ require.NoError(t, err)
+
+ if tt.firstIndex != querySlice.FirstIndexOffset ||
+ tt.lastIndex != querySlice.LastIndexOffset {
+
+ t.Errorf("First or last index does not match "+
+ "expected index. Want (%d, %d), "+
+ "got (%d, %d).",
+ tt.firstIndex, tt.lastIndex,
+ querySlice.FirstIndexOffset,
+ querySlice.LastIndexOffset)
+ }
+
+ if len(querySlice.Payments) != len(tt.expectedSeqNrs) {
+ t.Errorf("expected: %v payments, got: %v",
+ len(tt.expectedSeqNrs),
+ len(querySlice.Payments))
+ }
+
+ for i, seqNr := range tt.expectedSeqNrs {
+ q := querySlice.Payments[i]
+ if seqNr != q.SequenceNum {
+ t.Errorf("sequence numbers do not "+
+ "match, got %v, want %v",
+ q.SequenceNum, seqNr)
+ }
+ }
+
+ // Verify CountTotal is set correctly when requested.
+ if tt.query.CountTotal {
+ // We should have 5 total payments
+ // (6 created - 1 deleted).
+ expectedTotal := uint64(5)
+ require.Equal(
+ t, expectedTotal, querySlice.TotalCount,
+ "expected total count %v, got %v",
+ expectedTotal, querySlice.TotalCount)
+ } else {
+ require.Equal(
+ t, uint64(0), querySlice.TotalCount,
+ "expected total count 0 when "+
+ "CountTotal=false")
+ }
+ })
+ }
+}
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.