diff --git a/x/bank/CHANGELOG.md b/x/bank/CHANGELOG.md index 9d5a91718557..22f1637fbfb7 100644 --- a/x/bank/CHANGELOG.md +++ b/x/bank/CHANGELOG.md @@ -33,6 +33,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements * [#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`. ### API Breaking Changes diff --git a/x/bank/README.md b/x/bank/README.md index 3bee1e5022a4..5a660c019d2f 100644 --- a/x/bank/README.md +++ b/x/bank/README.md @@ -275,7 +275,7 @@ Both functions compose the provided restriction with any previously provided res `PrependSendRestriction` adds the restriction to be run before any previously provided send restrictions. The composition will short-circuit when an error is encountered. I.e. if the first one returns an error, the second is not run. -During `SendCoins`, the send restriction is applied after coins are removed from the from address, but before adding them to the to address. +During `SendCoins`, the send restriction is applied before coins are removed from the from address and adding them to the to address. During `InputOutputCoins`, the send restriction is applied after the input coins are removed and once for each output before the funds are added. A send restriction function should make use of a custom value in the context to allow bypassing that specific restriction. diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 47b75f02915e..5820d9542284 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -1180,7 +1180,7 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { }, expErr: "test restriction error", expBals: expBals{ - from: sdk.NewCoins(newFooCoin(885), newBarCoin(273)), + from: sdk.NewCoins(newFooCoin(985), newBarCoin(473)), to1: sdk.NewCoins(newFooCoin(15)), to2: sdk.NewCoins(newBarCoin(27)), }, @@ -1194,12 +1194,9 @@ func (suite *KeeperTestSuite) TestSendCoinsWithRestrictions() { actualRestrictionArgs = nil suite.bankKeeper.SetSendRestriction(tc.fn) ctx := suite.ctx - if len(tc.expErr) > 0 { - suite.authKeeper.EXPECT().GetAccount(ctx, fromAddr).Return(fromAcc) - } else { + if len(tc.expErr) == 0 { suite.mockSendCoins(ctx, fromAcc, tc.finalAddr) } - var err error testFunc := func() { err = suite.bankKeeper.SendCoins(ctx, fromAddr, tc.toAddr, tc.amt) diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 47321da51b60..31354c1b8566 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -189,12 +189,12 @@ func (k BaseSendKeeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccA } var err error - err = k.subUnlockedCoins(ctx, fromAddr, amt) + toAddr, err = k.sendRestriction.apply(ctx, fromAddr, toAddr, amt) if err != nil { return err } - toAddr, err = k.sendRestriction.apply(ctx, fromAddr, toAddr, amt) + err = k.subUnlockedCoins(ctx, fromAddr, amt) if err != nil { return err }