[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.14% -0.01%
====================================================================
Files 2187 2185 -2
Lines 178444 178547 +103
====================================================================
+ Hits 103769 103824 +55
- Misses 65379 65409 +30
- Partials 9296 9314 +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. |
codchen
force-pushed
the
codex/evmonly-staking-precompile
branch
from
27e3acc to
550f6fa
Compare
codchen
force-pushed
the
codex/evmonly-staking-dynamic-gas
branch
from
56bf186 to
bd57e4c
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
| if value == (common.Hash{}) { | ||
| db.AddRefund(params.SstoreClearsScheduleRefundEIP3529) | ||
| } | ||
| return m.charge(gasAdd(cost, params.SstoreResetGasEIP2200-params.ColdSloadCostEIP2929), tracing.GasChangeCallPrecompiledContract) |
There was a problem hiding this comment.
SSTORE reset gas undercharged
High Severity
When chargeSStore prices a non-zero slot rewrite, it adds SstoreResetGasEIP2200 - ColdSloadCostEIP2929 instead of SstoreResetGasEIP2200 on top of the cold/warm access cost already in cost. Warm updates pay about 2900 gas instead of 5000; cold updates pay about 5000 instead of 7100, so precompile storage writes cost less than normal EVM SSTORE.
Reviewed by Cursor Bugbot for commit bd57e4c. Configure here.
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
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.
1 participant
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/...