Adding MAXIMUM_GOSSIP_CLOCK_DISPARITY for p2p validation

Author: mrzeszutko

docs/docs-network/reference/changelog/v4.md

@@ -57,6 +57,14 @@ aztec add-l1-validator --withdrawer-address <address>
 
 ## New features
 
+### P2P clock tolerance for slot validation
+
+Added a 500ms tolerance window for P2P messages from the previous slot, following Ethereum's `MAXIMUM_GOSSIP_CLOCK_DISPARITY` approach. This prevents peers from being penalized for valid messages that arrive slightly after the slot boundary due to network latency.
+
+The tolerance is hardcoded at 500ms (matching Ethereum's current value) and can be made configurable via environment variables in the future if needed.
+
+**Impact**: Improved network stability with no action required from node operators.
+
 ## Changed defaults
 
 ## Troubleshooting

yarn-project/aztec-node/src/sentinel/sentinel.test.ts

@@ -77,7 +77,7 @@ describe('sentinel', () => {
       proofSubmissionEpochs: 1,
     };
 
-    epochCache.getEpochAndSlotNow.mockReturnValue({ epoch, slot, ts, now: ts });
+    epochCache.getEpochAndSlotNow.mockReturnValue({ epoch, slot, ts, nowMs: ts * 1000n });
     epochCache.getL1Constants.mockReturnValue(l1Constants);
 
     sentinel = new TestSentinel(epochCache, archiver, p2p, store, config, blockStream);
@@ -566,7 +566,7 @@ describe('sentinel', () => {
         epoch: epochNumber,
         slot,
         ts,
-        now: ts,
+        nowMs: ts * 1000n,
       });
       archiver.getL2BlockNew.calledWith(blockNumber).mockResolvedValue(mockBlock);
       archiver.getL1Constants.mockResolvedValue(l1Constants);

yarn-project/epoch-cache/src/epoch_cache.ts

@@ -36,7 +36,7 @@ export type SlotTag = 'now' | 'next' | SlotNumber;
 
 export interface EpochCacheInterface {
   getCommittee(slot: SlotTag | undefined): Promise<EpochCommitteeInfo>;
-  getEpochAndSlotNow(): EpochAndSlot;
+  getEpochAndSlotNow(): EpochAndSlot & { nowMs: bigint };
   getEpochAndSlotInNextL1Slot(): EpochAndSlot & { now: bigint };
   getProposerIndexEncoding(epoch: EpochNumber, slot: SlotNumber, seed: bigint): `0x${string}`;
   computeProposerIndex(slot: SlotNumber, epoch: EpochNumber, seed: bigint, size: bigint): bigint;
@@ -134,9 +134,10 @@ export class EpochCache implements EpochCacheInterface {
     return this.l1constants;
   }
 
-  public getEpochAndSlotNow(): EpochAndSlot & { now: bigint } {
-    const now = this.nowInSeconds();
-    return { ...this.getEpochAndSlotAtTimestamp(now), now };
+  public getEpochAndSlotNow(): EpochAndSlot & { nowMs: bigint } {
+    const nowMs = BigInt(this.dateProvider.now());
+    const nowSeconds = nowMs / 1000n;
+    return { ...this.getEpochAndSlotAtTimestamp(nowSeconds), nowMs };
   }
 
   public nowInSeconds(): bigint {

yarn-project/p2p/src/msg_validators/attestation_validator/attestation_validator.test.ts

@@ -23,8 +23,8 @@ describe('CheckpointAttestationValidator', () => {
     attester = Secp256k1Signer.random();
   });
 
-  it('returns high tolerance error if slot number is not current or next slot', async () => {
-    // Create an attestation for slot 97
+  it('returns high tolerance error if slot number is not current or next slot (outside clock tolerance)', async () => {
+    // Create an attestation for slot 97 (previous slot)
     const header = CheckpointHeader.random({ slotNumber: SlotNumber(97) });
     const mockAttestation = makeCheckpointAttestation({
       header,
@@ -37,12 +37,47 @@ describe('CheckpointAttestationValidator', () => {
       currentSlot: SlotNumber(98),
       nextSlot: SlotNumber(99),
     });
+    // Mock getEpochAndSlotNow to return time OUTSIDE clock tolerance (1000ms elapsed)
+    (epochCache.getEpochAndSlotNow as jest.Mock).mockReturnValue({
+      epoch: 1,
+      slot: SlotNumber(98),
+      ts: 1000n, // slot started at 1000 seconds
+      nowMs: 1001000n, // 1000ms elapsed, outside 500ms tolerance
+    });
     (epochCache.isInCommittee as jest.Mock).mockResolvedValue(true);
 
     const result = await validator.validate(mockAttestation);
     expect(result).toBe(PeerErrorSeverity.HighToleranceError);
   });
 
+  it('returns undefined if previous slot attestation is within clock tolerance', async () => {
+    // Create an attestation for slot 97 (previous slot)
+    const header = CheckpointHeader.random({ slotNumber: SlotNumber(97) });
+    const mockAttestation = makeCheckpointAttestation({
+      header,
+      attesterSigner: attester,
+      proposerSigner: proposer,
+    });
+
+    // Mock epoch cache - attestation is for previous slot (97) when current is 98
+    (epochCache.getCurrentAndNextSlot as jest.Mock).mockReturnValue({
+      currentSlot: SlotNumber(98),
+      nextSlot: SlotNumber(99),
+    });
+    // Mock getEpochAndSlotNow to return time WITHIN clock tolerance (100ms elapsed)
+    (epochCache.getEpochAndSlotNow as jest.Mock).mockReturnValue({
+      epoch: 1,
+      slot: SlotNumber(98),
+      ts: 1000n, // slot started at 1000 seconds
+      nowMs: 1000100n, // 100ms elapsed, within 500ms tolerance
+    });
+    (epochCache.isInCommittee as jest.Mock).mockResolvedValue(true);
+    (epochCache.getProposerAttesterAddressInSlot as jest.Mock).mockResolvedValue(proposer.address);
+
+    const result = await validator.validate(mockAttestation);
+    expect(result).toBeUndefined();
+  });
+
   it('returns high tolerance error if attester is not in committee', async () => {
     // The slot is correct, but the attester is not in the committee
     const header = CheckpointHeader.random({ slotNumber: SlotNumber(100) });

yarn-project/p2p/src/msg_validators/attestation_validator/attestation_validator.ts

@@ -3,6 +3,8 @@ import { NoCommitteeError } from '@aztec/ethereum/contracts';
 import { type Logger, createLogger } from '@aztec/foundation/log';
 import { type CheckpointAttestation, type P2PValidator, PeerErrorSeverity } from '@aztec/stdlib/p2p';
 
+import { isWithinClockTolerance } from '../clock_tolerance.js';
+
 export class CheckpointAttestationValidator implements P2PValidator<CheckpointAttestation> {
   protected epochCache: EpochCacheInterface;
   protected logger: Logger;
@@ -19,10 +21,14 @@ export class CheckpointAttestationValidator implements P2PValidator<CheckpointAt
       const { currentSlot, nextSlot } = this.epochCache.getCurrentAndNextSlot();
 
       if (slotNumber !== currentSlot && slotNumber !== nextSlot) {
-        this.logger.warn(
-          `Checkpoint attestation slot ${slotNumber} is not current (${currentSlot}) or next (${nextSlot}) slot`,
-        );
-        return PeerErrorSeverity.HighToleranceError;
+        // Check if message is for previous slot and within clock tolerance
+        if (!isWithinClockTolerance(slotNumber, currentSlot, this.epochCache)) {
+          this.logger.warn(
+            `Checkpoint attestation slot ${slotNumber} is not current (${currentSlot}) or next (${nextSlot}) slot`,
+          );
+          return PeerErrorSeverity.HighToleranceError;
+        }
+        this.logger.debug(`Accepting checkpoint attestation for previous slot ${slotNumber} within clock tolerance`);
       }
 
       // Verify the signature is valid

yarn-project/p2p/src/msg_validators/clock_tolerance.test.ts

@@ -0,0 +1,186 @@
+import type { EpochCacheInterface } from '@aztec/epoch-cache';
+import { SlotNumber } from '@aztec/foundation/branded-types';
+
+import { mock } from 'jest-mock-extended';
+
+import { MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS, isWithinClockTolerance } from './clock_tolerance.js';
+
+describe('clock_tolerance', () => {
+  describe('MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS', () => {
+    it('is set to 500ms', () => {
+      expect(MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS).toBe(500);
+    });
+  });
+
+  describe('isWithinClockTolerance', () => {
+    let epochCache: ReturnType<typeof mock<EpochCacheInterface>>;
+
+    beforeEach(() => {
+      epochCache = mock<EpochCacheInterface>();
+    });
+
+    it('returns true for previous slot message within tolerance window (100ms elapsed)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(99); // previous slot
+
+      // Slot started at 1000 seconds (1000000ms), now is 1000100ms (100ms elapsed)
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n, // seconds
+        nowMs: 1000100n, // 100ms after slot start
+      });
+
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(true);
+    });
+
+    it('returns true at exactly 499ms elapsed (just under tolerance)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(99);
+
+      // 499ms elapsed - should be within tolerance (499 < 500)
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1000499n,
+      });
+
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(true);
+    });
+
+    it('returns false at exactly 500ms elapsed (at boundary)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(99);
+
+      // 500ms elapsed - should be outside tolerance (500 is NOT < 500)
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1000500n,
+      });
+
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(false);
+    });
+
+    it('returns false at 501ms elapsed (just over tolerance)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(99);
+
+      // 501ms elapsed - clearly outside tolerance
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1000501n,
+      });
+
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(false);
+    });
+
+    it('returns false for previous slot message well outside tolerance (1000ms elapsed)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(99);
+
+      // 1000ms elapsed - outside 500ms tolerance
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1001000n,
+      });
+
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(false);
+    });
+
+    it('returns false for message two slots behind (currentSlot - 2)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(98); // two slots behind
+
+      // Even within time tolerance, should reject slots older than previous
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1000000n, // 0ms elapsed
+      });
+
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(false);
+    });
+
+    it('returns false for current slot message (handled by main validation)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(100); // current slot
+
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1000000n,
+      });
+
+      // Current slot messages are handled by the main validation, not clock tolerance
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(false);
+    });
+
+    it('returns false for next slot message (handled by main validation)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(101); // next slot
+
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1000000n,
+      });
+
+      // Next slot messages are handled by the main validation, not clock tolerance
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(false);
+    });
+
+    it('returns false when current slot is 0 (genesis edge case)', () => {
+      const currentSlot = SlotNumber(0);
+      const messageSlot = SlotNumber(0);
+
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 0 as any,
+        slot: currentSlot,
+        ts: 0n,
+        nowMs: 0n,
+      });
+
+      // Cannot have a previous slot when at genesis
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(false);
+    });
+
+    it('returns false for future slot message', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(105); // future slot
+
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1000000n,
+      });
+
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(false);
+    });
+
+    it('returns true at 0ms elapsed (exactly at slot boundary)', () => {
+      const currentSlot = SlotNumber(100);
+      const messageSlot = SlotNumber(99);
+
+      // 0ms elapsed - definitely within tolerance
+      epochCache.getEpochAndSlotNow.mockReturnValue({
+        epoch: 1 as any,
+        slot: currentSlot,
+        ts: 1000n,
+        nowMs: 1000000n, // exactly at slot start
+      });
+
+      expect(isWithinClockTolerance(messageSlot, currentSlot, epochCache)).toBe(true);
+    });
+  });
+});

