Fix Initgenesis bug in tokenfactory, when the denom creation fee para… (

#2011)

* Fix Initgenesis bug in tokenfactory, when the denom creation fee param is set

* Add more to genesis test that would've caught this

* Update changelog

* Fix remaining bug adam pointed out

* Make test account for this

* Update code comment

.gitignore

@@ -202,3 +202,5 @@ $RECYCLE.BIN/
 /x/incentives/keeper/osmosis_testing/
 tools-stamp
 /tests/localosmosis/.osmosisd/*
+*.save
+*.save.*

CHANGELOG.md

@@ -78,6 +78,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [1759](https://github.com/osmosis-labs/osmosis/pull/1759) Fix pagination filter in incentives query.
 * [1698](https://github.com/osmosis-labs/osmosis/pull/1698) Register wasm snapshotter extension.
 * [1931](https://github.com/osmosis-labs/osmosis/pull/1931) Add explicit check for input denoms to `CalcJoinPoolShares`
+* [2011](https://github.com/osmosis-labs/osmosis/pull/2011) Fix bug in TokenFactory initGenesis, relating to denom creation fee param.
+
 
 ## [v9.0.0 - Nitrogen](https://github.com/osmosis-labs/osmosis/releases/tag/v9.0.0)
 

x/tokenfactory/keeper/createdenom.go

@@ -11,34 +11,23 @@ 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) {
-	// 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, " +
-			"can't create subdenoms that are the same as a native denom")
-	}
-
-	// Send creation fee to community pool
-	creationFee := k.GetParams(ctx).DenomCreationFee
-	accAddr, err := sdk.AccAddressFromBech32(creatorAddr)
+	err = k.chargeForCreateDenom(ctx, creatorAddr, subdenom)
 	if err != nil {
 		return "", err
 	}
-	if len(creationFee) > 0 {
-		if err := k.distrKeeper.FundCommunityPool(ctx, creationFee, accAddr); err != nil {
-			return "", err
-		}
-	}
 
-	denom, err := types.GetTokenDenom(creatorAddr, subdenom)
+	denom, err := k.validateCreateDenom(ctx, creatorAddr, subdenom)
 	if err != nil {
 		return "", err
 	}
 
-	_, found := k.bankKeeper.GetDenomMetaData(ctx, denom)
-	if found {
-		return "", types.ErrDenomExists
-	}
+	err = k.createDenomAfterValidation(ctx, creatorAddr, denom)
+	return denom, err
+}
 
+// 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) {
 	denomMetaData := banktypes.Metadata{
 		DenomUnits: []*banktypes.DenomUnit{{
 			Denom:    denom,
@@ -54,9 +43,44 @@ func (k Keeper) CreateDenom(ctx sdk.Context, creatorAddr string, subdenom string
 	}
 	err = k.setAuthorityMetadata(ctx, denom, authorityMetadata)
 	if err != nil {
-		return "", err
+		return err
 	}
 
 	k.addDenomFromCreator(ctx, creatorAddr, denom)
+	return nil
+}
+
+func (k Keeper) validateCreateDenom(ctx sdk.Context, creatorAddr string, 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, " +
+			"can't create subdenoms that are the same as a native denom")
+	}
+
+	denom, err := types.GetTokenDenom(creatorAddr, subdenom)
+	if err != nil {
+		return "", err
+	}
+
+	_, found := k.bankKeeper.GetDenomMetaData(ctx, denom)
+	if found {
+		return "", types.ErrDenomExists
+	}
+
 	return denom, nil
 }
+
+func (k Keeper) chargeForCreateDenom(ctx sdk.Context, creatorAddr string, subdenom string) (err error) {
+	// Send creation fee to community pool
+	creationFee := k.GetParams(ctx).DenomCreationFee
+	accAddr, err := sdk.AccAddressFromBech32(creatorAddr)
+	if err != nil {
+		return err
+	}
+	if len(creationFee) > 0 {
+		if err := k.distrKeeper.FundCommunityPool(ctx, creationFee, accAddr); err != nil {
+			return err
+		}
+	}
+	return nil
+}

x/tokenfactory/keeper/createdenom_test.go

@@ -5,7 +5,6 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
-	"github.com/osmosis-labs/osmosis/v7/x/tokenfactory/keeper"
 	"github.com/osmosis-labs/osmosis/v7/x/tokenfactory/types"
 )
 
@@ -90,7 +89,6 @@ func (suite *KeeperTestSuite) TestCreateDenom() {
 		},
 	} {
 		suite.Run(fmt.Sprintf("Case %s", tc.desc), func() {
-			suite.msgServer = keeper.NewMsgServerImpl(*suite.App.TokenFactoryKeeper)
 			if tc.setup != nil {
 				tc.setup()
 			}

x/tokenfactory/keeper/genesis.go

@@ -17,11 +17,11 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState types.GenesisState) {
 	k.SetParams(ctx, genState.Params)
 
 	for _, genDenom := range genState.GetFactoryDenoms() {
-		creator, subdenom, err := types.DeconstructDenom(genDenom.GetDenom())
+		creator, _, err := types.DeconstructDenom(genDenom.GetDenom())
 		if err != nil {
 			panic(err)
 		}
-		_, err = k.CreateDenom(ctx, creator, subdenom)
+		err = k.createDenomAfterValidation(ctx, creator, genDenom.GetDenom())
 		if err != nil {
 			panic(err)
 		}

x/tokenfactory/keeper/genesis_test.go

@@ -1,20 +1,14 @@
 package keeper_test
 
 import (
-	"testing"
-
-	"github.com/stretchr/testify/require"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 
-	simapp "github.com/osmosis-labs/osmosis/v7/app"
-	appparams "github.com/osmosis-labs/osmosis/v7/app/params"
-
 	"github.com/osmosis-labs/osmosis/v7/x/tokenfactory/types"
 )
 
-func TestGenesis(t *testing.T) {
-	appparams.SetAddressPrefixes()
-
+func (suite *KeeperTestSuite) TestGenesis() {
 	genesisState := types.GenesisState{
 		FactoryDenoms: []types.GenesisDenom{
 			{
@@ -23,6 +17,12 @@ func TestGenesis(t *testing.T) {
 					Admin: "osmo1t7egva48prqmzl59x5ngv4zx0dtrwewc9m7z44",
 				},
 			},
+			{
+				Denom: "factory/osmo1t7egva48prqmzl59x5ngv4zx0dtrwewc9m7z44/diff-admin",
+				AuthorityMetadata: types.DenomAuthorityMetadata{
+					Admin: "osmo15czt5nhlnvayqq37xun9s9yus0d6y26dw9xnzn",
+				},
+			},
 			{
 				Denom: "factory/osmo1t7egva48prqmzl59x5ngv4zx0dtrwewc9m7z44/litecoin",
 				AuthorityMetadata: types.DenomAuthorityMetadata{
@@ -31,11 +31,19 @@ func TestGenesis(t *testing.T) {
 			},
 		},
 	}
-	app := simapp.Setup(false)
-	ctx := app.BaseApp.NewContext(false, tmproto.Header{})
+	app := suite.App
+	suite.Ctx = app.BaseApp.NewContext(false, tmproto.Header{})
+	// Test both with bank denom metadata set, and not set.
+	for i, denom := range genesisState.FactoryDenoms {
+		// hacky, sets bank metadata to exist if i != 0, to cover both cases.
+		if i != 0 {
+			app.BankKeeper.SetDenomMetaData(suite.Ctx, banktypes.Metadata{Base: denom.GetDenom()})
+		}
+	}
 
-	app.TokenFactoryKeeper.InitGenesis(ctx, genesisState)
-	exportedGenesis := app.TokenFactoryKeeper.ExportGenesis(ctx)
-	require.NotNil(t, exportedGenesis)
-	require.Equal(t, genesisState, *exportedGenesis)
+	app.TokenFactoryKeeper.SetParams(suite.Ctx, types.Params{DenomCreationFee: sdk.Coins{sdk.NewInt64Coin("uosmo", 100)}})
+	app.TokenFactoryKeeper.InitGenesis(suite.Ctx, genesisState)
+	exportedGenesis := app.TokenFactoryKeeper.ExportGenesis(suite.Ctx)
+	suite.Require().NotNil(exportedGenesis)
+	suite.Require().Equal(genesisState, *exportedGenesis)
 }

x/tokenfactory/types/errors.go

@@ -10,7 +10,7 @@ import (
 
 // x/tokenfactory module sentinel errors
 var (
-	ErrDenomExists              = sdkerrors.Register(ModuleName, 2, "denom already exists")
+	ErrDenomExists              = sdkerrors.Register(ModuleName, 2, "attempting to create a denom that already exists (has bank metadata)")
 	ErrUnauthorized             = sdkerrors.Register(ModuleName, 3, "unauthorized account")
 	ErrInvalidDenom             = sdkerrors.Register(ModuleName, 4, "invalid denom")
 	ErrInvalidCreator           = sdkerrors.Register(ModuleName, 5, "invalid creator")