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

Migrate remaining CLI cmd's to proto #6987

Closed
8 tasks done
anilcse opened this issue Aug 7, 2020 · 5 comments
Closed
8 tasks done

Migrate remaining CLI cmd's to proto #6987

anilcse opened this issue Aug 7, 2020 · 5 comments

Comments

@anilcse
Copy link
Collaborator

anilcse commented Aug 7, 2020

A few cli commands are still using amino or couldn't be easily migrated to use the proto pathway. These were discovered and flagged in #6859. Migrate them to use proto.

@anilcse anilcse added this to the v0.40 [Stargate] milestone Aug 7, 2020
@aaronc aaronc changed the title Migrate x/gov, x/upgrade cli queries to use proto Migrate remaining CLI cmd's to proto Aug 7, 2020
@fedekunze
Copy link
Collaborator

@anilcse @aaronc can you paste an example blob of how the output for the IBC queries looks like? We can tackle it this week

@aaronc
Copy link
Member

aaronc commented Aug 10, 2020

I don't have IBC test data to do that @fedekunze. Once #6999 is done, you just need to try to use PrintOutput instead of PrintOutputLegacy and you'll see the errors.

@sahith-narahari
Copy link
Contributor

@alexanderbez QueryTxsByEvents uses TxSearch from tendermint which uses slightly different pagination. This causes some inconsistency in queries using this.

resTxs, err := node.TxSearch(query, true, page, limit, orderBy)

One option I can think of is fetch all data, and paginate in sdk to act similar to other queries. Any thoughs?
cc @aaronc @anilcse

@alexanderbez
Copy link
Contributor

I would honestly opt to use Tendermint's pagination when it comes to tx searching. I don't think getting all txs behind the scenes is a good strategy.

@clevinson
Copy link
Contributor

@anilcse can you provide a written update for this issue here?

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

No branches or pull requests

6 participants