feat(precompiles): expose module Query rpcs as precompile methods#3767
feat(precompiles): expose module Query rpcs as precompile methods#3767codchen wants to merge 4 commits into
Conversation
Add query counterparts for Cosmos module Query rpcs that had no EVM precompile surface: - bank: spendableBalances, totalSupply, params, denomMetadata, denomsMetadata - distribution: params, validatorOutstandingRewards, validatorCommission, validatorSlashes, delegationRewards, delegatorValidators, delegatorWithdrawAddress, communityPool - gov: proposal, proposals, vote, votes, params, deposit, deposits, tallyResult New query-only precompiles: auth (0x100D), authz (0x100E), evidence (0x100F), feegrant (0x1010), mint (0x1012), params (0x1013), slashing (0x1014), upgrade (0x1015). Querier interfaces and codec plumbing are added to precompiles/utils and wired in app/precompiles.go. Any-typed response fields (gov content, authz authorizations, evidence, feegrant allowances) are returned as registry-resolved JSON bytes for use with the json precompile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3767 +/- ##
==========================================
- Coverage 59.88% 59.01% -0.87%
==========================================
Files 2288 2217 -71
Lines 190023 181696 -8327
==========================================
- Hits 113786 107234 -6552
+ Misses 66091 64914 -1177
+ Partials 10146 9548 -598
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Overloading the vote/deposit transaction methods breaks tooling that resolves functions by name (ethers.js rejects ambiguous names with INVALID_ARGUMENT). Rename the query variants instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR SummaryMedium Risk Overview New query-only precompiles at fixed addresses ( Extended existing precompiles with read-only methods: bank (+5: spendable balances, paginated total supply, params, denom metadata), distribution (+8: params, validator rewards/commission/slashes, delegation rewards, delegator validators/withdraw address, community pool), and gov (+8 queries; Plumbing: Reviewed by Cursor Bugbot for commit 5be3ea2. Bugbot is set up for automated code reviews on this repo. Configure here. |
Register the auth, authz, evidence, feegrant, mint, params, slashing, and upgrade precompile addresses in giga's fail-fast list so calls to them abort and fall back to v2 instead of executing as empty accounts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR exposes many Cosmos module Query RPCs as EVM precompiles. The design and per-method implementations are largely solid and well-tested, but two consensus/gas issues block merge: (1) the new precompiles and new bank/gov/distribution methods are all registered at the existing head upgrade tag (v6.6) without a new tag or archival, so the same version has divergent behavior across binaries; and (2) the new gov query methods sit on the fixed-gas precompile path and undercharge for paginated work.
Findings: 4 blocking | 6 non-blocking | 3 posted inline
Blockers
- Consensus/app-hash divergence from missing version bump (matches Codex P1). All 8 new precompiles register only
latestUpgradein theirGetVersioned/setup.go, and the newviewmethods on bank/gov/distribution are added to the head implementation registered at the same latest tag.app/tagsalready ends atv6.6and this PR does not add a new tag, soLatestUpgrade==v6.6— an existing, already-live upgrade.GetCustomPrecompilesVersions(x/evm/keeper/keeper.go) resolves historical heights that are >= the v6.6 done-height to the v6.6 impl, so replaying past v6.6 blocks (or any node still on the old binary) now sees different precompile behavior (new addresses that were previously empty accounts; new bank/gov/distribution selectors that previously reverted) → app-hash mismatch. Per the repo'sbump_versionconvention (and the author's own 'Notes for reviewers'), this must ride a NEW upgrade tag (e.g. v6.7) with the current bank/gov/distribution head impls archived as legacy versions, and the new precompiles registered at that new tag. - The new gov query methods are underpriced (matches Codex P1 #2 and confirmed): unlike the 8 new precompiles which use
DynamicGasPrecompile(meter bounded from supplied EVM gas, actual work billed viaGetRemainingGas), gov is built with the fixed-gaspcommon.NewPrecompile, andRequiredGascharges query methods onlyDefaultGasCost(input, false)(calldata cost). Paginatedproposals/votes/depositsiterate up to the page limit and JSON-marshal per item for a flat EVM charge, letting a contract grief the block gas meter. Either migrate the gov query methods to the dynamic-gas path or charge gas proportional to work. - 2 blocking issue(s) flagged inline on specific lines.
Non-blocking
- Test coverage gap:
TestGetCustomPrecompilesin x/evm/keeper/keeper_test.go listsprecompileDirsexplicitly and omits all 8 new precompiles (auth, authz, evidence, feegrant, mint, params, slashing, upgrade), so theirversionsfiles are never validated by the consensus-version resolution test. Add the new directories. - gov
Execute(precompiles/gov/gov.go:130) is the only reviewed precompile whoseExecutelacks adefer recover(); the new query methods use bare type assertions (args[0].(uint64), and thevoteWeightedanonymous-struct assertion). A decoder/ABI mismatch would surface as a raw Go panic instead ofexecution revertedas elsewhere. Low probability given ABI-driven decoding, but inconsistent with every sibling precompile. - gov
getVote/getDepositand theproposalsvoter/depositor filters resolve the queried address viaGetSeiAddressFromArg, which errors for any address lacking an EVM association; votes/deposits cast by cosmos-native (unassociated) accounts cannot be queried and instead revert. Consider a read-path fallback to the casted address (asbank.accAddressFromArgdoes). - Pre-existing (not introduced here): distribution
rewards(dispatched at distribution.go:157) does not callValidateNonPayable, unlike every other query which routes throughvalidateInput; aCALLwith value>0 is silently accepted. Out of this PR's scope but worth a follow-up. - The Cursor second-opinion file (cursor-review.md) and REVIEW_GUIDELINES.md were both empty/absent — no Cursor pass output and no repo-specific guidelines were available to apply.
- 1 suggestion(s)/nit(s) flagged inline on specific lines.
| return 50000 | ||
| } else if !p.IsTransaction(method.Name) { | ||
| // query methods are charged the default read gas cost | ||
| return pcommon.DefaultGasCost(input, false) |
There was a problem hiding this comment.
[blocker] Blocker (gas underpricing): gov is built with the fixed-gas pcommon.NewPrecompile (line 99), so this query path charges only DefaultGasCost(input, false) (calldata cost) regardless of work done. The new paginated queries (proposals, votes, deposits) iterate up to the page limit and JSON-marshal each item, but that work is billed against the cosmos block gas meter and never reflected back into EVM gas. Every new precompile in this PR deliberately uses DynamicGasPrecompile + GetRemainingGas for exactly this reason. Migrate the gov query methods to the dynamic-gas path (or charge proportionally); as-is a contract can repeatedly call these for a flat ~1k gas and grief the block gas meter.
| } | ||
| } | ||
|
|
||
| func (p PrecompileExecutor) Execute(ctx sdk.Context, method *abi.Method, caller common.Address, callingContract common.Address, args []interface{}, value *big.Int, readOnly bool, evm *vm.EVM, hooks *tracing.Hooks) (bz []byte, err error) { |
There was a problem hiding this comment.
[nit] Nit/robustness: unlike every other precompile touched here, gov's Execute has no defer recover(). The new query methods rely on bare type assertions (args[0].(uint64), and voteWeighted's anonymous-struct assertion at ~line 216). Consider adding the same recover guard so an unexpected decode mismatch reverts as execution reverted rather than panicking.
| ) | ||
|
|
||
| func GetVersioned(latestUpgrade string, keepers utils.Keepers) utils.VersionedPrecompiles { | ||
| return utils.VersionedPrecompiles{ |
There was a problem hiding this comment.
[blocker] Blocker (versioning): this (and every new precompile's setup.go, plus the new bank/gov/distribution head methods) registers behavior only at latestUpgrade, which is the already-existing v6.6 tag (app/tags is unchanged by this PR). Since v6.6 is a past/live upgrade, GetCustomPrecompilesVersions will resolve historical v6.6 heights to this new impl, changing already-committed execution (new addresses, new selectors) → app-hash divergence between old and new binaries. These consensus-visible additions need a new upgrade tag with the current impls archived as legacy versions, per the bump_version flow.
Replace gov's static Precompile wrapper (fixed 30k/50k gas per tx method, flat read cost for queries) with the DynamicGasPrecompile pattern used by bank/staking/distribution, so execution is metered against actual Cosmos gas consumption via RunAndCalculateGas. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5be3ea2. Configure here.
| VotingEndTime: proposal.VotingEndTime.Unix(), | ||
| IsExpedited: proposal.IsExpedited, | ||
| Content: content, | ||
| }, nil |
There was a problem hiding this comment.
Gov unset voting times wrong
Medium Severity
The gov proposal and proposals query paths encode VotingStartTime and VotingEndTime with time.Time.Unix() on the stored proposal. While a proposal is still in the deposit period, those fields are unset zero time.Time values in state, and Unix() on that value is a large negative second count—not zero. EVM callers expecting “not started yet” to mean 0 get incorrect timestamps.
Reviewed by Cursor Bugbot for commit 5be3ea2. Configure here.
There was a problem hiding this comment.
A large, well-structured, and thoroughly-tested PR that exposes Cosmos module Query RPCs as EVM precompile methods; the precompile logic itself is correct across all reviewed files, but the consensus-visible changes are attached to the already-latest v6.6 head version instead of a new upgrade tag, which will diverge app-hashes during a normal binary rollout. This must be gated behind the next upgrade before merge.
Findings: 1 blocking | 3 non-blocking | 0 posted inline
Blockers
- Consensus fork risk from in-place v6.6 change. All new/extended precompile behavior is added to the head
NewPrecompile, whichSetCustomPrecompilesmaps tolatestUpgrade(v6.6) — and there is no frozenv66legacy snapshot (bank/gov/distribution legacy dirs stop atv65). At runtime, non-tracing execution always usesversioned[latestUpgrade](x/evm/keeper/keeper.go:166), so merging this changes what the already-released v6.6 upgrade does: nodes on the new binary would expose the new query methods and meter gov gas differently (static→DynamicGas migration) at the same v6.6 heights as nodes on the old binary, producing an app-hash divergence / chain halt during a routine rollout. Runscripts/bump_versionto cut the next tag (e.g. v6.7) so the current head is archived as the v6.6 legacy implementation and the new methods gate on the next upgrade height. The PR is labeledapp-hash-breakingand the author's own reviewer notes acknowledge this ("If v6.6 is already live, they should ride the next upgrade tag"); it must be resolved before merge. This spans every touched precompile: the 8 newversionsfiles (each a lonev6.6) and the extended bank/distribution/gov methods.
Non-blocking
- Second-opinion inputs:
cursor-review.mdandREVIEW_GUIDELINES.mdare both empty (no Cursor findings and no repo-specific guidelines were available for this pass); OpenAI Codex produced one finding, which matches the version-gating blocker above. - Consistency nit: the new gov query methods do not have the per-method
defer recover()that the distribution and the 8 new precompiles use, and neitherRunAndCalculateGasnorRunrecovers. It is harmless today (gov's assertions are on ABI-decoded args and its querier responses are value types with no nil derefs) and matches the pre-existing gov behavior, but adding a top-level recover in gov'sExecutewould make gas-meter/panic handling uniform across precompiles. - Documentation: several queries (auth
account, authzgrants/granterGrants/granteeGrants, feegrant, distribution delegation queries) require the queried EVM address to be association-mapped, so they cannot target Sei accounts that have no EVM association. This is consistent with existing precompiles but is a real limitation worth calling out in the Solidity NatSpec/user docs. (No security issue: all exposed params/state are already public on-chain.)
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ecommon.HexToAddress(pointerview.PointerViewAddress): pointerview.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(p256.P256VerifyAddress): p256.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(solo.SoloAddress): solo.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(auth.AuthAddress): auth.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(authz.AuthzAddress): authz.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(evidence.EvidenceAddress): evidence.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(feegrant.FeegrantAddress): feegrant.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(mint.MintAddress): mint.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(params.ParamsAddress): params.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(slashing.SlashingAddress): slashing.GetVersioned(latestUpgrade, keepers), | ||
| ecommon.HexToAddress(upgrade.UpgradeAddress): upgrade.GetVersioned(latestUpgrade, keepers), | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🔴 precompiles/setup.go wires all 8 new precompiles (auth, authz, evidence, feegrant, mint, params, slashing, upgrade) and the new bank/gov/distribution query methods only at latestUpgrade, which still resolves to the existing v6.6 tag since app/tags is unchanged by this PR. No legacy snapshot preserves the pre-PR v6.6 behavior, so if v6.6 has already produced committed blocks (as the PR's own reviewer notes and the app-hash-breaking label both suggest), a post-PR binary will replay those historical blocks with the new precompiles/methods active, producing a different app hash than a pre-PR binary — this is a consensus-breaking divergence, not merely a stylistic gap. Fix: cut a new upgrade tag (e.g. v6.7) and archive the current pre-PR implementations as v6.6 legacy versions via the bump_version flow before landing this new consensus surface.
Extended reasoning...
The bug. precompiles/setup.go (GetCustomPrecompiles) registers every new precompile added by this PR — auth, authz, evidence, feegrant, mint, params, slashing, upgrade — plus the new query methods added to bank, gov, and distribution, via each package's GetVersioned(latestUpgrade, keepers). Every one of those new setup.go files (e.g. precompiles/auth/setup.go) contains a single entry: latestUpgrade: check(NewPrecompile(keepers)), with no separate legacy snapshot archiving the pre-PR behavior. latestUpgrade is app.LatestUpgrade, computed in app/upgrades.go as the last line of app/tags — which is v6.6, and this PR does not touch app/tags (confirmed: the v6.6 line was added by a prior, unrelated commit, 8ebedda/#3665).\n\nWhy this matters for consensus. In x/evm/keeper/keeper.go, SetCustomPrecompiles builds k.latestCustomPrecompiles[addr] = versioned[latestUpgrade] for every address unconditionally, and CustomPrecompiles(ctx) returns that map for all normal (non-tracing) execution — i.e. every live transaction and every block replay uses the head implementation keyed at v6.6, regardless of the block's actual height or which upgrade was active when that block was originally produced. GetCustomPrecompilesVersions (used during tracing/replay) resolves a historical block to the version whose tag is <= the block's closest upgrade name, so any block already committed under v6.6 will now resolve to this PR's new code on a post-PR binary. There is no in-binary height gate protecting already-finalized blocks.\n\nConcrete evidence this is not hypothetical. precompiles/bank/legacy/v66/ already exists in the repo (added in #3665, well before this PR, and untouched by it) and contains a frozen snapshot of bank.go from before this PR — it has no SpendableBalances/denomMetadata/etc. methods. That legacy snapshot's existence is itself proof that v6.6 was already a cut, coordinated upgrade tag with its own fixed precompile behavior before this PR landed. But precompiles/bank/setup.go's GetVersioned maps latestUpgrade (v6.6) straight to the current bank.NewPrecompile, which now includes this PR's new methods — the old, frozen legacy/v66 snapshot is orphaned and unreferenced. So there are now two incompatible definitions of what "v6.6" precompile behavior means: the frozen legacy snapshot (correct, pre-PR) and the live head mapping in setup.go (incorrect, post-PR) — and the live mapping wins.\n\nConcrete divergence walkthrough. (1) Pre-PR binary is running at height H under upgrade tag v6.6; a STATICCALL to 0x...100D (the new auth precompile address) returns empty/no-code because that address is unallocated. (2) This PR merges without a new tag. (3) A post-PR binary starts up; latestUpgrade is still v6.6, so 0x...100D now resolves to the new auth precompile via GetCustomPrecompiles. (4) If any validator or full node replays/re-executes block H (or any block at/after the v6.6 upgrade height) with the post-PR binary — normal operation for state sync, IBC relayer verification, an archive node, or simply a validator that upgrades their binary without a coordinated halt height — the STATICCALL now succeeds and returns ABI-encoded account data instead of nothing. Any contract logic gated on that call's success/failure, or the exact returndata, produces a different state root than a node still running the pre-PR binary (or than the original block execution) → app-hash mismatch.\n\nWhy existing code doesn't already prevent this. The repo's own versioning discipline () exists precisely to solve this: every previously-shipped consensus-affecting precompile change archives the outgoing head implementation as a legacy version keyed to the tag it shipped under, and only the new tag gets the new head behavior (see precompiles/bank/setup.go's full legacy chain back to v5.5.2, or precompiles/staking/setup.go). This PR's new setup.go files skip that step entirely and land the new consensus surface directly on the existing head tag.\n\nThe fix. Run (or manually replicate) the bump_version flow: add a new tag (e.g. v6.7) to app/tags, and for every touched package (bank, gov, distribution, plus the 8 new precompiles) archive the current pre-PR implementation as the v6.6 legacy entry while pointing latestUpgrade at the new head. The PR's own "Notes for reviewers" section already flags this exact risk ("If v6.6 is already live, they should ride the next upgrade tag") but the tag was never bumped, so as merged the risk is unresolved.\n\nThis was independently flagged as a blocker by three separate verification passes and by the automated seidroid reviewer bot on the PR itself, all converging on the same root cause and fix.
|
|
||
| func (p PrecompileExecutor) Execute(ctx sdk.Context, method *abi.Method, caller common.Address, callingContract common.Address, args []interface{}, value *big.Int, readOnly bool, evm *vm.EVM, suppliedGas uint64, hooks *tracing.Hooks) (bz []byte, remainingGas uint64, err error) { | ||
| if ctx.EVMPrecompileCalledFromDelegateCall() { | ||
| return nil, errors.New("cannot delegatecall gov") | ||
| return nil, 0, errors.New("cannot delegatecall gov") | ||
| } | ||
|
|
||
| switch method.Name { | ||
| case ProposalQueryMethod: | ||
| return p.proposalQuery(ctx, method, args, value) | ||
| case ProposalsQueryMethod: | ||
| return p.proposalsQuery(ctx, method, args, value) | ||
| case VoteQueryMethod: | ||
| return p.voteQuery(ctx, method, args, value) | ||
| case VotesQueryMethod: | ||
| return p.votesQuery(ctx, method, args, value) | ||
| case ParamsQueryMethod: | ||
| return p.paramsQuery(ctx, method, args, value) | ||
| case DepositQueryMethod: | ||
| return p.depositQuery(ctx, method, args, value) | ||
| case DepositsQueryMethod: | ||
| return p.depositsQuery(ctx, method, args, value) | ||
| case TallyResultQueryMethod: | ||
| return p.tallyResultQuery(ctx, method, args, value) | ||
| } | ||
|
|
||
| if readOnly { |
There was a problem hiding this comment.
🟡 gov's Execute (precompiles/gov/gov.go) has no defer recover(), unlike every other precompile added or touched in this PR (auth, authz, bank, distribution, evidence, feegrant, mint, params, slashing, upgrade), all of which convert panics into a clean execution reverted: ... error. gov now dispatches to new query handlers (proposalsQuery, votesQuery, depositsQuery, voteWeighted's anonymous-struct assertion, etc.) that do bare type assertions and JSON-marshal Any-typed data, so an unexpected panic there propagates unrecovered instead of reverting like its siblings; recommend adding the same recover guard for consistency and defense-in-depth.
Extended reasoning...
What the bug is: gov.PrecompileExecutor.Execute (precompiles/gov/gov.go) has no top-level defer func(){ if r := recover(); r != nil { err = ... } }(). Every other precompile touched by this PR — auth, authz, bank, distribution, evidence, feegrant, mint, params, slashing, upgrade — wraps its Execute in exactly that pattern. gov is the one precompile in this PR that dispatches to brand-new, non-trivial query handlers (proposalQuery, proposalsQuery, voteQuery, votesQuery, paramsQuery, depositQuery, depositsQuery, tallyResultQuery, plus the pre-existing voteWeighted anonymous-struct assertion) without that safety net.
Code path: These handlers do bare type assertions on ABI-decoded args (args[0].(uint64), args[1].([]byte), the voteWeighted anonymous struct cast) and call p.cdc.MarshalAsJSON(proposal.Content) on protobuf Any values pulled straight from chain state. If any of that ever panics — a future maintenance bug, a codec edge case, an unregistered Any type, or any other unanticipated runtime panic — the panic unwinds through Execute, through pcommon.Precompile.Run (precompiles/common/precompiles.go), which also does not recover (its deferred func only maps a non-nil err to vm.ErrExecutionReverted), and keeps propagating up the EVM call stack.
Why existing code doesn't prevent it: gov is unique among the PR's precompiles in omitting the recover wrapper. Nothing else in the call chain (EVM interpreter, Precompile.Run) catches the panic on gov's behalf — it's designed to rely on the precompile executor doing its own recovery, which every sibling does and gov does not.
Impact, correcting the bug's original framing: the panic source the sibling precompiles' comments call out (// Needed to catch gas meter panics) does not apply to gov as currently wired: gov uses the fixed-gas pcommon.NewPrecompile path (not DynamicGasPrecompile), and during EVM tx execution the cosmos gas meter is swapped for sdk.NewInfiniteGasMeterWithMultiplier (x/evm/keeper/msg_server.go), so it cannot OOG-panic the way a bounded DynamicGasPrecompile meter can. Realistic panic triggers are also narrow: ABI-decoded args are type-safe by construction, and the gRPC query servers return errors rather than panicking on bad input. Should a panic occur anyway, cosmos baseapp's own recover in runTx will catch it — so there's no crash risk — but the failure surfaces as an ABCI-level tx failure instead of a controlled EVM revert, which is a worse (and inconsistent-with-siblings) failure mode for any contract that expects a normal execution reverted from a bad gov query call.
Fix: add the same defer func(){ if r := recover(); r != nil { err = fmt.Errorf("execution reverted: %v", r) } }() at the top of gov's Execute, matching every sibling precompile in this PR, for consistency and cheap defense-in-depth.
Step-by-step proof of the gap (not of an OOG trigger):
gov.NewPrecompilebuildsp := &PrecompileExecutor{...}and returnspcommon.NewPrecompile(newAbi, p, p.address, "gov")— the fixed-gas wrapper, confirmed by grep against precompiles/gov/gov.go.Executeswitches onmethod.Nameand calls e.g.p.proposalsQuery(ctx, method, args, value), which callsp.convertProposal(proposal)for each result, which callsp.cdc.MarshalAsJSON(proposal.Content).- Suppose (hypothetically, for illustration) a proposal's
ContentAnyholds a type not registered in the interface registry at the time of marshaling —MarshalAsJSONinternals could panic rather than return an error in some SDK codec paths. - That panic unwinds out of
convertProposal, out ofproposalsQuery, out ofExecute— none of which have a recover — and intopcommon.Precompile.Run, which also has no recover (grep-confirmed against precompiles/common/precompiles.go). - Contrast with
auth.accounts(precompiles/auth/auth.go), which does the analogousp.cdc.UnpackAny/iteration under its owndefer recover()inExecute— the same class of panic there is caught and converted tofmt.Errorf("execution reverted: %v", r), giving the caller a normal EVM revert instead of an ABCI-level failure. - Net effect: gov's query surface is the one precompile in this PR without that safety net, purely due to an oversight, not a structural need (there's no gas-meter panic to catch here as bug_001/verifiers established).
🔬 also observed by seidroid
| func (p PrecompileExecutor) Execute(ctx sdk.Context, method *abi.Method, caller common.Address, callingContract common.Address, args []interface{}, value *big.Int, readOnly bool, evm *vm.EVM, suppliedGas uint64, hooks *tracing.Hooks) (bz []byte, remainingGas uint64, err error) { | ||
| // Needed to catch gas meter panics | ||
| defer func() { | ||
| if r := recover(); r != nil { | ||
| err = fmt.Errorf("execution reverted: %v", r) | ||
| } | ||
| }() | ||
| switch method.Name { | ||
| case AccountMethod: | ||
| return p.account(ctx, method, args, value) | ||
| case AccountsMethod: | ||
| return p.accounts(ctx, method, args, value) | ||
| case ParamsMethod: | ||
| return p.params(ctx, method, args, value) | ||
| case NextAccountNumberMethod: | ||
| return p.nextAccountNumber(ctx, method, args, value) | ||
| } | ||
| return |
There was a problem hiding this comment.
🔴 auth's nextAccountNumber() is declared a Solidity view method, but its handler calls AccountKeeper.GetNextAccountNumber, which unconditionally increments and persists the global account-number counter on every invocation. Because Execute() never checks readOnly for this method and the write goes through the cosmos KVStore rather than the EVM StateDB, a plain eth_call or STATICCALL to this selector still mutates committed chain state and permanently burns an account number each time it is called, while RequiredGas still prices it as a flat read. Fix by reading the counter without incrementing it (or otherwise not exposing this mutating keeper method as a view).
Extended reasoning...
The bug. precompiles/auth/auth.go declares nextAccountNumber as a view method in both Auth.sol and abi.json, and Execute() (lines 80-97) dispatches it unconditionally to nextAccountNumber() without any readOnly check — unlike, for example, the sibling bank/gov precompiles in this same PR, which explicitly reject state-changing tx methods when readOnly is true. nextAccountNumber() calls p.authQuerier.NextAccountNumber(sdk.WrapSDKContext(ctx), ...), which resolves to AccountKeeper.NextAccountNumber in sei-cosmos/x/auth/keeper/grpc_query.go, which in turn calls AccountKeeper.GetNextAccountNumber(ctx). That keeper method's own doc comment says exactly what it does: "returns and increments the global account number counter." It performs an unconditional store.Get followed by store.Set(types.GlobalAccountNumberKey, accNumber+1) on every single call — this is the exact same counter that NewAccountWithAddress/NewAccount consume when a real account is created.
Why this isn't caught by normal precompile safety machinery. Cosmos SDK gRPC query handlers are typically safe to expose as EVM view methods because they only read from the KVStore. This one is an exception: it mutates as a side effect of "reading." Because the mutation happens directly against the live transaction's ctx.KVStore(...) (the same context backing the EVM StateDB during precompile execution, per RunAndCalculateGas in precompiles/common/precompiles.go), it bypasses go-ethereum's STATICCALL read-only guard entirely — that guard only prevents writes made through the EVM StateDB interface, not writes made directly against the underlying cosmos store by a precompile's own Go code. So a contract can STATICCALL address 0x...100D with the nextAccountNumber selector, and even though STATICCALL is supposed to guarantee no state changes are possible, the counter still advances.
Impact. (1) Any eth_call/staticcall-based read of nextAccountNumber — including routine, repeated polling from an off-chain dashboard or another contract's read-only helper — permanently advances real committed chain state with no account ever created for the burned number; (2) it violates the fundamental EVM invariant that a view-declared function (and specifically a STATICCALL) cannot mutate state, which off-chain tooling and other contracts rely on; (3) RequiredGas (pcommon.DefaultGasCost(input, p.IsTransaction(method.Name))) prices this as a flat read because IsTransaction always returns false in this file, so the KVStore write is under-priced relative to the work actually performed.
Concrete walkthrough. Suppose the current global account number is N. A contract issues STATICCALL 0x...100D with calldata for nextAccountNumber(). Execute() dispatches to nextAccountNumber() with no readOnly check → authQuerier.NextAccountNumber → GetNextAccountNumber(ctx) → reads N, then does store.Set(GlobalAccountNumberKey, N+1) → returns N. The call succeeds (STATICCALL did not revert, as go-ethereum's guard never triggered because the write bypassed StateDB), and the counter is now N+1 in committed state. Repeat the same STATICCALL again: it now returns N+1 and advances the counter to N+2. No account was ever created at any point, yet two account numbers were consumed — purely from what callers and tooling believe are side-effect-free reads.
Fix. Either drop this method from the precompile, or read the raw counter value from the store without performing the increment/store.Set (i.e., implement a true non-mutating read rather than reusing the keeper's read-and-increment helper).
| func (p PrecompileExecutor) spendableBalances(ctx sdk.Context, method *abi.Method, args []interface{}, value *big.Int) ([]byte, uint64, error) { | ||
| if err := pcommon.ValidateNonPayable(value); err != nil { | ||
| return nil, 0, err | ||
| } | ||
|
|
||
| if err := pcommon.ValidateArgsLength(args, 2); err != nil { | ||
| return nil, 0, err | ||
| } | ||
|
|
||
| seiAddr, err := pcommon.GetSeiAddressFromArg(ctx, args[0], p.evmKeeper) | ||
| if err != nil { | ||
| return nil, 0, err | ||
| } | ||
|
|
||
| req := &banktypes.QuerySpendableBalancesRequest{ | ||
| Address: seiAddr.String(), | ||
| Pagination: &query.PageRequest{ | ||
| Key: args[1].([]byte), | ||
| }, | ||
| } | ||
|
|
||
| resp, err := p.bankQuerier.SpendableBalances(sdk.WrapSDKContext(ctx), req) | ||
| if err != nil { |
There was a problem hiding this comment.
🟡 bank.spendableBalances resolves its address argument with the strict pcommon.GetSeiAddressFromArg, which reverts with an association-missing error if the EVM address has no explicit Sei association. Its siblings balance()/all_balances() in the same precompile fall back to the deterministic cast address in that case, so a funded-but-never-associated EVM address succeeds against balance()/all_balances() but reverts against spendableBalances() for the same account. Consider resolving the address the same way as balance/all_balances (accAddressFromArg with cast fallback) for consistency.
Extended reasoning...
spendableBalances (precompiles/bank/bank.go:418-440) resolves args[0] via pcommon.GetSeiAddressFromArg, which calls GetSeiAddressByEvmAddress (precompiles/common/precompiles.go:286-293). That helper returns types.NewAssociationMissingErr whenever the EVM address has no explicit Sei association recorded via SetAddressMapping — there is no fallback.
By contrast, the two pre-existing sibling query methods in the very same precompile, balance() and all_balances(), resolve their address argument through the file's own accAddressFromArg helper (bank.go, near the bottom of the file), which explicitly falls back to the deterministic cast address when no association is found:
seiAddr, found := p.evmKeeper.GetSeiAddress(ctx, addr)
if !found {
// return the casted version instead
return sdk.AccAddress(addr[:]), nil
}Per x/evm/AGENTS.md, EVM addresses without an explicit Associate transaction still hold real bank balances at their deterministic cast Sei address — this is a first-class, documented way accounts interact with bank state pre-association. balance()/all_balances() were written to support querying those cast-address balances without requiring association; spendableBalances(), added by this PR as the paginated analog of all_balances(), does not follow that same convention.
Concrete trigger: fund a fresh, never-associated EVM address (e.g. via a bank.send precompile call or a genesis allocation to its cast address) with some usei. Calling bank.balance(addr, "usei") succeeds and returns the funded amount, and bank.all_balances(addr) succeeds too — but bank.spendableBalances(addr, []) on the exact same address reverts with an association-missing error, even though the address indisputably has a nonzero spendable balance (a cast/plain account has no vesting, so spendable == total, meaning the cast-fallback path would return the correct data if used).
The PR's own test, TestSpendableBalancesQuery, does not catch this because it calls k.SetAddressMapping(ctx, seiAddr, evmAddr) before querying, which masks the unassociated/cast-address case entirely.
Fix: resolve the address in spendableBalances via the same accAddressFromArg helper (or an equivalent cast-fallback) used by balance/all_balances, instead of the strict pcommon.GetSeiAddressFromArg.
This does not block merge: it fails closed with a clear, well-formed revert (no incorrect data, no crash, no state corruption), and a caller can always work around it by associating first or by using all_balances to get the same data. The PR description also states that "account-typed query inputs take EVM address and require address association," which is the strict convention deliberately used by every other new query method in this PR (auth, authz, feegrant, distribution, gov). The issue here is narrower and specific to bank: the author's stated rationale for spendableBalances was consistency with "existing bank precompiles," but that's factually inverted for bank's own balance/all_balances, so the divergence is a legitimate intra-precompile consistency gap worth fixing, not a merge blocker.
| func (p PrecompileExecutor) convertProposal(proposal govtypes.Proposal) (ProposalData, error) { | ||
| content := []byte{} | ||
| if proposal.Content != nil { | ||
| bz, err := p.cdc.MarshalAsJSON(proposal.Content) | ||
| if err != nil { | ||
| return ProposalData{}, err | ||
| } | ||
| content = bz | ||
| } | ||
| return ProposalData{ | ||
| Id: proposal.ProposalId, | ||
| Status: int32(proposal.Status), | ||
| FinalTallyResult: convertTallyResult(proposal.FinalTallyResult), | ||
| SubmitTime: proposal.SubmitTime.Unix(), | ||
| DepositEndTime: proposal.DepositEndTime.Unix(), | ||
| TotalDeposit: convertCoins(proposal.TotalDeposit), | ||
| VotingStartTime: proposal.VotingStartTime.Unix(), | ||
| VotingEndTime: proposal.VotingEndTime.Unix(), | ||
| IsExpedited: proposal.IsExpedited, | ||
| Content: content, | ||
| }, nil |
There was a problem hiding this comment.
🟡 convertProposal (used by both the new proposal and proposals gov precompile queries) unconditionally calls .Unix() on proposal.VotingStartTime/VotingEndTime. For any proposal still in the deposit period these fields are the Go zero-value time.Time{}, whose .Unix() is -62135596800, not 0 — so EVM callers get a large, misleading sentinel instead of an expected 0 meaning "voting hasn't started". Fix by guarding with IsZero() and emitting 0.
Extended reasoning...
The bug. precompiles/gov/gov.go's convertProposal (used by both the proposal(uint64) and proposals(...) query methods added in this PR) builds the ProposalData struct with:
VotingStartTime: proposal.VotingStartTime.Unix(),
VotingEndTime: proposal.VotingEndTime.Unix(),In sei-cosmos, govtypes.Proposal.VotingStartTime and VotingEndTime are value-type (non-pointer, non-nullable) time.Time fields. They are only ever populated once a proposal exits the deposit period and enters voting, in Keeper.ActivateVotingPeriod (sei-cosmos/x/gov/keeper/proposal.go: proposal.VotingStartTime = ctx.BlockHeader().Time; proposal.VotingEndTime = ...Add(votingPeriod)). NewProposal (sei-cosmos/x/gov/types/proposal.go), which is what runs at submission time, only sets SubmitTime and DepositEndTime — it leaves both voting-time fields as the Go zero value time.Time{}.
Why this manifests, and why it's not benign. time.Time{}.Unix() is not 0 — it's -62135596800 (the number of seconds between January 1, year 1 and the Unix epoch). This is a basic Go footgun: the zero value of time.Time does not round-trip through .Unix() to 0. So any EVM caller querying a proposal that is still in StatusDepositPeriod (the initial state of every proposal, not an edge case) via the new proposal or proposals precompile methods receives votingStartTime/votingEndTime == -62135596800 in the packed int64 ABI fields.
Concrete proof.
- A proposal is submitted via
submitProposal;govKeeper.SubmitProposal→NewProposalcreates it withStatus = StatusDepositPeriod,SubmitTimeandDepositEndTimeset, butVotingStartTime/VotingEndTimeleft astime.Time{}. - A contract calls
gov.proposal(proposalID)before the deposit threshold is met. proposalQuery→convertProposalrunsproposal.VotingStartTime.Unix().- In Go,
time.Time{}.Unix()evaluates to-62135596800, confirmed by running it directly (year-1-to-epoch offset), not0. - The packed response therefore contains
votingStartTime = -62135596800instead of the natural "not started" sentinel of0.
Why nothing in the existing code path catches this. There is no IsZero() guard anywhere in convertProposal, and the proto/JSON round-trip for stdtime fields preserves the zero-time value unchanged (it doesn't get special-cased to zero on either the cosmos or EVM side). SubmitTime/DepositEndTime don't have this problem because NewProposal always sets them, so this is specific to the two voting-period fields.
Impact. This is read-only (view) data with no consensus-safety or fund-safety implications — every node computes the same deterministic value, so there's no app-hash risk. But it's a genuinely confusing return value for any consumer: a contract or off-chain caller checking votingStartTime == 0 to mean "not started" will get it wrong, and a caller naively feeding the value into time.Unix(votingStartTime, 0) will get a nonsensical timestamp (year 1) instead of anything meaningful. Since this is a brand-new, consensus-visible query API being introduced in this PR, it's worth fixing before it ships, since correcting the sentinel later constitutes a change to already-committed precompile output.
Fix. Guard both fields with IsZero() in convertProposal and emit 0 for unset times, e.g.:
votingStartTime := int64(0)
if !proposal.VotingStartTime.IsZero() {
votingStartTime = proposal.VotingStartTime.Unix()
}
// same for VotingEndTime
1 participant
Motivation
An audit of all
rpcdefs inservice Query(query.proto) andservice Msg(tx.proto) under/protoand/sei-cosmos/protofound that many module Query rpcs have no EVM precompile counterpart: bank/distribution were partial, gov was write-only, and auth, authz, evidence, feegrant, mint, params, slashing, and upgrade had no precompile at all.Changes
Extended existing precompiles (new
viewmethods, head version only)spendableBalances,totalSupply(paginated all-denoms; existingsupply(denom)unchanged),params,denomMetadata,denomsMetadataparams,validatorOutstandingRewards,validatorCommission,validatorSlashes,delegationRewards,delegatorValidators,delegatorWithdrawAddress,communityPoolproposal,proposals(status/voter/depositor filters),getVote,votes,params(voting+deposit+tallying merged),getDeposit,deposits,tallyResult—getVote(uint64,address)/getDeposit(uint64,address)(named to avoid overloading the tx methods, which breaks ethers.js name resolution)New query-only precompiles
0x…100Daccount,accounts,params,nextAccountNumber0x…100Egrants,granterGrants,granteeGrants0x…100Fevidence,allEvidence0x…1010allowance,allowances,allowancesByGranter0x…1012params,minter0x…1013params(subspace, key)0x…1014params,signingInfo,signingInfos0x…1015currentPlan,appliedPlan,upgradedConsensusState,moduleVersions(
0x…1011is already p256.) Each new precompile follows the oracle-style DynamicGasPrecompile shape withabi.json, a Solidity interface, aversionsfile (singlev6.6entry), and a generated-stylesetup.go, soscripts/bump_versionmanages them going forward.Plumbing
StakingQuerierpattern) plusCodec()added toprecompiles/utils/expected_keepers.go, wired to concrete keepers inapp/precompiles.go(mint viamintkeeper.NewQuerier; others implement their gRPC QueryServers directly)Precompilewrapper (fixed 30k/50k per tx method) to theDynamicGasPrecompilepattern used by bank/staking/distribution, metering actual Cosmos gas consumptiongiga/executor/precompiles/failfast.go) so giga aborts and falls back to v2 on calls to them, like the existing custom precompilesDec/Int→ string, times → Unix seconds, durations → seconds, pagination viabytes pageKey+nextKeyAny-typed response fields (gov proposal content, authz authorizations, evidence, feegrant allowances) are returned as registry-resolved JSON bytes ({"@type": …}), consumable on-chain with thejsonprecompileNotes for reviewers
v6.6). Ifv6.6is already live, they should ride the next upgrade tag — thebump_versionflow handles archival when the tag is added.addressand require address association (consistent with existing bank/staking/distribution precompiles); validator/consensus addresses are bech32 strings.precompiles/setup.goregisters the new precompiles inGetCustomPrecompilesonly (same assolo), not in the legacyInitializePrecompilesglobal-VM path.Testing
go build ./...cleango test ./precompiles/... -count=1— all 22 precompile packages pass, including new tests covering happy paths, pagination, error paths (unassociated address, not-found), and exact packed-output assertionsgo test ./x/evm/keeper/ -run TestGetCustomPrecompilespassesgo vetclean; all touched filesgofmt -s/goimportscompliant🤖 Generated with Claude Code