Skip to content

[codex] add dynamic custom precompile gas meter#3644

Open
codchen wants to merge 1 commit into
codex/evmonly-staking-precompilefrom
codex/evmonly-staking-dynamic-gas
Open

[codex] add dynamic custom precompile gas meter#3644
codchen wants to merge 1 commit into
codex/evmonly-staking-precompilefrom
codex/evmonly-staking-dynamic-gas

Conversation

@codchen

@codchen codchen commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds dynamic gas accounting for evm-only custom precompiles, with the staking precompile using the shared meter through the existing store, balance-transfer, and log boundaries.

Changes

  • Add a reusable precompile gas meter for base gas, SLOAD/SSTORE-style storage costs, keccak slot derivation, native value transfers, and log emission.
  • Wrap storageBackedStore, nativeBalanceTransfer, and log emission in the custom precompile adapter so gas is charged based on the actual execution path.
  • Reduce staking RequiredGas to only base/input gas, avoiding double charging now that state access is dynamically metered.
  • Add executor coverage for dynamic store gas and update staking lifecycle tests to use realistic gas limits.

Validation

  • go test ./giga/evmonly/...
  • golangci-lint v2.8.0 run ./giga/evmonly/...

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.66851% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.15%. Comparing base (04870c2) to head (9cb00d6).

Files with missing lines Patch % Lines
giga/evmonly/precompile_gas.go 68.46% 23 Missing and 18 partials ⚠️
giga/evmonly/precompile_adapter.go 34.69% 19 Missing and 13 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@                         Coverage Diff                          @@
##           codex/evmonly-staking-precompile    #3644      +/-   ##
====================================================================
- Coverage                             58.16%   58.15%   -0.01%     
====================================================================
  Files                                  2189     2187       -2     
  Lines                                178615   178718     +103     
====================================================================
+ Hits                                 103884   103939      +55     
- Misses                                65424    65454      +30     
- Partials                               9307     9325      +18     
Flag Coverage Δ
sei-chain-pr 56.85% <59.66%> (-0.13%) ⬇️
sei-db 70.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
giga/evmonly/precompiles/staking/staking.go 23.91% <100.00%> (-0.40%) ⬇️
giga/evmonly/precompile_adapter.go 66.35% <34.69%> (-8.65%) ⬇️
giga/evmonly/precompile_gas.go 68.46% <68.46%> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJul 2, 2026, 9:53 AM

@codchen codchen marked this pull request as ready for review June 26, 2026 03:42
@cursor

cursor Bot commented Jun 26, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes consensus-relevant gas accounting for custom precompiles and staking calls; incorrect metering could cause unexpected OOG or undercharging on state-heavy operations.

Overview
Custom precompile execution now charges gas dynamically for work done inside the adapter, not only the upfront RequiredGas amount.

RunAndCalculateGas uses a shared precompileGasMeter that deducts base gas, emits tracing hooks, and threads the meter into storageBackedStore, nativeBalanceTransfer, and a meteredLogSink. Store access bills Keccak for slot derivation plus EIP-2929/EIP-2200-style SLOAD/SSTORE (including refunds); transfers bill account access and value transfer; logs bill standard LOG costs. Out-of-gas from the meter surfaces as vm.ErrOutOfGas after the precompile returns.

The staking precompile drops fixed read/write gas in RequiredGas in favor of base + per-input-byte pricing so state-heavy paths are not double-charged. Tests assert store gas exceeds intrinsic cost, OOG on low gas limits leaves storage unchanged, and staking flows use higher tx gas limits.

Reviewed by Cursor Bugbot for commit 35864cf. Bugbot is set up for automated code reviews on this repo. Configure here.

@codchen codchen force-pushed the codex/evmonly-staking-precompile branch from 27e3acc to 550f6fa Compare June 29, 2026 12:34
@codchen codchen force-pushed the codex/evmonly-staking-dynamic-gas branch from 56bf186 to bd57e4c Compare June 29, 2026 12:35

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bd57e4c. Configure here.

