diff --git a/channeldb/invoice_test.go b/channeldb/invoice_test.go index 3f189c8fa6..d7bbc5b5c5 100644 --- 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, }, diff --git a/channeldb/invoices.go b/channeldb/invoices.go index f163f40b8b..9ffc6aa27d 100644 --- 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 } diff --git a/channeldb/payments.go b/channeldb/payments.go index c2d32ee8a0..15bcd83424 100644 --- 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 } diff --git a/channeldb/payments_test.go b/channeldb/payments_test.go index 37a7e30e72..769f4cc77f 100644 --- 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) diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md index 524231c175..1323c1f0ef 100644 --- 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 diff --git a/invoices/interface.go b/invoices/interface.go index 0da031842a..73274dfb5f 100644 --- 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 diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 390e9a49e2..e8cf80c5f6 100644 --- 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, }, { diff --git a/itest/lnd_payment_test.go b/itest/lnd_payment_test.go index d3d61582b8..143b645fde 100644 --- 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") - // 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, + // 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 } - resp := alice.RPC.ListPayments(req) - require.Len(ht, resp.Payments, 1) - // Use an end date long time ago should return us nothing. - req = &lnrpc.ListPaymentsRequest{ - CreationDateEnd: 1227035905, + // 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, + }, + } } - 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) + // Get the payment creation time in seconds. + paymentCreateSeconds := uint64( + p.CreationTimeNs / time.Second.Nanoseconds(), + ) - // 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` - // - // Use a start date long time ago should return us the invoice. - invReq := &lnrpc.ListInvoiceRequest{ - CreationDateStart: 1227035905, - } - invResp = bob.RPC.ListInvoices(invReq) - require.Len(ht, invResp.Invoices, numInvoices) + // Create test cases from the payment creation time. + testCases := createCases(paymentCreateSeconds) - // 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) + // We now check the timestamp filters in `ListPayments`. + 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 a start date far in the future should return us nothing. - invReq = &lnrpc.ListInvoiceRequest{ - CreationDateStart: 5392552705, + if tc.expected { + require.Lenf(t, resp.Payments, 1, "req=%v", req) + } else { + require.Emptyf(t, resp.Payments, "req=%v", req) + } + }) } - 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, + // Create test cases from the invoice creation time. + testCases = createCases(uint64(invoice.CreationDate)) + + // 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) + + 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) // Delete all payments from Alice. DB should have no payments. alice.RPC.DeleteAllPayments() diff --git a/rpcserver.go b/rpcserver.go index 370dfd9920..e415606b7b 100644 --- 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