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

LCD Pagnation Limit #173

Closed
Tracked by #1015
czarcas7ic opened this issue Mar 10, 2022 · 10 comments
Closed
Tracked by #1015

LCD Pagnation Limit #173

czarcas7ic opened this issue Mar 10, 2022 · 10 comments
Assignees
Labels
Stale T:Task ⚙️ A task belongs to an epic

Comments

@czarcas7ic
Copy link
Member

czarcas7ic commented Mar 10, 2022

As pointed out by Hathor on Discord:

https://lcd-osmosis.blockapsis.com/cosmos/tx/v1beta1/txs?events=message.action=%27superfluid_delegate%27 <-- Defaults to 100
https://lcd-osmosis.blockapsis.com/cosmos/tx/v1beta1/txs?events=message.action=%27superfluid_delegate%27&pagination.limit=50 <-- Sets it to 50 tx in a page
https://lcd-osmosis.blockapsis.com/cosmos/tx/v1beta1/txs?events=message.action=%27superfluid_delegate%27&pagination.limit=500 <-- Still at 100

This behavior is not just on publicly available endpoints, it is default behavior. It should also be known that when checking this via CLI, you are able to sift through x / y pages to see all txs with the requested event, so this behavior is only happening when querying with LCD

The only way around this using LCD is to first do one request to find the pagination.total, then loop through with &pagination.offset=100 etc until the pagination.offset is greater than or equal to the pagination total. RPC allows for any limit to be set, but this requires knowing the abci_query path and encodes which is also not a great method.

Edit:

Also, attempting to increase the limit on the CLI does not work (seems like the limit flag doesn't work as intended)
osmosisd query txs --events="message.action=superfluid_delegate" --limit=30000

@czarcas7ic czarcas7ic added the T:Task ⚙️ A task belongs to an epic label Mar 10, 2022
@czarcas7ic
Copy link
Member Author

All I have noticed so far is txs doesn't use the flags.AddPaginationFlagsToCmd(cmd, "NAME") like the other queries use. Also authtx.QueryTxsByEvents requires page and limit as arguments.

@ValarDragon
Copy link
Member

FlagPage and FlagLimit are a subset of AddPaginationFlagsToCmd, so we should be safe to just switch to the general pagination commands here. https://github.com/cosmos/cosmos-sdk/blob/7c0c347840ef863043f40e697ef6c0416ae9c1d6/client/flags/flags.go#L130-L137

@ValarDragon
Copy link
Member

we may need to update authtx.QueryTxsByEvents tho

@czarcas7ic
Copy link
Member Author

czarcas7ic commented Mar 10, 2022

I feel like the actual flags themselves aren't the problem though since it is understanding 1-100, so it might have something to do with how PrintProto is working behind the scenes. It was also complaining about the other flags being unit64s instead of ints (why are they using ints there for page and limits anyway?)

@alexanderbez
Copy link
Collaborator

Is this just for the txs query by events endpoint? If so, let's move this issue to the SDK (fork), is that OK?

@czarcas7ic czarcas7ic transferred this issue from osmosis-labs/osmosis Apr 4, 2022
@alexanderbez
Copy link
Collaborator

So I took a look and the gRPC proto request type does indeed have the pagination fields included. This means that the issue must be in the handler or the proxy call to Tendermint. My gut feeling tells me it's more of the latter.

I'm digging into this.

Also, reported in the SDK as well: cosmos#11538

@alexanderbez
Copy link
Collaborator

So the 100 per-page cap is actually internally limited to Tendermint, so there is nothing we can do about that directly apart from using our fork (which isn't worth it just for this request alone).

However, the offset/pagination is actually broken, which I can fix.

@ValarDragon
Copy link
Member

IIRC, ytou mentioned somewhere that theres a config in tendermint to bump this limit. Can we change our default node configs to increase that default limit?

@alexanderbez
Copy link
Collaborator

IIRC, ytou mentioned somewhere that theres a config in tendermint to bump this limit. Can we change our default node configs to increase that default limit?

Yes! By having Tendermint run in RPC "unsafe" mode, the page limit is bypassed all together, using whatever the request supplied. So if we want to at least fix this against the nodes we run, then we just need to simply set unsafe=true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale T:Task ⚙️ A task belongs to an epic
Projects
Archived in project
Development

No branches or pull requests

3 participants