Skip to content

Commit

Permalink
Fix bypass min fees wiring to use params value (#162)
Browse files Browse the repository at this point in the history
  • Loading branch information
@agouin @boojamya
agouin authored and boojamya committed Apr 19, 2023
1 parent 505bd16 commit 8b36086
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 12 deletions.
3 changes: 1 addition & 2 deletions app/ante.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ type HandlerOptions struct {
tokenFactoryKeeper *tokenfactory.Keeper
fiatTokenFactoryKeeper *fiattokenfactory.Keeper
IBCKeeper *ibckeeper.Keeper
BypassMinFeeMsgTypes []string
GlobalFeeSubspace paramtypes.Subspace
StakingSubspace paramtypes.Subspace
}
Expand Down Expand Up @@ -234,7 +233,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
ante.NewTxTimeoutHeightDecorator(),
ante.NewValidateMemoDecorator(options.AccountKeeper),
ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
feeante.NewFeeDecorator(options.BypassMinFeeMsgTypes, options.GlobalFeeSubspace, options.StakingSubspace, maxTotalBypassMinFeeMsgGasUsage),
feeante.NewFeeDecorator(options.GlobalFeeSubspace, options.StakingSubspace, maxTotalBypassMinFeeMsgGasUsage),

ante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper),
ante.NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
Expand Down
34 changes: 30 additions & 4 deletions interchaintest/globalfee_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestGlobalFee(t *testing.T) {
EncodingConfig: NobleEncoding(),
PreGenesis: func(cc ibc.ChainConfig) (err error) {
val := noble.Validators[0]
err = createTokenfactoryRoles(ctx, &roles, DenomMetadata_rupee, val, true)
err = createTokenfactoryRoles(ctx, &roles, DenomMetadata_rupee, val, false)
if err != nil {
return err
}
Expand All @@ -85,7 +85,7 @@ func TestGlobalFee(t *testing.T) {
if err := json.Unmarshal(b, &g); err != nil {
return nil, fmt.Errorf("failed to unmarshal genesis file: %w", err)
}
if err := modifyGenesisTokenfactory(g, "tokenfactory", DenomMetadata_rupee, &roles, true); err != nil {
if err := modifyGenesisTokenfactory(g, "tokenfactory", DenomMetadata_rupee, &roles, false); err != nil {
return nil, err
}
if err := modifyGenesisTokenfactory(g, "fiat-tokenfactory", DenomMetadata_drachma, &roles2, true); err != nil {
Expand Down Expand Up @@ -182,11 +182,37 @@ func TestGlobalFee(t *testing.T) {
require.Equal(t, uint32(0), tx.Code, "tx proposal failed")

// send tx with zero fees while the default MinimumGasPricesParam requires fees - tx should fail
_, err = nobleValidator.ExecTx(ctx, extraWallets.User2.KeyName, "bank", "send", extraWallets.User2.Address, extraWallets.Alice.Address, sendAmount100, "--gas-prices", zeroGasPrice)
_, err = nobleValidator.ExecTx(ctx, extraWallets.User2.KeyName,
"bank", "send",
extraWallets.User2.Address, extraWallets.Alice.Address, sendAmount100,
"--gas-prices", zeroGasPrice,
"-b", "block",
)
require.Error(t, err, "tx should not have succeeded with zero fees")

// send tx with the gas price set by MinimumGasPricesParam - tx should succeed
_, err = nobleValidator.ExecTx(ctx, extraWallets.User2.KeyName, "bank", "send", extraWallets.User2.Address, extraWallets.Alice.Address, sendAmount100, "--gas-prices", minGasPrice)
_, err = nobleValidator.ExecTx(ctx, extraWallets.User2.KeyName,
"bank", "send",
extraWallets.User2.Address, extraWallets.Alice.Address, sendAmount100,
"--gas-prices", minGasPrice,
"-b", "block",
)
require.NoError(t, err, "tx should have succeeded")

// send tx with zero fees while the default MinimumGasPricesParam requires fees, but update owner msg is in the bypass min fee msgs list - tx should succeed
_, err = nobleValidator.ExecTx(ctx, roles.Owner.KeyName,
"tokenfactory", "update-owner", roles.Owner2.Address,
"--gas-prices", zeroGasPrice,
"-b", "block",
)
require.NoError(t, err, "failed to execute update owner tx with zero fees")

// send tx with zero fees while the default MinimumGasPricesParam requires fees, but accept owner msg is in the bypass min fee msgs list - tx should succeed
_, err = nobleValidator.ExecTx(ctx, roles.Owner2.KeyName,
"tokenfactory", "accept-owner",
"--gas-prices", zeroGasPrice,
"-b", "block",
)
require.NoError(t, err, "failed to execute tx to accept ownership with zero fees")

}
6 changes: 2 additions & 4 deletions x/globalfee/ante/fee.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ import (
var _ sdk.AnteDecorator = FeeDecorator{}

type FeeDecorator struct {
BypassMinFeeMsgTypes []string
GlobalMinFee globalfee.ParamSource
StakingSubspace paramtypes.Subspace
MaxTotalBypassMinFeeMsgGasUsage uint64
}

func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace paramtypes.Subspace, maxTotalBypassMinFeeMsgGasUsage uint64) FeeDecorator {
func NewFeeDecorator(globalfeeSubspace, stakingSubspace paramtypes.Subspace, maxTotalBypassMinFeeMsgGasUsage uint64) FeeDecorator {
if !globalfeeSubspace.HasKeyTable() {
panic("global fee paramspace was not set up via module")
}
Expand All @@ -42,7 +41,6 @@ func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace
}

return FeeDecorator{
BypassMinFeeMsgTypes: bypassMsgTypes,
GlobalMinFee: globalfeeSubspace,
StakingSubspace: stakingSubspace,
MaxTotalBypassMinFeeMsgGasUsage: maxTotalBypassMinFeeMsgGasUsage,
Expand All @@ -66,7 +64,7 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne
// i.e., totalGas <= MaxTotalBypassMinFeeMsgGasUsage
// Otherwise, minimum fees and global fees are checked to prevent spam.
doesNotExceedMaxGasUsage := gas <= mfd.MaxTotalBypassMinFeeMsgGasUsage
allowedToBypassMinFee := mfd.containsOnlyBypassMinFeeMsgs(msgs) && doesNotExceedMaxGasUsage
allowedToBypassMinFee := mfd.containsOnlyBypassMinFeeMsgs(ctx, msgs) && doesNotExceedMaxGasUsage

var allFees sdk.Coins
requiredGlobalFees, err := mfd.getGlobalFee(ctx, feeTx)
Expand Down
11 changes: 9 additions & 2 deletions x/globalfee/ante/fee_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"math"

sdk "github.com/cosmos/cosmos-sdk/types"
globalfeetypes "github.com/strangelove-ventures/noble/x/globalfee/types"
tmstrings "github.com/tendermint/tendermint/libs/strings"
)

Expand All @@ -27,9 +28,15 @@ func getMinGasPrice(ctx sdk.Context, feeTx sdk.FeeTx) sdk.Coins {
return requiredFees.Sort()
}

func (mfd FeeDecorator) containsOnlyBypassMinFeeMsgs(msgs []sdk.Msg) bool {
func (mfd FeeDecorator) containsOnlyBypassMinFeeMsgs(ctx sdk.Context, msgs []sdk.Msg) bool {
var bypassMinFeeMsgTypes []string
if mfd.GlobalMinFee.Has(ctx, globalfeetypes.ParamStoreKeyBypassMinFeeMsgTypes) {
mfd.GlobalMinFee.Get(ctx, globalfeetypes.ParamStoreKeyBypassMinFeeMsgTypes, &bypassMinFeeMsgTypes)
} else {
bypassMinFeeMsgTypes = globalfeetypes.DefaultParams().BypassMinFeeMsgTypes
}
for _, msg := range msgs {
if tmstrings.StringInSlice(sdk.MsgTypeURL(msg), mfd.BypassMinFeeMsgTypes) {
if tmstrings.StringInSlice(sdk.MsgTypeURL(msg), bypassMinFeeMsgTypes) {
continue
}
return false
Expand Down
32 changes: 32 additions & 0 deletions x/globalfee/types/params.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,38 @@ func DefaultParams() Params {
"/ibc.applications.transfer.v1.MsgTransfer",
"/ibc.core.channel.v1.MsgTimeout",
"/ibc.core.channel.v1.MsgTimeoutOnClose",
"/cosmos.params.v1beta1.MsgUpdateParams",
"/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade",
"/cosmos.upgrade.v1beta1.MsgCancelUpgrade",
"/noble.fiattokenfactory.MsgUpdateMasterMinter",
"/noble.fiattokenfactory.MsgUpdatePauser",
"/noble.fiattokenfactory.MsgUpdateBlacklister",
"/noble.fiattokenfactory.MsgUpdateOwner",
"/noble.fiattokenfactory.MsgAcceptOwner",
"/noble.fiattokenfactory.MsgConfigureMinter",
"/noble.fiattokenfactory.MsgRemoveMinter",
"/noble.fiattokenfactory.MsgMint",
"/noble.fiattokenfactory.MsgBurn",
"/noble.fiattokenfactory.MsgBlacklist",
"/noble.fiattokenfactory.MsgUnblacklist",
"/noble.fiattokenfactory.MsgPause",
"/noble.fiattokenfactory.MsgUnpause",
"/noble.fiattokenfactory.MsgConfigureMinterController",
"/noble.fiattokenfactory.MsgRemoveMinterController",
"/noble.tokenfactory.MsgUpdatePauser",
"/noble.tokenfactory.MsgUpdateBlacklister",
"/noble.tokenfactory.MsgUpdateOwner",
"/noble.tokenfactory.MsgAcceptOwner",
"/noble.tokenfactory.MsgConfigureMinter",
"/noble.tokenfactory.MsgRemoveMinter",
"/noble.tokenfactory.MsgMint",
"/noble.tokenfactory.MsgBurn",
"/noble.tokenfactory.MsgBlacklist",
"/noble.tokenfactory.MsgUnblacklist",
"/noble.tokenfactory.MsgPause",
"/noble.tokenfactory.MsgUnpause",
"/noble.tokenfactory.MsgConfigureMinterController",
"/noble.tokenfactory.MsgRemoveMinterController",
},
}
}
Expand Down
32 changes: 32 additions & 0 deletions x/globalfee/types/params_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,38 @@ func TestDefaultParams(t *testing.T) {
"/ibc.applications.transfer.v1.MsgTransfer",
"/ibc.core.channel.v1.MsgTimeout",
"/ibc.core.channel.v1.MsgTimeoutOnClose",
"/cosmos.params.v1beta1.MsgUpdateParams",
"/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade",
"/cosmos.upgrade.v1beta1.MsgCancelUpgrade",
"/noble.fiattokenfactory.MsgUpdateMasterMinter",
"/noble.fiattokenfactory.MsgUpdatePauser",
"/noble.fiattokenfactory.MsgUpdateBlacklister",
"/noble.fiattokenfactory.MsgUpdateOwner",
"/noble.fiattokenfactory.MsgAcceptOwner",
"/noble.fiattokenfactory.MsgConfigureMinter",
"/noble.fiattokenfactory.MsgRemoveMinter",
"/noble.fiattokenfactory.MsgMint",
"/noble.fiattokenfactory.MsgBurn",
"/noble.fiattokenfactory.MsgBlacklist",
"/noble.fiattokenfactory.MsgUnblacklist",
"/noble.fiattokenfactory.MsgPause",
"/noble.fiattokenfactory.MsgUnpause",
"/noble.fiattokenfactory.MsgConfigureMinterController",
"/noble.fiattokenfactory.MsgRemoveMinterController",
"/noble.tokenfactory.MsgUpdatePauser",
"/noble.tokenfactory.MsgUpdateBlacklister",
"/noble.tokenfactory.MsgUpdateOwner",
"/noble.tokenfactory.MsgAcceptOwner",
"/noble.tokenfactory.MsgConfigureMinter",
"/noble.tokenfactory.MsgRemoveMinter",
"/noble.tokenfactory.MsgMint",
"/noble.tokenfactory.MsgBurn",
"/noble.tokenfactory.MsgBlacklist",
"/noble.tokenfactory.MsgUnblacklist",
"/noble.tokenfactory.MsgPause",
"/noble.tokenfactory.MsgUnpause",
"/noble.tokenfactory.MsgConfigureMinterController",
"/noble.tokenfactory.MsgRemoveMinterController",
})
}

Expand Down

0 comments on commit 8b36086

Please sign in to comment.