fix: nil check in post (backport #118) (#119)

* fix: nil check in post (#118)

* fix

* fix with check

* add early exit

* fix

(cherry picked from commit f0997fb)

# Conflicts:
#	x/feemarket/post/fee.go
#	x/feemarket/post/fee_test.go

* ok

* ok

* fix

---------

Co-authored-by: Alex Johnson <[email protected]>

Author: mergify[bot]

docs/INTEGRATIONS.md

@@ -67,7 +67,7 @@ There are two ways to construct a transaction with `gasPrice`:
 
 ### Understanding Fee Deducted
 
-The actual amount of fee deducted from the fee payer is based on gas consumed, not `gasLimit`.
+The actual amount of fee deducted from the fee payer is based on gas consumed, not `gasLimit`.  The total amount deducted (`fee + tip`) will be equal to the amount of fee specified on your transaction.
 
 The amount consumed is equal to the `inferredTip + gasPrice * gasConsumed`, where `inferredTip = feeAmount - gasLimit * gasPrice`  (This may be different than the tip you specified when building the transaction because the `gasPrice` on chain may have changed since when you queried it.)
 

tests/e2e/e2e_test.go

@@ -9,16 +9,10 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/auth"
 	"github.com/cosmos/cosmos-sdk/x/bank"
 	"github.com/cosmos/cosmos-sdk/x/gov"
-<<<<<<< HEAD
 	interchaintest "github.com/strangelove-ventures/interchaintest/v7"
 	"github.com/strangelove-ventures/interchaintest/v7/chain/cosmos"
 	"github.com/strangelove-ventures/interchaintest/v7/ibc"
 	ictestutil "github.com/strangelove-ventures/interchaintest/v7/testutil"
-=======
-	interchaintest "github.com/strangelove-ventures/interchaintest/v8"
-	"github.com/strangelove-ventures/interchaintest/v8/chain/cosmos"
-	"github.com/strangelove-ventures/interchaintest/v8/ibc"
->>>>>>> daca8c8 (test: make extendable (#112))
 	"github.com/stretchr/testify/suite"
 
 	"github.com/skip-mev/feemarket/tests/e2e"
@@ -121,11 +115,7 @@ var (
 func TestE2ETestSuite(t *testing.T) {
 	s := e2e.NewIntegrationSuite(
 		spec,
-<<<<<<< HEAD
-=======
-		oracleImage,
 		txCfg,
->>>>>>> daca8c8 (test: make extendable (#112))
 	)
 
 	suite.Run(t, s)

tests/e2e/suite.go

@@ -37,83 +37,6 @@ func init() {
 	r = rand.New(s)
 }
 
-<<<<<<< HEAD
-=======
-func DefaultOracleSidecar(image ibc.DockerImage) ibc.SidecarConfig {
-	return ibc.SidecarConfig{
-		ProcessName: "oracle",
-		Image:       image,
-		HomeDir:     "/oracle",
-		Ports:       []string{"8080", "8081"},
-		StartCmd: []string{
-			"slinky",
-			"--oracle-config", "/oracle/oracle.json",
-		},
-		ValidatorProcess: true,
-		PreStart:         true,
-	}
-}
-
-func DefaultOracleConfig(url string) oracleconfig.OracleConfig {
-	cfg := marketmap.DefaultAPIConfig
-	cfg.Endpoints = []oracleconfig.Endpoint{
-		{
-			URL: url,
-		},
-	}
-
-	// Create the oracle config
-	oracleConfig := oracleconfig.OracleConfig{
-		UpdateInterval: 500 * time.Millisecond,
-		MaxPriceAge:    1 * time.Minute,
-		Host:           "0.0.0.0",
-		Port:           "8080",
-		Providers: map[string]oracleconfig.ProviderConfig{
-			marketmap.Name: {
-				Name: marketmap.Name,
-				API:  cfg,
-				Type: "market_map_provider",
-			},
-		},
-	}
-
-	return oracleConfig
-}
-
-func DefaultMarketMap() mmtypes.MarketMap {
-	return mmtypes.MarketMap{}
-}
-
-func GetOracleSideCar(node *cosmos.ChainNode) *cosmos.SidecarProcess {
-	if len(node.Sidecars) == 0 {
-		panic("no sidecars found")
-	}
-	return node.Sidecars[0]
-}
-
-type TestTxConfig struct {
-	SmallSendsNum          int
-	LargeSendsNum          int
-	TargetIncreaseGasPrice math.LegacyDec
-}
-
-func (tx *TestTxConfig) Validate() error {
-	if tx.SmallSendsNum < 1 || tx.LargeSendsNum < 1 {
-		return fmt.Errorf("sends num should be greater than 1")
-	}
-
-	if tx.TargetIncreaseGasPrice.IsNil() {
-		return fmt.Errorf("target increase gas price is nil")
-	}
-
-	if tx.TargetIncreaseGasPrice.LTE(math.LegacyZeroDec()) {
-		return fmt.Errorf("target increase gas price is less than or equal to 0")
-	}
-
-	return nil
-}
-
->>>>>>> daca8c8 (test: make extendable (#112))
 // TestSuite runs the feemarket e2e test-suite against a given interchaintest specification
 type TestSuite struct {
 	suite.Suite
@@ -191,30 +114,18 @@ func WithChainConstructor(cc ChainConstructor) Option {
 	}
 }
 
-<<<<<<< HEAD
-func NewIntegrationSuite(spec *interchaintest.ChainSpec, opts ...Option) *TestSuite {
+func NewIntegrationSuite(spec *interchaintest.ChainSpec, txCfg TestTxConfig, opts ...Option) *TestSuite {
+	if err := txCfg.Validate(); err != nil {
+		panic(err)
+	}
+
 	suite := &TestSuite{
 		spec:      spec,
 		denom:     defaultDenom,
 		authority: authtypes.NewModuleAddress(govtypes.ModuleName),
 		icc:       DefaultInterchainConstructor,
 		cc:        DefaultChainConstructor,
-=======
-func NewIntegrationSuite(spec *interchaintest.ChainSpec, oracleImage ibc.DockerImage, txCfg TestTxConfig, opts ...Option) *TestSuite {
-	if err := txCfg.Validate(); err != nil {
-		panic(err)
-	}
-
-	suite := &TestSuite{
-		spec:         spec,
-		oracleConfig: DefaultOracleSidecar(oracleImage),
-		denom:        defaultDenom,
-		gasPrices:    "",
-		authority:    authtypes.NewModuleAddress(govtypes.ModuleName),
-		icc:          DefaultInterchainConstructor,
-		cc:           DefaultChainConstructor,
-		txConfig:     txCfg,
->>>>>>> daca8c8 (test: make extendable (#112))
+		txConfig:  txCfg,
 	}
 
 	for _, opt := range opts {
@@ -224,6 +135,28 @@ func NewIntegrationSuite(spec *interchaintest.ChainSpec, oracleImage ibc.DockerI
 	return suite
 }
 
+type TestTxConfig struct {
+	SmallSendsNum          int
+	LargeSendsNum          int
+	TargetIncreaseGasPrice math.LegacyDec
+}
+
+func (tx *TestTxConfig) Validate() error {
+	if tx.SmallSendsNum < 1 || tx.LargeSendsNum < 1 {
+		return fmt.Errorf("sends num should be greater than 1")
+	}
+
+	if tx.TargetIncreaseGasPrice.IsNil() {
+		return fmt.Errorf("target increase gas price is nil")
+	}
+
+	if tx.TargetIncreaseGasPrice.LTE(math.LegacyZeroDec()) {
+		return fmt.Errorf("target increase gas price is less than or equal to 0")
+	}
+
+	return nil
+}
+
 func (s *TestSuite) WithKeyringOptions(cdc codec.Codec, opts keyring.Option) {
 	s.broadcasterOverrides = &KeyringOverride{
 		cdc:            cdc,
@@ -353,13 +286,6 @@ func (s *TestSuite) TestSendTxDecrease() {
 
 	s.Run("expect fee market state to decrease", func() {
 		s.T().Log("performing sends...")
-<<<<<<< HEAD
-		for {
-			// send with the exact expected fee
-			height, err := s.chain.(*cosmos.CosmosChain).Height(context.Background())
-			s.Require().NoError(err)
-			// send with the exact expected defaultGasPrice
-=======
 		sig := make(chan struct{})
 		quit := make(chan struct{})
 		defer close(quit)
@@ -384,7 +310,6 @@ func (s *TestSuite) TestSendTxDecrease() {
 			break
 
 		case <-time.After(100 * time.Millisecond):
->>>>>>> daca8c8 (test: make extendable (#112))
 			wg := sync.WaitGroup{}
 			wg.Add(3)
 
@@ -436,17 +361,6 @@ func (s *TestSuite) TestSendTxDecrease() {
 				s.Require().Equal(uint32(0), txResp.DeliverTx.Code, txResp.DeliverTx)
 			}()
 			wg.Wait()
-<<<<<<< HEAD
-			s.WaitForHeight(s.chain.(*cosmos.CosmosChain), height+1)
-
-			gasPrice := s.QueryDefaultGasPrice()
-			s.T().Log("base defaultGasPrice", gasPrice.String())
-
-			if gasPrice.Amount.Equal(params.MinBaseGasPrice) {
-				break
-			}
-=======
->>>>>>> daca8c8 (test: make extendable (#112))
 		}
 
 		// wait for 5 blocks
@@ -475,13 +389,6 @@ func (s *TestSuite) TestSendTxIncrease() {
 	nodes := cosmosChain.Nodes()
 	s.Require().True(len(nodes) > 0)
 
-<<<<<<< HEAD
-	baseGasPrice := s.QueryDefaultGasPrice()
-	gas := int64(20000100)
-	sendAmt := int64(100)
-
-=======
->>>>>>> daca8c8 (test: make extendable (#112))
 	params := s.QueryParams()
 
 	gas := int64(params.MaxBlockUtilization)
@@ -519,7 +426,7 @@ func (s *TestSuite) TestSendTxIncrease() {
 			// add headroom
 			minBaseFeeCoins := sdk.NewCoins(sdk.NewCoin(minBaseFee.Denom, minBaseFee.Amount.Add(math.LegacyNewDec(10)).TruncateInt()))
 
-			height, err := s.chain.(*cosmos.CosmosChain).Height(context.Background())
+			_, err := s.chain.(*cosmos.CosmosChain).Height(context.Background())
 			s.Require().NoError(err)
 			wg := sync.WaitGroup{}
 			wg.Add(3)
@@ -572,17 +479,6 @@ func (s *TestSuite) TestSendTxIncrease() {
 				s.Require().Equal(uint32(0), txResp.DeliverTx.Code, txResp.DeliverTx)
 			}()
 			wg.Wait()
-<<<<<<< HEAD
-			s.WaitForHeight(s.chain.(*cosmos.CosmosChain), height+1)
-
-			baseGasPrice = s.QueryDefaultGasPrice()
-			s.T().Log("gas price", baseGasPrice.String())
-
-			if baseGasPrice.Amount.GT(params.MinBaseGasPrice.Mul(math.LegacyNewDec(10))) {
-				break
-			}
-=======
->>>>>>> daca8c8 (test: make extendable (#112))
 		}
 
 		// wait for 5 blocks

x/feemarket/ante/fee.go

@@ -135,7 +135,6 @@ func (dfd feeMarketCheckDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simula
 		}
 
 		ctx = ctx.WithPriority(GetTxPriority(priorityFee, int64(gas), baseGasPrice))
-		return next(ctx, tx, simulate)
 	}
 	return next(ctx, tx, simulate)
 }

x/feemarket/post/fee.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 
 	errorsmod "cosmossdk.io/errors"
-
+	"cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
@@ -53,11 +53,6 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul
 		return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas")
 	}
 
-	var (
-		tip     sdk.Coin
-		payCoin sdk.Coin
-	)
-
 	// update fee market params
 	params, err := dfd.feemarketKeeper.GetParams(ctx)
 	if err != nil {
@@ -88,6 +83,11 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul
 	feeCoin := feeCoins[0]
 	feeGas := int64(feeTx.GetGas())
 
+	var (
+		tip     = sdk.NewCoin(feeCoin.Denom, math.ZeroInt())
+		payCoin = feeCoin
+	)
+
 	minGasPrice, err := dfd.feemarketKeeper.GetMinGasPrice(ctx, feeCoin.GetDenom())
 	if err != nil {
 		return ctx, errorsmod.Wrapf(err, "unable to get min gas price for denom %s", feeCoins[0].GetDenom())
@@ -106,7 +106,7 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul
 	}
 
 	ctx.Logger().Info("fee deduct post handle",
-		"fee", feeCoins,
+		"fee", payCoin,
 		"tip", tip,
 	)
 
@@ -159,9 +159,11 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T
 		if dfd.feegrantKeeper == nil {
 			return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled")
 		} else if !feeGranter.Equals(feePayer) {
-			err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, sdk.NewCoins(fee), sdkTx.GetMsgs())
-			if err != nil {
-				return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer)
+			if !fee.IsNil() {
+				err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, sdk.NewCoins(fee), sdkTx.GetMsgs())
+				if err != nil {
+					return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", feeGranter, feePayer)
+				}
 			}
 		}
 
@@ -176,7 +178,7 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T
 	var events sdk.Events
 
 	// deduct the fees and tip
-	if !fee.Amount.IsNil() && !fee.IsZero() {
+	if !fee.IsNil() {
 		err := DeductCoins(dfd.bankKeeper, ctx, deductFeesFromAcc, sdk.NewCoins(fee), distributeFees)
 		if err != nil {
 			return err
@@ -190,7 +192,7 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T
 	}
 
 	proposer := sdk.AccAddress(ctx.BlockHeader().ProposerAddress)
-	if !tip.Amount.IsNil() && !tip.IsZero() {
+	if !tip.IsNil() {
 		err := SendTip(dfd.bankKeeper, ctx, deductFeesFromAcc.GetAddress(), proposer, sdk.NewCoins(tip))
 		if err != nil {
 			return err
@@ -211,10 +213,6 @@ func (dfd FeeMarketDeductDecorator) DeductFeeAndTip(ctx sdk.Context, sdkTx sdk.T
 // DeductCoins deducts coins from the given account.
 // Coins can be sent to the default fee collector (causes coins to be distributed to stakers) or sent to the feemarket fee collector account (causes coins to be burned).
 func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI, coins sdk.Coins, distributeFees bool) error {
-	if !coins.IsValid() {
-		return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins)
-	}
-
 	targetModuleAcc := feemarkettypes.FeeCollectorName
 	if distributeFees {
 		targetModuleAcc = authtypes.FeeCollectorName
@@ -230,10 +228,6 @@ func DeductCoins(bankKeeper BankKeeper, ctx sdk.Context, acc authtypes.AccountI,
 
 // SendTip sends a tip to the current block proposer.
 func SendTip(bankKeeper BankKeeper, ctx sdk.Context, acc, proposer sdk.AccAddress, coins sdk.Coins) error {
-	if !coins.IsValid() {
-		return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid coin amount: %s", coins)
-	}
-
 	err := bankKeeper.SendCoins(ctx, acc, proposer, coins)
 	if err != nil {
 		return err