Skip to content

Commit

Permalink
Revert "Seperate out before sned hooks: TrackBeforeSend, BlockBeforeS…
Browse files Browse the repository at this point in the history 
…end"

This reverts commit 2ebe377.
  • Loading branch information
@mattverse
mattverse committed Sep 27, 2022
1 parent 2ebe377 commit 29b1ddb
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 118 deletions.
14 changes: 3 additions & 11 deletions x/bank/keeper/hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,10 @@ import (
// Implements StakingHooks interface
var _ types.BankHooks = BaseSendKeeper{}

// TrackbeforeSend executes the TrackBeforeSend hook if registered.
func (k BaseSendKeeper) TrackBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
// BeforeSend executes the BeforeSend hook if registered.
func (k BaseSendKeeper) BeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
if k.hooks != nil {
return k.hooks.TrackBeforeSend(ctx, from, to, amount)
}
return nil
}

// BlockBeforeSend executes the BlockBeforeSend hook if registered.
func (k BaseSendKeeper) BlockBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
if k.hooks != nil {
return k.hooks.BlockBeforeSend(ctx, from, to, amount)
return k.hooks.BeforeSend(ctx, from, to, amount)
}
return nil
}
54 changes: 11 additions & 43 deletions x/bank/keeper/hooks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ var _ types.BankHooks = &MockBankHooksReceiver{}
// BankHooks event hooks for bank (noalias)
type MockBankHooksReceiver struct{}

// Mock BlockBeforeSend bank hook that doesn't allow the sending of exactly 100 coins of any denom.
func (h *MockBankHooksReceiver) BlockBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
// Mock BeforeSend bank hook that doesn't allow the sending of exactly 100 coins of any denom.
func (h *MockBankHooksReceiver) BeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
for _, coin := range amount {
if coin.Amount.Equal(sdk.NewInt(100)) {
return fmt.Errorf("not allowed; expected %v, got: %v", 100, coin.Amount)
Expand All @@ -31,17 +31,6 @@ func (h *MockBankHooksReceiver) BlockBeforeSend(ctx sdk.Context, from, to sdk.Ac
return nil
}

// Mock TrackBeforeSend bank hook that doesn't allow the sending of exactly 50 coins of any denom.
func (h *MockBankHooksReceiver) TrackBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
for _, coin := range amount {
if coin.Amount.Equal(sdk.NewInt(50)) {
return fmt.Errorf("not allowed; expected %v, got: %v", 100, coin.Amount)
}
}

return nil
}

func TestHooks(t *testing.T) {
app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})
Expand All @@ -51,8 +40,7 @@ func TestHooks(t *testing.T) {

// create a valid send amount which is 1 coin, and an invalidSendAmount which is 100 coins
validSendAmount := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(1)))
invalidTrackSendAmount := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(50)))
invalidBlockSendAmount := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100)))
invalidSendAmount := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100)))

// setup our mock bank hooks receiver that prevents the send of 100 coins
bankHooksReceiver := MockBankHooksReceiver{}
Expand All @@ -68,66 +56,46 @@ func TestHooks(t *testing.T) {
require.NoError(t, err)

// try sending an invalidSendAmount and it should not work
err = app.BankKeeper.SendCoins(ctx, addrs[0], addrs[1], invalidTrackSendAmount)
require.Error(t, err)

// try sending an invalidSendAmount and it should not work
err = app.BankKeeper.SendCoins(ctx, addrs[0], addrs[1], invalidBlockSendAmount)
err = app.BankKeeper.SendCoins(ctx, addrs[0], addrs[1], invalidSendAmount)
require.Error(t, err)

// try doing SendManyCoins and make sure if even a single subsend is invalid, the entire function fails
err = app.BankKeeper.SendManyCoins(ctx, addrs[0], []sdk.AccAddress{addrs[0], addrs[1]}, []sdk.Coins{invalidBlockSendAmount, validSendAmount})
require.Error(t, err)

err = app.BankKeeper.SendManyCoins(ctx, addrs[0], []sdk.AccAddress{addrs[0], addrs[1]}, []sdk.Coins{invalidTrackSendAmount, validSendAmount})
err = app.BankKeeper.SendManyCoins(ctx, addrs[0], []sdk.AccAddress{addrs[0], addrs[1]}, []sdk.Coins{invalidSendAmount, validSendAmount})
require.Error(t, err)

// make sure that account to module doesn't bypass hook
err = app.BankKeeper.SendCoinsFromAccountToModule(ctx, addrs[0], stakingtypes.BondedPoolName, validSendAmount)
require.NoError(t, err)
err = app.BankKeeper.SendCoinsFromAccountToModule(ctx, addrs[0], stakingtypes.BondedPoolName, invalidBlockSendAmount)
require.Error(t, err)
err = app.BankKeeper.SendCoinsFromAccountToModule(ctx, addrs[0], stakingtypes.BondedPoolName, invalidTrackSendAmount)
err = app.BankKeeper.SendCoinsFromAccountToModule(ctx, addrs[0], stakingtypes.BondedPoolName, invalidSendAmount)
require.Error(t, err)

// make sure that module to account doesn't bypass hook
err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, stakingtypes.BondedPoolName, addrs[0], validSendAmount)
require.NoError(t, err)
err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, stakingtypes.BondedPoolName, addrs[0], invalidBlockSendAmount)
require.Error(t, err)
err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, stakingtypes.BondedPoolName, addrs[0], invalidTrackSendAmount)
err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, stakingtypes.BondedPoolName, addrs[0], invalidSendAmount)
require.Error(t, err)

