fix(sequencer): stop signalling already-executed governance payloads#24764
Open
spalladino wants to merge 2 commits into
Open
fix(sequencer): stop signalling already-executed governance payloads#24764spalladino wants to merge 2 commits into
spalladino wants to merge 2 commits into
Conversation
…s executed A mainnet sequencer kept signalling a governance payload whose proposal had already been executed, wasting ~100k gas per slot on a signal the canonical rollup rejected. Three fixes: - Fix the `ProposalState` enum, which was missing `Droppable` (Solidity `IGovernance` has 9 states). The mismatch made Solidity `Expired` decode as out-of-range and throw in `asProposalState`, so the publisher failed open and signalled anyway. Add an explicit `LIVE_PROPOSAL_STATES` set for the sweep. - Replace `hasActiveProposalWithPayload` (boolean) with `getPayloadProposalStatus` returning `'live' | 'executed' | 'none'`, matching a proposal by its stored payload directly or via its GSEPayload wrapper. The publisher now stops signalling an executed payload (memoized in-process, live takes precedence over executed) unless `GOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE` is set, for payloads designed to be re-executed. - Add a canonicality guard: resolve the canonical rollup instance once and skip the signal when the configured rollup is not canonical, reusing that instance for round accounting and the EIP-712 digest so the read cannot race a canonical-rollup change.
…ser API Resolve the canonical rollup via getRollupAddress() instead of the getInstance() wrapper, dropping the now-unused instance parameter threaded through createSignalRequestWithSignature. Since the guard returns early when the configured rollup is not canonical, round accounting and the EIP-712 digest can just use the configured rollup address. Also drop the redundant sawExecuted flag from getPayloadProposalStatus (the executed verdict is already memoized in the set).
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.
Context
A mainnet sequencer kept casting governance signals for a payload whose proposal had already been executed: the outer Multicall3 tx succeeds but the inner
signalWithSigreverts (swallowed byallowFailure), wasting ~100k gas per slot. Two client-side bugs allowed this: executed proposals were treated like any other terminal state, so the payload was re-signalled every round; and the TSProposalStateenum was missingDroppable(present inIGovernance.solsince July 2025), so decoding anExpiredproposal threw, and the publisher failed open and signalled anyway. Additionally, since the executed payload upgraded the canonical rollup, proposers of the old rollup kept signalling on aGovernanceProposernow keyed to a different instance, where their signals always revert.Approach
ProposalStateenum to matchIGovernance.sol(9 states includingDroppable) and drive the sweep from an explicitLIVE_PROPOSAL_STATESset.hasActiveProposalWithPayloadsweep withgetPayloadProposalStatusreturning'live' | 'executed' | 'none'. The publisher stops signalling a payload whose proposal was executed, unless the newGOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTEconfig is set (L1 permits re-proposing an executed payload, and some payloads are designed to be re-executed). Live proposals take precedence over executed ones, executed verdicts are memoized in-process, and the sweep remains a bounded lookback capped by the protocol-wide proposal lifetime.enqueueCastSignalHelper: resolve the canonical rollup viagetRollupAddress()and skip the signal when the configured rollup is not canonical. Only the canonical rollup's current proposer can signal, so a non-canonical node would otherwise waste gas on a reverting signal. Because the guard returns early on a mismatch, round accounting and the EIP-712 digest use the configured (verified-canonical) rollup address.API changes
ReadOnlyGovernanceContract.hasActiveProposalWithPayloadand theGovernanceProposerContractwrapper are replaced bygetPayloadProposalStatus(payload). The redundantGovernanceProposerContract.getInstance()wrapper is removed in favour of the existinggetRollupAddress(). New optional sequencer configgovernanceProposerForcePayloadVote(envGOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE, defaultfalse).To be forward-ported to
merge-train/spartanafter landing on the v5 line.Fixes A-1422