yarn-project/p2p/src/msg_validators/clock_tolerance.ts

@@ -0,0 +1,45 @@
+import type { EpochCacheInterface } from '@aztec/epoch-cache';
+import { SlotNumber } from '@aztec/foundation/branded-types';
+
+/**
+ * Maximum clock disparity tolerance for P2P message validation (in milliseconds).
+ * Messages for the previous slot are accepted if we're within this many milliseconds
+ * of the current slot start. This prevents penalizing peers for messages that
+ * were valid when sent but arrived slightly late due to network latency.
+ *
+ * This follows Ethereum's MAXIMUM_GOSSIP_CLOCK_DISPARITY approach.
+ */
+export const MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS = 500;
+
+/**
+ * Checks if a message for the previous slot should be accepted due to clock tolerance.
+ *
+ * @param messageSlot - The slot number from the received message
+ * @param currentSlot - The current slot number
+ * @param epochCache - EpochCache to get timing information
+ * @returns true if the message is for the previous slot AND we're within the clock tolerance window
+ */
+export function isWithinClockTolerance(
+  messageSlot: SlotNumber,
+  currentSlot: SlotNumber,
+  epochCache: EpochCacheInterface,
+): boolean {
+  // Guard against slot 0 edge case (genesis)
+  if (currentSlot === SlotNumber(0)) {
+    return false;
+  }
+
+  // Only apply tolerance to messages for the previous slot
+  const previousSlot = SlotNumber(currentSlot - 1);
+  if (messageSlot !== previousSlot) {
+    return false;
+  }
+
+  // Check how far we are into the current slot (in milliseconds)
+  const { ts: slotStartTs, nowMs } = epochCache.getEpochAndSlotNow();
+  // ts is in seconds, convert to ms; nowMs is already in milliseconds
+  const slotStartMs = slotStartTs * 1000n;
+  const elapsedMs = Number(nowMs - slotStartMs);
+
+  return elapsedMs < MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS;
+}