// make sure that module to module doesn't bypass hook
err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, stakingtypes.BondedPoolName, stakingtypes.NotBondedPoolName, validSendAmount)
require.NoError(t, err)
err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, stakingtypes.BondedPoolName, stakingtypes.NotBondedPoolName, invalidBlockSendAmount)
// there should be no error since module to module does not call block before send hooks
require.NoError(t, err)
err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, stakingtypes.BondedPoolName, stakingtypes.NotBondedPoolName, invalidTrackSendAmount)
err = app.BankKeeper.SendCoinsFromModuleToModule(ctx, stakingtypes.BondedPoolName, stakingtypes.NotBondedPoolName, invalidSendAmount)
require.Error(t, err)

// make sure that module to many accounts doesn't bypass hook
err = app.BankKeeper.SendCoinsFromModuleToManyAccounts(ctx, stakingtypes.BondedPoolName, []sdk.AccAddress{addrs[0], addrs[1]}, []sdk.Coins{validSendAmount, validSendAmount})
require.NoError(t, err)
err = app.BankKeeper.SendCoinsFromModuleToManyAccounts(ctx, stakingtypes.BondedPoolName, []sdk.AccAddress{addrs[0], addrs[1]}, []sdk.Coins{validSendAmount, invalidBlockSendAmount})
require.Error(t, err)
err = app.BankKeeper.SendCoinsFromModuleToManyAccounts(ctx, stakingtypes.BondedPoolName, []sdk.AccAddress{addrs[0], addrs[1]}, []sdk.Coins{validSendAmount, invalidTrackSendAmount})
err = app.BankKeeper.SendCoinsFromModuleToManyAccounts(ctx, stakingtypes.BondedPoolName, []sdk.AccAddress{addrs[0], addrs[1]}, []sdk.Coins{validSendAmount, invalidSendAmount})
require.Error(t, err)

// make sure that DelegateCoins doesn't bypass the hook
err = app.BankKeeper.DelegateCoins(ctx, addrs[0], app.AccountKeeper.GetModuleAddress(stakingtypes.BondedPoolName), validSendAmount)
require.NoError(t, err)
err = app.BankKeeper.DelegateCoins(ctx, addrs[0], app.AccountKeeper.GetModuleAddress(stakingtypes.BondedPoolName), invalidBlockSendAmount)
require.Error(t, err)
err = app.BankKeeper.DelegateCoins(ctx, addrs[0], app.AccountKeeper.GetModuleAddress(stakingtypes.BondedPoolName), invalidTrackSendAmount)
err = app.BankKeeper.DelegateCoins(ctx, addrs[0], app.AccountKeeper.GetModuleAddress(stakingtypes.BondedPoolName), invalidSendAmount)
require.Error(t, err)

// make sure that UndelegateCoins doesn't bypass the hook
err = app.BankKeeper.UndelegateCoins(ctx, app.AccountKeeper.GetModuleAddress(stakingtypes.BondedPoolName), addrs[0], validSendAmount)
require.NoError(t, err)
err = app.BankKeeper.UndelegateCoins(ctx, app.AccountKeeper.GetModuleAddress(stakingtypes.BondedPoolName), addrs[0], invalidBlockSendAmount)
require.Error(t, err)
err = app.BankKeeper.UndelegateCoins(ctx, app.AccountKeeper.GetModuleAddress(stakingtypes.BondedPoolName), addrs[0], invalidTrackSendAmount)
err = app.BankKeeper.UndelegateCoins(ctx, app.AccountKeeper.GetModuleAddress(stakingtypes.BondedPoolName), addrs[0], invalidSendAmount)
require.Error(t, err)
}
19 changes: 5 additions & 14 deletions x/bank/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,8 @@ func (k BaseKeeper) DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
}

// call the TrackBeforeSend hooks and the BlockBeforeSend hooks
err := k.TrackBeforeSend(ctx, delegatorAddr, moduleAccAddr, amt)
if err != nil {
return err
}
err = k.BlockBeforeSend(ctx, delegatorAddr, moduleAccAddr, amt)
// call the BeforeSend hooks
err := k.BeforeSend(ctx, delegatorAddr, moduleAccAddr, amt)
if err != nil {
return err
}
Expand Down Expand Up @@ -241,12 +237,8 @@ func (k BaseKeeper) UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAdd
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
}

