rpc: Add options argument to listtransactions, with paginatebypointer options #19443

kristapsk · 2020-07-04T08:01:41Z

This is #14898 rebased against current master, review comments addressed and help text updated with output changes when pageinatebypointer option is used.

~~Added fifth param to listtransactions named options,~~ Previous skip argument is replaced with options, with backwards compatibility, where it's treated as a skip if it's integer, not an object. options may be an object containing paginatebypointer (boolean default: false) and nextpagepointer (string default: OMITTED).~

With paginatebypointer output will have the following changes.

Return transactions is ordered by most recent transactions. Though the default does reverse the order after transactions are fetched and clipped.
skip argument has no effect. Instead nextpagepointer will be used for pagination.
Return value is an object containing, records (array of txs) and nextpagepointer (string)

src/wallet/rpcwallet.cpp

DrahtBot · 2020-07-04T14:51:45Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	instagibbs

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

rpc: Remove duplicate name and argNames from CRPCCommand #20012 (rpc: Remove duplicate name and argNames from CRPCCommand by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

instagibbs · 2020-08-05T15:30:16Z

concept ACK, this is a pretty straight forward improvement I think as far as an API is concerned. Would be helpful to find people needing this API to test.

kristapsk · 2020-08-26T21:32:43Z

Addressed review comment by @luke-jr , agree this way is better.

kristapsk · 2020-09-25T22:22:47Z

Rebased (+ options|skip changes required by #19994).

src/wallet/rpcwallet.cpp

Added fifth param to `listtransactions` named `options`, `options` may be an object containing `paginatebypointer` (boolean default: false) and `nextpagepointer` (string default: OMITTED). With `paginatebypointer` output will have the following changes. 1. Return transactions is ordered by most recent transactions. Though the default does reverse the order after transactions are fetched and clipped. 2. `skip` argument has no effect. Instead `nextpagepointer` will be used for pagination. 3. Return value is an object containing, records (array of txs) and nextpagepointer (string) Co-authored-by: Kristaps Kaupe <kristaps@blogiem.lv>

maflcko · 2021-01-28T18:41:49Z

src/wallet/rpcwallet.cpp

@@ -4492,7 +4652,7 @@ static const CRPCCommand commands[] =
    { "wallet",             "listreceivedbyaddress",            &listreceivedbyaddress,         {"minconf","include_empty","include_watchonly","address_filter"} },
    { "wallet",             "listreceivedbylabel",              &listreceivedbylabel,           {"minconf","include_empty","include_watchonly"} },
    { "wallet",             "listsinceblock",                   &listsinceblock,                {"blockhash","target_confirmations","include_watchonly","include_removed"} },
-    { "wallet",             "listtransactions",                 &listtransactions,              {"label|dummy","count","skip","include_watchonly"} },
+    { "wallet",             "listtransactions",                 &listtransactions,              {"label|dummy","count","options|skip","include_watchonly"} },


can remove this diff, and then rebase

DrahtBot · 2021-01-28T18:59:48Z

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

ghost · 2021-08-25T14:59:59Z

Tried this with new argument added according to the examples. TBH this looks complex compared to PR 22775 or maybe I am not using it correctly. Only thing I noticed different which could be helpful in some cases was nextpagepointer in results.

kristapsk · 2021-08-25T16:34:16Z

@prayank23 Most of the complexity is for paginatebypointer. Probably I could rebase this and split into two commits, then adding options object as a second argument could be applied to #22775 separately.

kristapsk · 2021-08-26T18:51:18Z

I'm putting this on hold and not going to rebase until either #19762 or #22807 is merged, don't think anymore it's good idea to add additional positional arguments to listtransactions for this.

DrahtBot · 2021-12-22T12:29:13Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

glozow · 2022-09-26T10:01:24Z

@kristapsk are you still working on this?

kristapsk · 2022-09-26T13:34:53Z

@glozow Waiting on #19762, will it be merged or rejected.

luke-jr · 2022-09-26T14:07:35Z

#19762 shouldn't be relevant to this. Even if both named and positional are supported, the pure positional API should be clean.

kristapsk · 2022-09-26T14:17:16Z

@luke-jr So you think it's worth resuming work on #22807?

luke-jr · 2022-09-26T14:20:59Z

Yes

kristapsk · 2022-09-27T08:25:45Z

#22807 is rebased, don't think it's worth rebasing this before that one is either merged or rejected.

DrahtBot · 2022-12-26T02:00:53Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

fanquake · 2023-01-12T13:44:44Z

don't think it's worth rebasing this before that one is either merged or rejected.

Made this a draft for now then.

DrahtBot · 2023-04-12T01:03:06Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

DrahtBot · 2023-07-11T01:47:36Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

maflcko · 2023-09-20T16:54:05Z

Are you still working on this? If not, maybe close for now?

DrahtBot · 2023-12-19T01:53:24Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

glozow · 2023-12-20T11:17:08Z

Closing due to lack of activity. Feel free to ping if this is picked up again.

fanquake added the RPC/REST/ZMQ label Jul 4, 2020

luke-jr reviewed Jul 4, 2020

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

This was referenced Jul 4, 2020

doc: Include wallet path to relevant RPC calls #19349

Closed

rpc: remove deprecated CRPCCommand constructor #18531

Merged

Use shared pointers only in validation interface #18354

Closed

DrahtBot mentioned this pull request Jul 13, 2020

Log RPC parameters (arguments) if -debug=rpcparams #16365

Closed

kristapsk force-pushed the listtransactions-paginatebypointer branch from 4d64902 to fd18204 Compare August 26, 2020 21:30

DrahtBot mentioned this pull request Sep 22, 2020

Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) #19994

