fix(x/bank): Better handling of negative spendable balances #24060
Conversation
|
@hoangdv2429 needs a changelog |
|
@hoangdv2429 looks like lint failing |
| @@ -194,7 +194,23 @@ func (k BaseViewKeeper) LockedCoins(ctx context.Context, addr sdk.AccAddress) sd | |||
| // by address. If the account has no spendable coins, an empty Coins slice is | |||
| // returned. | |||
| func (k BaseViewKeeper) SpendableCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins { | |||
| spendable, _ := k.spendableCoins(ctx, addr) | |||
| total := k.GetAllBalances(ctx, addr) | |||
There was a problem hiding this comment.
Can we add a comment to the function that describes exactly how this works?
There was a problem hiding this comment.
i.e.
What we do when we have:
- no locked coins
- more locked than in balances
- some locked, some unlocked
There was a problem hiding this comment.
I updated with specific flow
| @@ -1473,12 +1473,27 @@ func (suite *KeeperTestSuite) TestMsgMultiSendEvents() { | |||
| require.Equal(abci.Event(event3), events[29]) | |||
| } | |||
|
|
|||
| // TestSpendableCoins verifies the behavior of SpendableCoins under various scenarios: | |||
There was a problem hiding this comment.
@aljo242 I also added document on what testcases we have. I hope it cover every paths you need.
Now code will consider locked coins when returning balances (minus it). Smart contracts that interact with the bank module to check account balances or spending limits will now receive more accurate data, reflecting the actual spendable amounts. This change can influence the logic within smart contracts that perform balance checks, potentially preventing overspending or ensuring compliance with spending rules. For module, I don't think it alter any behavior. |
makes sense. i think we are probably safe to merge here. wdyt @aljo242 ? |
|
Yeah this makes sense to me - this is "query breaking" - we should make note that it is in the changelog |
Description
Closes: #23828
Backport
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...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers 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...