diff --git a/CHANGELOG.md b/CHANGELOG.md index c7ebdc67fd..c41e42d710 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (x/bank) [#24106](https://github.com/cosmos/cosmos-sdk/pull/24106) `SendCoins` now checks for `SendRestrictions` before instead of after deducting coins using `subUnlockedCoins`. * (crypto/ledger) [#24036](https://github.com/cosmos/cosmos-sdk/pull/24036) Improve error message when deriving paths using index > 100 * (gRPC) [#23844](https://github.com/cosmos/cosmos-sdk/pull/23844) Add debug log prints for each gRPC request. * (server) [#24072](https://github.com/cosmos/cosmos-sdk/pull/24072) Return BlockHeader by shallow copy in server Context. diff --git a/x/bank/README.md b/x/bank/README.md index 885a9f1f6f..6f0a701bc3 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 be46327feb..dee6c39d4c 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -1163,7 +1163,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)), }, @@ -1177,12 +1177,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 7628821272..9b2f22fc39 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -208,12 +208,12 @@ func (k BaseSendKeeper) InputOutputCoins(ctx context.Context, input types.Input, // An error is returned upon failure. func (k BaseSendKeeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error { 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 }