diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c6235795..27e54befb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,10 @@ that gas consumed during any send operation of the "NibiruBankKeeper" depends only on the "bankkeeper.BaseKeeper"'s gas consumption. - [#2120](https://github.com/NibiruChain/nibiru/pull/2120) - fix: Use canonical hexadecimal strings for Eip155 address encoding - [#2122](https://github.com/NibiruChain/nibiru/pull/2122) - test(evm): more bank extension tests and EVM ABCI integration tests to prevent regressions +- [#2124](https://github.com/NibiruChain/nibiru/pull/2124) - refactor(evm): +Remove unnecessary argument in the `VerifyFee` function, which returns the token +payment required based on the effective fee from the tx data. Improve +documentation. #### Nibiru EVM | Before Audit 2 - 2024-12-06 diff --git a/app/evmante/evmante_gas_consume.go b/app/evmante/evmante_gas_consume.go index 527d6fdaf..01d39668e 100644 --- a/app/evmante/evmante_gas_consume.go +++ b/app/evmante/evmante_gas_consume.go @@ -99,7 +99,6 @@ func (anteDec AnteDecEthGasConsume) AnteHandle( fees, err := keeper.VerifyFee( txData, - evm.EVMBankDenom, baseFeeMicronibiPerGas, ctx.IsCheckTx(), ) diff --git a/x/evm/keeper/gas_fees.go b/x/evm/keeper/gas_fees.go index 4d17de0bd..58de7703d 100644 --- a/x/evm/keeper/gas_fees.go +++ b/x/evm/keeper/gas_fees.go @@ -140,13 +140,25 @@ func (k *Keeper) DeductTxCostsFromUserBalance( return nil } -// VerifyFee is used to return the fee for the given transaction data in -// sdk.Coins. It checks that the gas limit is not reached, the gas limit is -// higher than the intrinsic gas and that the base fee is lower than the gas fee -// cap. +// VerifyFee is used to return the fee, or token payment, for the given +// transaction data in [sdk.Coin]s. It checks that the the gas limit and uses the +// "effective fee" from the [evm.TxData]. +// +// - For [evm.DynamicFeeTx], the effective gas price is the minimum of +// (baseFee + tipCap) and the gas fee cap (feeCap). +// - For [evm.LegacyTx] and [evm.AccessListTx], the effective gas price is the +// max of the gas price and baseFee. +// +// Transactions where the baseFee exceeds the feeCap are priced out +// under EIP-1559 and will not pass validation. +// +// Args: +// - txData: Tx data related to gas, effectie gas, nonce, and chain ID +// implemented by every Ethereum tx type. +// - baseFeeMicronibi:EIP1559 base fee in units of micronibi ("unibi"). +// - isCheckTx: Comes from `[sdk.Context].isCheckTx()` func VerifyFee( txData evm.TxData, - denom string, baseFeeMicronibi *big.Int, isCheckTx bool, ) (sdk.Coins, error) { @@ -180,22 +192,13 @@ func VerifyFee( baseFeeMicronibi = evm.BASE_FEE_MICRONIBI } - // gasFeeCapMicronibi := evm.WeiToNative(txData.GetGasFeeCapWei()) - // if baseFeeMicronibi != nil && gasFeeCapMicronibi.Cmp(baseFeeMicronibi) < 0 { - // baseFeeWei := evm.NativeToWei(baseFeeMicronibi) - // return nil, errors.Wrapf(errortypes.ErrInsufficientFee, - // "the tx gasfeecap is lower than the tx baseFee: %s (gasfeecap), %s (basefee) wei per gas", - // txData.GetGasFeeCapWei(), - // baseFeeWei, - // ) - // } - baseFeeWei := evm.NativeToWei(baseFeeMicronibi) feeAmtMicronibi := evm.WeiToNative(txData.EffectiveFeeWei(baseFeeWei)) + bankDenom := evm.EVMBankDenom if feeAmtMicronibi.Sign() == 0 { // zero fee, no need to deduct - return sdk.Coins{{Denom: denom, Amount: sdkmath.ZeroInt()}}, nil + return sdk.Coins{{Denom: bankDenom, Amount: sdkmath.ZeroInt()}}, nil } - return sdk.Coins{{Denom: denom, Amount: sdkmath.NewIntFromBigInt(feeAmtMicronibi)}}, nil + return sdk.Coins{{Denom: bankDenom, Amount: sdkmath.NewIntFromBigInt(feeAmtMicronibi)}}, nil } diff --git a/x/evm/keeper/gas_fees_test.go b/x/evm/keeper/gas_fees_test.go index e9a1232fc..0ee23b00c 100644 --- a/x/evm/keeper/gas_fees_test.go +++ b/x/evm/keeper/gas_fees_test.go @@ -101,19 +101,18 @@ func (s *Suite) TestVerifyFee() { } }, } { - feeDenom := evm.EVMBankDenom isCheckTx := true tc := getTestCase() s.Run(tc.name, func() { gotCoins, err := evmkeeper.VerifyFee( - tc.txData, feeDenom, tc.baseFeeMicronibi, isCheckTx, + tc.txData, tc.baseFeeMicronibi, isCheckTx, ) if tc.wantErr != "" { s.Require().ErrorContains(err, tc.wantErr) return } s.Require().NoError(err) - s.Equal(tc.wantCoinAmt, gotCoins.AmountOf(feeDenom).String()) + s.Equal(tc.wantCoinAmt, gotCoins.AmountOf(evm.EVMBankDenom).String()) }) } } diff --git a/x/evm/tx_data.go b/x/evm/tx_data.go index 5b1a75a78..c8bf3720c 100644 --- a/x/evm/tx_data.go +++ b/x/evm/tx_data.go @@ -32,6 +32,8 @@ var ( // provide custom implementations for these methods without creating a new TxData // data structure. Thus, the current interface exists. type TxData interface { + // TxType returns a byte (uint8) identifying the tx as a [LegacyTx] (0), + // [AccessListTx] (1), or [DynamicFeeTx] (2). TxType() byte Copy() TxData GetChainID() *big.Int @@ -86,14 +88,24 @@ type TxData interface { AsEthereumData() gethcore.TxData Validate() error - // static fee + // Fee in units of wei := gasPrice (wei per gas) * gasLimit (gas). Fee() *big.Int - // Cost is the gas cost of the transaction in wei + // Cost in units of wei := Fee * Value. Cost is the gas cost of the + // transaction in wei, accounting for value transferred. Cost() *big.Int - // effective gasPrice/fee/cost according to current base fee + // EffectiveGasPriceWeiPerGas is effective gas price in units of wei per gas. + // "Effective" means depending on the base fee of the network. EffectiveGasPriceWeiPerGas(baseFeeWei *big.Int) *big.Int + + // EffectiveFeeWei := effectiveGasPrice (wei per gas) * gasLimit (gas). + // "Effective" means depending on the base fee of the network. EffectiveFeeWei(baseFeeWei *big.Int) *big.Int + + // EffectiveCostWei is the same as cost, except it uses effective fee in wei + // rathen than fee. Effective cost is the gas cost of the transaction in wei, + // accounting for value (wei) transferred and using effective gas price (wei + // per gas). EffectiveCostWei(baseFeeWei *big.Int) *big.Int } diff --git a/x/evm/tx_data_access_list.go b/x/evm/tx_data_access_list.go index f55828421..acbcca26b 100644 --- a/x/evm/tx_data_access_list.go +++ b/x/evm/tx_data_access_list.go @@ -101,7 +101,8 @@ func newAccessListTx(tx *gethcore.Transaction) (*AccessListTx, error) { return txData, nil } -// TxType returns the tx type +// TxType returns a byte (uint8) identifying the tx as a [LegacyTx] (0), +// [AccessListTx] (1), or [DynamicFeeTx] (2). func (tx *AccessListTx) TxType() uint8 { return gethcore.AccessListTxType } @@ -280,12 +281,12 @@ func (tx AccessListTx) Validate() error { return nil } -// Fee returns gasprice * gaslimit. +// Fee returns gasPrice * gasLimit. func (tx AccessListTx) Fee() *big.Int { return priceTimesGas(tx.GetGasPrice(), tx.GetGas()) } -// Cost returns amount + gasprice * gaslimit. +// Cost returns amount + gasPrice * gasLimit. func (tx AccessListTx) Cost() *big.Int { return cost(tx.Fee(), tx.GetValueWei()) } diff --git a/x/evm/tx_data_dynamic_fee.go b/x/evm/tx_data_dynamic_fee.go index 694ed7bfb..040fa5d4d 100644 --- a/x/evm/tx_data_dynamic_fee.go +++ b/x/evm/tx_data_dynamic_fee.go @@ -75,7 +75,8 @@ func NewDynamicFeeTx(tx *gethcore.Transaction) (*DynamicFeeTx, error) { return txData, nil } -// TxType returns the tx type +// TxType returns a byte (uint8) identifying the tx as a [LegacyTx] (0), +// [AccessListTx] (1), or [DynamicFeeTx] (2). func (tx *DynamicFeeTx) TxType() uint8 { return gethcore.DynamicFeeTxType } @@ -285,12 +286,12 @@ func (tx DynamicFeeTx) Validate() error { return nil } -// Fee returns gasprice * gaslimit. +// Fee returns gasPrice * gasLimit. func (tx DynamicFeeTx) Fee() *big.Int { return priceTimesGas(tx.GetGasFeeCapWei(), tx.GasLimit) } -// Cost returns amount + gasprice * gaslimit. +// Cost returns amount + gasPrice * gasLimit. func (tx DynamicFeeTx) Cost() *big.Int { return cost(tx.Fee(), tx.GetValueWei()) } @@ -305,12 +306,12 @@ func (tx *DynamicFeeTx) EffectiveGasPriceWeiPerGas(baseFeeWei *big.Int) *big.Int return BigIntMax(baseFeeWei, rawEffectiveGasPrice) } -// EffectiveFeeWei returns effective_gasprice * gaslimit. +// EffectiveFeeWei returns effective_gasPrice * gasLimit. func (tx DynamicFeeTx) EffectiveFeeWei(baseFeeWei *big.Int) *big.Int { return priceTimesGas(tx.EffectiveGasPriceWeiPerGas(baseFeeWei), tx.GasLimit) } -// EffectiveCostWei returns amount + effective_gasprice * gaslimit. +// EffectiveCostWei returns amount + effective_gasPrice * gasLimit. func (tx DynamicFeeTx) EffectiveCostWei(baseFeeWei *big.Int) *big.Int { return cost(tx.EffectiveFeeWei(baseFeeWei), tx.GetValueWei()) } diff --git a/x/evm/tx_data_legacy.go b/x/evm/tx_data_legacy.go index 9f334ce97..1b3bab940 100644 --- a/x/evm/tx_data_legacy.go +++ b/x/evm/tx_data_legacy.go @@ -43,7 +43,8 @@ func NewLegacyTx(tx *gethcore.Transaction) (*LegacyTx, error) { return txData, nil } -// TxType returns the tx type +// TxType returns a byte (uint8) identifying the tx as a [LegacyTx] (0), +// [AccessListTx] (1), or [DynamicFeeTx] (2). func (tx *LegacyTx) TxType() uint8 { return gethcore.LegacyTxType } @@ -184,12 +185,13 @@ func (tx LegacyTx) Validate() error { return nil } -// Fee returns gasprice * gaslimit. +// Fee := gasPrice (wei per gas) * gasLimit (gas). Thus, fee is in units of +// wei. func (tx LegacyTx) Fee() *big.Int { return priceTimesGas(tx.GetGasPrice(), tx.GetGas()) } -// Cost returns amount + gasprice * gaslimit. +// Cost returns amount + gasPrice * gasLimit. func (tx LegacyTx) Cost() *big.Int { return cost(tx.Fee(), tx.GetValueWei()) }