refactor(evm): Remove dead code and document non-EVM ante handler (#1914

) * test(evm): grpc_query full coverage * chore: changelog update * chore: refactored eth util methods * fix: removed hardcoded gas value in grpc_query test * Squashed commit of the following: commit b5687130ff5f3d020a3b14d219fec3a816579c30 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 20:57:44 2024 -0500 chore: run tidy commit 1f1f938 Merge: 3e3cc83 bbcc6f8 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 19:16:30 2024 -0500 Merge branch 'main' into ud/fix-race-condition commit 3e3cc83 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 19:15:40 2024 -0500 chore: changelog commit 3876ccb Author: Unique-Divine <[email protected]> Date: Wed Jun 5 19:04:00 2024 -0500 refactor: more consistent test names commit aaa0a19 Author: Unique-Divine <[email protected]> Date: Wed Jun 5 18:53:09 2024 -0500 test(oracle): Fix missing tear down step for oracle integration test commit 8c3c35e Author: Unique-Divine <[email protected]> Date: Wed Jun 5 17:55:56 2024 -0500 chore: add test comands to justfile commit 4916282 Merge: 64ed0a2 e7e708d Author: Unique-Divine <[email protected]> Date: Fri May 31 09:35:33 2024 -0500 Merge branch 'main' into ud/fix-race-condition commit 64ed0a2 Author: Unique-Divine <[email protected]> Date: Fri May 31 01:44:55 2024 -0500 fix(gosdk): tests parallel race condition * refactor: ante handler and evm cleanup * refactor: remove dead code * refactor(evm): Remove dead code and document non-EVM ante handler * chore: add issue number for TODO comment * chore: another issue ticket
NibiruChain · Jun 8, 2024 · f88a395 · f88a395
1 parent 359e310
commit f88a395
Show file tree

Hide file tree

