diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts index d944412263d5..9ec7c638070f 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.test.ts @@ -12,6 +12,7 @@ import { type GasPrice, type L1TxUtils, type L1TxUtilsConfig, + MAX_L1_TX_LIMIT, defaultL1TxUtilsConfig, } from '@aztec/ethereum/l1-tx-utils'; import { FormattedViemError } from '@aztec/ethereum/utils'; @@ -384,6 +385,80 @@ describe('SequencerPublisher', () => { expect((publisher as any).requests.length).toEqual(0); }); + describe('gas budget packing in sendRequests', () => { + const currentSlot = SlotNumber(2); // matches the epochCache mock in beforeEach + + const makeRequest = (action: Action, gasLimit: bigint) => ({ + action, + request: { to: mockRollupAddress as `0x${string}`, data: '0x' as `0x${string}` }, + lastValidL2Slot: currentSlot, + gasConfig: { gasLimit }, + checkSuccess: () => true, + }); + + beforeEach(() => { + forwardSpy.mockResolvedValue({ receipt: proposeTxReceipt, errorMsg: undefined }); + }); + + it('sends all actions when they fit within the gas budget', async () => { + publisher.addRequest(makeRequest('propose', 10_000_000n)); + publisher.addRequest(makeRequest('execute-slash', 6_000_000n)); + + const result = await publisher.sendRequests(); + + expect(forwardSpy).toHaveBeenCalledTimes(1); + expect(forwardSpy.mock.calls[0][0]).toHaveLength(2); + expect(result?.droppedActions).toEqual([]); + expect(result?.sentActions).toEqual(['propose', 'execute-slash']); + }); + + it('drops execute-slash when it would push the bundle over MAX_L1_TX_LIMIT', async () => { + // propose takes almost all the budget; execute-slash cannot fit + publisher.addRequest(makeRequest('propose', MAX_L1_TX_LIMIT - 100n)); + publisher.addRequest(makeRequest('execute-slash', 200n)); + + const result = await publisher.sendRequests(); + + expect(forwardSpy).toHaveBeenCalledTimes(1); + // Only propose should be forwarded + expect(forwardSpy.mock.calls[0][0]).toHaveLength(1); + expect(result?.droppedActions).toEqual(['execute-slash']); + expect(result?.sentActions).toEqual(['propose']); + }); + + it('respects action sort order so high-priority actions claim the budget first', async () => { + // execute-slash is enqueued first but should be sorted after propose. + // With only room for one action, propose (higher priority) must win. + publisher.addRequest(makeRequest('execute-slash', MAX_L1_TX_LIMIT - 100n)); + publisher.addRequest(makeRequest('propose', MAX_L1_TX_LIMIT - 100n)); + + const result = await publisher.sendRequests(); + + expect(forwardSpy).toHaveBeenCalledTimes(1); + expect(forwardSpy.mock.calls[0][0]).toHaveLength(1); + expect(result?.sentActions).toEqual(['propose']); + expect(result?.droppedActions).toEqual(['execute-slash']); + }); + + it('includes actions without a gasConfig without consuming budget', async () => { + // governance-signal has no explicit gasConfig in this test + publisher.addRequest({ + action: 'governance-signal', + request: { to: mockRollupAddress as `0x${string}`, data: '0x' as `0x${string}` }, + lastValidL2Slot: currentSlot, + checkSuccess: () => true, + }); + publisher.addRequest(makeRequest('execute-slash', MAX_L1_TX_LIMIT)); + + const result = await publisher.sendRequests(); + + // Both should be sent: governance-signal contributes 0 to accumulator + expect(result?.droppedActions).toEqual([]); + expect(result?.sentActions).toContain('governance-signal'); + expect(result?.sentActions).toContain('execute-slash'); + }); + }); + it('does not signal for payload when quorum is reached', async () => { const { govPayload } = mockGovernancePayload(); diff --git a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts index bbc0335d29c8..1d64cb71f107 100644 --- a/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts +++ b/yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts @@ -25,7 +25,6 @@ import { WEI_CONST, } from '@aztec/ethereum/l1-tx-utils'; import { FormattedViemError, formatViemError, mergeAbis, tryExtractEvent } from '@aztec/ethereum/utils'; -import { sumBigint } from '@aztec/foundation/bigint'; import { toHex as toPaddedHex } from '@aztec/foundation/bigint-buffer'; import { CheckpointNumber, SlotNumber } from '@aztec/foundation/branded-types'; import { pick } from '@aztec/foundation/collection'; @@ -330,7 +329,6 @@ export class SequencerPublisher { const currentL2Slot = this.getCurrentL2Slot(); this.log.debug(`Sending requests on L2 slot ${currentL2Slot}`); const validRequests = requestsToProcess.filter(request => request.lastValidL2Slot >= currentL2Slot); - const validActions = validRequests.map(x => x.action); const expiredActions = requestsToProcess .filter(request => request.lastValidL2Slot < currentL2Slot) .map(x => x.action); @@ -353,53 +351,68 @@ export class SequencerPublisher { return undefined; } + // Sort the requests so that proposals always go first. + // This ensures the committee gets precomputed correctly, and that high-priority + // actions consume the gas budget before lower-priority ones. + validRequests.sort((a, b) => compareActions(a.action, b.action)); + + // Greedily pack requests into the gas budget (EIP-7825 protocol cap). + // Actions that don't fit are dropped with a warning -- they are re-evaluated + // next slot via getProposerActions() and will be retried until their on-chain + // lifetime expires. This avoids silently capping gas and risking OOG failures. + const droppedActions: Action[] = []; + const requestsToSend: RequestWithExpiry[] = []; + let gasAccumulator = 0n; + for (const request of validRequests) { + const actionGas = request.gasConfig?.gasLimit ?? 0n; + if (gasAccumulator + actionGas > MAX_L1_TX_LIMIT) { + droppedActions.push(request.action); + } else { + requestsToSend.push(request); + gasAccumulator += actionGas; + } + } + + if (droppedActions.length > 0) { + this.log.warn('Dropping actions that exceed gas budget; they will be retried next slot', { + droppedActions, + gasAccumulator, + maxGas: MAX_L1_TX_LIMIT, + }); + } + // @note - we can only have one blob config per bundle - // find requests with gas and blob configs // See https://github.com/AztecProtocol/aztec-packages/issues/11513 - const gasConfigs = requestsToProcess.filter(request => request.gasConfig).map(request => request.gasConfig); - const blobConfigs = requestsToProcess.filter(request => request.blobConfig).map(request => request.blobConfig); - + const blobConfigs = requestsToSend.filter(request => request.blobConfig).map(request => request.blobConfig); if (blobConfigs.length > 1) { throw new Error('Multiple blob configs found'); } - const blobConfig = blobConfigs[0]; - // Merge gasConfigs. Yields the sum of gasLimits, and the earliest txTimeoutAt, or undefined if no gasConfig sets them. - const gasLimits = gasConfigs.map(g => g?.gasLimit).filter((g): g is bigint => g !== undefined); - let gasLimit = gasLimits.length > 0 ? sumBigint(gasLimits) : undefined; // sum - // Cap at L1 block gas limit so the node accepts the tx ("gas limit too high" otherwise). - const maxGas = MAX_L1_TX_LIMIT; - if (gasLimit !== undefined && gasLimit > maxGas) { - this.log.debug('Capping bundled tx gas limit to L1 max', { - requested: gasLimit, - capped: maxGas, - }); - gasLimit = maxGas; - } + // Merge gasConfigs from the requests that will actually be sent. + // Yields the sum of gasLimits, and the earliest txTimeoutAt, or undefined if no gasConfig sets them. + const gasConfigs = requestsToSend.filter(request => request.gasConfig).map(request => request.gasConfig); + const gasLimit = gasAccumulator > 0n ? gasAccumulator : undefined; const txTimeoutAts = gasConfigs.map(g => g?.txTimeoutAt).filter((g): g is Date => g !== undefined); const txTimeoutAt = txTimeoutAts.length > 0 ? new Date(Math.min(...txTimeoutAts.map(g => g.getTime()))) : undefined; // earliest const txConfig: RequestWithExpiry['gasConfig'] = { gasLimit, txTimeoutAt }; - // Sort the requests so that proposals always go first - // This ensures the committee gets precomputed correctly - validRequests.sort((a, b) => compareActions(a.action, b.action)); - + const sentActions = requestsToSend.map(r => r.action); try { this.log.debug('Forwarding transactions', { - validRequests: validRequests.map(request => request.action), + sentActions, txConfig, }); const result = await Multicall3.forward( - validRequests.map(request => request.request), + requestsToSend.map(request => request.request), this.l1TxUtils, txConfig, blobConfig, this.rollupContract.address, this.log, ); - const { successfulActions = [], failedActions = [] } = this.callbackBundledTransactions(validRequests, result); - return { result, expiredActions, sentActions: validActions, successfulActions, failedActions }; + const { successfulActions = [], failedActions = [] } = this.callbackBundledTransactions(requestsToSend, result); + return { result, expiredActions, droppedActions, sentActions, successfulActions, failedActions }; } catch (err) { const viemError = formatViemError(err); this.log.error(`Failed to publish bundled transactions`, viemError); diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts index 8ef5a19129ba..e5aae6bf5609 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts @@ -174,6 +174,7 @@ describe('CheckpointProposalJob', () => { failedActions: [], sentActions: ['propose'], expiredActions: [], + droppedActions: [], }); globalVariableBuilder = mock(); diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.timing.test.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.timing.test.ts index 2e9ebb18219e..8fe8572c822f 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.timing.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.timing.test.ts @@ -399,6 +399,7 @@ describe('CheckpointProposalJob Timing Tests', () => { failedActions: [], sentActions: ['propose'], expiredActions: [], + droppedActions: [], }); globalVariableBuilder = mock(); diff --git a/yarn-project/sequencer-client/src/sequencer/events.ts b/yarn-project/sequencer-client/src/sequencer/events.ts index 7c1e22cf5ca5..968286784fd5 100644 --- a/yarn-project/sequencer-client/src/sequencer/events.ts +++ b/yarn-project/sequencer-client/src/sequencer/events.ts @@ -21,6 +21,7 @@ export type SequencerEvents = { failedActions?: Action[]; sentActions?: Action[]; expiredActions?: Action[]; + droppedActions?: Action[]; }) => void; ['checkpoint-published']: (args: { checkpoint: CheckpointNumber; slot: SlotNumber }) => void; ['checkpoint-error']: (args: { error: Error }) => void; diff --git a/yarn-project/slasher/src/tally_slasher_client.test.ts b/yarn-project/slasher/src/tally_slasher_client.test.ts index 95bab349286a..5470aaceb5e7 100644 --- a/yarn-project/slasher/src/tally_slasher_client.test.ts +++ b/yarn-project/slasher/src/tally_slasher_client.test.ts @@ -372,6 +372,29 @@ describe('TallySlasherClient', () => { expect(slasherContract.isPayloadVetoed).toHaveBeenCalledWith(payloadAddress); }); + it('returns execute-slash again on a subsequent slot if the round was not executed', async () => { + // This tests the re-enqueueing invariant: if execute-slash was dropped from a bundle + // (e.g. due to gas budget overflow), the next slot's getProposerActions() call will + // produce it again because it queries on-chain state fresh each slot. + const currentRound = 5n; + const executableRound = 2n; // currentRound - delay(2) - 1 = 2 + + const slotN = currentRound * BigInt(roundSize); + const slotNplus1 = slotN + 1n; + + // Both calls to getRound return the same unexecuted round (round was NOT executed) + tallySlashingProposer.getRound.mockResolvedValue(executableRoundData); + + const actionsSlot1 = await tallySlasherClient.getProposerActions(SlotNumber.fromBigInt(slotN)); + expect(actionsSlot1).toHaveLength(1); + expectActionExecuteSlash(actionsSlot1[0], executableRound); + + // Simulate the next slot without executing the round + const actionsSlot2 = await tallySlasherClient.getProposerActions(SlotNumber.fromBigInt(slotNplus1)); + expect(actionsSlot2).toHaveLength(1); + expectActionExecuteSlash(actionsSlot2[0], executableRound); + }); + it('should not execute when slashing is disabled', async () => { const currentRound = 5n; const currentSlot = currentRound * BigInt(roundSize);