feat(engine): support targetGasLimit in PayloadAttributesV4#11611
feat(engine): support targetGasLimit in PayloadAttributesV4#11611LukaszRozmej wants to merge 7 commits into
Conversation
Implements ethereum/execution-apis#796, which adds a required `targetGasLimit` field to `PayloadAttributesV4` so the CL can supply the per-FCU gas-limit target instead of relying on the EL's static configuration. - `PayloadAttributes.TargetGasLimit` is included in `ToString`, the payload-id hash input, version inference, and required-field validation for V4 attributes. - `IGasLimitCalculator` gains an overload accepting an optional target override; `TargetAdjustedGasLimitCalculator` honors it, taking precedence over `BlocksConfig.TargetBlockGasLimit`. `AuRaContractGasLimitOverride` forwards the override to its inner calculator. - `BlockProducerBase` and `TestingRpcModule` pass the per-call target into the calculator so the standard gas-limit adjustment applies.
|
Claude finished @LukaszRozmej's task in 4m 20s —— View job PR Review: feat(engine): support targetGasLimit in PayloadAttributesV4Tasks:
SummaryThe overall approach is sound and well-structured. Medium1. Unchecked
2. When Low3. No tests for The existing Looks good
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Collapse `IGasLimitCalculator.GetGasLimit` to a single method with `long? targetGasLimitOverride = null` default; update all implementations (TargetAdjusted, AuRa, FollowOtherMiners, Optimism, Xdc, ManualGasLimitCalculator). - Reject `TargetGasLimit > long.MaxValue` in `PayloadAttributes.Validate` to prevent silent wrap of the `ulong? -> long?` narrowing. Cast sites in BlockProducerBase / TestingRpcModule now use a `checked` cast for defense-in-depth on paths that bypass validation. - AuRa contract override: when the contract returns a limit and the CL also supplies a `targetGasLimit`, log a warning so operators know the contract takes precedence; otherwise forward the override to the inner calculator. - Add focused unit tests for AuRa: contract wins (override ignored) and no-contract (override forwarded). Add a V4 validation case for `TargetGasLimit > Int64.MaxValue`.
- Type `TargetGasLimit` as `long?` directly (was `ulong?`), dropping the narrowing cast and the upper-bound `> long.MaxValue` validation. Gas limits in Nethermind are already stored as `long`. - Consolidate the parameterless `PayloadAttributes.GetGasLimit()` and the duplicated `payloadAttributes?` access at the call site into a single `GetGasLimit(BlockHeader parent, IGasLimitCalculator calc)`. Base implementation routes `TargetGasLimit` through the calculator; Boost/Optimism/Taiko override to return their unadjusted value. - AuRa: drop the conflict warning and flip precedence — the CL's per-FCU `targetGasLimit` now takes priority over the on-chain contract value, with the contract acting as fallback when no override is supplied. - Dedup `TargetAdjustedGasLimitCalculatorTests` and the new AuRa override tests around small construction helpers.
Align with the spec field name (`PayloadAttributesV4.targetGasLimit`). The "override" suffix was redundant — a non-null value already means override behavior. Inside `TargetAdjustedGasLimitCalculator`, reuse the parameter via `??=` instead of an extra local. Also drop the unused `using Nethermind.Xdc.Spec` in XdcGasLimitCalculator (left over from the previous expression-body collapse) — was failing the `IDE0005` lint check.
LukaszRozmej
requested review from
Marchhill,
MarekM25,
ak88,
benaadams,
cicr99,
dipkakwani,
emlautarom1,
flcl42 and
smartprogrammer93
as code owners
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
@claude review |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Review — PayloadAttributesV4
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
`AuRaContractGasLimitOverride.GetGasLimit` prefers the CL-supplied `targetGasLimit` (PayloadAttributesV4) over the on-chain contract value, but `IsGasLimitValid` — called from `AuRaBlockProcessor` — still strictly required the block to match the contract. On an AuRa chain with an active gas-limit contract that ever activates EIP-7843, a producer using the override path would emit a block its own validator rejects. Accept any block gas limit within parent ± delta when it differs from the contract value (HeaderValidator independently enforces the full consensus rule). Inject `ISpecProvider` so the bound divisor is read from the active release spec instead of being hardcoded.
`AuRaContractGasLimitOverride.IsGasLimitValid` now round-trips the candidate gas limit through `_innerCalculator` (a `TargetAdjustedGasLimitCalculator` in production): if the calculator returns the candidate unchanged it's within the standard parent ± delta range, otherwise it's been clamped. Drops the duplicated delta math and the `ISpecProvider` dependency added in the previous commit. Also strip the verbose XML doc comments added on `TargetGasLimit`, `PayloadAttributes.GetGasLimit`, `IGasLimitCalculator.GetGasLimit`, and the explanatory test comments.
|
Do we need to wait till on devnet? i.e. is this breaking change |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11611 +/- ##
===========================================
- Coverage 74.87% 0 -74.88%
===========================================
Files 2884 0 -2884
Lines 118618 0 -118618
Branches 17147 0 -17147
===========================================
- Hits 88815 0 -88815
+ Misses 26842 0 -26842
+ Partials 2961 0 -2961 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
your original description points to PR 796 from nethermind's repo instead of ethereum/execution-apis#796 |
3 participants
Changes
TargetGasLimittoPayloadAttributesper execution-apis Remove unnecessary allocations in FrameMacProcessor #796.Included in
ToString, payload-id hash input, version inference, andrequired-field validation for V4 attributes.
IGasLimitCalculatorgains an overload accepting an optional targetoverride;
TargetAdjustedGasLimitCalculatorhonors it (override takesprecedence over
BlocksConfig.TargetBlockGasLimit).AuRaContractGasLimitOverrideforwards the override to its innercalculator.
BlockProducerBaseandTestingRpcModulepass the per-call targetinto the calculator so the standard parent ± delta adjustment applies.
Implements the draft spec in ethereum/execution-apis#796.
Types of changes
Testing
required) in
EngineModuleTests.V1.targetGasLimitand refreshedexpected payload ids (plain + AuRa override).
TargetAdjustedGasLimitCalculatorTestscovers override precedenceand null-override fallback.
Documentation