From de50f471e818aaa14f9dcc03894dd9e1ec038be8 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 15 Oct 2025 18:09:54 +0200 Subject: [PATCH] paymentsdb: make QueryPayments test db agnostic We make the QueryPayments test db agnostic and also keep a small test for querying the duplicate payments case in the kv world. --- payments/db/kv_store_test.go | 215 ++-------------- payments/db/payment_test.go | 475 +++++++++++++++++++++++++++++++++++ 2 files changed, 502 insertions(+), 188 deletions(-) diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go index ccd2e4509..fbc8478d0 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, + firstIndex: 6, + lastIndex: 7, + expectedSeqNrs: []uint64{6, 7}, }, { - 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", + name: "query duplicate payment at end", query: Query{ IndexOffset: 6, MaxPayments: 2, @@ -748,93 +739,31 @@ func TestQueryPayments(t *testing.T) { expectedSeqNrs: []uint64{7}, }, { - name: "start at offset index outside of payments", + name: "query includes duplicate in reverse order", query: Query{ - IndexOffset: 20, + IndexOffset: 0, MaxPayments: 2, - Reversed: false, + Reversed: true, IncludeIncomplete: true, }, - firstIndex: 0, - lastIndex: 0, - expectedSeqNrs: nil, + firstIndex: 6, + lastIndex: 7, + expectedSeqNrs: []uint64{6, 7}, }, { - name: "overflow in forwards order", + name: "query all payments includes duplicate", query: Query{ - IndexOffset: 4, + IndexOffset: 0, 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", - query: Query{ - IndexOffset: 0, - MaxPayments: 2, - Reversed: true, - IncludeIncomplete: true, - }, - firstIndex: 6, - lastIndex: 7, - 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", - query: Query{ - IndexOffset: 0, - MaxPayments: 7, - Reversed: true, - IncludeIncomplete: true, - }, firstIndex: 1, lastIndex: 7, 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 22ef30f48..2c2d668aa 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") + } + }) + } +}