chore(evm): Deprecate x/params usage in x/evm (#1472)

* imp(evm): Migrate from old Cosmos SDK params module to new way of keeping params in module Keeper

* Updated changelog

* Apply changes from code review

* (impv): Added Shanghai and Cancun blocks to current types and latest migration

* (tests): Update unit tests to include Shanghai and Cancun blocks

* (fix) - ran golangci-lint on the entire project

* (fix) - remove deprecated params method

* (impv): added marshaling of booleans per individual param key

* (impv): added individual param getting and setting

* (impv): replaced getters with individual param

* (impv): added amino codec for MsgEthereumTx

* Added changes suggested in code review

* (fix): updated the migration files for v4

* (fix): fixed unit tests panic for incorrect interface

* (fix): updated module msg handler

* (fix): rename to original params getter method

* (refactor): registered implementation for the new msg

* (refactor): added correct amino codec for MsgUpdateParams and removed for MsgEthTx

* Applied changes from code review

* (fix): removed unnecessary duplicate

* (fix): removed params_legacy from the types and moved logic to migration

* (fix): Added v4 mocks to the migrations_test

* (fix): undo all the non related work regarding the Cancun and Shanghai blocks

* (fix): reverted linting the entire project - will make a separate PR for it

* Applied changes from review

* Applied changes from code review

* (fix): removed comments

* (fix): Ran formatter and fixed linting issues on unsed functions

* (fix): Linting issues resolved

* (fix): refactor migrations and added default EIPs

* (fix): Combined into one call

* (fix): Added more straightforward way to handle migration

* (fix): corrected migration test

* Applied changes from code review

* (fix): Linter fix

* (fix): Linter

* (fix): Lint proto files

* Apply suggestions from code review

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

* (fix): Added new block to migration

* (fix): Added additional comments and formatted proto files

* (fix): Added name to unit test cases

* (fix): removed unused import

* Apply changes from code review

* (fix): typo

* (fix): remove HTTP endpoint exposure

* Apply suggestions from code review

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

* applied changes from code review

* fix: extra line added in merge removed

* fix: applied changes from code review

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
 
+* (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)
 * (evm) [\#1272](https://github.com/evmos/ethermint/pull/1272) Implement modular interface for the EVM.

app/ante/interfaces.go

@@ -50,10 +50,10 @@ type EVMKeeper interface {
 	ResetTransientGasUsed(ctx sdk.Context)
 	GetTxIndexTransient(ctx sdk.Context) uint64
 	GetChainConfig(ctx sdk.Context) evmtypes.ChainConfig
+	GetAllowUnprotectedTxs(ctx sdk.Context) bool
 	GetEVMDenom(ctx sdk.Context) string
-	GetEnableCreate(ctx sdk.Context) bool
 	GetEnableCall(ctx sdk.Context) bool
-	GetAllowUnprotectedTxs(ctx sdk.Context) bool
+	GetEnableCreate(ctx sdk.Context) bool
 }
 
 type protoTxProvider interface {

app/app.go

@@ -416,8 +416,9 @@ func NewEthermintApp(
 		appCodec, app.GetSubspace(feemarkettypes.ModuleName), keys[feemarkettypes.StoreKey], tkeys[feemarkettypes.TransientKey],
 	)
 
+	// Set authority to x/gov module account to only expect the module account to update params
 	app.EvmKeeper = evmkeeper.NewKeeper(
-		appCodec, keys[evmtypes.StoreKey], tkeys[evmtypes.TransientKey], app.GetSubspace(evmtypes.ModuleName),
+		appCodec, keys[evmtypes.StoreKey], tkeys[evmtypes.TransientKey], authtypes.NewModuleAddress(govtypes.ModuleName),
 		app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.FeeMarketKeeper,
 		nil, geth.NewEVM, tracer,
 	)
@@ -505,7 +506,7 @@ func NewEthermintApp(
 		ibc.NewAppModule(app.IBCKeeper),
 		transferModule,
 		// Ethermint app modules
-		evm.NewAppModule(app.EvmKeeper, app.AccountKeeper),
+		evm.NewAppModule(app.EvmKeeper, app.AccountKeeper, app.GetSubspace(evmtypes.ModuleName)),
 		feemarket.NewAppModule(app.FeeMarketKeeper),
 	)
 

proto/ethermint/evm/v1/evm.proto

@@ -15,14 +15,24 @@ message Params {
   // enable_call toggles state transitions that use the vm.Call function
   bool enable_call = 3 [(gogoproto.moretags) = "yaml:\"enable_call\""];
   // extra_eips defines the additional EIPs for the vm.Config
-  repeated int64 extra_eips = 4 [(gogoproto.customname) = "ExtraEIPs", (gogoproto.moretags) = "yaml:\"extra_eips\""];
+  ExtraEIPs extra_eips = 4 [
+    (gogoproto.customname) = "ExtraEIPs",
+    (gogoproto.moretags) = "yaml:\"extra_eips\"",
+    (gogoproto.nullable) = false
+  ];
   // chain_config defines the EVM chain configuration parameters
   ChainConfig chain_config = 5 [(gogoproto.moretags) = "yaml:\"chain_config\"", (gogoproto.nullable) = false];
   // allow_unprotected_txs defines if replay-protected (i.e non EIP155
   // signed) transactions can be executed on the state machine.
   bool allow_unprotected_txs = 6;
 }
 
+// ExtraEIPs represents extra EIPs for the vm.Config
+message ExtraEIPs {
+  // eips defines the additional EIPs for the vm.Config
+  repeated int64 eips = 1 [(gogoproto.customname) = "EIPs", (gogoproto.moretags) = "yaml:\"eips\""];
+}
+
 // ChainConfig defines the Ethereum ChainConfig parameters using *sdk.Int values
 // instead of *big.Int.
 message ChainConfig {
@@ -39,7 +49,7 @@ message ChainConfig {
   ];
   // dao_fork_support defines whether the nodes supports or opposes the DAO hard-fork
   bool dao_fork_support = 3
-  [(gogoproto.customname) = "DAOForkSupport", (gogoproto.moretags) = "yaml:\"dao_fork_support\""];
+      [(gogoproto.customname) = "DAOForkSupport", (gogoproto.moretags) = "yaml:\"dao_fork_support\""];
   // eip150_block: EIP150 implements the Gas price changes
   // (https://github.com/ethereum/EIPs/issues/150) EIP150 HF block (nil no fork)
   string eip150_block = 4 [
@@ -127,7 +137,6 @@ message ChainConfig {
     (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
     (gogoproto.moretags) = "yaml:\"cancun_block\""
   ];
-
 }
 
 // State represents a single Storage key value pair item.

proto/ethermint/evm/v1/tx.proto

@@ -1,6 +1,7 @@
 syntax = "proto3";
 package ethermint.evm.v1;
 
+import "cosmos/msg/v1/msg.proto";
 import "cosmos_proto/cosmos.proto";
 import "ethermint/evm/v1/evm.proto";
 import "gogoproto/gogo.proto";
@@ -15,6 +16,9 @@ service Msg {
   rpc EthereumTx(MsgEthereumTx) returns (MsgEthereumTxResponse) {
     option (google.api.http).post = "/ethermint/evm/v1/ethereum_tx";
   };
+  // UpdateParams defined a governance operation for updating the x/evm module parameters.
+  // The authority is hard-coded to the Cosmos SDK x/gov module account
+  rpc UpdateParams(MsgUpdateParams) returns (MsgUpdateParamsResponse);
 }
 
 // MsgEthereumTx encapsulates an Ethereum transaction as an SDK message.
@@ -158,3 +162,19 @@ message MsgEthereumTxResponse {
   // gas_used specifies how much gas was consumed by the transaction
   uint64 gas_used = 5;
 }
+
+// MsgUpdateParams defines a Msg for updating the x/evm 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/evm 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 {}

rpc/backend/backend_suite_test.go

@@ -7,8 +7,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/evmos/ethermint/crypto/ethsecp256k1"
-
 	dbm "github.com/tendermint/tm-db"
 
 	"github.com/cosmos/cosmos-sdk/client"
@@ -21,6 +19,7 @@ import (
 	tmrpctypes "github.com/tendermint/tendermint/rpc/core/types"
 
 	"github.com/evmos/ethermint/app"
+	"github.com/evmos/ethermint/crypto/ethsecp256k1"
 	"github.com/evmos/ethermint/crypto/hd"
 	"github.com/evmos/ethermint/encoding"
 	"github.com/evmos/ethermint/indexer"

x/evm/genesis.go

@@ -39,7 +39,10 @@ func InitGenesis(
 ) []abci.ValidatorUpdate {
 	k.WithChainID(ctx)
 
-	k.SetParams(ctx, data.Params)
+	err := k.SetParams(ctx, data.Params)
+	if err != nil {
+		panic(fmt.Errorf("error setting params %s", err))
+	}
 
 	// ensure evm module account is set
 	if addr := accountKeeper.GetModuleAddress(types.ModuleName); addr == nil {

x/evm/handler.go

@@ -30,10 +30,11 @@ func NewHandler(server types.MsgServer) sdk.Handler {
 
 		switch msg := msg.(type) {
 		case *types.MsgEthereumTx:
-			// execute state transition
 			res, err := server.EthereumTx(sdk.WrapSDKContext(ctx), msg)
 			return sdk.WrapServiceResult(ctx, res, err)
-
+		case *types.MsgUpdateParams:
+			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/evm/keeper/grpc_query_test.go

@@ -540,7 +540,11 @@ func (suite *KeeperTestSuite) TestEstimateGas() {
 			"enough balance",
 			func() {
 				args = types.TransactionArgs{To: &common.Address{}, From: &suite.address, Value: (*hexutil.Big)(big.NewInt(100))}
-			}, false, 0, false},
+			},
+			false,
+			0,
+			false,
+		},
 		// should success, because gas limit lower than 21000 is ignored
 		{
 			"gas exceed allowance",
@@ -1261,9 +1265,7 @@ func (suite *KeeperTestSuite) TestQueryBaseFee() {
 }
 
 func (suite *KeeperTestSuite) TestEthCall() {
-	var (
-		req *types.EthCallRequest
-	)
+	var req *types.EthCallRequest
 
 	address := tests.GenerateAddress()
 	suite.Require().Equal(uint64(0), suite.app.EvmKeeper.GetNonce(suite.ctx, address))
@@ -1334,7 +1336,6 @@ func (suite *KeeperTestSuite) TestEthCall() {
 			}
 		})
 	}
-
 }
 
 func (suite *KeeperTestSuite) TestEmptyRequest() {

x/evm/keeper/keeper.go

@@ -23,7 +23,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	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/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
@@ -51,8 +50,8 @@ type Keeper struct {
 	// key to access the transient store, which is reset on every block during Commit
 	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
 	// access to account state
 	accountKeeper types.AccountKeeper
 	// update balance and accounting operations with coins
@@ -82,7 +81,7 @@ type Keeper struct {
 func NewKeeper(
 	cdc codec.BinaryCodec,
 	storeKey, transientKey storetypes.StoreKey,
-	paramSpace paramtypes.Subspace,
+	authority sdk.AccAddress,
 	ak types.AccountKeeper,
 	bankKeeper types.BankKeeper,
 	sk types.StakingKeeper,
@@ -96,15 +95,15 @@ func NewKeeper(
 		panic("the EVM module account has not been set")
 	}
 
-	// set KeyTable if it has not already been set
-	if !paramSpace.HasKeyTable() {
-		paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
+	// ensure the authority account is correct
+	if err := sdk.VerifyAddressFormat(authority); err != nil {
+		panic(err)
 	}
 
 	// NOTE: we pass in the parameter space to the CommitStateDB in order to use custom denominations for the EVM operations
 	return &Keeper{
 		cdc:               cdc,
-		paramSpace:        paramSpace,
+		authority:         authority,
 		accountKeeper:     ak,
 		bankKeeper:        bankKeeper,
 		stakingKeeper:     sk,
@@ -156,6 +155,11 @@ func (k Keeper) EmitBlockBloomEvent(ctx sdk.Context, bloom ethtypes.Bloom) {
 	)
 }
 
+// GetAuthority returns the x/evm module authority address
+func (k Keeper) GetAuthority() sdk.AccAddress {
+	return k.authority
+}
+
 // GetBlockBloomTransient returns bloom bytes for the current block height
 func (k Keeper) GetBlockBloomTransient(ctx sdk.Context) *big.Int {
 	store := prefix.NewStore(ctx.TransientStore(k.transientKey), types.KeyPrefixTransientBloom)
@@ -312,8 +316,7 @@ func (k *Keeper) GetNonce(ctx sdk.Context, addr common.Address) uint64 {
 // GetBalance load account's balance of gas token
 func (k *Keeper) GetBalance(ctx sdk.Context, addr common.Address) *big.Int {
 	cosmosAddr := sdk.AccAddress(addr.Bytes())
-	evmDenom := ""
-	k.paramSpace.GetIfExists(ctx, types.ParamStoreKeyEVMDenom, &evmDenom)
+	evmDenom := k.GetEVMDenom(ctx)
 	// if node is pruned, params is empty. Return invalid value
 	if evmDenom == "" {
 		return big.NewInt(-1)

x/evm/keeper/keeper_test.go

@@ -489,10 +489,8 @@ func (suite *KeeperTestSuite) TestGetAccountStorage() {
 				i++
 				return false
 			})
-
 		})
 	}
-
 }
 
 func (suite *KeeperTestSuite) TestGetAccountOrEmpty() {
@@ -529,7 +527,6 @@ func (suite *KeeperTestSuite) TestGetAccountOrEmpty() {
 			} else {
 				suite.Require().NotEqual(empty, res)
 			}
-
 		})
 	}
 }

x/evm/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/evm/migrations/v4"
+	"github.com/evmos/ethermint/x/evm/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/evm/keeper/migrations_test.go

@@ -0,0 +1,42 @@
+package keeper_test
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	evmkeeper "github.com/evmos/ethermint/x/evm/keeper"
+	v4types "github.com/evmos/ethermint/x/evm/migrations/v4/types"
+	"github.com/evmos/ethermint/x/evm/types"
+)
+
+type mockSubspace struct {
+	ps v4types.V4Params
+}
+
+func newMockSubspace(ps v4types.V4Params) mockSubspace {
+	return mockSubspace{ps: ps}
+}
+
+func (ms mockSubspace) GetParamSetIfExists(_ sdk.Context, ps types.LegacyParams) {
+	*ps.(*v4types.V4Params) = ms.ps
+}
+
+func (suite *KeeperTestSuite) TestMigrations() {
+	legacySubspace := newMockSubspace(v4types.DefaultParams())
+	migrator := evmkeeper.NewMigrator(*suite.app.EvmKeeper, 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/evm/keeper/msg_server.go

@@ -21,6 +21,8 @@ import (
 	"fmt"
 	"strconv"
 
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+
 	tmbytes "github.com/tendermint/tendermint/libs/bytes"
 	tmtypes "github.com/tendermint/tendermint/types"
 
@@ -140,3 +142,20 @@ func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*t
 
 	return response, nil
 }
+
+// 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
+}