Showing 10 changed files with 152 additions and 178 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -70,6 +70,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#1907](https://github.com/NibiruChain/nibiru/pull/1907) - test(evm): grpc_query full coverage
 - [#1909](https://github.com/NibiruChain/nibiru/pull/1909) - chore(evm): set is_london true by default and removed from config
 - [#1911](https://github.com/NibiruChain/nibiru/pull/1911) - chore(evm): simplified config by removing old eth forks
+- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler 
 
 #### Dapp modules: perp, spot, oracle, etc
 

diff --git a/app/ante.go b/app/ante.go
@@ -31,66 +31,21 @@ func NewAnteHandler(
 
 		var anteHandler sdk.AnteHandler
 		hasExt, typeUrl := TxHasExtensions(tx)
-		// TODO: handle ethereum txs
 		if hasExt && typeUrl != "" {
 			anteHandler = AnteHandlerExtendedTx(typeUrl, keepers, opts, ctx)
 			return anteHandler(ctx, tx, sim)
 		}
 
 		switch tx.(type) {
 		case sdk.Tx:
-			anteHandler = AnteHandlerStandardTx(opts)
+			anteHandler = NewAnteHandlerNonEVM(keepers, opts)
 		default:
 			return ctx, fmt.Errorf("invalid tx type (%T) in AnteHandler", tx)
 		}
 		return anteHandler(ctx, tx, sim)
 	}
 }
 
-func AnteHandlerStandardTx(opts ante.AnteHandlerOptions) sdk.AnteHandler {
-	anteDecorators := []sdk.AnteDecorator{
-		AnteDecoratorPreventEtheruemTxMsgs{}, // reject MsgEthereumTxs
-		authante.NewSetUpContextDecorator(),
-		wasmkeeper.NewLimitSimulationGasDecorator(opts.WasmConfig.SimulationGasLimit),
-		wasmkeeper.NewCountTXDecorator(opts.TxCounterStoreKey),
-		authante.NewExtensionOptionsDecorator(opts.ExtensionOptionChecker),
-		authante.NewValidateBasicDecorator(),
-		authante.NewTxTimeoutHeightDecorator(),
-		authante.NewValidateMemoDecorator(opts.AccountKeeper),
-		ante.NewPostPriceFixedPriceDecorator(),
-		ante.AnteDecoratorStakingCommission{},
-		authante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
-		// Replace fee ante from cosmos auth with a custom one.
-		authante.NewDeductFeeDecorator(
-			opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, opts.TxFeeChecker),
-		devgasante.NewDevGasPayoutDecorator(
-			opts.DevGasBankKeeper, opts.DevGasKeeper),
-		// NOTE: SetPubKeyDecorator must be called before all signature verification decorators
-		authante.NewSetPubKeyDecorator(opts.AccountKeeper),
-		authante.NewValidateSigCountDecorator(opts.AccountKeeper),
-		authante.NewSigGasConsumeDecorator(opts.AccountKeeper, opts.SigGasConsumer),
-		authante.NewSigVerificationDecorator(opts.AccountKeeper, opts.SignModeHandler),
-		authante.NewIncrementSequenceDecorator(opts.AccountKeeper),
-		ibcante.NewRedundantRelayDecorator(opts.IBCKeeper),
-	}
-
-	return sdk.ChainAnteDecorators(anteDecorators...)
-}
-
-func TxHasExtensions(tx sdk.Tx) (hasExt bool, typeUrl string) {
-	extensionTx, ok := tx.(authante.HasExtensionOptionsTx)
-	if !ok {
-		return false, ""
-	}
-
-	extOpts := extensionTx.GetExtensionOptions()
-	if len(extOpts) == 0 {
-		return false, ""
-	}
-
-	return true, extOpts[0].GetTypeUrl()
-}
-
 func AnteHandlerExtendedTx(
 	typeUrl string,
 	keepers AppKeepers,
@@ -123,31 +78,46 @@ func NewAnteHandlerNonEVM(
 		authante.NewSetUpContextDecorator(),
 		wasmkeeper.NewLimitSimulationGasDecorator(opts.WasmConfig.SimulationGasLimit),
 		wasmkeeper.NewCountTXDecorator(opts.TxCounterStoreKey),
-		// TODO: UD
-		// cosmosante.NewAuthzLimiterDecorator( // disable the Msg types that cannot be included on an authz.MsgExec msgs field
-		// 	sdk.MsgTypeURL(&evm.MsgEthereumTx{}),
-		// 	sdk.MsgTypeURL(&sdkvesting.MsgCreateVestingAccount{}),
-		// ),
+		// TODO: bug(security): Authz is unsafe. Let's include a guard to make
+		// things safer.
+		// ticket: https://github.com/NibiruChain/nibiru/issues/1915
 		authante.NewExtensionOptionsDecorator(opts.ExtensionOptionChecker),
 		authante.NewValidateBasicDecorator(),
 		authante.NewTxTimeoutHeightDecorator(),
 		authante.NewValidateMemoDecorator(opts.AccountKeeper),
-		// TODO: UD
-		// cosmosante.NewMinGasPriceDecorator(options.FeeMarketKeeper, options.EvmKeeper),
-		ante.NewPostPriceFixedPriceDecorator(),
+		ante.AnteDecoratorEnsureSinglePostPriceMessage{},
 		ante.AnteDecoratorStakingCommission{},
+		// ----------- Ante Handlers: Gas
 		authante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
+		// TODO: spike(security): Does minimum gas price of 0 pose a risk?
+		// ticket: https://github.com/NibiruChain/nibiru/issues/1916
 		authante.NewDeductFeeDecorator(
 			opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, opts.TxFeeChecker),
+		// ----------- Ante Handlers:  devgas
 		devgasante.NewDevGasPayoutDecorator(
 			opts.DevGasBankKeeper, opts.DevGasKeeper),
+		// ----------- Ante Handlers:  Keys and signatures
 		// NOTE: SetPubKeyDecorator must be called before all signature verification decorators
 		authante.NewSetPubKeyDecorator(opts.AccountKeeper),
 		authante.NewValidateSigCountDecorator(opts.AccountKeeper),
 		authante.NewSigGasConsumeDecorator(opts.AccountKeeper, opts.SigGasConsumer),
 		authante.NewSigVerificationDecorator(opts.AccountKeeper, opts.SignModeHandler),
 		authante.NewIncrementSequenceDecorator(opts.AccountKeeper),
 		ibcante.NewRedundantRelayDecorator(opts.IBCKeeper),
-		NewGasWantedDecorator(k),
+		AnteDecoratorGasWanted{},
 	)
 }
+
+func TxHasExtensions(tx sdk.Tx) (hasExt bool, typeUrl string) {
+	extensionTx, ok := tx.(authante.HasExtensionOptionsTx)
+	if !ok {
+		return false, ""
+	}
+
+	extOpts := extensionTx.GetExtensionOptions()
+	if len(extOpts) == 0 {
+		return false, ""
+	}
+
+	return true, extOpts[0].GetTypeUrl()
+}
diff --git a/app/ante/authz_guard.go b/app/ante/authz_guard.go
@@ -0,0 +1,9 @@
+package ante
+
+// TODO: https://github.com/NibiruChain/nibiru/issues/1915
+// feat(ante): Add an authz guard to disable authz Ethereum txs and provide
+// additional security around the default functionality exposed by the module.
+//
+// Implemenetation Notes
+// UD-NOTE - IsAuthzMessage fn. Use authz import with module name
+// UD-NOTE - Define set of disabled txMsgs
diff --git a/app/ante/fixed_gas.go b/app/ante/fixed_gas.go
@@ -9,17 +9,14 @@ import (
 
 const OracleMessageGas = 500
 
-var _ sdk.AnteDecorator = EnsureSinglePostPriceMessageDecorator{}
+var _ sdk.AnteDecorator = AnteDecoratorEnsureSinglePostPriceMessage{}
 
-// EnsureSinglePostPriceMessageDecorator ensures that there is only one oracle vote message in the transaction
-// and sets the gas meter to a fixed value.
-type EnsureSinglePostPriceMessageDecorator struct{}
+// AnteDecoratorEnsureSinglePostPriceMessage ensures that there is only one
+// oracle vote message in the transaction and sets the gas meter to a fixed
+// value.
+type AnteDecoratorEnsureSinglePostPriceMessage struct{}
 
-func NewPostPriceFixedPriceDecorator() EnsureSinglePostPriceMessageDecorator {
-	return EnsureSinglePostPriceMessageDecorator{}
-}
-
-func (gd EnsureSinglePostPriceMessageDecorator) AnteHandle(
+func (gd AnteDecoratorEnsureSinglePostPriceMessage) AnteHandle(
 	ctx sdk.Context,
 	tx sdk.Tx,
 	simulate bool,

diff --git a/app/ante/testutil_test.go b/app/ante/testutil_test.go
@@ -66,7 +66,7 @@ func (suite *AnteTestSuite) SetupTest() {
 		ante.NewValidateBasicDecorator(),
 		ante.NewTxTimeoutHeightDecorator(),
 		ante.NewValidateMemoDecorator(suite.app.AccountKeeper),
-		nibiruante.NewPostPriceFixedPriceDecorator(),
+		nibiruante.AnteDecoratorEnsureSinglePostPriceMessage{},
 		nibiruante.AnteDecoratorStakingCommission{},
 		ante.NewConsumeGasForTxSizeDecorator(suite.app.AccountKeeper),
 		ante.NewDeductFeeDecorator(suite.app.AccountKeeper, suite.app.BankKeeper, suite.app.FeeGrantKeeper, nil), // Replace fee ante from cosmos auth with a custom one.

diff --git a/app/evmante_fee_market.go b/app/evmante_fee_market.go
@@ -9,23 +9,11 @@ import (
 	"github.com/NibiruChain/nibiru/eth"
 )
 
-// GasWantedDecorator keeps track of the gasWanted amount on the current block in transient store
-// for BaseFee calculation.
-// NOTE: This decorator does not perform any validation
-type GasWantedDecorator struct {
-	AppKeepers
-}
-
-// NewGasWantedDecorator creates a new NewGasWantedDecorator
-func NewGasWantedDecorator(
-	k AppKeepers,
-) GasWantedDecorator {
-	return GasWantedDecorator{
-		AppKeepers: k,
-	}
-}
+// AnteDecoratorGasWanted keeps track of the gasWanted amount on the current block in
+// transient store for BaseFee calculation.
+type AnteDecoratorGasWanted struct{}
 
-func (gwd GasWantedDecorator) AnteHandle(
+func (gwd AnteDecoratorGasWanted) AnteHandle(
 	ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler,
 ) (newCtx sdk.Context, err error) {
 	feeTx, ok := tx.(sdk.FeeTx)

diff --git a/app/evmante_handler.go b/app/evmante_handler.go
@@ -22,7 +22,7 @@ func NewAnteHandlerEVM(
 		NewCanTransferDecorator(k),
 		NewAnteDecEthGasConsume(k, options.MaxTxGasWanted),
 		NewAnteDecEthIncrementSenderSequence(k),
-		NewGasWantedDecorator(k),
+		AnteDecoratorGasWanted{},
 		// emit eth tx hash and index at the very last ante handler.
 		NewEthEmitEventDecorator(k),
 	)

diff --git a/app/evmante_test.go b/app/evmante_test.go
@@ -189,3 +189,6 @@ func (s *TestSuite) TestAnteDecEthGasConsume() {
 		})
 	}
 }
+
+// func (s *TestSuite) TestAnteDecoratorVerifyEthAcc_CheckTx() {
+// }
diff --git a/x/common/testutil/cli/network.go b/x/common/testutil/cli/network.go
@@ -59,92 +59,90 @@ var lock = new(sync.Mutex)
 // creates an ABCI Application to provide to Tendermint.
 type AppConstructor = func(val Validator) servertypes.Application
 
-type (
-	// Network defines a in-process testing network. It is primarily intended
-	// for client and integration testing. The Network struct can spawn any
-	// number of validators, each with its own RPC and API clients.
-	//
-	// ### Constraints
-	//
-	// 1. Only the first validator will have a functional RPC and API
-	//    server/client.
-	// 2. Due to constraints in Tendermint's JSON-RPC implementation, only one
-	//    test network can run at a time. For this reason, it's essential to
-	//    invoke `Network.Cleanup` after testing to allow other tests to create
-	//    networks.
-	Network struct {
-		BaseDir    string
-		Config     Config
-		Validators []*Validator
-		Logger     Logger
-	}
+// Network defines a in-process testing network. It is primarily intended
+// for client and integration testing. The Network struct can spawn any
+// number of validators, each with its own RPC and API clients.
+//
+// ### Constraints
+//
+//  1. Only the first validator will have a functional RPC and API
+//     server/client.
+//  2. Due to constraints in Tendermint's JSON-RPC implementation, only one
+//     test network can run at a time. For this reason, it's essential to
+//     invoke `Network.Cleanup` after testing to allow other tests to create
+//     networks.
+type Network struct {
+	BaseDir    string
+	Config     Config
+	Validators []*Validator
+	Logger     Logger
+}
 
-	// Validator defines an in-process Tendermint validator node. Through this
-	// object, a client can make RPC and API calls and interact with any client
-	// command or handler.
-	Validator struct {
-		AppConfig *serverconfig.Config
-		ClientCtx client.Context
-		Ctx       *server.Context
-		// Dir is the root directory of the validator node data and config. Passed to the Tendermint config.
-		Dir string
-
-		// NodeID is a unique ID for the validator generated when the
-		// 'cli.Network' is started.
-		NodeID string
-		PubKey cryptotypes.PubKey
-
-		// Moniker is a human-readable name that identifies a validator. A
-		// moniker is optional and may be empty.
-		Moniker string
-
-		// APIAddress is the endpoint that the validator API server binds to.
-		// Only the first validator of a 'cli.Network' exposes the full API.
-		APIAddress string
-
-		// RPCAddress is the endpoint that the RPC server binds to. Only the
-		// first validator of a 'cli.Network' exposes the full API.
-		RPCAddress string
-
-		// P2PAddress is the endpoint that the RPC server binds to. The P2P
-		// server handles Tendermint peer-to-peer (P2P) networking and is
-		// critical for blockchain replication and consensus. It allows nodes
-		// to gossip blocks, transactions, and consensus messages. Only the
-		// first validator of a 'cli.Network' exposes the full API.
-		P2PAddress string
-
-		// Address - account address
-		Address sdk.AccAddress
-
-		// EthAddress - Ethereum address
-		EthAddress common.Address
-
-		// ValAddress - validator operator (valoper) address
-		ValAddress sdk.ValAddress
-
-		// RPCClient wraps most important rpc calls a client would make to
-		// listen for events, test if it also implements events.EventSwitch.
-		//
-		// RPCClient implementations in "github.com/cometbft/cometbft/rpc" v0.37.2:
-		// - rpc.HTTP
-		// - rpc.Local
-		RPCClient tmclient.Client
-
-		JSONRPCClient *ethclient.Client
-
-		tmNode *node.Node
-
-		// API exposes the app's REST and gRPC interfaces, allowing clients to
-		// read from state and broadcast txs. The API server connects to the
-		// underlying ABCI application.
-		api            *serverapi.Server
-		grpc           *grpc.Server
-		grpcWeb        *http.Server
-		secretMnemonic string
-		jsonrpc        *http.Server
-		jsonrpcDone    chan struct{}
-	}
-)
+// Validator defines an in-process Tendermint validator node. Through this
+// object, a client can make RPC and API calls and interact with any client
+// command or handler.
+type Validator struct {
+	AppConfig *serverconfig.Config
+	ClientCtx client.Context
+	Ctx       *server.Context
+	// Dir is the root directory of the validator node data and config. Passed to the Tendermint config.
+	Dir string
+
+	// NodeID is a unique ID for the validator generated when the
+	// 'cli.Network' is started.
+	NodeID string
+	PubKey cryptotypes.PubKey
+
+	// Moniker is a human-readable name that identifies a validator. A
+	// moniker is optional and may be empty.
+	Moniker string
+
+	// APIAddress is the endpoint that the validator API server binds to.
+	// Only the first validator of a 'cli.Network' exposes the full API.
+	APIAddress string
+
+	// RPCAddress is the endpoint that the RPC server binds to. Only the
+	// first validator of a 'cli.Network' exposes the full API.
+	RPCAddress string
+
+	// P2PAddress is the endpoint that the RPC server binds to. The P2P
+	// server handles Tendermint peer-to-peer (P2P) networking and is
+	// critical for blockchain replication and consensus. It allows nodes
+	// to gossip blocks, transactions, and consensus messages. Only the
+	// first validator of a 'cli.Network' exposes the full API.
+	P2PAddress string
+
+	// Address - account address
+	Address sdk.AccAddress
+
+	// EthAddress - Ethereum address
+	EthAddress common.Address
+
+	// ValAddress - validator operator (valoper) address
+	ValAddress sdk.ValAddress
+
+	// RPCClient wraps most important rpc calls a client would make to
+	// listen for events, test if it also implements events.EventSwitch.
+	//
+	// RPCClient implementations in "github.com/cometbft/cometbft/rpc" v0.37.2:
+	// - rpc.HTTP
+	// - rpc.Local
+	RPCClient tmclient.Client
+
+	JSONRPCClient *ethclient.Client
+
+	tmNode *node.Node
+
+	// API exposes the app's REST and gRPC interfaces, allowing clients to
+	// read from state and broadcast txs. The API server connects to the
+	// underlying ABCI application.
+	api            *serverapi.Server
+	grpc           *grpc.Server
+	grpcWeb        *http.Server
+	secretMnemonic string
+	jsonrpc        *http.Server
+	jsonrpcDone    chan struct{}
+}
 
 // NewAppConstructor returns a new simapp AppConstructor
 func NewAppConstructor(encodingCfg app.EncodingConfig, chainID string) AppConstructor {
-Original file line number
+Diff line change
@@ Expand Up / @@ -189,3 +189,6 @@ func (s *TestSuite) TestAnteDecEthGasConsume() { @@
     		})
     	}
     }
+    // func (s *TestSuite) TestAnteDecoratorVerifyEthAcc_CheckTx() {
+    // }