fix(evm): align debug_trace fee with virtual ante path#967
Conversation
thomas-nguy
requested review from
JayT106 and
randy-cro
and removed request for
a team
|
@thomas-nguy your pull request is missing a changelog! |
|
@claude review |
|
Claude finished @thomas-nguy's task in 5m 44s —— View job PR Review:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if baseFee != nil && msg.GasFeeCap != nil && msg.GasTipCap != nil { | ||
| gasPrice = ethermint.BigMin(new(big.Int).Add(msg.GasTipCap, baseFee), msg.GasFeeCap) | ||
| } | ||
| return new(big.Int).Mul(gasPrice, new(big.Int).SetUint64(msg.GasLimit)) |
There was a problem hiding this comment.
Potential asymmetry with RefundGas: debugTraceFeeAmount correctly uses the effective gas price (min(tip+baseFee, feeCap)) to compute the initial deduction. However, RefundGas (in gas.go) still computes the refund as leftoverGas * msg.GasPrice, where msg.GasPrice is the raw GasFeeCap for EIP-1559 txs.
This creates an asymmetry for EIP-1559 transactions in the debug trace path:
- Deducted:
effectiveGasPrice * gasLimit - Refunded:
gasFeeCap * leftoverGas
For example with baseFee=1gwei, tipCap=0, feeCap=5000gwei, gasLimit=2M, leftoverGas=1M:
- Deducted:
1 * 2M = 2M gwei - Refund attempt:
5000 * 1M = 5B gwei→ fee collector would not have enough → panic/error
The test in state_transition_test.go uses gasTipCap=0 so effectiveGasPrice == baseFee and doesn't cover this case. Consider passing the effective gas price to RefundGas or fixing the refund to also use effective price for the debug trace path. Fix this →
| suite.mintFeeCollector = true | ||
| suite.SetupTest() | ||
|
|
||
| baseFee := big.NewInt(1_000_000_000) |
There was a problem hiding this comment.
The test only covers the case where gasTipCap = 0, so effectiveGasPrice == baseFee, which means deduction and refund both happen at the same price and the asymmetry between debugTraceFeeAmount (effective price) and RefundGas (msg.GasPrice = gasFeeCap) doesn't manifest.
Consider adding a second case where gasTipCap > 0 and gasFeeCap >> baseFee + tip, setting the balance to exactly effectiveFee (not gasFeeCap * gasLimit), to validate the whole round-trip deduct→execute→refund without over-refunding.
| feemarketGenesis.Params.NoBaseFee = false | ||
| genesis[feemarkettypes.ModuleName] = a.AppCodec().MustMarshalJSON(feemarketGenesis) | ||
| return genesis | ||
| }) |
There was a problem hiding this comment.
Minor: calling suite.SetupTestWithCb followed immediately by suite.SetupTest on line 672 means the callback-customized genesis is discarded — SetupTest reinitialises the suite with default genesis. The SetupTestWithCb call appears redundant here unless SetupTest is designed to call the previously registered callback. Worth confirming the intent.
| return nil, err | ||
| } | ||
| } | ||
| tracingStateDB.SetNonce(sender, stateDB.GetNonce(sender)+1, tracing.NonceChangeEoACall) |
There was a problem hiding this comment.
The nonce is incremented here before the EVM call, but for the non-DebugTrace contract-creation path the code explicitly resets the nonce to msg.Nonce before calling evm.Create (line 444) to match ante-handler pre-increment semantics. For DebugTrace + contractCreation, the nonce increment here plus the reset to msg.Nonce in the Create branch would interact in an unexpected way — confirm this is correct or add a note.
| } | ||
| if req.BaseFee != nil { | ||
| baseFee = big.NewInt(req.BaseFee.Int64()) | ||
| if req.BaseFee != nil && !req.BaseFee.IsNil() { |
There was a problem hiding this comment.
Good fix — using .BigInt() avoids silent truncation on values > math.MaxInt64. The added !req.BaseFee.IsNil() guard is also correct since a non-nil *sdkmath.Int can hold a nil internal value.
One minor note: sdkmath.Int.BigInt() returns a copy of the internal *big.Int, so the caller can mutate it safely — no defensive copy needed.
1 participant
Closes: #XXX
Description
Debug trace used StateDB burn/refund for gasLimit×gasPrice, which
diverged from CheckEthGasConsume (virtual fee collector) and could
RPC-fail with bank insufficient funds on txs that actually executed.
Use DeductTxCostsFromUserBalance + RefundGas when DebugTrace is set,
compute effective gas price for EIP-1559 holds, and parse TraceTx
BaseFee via BigInt to avoid truncation.
For contributor use:
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerFor admin use:
WIP,R4R,docs, etc)