-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
channeldb+lnrpc+lncli: listpayments pagination support #3960
Merged
Roasbeef
merged 6 commits into
lightningnetwork:master
from
bitromortac:listpayments-pagination
Apr 8, 2020
Merged
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
4800a84
channeldb: export sequenceNum in MPPayment
bitromortac 4c5e8ae
channeldb: add payments query
bitromortac d5dd48f
channeldb/test: unit tests for payments query
bitromortac 39c58d9
lnrpc: use queried payments to list payments in the rpc
bitromortac 97b7597
itest: fix comment in list_outgoing_payments test
bitromortac 4593cfa
lncli: modify listpayments to use queried payments and update cli docs
bitromortac File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import ( | |
"encoding/binary" | ||
"fmt" | ||
"io" | ||
"math" | ||
"sort" | ||
"time" | ||
|
||
|
@@ -423,6 +424,140 @@ func fetchHtlcFailInfo(bucket kvdb.ReadBucket) (*HTLCFailInfo, error) { | |
return deserializeHTLCFailInfo(r) | ||
} | ||
|
||
// PaymentsQuery represents a query to the payments database starting or ending | ||
// at a certain offset index. The number of retrieved records can be limited. | ||
type PaymentsQuery struct { | ||
// IndexOffset determines the starting point of the payments query and | ||
// is always exclusive. In normal order, the query starts at the next | ||
// higher (available) index compared to IndexOffset. In reversed order, | ||
// the query ends at the next lower (available) index compared to the | ||
// IndexOffset. In the case of a zero index_offset, the query will start | ||
// with the oldest payment when paginating forwards, or will end with | ||
// the most recent payment when paginating backwards. | ||
IndexOffset uint64 | ||
|
||
// MaxPayments is the maximal number of payments returned in the | ||
// payments query. | ||
MaxPayments uint64 | ||
|
||
// Reversed gives a meaning to the IndexOffset. If reversed is set to | ||
// true, the query will fetch payments with indices lower than the | ||
// IndexOffset, otherwise, it will return payments with indices greater | ||
// than the IndexOffset. | ||
Reversed bool | ||
|
||
// If IncludeIncomplete is true, then return payments that have not yet | ||
// fully completed. This means that pending payments, as well as failed | ||
// payments will show up if this field is set to true. | ||
IncludeIncomplete bool | ||
bitromortac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// PaymentsResponse contains the result of a query to the payments database. | ||
// It includes the set of payments that match the query and integers which | ||
// represent the index of the first and last item returned in the series of | ||
// payments. These integers allow callers to resume their query in the event | ||
// that the query's response exceeds the max number of returnable events. | ||
type PaymentsResponse struct { | ||
// Payments is the set of payments returned from the database for the | ||
// PaymentsQuery. | ||
Payments []MPPayment | ||
|
||
// FirstIndexOffset is the index of the first element in the set of | ||
// returned MPPayments. Callers can use this to resume their query | ||
// in the event that the slice has too many events to fit into a single | ||
// response. The offset can be used to continue reverse pagination. | ||
FirstIndexOffset uint64 | ||
bitromortac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// LastIndexOffset is the index of the last element in the set of | ||
// returned MPPayments. Callers can use this to resume their query | ||
// in the event that the slice has too many events to fit into a single | ||
// response. The offset can be used to continue forward pagination. | ||
LastIndexOffset uint64 | ||
} | ||
|
||
// QueryPayments is a query to the payments database which is restricted | ||
// to a subset of payments by the payments query, containing an offset | ||
// index and a maximum number of returned payments. | ||
func (db *DB) QueryPayments(query PaymentsQuery) (PaymentsResponse, error) { | ||
var resp PaymentsResponse | ||
|
||
allPayments, err := db.FetchPayments() | ||
bitromortac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return resp, err | ||
bitromortac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if len(allPayments) == 0 { | ||
return resp, nil | ||
} | ||
|
||
indexExclusiveLimit := query.IndexOffset | ||
// In backward pagination, if the index limit is the default 0 value, | ||
// we set our limit to maxint to include all payments from the highest | ||
// sequence number on. | ||
if query.Reversed && indexExclusiveLimit == 0 { | ||
indexExclusiveLimit = math.MaxInt64 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Final comment about this is that this non-functional special casing could also be moved into |
||
} | ||
|
||
for i := range allPayments { | ||
var payment *MPPayment | ||
|
||
// If we have the max number of payments we want, exit. | ||
if uint64(len(resp.Payments)) == query.MaxPayments { | ||
break | ||
} | ||
|
||
if query.Reversed { | ||
payment = allPayments[len(allPayments)-1-i] | ||
|
||
// In the reversed direction, skip over all payments | ||
// that have sequence numbers greater than or equal to | ||
// the index offset. We skip payments with equal index | ||
// because the offset is exclusive. | ||
if payment.SequenceNum >= indexExclusiveLimit { | ||
continue | ||
} | ||
} else { | ||
payment = allPayments[i] | ||
|
||
// In the forward direction, skip over all payments that | ||
// have sequence numbers less than or equal to the index | ||
// offset. We skip payments with equal indexes because | ||
// the index offset is exclusive. | ||
if payment.SequenceNum <= indexExclusiveLimit { | ||
continue | ||
} | ||
} | ||
bitromortac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// To keep compatibility with the old API, we only return | ||
bitromortac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// non-succeeded payments if requested. | ||
if payment.Status != StatusSucceeded && | ||
!query.IncludeIncomplete { | ||
|
||
continue | ||
} | ||
|
||
resp.Payments = append(resp.Payments, *payment) | ||
} | ||
|
||
// Need to swap the payments slice order if reversed order. | ||
if query.Reversed { | ||
joostjager marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for l, r := 0, len(resp.Payments)-1; l < r; l, r = l+1, r-1 { | ||
resp.Payments[l], resp.Payments[r] = | ||
resp.Payments[r], resp.Payments[l] | ||
} | ||
} | ||
|
||
// Set the first and last index of the returned payments so that the | ||
// caller can resume from this point later on. | ||
if len(resp.Payments) > 0 { | ||
bitromortac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
resp.FirstIndexOffset = resp.Payments[0].SequenceNum | ||
resp.LastIndexOffset = | ||
resp.Payments[len(resp.Payments)-1].SequenceNum | ||
} | ||
|
||
return resp, err | ||
} | ||
|
||
// DeletePayments deletes all completed and failed payments from the DB. | ||
func (db *DB) DeletePayments() error { | ||
return kvdb.Update(db, func(tx kvdb.RwTx) error { | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document special case
0
in combined withReversed
? Would almost think that it is more natural to use maxint as the special value. It would then not be a special value anymore. This can also be done while still holding on to the magic0
on the rpc level.Also, the query doesn't necessarily start one index higher than
IndexOffset
. If there are gaps, it may start more than one index higher. I get the point, but it isn't fully correct?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, have corrected both comments. Concerning the maxint, it also works when supplied, but left zero to be the documented special value.