Merge pull request #8432 from yyforyongyu/fix-timestamp-precision

invoice+payment: fix timestamp precision for queries
2024-11-19 01:43:16 +01:00 · 2024-02-05 18:28:39 +08:00 · 2024-02-05 18:28:39 +08:00 · 63e698ec49
commit 63e698ec49
parent 4584f7e39c 4da9582862
9 changed files with 157 additions and 147 deletions
--- a/channeldb/invoice_test.go
+++ b/channeldb/invoice_test.go
@ -1407,7 +1407,7 @@ func TestQueryInvoices(t *testing.T) {
 		{
 			query: invpkg.InvoiceQuery{
 				NumMaxInvoices:  numInvoices,
-				CreationDateEnd: time.Unix(25, 0),
+				CreationDateEnd: 25,
 			},
 			expected: invoices[:25],
 		},
@ -1415,7 +1415,7 @@ func TestQueryInvoices(t *testing.T) {
 		{
 			query: invpkg.InvoiceQuery{
 				NumMaxInvoices:    numInvoices,
-				CreationDateStart: time.Unix(26, 0),
+				CreationDateStart: 26,
 			},
 			expected: invoices[25:],
 		},
@ -1424,7 +1424,7 @@ func TestQueryInvoices(t *testing.T) {
 			query: invpkg.InvoiceQuery{
 				PendingOnly:     true,
 				NumMaxInvoices:  numInvoices,
-				CreationDateEnd: time.Unix(25, 0),
+				CreationDateEnd: 25,
 			},
 			expected: pendingInvoices[:13],
 		},
@ -1433,7 +1433,7 @@ func TestQueryInvoices(t *testing.T) {
 			query: invpkg.InvoiceQuery{
 				PendingOnly:       true,
 				NumMaxInvoices:    numInvoices,
-				CreationDateStart: time.Unix(26, 0),
+				CreationDateStart: 26,
 			},
 			expected: pendingInvoices[13:],
 		},
@ -1442,7 +1442,7 @@ func TestQueryInvoices(t *testing.T) {
 			query: invpkg.InvoiceQuery{
 				IndexOffset:     20,
 				NumMaxInvoices:  numInvoices,
-				CreationDateEnd: time.Unix(30, 0),
+				CreationDateEnd: 30,
 			},
 			// Since we're skipping to invoice 20 and iterating
 			// to invoice 30, we'll expect those invoices.
@ -1455,7 +1455,7 @@ func TestQueryInvoices(t *testing.T) {
 				IndexOffset:       21,
 				Reversed:          true,
 				NumMaxInvoices:    numInvoices,
-				CreationDateStart: time.Unix(11, 0),
+				CreationDateStart: 11,
 			},
 			// Since we're skipping to invoice 20 and iterating
 			// backward to invoice 10, we'll expect those invoices.
@ -1465,8 +1465,8 @@ func TestQueryInvoices(t *testing.T) {
 		{
 			query: invpkg.InvoiceQuery{
 				NumMaxInvoices:    numInvoices,
-				CreationDateStart: time.Unix(11, 0),
-				CreationDateEnd:   time.Unix(20, 0),
+				CreationDateStart: 11,
+				CreationDateEnd:   20,
 			},
 			expected: invoices[10:20],
 		},
@ -1475,8 +1475,8 @@ func TestQueryInvoices(t *testing.T) {
 			query: invpkg.InvoiceQuery{
 				PendingOnly:       true,
 				NumMaxInvoices:    numInvoices,
-				CreationDateStart: time.Unix(11, 0),
-				CreationDateEnd:   time.Unix(20, 0),
+				CreationDateStart: 11,
+				CreationDateEnd:   20,
 			},
 			expected: pendingInvoices[5:10],
 		},
@ -1486,8 +1486,8 @@ func TestQueryInvoices(t *testing.T) {
 			query: invpkg.InvoiceQuery{
 				Reversed:          true,
 				NumMaxInvoices:    numInvoices,
-				CreationDateStart: time.Unix(11, 0),
-				CreationDateEnd:   time.Unix(20, 0),
+				CreationDateStart: 11,
+				CreationDateEnd:   20,
 			},
 			expected: invoices[10:20],
 		},
@ -1498,8 +1498,8 @@ func TestQueryInvoices(t *testing.T) {
 				PendingOnly:       true,
 				Reversed:          true,
 				NumMaxInvoices:    numInvoices,
-				CreationDateStart: time.Unix(11, 0),
-				CreationDateEnd:   time.Unix(20, 0),
+				CreationDateStart: 11,
+				CreationDateEnd:   20,
 			},
 			expected: pendingInvoices[5:10],
 		},
@ -1508,8 +1508,8 @@ func TestQueryInvoices(t *testing.T) {
 		{
 			query: invpkg.InvoiceQuery{
 				NumMaxInvoices:    numInvoices,
-				CreationDateStart: time.Unix(20, 0),
-				CreationDateEnd:   time.Unix(11, 0),
+				CreationDateStart: 20,
+				CreationDateEnd:   11,
 			},
 			expected: nil,
 		},
--- a/channeldb/invoices.go
+++ b/channeldb/invoices.go
@ -497,11 +497,7 @@ func (d *DB) FetchPendingInvoices(_ context.Context) (
 func (d *DB) QueryInvoices(_ context.Context, q invpkg.InvoiceQuery) (
 	invpkg.InvoiceSlice, error) {

-	var (
-		resp         invpkg.InvoiceSlice
-		startDateSet = !q.CreationDateStart.IsZero()
-		endDateSet   = !q.CreationDateEnd.IsZero()
-	)
+	var resp invpkg.InvoiceSlice

 	err := kvdb.View(d, func(tx kvdb.RTx) error {
 		// If the bucket wasn't found, then there aren't any invoices
@ -541,20 +537,20 @@ func (d *DB) QueryInvoices(_ context.Context, q invpkg.InvoiceQuery) (
 				return false, nil
 			}

+			// Get the creation time in Unix seconds, this always
+			// rounds down the nanoseconds to full seconds.
+			createTime := invoice.CreationDate.Unix()
+
 			// Skip any invoices that were created before the
 			// specified time.
-			if startDateSet && invoice.CreationDate.Before(
-				q.CreationDateStart,
-			) {
-
+			if createTime < q.CreationDateStart {
 				return false, nil
 			}

 			// Skip any invoices that were created after the
 			// specified time.
-			if endDateSet && invoice.CreationDate.After(
-				q.CreationDateEnd,
-			) {
+			if q.CreationDateEnd != 0 &&
+				createTime > q.CreationDateEnd {

 				return false, nil
 			}
--- a/channeldb/payments.go
+++ b/channeldb/payments.go
@ -471,13 +471,13 @@ type PaymentsQuery struct {
 	// payment index (complete and incomplete) should be counted.
 	CountTotal bool

-	// CreationDateStart, if set, filters out all payments with a creation
-	// date greater than or euqal to it.
-	CreationDateStart time.Time
+	// CreationDateStart, expressed in Unix seconds, if set, filters out
+	// all payments with a creation date greater than or equal to it.
+	CreationDateStart int64

-	// CreationDateEnd, if set, filters out all payments with a creation
-	// date less than or euqal to it.
-	CreationDateEnd time.Time
+	// CreationDateEnd, expressed in Unix seconds, if set, filters out all
+	// payments with a creation date less than or equal to it.
+	CreationDateEnd int64
 }

 // PaymentsResponse contains the result of a query to the payments database.
@ -512,11 +512,7 @@ type PaymentsResponse struct {
 // to a subset of payments by the payments query, containing an offset
 // index and a maximum number of returned payments.
 func (d *DB) QueryPayments(query PaymentsQuery) (PaymentsResponse, error) {
-	var (
-		resp         PaymentsResponse
-		startDateSet = !query.CreationDateStart.IsZero()
-		endDateSet   = !query.CreationDateEnd.IsZero()
-	)
+	var resp PaymentsResponse

 	if err := kvdb.View(d, func(tx kvdb.RTx) error {
 		// Get the root payments bucket.
@ -561,20 +557,20 @@ func (d *DB) QueryPayments(query PaymentsQuery) (PaymentsResponse, error) {
 				return false, err
 			}

+			// Get the creation time in Unix seconds, this always
+			// rounds down the nanoseconds to full seconds.
+			createTime := payment.Info.CreationTime.Unix()
+
 			// Skip any payments that were created before the
 			// specified time.
-			if startDateSet && payment.Info.CreationTime.Before(
-				query.CreationDateStart,
-			) {
-
+			if createTime < query.CreationDateStart {
 				return false, nil
 			}

 			// Skip any payments that were created after the
 			// specified time.
-			if endDateSet && payment.Info.CreationTime.After(
-				query.CreationDateEnd,
-			) {
+			if query.CreationDateEnd != 0 &&
+				createTime > query.CreationDateEnd {

 				return false, nil
 			}
--- a/channeldb/payments_test.go
+++ b/channeldb/payments_test.go
@ -438,7 +438,7 @@ func TestQueryPayments(t *testing.T) {
 				MaxPayments:       2,
 				Reversed:          false,
 				IncludeIncomplete: true,
-				CreationDateStart: time.Unix(0, 5),
+				CreationDateStart: 5,
 			},
 			firstIndex:     5,
 			lastIndex:      6,
@ -452,7 +452,7 @@ func TestQueryPayments(t *testing.T) {
 				MaxPayments:       2,
 				Reversed:          false,
 				IncludeIncomplete: true,
-				CreationDateStart: time.Unix(0, 7),
+				CreationDateStart: 7,
 			},
 			firstIndex:     7,
 			lastIndex:      7,
@ -465,8 +465,8 @@ func TestQueryPayments(t *testing.T) {
 				MaxPayments:       math.MaxUint64,
 				Reversed:          true,
 				IncludeIncomplete: true,
-				CreationDateStart: time.Unix(0, 3),
-				CreationDateEnd:   time.Unix(0, 5),
+				CreationDateStart: 3,
+				CreationDateEnd:   5,
 			},
 			firstIndex:     3,
 			lastIndex:      5,
@ -509,7 +509,7 @@ func TestQueryPayments(t *testing.T) {
 				}
 				// Override creation time to allow for testing
 				// of CreationDateStart and CreationDateEnd.
-				info.CreationTime = time.Unix(0, int64(i+1))
+				info.CreationTime = time.Unix(int64(i+1), 0)

 				// Create a new payment entry in the database.
 				err = pControl.InitPayment(info.PaymentIdentifier, info)
--- a/docs/release-notes/release-notes-0.18.0.md
+++ b/docs/release-notes/release-notes-0.18.0.md
@ -81,6 +81,10 @@
 * [Fixed](https://github.com/lightningnetwork/lnd/pull/7852) the payload size
  calculation in our pathfinder because blinded hops introduced new tlv records.

+* [Fixed](https://github.com/lightningnetwork/lnd/pull/8432) a timestamp
+  precision issue when querying payments and invoices using the start and end
+  date filters.
+
 # New Features
 ## Functional Enhancements

--- a/invoices/interface.go
+++ b/invoices/interface.go
@ -2,7 +2,6 @@ package invoices

 import (
 	"context"
-	"time"

 	"github.com/lightningnetwork/lnd/channeldb/models"
 	"github.com/lightningnetwork/lnd/lntypes"
@ -126,13 +125,13 @@ type InvoiceQuery struct {
 	// from the IndexOffset and go backwards.
 	Reversed bool

-	// CreationDateStart, if set, filters out all invoices with a creation
-	// date greater than or euqal to it.
-	CreationDateStart time.Time
+	// CreationDateStart, expressed in Unix seconds, if set, filters out
+	// all invoices with a creation date greater than or equal to it.
+	CreationDateStart int64

 	// CreationDateEnd, if set, filters out all invoices with a creation
-	// date less than or euqal to it.
-	CreationDateEnd time.Time
+	// date less than or equal to it.
+	CreationDateEnd int64
 }

 // InvoiceSlice is the response to a invoice query. It includes the original
--- a/itest/list_on_test.go
+++ b/itest/list_on_test.go
@ -174,7 +174,7 @@ var allTestCases = []*lntest.TestCase{
 		TestFunc: testUpdateNodeAnnouncement,
 	},
 	{
-		Name:     "list outgoing payments",
+		Name:     "list payments",
 		TestFunc: testListPayments,
 	},
 	{
--- a/itest/lnd_payment_test.go
+++ b/itest/lnd_payment_test.go
@ -153,13 +153,6 @@ func testListPayments(ht *lntest.HarnessTest) {
 		alice, bob, lntest.OpenChannelParams{Amt: chanAmt},
 	)

-	// Get the number of invoices Bob already has.
-	//
-	// TODO(yy): we can remove this check once the `DeleteAllInvoices` rpc
-	// is added.
-	invResp := bob.RPC.ListInvoices(nil)
-	numOldInvoices := len(invResp.Invoices)
-
 	// Now that the channel is open, create an invoice for Bob which
 	// expects a payment of 1000 satoshis from Alice paid via a particular
 	// preimage.
@ -173,8 +166,7 @@ func testListPayments(ht *lntest.HarnessTest) {
 	invoiceResp := bob.RPC.AddInvoice(invoice)

 	// Check that Bob has added the invoice.
-	numInvoices := numOldInvoices + 1
-	ht.AssertNumInvoices(bob, 1)
+	invoice = ht.AssertNumInvoices(bob, 1)[0]

 	// With the invoice for Bob added, send a payment towards Alice paying
 	// to the above generated invoice.
@ -210,65 +202,108 @@ func testListPayments(ht *lntest.HarnessTest) {
 	require.Equal(ht, invoiceResp.PaymentRequest, p.PaymentRequest,
 		"incorrect payreq")

+	// testCase holds a case to be used by both the payment and the invoice
+	// tests.
+	type testCase struct {
+		name      string
+		startDate uint64
+		endDate   uint64
+		expected  bool
+	}
+
+	// Create test cases to check the timestamp filters.
+	createCases := func(createTimeSeconds uint64) []testCase {
+		return []testCase{
+			{
+				// Use a start date same as the creation date
+				// should return us the item.
+				name:      "exact start date",
+				startDate: createTimeSeconds,
+				expected:  true,
+			},
+			{
+				// Use an earlier start date should return us
+				// the item.
+				name:      "earlier start date",
+				startDate: createTimeSeconds - 1,
+				expected:  true,
+			},
+			{
+				// Use a future start date should return us
+				// nothing.
+				name:      "future start date",
+				startDate: createTimeSeconds + 1,
+				expected:  false,
+			},
+			{
+				// Use an end date same as the creation date
+				// should return us the item.
+				name:     "exact end date",
+				endDate:  createTimeSeconds,
+				expected: true,
+			},
+			{
+				// Use an end date in the future should return
+				// us the item.
+				name:     "future end date",
+				endDate:  createTimeSeconds + 1,
+				expected: true,
+			},
+			{
+				// Use an earlier end date should return us
+				// nothing.
+				name:     "earlier end date",
+				endDate:  createTimeSeconds - 1,
+				expected: false,
+			},
+		}
+	}
+
+	// Get the payment creation time in seconds.
+	paymentCreateSeconds := uint64(
+		p.CreationTimeNs / time.Second.Nanoseconds(),
+	)
+
+	// Create test cases from the payment creation time.
+	testCases := createCases(paymentCreateSeconds)
+
 	// We now check the timestamp filters in `ListPayments`.
-	//
-	// Use a start date long time ago should return us the payment.
-	req := &lnrpc.ListPaymentsRequest{
-		CreationDateStart: 1227035905,
-	}
-	resp := alice.RPC.ListPayments(req)
-	require.Len(ht, resp.Payments, 1)
+	for _, tc := range testCases {
+		ht.Run("payment_"+tc.name, func(t *testing.T) {
+			req := &lnrpc.ListPaymentsRequest{
+				CreationDateStart: tc.startDate,
+				CreationDateEnd:   tc.endDate,
+			}
+			resp := alice.RPC.ListPayments(req)

-	// Use an end date long time ago should return us nothing.
-	req = &lnrpc.ListPaymentsRequest{
-		CreationDateEnd: 1227035905,
+			if tc.expected {
+				require.Lenf(t, resp.Payments, 1, "req=%v", req)
+			} else {
+				require.Emptyf(t, resp.Payments, "req=%v", req)
+			}
+		})
 	}
-	resp = alice.RPC.ListPayments(req)
-	require.Empty(ht, resp.Payments)

-	// Use a start date far in the future should return us nothing.
-	req = &lnrpc.ListPaymentsRequest{
-		CreationDateStart: 5392552705,
-	}
-	resp = alice.RPC.ListPayments(req)
-	require.Empty(ht, resp.Payments)
+	// Create test cases from the invoice creation time.
+	testCases = createCases(uint64(invoice.CreationDate))

-	// Use an end date far in the future should return us the payment.
-	req = &lnrpc.ListPaymentsRequest{
-		CreationDateEnd: 5392552705,
-	}
-	resp = alice.RPC.ListPayments(req)
-	require.Len(ht, resp.Payments, 1)
+	// We now do the same check for `ListInvoices`.
+	for _, tc := range testCases {
+		ht.Run("invoice_"+tc.name, func(t *testing.T) {
+			req := &lnrpc.ListInvoiceRequest{
+				CreationDateStart: tc.startDate,
+				CreationDateEnd:   tc.endDate,
+			}
+			resp := bob.RPC.ListInvoices(req)

-	// We now do the same check for `ListInvoices`
-	//
-	// Use a start date long time ago should return us the invoice.
-	invReq := &lnrpc.ListInvoiceRequest{
-		CreationDateStart: 1227035905,
+			if tc.expected {
+				require.Lenf(t, resp.Invoices, 1, "req: %v",
+					req)
+			} else {
+				require.Emptyf(t, resp.Invoices, "req: %v", req)
+			}
+		})
 	}
-	invResp = bob.RPC.ListInvoices(invReq)
-	require.Len(ht, invResp.Invoices, numInvoices)
-
-	// Use an end date long time ago should return us nothing.
-	invReq = &lnrpc.ListInvoiceRequest{
-		CreationDateEnd: 1227035905,
-	}
-	invResp = bob.RPC.ListInvoices(invReq)
-	require.Empty(ht, invResp.Invoices)
-
-	// Use a start date far in the future should return us nothing.
-	invReq = &lnrpc.ListInvoiceRequest{
-		CreationDateStart: 5392552705,
-	}
-	invResp = bob.RPC.ListInvoices(invReq)
-	require.Empty(ht, invResp.Invoices)
-
-	// Use an end date far in the future should return us the invoice.
-	invReq = &lnrpc.ListInvoiceRequest{
-		CreationDateEnd: 5392552705,
-	}
-	invResp = bob.RPC.ListInvoices(invReq)
-	require.Len(ht, invResp.Invoices, numInvoices)

 	// Delete all payments from Alice. DB should have no payments.
 	alice.RPC.DeleteAllPayments()
--- a/rpcserver.go
+++ b/rpcserver.go
@ -5796,20 +5796,12 @@ func (r *rpcServer) ListInvoices(ctx context.Context,
 	// Next, we'll map the proto request into a format that is understood by
 	// the database.
 	q := invoices.InvoiceQuery{
-		IndexOffset:    req.IndexOffset,
-		NumMaxInvoices: req.NumMaxInvoices,
-		PendingOnly:    req.PendingOnly,
-		Reversed:       req.Reversed,
-	}
-
-	// Attach the start date if set.
-	if req.CreationDateStart != 0 {
-		q.CreationDateStart = time.Unix(int64(req.CreationDateStart), 0)
-	}
-
-	// Attach the end date if set.
-	if req.CreationDateEnd != 0 {
-		q.CreationDateEnd = time.Unix(int64(req.CreationDateEnd), 0)
+		IndexOffset:       req.IndexOffset,
+		NumMaxInvoices:    req.NumMaxInvoices,
+		PendingOnly:       req.PendingOnly,
+		Reversed:          req.Reversed,
+		CreationDateStart: int64(req.CreationDateStart),
+		CreationDateEnd:   int64(req.CreationDateEnd),
 	}

 	invoiceSlice, err := r.server.miscDB.QueryInvoices(ctx, q)
@ -6645,20 +6637,8 @@ func (r *rpcServer) ListPayments(ctx context.Context,
 		Reversed:          req.Reversed,
 		IncludeIncomplete: req.IncludeIncomplete,
 		CountTotal:        req.CountTotalPayments,
-	}
-
-	// Attach the start date if set.
-	if req.CreationDateStart != 0 {
-		query.CreationDateStart = time.Unix(
-			int64(req.CreationDateStart), 0,
-		)
-	}
-
-	// Attach the end date if set.
-	if req.CreationDateEnd != 0 {
-		query.CreationDateEnd = time.Unix(
-			int64(req.CreationDateEnd), 0,
-		)
+		CreationDateStart: int64(req.CreationDateStart),
+		CreationDateEnd:   int64(req.CreationDateEnd),
 	}

 	// If the maximum number of payments wasn't specified, then we'll