Repository navigation
fix: F-2026-18187 | [Dual Defense] Gov Precompile getTallyResult Persists SDK Tally Vote Deletions - #42
Merged
Conversation
The SDK tally deletes the votes it counts; precompiles commit unconditionally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
F-2026-18187 — a "query" that deletes votes, and commits
getTallyResulton the gov precompile is classified a query: it is absent fromIsTransaction(
precompiles/gov/gov.go:148-154). That classification is load-bearing in exactly two places —SetupABI's staticcall guard (readOnly && isTransaction → ErrWriteProtection) andRequiredGas.Neither of them gates whether the call's state writes are kept.
What decides that is
RunNativeAction→runNativeAction(precompiles/common/precompile.go:57-90),which calls
stateDB.CommitWithCacheCtx()unconditionally and never branches on query vstransaction. So the precompile's whole cache context — everything the method wrote — rides out with
the enclosing EVM transaction.
Meanwhile
getTallyResult→ SDKTallyResult→Keeper.Tally→Votes.Remove(
x/gov/keeper/tally.go:117). Tallying deleting the votes it counts is not an SDK bug: it runsonce, terminally, in the gov EndBlocker, and its gRPC
TallyResultquery is safe becausequeries run against a context that is never committed. The SDK's invariant is "tallying mutates,
but query paths never commit". The precompile takes that mutating query and runs it in a committing
context.
Net effect: an ordinary EVM transaction calling
getTallyResult(proposalId)on a proposal in thevoting period deletes that proposal's votes, and the deletion commits. Permissionless,
repeatable, costs only gas. The gov precompile is live on donut.
The fix
GetTallyResultnow runs the SDK query against a throwaway cache and discards its store writes:The vote deletions evaporate with the cache; the tally itself is still the one the SDK computes —
delegations, weighted votes, validator defaults — so nothing about the returned numbers changes.
Why it is not a bare
ctx.CacheContext()The intended two-liner was
cacheCtx, _ := ctx.CacheContext(). That does not isolate anythinginside a precompile, and it was verified not to — with it in place both regression tests below
still failed with the votes gone.
sdk.Context.CacheContextdefers to the underlying multi store'sCacheMultiStore(). On the SDK'sown stores that hands back a detached cache, and dropping the write function is enough. The store
the EVM runs precompiles on is
x/vm/store/snapshotmulti, whoseCacheMultiStore()is:It pushes a snapshot layer and returns the very same store. The layer stays live as
CurrentStore(), andWrite()commits every layer top-to-bottom when the state DB commits — so thedeletions land regardless of the discarded write function.
newDiscardedCacheContexttherefore pops the layer explicitly on that store (Snapshot()/RevertToSnapshot()— the same mechanismrunNativeActionalready uses to roll a failed precompileback), and falls back to plain
CacheContext()on any other store. The snapshot/revert pair isbalanced, so the state DB's own snapshot bookkeeping is left exactly as it was found. This is the
same idea as the agreed approach, expressed in the vocabulary this multi store actually implements;
the change stays entirely inside
GetTallyResult.Tests
TestGetTallyResultDoesNotDeleteVotes(tests/integration/precompiles/gov/test_gov.go) drives theprecompile through
Runand then commits the state DB, exactly as a delivered transaction does.Setup: proposal 1 in the voting period with 3 stored votes — a weighted vote from the network's
only delegator split 0.4 Yes / 0.3 Abstain / 0.2 No / 0.1 NoWithVeto, plus two zero-power voters that
contribute nothing to the tally but are votes
Keeper.Tallywould delete all the same.Run for both
readOnly = false(normal committing CALL — the actual attack) andreadOnly = true(STATICCALL), asserting:1.2e18 / 0.9e18 / 0.6e18 / 0.3e18— so mutation was not traded for wrong numbers.fix the second call would have returned all zeros).
Regression-detector check: with the fix reverted, both subtests fail on
getTallyResult must not delete the proposal's votes; restored, both pass.Results:
evmd/tests/integration/precompiles/gov(full gov precompile suite, unit + Ginkgo integration) — okprecompiles/gov— okx/vm/store/snapshotkv,x/vm/store/snapshotmulti— ok (untouched, run because the fix relieson their semantics)
go vetclean,gofmtclean.Hacken's other recommendations
2 — return empty/partial for voting-period proposals: declined. A live tally is the whole point
of the call; returning nothing during the voting period is a functional regression, and it is
redundant once the query stops mutating.
3 — classify the mutating path as
IsTransaction == true/ reject whenreadOnly: declined aswritten.
IsTransactiononly feeds the staticcall guard and gas pricing. The attack is a normalcommitting CALL with
readOnly == false, so this blocks the harmless staticcall and leaves theharmful call untouched — the tests above confirm both paths reach the mutating SDK code.
The structural form of that idea — gating the commit on the classification in
runNativeActioninstead of committing unconditionally — is the real class fix, but it changes behaviour for every
precompile's query methods and the framework is upstream's. Confirmed still present on cosmos/evm
main(same unconditional pattern viaFlushToCacheCtx), so it is being reported upstream ratherthan forked here. Note this is not covered by ISA-2025-004 / GHSA-mjfq-3qr2-6g84 ("Partial
Precompile State Writes"), which is about atomicity on failure and was fixed with
RunAtomic.