Skip to content

Add optional state-transition gas surcharge (CON-300) #99

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.

Sign up for GitHub

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

Closed
wen-coding wants to merge 4 commits into from
Closed

Add optional state-transition gas surcharge (CON-300) #99

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8174170
Add optional non-refundable gas surcharge on state transition.
wen-coding Sep 15, 2026
18d1bd6
Point surcharge Execute tests at a non-precompile recipient.
wen-coding Sep 15, 2026
5213511
Assign GasChangeTxAutoAssociation 0x80 so it does not collide with up…
wen-coding Sep 15, 2026
321a5f7
Document WithGasSurcharge caller contract and admit the Prague floor …
wen-coding Sep 15, 2026
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
41 changes: 33 additions & 8 deletions core/state_transition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ type StateTransition struct {
msg *Message
gasRemaining uint64
initialGas uint64
gasSurcharge uint64
gasSurchargeReason tracing.GasChangeReason
state vm.StateDB
evm *vm.EVM
feeCharged bool
Expand All @@ -253,6 +255,15 @@ func NewStateTransition(evm *vm.EVM, msg *Message, gp *GasPool, feeCharged bool,
}
}

// WithGasSurcharge reserves non-refundable gas before intrinsic gas and EVM execution.
// The surcharge is included in UsedGas and fees, and excluded from refund caps and data-floor gas.
// reason is the tracer category emitted when the surcharge is reserved.
func (st *StateTransition) WithGasSurcharge(gas uint64, reason tracing.GasChangeReason) *StateTransition {

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] The doc comment covers the accounting well but omits the usage contract a caller needs: it must be called before Execute(), it applies only to this top-level transition (ApplyMessage and nested calls are unaffected), and a gas limit below the surcharge surfaces as ErrIntrinsicGas. Worth stating, since the only caller lives outside this repo.

st.gasSurcharge = gas
st.gasSurchargeReason = reason
return st
}

