LaconicNetwork/cosmos-sdk
10
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

refactor(x/bank): swap sendrestriction check in InputOutputCoins (backport #21976) (#22086)

Browse Source 
Co-authored-by: Julien Robert <julien@rbrt.fr>
This commit is contained in:
mergify[bot] 2024-10-03 11:45:06 +00:00 committed by GitHub
parent 41e88d82ce
commit a72390a049
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 34 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
x/bank/CHANGELOG.md
View File

@ -35,13 +35,14 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `SendCoinsFromModuleToAccount`, `SendCoinsFromModuleToModule`, `SendCoinsFromAccountToModule`, `DelegateCoinsFromAccountToModule`, `UndelegateCoinsFromModuleToAccount`, `MintCoins` and `BurnCoins` methods now returns an error instead of panicking if any module accounts does not exist or unauthorized.
* [#20517](https://github.com/cosmos/cosmos-sdk/pull/20517) `SendCoins` now checks for `SendRestrictions` before instead of after deducting coins using `subUnlockedCoins`.
* [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.
* [#21976](https://github.com/cosmos/cosmos-sdk/pull/21976) Resolve a footgun by swapping send restrictions check in `InputOutputCoins` before coin deduction.
### Bug Fixes
* [#21407](https://github.com/cosmos/cosmos-sdk/pull/21407) Fix handling of negative spendable balances.
* The `SpendableBalances` query now correctly reports spendable balances when one or more denoms are negative (used to report all zeros). Also, this query now looks up only the balances for the requested page.
* The `SpendableCoins` keeper method now returns the positive spendable balances even when one or more denoms have more locked than available (used to return an empty `Coins`).
* The `SpendableCoin` keeper method now returns a zero coin if there's more locked than available (used to return a negative coin).
* The `SpendableBalances` query now correctly reports spendable balances when one or more denoms are negative (used to report all zeros). Also, this query now looks up only the balances for the requested page.
* The `SpendableCoins` keeper method now returns the positive spendable balances even when one or more denoms have more locked than available (used to return an empty `Coins`).
* The `SpendableCoin` keeper method now returns a zero coin if there's more locked than available (used to return a negative coin).
### API Breaking Changes

14
x/bank/keeper/keeper_test.go
View File

@ -790,6 +790,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() {
suite.Require().NoError(err)
fromAcc := authtypes.NewBaseAccountWithAddress(fromAddr)
inputAccs := []sdk.AccountI{fromAcc}
suite.authKeeper.EXPECT().GetAccount(suite.ctx, inputAccs[0].GetAddress()).Return(inputAccs[0]).AnyTimes()
toAddr1 := accAddrs[1]
toAddr1Str, err := suite.authKeeper.AddressCodec().BytesToString(toAddr1)
suite.Require().NoError(err)
@ -878,7 +879,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() {
},
expErr: "restriction test error",
expBals: expBals{
from: sdk.NewCoins(newFooCoin(959), newBarCoin(412)),
from: sdk.NewCoins(newFooCoin(959), newBarCoin(500)),
to1: sdk.NewCoins(newFooCoin(15)),
to2: sdk.NewCoins(newFooCoin(26)),
},
@ -907,7 +908,7 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() {
},
},
expBals: expBals{
from: sdk.NewCoins(newFooCoin(948), newBarCoin(400)),
from: sdk.NewCoins(newFooCoin(948), newBarCoin(488)),
to1: sdk.NewCoins(newFooCoin(26)),
to2: sdk.NewCoins(newFooCoin(26), newBarCoin(12)),
},
@ -937,8 +938,8 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() {
},
expErr: "second restriction error",
expBals: expBals{
from: sdk.NewCoins(newFooCoin(904), newBarCoin(400)),
to1: sdk.NewCoins(newFooCoin(38)),
from: sdk.NewCoins(newFooCoin(948), newBarCoin(488)),
to1: sdk.NewCoins(newFooCoin(26)),
to2: sdk.NewCoins(newFooCoin(26), newBarCoin(12)),
},
},
@ -966,8 +967,8 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() {
},
},
expBals: expBals{
from: sdk.NewCoins(newFooCoin(904), newBarCoin(365)),
to1: sdk.NewCoins(newFooCoin(38), newBarCoin(25)),
from: sdk.NewCoins(newFooCoin(948), newBarCoin(453)),
to1: sdk.NewCoins(newFooCoin(26), newBarCoin(25)),
to2: sdk.NewCoins(newFooCoin(26), newBarCoin(22)),
},
},
@ -980,7 +981,6 @@ func (suite *KeeperTestSuite) TestInputOutputCoinsWithRestrictions() {
actualRestrictionArgs = nil
suite.bankKeeper.SetSendRestriction(tc.fn)
ctx := suite.ctx
suite.mockInputOutputCoins(inputAccs, tc.outputAddrs)
input := banktypes.Input{
Address: fromStrAddr,
Coins: tc.inputCoins,

29
x/bank/keeper/send.go
View File

@ -148,14 +148,15 @@ func (k BaseSendKeeper) InputOutputCoins(ctx context.Context, input types.Input,
return err
}
err = k.subUnlockedCoins(ctx, inAddress, input.Coins)
if err != nil {
return err
// ensure all coins can be sent
type toSend struct {
AddressStr string
Address []byte
Coins sdk.Coins
}
var outAddress sdk.AccAddress
sending := make([]toSend, 0)
for _, out := range outputs {
outAddress, err = k.ak.AddressCodec().StringToBytes(out.Address)
outAddress, err := k.ak.AddressCodec().StringToBytes(out.Address)
if err != nil {
return err
}
@ -165,13 +166,25 @@ func (k BaseSendKeeper) InputOutputCoins(ctx context.Context, input types.Input,
return err
}
if err := k.addCoins(ctx, outAddress, out.Coins); err != nil {
sending = append(sending, toSend{
Address: outAddress,
AddressStr: out.Address,
Coins: out.Coins,
})
}
if err := k.subUnlockedCoins(ctx, inAddress, input.Coins); err != nil {
return err
}
for _, out := range sending {
if err := k.addCoins(ctx, out.Address, out.Coins); err != nil {
return err
}
if err := k.EventService.EventManager(ctx).EmitKV(
types.EventTypeTransfer,
event.NewAttribute(types.AttributeKeyRecipient, out.Address),
event.NewAttribute(types.AttributeKeyRecipient, out.AddressStr),
event.NewAttribute(types.AttributeKeySender, input.Address),
event.NewAttribute(sdk.AttributeKeyAmount, out.Coins.String()),
); err != nil {

4
x/group/keeper/abci.go
View File

@ -3,7 +3,7 @@ package keeper
import (
"context"
"cosmossdk.io/x/gov/types"
"cosmossdk.io/x/group"
"github.com/cosmos/cosmos-sdk/telemetry"
)
@ -12,7 +12,7 @@ import (
// prunes expired proposals.
func (k Keeper) EndBlocker(ctx context.Context) error {
start := telemetry.Now()
defer telemetry.ModuleMeasureSince(types.ModuleName, start, telemetry.MetricKeyEndBlocker)
defer telemetry.ModuleMeasureSince(group.ModuleName, start, telemetry.MetricKeyEndBlocker)
if err := k.TallyProposalsAtVPEnd(ctx); err != nil {
return err