[codex] add dynamic custom precompile gas meter#3644
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## codex/evmonly-staking-precompile #3644 +/- ##
====================================================================
- Coverage 58.15% 58.15% -0.01%
====================================================================
Files 2187 2185 -2
Lines 178461 178564 +103
====================================================================
+ Hits 103781 103836 +55
- Misses 65383 65413 +30
- Partials 9297 9315 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
PR SummaryMedium Risk Overview
The staking precompile drops fixed read/write gas in Tests assert higher gas use on successful store writes, OOG with no storage when gas is too low, and staking flows use realistic gas limits (8M). Reviewed by Cursor Bugbot for commit bd57e4c. Bugbot is set up for automated code reviews on this repo. Configure here. |
27e3acc to
550f6fa
Compare
56bf186 to
bd57e4c
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ 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.
There was a problem hiding this comment.
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
isTransactioningiga/evmonly/precompiles/staking/helpers.gowas 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'sunusedcheck 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 flatreadGas(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. |
There was a problem hiding this comment.
[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.
550f6fa to
accacbf
Compare
bd57e4c to
7c6c44c
Compare
There was a problem hiding this comment.
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.mdwas 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:
chargeNativeTransfercharges account-access (cold/warm) for bothfromandto. 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) |
There was a problem hiding this comment.
[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.

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
storageBackedStore,nativeBalanceTransfer, and log emission in the custom precompile adapter so gas is charged based on the actual execution path.RequiredGasto only base/input gas, avoiding double charging now that state access is dynamically metered.Validation
go test ./giga/evmonly/...golangci-lint v2.8.0 run ./giga/evmonly/...