Comment thread giga/evmonly/precompile_gas.go

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The dynamic precompile gas meter is implemented carefully and faithfully mirrors go-ethereum's EIP-2929/2200/3529 SLOAD/SSTORE/refund accounting, with refund state being snapshot-safe and OOG correctly surfaced and discarded; tests cover the new metering. The only issue is a now-orphaned helper function and a behavioral note about read-method gas.

Findings: 0 blocking | 4 non-blocking | 1 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • isTransaction in giga/evmonly/precompiles/staking/helpers.go was the only consumer of the removed read/write gas split and is now unused dead code (no remaining references in the package). Remove it — golangci-lint's unused check may flag this and fail CI even though the PR notes lint passed.
  • Behavior change worth noting: read-only query methods (validators, delegatorDelegations, etc.) previously cost a flat readGas (3000) and now meter per-SLOAD dynamically. Read-heavy paged queries (up to pageLimit=100 validators, each a multi-chunk read) can consume substantially more gas than before; callers/eth_call gas caps must account for this. This is more accurate, just a notable semantics change.
  • Codex and Cursor second-opinion passes both produced no material findings.
  • 1 suggestion(s)/nit(s) flagged inline on specific lines.

gas = writeGas
}
return gas + inputByteGas*uint64(len(input)) //nolint:gosec // input length is bounded by memory.
return baseGas + inputByteGas*uint64(len(input)) //nolint:gosec // input length is bounded by memory.

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] With the read/write gas split removed here, isTransaction in helpers.go is no longer referenced anywhere in the package and is now dead code. Consider removing it — the unused linter may otherwise fail CI.

@codchen codchen force-pushed the codex/evmonly-staking-precompile branch from 550f6fa to accacbf Compare June 30, 2026 06:11
@codchen codchen force-pushed the codex/evmonly-staking-dynamic-gas branch from bd57e4c to 7c6c44c Compare June 30, 2026 06:11
seidroid[bot]
seidroid Bot previously requested changes Jun 30, 2026

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The PR adds a dynamic gas meter for evm-only custom precompiles whose SLOAD/SSTORE/access-list/keccak/log/transfer pricing faithfully mirrors go-ethereum (EIP-2200/2929/3529), with refund and access-list mutations correctly rolled back on OOG via RevertToSnapshot and good test coverage. The metering relies on GetCommittedState as the SSTORE "original" value, but nativeStateDB.base is block-start state that Finalise never advances, so SSTORE cost/refunds are mispriced for slots written across multiple transactions in the same block.

Findings: 1 blocking | 2 non-blocking | 1 posted inline

Blockers

  • None at the file/PR level.
  • 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • cursor-review.md was empty — the Cursor second-opinion pass produced no output, so its perspective is not reflected here. (REVIEW_GUIDELINES.md was also empty; reviewed without repo-specific guidance.)
  • Minor: chargeNativeTransfer charges account-access (cold/warm) for both from and to. Standard EVM CALL gas does not recharge access for the caller/from (it is already warm), so transfers slightly overcharge relative to geth. Likely acceptable as a custom model, but worth confirming it is intentional.

if current == value {
return m.charge(gasAdd(cost, params.WarmStorageReadCostEIP2929), tracing.GasChangeCallPrecompiledContract)
}
original := db.GetCommittedState(addr, slot)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[blocker] GetCommittedState is used as the EIP-2200 "original" (transaction-start) value, but in this nativeStateDB the base map holds block-start state and is never advanced between transactions — Finalise (state_db.go:492) only clears selfdestruct/created flags and resets refund, and account()/baseAccount() populate base lazily from source (block start). The executor reuses one stateDB for the whole block (executor.go:80), so for a slot written in tx N and re-written/cleared in tx N+1 of the same block, original here is the block-start value rather than the tx N+1-start value.

This diverges from go-ethereum (where per-tx Finalise moves dirty storage into pending storage so GetCommittedState reflects prior txs in the block), causing SSTORE to take the wrong SET/RESET/dirty branch and emit incorrect AddRefund/SubRefund amounts (under/over-charging and bogus clear refunds). Before this PR the flat read/write gas hid the issue; the dynamic meter now makes it consensus-relevant for gas accounting. Consider tracking a separate transaction-start snapshot (advanced in Finalise) distinct from the block-start base used by ChangeSet.

