chore(feemarket) - Depreacte usage of x/params in x/feemarket (#1509)

* (refactor): Added new MsgUpdateParams tx and generated new proto files

* (refactor): Refactor for migration of x/params

* (fix): Refactor to use single Params store key for easier more readable code

* (fix): removed unused

* (fix): add validation

* (fix): fix linter

* remove line

* Added changes from code review

* Apply changes from code review

* (fix): Made ParamKey back to a string

* Added CHANGELOG entry

* Apply suggestions from code review

Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com>

* (fix): remove HTTP endpoint exposure

* Apply suggestions from code review

Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>

* fix: Apply changes from code review and run linter

* Update x/feemarket/keeper/params.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* Update x/feemarket/types/msg.go

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

* tests: added tests for msg_server and msg

* tests: add failing test for migration

* Update x/feemarket/keeper/params.go

Co-authored-by: Tomas Guerra <54514587+GAtom22@users.noreply.github.com>
Co-authored-by: MalteHerrmann <42640438+MalteHerrmann@users.noreply.github.com>
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

CHANGELOG.md

@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### State Machine Breaking
 
+* (feemarket) [#1509](https://github.com/evmos/ethermint/pull/1509) Deprecate usage of x/params in x/feemarket
 * (evm) [#1472](https://github.com/evmos/ethermint/pull/1472) Deprecate x/params usage in x/evm
 * (deps) #[1575](https://github.com/evmos/ethermint/pull/1575) bump ibc-go to [`v6.1.0`]
 * (deps) [#1361](https://github.com/evmos/ethermint/pull/1361) Bump ibc-go to [`v5.1.0`](https://github.com/cosmos/ibc-go/releases/tag/v5.1.0)

app/app.go

@@ -413,7 +413,8 @@ func NewEthermintApp(
 
 	// Create Ethermint keepers
 	app.FeeMarketKeeper = feemarketkeeper.NewKeeper(
-		appCodec, app.GetSubspace(feemarkettypes.ModuleName), keys[feemarkettypes.StoreKey], tkeys[feemarkettypes.TransientKey],
+		appCodec, authtypes.NewModuleAddress(govtypes.ModuleName),
+		keys[feemarkettypes.StoreKey], tkeys[feemarkettypes.TransientKey],
 	)
 
 	// Set authority to x/gov module account to only expect the module account to update params
@@ -506,8 +507,8 @@ func NewEthermintApp(
 		ibc.NewAppModule(app.IBCKeeper),
 		transferModule,
 		// Ethermint app modules
+		feemarket.NewAppModule(app.FeeMarketKeeper, app.GetSubspace(feemarkettypes.ModuleName)),
 		evm.NewAppModule(app.EvmKeeper, app.AccountKeeper, app.GetSubspace(evmtypes.ModuleName)),
-		feemarket.NewAppModule(app.FeeMarketKeeper),
 	)
 
 	// During begin block slashing happens after distr.BeginBlocker so that

proto/ethermint/feemarket/v1/tx.proto

@@ -0,0 +1,30 @@
+syntax = "proto3";
+package ethermint.feemarket.v1;
+
+import "cosmos/msg/v1/msg.proto";
+import "cosmos_proto/cosmos.proto";
+import "ethermint/feemarket/v1/feemarket.proto";
+import "gogoproto/gogo.proto";
+
+option go_package = "github.com/evmos/ethermint/x/feemarket/types";
+
+// Msg defines the erc20 Msg service.
+service Msg {
+  // UpdateParams defined a governance operation for updating the x/feemarket module parameters.
+  // The authority is hard-coded to the Cosmos SDK x/gov module account
+  rpc UpdateParams(MsgUpdateParams) returns (MsgUpdateParamsResponse);
+}
+
+// MsgUpdateParams defines a Msg for updating the x/feemarket module parameters.
+message MsgUpdateParams {
+  option (cosmos.msg.v1.signer) = "authority";
+  // authority is the address of the governance account.
+  string authority = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  // params defines the x/feemarket parameters to update.
+  // NOTE: All parameters must be supplied.
+  Params params = 2 [(gogoproto.nullable) = false];
+}
+
+// MsgUpdateParamsResponse defines the response structure for executing a
+// MsgUpdateParams message.
+message MsgUpdateParamsResponse {}

x/feemarket/genesis.go

@@ -16,6 +16,7 @@
 package feemarket
 
 import (
+	errorsmod "cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	abci "github.com/tendermint/tendermint/abci/types"
 
@@ -29,7 +30,11 @@ func InitGenesis(
 	k keeper.Keeper,
 	data types.GenesisState,
 ) []abci.ValidatorUpdate {
-	k.SetParams(ctx, data.Params)
+	err := k.SetParams(ctx, data.Params)
+	if err != nil {
+		panic(errorsmod.Wrap(err, "could not set parameters at genesis"))
+	}
+
 	k.SetBlockGasWanted(ctx, data.BlockGas)
 
 	return []abci.ValidatorUpdate{}

x/feemarket/handler.go

@@ -0,0 +1,26 @@
+package feemarket
+
+import (
+	errorsmod "cosmossdk.io/errors"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	errortypes "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/evmos/ethermint/x/feemarket/types"
+)
+
+// NewHandler returns a handler for Ethermint type messages.
+func NewHandler(server types.MsgServer) sdk.Handler {
+	return func(ctx sdk.Context, msg sdk.Msg) (result *sdk.Result, err error) {
+		ctx = ctx.WithEventManager(sdk.NewEventManager())
+
+		switch msg := msg.(type) {
+		case *types.MsgUpdateParams:
+			// execute state transition
+			res, err := server.UpdateParams(sdk.WrapSDKContext(ctx), msg)
+			return sdk.WrapServiceResult(ctx, res, err)
+
+		default:
+			err := errorsmod.Wrapf(errortypes.ErrUnknownRequest, "unrecognized %s message type: %T", types.ModuleName, msg)
+			return nil, err
+		}
+	}
+}

x/feemarket/keeper/keeper.go

@@ -21,7 +21,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	"github.com/tendermint/tendermint/libs/log"
 
 	"github.com/evmos/ethermint/x/feemarket/types"
@@ -37,23 +36,23 @@ type Keeper struct {
 	// Store key required for the Fee Market Prefix KVStore.
 	storeKey     storetypes.StoreKey
 	transientKey storetypes.StoreKey
-	// module specific parameter space that can be configured through governance
-	paramSpace paramtypes.Subspace
+	// the address capable of executing a MsgUpdateParams message. Typically, this should be the x/gov module account.
+	authority sdk.AccAddress
 }
 
 // NewKeeper generates new fee market module keeper
 func NewKeeper(
-	cdc codec.BinaryCodec, paramSpace paramtypes.Subspace, storeKey, transientKey storetypes.StoreKey,
+	cdc codec.BinaryCodec, authority sdk.AccAddress, storeKey, transientKey storetypes.StoreKey,
 ) Keeper {
-	// set KeyTable if it has not already been set
-	if !paramSpace.HasKeyTable() {
-		paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
+	// ensure authority account is correctly formatted
+	if err := sdk.VerifyAddressFormat(authority); err != nil {
+		panic(err)
 	}
 
 	return Keeper{
 		cdc:          cdc,
 		storeKey:     storeKey,
-		paramSpace:   paramSpace,
+		authority:    authority,
 		transientKey: transientKey,
 	}
 }
@@ -113,6 +112,7 @@ func (k Keeper) AddTransientGasWanted(ctx sdk.Context, gasWanted uint64) (uint64
 
 // GetBaseFeeV1 get the base fee from v1 version of states.
 // return nil if base fee is not enabled
+// TODO: Figure out if this will be deleted ?
 func (k Keeper) GetBaseFeeV1(ctx sdk.Context) *big.Int {
 	store := ctx.KVStore(k.storeKey)
 	bz := store.Get(KeyPrefixBaseFeeV1)

x/feemarket/keeper/migrations.go

@@ -15,14 +15,27 @@
 // along with the Ethermint library. If not, see https://github.com/evmos/ethermint/blob/main/LICENSE
 package keeper
 
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	v4 "github.com/evmos/ethermint/x/feemarket/migrations/v4"
+	"github.com/evmos/ethermint/x/feemarket/types"
+)
+
 // Migrator is a struct for handling in-place store migrations.
 type Migrator struct {
-	keeper Keeper
+	keeper         Keeper
+	legacySubspace types.Subspace
 }
 
 // NewMigrator returns a new Migrator.
-func NewMigrator(keeper Keeper) Migrator {
+func NewMigrator(keeper Keeper, legacySubspace types.Subspace) Migrator {
 	return Migrator{
-		keeper: keeper,
+		keeper:         keeper,
+		legacySubspace: legacySubspace,
 	}
 }
+
+// Migrate3to4 migrates the store from consensus version 3 to 4
+func (m Migrator) Migrate3to4(ctx sdk.Context) error {
+	return v4.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc)
+}

x/feemarket/keeper/migrations_test.go

@@ -1 +1,42 @@
 package keeper_test
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	feemarketkeeper "github.com/evmos/ethermint/x/feemarket/keeper"
+	v4types "github.com/evmos/ethermint/x/feemarket/migrations/v4/types"
+	"github.com/evmos/ethermint/x/feemarket/types"
+)
+
+type mockSubspace struct {
+	ps v4types.Params
+}
+
+func newMockSubspace(ps v4types.Params) mockSubspace {
+	return mockSubspace{ps: ps}
+}
+
+func (ms mockSubspace) GetParamSetIfExists(_ sdk.Context, ps types.LegacyParams) {
+	*ps.(*v4types.Params) = ms.ps
+}
+
+func (suite *KeeperTestSuite) TestMigrations() {
+	legacySubspace := newMockSubspace(v4types.DefaultParams())
+	migrator := feemarketkeeper.NewMigrator(suite.app.FeeMarketKeeper, legacySubspace)
+
+	testCases := []struct {
+		name        string
+		migrateFunc func(ctx sdk.Context) error
+	}{
+		{
+			"Run Migrate3to4",
+			migrator.Migrate3to4,
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			err := tc.migrateFunc(suite.ctx)
+			suite.Require().NoError(err)
+		})
+	}
+}

x/feemarket/keeper/msg_server.go

@@ -0,0 +1,27 @@
+package keeper
+
+import (
+	"context"
+
+	errorsmod "cosmossdk.io/errors"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+	"github.com/evmos/ethermint/x/feemarket/types"
+)
+
+// UpdateParams implements the gRPC MsgServer interface. When an UpdateParams
+// proposal passes, it updates the module parameters. The update can only be
+// performed if the requested authority is the Cosmos SDK governance module
+// account.
+func (k *Keeper) UpdateParams(goCtx context.Context, req *types.MsgUpdateParams) (*types.MsgUpdateParamsResponse, error) {
+	if k.authority.String() != req.Authority {
+		return nil, errorsmod.Wrapf(govtypes.ErrInvalidSigner, "invalid authority; expected %s, got %s", k.authority.String(), req.Authority)
+	}
+
+	ctx := sdk.UnwrapSDKContext(goCtx)
+	if err := k.SetParams(ctx, req.Params); err != nil {
+		return nil, err
+	}
+
+	return &types.MsgUpdateParamsResponse{}, nil
+}

x/feemarket/keeper/msg_server_test.go

@@ -0,0 +1,40 @@
+package keeper_test
+
+import (
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+	"github.com/evmos/ethermint/x/feemarket/types"
+)
+
+func (suite *KeeperTestSuite) TestUpdateParams() {
+	testCases := []struct {
+		name      string
+		request   *types.MsgUpdateParams
+		expectErr bool
+	}{
+		{
+			name:      "fail - invalid authority",
+			request:   &types.MsgUpdateParams{Authority: "foobar"},
+			expectErr: true,
+		},
+		{
+			name: "pass - valid Update msg",
+			request: &types.MsgUpdateParams{
+				Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
+				Params:    types.DefaultParams(),
+			},
+			expectErr: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run("MsgUpdateParams", func() {
+			_, err := suite.app.FeeMarketKeeper.UpdateParams(suite.ctx, tc.request)
+			if tc.expectErr {
+				suite.Require().Error(err)
+			} else {
+				suite.Require().NoError(err)
+			}
+		})
+	}
+}

x/feemarket/keeper/params.go

@@ -18,24 +18,34 @@ package keeper
 import (
 	"math/big"
 
-	sdkmath "cosmossdk.io/math"
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/evmos/ethermint/x/feemarket/types"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
 // GetParams returns the total set of fee market parameters.
 func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
-	// TODO: update once https://github.com/cosmos/cosmos-sdk/pull/12615 is merged
-	// and released
-	for _, pair := range params.ParamSetPairs() {
-		k.paramSpace.GetIfExists(ctx, pair.Key, pair.Value)
+	store := ctx.KVStore(k.storeKey)
+	bz := store.Get(types.ParamsKey)
+	if len(bz) == 0 {
+		return params
 	}
+
+	k.cdc.MustUnmarshal(bz, &params)
 	return params
 }
 
-// SetParams sets the fee market parameters to the param space.
-func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
-	k.paramSpace.SetParamSet(ctx, &params)
+// SetParams sets the fee market params in a single key
+func (k Keeper) SetParams(ctx sdk.Context, params types.Params) error {
+	store := ctx.KVStore(k.storeKey)
+	bz, err := k.cdc.Marshal(&params)
+	if err != nil {
+		return err
+	}
+
+	store.Set(types.ParamsKey, bz)
+
+	return nil
 }
 
 // ----------------------------------------------------------------------------
@@ -45,15 +55,11 @@ func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
 
 // GetBaseFeeEnabled returns true if base fee is enabled
 func (k Keeper) GetBaseFeeEnabled(ctx sdk.Context) bool {
-	var noBaseFee bool
-	var enableHeight int64
-	k.paramSpace.GetIfExists(ctx, types.ParamStoreKeyNoBaseFee, &noBaseFee)
-	k.paramSpace.GetIfExists(ctx, types.ParamStoreKeyEnableHeight, &enableHeight)
-	return !noBaseFee && ctx.BlockHeight() >= enableHeight
+	params := k.GetParams(ctx)
+	return !params.NoBaseFee && ctx.BlockHeight() >= params.EnableHeight
 }
 
-// GetBaseFee get's the base fee from the paramSpace
-// return nil if base fee is not enabled
+// GetBaseFee gets the base fee from the store
 func (k Keeper) GetBaseFee(ctx sdk.Context) *big.Int {
 	params := k.GetParams(ctx)
 	if params.NoBaseFee {
@@ -65,11 +71,15 @@ func (k Keeper) GetBaseFee(ctx sdk.Context) *big.Int {
 		// try v1 format
 		return k.GetBaseFeeV1(ctx)
 	}
-
 	return baseFee
 }
 
-// SetBaseFee set's the base fee in the paramSpace
+// SetBaseFee set's the base fee in the store
 func (k Keeper) SetBaseFee(ctx sdk.Context, baseFee *big.Int) {
-	k.paramSpace.Set(ctx, types.ParamStoreKeyBaseFee, sdkmath.NewIntFromBigInt(baseFee))
+	params := k.GetParams(ctx)
+	params.BaseFee = sdk.NewIntFromBigInt(baseFee)
+	err := k.SetParams(ctx, params)
+	if err != nil {
+		return
+	}
 }