yarn-project/p2p/src/msg_validators/proposal_validator/proposal_validator.ts

@@ -3,6 +3,8 @@ import { NoCommitteeError } from '@aztec/ethereum/contracts';
 import { type Logger, createLogger } from '@aztec/foundation/log';
 import { BlockProposal, CheckpointProposal, PeerErrorSeverity } from '@aztec/stdlib/p2p';
 
+import { isWithinClockTolerance } from '../clock_tolerance.js';
+
 export abstract class ProposalValidator<TProposal extends BlockProposal | CheckpointProposal> {
   protected epochCache: EpochCacheInterface;
   protected logger: Logger;
@@ -20,8 +22,12 @@ export abstract class ProposalValidator<TProposal extends BlockProposal | Checkp
       const { currentSlot, nextSlot } = this.epochCache.getCurrentAndNextSlot();
       const slotNumber = proposal.slotNumber;
       if (slotNumber !== currentSlot && slotNumber !== nextSlot) {
-        this.logger.debug(`Penalizing peer for invalid slot number ${slotNumber}`, { currentSlot, nextSlot });
-        return PeerErrorSeverity.HighToleranceError;
+        // Check if message is for previous slot and within clock tolerance
+        if (!isWithinClockTolerance(slotNumber, currentSlot, this.epochCache)) {
+          this.logger.debug(`Penalizing peer for invalid slot number ${slotNumber}`, { currentSlot, nextSlot });
+          return PeerErrorSeverity.HighToleranceError;
+        }
+        this.logger.debug(`Accepting proposal for previous slot ${slotNumber} within clock tolerance`);
       }
 
       // Signature validity