cosmos-sdk feat: (x/bank) add spendable balances cmd

Description

This PR adds a spendable-balances to x/bank's queries.

Questions:

Should we move this command under balances and activate it with a bool flag (--only-spendable)? This would invalidate the --denom flag. If we are ok with that, maybe it's cleaner that way.

Closes: #14030

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.

I have...

[ ] included the correct type prefix in the PR title
[ ] added ! to the type prefix if API or client breaking change
[ ] targeted the correct branch (see PR Targeting)
[ ] provided a link to the relevant issue or specification
[ ] followed the guidelines for building modules
[ ] included the necessary unit and integration tests
[ ] added a changelog entry to CHANGELOG.md
[ ] included comments for documenting Go code
[ ] updated the relevant documentation or specification
[ ] reviewed "Files changed" and left comments if necessary
[ ] confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.

I have...

[ ] confirmed the correct type prefix in the PR title
[ ] confirmed ! in the type prefix if API or client breaking change
[ ] confirmed all author checklist items have been addressed
[ ] reviewed state machine logic
[ ] reviewed API design and naming
[ ] reviewed documentation is accurate
[ ] reviewed tests and test coverage
[ ] manually tested (if applicable)

Nov 28 '22 14:11 facundomedica

Should we move this command under balances and activate it with a bool flag (--only-spendable)? This would invalidate the --denom flag. If we are ok with that, maybe it's cleaner that way.

I think spendableCoins uses gas more efficiently when compared to get the balance of an account multiple times after txs. Maybe I'd prefer having it in a separate command, not to invalidate existing functionality of --denom flag usage. cc @alexanderbez

Nov 29 '22 14:11 likhita-809

Should we move this command under balances and activate it with a bool flag (--only-spendable)? This would invalidate the --denom flag. If we are ok with that, maybe it's cleaner that way.

I think spendableCoins uses gas more efficiently when compared to get the balance of an account multiple times after txs. Maybe I'd prefer having it in a separate command, not to invalidate existing functionality of --denom flag usage. cc @alexanderbez

I like @facundomedica's proposal! Why exactly do you think having a --spendable-only flag is a bad idea?

Nov 29 '22 15:11 alexanderbez

Should we move this command under balances and activate it with a bool flag (--only-spendable)? This would invalidate the --denom flag. If we are ok with that, maybe it's cleaner that way.

I think spendableCoins uses gas more efficiently when compared to get the balance of an account multiple times after txs. Maybe I'd prefer having it in a separate command, not to invalidate existing functionality of --denom flag usage. cc @alexanderbez

I like @facundomedica's proposal! Why exactly do you think having a --spendable-only flag is a bad idea?

I'm not opposed to it and don't really think its a bad idea, but just concerned if we could remove --denom's functionality and its not needed in future, but definitely agree that it makes balances cleaner.

Nov 29 '22 15:11 likhita-809

So in other words --denom will be ignored if --spendable-only is passed. Another option would be adding a new query that only gets the spendable amount of an specific denom to mimic what GetBalances is doing. Then both --denom and --spendable-only could be used together.

Nov 29 '22 18:11 facundomedica

So in other words --denom will be ignored if --spendable-only is passed. Another option would be adding a new query that only gets the spendable amount of an specific denom to mimic what GetBalances is doing. Then both --denom and --spendable-only could be used together.

I like this better

Nov 30 '22 05:11 likhita-809

@facundomedica well if it's a new query for just spendable balances, then --spendable-only isn't needed anymore and we're back to where we started, no?

Nov 30 '22 15:11 alexanderbez

Coming back to this after a while. I think we have to options:

Modify the current GetBalancesCmd to take in --spendable-only. For this we would need to add a new query like SpendableBalance (And maybe modify SpendableBalances to AllSpendableBalances, but that would be breaking). To mimic Balance and AllBalances, and making possible to get a single denom by passing --denom.
Leave this PR as is, returning a list of spendable balances in a separate command.

If we really want to be able to get a single denom at a time, we should go for 1, if not, I would just stick with 2.

Dec 14 '22 22:12 facundomedica

So now the spendable-balances accepts the --denom flag, so it works just like the balances cmd.

Dec 20 '22 14:12 facundomedica

https://github.com/cosmos/cosmos-sdk/pull/14045/commits/3b17966ed025ad12e21f68ff6f8600bd85945de4: Actually we shouldn't remove them, we want them, it's just that if we don't backport it should be 0.48 :)