Skip to content

Commit 823b9e6

Browse files
committed
refactor(sequencer): simplify canonicality guard and governance proposer 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).
1 parent dfd53f4 commit 823b9e6

6 files changed

Lines changed: 15 additions & 45 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ export interface IEmpireBase {
2121
chainId: number,
2222
signerAddress: Hex,
2323
signer: (msg: TypedDataDefinition) => Promise<Hex>,
24-
instance?: Hex,
2524
): Promise<L1TxRequest>;
2625
}
2726

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,6 @@ export class ReadOnlyGovernanceContract {
325325
const block = await this.client.getBlock();
326326
const hardCutoff = block.timestamp - MAX_PROPOSAL_LIFETIME_SECONDS;
327327

328-
let sawExecuted = false;
329-
330328
// Proposals are append-only with monotonically non-decreasing creation timestamps, so iterating
331329
// from newest -> oldest lets us early-stop as soon as we cross the lifetime cutoff.
332330
for (let id = proposalCount - 1n; id >= 0n; id--) {
@@ -350,16 +348,13 @@ export class ReadOnlyGovernanceContract {
350348
}
351349

352350
if (proposal.state === ProposalState.Executed) {
353-
sawExecuted = true;
354351
this.executedPayloads.add(target);
355352
}
353+
356354
// Rejected/Dropped/Expired proposals allow re-proposing the same payload; keep scanning.
357355
}
358356

359-
if (sawExecuted || this.executedPayloads.has(target)) {
360-
return 'executed';
361-
}
362-
return 'none';
357+
return this.executedPayloads.has(target) ? 'executed' : 'none';
363358
}
364359

365360
/**

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -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
}
@@ -102,13 +98,12 @@ export class GovernanceProposerContract implements IEmpireBase {
10298
chainId: number,
10399
signerAddress: Hex,
104100
signer: (msg: TypedDataDefinition) => Promise<Hex>,
105-
instance?: Hex,
106101
): Promise<L1TxRequest> {
107102
const signature = await signSignalWithSig(
108103
signer,
109104
payload,
110105
slot,
111-
instance ?? (await this.getInstance()),
106+
(await this.getRollupAddress()).toString(),
112107
this.address.toString(),
113108
chainId,
114109
);

yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ describe('SequencerPublisher', () => {
157157

158158
governanceProposerContract = mock<GovernanceProposerContract>();
159159
// By default the configured rollup is the canonical instance, so the canonicality guard passes.
160-
governanceProposerContract.getInstance.mockResolvedValue(mockRollupAddress);
160+
governanceProposerContract.getRollupAddress.mockResolvedValue(EthAddress.fromString(mockRollupAddress));
161161
governanceProposerContract.getPayloadProposalStatus.mockResolvedValue('none');
162162

163163
epochCache = mock<EpochCache>();
@@ -1051,7 +1051,7 @@ describe('SequencerPublisher', () => {
10511051

10521052
it('does not signal when the configured rollup is not canonical', async () => {
10531053
const { govPayload } = mockGovernancePayload();
1054-
governanceProposerContract.getInstance.mockResolvedValue(EthAddress.random().toString());
1054+
governanceProposerContract.getRollupAddress.mockResolvedValue(EthAddress.random());
10551055

10561056
expect(
10571057
await publisher.enqueueGovernanceCastSignal(

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

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -961,37 +961,21 @@ export class SequencerPublisher {
961961
this.log.warn(`Cannot enqueue vote cast signal ${signalType} for address zero at slot ${slotNumber}`);
962962
return false;
963963
}
964-
// Resolve the canonical rollup instance once and reuse it for round accounting and the EIP-712
965-
// digest, so this signal can't straddle a canonical-rollup change between reads. Only the
966-
// canonical rollup's current proposer can signal, so a non-canonical node would only waste gas
967-
// on a reverting signal. Fail open (proceed with the configured rollup) if the lookup errors,
968-
// for the same liveness reason as the proposal-status check below.
969-
let canonicalInstance: Hex | undefined;
970-
try {
971-
canonicalInstance = await base.getInstance();
972-
} catch (err) {
973-
this.log.error(`Failed to resolve canonical rollup instance (signalling anyway)`, err, {
974-
slotNumber,
975-
signalType,
976-
});
977-
}
978964

979-
if (
980-
canonicalInstance !== undefined &&
981-
!EthAddress.fromString(canonicalInstance).equals(EthAddress.fromString(this.rollupContract.address.toString()))
982-
) {
965+
const canonicalRollup = await base.getRollupAddress();
966+
if (!canonicalRollup.equals(EthAddress.fromString(this.rollupContract.address))) {
983967
this.log.warn(`Rollup ${this.rollupContract.address} is not canonical, skipping governance signal`, {
984968
slotNumber,
985969
signalType,
986-
canonicalInstance,
970+
canonicalRollup,
971+
targetRollup: this.rollupContract.address,
987972
payload: payload.toString(),
988973
});
989974
return false;
990975
}
991976

992-
const rollupForRound = canonicalInstance ?? this.rollupContract.address;
993977
const round = await base.computeRound(slotNumber);
994-
const roundInfo = await base.getRoundInfo(rollupForRound, round);
978+
const roundInfo = await base.getRoundInfo(this.rollupContract.address, round);
995979

996980
if (roundInfo.quorumReached) {
997981
return false;
@@ -1051,7 +1035,6 @@ export class SequencerPublisher {
10511035
this.config.l1ChainId,
10521036
signerAddress.toString(),
10531037
signer,
1054-
canonicalInstance,
10551038
);
10561039
this.log.debug(`Created ${action} request with signature`, {
10571040
request,

yarn-project/sequencer-client/src/sequencer/checkpoint_voter.ha.integration.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,7 @@ describe('CheckpointVoter HA Integration', () => {
7272
function createMockGovernanceContract(): MockProxy<GovernanceProposerContract> {
7373
const contract = mock<GovernanceProposerContract>();
7474
Object.defineProperty(contract, 'address', { value: EthAddress.random(), writable: false });
75-
// The configured rollup is the canonical instance, so the publisher's canonicality guard passes.
76-
contract.getInstance.mockResolvedValue(rollupContract.address.toString() as `0x${string}`);
75+
contract.getRollupAddress.mockResolvedValue(EthAddress.fromString(rollupContract.address));
7776
contract.getPayloadProposalStatus.mockResolvedValue('none');
7877
contract.getRoundInfo.mockResolvedValue({
7978
lastSignalSlot: SlotNumber(1),
@@ -179,7 +178,7 @@ describe('CheckpointVoter HA Integration', () => {
179178

180179
// Set up mocks using helper functions
181180
rollupContract = mock<RollupContract>();
182-
Object.defineProperty(rollupContract, 'address', { value: EthAddress.random(), writable: false });
181+
Object.defineProperty(rollupContract, 'address', { value: EthAddress.random().toString(), writable: false });
183182
rollupContract.listenToSlasherChanged.mockReturnValue(undefined as any);
184183
rollupContract.getSlashingProposer.mockResolvedValue(undefined);
185184

@@ -734,7 +733,6 @@ describe('CheckpointVoter HA Integration', () => {
734733
expect.any(Number), // chainId
735734
expect.any(String), // signerAddress
736735
expect.any(Function), // signer function
737-
expect.any(String), // canonical rollup instance
738736
);
739737

740738
// Verify Node A's request was sent to L1 via Multicall3.forward

0 commit comments

Comments
 (0)