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

hermes keys balance doesn't show all denoms #2726

Closed
5 tasks
ancazamfir opened this issue Oct 11, 2022 · 2 comments · Fixed by #2729
Closed
5 tasks

hermes keys balance doesn't show all denoms #2726

ancazamfir opened this issue Oct 11, 2022 · 2 comments · Fixed by #2729
Milestone

Comments

@ancazamfir
Copy link
Collaborator

Summary of Bug

hermes keys balance only shows the balance of the config's gas_price denom.

$ hermes --config config.toml keys list --chain ibc-1
SUCCESS
- relayer (cosmos1zc7qkhjr4delcr785q0a4jsqcchjef4vvpup70)
- wallet (cosmos14ev9kx7tj4kl48jgm698qqpeth54ykz6l2x3k5)
- user1 (cosmos1gef995hlygyyqfqcjkgt0a58rt356ssh4zmy2j)
- user2 (cosmos15jyrwymfshhsjznelhvleklzjas0vjga3wrrk5)
- testkey (cosmos1wn7kusrmrn2ms6e2cex7uud3uhvsm6xt3p96jj)

$ hermes --config config.toml keys balance --chain ibc-1 --key-name testkey
SUCCESS balance for key `testkey`: 99999998688 stake

$ gaiad query bank balances cosmos1wn7kusrmrn2ms6e2cex7uud3uhvsm6xt3p96jj --node http://0.0.0.0:26557
balances:
- amount: "30"
  denom: ibc/27A6394C3F9FF9C9DCF5DFFADF9BB5FE9A37C7E92B006199894CF1824DF9AC7C
- amount: "100000000000"
  denom: samoleans
- amount: "99999998688"
  denom: stake
pagination:
  next_key: null
  total: "0"

I am pretty sure this used to work like the gaiad query bank balance counterpart but haven't traced the PR that change this.

Version

Steps to Reproduce

Acceptance Criteria


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ljoss17
Copy link
Contributor

ljoss17 commented Oct 11, 2022

The current hermes keys balance command was introduced with this PR, #2232. As I recall the idea was to output the balance of the token used for gas, as the interest was knowing if there was enough funds to submit a transaction.

I think it would be a good idea to improve this command, the following solutions come to my mind:

  1. Add an optional flag --denom to specify the denom to query, with a special value to query all denom. For example:
    • hermes keys balance --chain <CHAIN_ID> --key-name <KEY_NAME> --denom samoleans to display only the amount of samoleans
    • hermes keys balance --chain <CHAIN_ID> --key-name <KEY_NAME> --denom all to display the balance for all denom
    • hermes keys balance --chain <CHAIN_ID> --key-name <KEY_NAME> to display the balance of the gas_price denom
  2. Add an optional flag --all which outputs the balance for all denom when used, and only the balance of the gas_price denom if not used. For example:
    • hermes keys balance --chain <CHAIN_ID> --key-name <KEY_NAME> to display only the balance of gas_price denom
    • hermes keys balance --chain <CHAIN_ID> --key-name <KEY_NAME> --all to display the balance of all denom
  3. Update the command to always output the balance of all denoms

I have a preference for the first solution, adding a --denom flag, as it keeps the actual command as it is, but gives the user the possibility to have a more specific output.

What do you think ?

@ancazamfir
Copy link
Collaborator Author

Option 1 looks good.

@adizere adizere added this to the v1.1 milestone Oct 31, 2022
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

Successfully merging a pull request may close this issue.

3 participants