// call the TrackBeforeSend hooks and the BlockBeforeSend hooks
err := k.TrackBeforeSend(ctx, moduleAccAddr, delegatorAddr, amt)
if err != nil {
return err
}
err = k.BlockBeforeSend(ctx, moduleAccAddr, delegatorAddr, amt)
// call the BeforeSend hooks
err := k.BeforeSend(ctx, moduleAccAddr, delegatorAddr, amt)
if err != nil {
return err
}
Expand Down Expand Up @@ -463,8 +455,7 @@ func (k BaseKeeper) SendCoinsFromModuleToModule(
panic(sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", recipientModule))
}

// we do not want to block sends from module to module in any case via hooks.
return k.SendCoinsWithoutBlockHook(ctx, senderAddr, recipientAcc.GetAddress(), amt)
return k.SendCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt)
}

// SendCoinsFromAccountToModule transfers coins from an AccAddress to a ModuleAccount.
Expand Down
37 changes: 3 additions & 34 deletions x/bank/keeper/send.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,37 +81,11 @@ func (k BaseSendKeeper) SetParams(ctx sdk.Context, params types.Params) {
k.paramSpace.SetParamSet(ctx, &params)
}

// SendCoinsWithoutBlockHook calls sendCoins without calling the `BlockBeforeSend` hook.
func (k BaseSendKeeper) SendCoinsWithoutBlockHook(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error {
// call the TrackBeforeSend hooks
err := k.TrackBeforeSend(ctx, fromAddr, toAddr, amt)
if err != nil {
return err
}

return k.sendCoins(ctx, fromAddr, toAddr, amt)
}

// SendCoins transfers amt coins from a sending account to a receiving account.
// An error is returned upon failure.
func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error {
err := k.TrackBeforeSend(ctx, fromAddr, toAddr, amt)
if err != nil {
return err
}

err = k.BlockBeforeSend(ctx, fromAddr, toAddr, amt)
if err != nil {
return err
}

return k.sendCoins(ctx, fromAddr, toAddr, amt)
}

// sendCoins has the internal logic for sending coins.
func (k BaseSendKeeper) sendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error {
// call the TrackBeforeSend hooks
err := k.TrackBeforeSend(ctx, fromAddr, toAddr, amt)
// call the BeforeSend hooks
err := k.BeforeSend(ctx, fromAddr, toAddr, amt)
if err != nil {
return err
}
Expand Down Expand Up @@ -162,15 +136,10 @@ func (k BaseSendKeeper) SendManyCoins(ctx sdk.Context, fromAddr sdk.AccAddress,
totalAmt := sdk.Coins{}
for i, amt := range amts {
// make sure to trigger the BeforeSend hooks for all the sends that are about to occur
err := k.TrackBeforeSend(ctx, fromAddr, toAddrs[i], amts[i])
err := k.BeforeSend(ctx, fromAddr, toAddrs[i], amts[i])
if err != nil {
return err
}
err = k.BlockBeforeSend(ctx, fromAddr, toAddrs[i], amts[i])
if err != nil {
return err
}

totalAmt = sdk.Coins.Add(totalAmt, amt...)
}

Expand Down
3 changes: 1 addition & 2 deletions x/bank/types/expected_keepers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,5 @@ type AccountKeeper interface {

// BankHooks event hooks for bank sends
type BankHooks interface {
TrackBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error // Must be before any send is executed
BlockBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error // Must be before any send is executed
BeforeSend(ctx sdk.Context, from sdk.AccAddress, to sdk.AccAddress, amount sdk.Coins) error // Must be before any send is executed
}
17 changes: 3 additions & 14 deletions x/bank/types/hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,10 @@ func NewMultiBankHooks(hooks ...BankHooks) MultiBankHooks {
return hooks
}

// TrackBeforeSend runs the TrackBeforeSend hooks in order for each BankHook in a MultiBankHooks struct
func (h MultiBankHooks) TrackBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
// BeforeSend runs the BeforeSend hooks in order for each BankHook in a MultiBankHooks struct
func (h MultiBankHooks) BeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
for i := range h {
err := h[i].TrackBeforeSend(ctx, from, to, amount)
if err != nil {
return err
}
}
return nil
}

// BlockBeforeSend runs the BlockBeforeSend hooks in order for each BankHook in a MultiBankHooks struct
func (h MultiBankHooks) BlockBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) error {
for i := range h {
err := h[i].BlockBeforeSend(ctx, from, to, amount)
err := h[i].BeforeSend(ctx, from, to, amount)
if err != nil {
return err
}
Expand Down

0 comments on commit 29b1ddb

Please sign in to comment.