Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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. #2124

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion app/evmante/evmante_gas_consume.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ func (anteDec AnteDecEthGasConsume) AnteHandle(

fees, err := keeper.VerifyFee(
txData,
evm.EVMBankDenom,
baseFeeMicronibiPerGas,
ctx.IsCheckTx(),
)
Expand Down
37 changes: 20 additions & 17 deletions x/evm/keeper/gas_fees.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
5 changes: 2 additions & 3 deletions x/evm/keeper/gas_fees_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
}
}
Expand Down
18 changes: 15 additions & 3 deletions x/evm/tx_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
7 changes: 4 additions & 3 deletions x/evm/tx_data_access_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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())
}
Expand Down
11 changes: 6 additions & 5 deletions x/evm/tx_data_dynamic_fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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())
}
Expand All @@ -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())
}
8 changes: 5 additions & 3 deletions x/evm/tx_data_legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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())
}
Expand Down
Loading