Skip to content

Commit f61a033

Browse files
PhilWindleaztec-bot
authored andcommitted
fix(sequencer): stop signalling already-executed governance payloads (#24764)
## 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 `signalWithSig` reverts (swallowed by `allowFailure`), 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 TS `ProposalState` enum was missing `Droppable` (present in `IGovernance.sol` since July 2025), so decoding an `Expired` proposal 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 a `GovernanceProposer` now keyed to a different instance, where their signals always revert. ## Approach - Fix the `ProposalState` enum to match `IGovernance.sol` (9 states including `Droppable`) and drive the sweep from an explicit `LIVE_PROPOSAL_STATES` set. - Replace the boolean `hasActiveProposalWithPayload` sweep with `getPayloadProposalStatus` returning `'live' | 'executed' | 'none'`. The publisher stops signalling a payload whose proposal was executed, unless the new `GOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE` config 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. - Add a canonicality guard to `enqueueCastSignalHelper`: resolve the canonical rollup via `getRollupAddress()` 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. - The proposal-status guard fails open on RPC errors — at worst a duplicate signal the contract simply counts alongside others in the round — so a flaky L1 endpoint cannot silence governance participation. A failure to resolve the canonical rollup instead skips the signal for that slot and is retried on the next one. ## API changes `ReadOnlyGovernanceContract.hasActiveProposalWithPayload` and the `GovernanceProposerContract` wrapper are replaced by `getPayloadProposalStatus(payload)`. The redundant `GovernanceProposerContract.getInstance()` wrapper is removed in favour of the existing `getRollupAddress()`. New optional sequencer config `governanceProposerForcePayloadVote` (env `GOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE`, default `false`). To be forward-ported to `merge-train/spartan` after landing on the v5 line. Fixes A-1422
1 parent 3cbac3e commit f61a033

9 files changed

Lines changed: 363 additions & 85 deletions

File tree

yarn-project/ethereum/src/contracts/governance.test.ts

Lines changed: 152 additions & 25 deletions
Large diffs are not rendered by default.

yarn-project/ethereum/src/contracts/governance.ts

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,29 @@ export type L1GovernanceContractAddresses = Pick<
3737
'governanceAddress' | 'rollupAddress' | 'registryAddress' | 'governanceProposerAddress'
3838
>;
3939

40-
// NOTE: Must be kept in sync with DataStructures.ProposalState in l1-contracts
40+
// NOTE: Must be kept in sync with IGovernance.ProposalState in l1-contracts
4141
export enum ProposalState {
4242
Pending,
4343
Active,
4444
Queued,
4545
Executable,
4646
Rejected,
4747
Executed,
48+
Droppable,
4849
Dropped,
4950
Expired,
5051
}
5152

53+
/**
54+
* Outcome of {@link ReadOnlyGovernanceContract.getPayloadProposalStatus} for a queried payload.
55+
* - `'live'`: a proposal referencing the payload is still progressing (Pending/Active/Queued/
56+
* Executable) or is `Droppable`, so signalling for it again is redundant or premature.
57+
* - `'executed'`: no live proposal references the payload, but one was already executed within the
58+
* bounded lookback (or a prior sweep observed the execution and memoized it).
59+
* - `'none'`: no live or executed proposal references the payload within the bounded lookback.
60+
*/
61+
export type PayloadProposalStatus = 'live' | 'executed' | 'none';
62+
5263
/** Vote tallies on a single proposal. Both fields are mutated by `Governance.vote`. */
5364
export interface Ballot {
5465
yea: bigint;
@@ -119,6 +130,17 @@ const TERMINAL_PROPOSAL_STATES: ReadonlySet<ProposalState> = new Set([
119130
ProposalState.Expired,
120131
]);
121132

133+
// Set of `ProposalState` values in which a proposal is still progressing towards execution.
134+
// `Droppable` is deliberately excluded: it is neither live (it cannot progress on its own) nor
135+
// terminal (it can resume its lifecycle if the governanceProposer is restored), so it is handled
136+
// separately and is never memoized as immutable.
137+
const LIVE_PROPOSAL_STATES: ReadonlySet<ProposalState> = new Set([
138+
ProposalState.Pending,
139+
ProposalState.Active,
140+
ProposalState.Queued,
141+
ProposalState.Executable,
142+
]);
143+
122144
// Hard upper bound on the wall-clock lifetime of any Governance proposal, in seconds.
123145
// Each proposal stores its own snapshot of `ProposalConfiguration` at creation time and progresses
124146
// through Pending -> Active -> Queued -> Executable using those frozen durations
@@ -173,6 +195,14 @@ export class ReadOnlyGovernanceContract {
173195
*/
174196
private readonly originalPayloadCache: Map<Hex, Hex | undefined> = new Map();
175197

198+
/**
199+
* Payloads (lowercased hex) observed as the subject of an `Executed` Governance proposal. Execution
200+
* is immutable on-chain, so this verdict never expires within a process. It restores the
201+
* `'executed'` classification for payloads whose executed proposal has since aged past the bounded
202+
* lookback below. Lost on restart, at which point the bounded-lookback limitation applies again.
203+
*/
204+
private readonly executedPayloads: Set<string> = new Set();
205+
176206
constructor(
177207
address: Hex,
178208
public readonly client: ViemClient,
@@ -263,55 +293,68 @@ export class ReadOnlyGovernanceContract {
263293
}
264294

265295
/**
266-
* Checks whether the given original payload is currently the subject of a live (non-terminal)
267-
* Governance proposal. Returns true only if a proposal references this payload and is still in
268-
* Pending, Active, Queued, or Executable state. Terminal proposals (Executed, Rejected, Dropped,
269-
* Expired) are ignored, because once a proposal reaches a terminal state the same original
270-
* payload may legitimately be re-signaled and re-submitted via the GovernanceProposer (each round
271-
* is independent and there is no payload-level uniqueness check on-chain).
296+
* Classifies the given original payload against the Governance proposal history. Distinguishes a
297+
* payload that is still the subject of a live proposal (`'live'`) from one whose proposal was
298+
* already executed (`'executed'`) from one that has no relevant proposal (`'none'`), so callers can
299+
* stop re-signalling an executed payload while still re-signalling one whose proposal was merely
300+
* rejected/dropped/expired (each GovernanceProposer round is independent and there is no
301+
* payload-level uniqueness check on-chain).
302+
*
303+
* A proposal matches the payload either directly (its stored `payload` equals the target, as for
304+
* `proposeWithLock` proposals) or via its `GSEPayload` wrapper unwrapping to the target. `'live'`
305+
* (including `Droppable`) takes precedence over `'executed'`, so a payload re-submitted while a
306+
* prior execution is still in the lookback window reads as `'live'`.
272307
*
273308
* Implemented as a bounded view-call sweep over `Governance.proposals` rather than an event scan,
274309
* because `eth_getLogs` over the full deployment history of a long-lived rollup exceeds typical
275310
* RPC block-range caps. The number of proposals (`proposalCount`) is small in practice, and we
276-
* walk newest -> oldest with a hard early-stop on the protocol-wide proposal lifetime cap.
311+
* walk newest -> oldest with a hard early-stop on the protocol-wide proposal lifetime cap. This
312+
* makes the `'executed'` verdict a *bounded lookback* rather than permanent suppression: a proposal
313+
* executed longer ago than the lifetime cap is only reported once it has been observed and
314+
* memoized in-process (memo is lost on restart).
277315
*/
278-
public async hasActiveProposalWithPayload(payload: Hex): Promise<boolean> {
316+
public async getPayloadProposalStatus(payload: Hex): Promise<PayloadProposalStatus> {
317+
const target = payload.toLowerCase() as Hex;
318+
279319
const proposalCount = await this.getProposalCount();
280320
if (proposalCount === 0n) {
281-
return false;
321+
return this.executedPayloads.has(target) ? 'executed' : 'none';
282322
}
283323

284324
// Anything created before this cutoff is guaranteed terminal regardless of its frozen config.
285325
const block = await this.client.getBlock();
286326
const hardCutoff = block.timestamp - MAX_PROPOSAL_LIFETIME_SECONDS;
287327

288-
const target = payload.toLowerCase() as Hex;
289-
290328
// Proposals are append-only with monotonically non-decreasing creation timestamps, so iterating
291329
// from newest -> oldest lets us early-stop as soon as we cross the lifetime cutoff.
292330
for (let id = proposalCount - 1n; id >= 0n; id--) {
293331
const proposal = await this.getProposal(id);
294332

295333
// Hard early-stop: every older proposal is also older than the cutoff and therefore terminal.
296334
if (proposal.creation < hardCutoff) {
297-
return false;
335+
break;
298336
}
299337

338+
const proposalPayload = proposal.payload.toString().toLowerCase();
300339
const original = await this.getOriginalPayload(proposal.payload);
301-
if (original === undefined || original.toLowerCase() !== target) {
340+
const matches = proposalPayload === target || (original !== undefined && original.toLowerCase() === target);
341+
if (!matches) {
302342
continue;
303343
}
304344

305-
// The wrapper unwraps to our payload. Only treat this as "already proposed" if the proposal
306-
// is still live -- terminal states allow re-proposing the same payload in a later round.
307-
if (TERMINAL_PROPOSAL_STATES.has(proposal.state)) {
308-
continue;
345+
// A live or Droppable proposal blocks re-signalling and wins over any executed one.
346+
if (LIVE_PROPOSAL_STATES.has(proposal.state) || proposal.state === ProposalState.Droppable) {
347+
return 'live';
348+
}
349+
350+
if (proposal.state === ProposalState.Executed) {
351+
this.executedPayloads.add(target);
309352
}
310353

311-
return true;
354+
// Rejected/Dropped/Expired proposals allow re-proposing the same payload; keep scanning.
312355
}
313356

314-
return false;
357+
return this.executedPayloads.has(target) ? 'executed' : 'none';
315358
}
316359

317360
/**

yarn-project/ethereum/src/contracts/governance_proposer.ts

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
import type { L1TxRequest, L1TxUtils } from '../l1_tx_utils/index.js';
1616
import type { ViemClient } from '../types.js';
1717
import { type IEmpireBase, encodeSignal, encodeSignalWithSignature, signSignalWithSig } from './empire_base.js';
18-
import { ReadOnlyGovernanceContract, extractProposalIdFromLogs } from './governance.js';
18+
import { type PayloadProposalStatus, ReadOnlyGovernanceContract, extractProposalIdFromLogs } from './governance.js';
1919

2020
export class GovernanceProposerContract implements IEmpireBase {
2121
private readonly proposer: GetContractReturnType<typeof GovernanceProposerAbi, ViemClient>;
@@ -38,16 +38,16 @@ export class GovernanceProposerContract implements IEmpireBase {
3838
this.proposer = getContract({ address, abi: GovernanceProposerAbi, client });
3939
}
4040

41-
public get address() {
41+
public get address(): EthAddress {
4242
return EthAddress.fromString(this.proposer.address);
4343
}
4444

45-
public async getRollupAddress() {
45+
public async getRollupAddress(): Promise<EthAddress> {
4646
return EthAddress.fromString(await this.proposer.read.getInstance());
4747
}
4848

4949
@memoize
50-
public async getRegistryAddress() {
50+
public async getRegistryAddress(): Promise<EthAddress> {
5151
return EthAddress.fromString(await this.proposer.read.REGISTRY());
5252
}
5353

@@ -59,10 +59,6 @@ export class GovernanceProposerContract implements IEmpireBase {
5959
return this.proposer.read.ROUND_SIZE();
6060
}
6161

62-
public getInstance() {
63-
return this.proposer.read.getInstance();
64-
}
65-
6662
public computeRound(slot: SlotNumber): Promise<bigint> {
6763
return this.proposer.read.computeRound([BigInt(slot)]);
6864
}
@@ -107,7 +103,7 @@ export class GovernanceProposerContract implements IEmpireBase {
107103
signer,
108104
payload,
109105
slot,
110-
await this.getInstance(),
106+
(await this.getRollupAddress()).toString(),
111107
this.address.toString(),
112108
chainId,
113109
);
@@ -130,15 +126,15 @@ export class GovernanceProposerContract implements IEmpireBase {
130126
}
131127

132128
/**
133-
* Returns true iff the given original payload is currently the subject of a live (non-terminal)
134-
* Governance proposal. Delegates to `ReadOnlyGovernanceContract.hasActiveProposalWithPayload`, which
135-
* implements the actual sweep against the Governance contract -- this method exists only as a
136-
* convenience wrapper so callers that already hold a GovernanceProposer reference don't have to
129+
* Classifies the given original payload against the Governance proposal history (`'live'` /
130+
* `'executed'` / `'none'`). Delegates to `ReadOnlyGovernanceContract.getPayloadProposalStatus`,
131+
* which implements the actual sweep against the Governance contract -- this method exists only as
132+
* a convenience wrapper so callers that already hold a GovernanceProposer reference don't have to
137133
* resolve the Governance address themselves.
138134
*/
139-
public async hasActiveProposalWithPayload(payload: Hex): Promise<boolean> {
135+
public async getPayloadProposalStatus(payload: Hex): Promise<PayloadProposalStatus> {
140136
const governance = await this.getGovernance();
141-
return governance.hasActiveProposalWithPayload(payload);
137+
return governance.getPayloadProposalStatus(payload);
142138
}
143139

144140
/**

yarn-project/foundation/src/config/env_var.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export type EnvVar =
8484
| 'ETHEREUM_ALLOW_NO_DEBUG_HOSTS'
8585
| 'FEE_RECIPIENT'
8686
| 'FORCE_COLOR'
87+
| 'GOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE'
8788
| 'GOVERNANCE_PROPOSER_PAYLOAD_ADDRESS'
8889
| 'KEY_STORE_DIRECTORY'
8990
| 'L1_CHAIN_ID'

yarn-project/sequencer-client/src/config.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export const DefaultSequencerConfig = {
6464
injectUnrecoverableSignatureAttestation: false,
6565
injectYParityAttestation: false,
6666
fishermanMode: false,
67+
governanceProposerForcePayloadVote: false,
6768
shuffleAttestationOrdering: false,
6869
skipPushProposedBlocksToArchiver: false,
6970
skipPublishingCheckpointsPercent: 0,
@@ -161,6 +162,12 @@ export const sequencerConfigMappings: ConfigMappingsType<SequencerConfig> = {
161162
description: 'The address of the payload for the governanceProposer',
162163
parseEnv: (val: string) => EthAddress.fromString(val),
163164
},
165+
governanceProposerForcePayloadVote: {
166+
env: 'GOVERNANCE_PROPOSER_FORCE_PAYLOAD_VOTE',
167+
description:
168+
'Keep signalling the configured governance payload even if a proposal referencing it was already executed.',
169+
...booleanConfigHelper(DefaultSequencerConfig.governanceProposerForcePayloadVote),
170+
},
164171
l1PublishingTime: {
165172
env: 'SEQ_L1_PUBLISHING_TIME_ALLOWANCE_IN_SLOT',
166173
description: 'How much time in seconds to allow in the slot for publishing the L1 transaction.',

0 commit comments

Comments
 (0)