@codchen codchen force-pushed the codex/evmonly-staking-precompile branch from accacbf to 93a6625 Compare July 2, 2026 05:52
@codchen codchen force-pushed the codex/evmonly-staking-dynamic-gas branch from 7c6c44c to 6f9547a Compare July 2, 2026 05:53

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Adds a dynamic gas meter for evm-only custom precompiles; the SSTORE/SLOAD/refund/transfer/log accounting faithfully mirrors go-ethereum's EIP-2929/3529 rules, is overflow-guarded, and correctly reverts (refund/access-list/logs/storage) on out-of-gas, with new test coverage. No blocking issues found; a few non-blocking notes.

Findings: 0 blocking | 5 non-blocking | 0 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • Cursor's second-opinion review (cursor-review.md) is empty — that pass produced no output.
  • Codex (codex-review.md) reported no material findings but could not run go test ./giga/evmonly/... because the sandbox had Go 1.24.13 while go.mod requires 1.25.6; its pass was therefore lint/inspection-only.
  • Design fragility (not a bug today): when a storage/transfer/log charge fails mid-execution, storageBackedStore.Get/Set/Delete and meteredLogSink.AddLog silently no-op without surfacing the out-of-gas to the precompile. Correctness currently relies entirely on RunAndCalculateGas observing meter.err and the caller reverting. A future precompile that branches on a Get() result after OOG would compute on stale/empty data (e.g. treating an existing key as absent). Consider surfacing the OOG through the Store/LogSink interfaces so the contract short-circuits deterministically.
  • Tracing cosmetics: warm SLOAD reads, all SSTORE variants, native transfers, and log emission report tracing.GasChangeCallPrecompiledContract rather than storage/log-specific reasons (only cold accesses use GasChangeCallStorageColdAccess). This only affects tracer output, but it makes gas-change traces harder to interpret.
  • Gas scaling observation: metering now makes query/write methods cost proportional to the store index lists they touch (e.g. getStringList-backed indices rewritten on delegation), which is why lifecycle tests needed 8M gas. This is inherent to the existing store design rather than introduced here, but worth confirming the per-op costs are acceptable for large validator/delegator sets.

@seidroid seidroid Bot dismissed their stale review July 2, 2026 06:03

Superseded: latest AI review found no blocking issues.

@codchen codchen force-pushed the codex/evmonly-staking-precompile branch from 93a6625 to 8548c28 Compare July 2, 2026 08:38
@codchen codchen force-pushed the codex/evmonly-staking-dynamic-gas branch from 6f9547a to 31f16a8 Compare July 2, 2026 08:39

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Adds a dynamic gas meter for evm-only custom precompiles and rewires staking's state/balance/log boundaries through it, replacing static read/write gas with EIP-2929/2200/3529-style accounting. The gas math faithfully mirrors go-ethereum, refunds flow through the existing GetRefund path (core applies the 3529 cap), and the OOG path is tested end-to-end; no blocking issues, only minor notes.

Findings: 0 blocking | 7 non-blocking | 2 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • Cursor's second-opinion review file (cursor-review.md) is empty — that pass produced no output. Codex (codex-review.md) reported "No material findings."
  • Because the staking precompile is SDK-free and performs all state access through ctx.Store/ctx.Balances/ctx.Logs, the reduction of RequiredGas to baseGas (700) is coherent — every read/write is now dynamically metered. Worth confirming there is no un-metered path (e.g. large in-memory work in list/pagination methods like validators()/redelegations() that iterate index lists) that could be underpriced relative to its cost; today only the storage touches are charged, not the O(n) decode/filter loops.
  • On out-of-gas, storageBackedStore.Get returns (nil,false) and Set/Delete return silently, relying entirely on RunAndCalculateGas checking meter.err afterward. This is correct and tested for storeWritePrecompile/staking, but it is a fragile contract: a precompile that branches on a read result (now silently empty on OOG) before the adapter re-checks meter.err could take an unintended code path. Consider a doc comment on the pattern and/or a test that OOGs mid-read.
  • Nit: most charges (warm SLOAD, native-transfer sub-costs, log gas, keccak) all pass reason tracing.GasChangeCallPrecompiledContract rather than warm/cold-specific reasons; purely cosmetic for tracers but slightly imprecise.
  • Nit: chargeNativeTransfer charges account-access for from as well as to; real CALL semantics treat the caller as already warm. In practice from is usually warmed via Prepare so it charges the warm cost, but the extra access is a conservative (safe) over-charge worth a brief comment.
  • 2 suggestion(s)/nit(s) flagged inline on specific lines.

}

