Add validator option not to submit attestation early (#3944)

* Add validator option not to submit attestation early

* Add earlyAttestationDelayMs option

* Review PR

* Fix sleep unit test

Co-authored-by: dapplion <[email protected]>

packages/cli/src/cmds/validator/handler.ts

@@ -129,7 +129,15 @@ export async function validatorHandler(args: IValidatorCliArgs & IGlobalArgs): P
   // It will wait for genesis, so this promise can be potentially very long
 
   const validator = await Validator.initializeFromBeaconNode(
-    {dbOps, slashingProtection, api: args.server, logger, signers, graffiti},
+    {
+      dbOps,
+      slashingProtection,
+      api: args.server,
+      logger,
+      signers,
+      graffiti,
+      afterBlockDelaySlotFraction: args.afterBlockDelaySlotFraction,
+    },
     controller.signal,
     metrics
   );

packages/cli/src/cmds/validator/options.ts

@@ -18,6 +18,7 @@ export type IValidatorCliArgs = IAccountValidatorArgs &
     server: string;
     force: boolean;
     graffiti: string;
+    afterBlockDelaySlotFraction?: number;
     importKeystoresPath?: string[];
     importKeystoresPassword?: string;
     externalSignerUrl?: string;
@@ -61,6 +62,12 @@ export const validatorOptions: ICliCommandOptions<IValidatorCliArgs> = {
     type: "string",
   },
 
+  afterBlockDelaySlotFraction: {
+    hidden: true,
+    description: "Delay before publishing attestations if block comes early, as a fraction of SECONDS_PER_SLOT",
+    type: "number",
+  },
+
   importKeystoresPath: {
     description: "Path(s) to a directory or single filepath to validator keystores, i.e. Launchpad validators",
     defaultDescription: "./keystores/*.json",

packages/utils/src/sleep.ts

@@ -6,7 +6,9 @@ import {ErrorAborted} from "./errors";
  * On abort throws ErrorAborted
  */
 export async function sleep(ms: number, signal?: AbortSignal): Promise<void> {
-  if (ms < 0) return;
+  if (ms <= 0) {
+    return;
+  }
 
   return new Promise((resolve, reject) => {
     if (signal && signal.aborted) return reject(new ErrorAborted());

packages/utils/test/unit/sleep.test.ts

@@ -29,6 +29,6 @@ describe("sleep", function () {
     controller.abort();
     expect(controller.signal.aborted, "Signal should already be aborted").to.be.true;
 
-    await expect(sleep(0, controller.signal)).to.rejectedWith(ErrorAborted);
+    await expect(sleep(10, controller.signal)).to.rejectedWith(ErrorAborted);
   });
 });

packages/validator/src/services/attestation.ts

@@ -14,6 +14,10 @@ import {ValidatorEvent, ValidatorEventEmitter} from "./emitter";
 import {PubkeyHex} from "../types";
 import {Metrics} from "../metrics";
 
+type AttestationServiceOpts = {
+  afterBlockDelaySlotFraction?: number;
+};
+
 /**
  * Service that sets up and handles validator attester duties.
  */
@@ -28,7 +32,8 @@ export class AttestationService {
     private readonly emitter: ValidatorEventEmitter,
     indicesService: IndicesService,
     chainHeadTracker: ChainHeaderTracker,
-    private readonly metrics: Metrics | null
+    private readonly metrics: Metrics | null,
+    private readonly opts?: AttestationServiceOpts
   ) {
     this.dutiesService = new AttestationDutiesService(
       logger,
@@ -148,19 +153,27 @@ export class AttestationService {
       })
     );
 
+    // signAndPublishAttestations() may be called before the 1/3 cutoff time if the block was received early.
+    // If we produced the block or we got the block sooner than our peers, our attestations can be dropped because
+    // they reach our peers before the block. To prevent that, we wait 2 extra seconds AFTER block arrival, but
+    // never beyond the 1/3 cutoff time.
+    // https://github.com/status-im/nimbus-eth2/blob/7b64c1dce4392731a4a59ee3a36caef2e0a8357a/beacon_chain/validators/validator_duties.nim#L1123
+    const msToOneThirdSlot = this.clock.msToSlot(slot + 1 / 3);
+    // Default = 6, which is half of attestation offset
+    const afterBlockDelayMs = (1000 * this.clock.secondsPerSlot) / (this.opts?.afterBlockDelaySlotFraction ?? 6);
+    await sleep(Math.min(msToOneThirdSlot, afterBlockDelayMs));
+
     this.metrics?.attesterStepCallPublishAttestation.observe(this.clock.secFromSlot(slot + 1 / 3));
 
     // Step 2. Publish all `Attestations` in one go
-    if (signedAttestations.length > 0) {
-      try {
-        await this.api.beacon.submitPoolAttestations(signedAttestations);
-        this.logger.info("Published attestations", {slot, count: signedAttestations.length});
-        this.metrics?.publishedAttestations.inc(signedAttestations.length);
-      } catch (e) {
-        // Note: metric counts only 1 since we don't know how many signedAttestations are invalid
-        this.metrics?.attestaterError.inc({error: "publish"});
-        this.logger.error("Error publishing attestations", {slot}, e as Error);
-      }
+    try {
+      await this.api.beacon.submitPoolAttestations(signedAttestations);
+      this.logger.info("Published attestations", {slot, count: signedAttestations.length});
+      this.metrics?.publishedAttestations.inc(signedAttestations.length);
+    } catch (e) {
+      // Note: metric counts only 1 since we don't know how many signedAttestations are invalid
+      this.metrics?.attestaterError.inc({error: "publish"});
+      this.logger.error("Error publishing attestations", {slot}, e as Error);
     }
   }
 

packages/validator/src/services/block.ts

@@ -10,6 +10,10 @@ import {BlockDutiesService, GENESIS_SLOT} from "./blockDuties";
 import {PubkeyHex} from "../types";
 import {Metrics} from "../metrics";
 
+type BlockProposingServiceOpts = {
+  graffiti?: string;
+};
+
 /**
  * Service that sets up and handles validator block proposal duties.
  */
@@ -23,7 +27,7 @@ export class BlockProposingService {
     private readonly clock: IClock,
     private readonly validatorStore: ValidatorStore,
     private readonly metrics: Metrics | null,
-    private readonly graffiti?: string
+    private readonly opts: BlockProposingServiceOpts
   ) {
     this.dutiesService = new BlockDutiesService(
       logger,
@@ -66,7 +70,7 @@ export class BlockProposingService {
     // Wrap with try catch here to re-use `logCtx`
     try {
       const randaoReveal = await this.validatorStore.signRandao(pubkey, slot);
-      const graffiti = this.graffiti || "";
+      const graffiti = this.opts.graffiti || "";
       const debugLogCtx = {...logCtx, validator: pubkeyHex};
 
       this.logger.debug("Producing block", debugLogCtx);

packages/validator/src/util/clock.ts

@@ -9,6 +9,7 @@ type RunEveryFn = (slot: Slot, signal: AbortSignal) => Promise<void>;
 
 export interface IClock {
   readonly genesisTime: number;
+  readonly secondsPerSlot: number;
   start(signal: AbortSignal): void;
   runEverySlot(fn: (slot: Slot, signal: AbortSignal) => Promise<void>): void;
   runEveryEpoch(fn: (epoch: Epoch, signal: AbortSignal) => Promise<void>): void;
@@ -23,12 +24,14 @@ export enum TimeItem {
 
 export class Clock implements IClock {
   readonly genesisTime: number;
+  readonly secondsPerSlot: number;
   private readonly config: IChainForkConfig;
   private readonly logger: ILogger;
   private readonly fns: {timeItem: TimeItem; fn: RunEveryFn}[] = [];
 
   constructor(config: IChainForkConfig, logger: ILogger, opts: {genesisTime: number}) {
     this.genesisTime = opts.genesisTime;
+    this.secondsPerSlot = config.SECONDS_PER_SLOT;
     this.config = config;
     this.logger = logger;
   }

packages/validator/src/validator.ts

@@ -28,6 +28,7 @@ export type ValidatorOptions = {
   api: Api | string;
   signers: Signer[];
   logger: ILogger;
+  afterBlockDelaySlotFraction?: number;
   graffiti?: string;
 };
 
@@ -78,30 +79,25 @@ export class Validator {
 
     const clock = new Clock(config, logger, {genesisTime: Number(genesis.genesisTime)});
     const validatorStore = new ValidatorStore(config, slashingProtection, metrics, signers, genesis);
-    this.indicesService = new IndicesService(logger, api, validatorStore, metrics);
-    this.emitter = new ValidatorEventEmitter();
-    this.chainHeaderTracker = new ChainHeaderTracker(logger, api, this.emitter);
+    const indicesService = new IndicesService(logger, api, validatorStore, metrics);
+    const emitter = new ValidatorEventEmitter();
+    const chainHeaderTracker = new ChainHeaderTracker(logger, api, emitter);
     const loggerVc = getLoggerVc(logger, clock);
 
-    this.blockProposingService = new BlockProposingService(
-      config,
-      loggerVc,
-      api,
-      clock,
-      validatorStore,
-      metrics,
-      graffiti
-    );
+    this.blockProposingService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, metrics, {
+      graffiti,
+    });
 
     this.attestationService = new AttestationService(
       loggerVc,
       api,
       clock,
       validatorStore,
-      this.emitter,
-      this.indicesService,
-      this.chainHeaderTracker,
-      metrics
+      emitter,
+      indicesService,
+      chainHeaderTracker,
+      metrics,
+      {afterBlockDelaySlotFraction: opts.afterBlockDelaySlotFraction}
     );
 
     this.syncCommitteeService = new SyncCommitteeService(
@@ -110,8 +106,8 @@ export class Validator {
       api,
       clock,
       validatorStore,
-      this.chainHeaderTracker,
-      this.indicesService,
+      chainHeaderTracker,
+      indicesService,
       metrics
     );
 
@@ -120,6 +116,9 @@ export class Validator {
     this.api = api;
     this.clock = clock;
     this.validatorStore = validatorStore;
+    this.indicesService = indicesService;
+    this.emitter = emitter;
+    this.chainHeaderTracker = chainHeaderTracker;
   }
 
   /** Waits for genesis and genesis time */

packages/validator/test/unit/services/block.test.ts

@@ -48,7 +48,7 @@ describe("BlockDutiesService", function () {
     api.validator.getProposerDuties.resolves(duties);
 
     const clock = new ClockMock();
-    const blockService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, null);
+    const blockService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, null, {});
 
     const signedBlock = generateEmptySignedBlock();
     validatorStore.signRandao.resolves(signedBlock.message.body.randaoReveal);

packages/validator/test/utils/clock.ts

@@ -6,6 +6,7 @@ type RunEveryFn = (slot: Slot, signal: AbortSignal) => Promise<void>;
 
 export class ClockMock implements IClock {
   readonly genesisTime = 0;
+  readonly secondsPerSlot = 12;
 
   private readonly everySlot: RunEveryFn[] = [];
   private readonly everyEpoch: RunEveryFn[] = [];