Merged

DrahtBot added the Needs rebase label Sep 23, 2020

kristapsk force-pushed the listtransactions-paginatebypointer branch from fd18204 to d233809 Compare September 25, 2020 22:20

DrahtBot removed the Needs rebase label Sep 25, 2020

This was referenced Sep 26, 2020

rpc: Remove duplicate name and argNames from CRPCCommand #20012

Merged

upstream: add and use defaulted getters to UniValue #20045

Closed

luke-jr reviewed Dec 13, 2020

View reviewed changes

src/wallet/rpcwallet.cpp Outdated Show resolved Hide resolved

kristapsk force-pushed the listtransactions-paginatebypointer branch from d233809 to 480df47 Compare December 14, 2020 00:42

maflcko reviewed Jan 28, 2021

View reviewed changes

DrahtBot added the Needs rebase label Jan 28, 2021

kristapsk mentioned this pull request Aug 24, 2021

rpc: Add option to list transactions from oldest to newest in listtransactions RPC command #22775

Closed

kristapsk mentioned this pull request Aug 26, 2021

RPC: Add universal options argument to listtransactions #22807

Closed

kristapsk mentioned this pull request Mar 15, 2022

WalletService: reduce polling overhead JoinMarket-Org/joinmarket-clientserver#1180

Merged

glozow closed this Sep 26, 2022

glozow reopened this Sep 26, 2022

glozow added the Waiting for author label Sep 26, 2022

fanquake marked this pull request as draft January 12, 2023 13:45

maflcko removed the Waiting for author label Sep 20, 2023

kristapsk mentioned this pull request Oct 11, 2023

Feature request: history via api JoinMarket-Org/joinmarket-clientserver#1358

Open

glozow closed this Dec 20, 2023

bitcoin locked and limited conversation to collaborators Dec 19, 2024

rpc: Add options argument to listtransactions, with paginatebypointer options #19443

rpc: Add options argument to listtransactions, with paginatebypointer options #19443

Uh oh!

Conversation

kristapsk commented Jul 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

DrahtBot commented Jul 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

instagibbs commented Aug 5, 2020

Uh oh!

kristapsk commented Aug 26, 2020

Uh oh!

kristapsk commented Sep 25, 2020

Uh oh!

Uh oh!

maflcko Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jan 28, 2021

Uh oh!

ghost commented Aug 25, 2021

Uh oh!

kristapsk commented Aug 25, 2021

Uh oh!

kristapsk commented Aug 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Dec 22, 2021

Uh oh!

glozow commented Sep 26, 2022

Uh oh!

kristapsk commented Sep 26, 2022

Uh oh!

luke-jr commented Sep 26, 2022

Uh oh!

kristapsk commented Sep 26, 2022

Uh oh!

luke-jr commented Sep 26, 2022

Uh oh!

kristapsk commented Sep 27, 2022

Uh oh!

DrahtBot commented Dec 26, 2022

Uh oh!

fanquake commented Jan 12, 2023

Uh oh!

DrahtBot commented Apr 12, 2023

Uh oh!

DrahtBot commented Jul 11, 2023

Uh oh!

maflcko commented Sep 20, 2023

Uh oh!

DrahtBot commented Dec 19, 2023

Uh oh!

glozow commented Dec 20, 2023

Uh oh!

Uh oh!

kristapsk commented Jul 4, 2020 •

edited

Loading

DrahtBot commented Jul 4, 2020 •

edited

Loading

kristapsk commented Aug 26, 2021 •

edited

Loading