Skip to content
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

Should retrieve_txs() API allow for constrained response size? #659

Open
cliik opened this issue Jul 25, 2022 · 2 comments
Open

Should retrieve_txs() API allow for constrained response size? #659

cliik opened this issue Jul 25, 2022 · 2 comments

Comments

@cliik
Copy link
Contributor

cliik commented Jul 25, 2022

Is your feature request related to a problem? Please describe.
Yes. Retrieving TXs for a wallet with a large transaction history, may result in an unknowably large response. This could cause performance or UX issues for users with lots of transactions. Super users (or exchanges) with thousands or maybe millions of txs one day may have a problem with this API as it exists today.

Describe the solution you'd like
I think the API should allow the requester to constrain the response to a limited number of TXs.

Describe alternatives you've considered
A simple alternative, would be to add count and start_at args to the retrieve_txs() method. In addition to constraining the response to a limited number of results, this will also facilitate pagination. Pagination may be the best solution, when grin super-users or exchanges begin to stress the limits of a wallet with 10Ks/100Ks/1Ms of transactions.

Additional context
Constraints like this will also be valuable/necessary for wallets running in resource limited environments (RaspPi, hardware wallet, etc).

I volunteer to do this work if there is community buy-in around the idea. This would be a breaking change to the API, so it would be nice to coordinate this change with other breaking changes (e.g. #657).

@cliik
Copy link
Contributor Author

cliik commented Jul 25, 2022

FYI, I've opened a CLI-only PR to address the more immediate UX issue I am experiencing (too many TXs to view in my ssh window!): #660. That PR does not address the forward-looking needs of the API though.

@yeastplume
Copy link
Member

Yes, this is going to be required for the wallet gui work, as we'll eventually need some kind of paging mechanism, so would be quite happy to see this. Again, I don't think it needs to be breaking as we're perfectly able to support optional arguments and flags throughout.

'Super Users' or exchanges need to be better supported overall, and one idea is to refactor the backend some to allow for sql databases as opposed to lmdb (which is a much larger project).

If you'd like to discuss what's going on in the gui a bit more I'd be happy to. There's likely to be a few things that fall out of it that turn out to be missing or really really nice to have, this being one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants