Skip to content

Commit

Permalink
fix: Validator ignores block limits during reexec (#11288)
Browse files Browse the repository at this point in the history
Fixes #11827

Also enables the max block gas limit, which was disconnected to the
public processor.
  • Loading branch information
spalladino authored Jan 17, 2025
1 parent a6a226e commit 920a521
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 11 deletions.
43 changes: 42 additions & 1 deletion yarn-project/sequencer-client/src/sequencer/sequencer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
type ContractDataSource,
EthAddress,
Fr,
type Gas,
GasFees,
GlobalVariables,
NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP,
Expand All @@ -35,7 +36,7 @@ import { Buffer32 } from '@aztec/foundation/buffer';
import { times } from '@aztec/foundation/collection';
import { Signature } from '@aztec/foundation/eth-signature';
import { type Logger, createLogger } from '@aztec/foundation/log';
import { TestDateProvider } from '@aztec/foundation/timer';
import { TestDateProvider, type Timer } from '@aztec/foundation/timer';
import { type P2P, P2PClientState } from '@aztec/p2p';
import { type BlockBuilderFactory } from '@aztec/prover-client/block-builder';
import { type PublicProcessor, type PublicProcessorFactory } from '@aztec/simulator/server';
Expand Down Expand Up @@ -265,6 +266,29 @@ describe('sequencer', () => {
expectPublisherProposeL2Block([txHash]);
});

it('builds a block for proposal setting limits', async () => {
const txs = times(5, i => makeTx(i * 0x10000));
await sequencer.buildBlock(txs, globalVariables, undefined, { validateOnly: false });

expect(publicProcessor.process).toHaveBeenCalledWith(
txs,
{
deadline: expect.any(Date),
maxTransactions: 4,
maxBlockSize: expect.any(Number),
maxBlockGas: expect.anything(),
},
expect.anything(),
);
});

it('builds a block for validation ignoring limits', async () => {
const txs = times(5, i => makeTx(i * 0x10000));
await sequencer.buildBlock(txs, globalVariables, undefined, { validateOnly: true });

expect(publicProcessor.process).toHaveBeenCalledWith(txs, { deadline: expect.any(Date) }, expect.anything());
});

it('does not build a block if it does not have enough time left in the slot', async () => {
// Trick the sequencer into thinking that we are just too far into slot 1
sequencer.setL1GenesisTime(
Expand Down Expand Up @@ -664,4 +688,21 @@ class TestSubject extends Sequencer {
this.setState(SequencerState.IDLE, 0n, true /** force */);
return super.doRealWork();
}

public override buildBlock(
pendingTxs: Iterable<Tx>,
newGlobalVariables: GlobalVariables,
historicalHeader?: BlockHeader | undefined,
opts?: { validateOnly?: boolean | undefined },
): Promise<{
block: L2Block;
publicGas: Gas;
publicProcessorDuration: number;
numMsgs: number;
numTxs: number;
numFailedTxs: number;
blockBuildingTimer: Timer;
}> {
return super.buildBlock(pendingTxs, newGlobalVariables, historicalHeader, opts);
}
}
25 changes: 15 additions & 10 deletions yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ export class Sequencer {
private runningPromise?: RunningPromise;
private pollingIntervalMs: number = 1000;
private maxTxsPerBlock = 32;
private minTxsPerBLock = 1;
private minTxsPerBlock = 1;
private maxL1TxInclusionTimeIntoSlot = 0;
// TODO: zero values should not be allowed for the following 2 values in PROD
private _coinbase = EthAddress.ZERO;
private _feeRecipient = AztecAddress.ZERO;
private state = SequencerState.STOPPED;
private allowedInSetup: AllowedElement[] = getDefaultAllowedSetupFunctions();
private maxBlockSizeInBytes: number = 1024 * 1024;
private maxBlockGas: Gas = new Gas(10e9, 10e9);
private maxBlockGas: Gas = new Gas(100e9, 100e9);
private metrics: SequencerMetrics;
private isFlushing: boolean = false;

Expand Down Expand Up @@ -126,7 +126,7 @@ export class Sequencer {
this.maxTxsPerBlock = config.maxTxsPerBlock;
}
if (config.minTxsPerBlock !== undefined) {
this.minTxsPerBLock = config.minTxsPerBlock;
this.minTxsPerBlock = config.minTxsPerBlock;
}
if (config.maxDABlockGas !== undefined) {
this.maxBlockGas = new Gas(config.maxDABlockGas, this.maxBlockGas.l2Gas);
Expand Down Expand Up @@ -267,8 +267,8 @@ export class Sequencer {

// Check the pool has enough txs to build a block
const pendingTxCount = this.p2pClient.getPendingTxCount();
if (pendingTxCount < this.minTxsPerBLock && !this.isFlushing) {
this.log.verbose(`Not enough txs to propose block. Got ${pendingTxCount} min ${this.minTxsPerBLock}.`, {
if (pendingTxCount < this.minTxsPerBlock && !this.isFlushing) {
this.log.verbose(`Not enough txs to propose block. Got ${pendingTxCount} min ${this.minTxsPerBlock}.`, {
slot,
blockNumber: newBlockNumber,
});
Expand Down Expand Up @@ -375,7 +375,7 @@ export class Sequencer {
* @param historicalHeader - The historical header of the parent
* @param opts - Whether to just validate the block as a validator, as opposed to building it as a proposal
*/
private async buildBlock(
protected async buildBlock(
pendingTxs: Iterable<Tx>,
newGlobalVariables: GlobalVariables,
historicalHeader?: BlockHeader,
Expand Down Expand Up @@ -443,7 +443,12 @@ export class Sequencer {
// TODO(#11000): Public processor should just handle processing, one tx at a time. It should be responsibility
// of the sequencer to update world state and iterate over txs. We should refactor this along with unifying the
// publicProcessorFork and orchestratorFork, to avoid doing tree insertions twice when building the block.
const limits = { deadline, maxTransactions: this.maxTxsPerBlock, maxBlockSize: this.maxBlockSizeInBytes };
const proposerLimits = {
maxTransactions: this.maxTxsPerBlock,
maxBlockSize: this.maxBlockSizeInBytes,
maxBlockGas: this.maxBlockGas,
};
const limits = opts.validateOnly ? { deadline } : { deadline, ...proposerLimits };
const [publicProcessorDuration, [processedTxs, failedTxs]] = await elapsed(() =>
processor.process(pendingTxs, limits, validators),
);
Expand All @@ -457,11 +462,11 @@ export class Sequencer {
if (
!opts.validateOnly && // We check for minTxCount only if we are proposing a block, not if we are validating it
!this.isFlushing && // And we skip the check when flushing, since we want all pending txs to go out, no matter if too few
this.minTxsPerBLock !== undefined &&
processedTxs.length < this.minTxsPerBLock
this.minTxsPerBlock !== undefined &&
processedTxs.length < this.minTxsPerBlock
) {
this.log.warn(
`Block ${blockNumber} has too few txs to be proposed (got ${processedTxs.length} but required ${this.minTxsPerBLock})`,
`Block ${blockNumber} has too few txs to be proposed (got ${processedTxs.length} but required ${this.minTxsPerBlock})`,
{ slot, blockNumber, processedTxCount: processedTxs.length },
);
throw new Error(`Block has too few successful txs to be proposed`);
Expand Down

0 comments on commit 920a521

Please sign in to comment.