-
Notifications
You must be signed in to change notification settings - Fork 906
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
add listing options to listpays #7385
add listing options to listpays #7385
Conversation
1a2bf46
to
c3f185a
Compare
Hi @daywalker90. The way I read it it seems that the option |
Instead of 3 extra cmd line arguments you could have only 1. For example example 1. I want to see a range of known indexes
example 2. I want to see the last 10 pays (wit pythonic indexing):
I think it is very useful to be able to see the last N pays instead of the first N pays. |
I wanted it to be the same as |
Cool. I didn't know about those. |
3f5e4f6
to
d5e56d6
Compare
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.
This is great! Minor changes only...
@@ -491,6 +514,8 @@ static struct command_result *listsendpays_done(struct command *cmd, | |||
pm->sortkey.payment_hash = pm->payment_hash; | |||
pm->sortkey.groupid = groupid; | |||
pm->success_at = UINT64_MAX; | |||
pm->created_index = created_index; | |||
pm->updated_index = updated_index; |
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.
OK, here's the problem the other side of this "if (!pm)" statement. If we have seen part of this payment before, what do we use the created and update indexes?
For updated_index, I think we have to use the greatest we've seen (if any part of a payment gets updated, it's been updated), and for created_index I think it's the smallest we've seen (that's stable: we might create a new part of an existing payment).
This makes sense if we were to implement "wait" for pays, too (that requires more infrastructure to allow that to work in plugins).
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.
I'm sorry i don't have a clue what's going on here, i just put it there because i saw the other values there :/
For updated_index, I think we have to use the greatest we've seen (if any part of a payment gets updated, it's been updated), and for created_index I think it's the smallest we've seen (that's stable: we might create a new part of an existing payment).
Yes, but i don't know how to do it, sorry.
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.
No problems, I can fix this up for you to review!
d5e56d6
to
eb84e43
Compare
Re-requesting review! |
I've tested this with my plugin summars and on the cli. It works as intended, thanks rusty! |
Changelog-Added: JSON-RPC: `listpays` has `index`, `start` and `limit` parameters for listing control.
1. It's called listpays not listpay. 2. "index" does NOT have a default value (it must be specified if limit or start are used) 3. Note that limit and start have effects on accuracy, since we combine records. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
048c718
to
fef1649
Compare
Trivial rebase for conflict in genfile. |
fcdbbd8
into
ElementsProject:master
Fixes #6338 and #6348?
First time
writingcopy-pasting c code btw