diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml new file mode 100644 index 0000000000..9570d52ddc --- /dev/null +++ b/.github/workflows/semgrep.yml @@ -0,0 +1,41 @@ +name: Semgrep +on: + # Scan changed files in PRs, block on new issues only (existing issues ignored) + pull_request: {} + push: + branches: + - main + paths: + - .github/workflows/semgrep.yml + schedule: + - cron: '0 0 * * 0' +jobs: + semgrep: + name: Scan + runs-on: ubuntu-latest + if: (github.actor != 'dependabot[bot]') + steps: + - uses: actions/checkout@v2 + - name: Get Diff + uses: technote-space/get-diff-action@v6.0.1 + with: + PATTERNS: | + **/*.go + **/*.js + **/*.ts + **/*.sol + go.mod + go.sum + - uses: returntocorp/semgrep-action@v1 + with: + publishToken: ${{ secrets.SEMGREP_APP_TOKEN }} + # Upload findings to GitHub Advanced Security Dashboard [step 1/2] + # See also the next step. + generateSarif: "1" + if: "env.GIT_DIFF_FILTERED != ''" + # Upload findings to GitHub Advanced Security Dashboard [step 2/2] + - name: Upload SARIF file for GitHub Advanced Security Dashboard + uses: github/codeql-action/upload-sarif@v1 + with: + sarif_file: semgrep.sarif + if: "env.GIT_DIFF_FILTERED != ''" diff --git a/.semgrepignore b/.semgrepignore new file mode 100644 index 0000000000..cb655af10c --- /dev/null +++ b/.semgrepignore @@ -0,0 +1,29 @@ +# Ignore git items +.gitignore +.git/ +:include .gitignore + +# Common large paths +node_modules/ +build/ +dist/ +vendor/ +.env/ +.venv/ +.tox/ +*.min.js +*.pb.gw.go + +# Common test paths +test/ +tests/ +*_test.go + +# Semgrep rules folder +.semgrep + +# Semgrep-action log folder +.semgrep_logs/ + +# Documentation +client/docs/ \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index a99751bf4a..598ea49616 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (ante) [\#866](https://github.com/tharsis/ethermint/pull/866) `NewAnteHandler` constructor now receives a `HandlerOptions` field. * (evm) [\#849](https://github.com/tharsis/ethermint/pull/849) `PostTxProcessing` hook now takes an Ethereum tx `Receipt` and a `from` `Address` as arguments. +* (ante) [#916](https://github.com/tharsis/ethermint/pull/916) don't check min-gas-price for eth tx if london hardfork enabled and feemarket enabled. ### State Machine Breaking @@ -65,6 +66,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (app) [tharsis#873](https://github.com/tharsis/ethermint/pull/873) Validate code hash in GenesisAccount * (evm) [tharsis#901](https://github.com/tharsis/ethermint/pull/901) Support multiple MsgEthereumTx in single tx. * (config) [tharsis#908](https://github.com/tharsis/ethermint/pull/908) Add api.enable flag for Cosmos SDK Rest server +* (feemarket) [tharsis#919](https://github.com/tharsis/ethermint/pull/919) Initialize baseFee in default genesis state. ### Bug Fixes diff --git a/app/ante/eth.go b/app/ante/eth.go index ae4dce5a57..4ed65a644b 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -221,15 +221,13 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // CanTransferDecorator checks if the sender is allowed to transfer funds according to the EVM block // context rules. type CanTransferDecorator struct { - evmKeeper EVMKeeper - feemarketKeeper evmtypes.FeeMarketKeeper + evmKeeper EVMKeeper } // NewCanTransferDecorator creates a new CanTransferDecorator instance. -func NewCanTransferDecorator(evmKeeper EVMKeeper, fmk evmtypes.FeeMarketKeeper) CanTransferDecorator { +func NewCanTransferDecorator(evmKeeper EVMKeeper) CanTransferDecorator { return CanTransferDecorator{ - evmKeeper: evmKeeper, - feemarketKeeper: fmk, + evmKeeper: evmKeeper, } } @@ -477,45 +475,38 @@ func (esc EthSetupContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul // If fee is high enough or not CheckTx, then call next AnteHandler // CONTRACT: Tx must implement FeeTx to use MempoolFeeDecorator type EthMempoolFeeDecorator struct { - feemarketKeeper evmtypes.FeeMarketKeeper - evmKeeper EVMKeeper + evmKeeper EVMKeeper } -func NewEthMempoolFeeDecorator(ek EVMKeeper, fmk evmtypes.FeeMarketKeeper) EthMempoolFeeDecorator { +func NewEthMempoolFeeDecorator(ek EVMKeeper) EthMempoolFeeDecorator { return EthMempoolFeeDecorator{ - feemarketKeeper: fmk, - evmKeeper: ek, + evmKeeper: ek, } } // AnteHandle ensures that the provided fees meet a minimum threshold for the validator, // if this is a CheckTx. This is only for local mempool purposes, and thus // is only ran on check tx. +// It only do the check if london hardfork not enabled or feemarket not enabled, because in that case feemarket will take over the task. func (mfd EthMempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { if ctx.IsCheckTx() && !simulate { - for _, msg := range tx.GetMsgs() { - ethMsg, ok := msg.(*evmtypes.MsgEthereumTx) - if !ok { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil)) - } - - var feeAmt *big.Int - - params := mfd.evmKeeper.GetParams(ctx) - chainID := mfd.evmKeeper.ChainID() - ethCfg := params.ChainConfig.EthereumConfig(chainID) - evmDenom := params.EvmDenom - baseFee := mfd.evmKeeper.BaseFee(ctx, ethCfg) - if baseFee != nil { - feeAmt = ethMsg.GetEffectiveFee(baseFee) - } else { - feeAmt = ethMsg.GetFee() - } - - glDec := sdk.NewDec(int64(ethMsg.GetGas())) - requiredFee := ctx.MinGasPrices().AmountOf(evmDenom).Mul(glDec) - if sdk.NewDecFromBigInt(feeAmt).LT(requiredFee) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeAmt, requiredFee) + params := mfd.evmKeeper.GetParams(ctx) + ethCfg := params.ChainConfig.EthereumConfig(mfd.evmKeeper.ChainID()) + baseFee := mfd.evmKeeper.BaseFee(ctx, ethCfg) + if baseFee == nil { + for _, msg := range tx.GetMsgs() { + ethMsg, ok := msg.(*evmtypes.MsgEthereumTx) + if !ok { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil)) + } + + evmDenom := params.EvmDenom + feeAmt := ethMsg.GetFee() + glDec := sdk.NewDec(int64(ethMsg.GetGas())) + requiredFee := ctx.MinGasPrices().AmountOf(evmDenom).Mul(glDec) + if sdk.NewDecFromBigInt(feeAmt).LT(requiredFee) { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeAmt, requiredFee) + } } } } diff --git a/app/ante/eth_test.go b/app/ante/eth_test.go index e181e1fc96..80f5455baa 100644 --- a/app/ante/eth_test.go +++ b/app/ante/eth_test.go @@ -293,7 +293,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() { } func (suite AnteTestSuite) TestCanTransferDecorator() { - dec := ante.NewCanTransferDecorator(suite.app.EvmKeeper, suite.app.FeeMarketKeeper) + dec := ante.NewCanTransferDecorator(suite.app.EvmKeeper) addr, privKey := tests.NewAddrKey() diff --git a/app/ante/handler_options.go b/app/ante/handler_options.go index e6b9bdcfb6..68bc55c32e 100644 --- a/app/ante/handler_options.go +++ b/app/ante/handler_options.go @@ -48,13 +48,13 @@ func (options HandlerOptions) Validate() error { func newEthAnteHandler(options HandlerOptions) sdk.AnteHandler { return sdk.ChainAnteDecorators( - NewEthSetUpContextDecorator(options.EvmKeeper), // outermost AnteDecorator. SetUpContext must be called first - NewEthMempoolFeeDecorator(options.EvmKeeper, options.FeeMarketKeeper), // Check eth effective gas price against minimal-gas-prices + NewEthSetUpContextDecorator(options.EvmKeeper), // outermost AnteDecorator. SetUpContext must be called first + NewEthMempoolFeeDecorator(options.EvmKeeper), // Check eth effective gas price against minimal-gas-prices NewEthValidateBasicDecorator(options.EvmKeeper), NewEthSigVerificationDecorator(options.EvmKeeper), NewEthAccountVerificationDecorator(options.AccountKeeper, options.BankKeeper, options.EvmKeeper), NewEthGasConsumeDecorator(options.EvmKeeper), - NewCanTransferDecorator(options.EvmKeeper, options.FeeMarketKeeper), + NewCanTransferDecorator(options.EvmKeeper), NewEthIncrementSenderSequenceDecorator(options.AccountKeeper), // innermost AnteDecorator. ) } diff --git a/rpc/ethereum/backend/backend.go b/rpc/ethereum/backend/backend.go index 0cb68c0560..850737dcb5 100644 --- a/rpc/ethereum/backend/backend.go +++ b/rpc/ethereum/backend/backend.go @@ -10,10 +10,8 @@ import ( "time" "github.com/cosmos/cosmos-sdk/client/flags" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/server" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" @@ -802,23 +800,6 @@ func (e *EVMBackend) SendTransaction(args evmtypes.TransactionArgs) (common.Hash return common.Hash{}, err } - // Assemble transaction from fields - builder, ok := e.clientCtx.TxConfig.NewTxBuilder().(authtx.ExtensionOptionsTxBuilder) - if !ok { - e.logger.Error("clientCtx.TxConfig.NewTxBuilder returns unsupported builder", "error", err.Error()) - } - - option, err := codectypes.NewAnyWithValue(&evmtypes.ExtensionOptionsEthereumTx{}) - if err != nil { - e.logger.Error("codectypes.NewAnyWithValue failed to pack an obvious value", "error", err.Error()) - return common.Hash{}, err - } - - builder.SetExtensionOptions(option) - if err = builder.SetMsgs(msg); err != nil { - e.logger.Error("builder.SetMsgs failed", "error", err.Error()) - } - // Query params to use the EVM denomination res, err := e.queryClient.QueryClient.Params(e.ctx, &evmtypes.QueryParamsRequest{}) if err != nil { @@ -826,19 +807,16 @@ func (e *EVMBackend) SendTransaction(args evmtypes.TransactionArgs) (common.Hash return common.Hash{}, err } - txData, err := evmtypes.UnpackTxData(msg.Data) + // Assemble transaction from fields + tx, err := msg.BuildTx(e.clientCtx.TxConfig.NewTxBuilder(), res.Params.EvmDenom) if err != nil { - e.logger.Error("failed to unpack tx data", "error", err.Error()) + e.logger.Error("build cosmos tx failed", "error", err.Error()) return common.Hash{}, err } - fees := sdk.Coins{sdk.NewCoin(res.Params.EvmDenom, sdk.NewIntFromBigInt(txData.Fee()))} - builder.SetFeeAmount(fees) - builder.SetGasLimit(msg.GetGas()) - // Encode transaction by default Tx encoder txEncoder := e.clientCtx.TxConfig.TxEncoder() - txBytes, err := txEncoder(builder.GetTx()) + txBytes, err := txEncoder(tx) if err != nil { e.logger.Error("failed to encode eth tx using default encoder", "error", err.Error()) return common.Hash{}, err @@ -976,9 +954,9 @@ func (e *EVMBackend) ChainConfig() *params.ChainConfig { } // SuggestGasTipCap returns the suggested tip cap +// always return zero since we don't support tx prioritization yet. func (e *EVMBackend) SuggestGasTipCap() (*big.Int, error) { - out := new(big.Int).SetInt64(e.RPCMinGasPrice()) - return out, nil + return big.NewInt(0), nil } // BaseFee returns the base fee tracked by the Fee Market module. If the base fee is not enabled, diff --git a/rpc/ethereum/namespaces/eth/api.go b/rpc/ethereum/namespaces/eth/api.go index 2cac0699d8..a9f70e8690 100644 --- a/rpc/ethereum/namespaces/eth/api.go +++ b/rpc/ethereum/namespaces/eth/api.go @@ -193,16 +193,22 @@ func (e *PublicAPI) Hashrate() hexutil.Uint64 { // GasPrice returns the current gas price based on Ethermint's gas price oracle. func (e *PublicAPI) GasPrice() (*hexutil.Big, error) { e.logger.Debug("eth_gasPrice") - tipcap, err := e.backend.SuggestGasTipCap() - if err != nil { - return nil, err - } - + var ( + result *big.Int + err error + ) if head := e.backend.CurrentHeader(); head.BaseFee != nil { - tipcap.Add(tipcap, head.BaseFee) + result, err = e.backend.SuggestGasTipCap() + if err != nil { + return nil, err + } + + result = result.Add(result, head.BaseFee) + } else { + result = big.NewInt(e.backend.RPCMinGasPrice()) } - return (*hexutil.Big)(tipcap), nil + return (*hexutil.Big)(result), nil } // MaxPriorityFeePerGas returns a suggestion for a gas tip cap for dynamic fee transactions. diff --git a/x/evm/keeper/keeper_test.go b/x/evm/keeper/keeper_test.go index 4973863a66..e4464d47af 100644 --- a/x/evm/keeper/keeper_test.go +++ b/x/evm/keeper/keeper_test.go @@ -87,13 +87,16 @@ func (suite *KeeperTestSuite) DoSetupTest(t require.TestingT) { suite.consAddress = sdk.ConsAddress(priv.PubKey().Address()) suite.app = app.Setup(checkTx, func(app *app.EthermintApp, genesis simapp.GenesisState) simapp.GenesisState { + feemarketGenesis := feemarkettypes.DefaultGenesisState() if suite.enableFeemarket { - feemarketGenesis := feemarkettypes.DefaultGenesisState() feemarketGenesis.Params.EnableHeight = 1 feemarketGenesis.Params.NoBaseFee = false feemarketGenesis.BaseFee = sdk.NewInt(feemarketGenesis.Params.InitialBaseFee) - genesis[feemarkettypes.ModuleName] = app.AppCodec().MustMarshalJSON(feemarketGenesis) + } else { + feemarketGenesis.Params.NoBaseFee = true + feemarketGenesis.BaseFee = sdk.NewInt(0) } + genesis[feemarkettypes.ModuleName] = app.AppCodec().MustMarshalJSON(feemarketGenesis) if !suite.enableLondonHF { evmGenesis := types.DefaultGenesisState() maxInt := sdk.NewInt(math.MaxInt64) diff --git a/x/evm/keeper/state_transition_test.go b/x/evm/keeper/state_transition_test.go index c2586ba494..37314ccbb5 100644 --- a/x/evm/keeper/state_transition_test.go +++ b/x/evm/keeper/state_transition_test.go @@ -501,7 +501,7 @@ func (suite *KeeperTestSuite) TestEVMConfig() { suite.Require().NoError(err) suite.Require().Equal(types.DefaultParams(), cfg.Params) // london hardfork is enabled by default - suite.Require().Equal(new(big.Int), cfg.BaseFee) + suite.Require().Equal(big.NewInt(0), cfg.BaseFee) suite.Require().Equal(suite.address, cfg.CoinBase) suite.Require().Equal(types.DefaultParams().ChainConfig.EthereumConfig(big.NewInt(9000)), cfg.ChainConfig) } diff --git a/x/evm/keeper/utils.go b/x/evm/keeper/utils.go index 3846e03afe..ed4a2e2f0d 100644 --- a/x/evm/keeper/utils.go +++ b/x/evm/keeper/utils.go @@ -66,6 +66,11 @@ func (k Keeper) DeductTxCostsFromUserBalance( feeAmt = txData.Fee() } + if feeAmt.Sign() == 0 { + // zero fee, no need to deduct + return sdk.NewCoins(), nil + } + fees := sdk.Coins{sdk.NewCoin(denom, sdk.NewIntFromBigInt(feeAmt))} // deduct the full gas cost from the user balance diff --git a/x/evm/types/msg.go b/x/evm/types/msg.go index 469fcd8a9c..7cec50c944 100644 --- a/x/evm/types/msg.go +++ b/x/evm/types/msg.go @@ -330,11 +330,10 @@ func (msg *MsgEthereumTx) BuildTx(b client.TxBuilder, evmDenom string) (signing. if err != nil { return nil, err } - fees := sdk.Coins{ - { - Denom: evmDenom, - Amount: sdk.NewIntFromBigInt(txData.Fee()), - }, + fees := make(sdk.Coins, 0) + feeAmt := sdk.NewIntFromBigInt(txData.Fee()) + if feeAmt.Sign() > 0 { + fees = append(fees, sdk.NewCoin(evmDenom, feeAmt)) } builder.SetExtensionOptions(option) diff --git a/x/feemarket/keeper/grpc_query_test.go b/x/feemarket/keeper/grpc_query_test.go index 8bda0f2442..b97f4d311a 100644 --- a/x/feemarket/keeper/grpc_query_test.go +++ b/x/feemarket/keeper/grpc_query_test.go @@ -2,6 +2,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + ethparams "github.com/ethereum/go-ethereum/params" "github.com/tharsis/ethermint/x/feemarket/types" ) @@ -41,9 +42,10 @@ func (suite *KeeperTestSuite) TestQueryBaseFee() { expPass bool }{ { - "pass - nil Base Fee", + "pass - default Base Fee", func() { - expRes = &types.QueryBaseFeeResponse{} + initialBaseFee := sdk.NewInt(ethparams.InitialBaseFee) + expRes = &types.QueryBaseFeeResponse{BaseFee: &initialBaseFee} }, true, }, diff --git a/x/feemarket/types/genesis.go b/x/feemarket/types/genesis.go index a81829e98e..b92f5b92ad 100644 --- a/x/feemarket/types/genesis.go +++ b/x/feemarket/types/genesis.go @@ -4,13 +4,15 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/ethereum/go-ethereum/params" ) // DefaultGenesisState sets default fee market genesis state. func DefaultGenesisState() *GenesisState { return &GenesisState{ - Params: DefaultParams(), - BaseFee: sdk.ZeroInt(), + Params: DefaultParams(), + // the default base fee should be initialized because the default enable height is zero. + BaseFee: sdk.NewIntFromUint64(params.InitialBaseFee), BlockGas: 0, } }