Skip to content

Commit

Permalink
feat: Add TrackBeforeSend , BlockBeforeSend hooks. Deprecate `Bef…
Browse files Browse the repository at this point in the history 
…oreSend` hook (#421)

* Add separated hooks for bank send

* Update x/bank/keeper/hooks.go

Co-authored-by: Nicolas Lara <nicolaslara@gmail.com>

---------

Co-authored-by: Nicolas Lara <nicolaslara@gmail.com>
  • Loading branch information
@mattverse @nicolaslara
mattverse and nicolaslara committed Mar 13, 2023
1 parent f94c709 commit 4f857c6
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 29 deletions.
13 changes: 10 additions & 3 deletions x/bank/keeper/hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,17 @@ import (
// Implements StakingHooks interface
var _ types.BankHooks = BaseSendKeeper{}

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

// 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 nil
}
72 changes: 59 additions & 13 deletions x/bank/keeper/hooks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,31 @@ var _ types.BankHooks = &MockBankHooksReceiver{}
// BankHooks event hooks for bank (noalias)
type MockBankHooksReceiver struct{}

// 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 {
// 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 {
for _, coin := range amount {
if coin.Amount.Equal(sdk.NewInt(100)) {
return fmt.Errorf("not allowed; expected %v, got: %v", 100, coin.Amount)
}
}

return nil
}

// variable for counting `TrackBeforeSend`
var (
countTrackBeforeSend = 0
expNextCount = 1
)

// Mock TrackBeforeSend bank hook that simply tracks the sending of exactly 50 coins of any denom.
func (h *MockBankHooksReceiver) TrackBeforeSend(ctx sdk.Context, from, to sdk.AccAddress, amount sdk.Coins) {
for _, coin := range amount {
if coin.Amount.Equal(sdk.NewInt(50)) {
countTrackBeforeSend += 1
}
}
}

func TestHooks(t *testing.T) {
app := simapp.Setup(false)
ctx := app.BaseApp.NewContext(false, tmproto.Header{})
Expand All @@ -40,7 +54,8 @@ 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)))
invalidSendAmount := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(100)))
triggerTrackSendAmount := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), sdk.NewInt(50)))
invalidBlockSendAmount := 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 @@ -55,47 +70,78 @@ func TestHooks(t *testing.T) {
err := app.BankKeeper.SendCoins(ctx, addrs[0], addrs[1], validSendAmount)
require.NoError(t, err)

// try sending an trigger track send amount and it should work
err = app.BankKeeper.SendCoins(ctx, addrs[0], addrs[1], triggerTrackSendAmount)
require.NoError(t, err)

require.Equal(t, countTrackBeforeSend, expNextCount)
expNextCount++

// try sending an invalidSendAmount and it should not work
err = app.BankKeeper.SendCoins(ctx, addrs[0], addrs[1], invalidSendAmount)
err = app.BankKeeper.SendCoins(ctx, addrs[0], addrs[1], invalidBlockSendAmount)
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{invalidSendAmount, validSendAmount})
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{triggerTrackSendAmount, validSendAmount})
require.Equal(t, countTrackBeforeSend, expNextCount)
expNextCount++

// 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, invalidSendAmount)
err = app.BankKeeper.SendCoinsFromAccountToModule(ctx, addrs[0], stakingtypes.BondedPoolName, invalidBlockSendAmount)
require.Error(t, err)
err = app.BankKeeper.SendCoinsFromAccountToModule(ctx, addrs[0], stakingtypes.BondedPoolName, triggerTrackSendAmount)
require.Equal(t, countTrackBeforeSend, expNextCount)
expNextCount++

// 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], invalidSendAmount)
err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, stakingtypes.BondedPoolName, addrs[0], invalidBlockSendAmount)
require.Error(t, err)
err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, stakingtypes.BondedPoolName, addrs[0], triggerTrackSendAmount)
require.Equal(t, countTrackBeforeSend, expNextCount)
expNextCount++

// 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, invalidSendAmount)
require.Error(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, triggerTrackSendAmount)
require.Equal(t, countTrackBeforeSend, expNextCount)
expNextCount++

// 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, invalidSendAmount})
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, triggerTrackSendAmount})
require.Equal(t, countTrackBeforeSend, expNextCount)
expNextCount++

// 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), invalidSendAmount)
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), triggerTrackSendAmount)
require.Equal(t, countTrackBeforeSend, expNextCount)
expNextCount++

// 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], invalidSendAmount)
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], triggerTrackSendAmount)
require.Equal(t, countTrackBeforeSend, expNextCount)
expNextCount++
}
15 changes: 10 additions & 5 deletions x/bank/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,12 @@ func (k BaseKeeper) DelegateCoins(ctx sdk.Context, delegatorAddr, moduleAccAddr
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
}

// call the BeforeSend hooks
err := k.BeforeSend(ctx, delegatorAddr, moduleAccAddr, amt)
err := k.BlockBeforeSend(ctx, delegatorAddr, moduleAccAddr, amt)
if err != nil {
return err
}
// call the TrackBeforeSend hooks and the BlockBeforeSend hooks
k.TrackBeforeSend(ctx, delegatorAddr, moduleAccAddr, amt)

balances := sdk.NewCoins()

Expand Down Expand Up @@ -237,12 +238,14 @@ func (k BaseKeeper) UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAdd
return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, amt.String())
}

// call the BeforeSend hooks
err := k.BeforeSend(ctx, moduleAccAddr, delegatorAddr, amt)
// call the TrackBeforeSend hooks and the BlockBeforeSend hooks
err := k.BlockBeforeSend(ctx, moduleAccAddr, delegatorAddr, amt)
if err != nil {
return err
}

k.TrackBeforeSend(ctx, moduleAccAddr, delegatorAddr, amt)

err = k.subUnlockedCoins(ctx, moduleAccAddr, amt)
if err != nil {
return err
Expand Down Expand Up @@ -441,6 +444,8 @@ func (k BaseKeeper) SendCoinsFromModuleToManyAccounts(

// SendCoinsFromModuleToModule transfers coins from a ModuleAccount to another.
// It will panic if either module account does not exist.
// SendCoinsFromModuleToModule is the only send method that does not call both BlockBeforeSend and TrackBeforeSend hook.
// It only calls the TrackBeforeSend hook.
func (k BaseKeeper) SendCoinsFromModuleToModule(
ctx sdk.Context, senderModule, recipientModule string, amt sdk.Coins,
) error {
Expand All @@ -455,7 +460,7 @@ func (k BaseKeeper) SendCoinsFromModuleToModule(
panic(sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", recipientModule))
}

return k.SendCoins(ctx, senderAddr, recipientAcc.GetAddress(), amt)
return k.SendCoinsWithoutBlockHook(ctx, senderAddr, recipientAcc.GetAddress(), amt)
}

// SendCoinsFromAccountToModule transfers coins from an AccAddress to a ModuleAccount.
Expand Down
24 changes: 20 additions & 4 deletions x/bank/keeper/send.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,29 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input,
return nil
}

// 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 {
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 {
// call the BeforeSend hooks
err := k.BeforeSend(ctx, fromAddr, toAddr, amt)
// BlockBeforeSend hook should always be called before the TrackBeforeSend hook.
err := k.BlockBeforeSend(ctx, fromAddr, toAddr, amt)
if err != nil {
return err
}

err = k.subUnlockedCoins(ctx, fromAddr, amt)
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
k.TrackBeforeSend(ctx, fromAddr, toAddr, amt)

err := k.subUnlockedCoins(ctx, fromAddr, amt)
if err != nil {
return err
}
Expand Down Expand Up @@ -198,10 +211,13 @@ 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.BeforeSend(ctx, fromAddr, toAddrs[i], amts[i])
k.TrackBeforeSend(ctx, fromAddr, toAddrs[i], amts[i])

err := k.BlockBeforeSend(ctx, fromAddr, toAddrs[i], amts[i])
if err != nil {
return err
}

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

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

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

// 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 {
// 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) {
for i := range h {
err := h[i].BeforeSend(ctx, from, to, amount)
h[i].TrackBeforeSend(ctx, from, to, amount)
}
}

// 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)
if err != nil {
return err
}
Expand Down

0 comments on commit 4f857c6

Please sign in to comment.