review fixes

proto/osmosis/tokenfactory/v1beta1/query.proto

@@ -86,6 +86,6 @@ message QueryBeforeSendHookAddressRequest {
 // QueryBeforeSendHookAddressResponse defines the response structure for the
 // DenomBeforeSendHook gRPC query.
 message QueryBeforeSendHookAddressResponse {
-  string cosmwasm_address = 1
-  [ (gogoproto.moretags) = "yaml:\"cosmwasm_address\"" ];
+  string contract_addr = 1
+  [ (gogoproto.moretags) = "yaml:\"contract_addr\"" ];
 }

proto/osmosis/tokenfactory/v1beta1/tx.proto

@@ -103,8 +103,8 @@ message MsgSetBeforeSendHook {
 
   string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
   string denom = 2 [ (gogoproto.moretags) = "yaml:\"denom\"" ];
-  string cosmwasm_address = 3
-  [ (gogoproto.moretags) = "yaml:\"cosmwasm_address\"" ];
+  string contract_addr = 3
+  [ (gogoproto.moretags) = "yaml:\"contract_addr\"" ];
 }
 
 // MsgSetBeforeSendHookResponse defines the response structure for an executed

wasmbinding/bindings/msg.go

@@ -185,7 +185,7 @@ type BurnTokens struct {
 
 type SetBeforeSendHook struct {
 	Denom        string `json:"denom"`
-	CosmWasmAddr string `json:"cosm_wasm_addr"`
+	ContractAddr string `json:"contract_addr"`
 }
 
 // AddSchedule adds new schedule to the cron module

wasmbinding/message_plugin.go

@@ -536,7 +536,7 @@ func PerformMint(f *tokenfactorykeeper.Keeper, b *bankkeeper.BaseKeeper, ctx sdk
 }
 
 func PerformSetBeforeSendHook(f *tokenfactorykeeper.Keeper, ctx sdk.Context, contractAddr sdk.AccAddress, set *bindings.SetBeforeSendHook) error {
-	sdkMsg := tokenfactorytypes.NewMsgSetBeforeSendHook(contractAddr.String(), set.Denom, set.CosmWasmAddr)
+	sdkMsg := tokenfactorytypes.NewMsgSetBeforeSendHook(contractAddr.String(), set.Denom, set.ContractAddr)
 	if err := sdkMsg.ValidateBasic(); err != nil {
 		return err
 	}

wasmbinding/queries.go

@@ -97,9 +97,9 @@ func (qp QueryPlugin) GetDenomAdmin(ctx sdk.Context, denom string) (*bindings.De
 
 // GetBeforeSendHook is a query to get denom before send hook.
 func (qp QueryPlugin) GetBeforeSendHook(ctx sdk.Context, denom string) (*bindings.BeforeSendHookResponse, error) {
-	cosmWasmAddr := qp.tokenFactoryKeeper.GetBeforeSendHook(ctx, denom)
+	contractAddr := qp.tokenFactoryKeeper.GetBeforeSendHook(ctx, denom)
 
-	return &bindings.BeforeSendHookResponse{ContractAddr: cosmWasmAddr}, nil
+	return &bindings.BeforeSendHookResponse{ContractAddr: contractAddr}, nil
 }
 
 func (qp *QueryPlugin) GetTotalBurnedNeutronsAmount(ctx sdk.Context, _ *bindings.QueryTotalBurnedNeutronsAmountRequest) (*bindings.QueryTotalBurnedNeutronsAmountResponse, error) {

x/tokenfactory/client/cli/query.go

@@ -79,8 +79,8 @@ func GetCmdDenomAuthorityMetadata() *cobra.Command {
 			}
 
 			res, err := queryClient.DenomAuthorityMetadata(cmd.Context(), &types.QueryDenomAuthorityMetadataRequest{
-				Creator:  args[1],
-				Subdenom: args[2],
+				Creator:  denom[1],
+				Subdenom: denom[2],
 			})
 			if err != nil {
 				return err

x/tokenfactory/client/cli/tx.go

@@ -204,7 +204,7 @@ func NewChangeAdminCmd() *cobra.Command {
 // NewSetBeforeSendHook broadcast MsgSetBeforeSendHook
 func NewSetBeforeSendHook() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "set-before-send-hook [denom] [cosm-wasm-addr] [flags]",
+		Use:   "set-before-send-hook [denom] [contract-addr] [flags]",
 		Short: "Sets the before send hook for a factory-created denom. Must have admin authority to do so.",
 		Args:  cobra.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {

x/tokenfactory/keeper/admins.go

@@ -37,7 +37,7 @@ func (k Keeper) setAuthorityMetadata(ctx sdk.Context, denom string, metadata typ
 	return nil
 }
 
-func (k Keeper) setAdmin(ctx sdk.Context, denom string, admin string) error {
+func (k Keeper) setAdmin(ctx sdk.Context, denom, admin string) error {
 	metadata, err := k.GetAuthorityMetadata(ctx, denom)
 	if err != nil {
 		return err

x/tokenfactory/keeper/bankactions.go

@@ -51,7 +51,7 @@ func (k Keeper) burnFrom(ctx sdk.Context, amount sdk.Coin, burnFrom string) erro
 	return k.bankKeeper.BurnCoins(ctx, types.ModuleName, sdk.NewCoins(amount))
 }
 
-func (k Keeper) forceTransfer(ctx sdk.Context, amount sdk.Coin, fromAddr string, toAddr string) error {
+func (k Keeper) forceTransfer(ctx sdk.Context, amount sdk.Coin, fromAddr, toAddr string) error {
 	// verify that denom is an x/tokenfactory denom
 	_, _, err := types.DeconstructDenom(amount.Denom)
 	if err != nil {

x/tokenfactory/keeper/before_send.go

@@ -11,7 +11,7 @@ import (
 	wasmvmtypes "github.com/CosmWasm/wasmvm/types"
 )
 
-func (k Keeper) setBeforeSendHook(ctx sdk.Context, denom string, cosmwasmAddress string) error {
+func (k Keeper) setBeforeSendHook(ctx sdk.Context, denom, contractAddr string) error {
 	// verify that denom is an x/tokenfactory denom
 	_, _, err := types.DeconstructDenom(denom)
 	if err != nil {
@@ -21,17 +21,17 @@ func (k Keeper) setBeforeSendHook(ctx sdk.Context, denom string, cosmwasmAddress
 	store := k.GetDenomPrefixStore(ctx, denom)
 
 	// delete the store for denom prefix store when cosmwasm address is nil
-	if cosmwasmAddress == "" {
+	if contractAddr == "" {
 		store.Delete([]byte(types.BeforeSendHookAddressPrefixKey))
 		return nil
 	}
 
-	_, err = sdk.AccAddressFromBech32(cosmwasmAddress)
+	_, err = sdk.AccAddressFromBech32(contractAddr)
 	if err != nil {
 		return err
 	}
 
-	store.Set([]byte(types.BeforeSendHookAddressPrefixKey), []byte(cosmwasmAddress))
+	store.Set([]byte(types.BeforeSendHookAddressPrefixKey), []byte(contractAddr))
 
 	return nil
 }
@@ -96,9 +96,9 @@ func (k Keeper) callBeforeSendListener(ctx sdk.Context, from, to sdk.AccAddress,
 	}()
 
 	for _, coin := range amount {
-		cosmwasmAddress := k.GetBeforeSendHook(ctx, coin.Denom)
-		if cosmwasmAddress != "" {
-			cwAddr, err := sdk.AccAddressFromBech32(cosmwasmAddress)
+		contractAddr := k.GetBeforeSendHook(ctx, coin.Denom)
+		if contractAddr != "" {
+			cwAddr, err := sdk.AccAddressFromBech32(contractAddr)
 			if err != nil {
 				return err
 			}

x/tokenfactory/keeper/createdenom.go

@@ -11,7 +11,7 @@ import (
 )
 
 // ConvertToBaseToken converts a fee amount in a whitelisted fee token to the base fee token amount
-func (k Keeper) CreateDenom(ctx sdk.Context, creatorAddr string, subdenom string) (newTokenDenom string, err error) {
+func (k Keeper) CreateDenom(ctx sdk.Context, creatorAddr, subdenom string) (newTokenDenom string, err error) {
 	denom, err := k.validateCreateDenom(ctx, creatorAddr, subdenom)
 	if err != nil {
 		return "", err
@@ -28,7 +28,7 @@ func (k Keeper) CreateDenom(ctx sdk.Context, creatorAddr string, subdenom string
 
 // Runs CreateDenom logic after the charge and all denom validation has been handled.
 // Made into a second function for genesis initialization.
-func (k Keeper) createDenomAfterValidation(ctx sdk.Context, creatorAddr string, denom string) (err error) {
+func (k Keeper) createDenomAfterValidation(ctx sdk.Context, creatorAddr, denom string) (err error) {
 	_, exists := k.bankKeeper.GetDenomMetaData(ctx, denom)
 	if !exists {
 		denomMetaData := banktypes.Metadata{
@@ -54,7 +54,7 @@ func (k Keeper) createDenomAfterValidation(ctx sdk.Context, creatorAddr string,
 	return nil
 }
 
-func (k Keeper) validateCreateDenom(ctx sdk.Context, creatorAddr string, subdenom string) (newTokenDenom string, err error) {
+func (k Keeper) validateCreateDenom(ctx sdk.Context, creatorAddr, subdenom string) (newTokenDenom string, err error) {
 	// Temporary check until IBC bug is sorted out
 	if k.bankKeeper.HasSupply(ctx, subdenom) {
 		return "", fmt.Errorf("temporary error until IBC bug is sorted out, " +

x/tokenfactory/keeper/grpc_query.go

@@ -40,7 +40,7 @@ func (k Keeper) BeforeSendHookAddress(ctx context.Context, req *types.QueryBefor
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 
 	denom := fmt.Sprintf("factory/%s/%s", req.GetCreator(), req.GetSubdenom())
-	cosmwasmAddress := k.GetBeforeSendHook(sdkCtx, denom)
+	contractAddr := k.GetBeforeSendHook(sdkCtx, denom)
 
-	return &types.QueryBeforeSendHookAddressResponse{CosmwasmAddress: cosmwasmAddress}, nil
+	return &types.QueryBeforeSendHookAddressResponse{ContractAddr: contractAddr}, nil
 }

x/tokenfactory/keeper/keeper.go

@@ -81,5 +81,6 @@ func (k *Keeper) SetContractKeeper(contractKeeper types.ContractKeeper) {
 // it purely mints and burns them on behalf of the admin of respective denoms,
 // and sends to the relevant address.
 func (k Keeper) CreateModuleAccount(ctx sdk.Context) {
+	// GetModuleAccount creates new module account if not present under the hood
 	k.accountKeeper.GetModuleAccount(ctx, types.ModuleName)
 }

x/tokenfactory/keeper/msg_server.go

@@ -218,7 +218,7 @@ func (server msgServer) SetBeforeSendHook(goCtx context.Context, msg *types.MsgS
 		return nil, types.ErrUnauthorized
 	}
 
-	err = server.Keeper.setBeforeSendHook(ctx, msg.Denom, msg.CosmwasmAddress)
+	err = server.Keeper.setBeforeSendHook(ctx, msg.Denom, msg.ContractAddr)
 	if err != nil {
 		return nil, err
 	}
@@ -227,7 +227,7 @@ func (server msgServer) SetBeforeSendHook(goCtx context.Context, msg *types.MsgS
 		sdk.NewEvent(
 			types.TypeMsgSetBeforeSendHook,
 			sdk.NewAttribute(types.AttributeDenom, msg.GetDenom()),
-			sdk.NewAttribute(types.AttributeBeforeSendHookAddress, msg.GetCosmwasmAddress()),
+			sdk.NewAttribute(types.AttributeBeforeSendHookAddress, msg.GetContractAddr()),
 		),
 	})
 
@@ -236,9 +236,9 @@ func (server msgServer) SetBeforeSendHook(goCtx context.Context, msg *types.MsgS
 
 // UpdateParams updates the module parameters
 func (k Keeper) UpdateParams(goCtx context.Context, req *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
-	//if err := req.ValidateBasic(); err != nil {
-	//	return nil, err
-	//}
+	if err := req.ValidateBasic(); err != nil {
+		return nil, err
+	}
 	authority := k.GetAuthority()
 	if authority != req.Authority {
 		return nil, errors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid authority; expected %s, got %s", authority, req.Authority)

x/tokenfactory/types/codec.go

@@ -14,7 +14,7 @@ func RegisterCodec(cdc *codec.LegacyAmino) {
 	cdc.RegisterConcrete(&MsgCreateDenom{}, "osmosis/tokenfactory/create-denom", nil)
 	cdc.RegisterConcrete(&MsgMint{}, "osmosis/tokenfactory/mint", nil)
 	cdc.RegisterConcrete(&MsgBurn{}, "osmosis/tokenfactory/burn", nil)
-	//cdc.RegisterConcrete(&MsgForceTransfer{}, "osmosis/tokenfactory/force-transfer", nil)
+	// cdc.RegisterConcrete(&MsgForceTransfer{}, "osmosis/tokenfactory/force-transfer", nil)
 	cdc.RegisterConcrete(&MsgChangeAdmin{}, "osmosis/tokenfactory/change-admin", nil)
 	cdc.RegisterConcrete(&MsgSetBeforeSendHook{}, "osmosis/tokenfactory/set-beforesend-hook", nil)
 	cdc.RegisterConcrete(&MsgUpdateParams{}, "osmosis/tokenfactory/update-params", nil)

x/tokenfactory/types/constants.go

@@ -1,5 +1,3 @@
 package types
 
-var (
-	TrackBeforeSendGasLimit = uint64(100_000)
-)
+var TrackBeforeSendGasLimit = uint64(100_000)

x/tokenfactory/types/denoms.go

@@ -40,7 +40,7 @@ func GetTokenDenom(creator, subdenom string) (string, error) {
 // DeconstructDenom takes a token denom string and verifies that it is a valid
 // denom of the tokenfactory module, and is of the form `factory/{creator}/{subdenom}`
 // If valid, it returns the creator address and subdenom
-func DeconstructDenom(denom string) (creator string, subdenom string, err error) {
+func DeconstructDenom(denom string) (creator, subdenom string, err error) {
 	err = sdk.ValidateDenom(denom)
 	if err != nil {
 		return "", "", err

x/tokenfactory/types/expected_keepers.go

@@ -18,7 +18,7 @@ type BankKeeper interface {
 	MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
 	BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error
 
-	SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
+	SendCoins(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error
 	HasBalance(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coin) bool
 }
 

x/tokenfactory/types/msgs.go

@@ -266,11 +266,11 @@ func (m MsgSetDenomMetadata) GetSigners() []sdk.AccAddress {
 var _ sdk.Msg = &MsgSetBeforeSendHook{}
 
 // NewMsgSetBeforeSendHook creates a message to set a new before send hook
-func NewMsgSetBeforeSendHook(sender string, denom string, cosmwasmAddress string) *MsgSetBeforeSendHook {
+func NewMsgSetBeforeSendHook(sender, denom, contractAddr string) *MsgSetBeforeSendHook {
 	return &MsgSetBeforeSendHook{
-		Sender:          sender,
-		Denom:           denom,
-		CosmwasmAddress: cosmwasmAddress,
+		Sender:       sender,
+		Denom:        denom,
+		ContractAddr: contractAddr,
 	}
 }
 
@@ -282,8 +282,8 @@ func (m MsgSetBeforeSendHook) ValidateBasic() error {
 		return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", err)
 	}
 
-	if m.CosmwasmAddress != "" {
-		_, err = sdk.AccAddressFromBech32(m.CosmwasmAddress)
+	if m.ContractAddr != "" {
+		_, err = sdk.AccAddressFromBech32(m.ContractAddr)
 		if err != nil {
 			return errorsmod.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid cosmwasm contract address (%s)", err)
 		}

x/tokenfactory/types/params.go

@@ -11,47 +11,58 @@ import (
 var (
 	KeyDenomCreationFee        = []byte("DenomCreationFee")
 	KeyDenomCreationGasConsume = []byte("DenomCreationGasConsume")
-	DefaultNeutronDenom        = "untrn"
-	// chosen as an arbitrary large number, less than the max_gas_wanted_per_tx in config.
-	DefaultCreationGasFee       = 1_000
-	DefaultFeeAmount      int64 = 1_000_000
+	KeyFeeCollectorAddress     = []byte("FeeCollectorAddress")
+	// We don't want to charge users for denom creation
+	DefaultDenomCreationFee        sdk.Coins = nil
+	DefaultDenomCreationGasConsume uint64    = 0
+	DefaultFeeCollectorAddress               = ""
 )
 
-// ParamTable for tokenfactory module.
+// ParamKeyTable the param key table for tokenfactory module.
 func ParamKeyTable() paramtypes.KeyTable {
 	return paramtypes.NewKeyTable().RegisterParamSet(&Params{})
 }
 
-func NewParams(denomCreationFee sdk.Coins, denomCreationGasConsume uint64) Params {
+func NewParams(denomCreationFee sdk.Coins, denomCreationGasConsume uint64, feeCollectorAddress string) Params {
 	return Params{
 		DenomCreationFee:        denomCreationFee,
 		DenomCreationGasConsume: denomCreationGasConsume,
+		FeeCollectorAddress:     feeCollectorAddress,
 	}
 }
 
-// default tokenfactory module parameters.
+// DefaultParams returns a default set of parameters
 func DefaultParams() Params {
-	return Params{
-		// We don't want to charge users for denom creation
-		DenomCreationFee:        nil,
-		DenomCreationGasConsume: 0,
-	}
+	return NewParams(DefaultDenomCreationFee, DefaultDenomCreationGasConsume, DefaultFeeCollectorAddress)
 }
 
-// validate params.
+// Validate validates params
 func (p Params) Validate() error {
+	// DenomCreationFee and FeeCollectorAddress must be both set, or both unset
+	hasDenomCreationFee := len(p.DenomCreationFee) > 0
+	hasFeeCollectorAddress := p.FeeCollectorAddress != ""
+
+	if hasDenomCreationFee != hasFeeCollectorAddress {
+		return fmt.Errorf("DenomCreationFee and FeeCollectorAddr must be both set or both unset")
+	}
+
 	if err := validateDenomCreationFee(p.DenomCreationFee); err != nil {
 		return err
 	}
 
+	if err := validateAddress(p.FeeCollectorAddress); err != nil {
+		return err
+	}
+
 	return nil
 }
 
-// Implements params.ParamSet.
+// ParamSetPairs get the params.ParamSet
 func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	return paramtypes.ParamSetPairs{
 		paramtypes.NewParamSetPair(KeyDenomCreationFee, &p.DenomCreationFee, validateDenomCreationFee),
 		paramtypes.NewParamSetPair(KeyDenomCreationGasConsume, &p.DenomCreationGasConsume, validateDenomCreationGasConsume),
+		paramtypes.NewParamSetPair(KeyFeeCollectorAddress, &p.FeeCollectorAddress, validateAddress),
 	}
 }
 
@@ -76,3 +87,21 @@ func validateDenomCreationGasConsume(i interface{}) error {
 
 	return nil
 }
+
+func validateAddress(i interface{}) error {
+	v, ok := i.(string)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+
+	if len(v) == 0 {
+		return nil
+	}
+
+	_, err := sdk.AccAddressFromBech32(v)
+	if err != nil {
+		return fmt.Errorf("invalid address: %w", err)
+	}
+
+	return nil
+}