chore!: Attestation collection times out based on sequencer timetable (…

…#11248)

Removes the env var `VALIDATOR_ATTESTATIONS_WAIT_TIMEOUT_MS` and
replaces it with the deadline for publishing the block to L1. This
ensures the sequencer does not get stuck waiting for attestations where
the validators have given up due to a reexecution timeout.

If timetable is not enforced, this value defaults to one aztec slot.

spartan/releases/README.md

@@ -83,7 +83,6 @@ The Sequencer Client is a criticial component that coordinates tx validation, L2
 |Variable| Description|
 |----|-----|
 | VALIDATOR_DISABLED | If this is True, the client won't perform any validator duties. |
-|VALIDATOR_ATTESTATIONS_WAIT_TIMEOUT_MS | Wait for attestations timeout. After this, client throws an error and does not propose a block for that slot. |
 | VALIDATOR_ATTESTATIONS_POLLING_INTERVAL_MS | If not enough attestations, sleep for this long and check again |
 |GOVERNANCE_PROPOSER_PAYLOAD_ADDRESS | To nominate proposals for voting, you must set this variable to the Ethereum address of the `proposal` payload. You must edit this to vote on a governance upgrade.|
 | SEQ_ENFORCE_TIME_TABLE | Whether to enforce strict timeliness requirement when building blocks. Refer [here](#sequencer-timeliness-requirements) for more on the timetable |

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

@@ -160,7 +160,6 @@ export type EnvVar =
   | 'TX_GOSSIP_VERSION'
   | 'TXE_PORT'
   | 'VALIDATOR_ATTESTATIONS_POLLING_INTERVAL_MS'
-  | 'VALIDATOR_ATTESTATIONS_WAIT_TIMEOUT_MS'
   | 'VALIDATOR_DISABLED'
   | 'VALIDATOR_PRIVATE_KEY'
   | 'VALIDATOR_REEXECUTE'

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

@@ -641,7 +641,15 @@ export class Sequencer {
     this.log.debug('Broadcasting block proposal to validators');
     this.validatorClient.broadcastBlockProposal(proposal);
 
-    const attestations = await this.validatorClient.collectAttestations(proposal, numberOfRequiredAttestations);
+    const attestationTimeAllowed = this.enforceTimeTable
+      ? this.timetable.getMaxAllowedTime(SequencerState.PUBLISHING_BLOCK)!
+      : this.aztecSlotDuration;
+    const attestationDeadline = new Date(this.dateProvider.now() + attestationTimeAllowed * 1000);
+    const attestations = await this.validatorClient.collectAttestations(
+      proposal,
+      numberOfRequiredAttestations,
+      attestationDeadline,
+    );
 
     // note: the smart contract requires that the signatures are provided in the order of the committee
     return orderAttestations(attestations, committee);

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

@@ -1,10 +1,9 @@
-import { NULL_KEY, l1ContractsConfigMappings } from '@aztec/ethereum';
+import { NULL_KEY } from '@aztec/ethereum';
 import {
   type ConfigMappingsType,
   booleanConfigHelper,
   getConfigFromMappings,
   numberConfigHelper,
-  pickConfigMappings,
 } from '@aztec/foundation/config';
 
 /**
@@ -20,9 +19,6 @@ export interface ValidatorClientConfig {
   /** Interval between polling for new attestations from peers */
   attestationPollingIntervalMs: number;
 
-  /** Wait for attestations timeout */
-  attestationWaitTimeoutMs: number;
-
   /** Re-execute transactions before attesting */
   validatorReexecute: boolean;
 }
@@ -43,14 +39,6 @@ export const validatorClientConfigMappings: ConfigMappingsType<ValidatorClientCo
     description: 'Interval between polling for new attestations',
     ...numberConfigHelper(200),
   },
-  attestationWaitTimeoutMs: {
-    env: 'VALIDATOR_ATTESTATIONS_WAIT_TIMEOUT_MS',
-    description: 'Wait for attestations timeout',
-    ...numberConfigHelper(
-      getConfigFromMappings(pickConfigMappings(l1ContractsConfigMappings, ['aztecSlotDuration'])).aztecSlotDuration *
-        1000,
-    ),
-  },
   validatorReexecute: {
     env: 'VALIDATOR_REEXECUTE',
     description: 'Re-execute transactions before attesting',

yarn-project/validator-client/src/factory.ts

@@ -24,5 +24,5 @@ export function createValidatorClient(
     config.validatorPrivateKey = generatePrivateKey();
   }
 
-  return ValidatorClient.new(config, deps.epochCache, deps.p2pClient, deps.telemetry);
+  return ValidatorClient.new(config, deps.epochCache, deps.p2pClient, deps.dateProvider, deps.telemetry);
 }

yarn-project/validator-client/src/validator.test.ts

@@ -7,8 +7,8 @@ import { type EpochCache } from '@aztec/epoch-cache';
 import { Secp256k1Signer } from '@aztec/foundation/crypto';
 import { EthAddress } from '@aztec/foundation/eth-address';
 import { Fr } from '@aztec/foundation/fields';
+import { TestDateProvider } from '@aztec/foundation/timer';
 import { type P2P } from '@aztec/p2p';
-import { NoopTelemetryClient } from '@aztec/telemetry-client/noop';
 
 import { describe, expect, it } from '@jest/globals';
 import { type MockProxy, mock } from 'jest-mock-extended';
@@ -30,28 +30,29 @@ describe('ValidationService', () => {
   let p2pClient: MockProxy<P2P>;
   let epochCache: MockProxy<EpochCache>;
   let validatorAccount: PrivateKeyAccount;
+  let dateProvider: TestDateProvider;
 
   beforeEach(() => {
     p2pClient = mock<P2P>();
     p2pClient.getAttestationsForSlot.mockImplementation(() => Promise.resolve([]));
     epochCache = mock<EpochCache>();
+    dateProvider = new TestDateProvider();
 
     const validatorPrivateKey = generatePrivateKey();
     validatorAccount = privateKeyToAccount(validatorPrivateKey);
 
     config = {
       validatorPrivateKey: validatorPrivateKey,
       attestationPollingIntervalMs: 1000,
-      attestationWaitTimeoutMs: 1000,
       disableValidator: false,
       validatorReexecute: false,
     };
-    validatorClient = ValidatorClient.new(config, epochCache, p2pClient, new NoopTelemetryClient());
+    validatorClient = ValidatorClient.new(config, epochCache, p2pClient, dateProvider);
   });
 
   it('Should throw error if an invalid private key is provided', () => {
     config.validatorPrivateKey = '0x1234567890123456789';
-    expect(() => ValidatorClient.new(config, epochCache, p2pClient, new NoopTelemetryClient())).toThrow(
+    expect(() => ValidatorClient.new(config, epochCache, p2pClient, dateProvider)).toThrow(
       InvalidValidatorPrivateKeyError,
     );
   });
@@ -79,7 +80,9 @@ describe('ValidationService', () => {
   it('Should a timeout if we do not collect enough attestations in time', async () => {
     const proposal = makeBlockProposal();
 
-    await expect(validatorClient.collectAttestations(proposal, 2)).rejects.toThrow(AttestationTimeoutError);
+    await expect(validatorClient.collectAttestations(proposal, 2, new Date(dateProvider.now() + 100))).rejects.toThrow(
+      AttestationTimeoutError,
+    );
   });
 
   it('Should throw an error if the transactions are not available', async () => {
@@ -200,7 +203,11 @@ describe('ValidationService', () => {
 
     // Perform the query
     const numberOfRequiredAttestations = 3;
-    const attestations = await validatorClient.collectAttestations(proposal, numberOfRequiredAttestations);
+    const attestations = await validatorClient.collectAttestations(
+      proposal,
+      numberOfRequiredAttestations,
+      new Date(dateProvider.now() + 5000),
+    );
 
     expect(attestations).toHaveLength(numberOfRequiredAttestations);
   });

yarn-project/validator-client/src/validator.ts

@@ -6,7 +6,7 @@ import { type Fr } from '@aztec/foundation/fields';
 import { createLogger } from '@aztec/foundation/log';
 import { RunningPromise } from '@aztec/foundation/running-promise';
 import { sleep } from '@aztec/foundation/sleep';
-import { type Timer } from '@aztec/foundation/timer';
+import { DateProvider, type Timer } from '@aztec/foundation/timer';
 import { type P2P } from '@aztec/p2p';
 import { BlockProposalValidator } from '@aztec/p2p/msg_validators';
 import { type TelemetryClient, WithTracer } from '@aztec/telemetry-client';
@@ -55,7 +55,7 @@ export interface Validator {
   attestToProposal(proposal: BlockProposal): void;
 
   broadcastBlockProposal(proposal: BlockProposal): void;
-  collectAttestations(proposal: BlockProposal, numberOfRequiredAttestations: number): Promise<BlockAttestation[]>;
+  collectAttestations(proposal: BlockProposal, required: number, deadline: Date): Promise<BlockAttestation[]>;
 }
 
 /**
@@ -80,6 +80,7 @@ export class ValidatorClient extends WithTracer implements Validator {
     private epochCache: EpochCache,
     private p2pClient: P2P,
     private config: ValidatorClientConfig,
+    private dateProvider: DateProvider = new DateProvider(),
     telemetry: TelemetryClient = new NoopTelemetryClient(),
     private log = createLogger('validator'),
   ) {
@@ -119,6 +120,7 @@ export class ValidatorClient extends WithTracer implements Validator {
     config: ValidatorClientConfig,
     epochCache: EpochCache,
     p2pClient: P2P,
+    dateProvider: DateProvider = new DateProvider(),
     telemetry: TelemetryClient = new NoopTelemetryClient(),
   ) {
     if (!config.validatorPrivateKey) {
@@ -128,7 +130,7 @@ export class ValidatorClient extends WithTracer implements Validator {
     const privateKey = validatePrivateKey(config.validatorPrivateKey);
     const localKeyStore = new LocalKeyStore(privateKey);
 
-    const validator = new ValidatorClient(localKeyStore, epochCache, p2pClient, config, telemetry);
+    const validator = new ValidatorClient(localKeyStore, epochCache, p2pClient, config, dateProvider, telemetry);
     validator.registerBlockProposalHandler();
     return validator;
   }
@@ -309,19 +311,21 @@ export class ValidatorClient extends WithTracer implements Validator {
   }
 
   // TODO(https://github.com/AztecProtocol/aztec-packages/issues/7962)
-  async collectAttestations(
-    proposal: BlockProposal,
-    numberOfRequiredAttestations: number,
-  ): Promise<BlockAttestation[]> {
+  async collectAttestations(proposal: BlockProposal, required: number, deadline: Date): Promise<BlockAttestation[]> {
     // Wait and poll the p2pClient's attestation pool for this block until we have enough attestations
     const slot = proposal.payload.header.globalVariables.slotNumber.toBigInt();
-    this.log.debug(`Collecting ${numberOfRequiredAttestations} attestations for slot ${slot}`);
+    this.log.debug(`Collecting ${required} attestations for slot ${slot} with deadline ${deadline.toISOString()}`);
+
+    if (+deadline < this.dateProvider.now()) {
+      this.log.error(
+        `Deadline ${deadline.toISOString()} for collecting ${required} attestations for slot ${slot} is in the past`,
+      );
+      throw new AttestationTimeoutError(required, slot);
+    }
 
     const proposalId = proposal.archive.toString();
     const myAttestation = await this.validationService.attestToProposal(proposal);
 
-    const startTime = Date.now();
-
     let attestations: BlockAttestation[] = [];
     while (true) {
       const collectedAttestations = [myAttestation, ...(await this.p2pClient.getAttestationsForSlot(slot, proposalId))];
@@ -333,15 +337,14 @@ export class ValidatorClient extends WithTracer implements Validator {
       }
       attestations = collectedAttestations;
 
-      if (attestations.length >= numberOfRequiredAttestations) {
-        this.log.verbose(`Collected all ${numberOfRequiredAttestations} attestations for slot ${slot}`);
+      if (attestations.length >= required) {
+        this.log.verbose(`Collected all ${required} attestations for slot ${slot}`);
         return attestations;
       }
 
-      const elapsedTime = Date.now() - startTime;
-      if (elapsedTime > this.config.attestationWaitTimeoutMs) {
-        this.log.error(`Timeout waiting for ${numberOfRequiredAttestations} attestations for slot ${slot}`);
-        throw new AttestationTimeoutError(numberOfRequiredAttestations, slot);
+      if (+deadline < this.dateProvider.now()) {
+        this.log.error(`Timeout ${deadline.toISOString()} waiting for ${required} attestations for slot ${slot}`);
+        throw new AttestationTimeoutError(required, slot);
       }
 
       this.log.debug(`Collected ${attestations.length} attestations so far`);