fix(x/bank): Better handling of negative spendable balances (backport #21407) #21639
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a few things related to spendable balances.
SpendableCoins
keeper method to return all positive balances instead of returning zero when one denom has more locked than available. Before this PR, if someone had10denoma
and15denomb
, but had11denoma
locked, this method would return an emptyCoins
even though there's actually15denomb
that they could spend. After this PR, it'd return that15denomb
.SpendableCoin
keeper method to report zero spendable if there's more locked than available for that denom. Before this PR, if there were more locked than available, this method would return a negative coin. In the SDK, the only place this method is used is in theSpendableBalanceByDenom
query, so the SDK didn't have any further bugs here, but there could easily be such bugs in other chains.SpendableBalances
query. Before this PR, it was using the pagination stuff to identify which denoms to include in the results, then calling theSpendableCoins
keeper method to get the amounts. But that keeper method callsGetAllBalances
and gets the balances of all denoms anyway, defeating the purpose of the pagination. With this PR, it only gets the page of balances and reduces each by their locked amounts.SpendableBalances
query so that it returns the correct amounts when an account has one or more denoms with more locked than available. Before this PR, if an account had an amount of two denoms, but has more locked than available for one of them, this query would return two zero-coin entries, one for each denom. After this PR, it'll return a zero-coin entry just for the one denom, and the actual spendable amount for the other.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, you can find examples of the prefixes below:
confirmed
!
in the type prefix if API or client breaking changetargeted the correct branch (see PR Targeting)
provided a link to the relevant issue or specification
reviewed "Files changed" and left comments if necessary
included the necessary unit and integration tests
added a changelog entry to
CHANGELOG.md
updated the relevant documentation or specification, including comments for documenting Go code
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.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
SpendableCoins
method to return only positive spendable balances.Tests
This is an automatic backport of pull request #21407 done by [Mergify](https://mergify.com).