fix(coredaos): defer MsgVetoProposal deposit/vote cleanup to EndBlocker - #365
Open
giunatale wants to merge 6 commits into
Open
fix(coredaos): defer MsgVetoProposal deposit/vote cleanup to EndBlocker#365giunatale wants to merge 6 commits into
giunatale wants to merge 6 commits into
Conversation
tbruyelle
requested changes
Aug 26, 2026
The veto's deposit/vote cleanup is deferred to the coredaos EndBlocker. Because gov's EndBlocker ran first, its quorum check (QuorumCheckQueue) could still observe a vetoed proposal's votes, pass quorum, and re-insert the proposal into the ActiveProposalsQueue, resurrecting it. Order the coredaos EndBlocker before gov's so the vetoed proposal's votes are deleted before the quorum check runs. Add a guard test for the ordering and document the requirement on the EndBlocker.
The previous implementation collected the ids first because the cleanup was assumed to be unsafe while walking the queue. That premise is wrong: the SDK collections walk tolerates removing the current key mid-iteration, exactly as x/gov's own EndBlocker does for its queues. Remove the proposal inline and drop the collect-then-delete indirection.
Returning an error from EndBlock halts the chain. The deferred veto cleanup is best-effort, so run each proposal's cleanup in a cached context: commit on success, and on failure log and leave the entry queued to be retried on the next block instead of aborting the block.
giunatale
force-pushed
the
fix/coredaos-veto-gas-exhaustion
branch
from
August 26, 2026 15:35
89c5f93 to
50c6e14
Compare
tbruyelle
approved these changes
Aug 26, 2026
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
3 participants
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.
The
x/coredaosMsgVetoProposalrefunded/burned every deposit and deleted every vote of the target proposal inline, inside the message handler. That handler runs under the metered transaction gas meter, which is itself bounded byblock.max_gas. Both loops are unbounded and scale linearly with the number of depositors and votes, quantities anyone can inflate during the voting period.Once the cumulative cost exceeds
block.max_gas(100,000,000 on mainnet), no veto transaction for that proposal can be included in any block, and the Oversight DAO's constitutional veto becomes permanently unavailable for it.Fix
x/gov's EndBlocker (which already performs exactly these deletions for rejected proposals under the infinite block gas meter) lives in the AtomOne-SDK, so deferring there would be a two-repo change. Moreover, while the fix affect proposals, it's related to behavior implemented in thex/coredaosmodule. For these reasons the fix stays entirely withinx/coredaos:VetoProposalis now O(1): it marks the proposalVetoed, resets the tally,ends voting immediately, removes the proposal from the gov active queue (so
gov never tallies or executes it), and enqueues its id for cleanup.
EndBlockerdrains that queue, performing the (potentiallyunbounded) deposit refund/burn and vote deletion.
EndBlockruns under theinfinite block gas meter, exactly like gov's own cleanup of rejected
proposals, so it can never be priced out of a block.