add listing options to listpays #7385
Conversation
daywalker90
force-pushed
the
listpays-index
branch
2 times, most recently
from
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. |
daywalker90
force-pushed
the
listpays-index
branch
2 times, most recently
from
3f5e4f6 to
d5e56d6
Compare
rustyrussell
left a comment
rustyrussell
left a comment
There was a problem hiding this comment.
This is great! Minor changes only...
| 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.
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.
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.
No problems, I can fix this up for you to review!
daywalker90
force-pushed
the
listpays-index
branch
from
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 <[email protected]>
rustyrussell
force-pushed
the
listpays-index
branch
from
048c718 to
fef1649
Compare
|
Trivial rebase for conflict in genfile. |
vincenzopalazzo
merged commit fcdbbd8
into
ElementsProject:master
5 participants
Fixes #6338 and #6348?
First time
writingcopy-pasting c code btw