// to returns the recipient of the message.
func (st *StateTransition) to() common.Address {
if st.msg == nil || st.msg.To == nil /* contract creation */ {
Expand Down Expand Up @@ -440,6 +451,15 @@ func (st *StateTransition) Execute() (*ExecutionResult, error) {
if err := st.preCheck(); err != nil {
return nil, err
}
if st.gasRemaining < st.gasSurcharge {

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Reusing ErrIntrinsicGas for a surcharge shortfall produces the self-contradictory message "intrinsic gas too low: have 999, want surcharge 1000", and makes surcharge failures indistinguishable from real intrinsic-gas failures for any errors.Is(err, ErrIntrinsicGas) consumer (txpool, RPC error mapping, metrics). A dedicated sentinel — e.g. ErrGasSurcharge in core/error.go, optionally wrapping ErrIntrinsicGas for backwards compatibility — would let callers tell the two apart.

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Reusing ErrIntrinsicGas for a surcharge shortfall makes the two failures indistinguishable to callers — RPC error mapping, txpool rejection reasons, and metrics all see "intrinsic gas too low" for a condition the sender cannot fix by looking at intrinsic cost. Consider a dedicated ErrGasSurcharge that wraps ErrIntrinsicGas (fmt.Errorf("%w: ...", ErrIntrinsicGas) already gives errors.Is compatibility, so a named sentinel costs nothing) so Sei's integration can report the actual cause.

return nil, fmt.Errorf("%w: have %d, want surcharge %d", ErrIntrinsicGas, st.gasRemaining, st.gasSurcharge)

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Reusing ErrIntrinsicGas conflates "gas limit below the node-injected surcharge" with a genuine intrinsic-gas shortfall — callers (txpool, RPC error mapping) can't tell them apart, and the message text (want surcharge N) is the only signal. Consider a distinct sentinel (e.g. ErrGasSurcharge) wrapping/alongside it. Note this is a consensus-level rejection of the whole transaction, not a failed execution, which is why distinguishing it matters for admission control.

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Reusing ErrIntrinsicGas renders as intrinsic gas too low: have N, want surcharge M, which reads oddly and is indistinguishable from a genuine intrinsic-gas shortfall for callers matching on the sentinel. Reusing the class is a reasonable deliberate choice (it keeps the miner skipping the tx rather than treating the block as invalid), so if that's intentional a one-line comment saying so would help; otherwise a dedicated sentinel wrapping ErrIntrinsicGas would let callers tell them apart.

}
if st.gasSurcharge > 0 {

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] This emits OnGasChange unconditionally on the caller-supplied reason, including when the caller passes tracing.GasChangeIgnored. Elsewhere (core/vm/contract.go:133,145) GasChangeIgnored means "suppress the hook, the change is emitted manually." Consider honoring that convention here for consistency: if reason != tracing.GasChangeIgnored.

if t := st.evm.Config.Tracer; t != nil && t.OnGasChange != nil {
t.OnGasChange(st.gasRemaining, st.gasRemaining-st.gasSurcharge, st.gasSurchargeReason)

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] The surcharge OnGasChange fires before the intrinsic-gas check (line 476) and the floor-data check (line 486), so a tracer observes the surcharge deduction for transactions that are then rejected and never execute. Upstream's GasChangeTxInitialBalance in initGas has the same property, so this is consistent — but if you want the hook to only reflect executed transactions, moving the emission after the two admission checks (keeping the gasRemaining subtraction where it is) would do it.

}
st.gasRemaining -= st.gasSurcharge
}

var (
msg = st.msg
Expand All @@ -462,8 +482,9 @@ func (st *StateTransition) Execute() (*ExecutionResult, error) {
if err != nil {
return nil, err
}
if msg.GasLimit < floorDataGas {
return nil, fmt.Errorf("%w: have %d, want %d", ErrFloorDataGas, msg.GasLimit, floorDataGas)
executionGasLimit := msg.GasLimit - st.gasSurcharge

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Prefer st.initialGas - st.gasSurcharge here (or st.gasRemaining + gas). The no-underflow guarantee for line 564 (st.initialGas - st.gasSurcharge - floorDataGas) comes from this admission check, but the check is written against msg.GasLimit while line 564 uses st.initialGas. They're equal only because initGas() sets initialGas = msg.GasLimit; sourcing both from initialGas makes the invariant local and keeps the pair correct if that ever changes.

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] msg.GasLimit - st.gasSurcharge recomputes a value that st.gasRemaining already holds: initGas sets gasRemaining = msg.GasLimit, the surcharge is subtracted at line 461, and intrinsic gas is not subtracted until line 493. Using st.gasRemaining directly removes the duplicated subtraction (and the need to re-derive the no-underflow argument from the line 454 guard).

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] msg.GasLimit - st.gasSurcharge recomputes a value st.gasRemaining already holds: initGas() sets gasRemaining = msg.GasLimit, and the surcharge was subtracted at line 461, so the two are equal here. Using st.gasRemaining directly keeps one source of truth, matches the intrinsic-gas check on line 476, and stays correct if initGas() ever stops setting gasRemaining == msg.GasLimit (at which point this expression could underflow, since the guard on line 454 is against gasRemaining, not msg.GasLimit).

if executionGasLimit < floorDataGas {
return nil, fmt.Errorf("%w: have %d, want %d", ErrFloorDataGas, executionGasLimit, floorDataGas)
}
}
if t := st.evm.Config.Tracer; t != nil && t.OnGasChange != nil {
Expand Down Expand Up @@ -538,9 +559,9 @@ func (st *StateTransition) Execute() (*ExecutionResult, error) {
st.gasRemaining += gasRefund
if rules.IsPrague {
// After EIP-7623: Data-heavy transactions pay the floor gas.
if st.gasUsed() < floorDataGas {
if st.executionGasUsed() < floorDataGas {
prev := st.gasRemaining
st.gasRemaining = st.initialGas - floorDataGas
st.gasRemaining = st.initialGas - st.gasSurcharge - floorDataGas
if t := st.evm.Config.Tracer; t != nil && t.OnGasChange != nil {
t.OnGasChange(prev, st.gasRemaining, tracing.GasChangeTxDataFloor)
}
Expand Down Expand Up @@ -647,11 +668,11 @@ func (st *StateTransition) applyAuthorization(auth *types.SetCodeAuthorization)
func (st *StateTransition) calcRefund() uint64 {
var refund uint64
if !st.evm.ChainConfig().IsLondon(st.evm.Context.BlockNumber) {
// Before EIP-3529: refunds were capped to gasUsed / 2
refund = st.gasUsed() / params.RefundQuotient
// Before EIP-3529: refunds were capped to execution gas / 2
refund = st.executionGasUsed() / params.RefundQuotient
} else {
// After EIP-3529: refunds are capped to gasUsed / 5
refund = st.gasUsed() / params.RefundQuotientEIP3529
// After EIP-3529: refunds are capped to execution gas / 5
refund = st.executionGasUsed() / params.RefundQuotientEIP3529
}
if refund > st.state.GetRefund() {
refund = st.state.GetRefund()
Expand All @@ -662,6 +683,10 @@ func (st *StateTransition) calcRefund() uint64 {
return refund
}

func (st *StateTransition) executionGasUsed() uint64 {
return st.gasUsed() - st.gasSurcharge

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] executionGasUsed underflows if it is ever reached before the surcharge has been deducted from gasRemaining. Both current call sites (refund cap, Prague floor) are post-deduction and safe — refunds are bounded by executionGasUsed/5, so the post-refund gasRemaining still cannot exceed initialGas - gasSurcharge — but a one-line comment stating that precondition would keep a future early-return from silently wrapping around.

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] Worth a doc comment stating the invariant, since a violation wraps silently rather than panicking. The subtraction is safe today because calcRefund caps the refund at executionGasUsed()/quotient, which keeps gasRemaining <= initialGas - gasSurcharge at both call sites (line 562 and line 672/675) — but that reasoning is non-local and easy to break with a future refund change. Something like:

// executionGasUsed returns the gas consumed by intrinsic cost and EVM execution,
// excluding the non-refundable surcharge. Safe from underflow because gasRemaining
// never exceeds initialGas-gasSurcharge after the surcharge is reserved.

@seidroid seidroid Bot Sep 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] This subtraction is unguarded. It is safe today — Execute always deducts the surcharge before calcRefund or the Prague floor branch can run, and the check at line 456 guarantees gasRemaining >= gasSurcharge — but if a future path ever reaches calcRefund without the deduction, this wraps to ~2^64 and the EIP-3529 refund cap silently disappears, which is a consensus-visible failure with no error. Either clamp (if st.gasUsed() < st.gasSurcharge { return 0 }) or state the invariant in the comment so the dependency on ordering is explicit.

}

// returnGas returns ETH for remaining gas,
// exchanged at the original rate.
func (st *StateTransition) returnGas() {
Expand Down
Loading
Loading