func (s storageBackedStore) Get(key []byte) ([]byte, bool) {
if !s.chargeStoreBaseSlot(key) {

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] Silent no-op on out-of-gas: when chargeStoreBaseSlot (or a later charge) fails, Get returns (nil,false) — indistinguishable from "key not found" — and Set/Delete return without applying the write. Correctness here depends entirely on RunAndCalculateGas re-checking meter.err after run() returns (which it does). It works and is tested, but it's a subtle invariant: any future precompile that acts on a read's result before returning would see empty data on OOG. A short comment documenting this contract would help.

}

func (m *precompileGasMeter) chargeSStore(db *nativeStateDB, addr common.Address, slot common.Hash, value common.Hash) bool {
if m.remaining <= params.SstoreSentryGasEIP2200 {

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] chargeSStore does not short-circuit on an already-set m.err before running the sentry check and mutating the access list / refund counter. In the current call ordering this is unreachable (chargeStoreBaseSlot's keccak charge fails first once m.err is set, so Set/Delete return before reaching chargeSStore), but adding an early if m.err != nil { return false } here would make the invariant robust to future refactors, since AddSlotToAccessList/AddRefund side effects run before the final charge.

@codchen codchen force-pushed the codex/evmonly-staking-precompile branch from 8548c28 to 04870c2 Compare July 2, 2026 09:51
@codchen codchen force-pushed the codex/evmonly-staking-dynamic-gas branch from 31f16a8 to 9cb00d6 Compare July 2, 2026 09:51

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Adds a dynamic gas meter for evm-only custom precompiles that faithfully reimplements EIP-2929/2200/3529 storage, keccak, transfer, and log costs; the OOG path reverts state correctly via snapshot and the staking base-gas reduction is safely compensated by per-slot metering. No blocking issues found; a couple of non-blocking notes around dedicated unit-test coverage and a subtle Store return-value semantic.

Findings: 0 blocking | 4 non-blocking | 1 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • The new precompileGasMeter reimplements consensus-sensitive EIP-2929/2200/3529 SSTORE/SLOAD gas and refund logic, but there is no dedicated unit test for it. Existing coverage only exercises one OOG path and the staking lifecycle. Consider adding targeted tests for the SSTORE transitions (cold vs warm, create/reset/clear, dirty re-write, refund add/sub, and the SstoreSentry boundary) so future edits to this logic are guarded.
  • Cursor's second-opinion review (cursor-review.md) is empty — that pass produced no output. Codex's review (codex-review.md) reported “No material findings.” Flagging so the empty Cursor pass isn't mistaken for an implicit approval.
  • Gas is charged before the corresponding s.db.SetState in every store mutation and the whole tx reverts on OOG, which is correct; worth a brief code comment documenting that invariant given how central it is to consensus safety.
  • 1 suggestion(s)/nit(s) flagged inline on specific lines.

}

func (s storageBackedStore) Get(key []byte) ([]byte, bool) {
if !s.chargeStoreBaseSlot(key) {

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] When a gas charge fails mid-read, Get returns (nil, false) — the same signal as "key not found". A precompile could therefore branch as if the key were absent rather than aborting. This is safe here only because meter.err is set and RunAndCalculateGas unconditionally converts a set meter.err into vm.ErrOutOfGas, overriding any success the contract subsequently returns. Worth a short comment noting this conflation is intentionally masked by the meter, so a future refactor doesn't accidentally rely on the false meaning "not found" after an OOG.

@codchen codchen force-pushed the codex/evmonly-staking-precompile branch from 04870c2 to 83c1b62 Compare July 2, 2026 12:35
@codchen codchen force-pushed the codex/evmonly-staking-dynamic-gas branch from 9cb00d6 to 35864cf Compare July 2, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant