Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 10 additions & 16 deletions x/tokenfactory/keeper/before_send.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"
"encoding/json"

types2 "cosmossdk.io/store/types"
storetypes "cosmossdk.io/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/neutron-org/neutron/v7/x/tokenfactory/types"
Expand Down Expand Up @@ -108,7 +108,7 @@ func (k Keeper) callBeforeSendListener(ctx context.Context, from, to sdk.AccAddr

defer func() {
if r := recover(); r != nil {
err = types.ErrTrackBeforeSendOutOfGas
err = errorsmod.Wrapf(types.ErrBeforeSendHookOutOfGas, "%v", r)
}
}()

Expand Down Expand Up @@ -163,22 +163,16 @@ func (k Keeper) callBeforeSendListener(ctx context.Context, from, to sdk.AccAddr
return err
}

// if its track before send, apply gas meter to prevent infinite loop
if blockBeforeSend {
_, err = k.contractKeeper.Sudo(c, cwAddr, msgBz)
if err != nil {
return errorsmod.Wrapf(err, "failed to call before send hook for denom %s", coin.Denom)
}
} else {
childCtx := c.WithGasMeter(types2.NewGasMeter(types.TrackBeforeSendGasLimit))
_, err = k.contractKeeper.Sudo(childCtx, cwAddr, msgBz)
if err != nil {
return errorsmod.Wrapf(err, "failed to call before send hook for denom %s", coin.Denom)
}
em := sdk.NewEventManager()

// consume gas used for calling contract to the parent ctx
c.GasMeter().ConsumeGas(childCtx.GasMeter().GasConsumed(), "track before send gas")
childCtx := c.WithGasMeter(storetypes.NewGasMeter(types.BeforeSendHookGasLimit))
_, err = k.contractKeeper.Sudo(childCtx.WithEventManager(em), cwAddr, msgBz)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. In the line above, you're creating a new context with a gas manager, and then again, line later, a new context with an event manager - maybe it's better to combine both With calls into a single statement?

  2. What is the reason for creating a new event manager? We're essentially ignoring all events from sudo calls.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, fixed that

if err != nil {
return errorsmod.Wrapf(err, "failed to call before send hook for denom %s", coin.Denom)
}

// consume gas used for calling contract to the parent ctx
c.GasMeter().ConsumeGas(childCtx.GasMeter().GasConsumed(), "track before send gas")
}
}
return nil
Expand Down
9 changes: 9 additions & 0 deletions x/tokenfactory/keeper/before_send_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,15 @@ func (suite *KeeperTestSuite) TestBeforeSendHook() {
} else {
suite.Require().Error(err, "test: %v", sendTc.desc)
}

// this is a check to ensure bank keeper wired in token factory keeper has hooks properly set
// to check this, we try triggering bank hooks via token factory keeper
for _, coin := range sendTc.msg(denom).Amount {
_, err = suite.msgServer.Mint(suite.ChainA.GetContext(), types.NewMsgMint(suite.TestAccs[0].String(), sdk.NewInt64Coin(coin.Denom, coin.Amount.Int64())))
if sendTc.desc == "sending 100 of factorydenom should error" {
suite.Require().Error(err, "test: %v", sendTc.desc)
}
}
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions x/tokenfactory/types/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ package types

const ConsensusVersion = 2

// TrackBeforeSendGasLimit value is increased to allow existing approved tokenfactory hooks to work properly.
// BeforeSendHookGasLimit value is increased to allow existing approved tokenfactory hooks to work properly.
// In the next coordinated upgrade, this will become a chain parameter.
var TrackBeforeSendGasLimit = uint64(500_000)
var BeforeSendHookGasLimit = uint64(500_000)
2 changes: 1 addition & 1 deletion x/tokenfactory/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var (
ErrCreatorTooLong = errorsmod.Register(ModuleName, 9, fmt.Sprintf("creator too long, max length is %d bytes", MaxCreatorLength))
ErrDenomDoesNotExist = errorsmod.Register(ModuleName, 10, "denom does not exist")
ErrBurnFromModuleAccount = errorsmod.Register(ModuleName, 11, "burning from Module Account is not allowed")
ErrTrackBeforeSendOutOfGas = errorsmod.Register(ModuleName, 12, "gas meter hit maximum limit")
ErrBeforeSendHookOutOfGas = errorsmod.Register(ModuleName, 12, "gas meter hit maximum limit")
ErrInvalidHookContractAddress = errorsmod.Register(ModuleName, 13, "invalid hook contract address")
ErrBeforeSendHookNotWhitelisted = errorsmod.Register(ModuleName, 14, "beforeSendHook is not whitelisted")
)
Loading