fix(gas-fees): use effective gas price in RefundGas (#2076)

* fix(gas-fees): use effective gas price in RefundGas

* Add even more unit clarity for base fee and delete code

* fix e2e test and address coderabbitai PR comments

* red, green, refactor

* red, green, refactor

* fix e2e tests

.gitignore

@@ -21,6 +21,7 @@ txout.json
 vote.json
 **__pycache**
 scratch-paper.md
+logs
 
 ### TypeScript and Friends
 

CHANGELOG.md

@@ -129,6 +129,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#2056](https://github.com/NibiruChain/nibiru/pull/2056) - feat(evm): add oracle precompile
 - [#2065](https://github.com/NibiruChain/nibiru/pull/2065) - refactor(evm)!: Refactor out dead code from the evm.Params
 - [#2073](https://github.com/NibiruChain/nibiru/pull/2073) - fix(evm-keeper): better utilize ERC20 metadata during FunToken creation
+- [#2076](https://github.com/NibiruChain/nibiru/pull/2076) - fix(evm-gas-fees):
+Use effective gas price in RefundGas and make sure that units are properly
+reflected on all occurences of "base fee" in the codebase. This fixes [#2059](https://github.com/NibiruChain/nibiru/issues/2059)
+and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724).
 
 
 #### Dapp modules: perp, spot, oracle, etc

app/ante/gas.go

@@ -35,8 +35,11 @@ func (g *fixedGasMeter) GasRemaining() storetypes.Gas {
 	return g.consumed
 }
 
+// ConsumeGas is a no-op because the fixed gas meter stays fixed.
 func (g *fixedGasMeter) ConsumeGas(types.Gas, string) {}
-func (g *fixedGasMeter) RefundGas(types.Gas, string)  {}
+
+// RefundGas is a no-op because the fixed gas meter stays fixed.
+func (g *fixedGasMeter) RefundGas(types.Gas, string) {}
 
 func (g *fixedGasMeter) IsPastLimit() bool {
 	return false

app/evmante/evmante_can_transfer.go

@@ -17,23 +17,16 @@ import (
 // CanTransferDecorator checks if the sender is allowed to transfer funds according to the EVM block
 // context rules.
 type CanTransferDecorator struct {
-	evmKeeper EVMKeeper
-}
-
-// NewCanTransferDecorator creates a new CanTransferDecorator instance.
-func NewCanTransferDecorator(k EVMKeeper) CanTransferDecorator {
-	return CanTransferDecorator{
-		evmKeeper: k,
-	}
+	*EVMKeeper
 }
 
 // AnteHandle creates an EVM from the message and calls the BlockContext CanTransfer function to
 // see if the address can execute the transaction.
 func (ctd CanTransferDecorator) AnteHandle(
 	ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler,
 ) (sdk.Context, error) {
-	params := ctd.evmKeeper.GetParams(ctx)
-	ethCfg := evm.EthereumConfig(ctd.evmKeeper.EthChainID(ctx))
+	params := ctd.GetParams(ctx)
+	ethCfg := evm.EthereumConfig(ctd.EVMKeeper.EthChainID(ctx))
 	signer := gethcore.MakeSigner(ethCfg, big.NewInt(ctx.BlockHeight()))
 
 	for _, msg := range tx.GetMsgs() {
@@ -44,44 +37,47 @@ func (ctd CanTransferDecorator) AnteHandle(
 				"invalid message type %T, expected %T", msg, (*evm.MsgEthereumTx)(nil),
 			)
 		}
-		baseFee := ctd.evmKeeper.GetBaseFee(ctx)
 
-		coreMsg, err := msgEthTx.AsMessage(signer, baseFee)
+		baseFeeWeiPerGas := evm.NativeToWei(ctd.EVMKeeper.BaseFeeMicronibiPerGas(ctx))
+
+		coreMsg, err := msgEthTx.AsMessage(signer, baseFeeWeiPerGas)
 		if err != nil {
 			return ctx, errors.Wrapf(
 				err,
 				"failed to create an ethereum core.Message from signer %T", signer,
 			)
 		}
 
-		if baseFee == nil {
+		if baseFeeWeiPerGas == nil {
 			return ctx, errors.Wrap(
 				evm.ErrInvalidBaseFee,
-				"base fee is supported but evm block context value is nil",
+				"base fee is nil for this block.",
 			)
 		}
-		if coreMsg.GasFeeCap().Cmp(baseFee) < 0 {
+
+		if msgEthTx.EffectiveGasCapWei(baseFeeWeiPerGas).Cmp(baseFeeWeiPerGas) < 0 {
 			return ctx, errors.Wrapf(
 				sdkerrors.ErrInsufficientFee,
-				"max fee per gas less than block base fee (%s < %s)",
-				coreMsg.GasFeeCap(), baseFee,
+				"gas fee cap (wei) less than block base fee (wei); (%s < %s)",
+				coreMsg.GasFeeCap(), baseFeeWeiPerGas,
 			)
 		}
 
-		// NOTE: pass in an empty coinbase address and nil tracer as we don't need them for the check below
 		cfg := &statedb.EVMConfig{
 			ChainConfig: ethCfg,
 			Params:      params,
-			CoinBase:    gethcommon.Address{},
-			BaseFee:     baseFee,
+			// Note that we use an empty coinbase here  because the field is not
+			// used during this Ante Handler.
+			BlockCoinbase: gethcommon.Address{},
+			BaseFeeWei:    baseFeeWeiPerGas,
 		}
 
 		stateDB := statedb.New(
 			ctx,
-			ctd.evmKeeper,
+			ctd.EVMKeeper,
 			statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash().Bytes())),
 		)
-		evmInstance := ctd.evmKeeper.NewEVM(ctx, coreMsg, cfg, evm.NewNoOpTracer(), stateDB)
+		evmInstance := ctd.EVMKeeper.NewEVM(ctx, coreMsg, cfg, evm.NewNoOpTracer(), stateDB)
 
 		// check that caller has enough balance to cover asset transfer for **topmost** call
 		// NOTE: here the gas consumed is from the context with the infinite gas meter

app/evmante/evmante_can_transfer_test.go

@@ -89,7 +89,7 @@ func (s *TestSuite) TestCanTransferDecorator() {
 		s.Run(tc.name, func() {
 			deps := evmtest.NewTestDeps()
 			stateDB := deps.StateDB()
-			anteDec := evmante.NewCanTransferDecorator(&deps.App.AppKeepers.EvmKeeper)
+			anteDec := evmante.CanTransferDecorator{&deps.App.AppKeepers.EvmKeeper}
 			tx := tc.txSetup(&deps)
 
 			if tc.ctxSetup != nil {

app/evmante/evmante_emit_event.go

@@ -13,11 +13,11 @@ import (
 
 // EthEmitEventDecorator emit events in ante handler in case of tx execution failed (out of block gas limit).
 type EthEmitEventDecorator struct {
-	evmKeeper EVMKeeper
+	evmKeeper *EVMKeeper
 }
 
 // NewEthEmitEventDecorator creates a new EthEmitEventDecorator
-func NewEthEmitEventDecorator(k EVMKeeper) EthEmitEventDecorator {
+func NewEthEmitEventDecorator(k *EVMKeeper) EthEmitEventDecorator {
 	return EthEmitEventDecorator{
 		evmKeeper: k,
 	}

app/evmante/evmante_gas_consume.go

@@ -17,13 +17,13 @@ import (
 // AnteDecEthGasConsume validates enough intrinsic gas for the transaction and
 // gas consumption.
 type AnteDecEthGasConsume struct {
-	evmKeeper    EVMKeeper
+	evmKeeper    *EVMKeeper
 	maxGasWanted uint64
 }
 
 // NewAnteDecEthGasConsume creates a new EthGasConsumeDecorator
 func NewAnteDecEthGasConsume(
-	k EVMKeeper,
+	k *EVMKeeper,
 	maxGasWanted uint64,
 ) AnteDecEthGasConsume {
 	return AnteDecEthGasConsume{
@@ -68,7 +68,7 @@ func (anteDec AnteDecEthGasConsume) AnteHandle(
 
 	// Use the lowest priority of all the messages as the final one.
 	minPriority := int64(math.MaxInt64)
-	baseFeeMicronibiPerGas := anteDec.evmKeeper.GetBaseFee(ctx)
+	baseFeeMicronibiPerGas := anteDec.evmKeeper.BaseFeeMicronibiPerGas(ctx)
 
 	for _, msg := range tx.GetMsgs() {
 		msgEthTx, ok := msg.(*evm.MsgEthereumTx)

app/evmante/evmante_handler.go

@@ -18,7 +18,7 @@ func NewAnteHandlerEVM(
 		NewEthValidateBasicDecorator(&options.EvmKeeper),
 		NewEthSigVerificationDecorator(&options.EvmKeeper),
 		NewAnteDecVerifyEthAcc(&options.EvmKeeper, options.AccountKeeper),
-		NewCanTransferDecorator(&options.EvmKeeper),
+		CanTransferDecorator{&options.EvmKeeper},
 		NewAnteDecEthGasConsume(&options.EvmKeeper, options.MaxTxGasWanted),
 		NewAnteDecEthIncrementSenderSequence(&options.EvmKeeper, options.AccountKeeper),
 		ante.AnteDecoratorGasWanted{},

app/evmante/evmante_increment_sender_seq.go

@@ -13,12 +13,12 @@ import (
 
 // AnteDecEthIncrementSenderSequence increments the sequence of the signers.
 type AnteDecEthIncrementSenderSequence struct {
-	evmKeeper     EVMKeeper
+	evmKeeper     *EVMKeeper
 	accountKeeper ante.AccountKeeper
 }
 
 // NewAnteDecEthIncrementSenderSequence creates a new EthIncrementSenderSequenceDecorator.
-func NewAnteDecEthIncrementSenderSequence(k EVMKeeper, ak ante.AccountKeeper) AnteDecEthIncrementSenderSequence {
+func NewAnteDecEthIncrementSenderSequence(k *EVMKeeper, ak ante.AccountKeeper) AnteDecEthIncrementSenderSequence {
 	return AnteDecEthIncrementSenderSequence{
 		evmKeeper:     k,
 		accountKeeper: ak,

app/evmante/evmante_mempool_fees.go

@@ -17,12 +17,12 @@ var _ sdk.AnteDecorator = MempoolGasPriceDecorator{}
 // is rejected. This applies to CheckTx only.
 // If fee is high enough, then call next AnteHandler
 type MempoolGasPriceDecorator struct {
-	evmKeeper EVMKeeper
+	evmKeeper *EVMKeeper
 }
 
 // NewMempoolGasPriceDecorator creates a new MinGasPriceDecorator instance used only for
 // Ethereum transactions.
-func NewMempoolGasPriceDecorator(k EVMKeeper) MempoolGasPriceDecorator {
+func NewMempoolGasPriceDecorator(k *EVMKeeper) MempoolGasPriceDecorator {
 	return MempoolGasPriceDecorator{
 		evmKeeper: k,
 	}
@@ -39,14 +39,14 @@ func (d MempoolGasPriceDecorator) AnteHandle(
 	}
 
 	minGasPrice := ctx.MinGasPrices().AmountOf(evm.EVMBankDenom)
-	baseFeeMicronibi := d.evmKeeper.GetBaseFee(ctx)
-	baseFeeDec := math.LegacyNewDecFromBigInt(baseFeeMicronibi)
+	baseFeeMicronibi := d.evmKeeper.BaseFeeMicronibiPerGas(ctx)
+	baseFeeMicronibiDec := math.LegacyNewDecFromBigInt(baseFeeMicronibi)
 
 	// if MinGasPrices is not set, skip the check
 	if minGasPrice.IsZero() {
 		return next(ctx, tx, simulate)
-	} else if minGasPrice.LT(baseFeeDec) {
-		minGasPrice = baseFeeDec
+	} else if minGasPrice.LT(baseFeeMicronibiDec) {
+		minGasPrice = baseFeeMicronibiDec
 	}
 
 	for _, msg := range tx.GetMsgs() {
@@ -61,7 +61,7 @@ func (d MempoolGasPriceDecorator) AnteHandle(
 
 		baseFeeWei := evm.NativeToWei(baseFeeMicronibi)
 		effectiveGasPriceDec := math.LegacyNewDecFromBigInt(
-			evm.WeiToNative(ethTx.GetEffectiveGasPrice(baseFeeWei)),
+			evm.WeiToNative(ethTx.EffectiveGasPriceWeiPerGas(baseFeeWei)),
 		)
 		if effectiveGasPriceDec.LT(minGasPrice) {
 			// if sdk.NewDecFromBigInt(effectiveGasPrice).LT(minGasPrice) {

app/evmante/evmante_setup_ctx.go

@@ -12,10 +12,10 @@ import (
 // EthSetupContextDecorator is adapted from SetUpContextDecorator from cosmos-sdk, it ignores gas consumption
 // by setting the gas meter to infinite
 type EthSetupContextDecorator struct {
-	evmKeeper EVMKeeper
+	evmKeeper *EVMKeeper
 }
 
-func NewEthSetUpContextDecorator(k EVMKeeper) EthSetupContextDecorator {
+func NewEthSetUpContextDecorator(k *EVMKeeper) EthSetupContextDecorator {
 	return EthSetupContextDecorator{
 		evmKeeper: k,
 	}

app/evmante/evmante_sigverify.go

@@ -14,11 +14,11 @@ import (
 
 // EthSigVerificationDecorator validates an ethereum signatures
 type EthSigVerificationDecorator struct {
-	evmKeeper EVMKeeper
+	evmKeeper *EVMKeeper
 }
 
 // NewEthSigVerificationDecorator creates a new EthSigVerificationDecorator
-func NewEthSigVerificationDecorator(k EVMKeeper) EthSigVerificationDecorator {
+func NewEthSigVerificationDecorator(k *EVMKeeper) EthSigVerificationDecorator {
 	return EthSigVerificationDecorator{
 		evmKeeper: k,
 	}

app/evmante/evmante_validate_basic.go

@@ -15,11 +15,11 @@ import (
 
 // EthValidateBasicDecorator is adapted from ValidateBasicDecorator from cosmos-sdk, it ignores ErrNoSignatures
 type EthValidateBasicDecorator struct {
-	evmKeeper EVMKeeper
+	evmKeeper *EVMKeeper
 }
 
 // NewEthValidateBasicDecorator creates a new EthValidateBasicDecorator
-func NewEthValidateBasicDecorator(k EVMKeeper) EthValidateBasicDecorator {
+func NewEthValidateBasicDecorator(k *EVMKeeper) EthValidateBasicDecorator {
 	return EthValidateBasicDecorator{
 		evmKeeper: k,
 	}
@@ -89,7 +89,7 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
 	txFee := sdk.Coins{}
 	txGasLimit := uint64(0)
 
-	baseFee := vbd.evmKeeper.GetBaseFee(ctx)
+	baseFeeMicronibi := vbd.evmKeeper.BaseFeeMicronibiPerGas(ctx)
 
 	for _, msg := range protoTx.GetMsgs() {
 		msgEthTx, ok := msg.(*evm.MsgEthereumTx)
@@ -115,7 +115,7 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
 			return ctx, errorsmod.Wrap(err, "failed to unpack MsgEthereumTx Data")
 		}
 
-		if baseFee == nil && txData.TxType() == gethcore.DynamicFeeTxType {
+		if baseFeeMicronibi == nil && txData.TxType() == gethcore.DynamicFeeTxType {
 			return ctx, errorsmod.Wrap(
 				gethcore.ErrTxTypeNotSupported,
 				"dynamic fee tx not supported",

app/evmante/evmante_verify_eth_acc.go

@@ -14,12 +14,12 @@ import (
 
 // AnteDecVerifyEthAcc validates an account balance checks
 type AnteDecVerifyEthAcc struct {
-	evmKeeper     EVMKeeper
+	evmKeeper     *EVMKeeper
 	accountKeeper evm.AccountKeeper
 }
 
 // NewAnteDecVerifyEthAcc creates a new EthAccountVerificationDecorator
-func NewAnteDecVerifyEthAcc(k EVMKeeper, ak evm.AccountKeeper) AnteDecVerifyEthAcc {
+func NewAnteDecVerifyEthAcc(k *EVMKeeper, ak evm.AccountKeeper) AnteDecVerifyEthAcc {
 	return AnteDecVerifyEthAcc{
 		evmKeeper:     k,
 		accountKeeper: ak,

app/evmante/interfaces.go

@@ -2,33 +2,12 @@
 package evmante
 
 import (
-	"math/big"
-
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/tx"
-	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/core"
-	"github.com/ethereum/go-ethereum/core/vm"
 
-	"github.com/NibiruChain/nibiru/v2/x/evm"
 	evmkeeper "github.com/NibiruChain/nibiru/v2/x/evm/keeper"
-	"github.com/NibiruChain/nibiru/v2/x/evm/statedb"
 )
 
-// EVMKeeper defines the expected keeper interface used on the AnteHandler
-type EVMKeeper interface {
-	statedb.Keeper
-
-	NewEVM(ctx sdk.Context, msg core.Message, cfg *statedb.EVMConfig, tracer vm.EVMLogger, stateDB vm.StateDB) *vm.EVM
-	DeductTxCostsFromUserBalance(ctx sdk.Context, fees sdk.Coins, from common.Address) error
-	GetEvmGasBalance(ctx sdk.Context, addr common.Address) *big.Int
-	ResetTransientGasUsed(ctx sdk.Context)
-	GetParams(ctx sdk.Context) evm.Params
-
-	EVMState() evmkeeper.EvmState
-	EthChainID(ctx sdk.Context) *big.Int
-	GetBaseFee(ctx sdk.Context) *big.Int
-}
+type EVMKeeper = evmkeeper.Keeper
 
 type protoTxProvider interface {
 	GetProtoTx() *tx.Tx

e2e/evm/test/contract_send_nibi.test.ts

@@ -10,7 +10,7 @@
  * "e2e/evm/contracts/SendReceiveNibi.sol".
  */
 import { describe, expect, it } from "@jest/globals"
-import { toBigInt, Wallet } from "ethers"
+import { parseEther, toBigInt, Wallet } from "ethers"
 import { account, provider } from "./setup"
 import { deployContractSendNibi } from "./utils"
 
@@ -33,16 +33,25 @@ async function testSendNibi(
   const txCostWei = txCostMicronibi * tenPow12
   const expectedOwnerWei = ownerBalanceBefore - txCostWei
 
+  const ownerBalanceAfter = await provider.getBalance(account)
+  const recipientBalanceAfter = await provider.getBalance(recipient)
+
   console.debug(`DEBUG method ${method} %o:`, {
     ownerBalanceBefore,
     weiToSend,
-    gasUsed: receipt.gasUsed,
-    gasPrice: `${receipt.gasPrice.toString()} micronibi`,
     expectedOwnerWei,
+    ownerBalanceAfter,
+    recipientBalanceBefore,
+    recipientBalanceAfter,
+    gasUsed: receipt.gasUsed,
+    gasPrice: `${receipt.gasPrice.toString()}`,
+    to: receipt.to,
+    from: receipt.from,
   })
-
-  await expect(provider.getBalance(account)).resolves.toBe(expectedOwnerWei)
-  await expect(provider.getBalance(recipient)).resolves.toBe(weiToSend)
+  expect(recipientBalanceAfter).toBe(weiToSend)
+  const delta = ownerBalanceAfter - expectedOwnerWei
+  const deltaFromExpectation = delta >= 0 ? delta : -delta
+  expect(deltaFromExpectation).toBeLessThan(parseEther("0.1"))
 }
 
 describe("Send NIBI via smart contract", () => {

e2e/evm/test/eth_queries.test.ts

@@ -49,7 +49,7 @@ describe("eth queries", () => {
   it("eth_gasPrice", async () => {
     const gasPrice = await provider.send("eth_gasPrice", [])
     expect(gasPrice).toBeDefined()
-    expect(gasPrice).toEqual(hexify(1))
+    expect(gasPrice).toEqual(hexify(1000000000000)) // 1 micronibi == 10^{12} wei
   })
 
   it("eth_getBalance", async () => {