fix(evm): false positive failed EVM txs due to sdk.GasMeter error after successful ApplyEvmMsg#2521
Conversation
…er successful ApplyEvmMsg
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a critical bug where successful EVM transactions were incorrectly reported as failed due to a gas accounting mismatch between the Cosmos SDK and the EVM. The core logic of resetting the gas meter before applying the EVM's gas usage is a solid and correct approach. I appreciate the detailed explanation and the thoroughness of the changes, including the updated tests which now correctly assert the new gas accounting behavior. My review includes a couple of minor suggestions to improve logging and test code quality.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2521 +/- ##
==========================================
+ Coverage 59.09% 59.11% +0.01%
==========================================
Files 361 361
Lines 24421 24429 +8
==========================================
+ Hits 14432 14440 +8
Misses 8751 8751
Partials 1238 1238
🚀 New features to boost your workflow:
|
Fix false failed EVM txs from SafeConsumeGas / gas-meter misalignment
Align Cosmos gas accounting with the EVM's own gas usage so that successful EVM transactions (including zero-fee ones) are never reported as failed solely due to a late
SafeConsumeGaserror on the SDK gas meter.Key Changes
Root cause
open_tradetx in26-02-10-zero-gas-debug.md, the EVM trace and receipt show success (23 logs), buteth.evm.v1.EventEthereumTxreportsvm_error="gas consumption failed: gasConsumed=719812, gasRemaining=0, {execute EthereumTx}"andstatus: 0x0.sdk.GasMeterwas counting both ante/Cosmos work (~138k) and EVMGasUsed(~581k) via a lateSafeConsumeGascall inEthereumTx, sogasConsumedcould exceed the user'sgaslimit even whenGasUsed ≤ gasLimit.EthereumTxgas accountingx/evm/evmstate/msg_server.go::EthereumTx, after a successfulApplyEvmMsgwe now:RefundGas(consumed, "reset gas before EVM charge")), preserving the limit but resetting consumption to zero.evmResp.GasUsedviaevm.SafeConsumeGas(sdb.RootCtx(), evmResp.GasUsed, "execute EthereumTx"), so the SDK meter reflects only EVM gas.SafeConsumeGasstill errors andevmResp.Failed() == false, we log the error and continue instead of overwritingevmResp.VmError, since EVM gas is the ground truth and a host-side meter issue should not flip a successful tx to failed.Post-execution behavior
msg.GasLimit - evmResp.GasUsedin wei; zero-fee txs still skip refunds.Ante handler responsibilities
x/evm/evmante/evmante_gas_consume.gois documented as a two-phase model:AnteStepGasWantedandAnteStepBlockGasMeteruse infinite meters with limits to validate tx/block gas and priority;AnteStepFiniteGasLimitForABCIDeliverTxinstalls a finite meter so ABCIGasWantedsees the user gas limit.EthereumTx'sSafeConsumeGascall (after the refund-to-zero) records finalGasUsedon the SDK meter, avoiding double-counting ante work.Tests and invariants
x/evm/evmstate/funtoken_from_erc20_test.go, the fun-token infinite recursion tests now assert that, after the fix:evmResp).EthereumTxsucceeds and satisfiesctx.GasMeter().GasConsumed() == evmResp.GasUsed, proving Cosmos gas accounting matches EVMGasUsedfor that path.26-02-10-zero-gas-debug.md, these tests guard against regressions where ante + EVM gas are recombined orSafeConsumeGaserrors are surfaced as false EVM failures.Appendix
gascontinues to mean “EVM execution gas limit”, and the reportedGasUsedin receipts and ABCI results now reflects only EVM work; a tx the EVM considers successful (receiptstatus = 0x1) will no longer be marked failed purely due to host gas-meter alignment issues.SafeConsumeGaserror after EVM success will appear only as a log mentioning gas-meter misalignment; the detailed reproduction in26-02-10-zero-gas-debug.mdremains a reference for tracing such issues and confirms that the original 581k/669k scenario no longer yields a false failed EVM tx.