fix(internal/ethapi): skip tx gas limit check for calls #32641#2211
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
and removed request for
Copilot
gzliudan
force-pushed
the
ethapi-tx-gaslimit
branch
from
fe5a9a3 to
f88c613
Compare
There was a problem hiding this comment.
Pull request overview
This PR adjusts call-style execution paths (e.g., eth_call, tracing, simulate, estimateGas) to bypass transaction-only validations—specifically the Osaka/EIP-7825 per-tx gas limit cap—by consolidating skip semantics into core.Message.SkipTransactionChecks. It also introduces and enforces a protocol-wide params.MaxTxGas cap for real transactions in state transition and txpool/miner flows.
Changes:
- Replaces
SkipFromEOACheckwithSkipTransactionChecksand enables it for call-styleMessageconstruction paths. - Introduces
params.MaxTxGasand enforces it post-Osaka in state transition, txpool validation, and miner pending selection. - Updates state processor tests and txpool behavior to reflect the new cap and fork activation behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/state_test_util.go | Updates state-test Message construction to the new skip flag field. |
| params/protocol_params.go | Defines MaxTxGas (EIP-7825 cap constant). |
| miner/worker.go | Applies a pending-selection gas cap for mining after Osaka. |
| internal/ethapi/transaction_args.go | Updates ToMessage signature and marks call Messages with SkipTransactionChecks=true. |
| internal/ethapi/simulate.go | Updates simulate call path to new ToMessage signature. |
| internal/ethapi/api.go | Simplifies call apply flow and updates ToMessage usage. |
| ethclient/simulated/backend.go | Marks simulated eth_call messages as non-transaction (SkipTransactionChecks=true). |
| eth/tracers/api.go | Updates trace call path to new ToMessage signature. |
| core/txpool/validation.go | Enforces MaxTxGas in txpool validation after Osaka. |
| core/txpool/subpool.go | Extends PendingFilter with GasLimitCap. |
| core/txpool/legacypool/legacypool.go | Adds GasLimitCap filtering in Pending() and purges oversized-gas txs at Osaka transition. |
| core/token_validator.go | Marks token-validator call messages as non-transaction (SkipTransactionChecks=true). |
| core/state_transition.go | Introduces SkipTransactionChecks and enforces MaxTxGas for real tx messages post-Osaka. |
| core/state_processor_test.go | Adds Osaka to test config and adds coverage for ErrGasLimitTooHigh. |
| core/error.go | Adds ErrGasLimitTooHigh. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if minTipBig != nil || filter.GasLimitCap != 0 { | ||
| for i, tx := range txs { | ||
| if !tx.IsSpecialTransaction() && tx.EffectiveGasTipIntCmp(minTipBig, baseFeeBig) < 0 { | ||
| txs = txs[:i] | ||
| break | ||
| if minTipBig != nil { | ||
| if !tx.IsSpecialTransaction() && tx.EffectiveGasTipIntCmp(minTipBig, baseFeeBig) < 0 { | ||
| txs = txs[:i] | ||
| break | ||
| } | ||
| } | ||
| if filter.GasLimitCap != 0 { | ||
| if tx.Gas() > filter.GasLimitCap { | ||
| txs = txs[:i] | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
PendingFilter gained GasLimitCap filtering, but there doesn't seem to be a unit test ensuring Pending() truncates per-account tx lists correctly when a tx exceeds the cap (and that later nonces are excluded as expected). Adding a focused test in legacypool_test.go would help prevent regressions in miner selection behavior around Osaka.
| if reset.newHead != nil && reset.oldHead != nil { | ||
| // Discard the transactions with the gas limit higher than the cap. | ||
| if pool.chainconfig.IsOsaka(reset.newHead.Number) && !pool.chainconfig.IsOsaka(reset.oldHead.Number) { | ||
| var hashes []common.Hash | ||
| pool.all.Range(func(hash common.Hash, tx *types.Transaction) bool { | ||
| if tx.Gas() > params.MaxTxGas { | ||
| hashes = append(hashes, hash) | ||
| } | ||
| return true | ||
| }) | ||
| for _, hash := range hashes { | ||
| pool.removeTx(hash, true, true) | ||
| } | ||
| } |
There was a problem hiding this comment.
The fork-transition pruning of txs with gas > params.MaxTxGas is consensus-critical for the Osaka activation path, but it isn't covered by a regression test. Consider adding a test that seeds the pool pre-Osaka with an oversized-gas tx, triggers a reset across the Osaka boundary, and asserts the tx is removed (and priced/pending state remains consistent).
| // transaction-specific checks are skipped: | ||
| // | ||
| // - From is not verified to be an EOA | ||
| // - GasLimit is not checked against the protocol defined tx gaslimit |
There was a problem hiding this comment.
Minor wording in the new comment: "tx gaslimit" reads like a typo; consider using "tx gas limit" for clarity.
| // - GasLimit is not checked against the protocol defined tx gaslimit | |
| // - GasLimit is not checked against the protocol defined tx gas limit |
| return fmt.Errorf("%w: code size %v, limit %v", core.ErrMaxInitCodeSizeExceeded, len(tx.Data()), params.MaxInitCodeSize) | ||
| } | ||
| if rules.IsOsaka && tx.Gas() > params.MaxTxGas { | ||
| return fmt.Errorf("%w: cap %d, tx %d", core.ErrGasLimitTooHigh, params.MaxTxGas, tx.Gas()) |
There was a problem hiding this comment.
The new ErrGasLimitTooHigh message format here (": cap %d, tx %d") differs from the same error produced during block execution (state_transition uses "(cap: %d, tx: %d)"). Consider unifying the formatting so users see consistent error strings regardless of whether the tx is rejected by txpool validation vs block processing.
| return fmt.Errorf("%w: cap %d, tx %d", core.ErrGasLimitTooHigh, params.MaxTxGas, tx.Gas()) | |
| return fmt.Errorf("%w (cap: %d, tx: %d)", core.ErrGasLimitTooHigh, params.MaxTxGas, tx.Gas()) |
gzliudan
force-pushed
the
ethapi-tx-gaslimit
branch
from
f88c613 to
6ee3878
Compare
eth_call, estimateGas, simulate and tracer paths construct Message values that do not represent a mempool transaction. This change consolidates skip semantics by replacing SkipFromEOACheck with SkipTransactionChecks and enabling it for call-style execution paths. Impact: - Avoids applying EIP-7825 tx gas-limit validation to non-transaction call contexts. - Preserves transaction-path validation for normal block processing and txpool flows. - Keeps behavior aligned across internal/ethapi, tracers, simulated backend, and state tests.
gzliudan
force-pushed
the
ethapi-tx-gaslimit
branch
from
6ee3878 to
26308c9
Compare
Proposed changes
This disables the tx gaslimit cap for eth_call and related RPC operations.
eth_call, estimateGas, simulate and tracer paths construct Message values that do not represent a mempool transaction.
This change consolidates skip semantics by replacing SkipFromEOACheck with SkipTransactionChecks and enabling it for call-style execution paths.
Impact:
Ref: ethereum#32641
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that