From 5692c842621da1fbfa46487a3161c768cf213b13 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Fri, 23 Jun 2023 15:14:14 +0200 Subject: [PATCH 01/15] Skip doppelganger detection if validator attested in previous epoch --- .../cli/src/cmds/validator/keymanager/impl.ts | 10 +++---- .../src/services/doppelgangerService.ts | 28 +++++++++++++++---- .../validator/src/services/validatorStore.ts | 6 ++-- .../slashingProtection/attestation/index.ts | 9 +++++- .../validator/src/slashingProtection/index.ts | 6 +++- .../src/slashingProtection/interface.ts | 7 ++++- packages/validator/src/validator.ts | 10 ++++++- .../test/unit/services/doppleganger.test.ts | 15 ++++++++-- .../test/utils/slashingProtectionMock.ts | 3 ++ 9 files changed, 75 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/cmds/validator/keymanager/impl.ts b/packages/cli/src/cmds/validator/keymanager/impl.ts index fc9b1e127a40..f4b28edfb3d1 100644 --- a/packages/cli/src/cmds/validator/keymanager/impl.ts +++ b/packages/cli/src/cmds/validator/keymanager/impl.ts @@ -149,7 +149,7 @@ export class KeymanagerApi implements Api { decryptKeystores.queue( {keystoreStr, password}, - (secretKeyBytes: Uint8Array) => { + async (secretKeyBytes: Uint8Array) => { const secretKey = bls.SecretKey.fromBytes(secretKeyBytes); // Persist the key to disk for restarts, before adding to in-memory store @@ -165,7 +165,7 @@ export class KeymanagerApi implements Api { }); // Add to in-memory store to start validating immediately - this.validator.validatorStore.addSigner({type: SignerType.Local, secretKey}); + await this.validator.validatorStore.addSigner({type: SignerType.Local, secretKey}); statuses[i] = {status: ImportStatus.imported}; }, @@ -292,7 +292,7 @@ export class KeymanagerApi implements Api { async importRemoteKeys( remoteSigners: Pick[] ): ReturnType { - const results = remoteSigners.map(({pubkey, url}): ResponseStatus => { + const importPromises = remoteSigners.map(async ({pubkey, url}): Promise> => { try { if (!isValidatePubkeyHex(pubkey)) { throw Error(`Invalid pubkey ${pubkey}`); @@ -308,7 +308,7 @@ export class KeymanagerApi implements Api { // Else try to add it - this.validator.validatorStore.addSigner({type: SignerType.Remote, pubkey, url}); + await this.validator.validatorStore.addSigner({type: SignerType.Remote, pubkey, url}); this.persistedKeysBackend.writeRemoteKey({ pubkey, @@ -325,7 +325,7 @@ export class KeymanagerApi implements Api { }); return { - data: results, + data: await Promise.all(importPromises), }; } diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index 4b891eb1b4b9..9068591999fe 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -1,7 +1,9 @@ +import {fromHexString} from "@chainsafe/ssz"; import {Epoch, ValidatorIndex} from "@lodestar/types"; import {Api, ApiError, routes} from "@lodestar/api"; import {Logger, sleep} from "@lodestar/utils"; import {computeStartSlotAtEpoch} from "@lodestar/state-transition"; +import {ISlashingProtection} from "../slashingProtection/index.js"; import {ProcessShutdownCallback, PubkeyHex} from "../types.js"; import {IClock} from "../util/index.js"; import {Metrics} from "../metrics.js"; @@ -11,6 +13,7 @@ import {IndicesService} from "./indices.js"; // no other duplicate validators on the network const DEFAULT_REMAINING_DETECTION_EPOCHS = 1; const REMAINING_EPOCHS_IF_DOPPLEGANGER = Infinity; +const REMAINING_EPOCHS_IF_SKIPPED = 0; /** Liveness responses for a given epoch */ type EpochLivenessData = { @@ -42,6 +45,7 @@ export class DoppelgangerService { private readonly clock: IClock, private readonly api: Api, private readonly indicesService: IndicesService, + private readonly slashingProtection: ISlashingProtection, private readonly processShutdownCallback: ProcessShutdownCallback, private readonly metrics: Metrics | null ) { @@ -54,16 +58,30 @@ export class DoppelgangerService { this.logger.info("Doppelganger protection enabled", {detectionEpochs: DEFAULT_REMAINING_DETECTION_EPOCHS}); } - registerValidator(pubkeyHex: PubkeyHex): void { + async registerValidator(pubkeyHex: PubkeyHex): Promise { const {currentEpoch} = this.clock; // Disable doppelganger protection when the validator was initialized before genesis. // There's no activity before genesis, so doppelganger is pointless. - const remainingEpochs = currentEpoch <= 0 ? 0 : DEFAULT_REMAINING_DETECTION_EPOCHS; + let remainingEpochs = currentEpoch <= 0 ? REMAINING_EPOCHS_IF_SKIPPED : DEFAULT_REMAINING_DETECTION_EPOCHS; const nextEpochToCheck = currentEpoch + 1; // Log here to alert that validation won't be active until remainingEpochs == 0 if (remainingEpochs > 0) { - this.logger.info("Registered validator for doppelganger", {remainingEpochs, nextEpochToCheck, pubkeyHex}); + const prevEpoch = currentEpoch - 1; + const attestedInPreviousEpoch = await this.slashingProtection.hasAttestedInEpoch( + fromHexString(pubkeyHex), + prevEpoch + ); + + if (attestedInPreviousEpoch) { + remainingEpochs = REMAINING_EPOCHS_IF_SKIPPED; + this.logger.info("Previous epoch attestation in slashing protection db, doppelganger detection skipped", { + prevEpoch, + pubkeyHex, + }); + } else { + this.logger.info("Registered validator for doppelganger", {remainingEpochs, nextEpochToCheck, pubkeyHex}); + } } this.doppelgangerStateByPubkey.set(pubkeyHex, { @@ -189,7 +207,7 @@ export class DoppelgangerService { if (violators.length > 0) { // If a single doppelganger is detected, enable doppelganger checks on all validators forever for (const state of this.doppelgangerStateByPubkey.values()) { - state.remainingEpochs = Infinity; + state.remainingEpochs = REMAINING_EPOCHS_IF_DOPPLEGANGER; } this.logger.error( @@ -225,7 +243,7 @@ export class DoppelgangerService { const {remainingEpochs, nextEpochToCheck} = state; if (remainingEpochs <= 0) { - this.logger.info("Doppelganger detection complete", {index: response.index}); + this.logger.info("Doppelganger detection complete", {index: response.index, epoch: currentEpoch}); } else { this.logger.info("Found no doppelganger", {remainingEpochs, nextEpochToCheck, index: response.index}); } diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 2c35a9f345d9..9f2b16c53601 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -282,18 +282,18 @@ export class ValidatorStore { return proposerConfig; } - addSigner(signer: Signer, valProposerConfig?: ValidatorProposerConfig): void { + async addSigner(signer: Signer, valProposerConfig?: ValidatorProposerConfig): Promise { const pubkey = getSignerPubkeyHex(signer); const proposerConfig = (valProposerConfig?.proposerConfig ?? {})[pubkey]; if (!this.validators.has(pubkey)) { + await this.doppelgangerService?.registerValidator(pubkey); + this.pubkeysToDiscover.push(pubkey); this.validators.set(pubkey, { signer, ...proposerConfig, }); - - this.doppelgangerService?.registerValidator(pubkey); } } diff --git a/packages/validator/src/slashingProtection/attestation/index.ts b/packages/validator/src/slashingProtection/attestation/index.ts index 08c5c7bd30c4..681bdf5f0b13 100644 --- a/packages/validator/src/slashingProtection/attestation/index.ts +++ b/packages/validator/src/slashingProtection/attestation/index.ts @@ -1,4 +1,4 @@ -import {BLSPubkey} from "@lodestar/types"; +import {BLSPubkey, Epoch} from "@lodestar/types"; import {isEqualNonZeroRoot, minEpoch} from "../utils.js"; import {MinMaxSurround, SurroundAttestationError, SurroundAttestationErrorCode} from "../minMaxSurround/index.js"; import {SlashingProtectionAttestation} from "../types.js"; @@ -133,6 +133,13 @@ export class SlashingProtectionAttestationService { await this.minMaxSurround.insertAttestation(pubKey, attestation); } + /** + * Retrieve an attestation from the slashing protection database for a given `pubkey` and `epoch` + */ + async getAttestationForEpoch(pubkey: BLSPubkey, epoch: Epoch): Promise { + return this.attestationByTarget.get(pubkey, epoch); + } + /** * Interchange import / export functionality */ diff --git a/packages/validator/src/slashingProtection/index.ts b/packages/validator/src/slashingProtection/index.ts index 7186e5d67d9c..dedbccf6cf94 100644 --- a/packages/validator/src/slashingProtection/index.ts +++ b/packages/validator/src/slashingProtection/index.ts @@ -1,5 +1,5 @@ import {toHexString} from "@chainsafe/ssz"; -import {BLSPubkey, Root} from "@lodestar/types"; +import {BLSPubkey, Epoch, Root} from "@lodestar/types"; import {Logger} from "@lodestar/utils"; import {LodestarValidatorDatabaseController} from "../types.js"; import {uniqueVectorArr} from "../slashingProtection/utils.js"; @@ -56,6 +56,10 @@ export class SlashingProtection implements ISlashingProtection { await this.attestationService.checkAndInsertAttestation(pubKey, attestation); } + async hasAttestedInEpoch(pubKey: BLSPubkey, epoch: Epoch): Promise { + return (await this.attestationService.getAttestationForEpoch(pubKey, epoch)) !== null; + } + async importInterchange(interchange: Interchange, genesisValidatorsRoot: Root, logger?: Logger): Promise { const {data} = parseInterchange(interchange, genesisValidatorsRoot); for (const validator of data) { diff --git a/packages/validator/src/slashingProtection/interface.ts b/packages/validator/src/slashingProtection/interface.ts index e7f660a998e9..c2a790c98a65 100644 --- a/packages/validator/src/slashingProtection/interface.ts +++ b/packages/validator/src/slashingProtection/interface.ts @@ -1,4 +1,4 @@ -import {BLSPubkey, Root} from "@lodestar/types"; +import {BLSPubkey, Epoch, Root} from "@lodestar/types"; import {Logger} from "@lodestar/utils"; import {Interchange, InterchangeFormatVersion} from "./interchange/types.js"; import {SlashingProtectionBlock, SlashingProtectionAttestation} from "./types.js"; @@ -13,6 +13,11 @@ export interface ISlashingProtection { */ checkAndInsertAttestation(pubKey: BLSPubkey, attestation: SlashingProtectionAttestation): Promise; + /** + * Check whether a validator as identified by `pubKey` has attested in the specified `epoch` + */ + hasAttestedInEpoch(pubKey: BLSPubkey, epoch: Epoch): Promise; + importInterchange(interchange: Interchange, genesisValidatorsRoot: Uint8Array | Root, logger?: Logger): Promise; exportInterchange( genesisValidatorsRoot: Uint8Array | Root, diff --git a/packages/validator/src/validator.ts b/packages/validator/src/validator.ts index 01a01b2afa20..cbe2fcc74117 100644 --- a/packages/validator/src/validator.ts +++ b/packages/validator/src/validator.ts @@ -98,7 +98,15 @@ export class Validator { const indicesService = new IndicesService(logger, api, metrics); const doppelgangerService = opts.doppelgangerProtection - ? new DoppelgangerService(logger, clock, api, indicesService, opts.processShutdownCallback, metrics) + ? new DoppelgangerService( + logger, + clock, + api, + indicesService, + slashingProtection, + opts.processShutdownCallback, + metrics + ) : null; const validatorStore = new ValidatorStore( diff --git a/packages/validator/test/unit/services/doppleganger.test.ts b/packages/validator/test/unit/services/doppleganger.test.ts index af8abd79204e..1345b720adc9 100644 --- a/packages/validator/test/unit/services/doppleganger.test.ts +++ b/packages/validator/test/unit/services/doppleganger.test.ts @@ -7,6 +7,7 @@ import {DoppelgangerService, DoppelgangerStatus} from "../../../src/services/dop import {IndicesService} from "../../../src/services/indices.js"; import {testLogger} from "../../utils/logger.js"; import {ClockMock} from "../../utils/clock.js"; +import {SlashingProtectionMock} from "../../utils/slashingProtectionMock.js"; // At genesis start validating immediately @@ -96,10 +97,20 @@ describe("doppelganger service", () => { const initialEpoch = 1; const clock = new ClockMockMsToSlot(initialEpoch); - const doppelganger = new DoppelgangerService(logger, clock, beaconApi, indicesService, noop, null); + const slashingProtection = new SlashingProtectionMock(); + + const doppelganger = new DoppelgangerService( + logger, + clock, + beaconApi, + indicesService, + slashingProtection, + noop, + null + ); // Add validator to doppelganger - doppelganger.registerValidator(pubkeyHex); + await doppelganger.registerValidator(pubkeyHex); // Go step by step for (const [step, [isLivePrev, isLiveCurr, expectedStatus]] of testCase.entries()) { diff --git a/packages/validator/test/utils/slashingProtectionMock.ts b/packages/validator/test/utils/slashingProtectionMock.ts index e5b758a8f134..6a714a404f81 100644 --- a/packages/validator/test/utils/slashingProtectionMock.ts +++ b/packages/validator/test/utils/slashingProtectionMock.ts @@ -10,6 +10,9 @@ export class SlashingProtectionMock implements ISlashingProtection { async checkAndInsertAttestation(): Promise { // } + async hasAttestedInEpoch(): Promise { + return false; + } async importInterchange(): Promise { // } From 921e227d49fa3cca87fd532009489d4c76109b78 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 08:15:15 +0200 Subject: [PATCH 02/15] Async initialization of ValidatorStore and Validator class --- .../validator/src/services/validatorStore.ts | 51 ++++-- packages/validator/src/validator.ts | 165 ++++++++++++------ .../validator/test/e2e/web3signer.test.ts | 27 +-- .../unit/services/attestationDuties.test.ts | 4 +- .../test/unit/services/blockDuties.test.ts | 4 +- .../test/unit/services/doppleganger.test.ts | 2 +- .../unit/services/syncCommitteDuties.test.ts | 4 +- .../test/unit/validatorStore.test.ts | 4 +- .../validator/test/utils/validatorStore.ts | 23 +-- 9 files changed, 186 insertions(+), 98 deletions(-) diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 9f2b16c53601..109078f9ec99 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -97,6 +97,14 @@ export type ValidatorProposerConfig = { defaultConfig: ProposerConfig; }; +export type ValidatorStoreModules = { + config: BeaconConfig; + slashingProtection: ISlashingProtection; + indicesService: IndicesService; + doppelgangerService: DoppelgangerService | null; + metrics: Metrics | null; +}; + /** * This cache stores SignedValidatorRegistrationV1 data for a validator so that * we do not create and send new registration objects to avoid DOSing the builder @@ -130,21 +138,25 @@ export const defaultOptions = { * Service that sets up and handles validator attester duties. */ export class ValidatorStore { + private readonly config: BeaconConfig; + private readonly slashingProtection: ISlashingProtection; + private readonly indicesService: IndicesService; + private readonly doppelgangerService: DoppelgangerService | null; + private readonly metrics: Metrics | null; + private readonly validators = new Map(); /** Initially true because there are no validators */ private pubkeysToDiscover: PubkeyHex[] = []; private readonly defaultProposerConfig: DefaultProposerConfig; - constructor( - private readonly config: BeaconConfig, - private readonly slashingProtection: ISlashingProtection, - private readonly indicesService: IndicesService, - private readonly doppelgangerService: DoppelgangerService | null, - private readonly metrics: Metrics | null, - initialSigners: Signer[], - valProposerConfig: ValidatorProposerConfig = {defaultConfig: {}, proposerConfig: {}}, - private readonly genesisValidatorRoot: Root - ) { + constructor(modules: ValidatorStoreModules, valProposerConfig: ValidatorProposerConfig) { + const {config, slashingProtection, indicesService, doppelgangerService, metrics} = modules; + this.config = config; + this.slashingProtection = slashingProtection; + this.indicesService = indicesService; + this.doppelgangerService = doppelgangerService; + this.metrics = metrics; + const defaultConfig = valProposerConfig.defaultConfig; this.defaultProposerConfig = { graffiti: defaultConfig.graffiti ?? "", @@ -157,15 +169,26 @@ export class ValidatorStore { }, }; - for (const signer of initialSigners) { - this.addSigner(signer, valProposerConfig); - } - if (metrics) { metrics.signers.addCollect(() => metrics.signers.set(this.validators.size)); } } + /** + * Create a validator store with initial signers + */ + static async init( + modules: ValidatorStoreModules, + initialSigners: Signer[], + valProposerConfig: ValidatorProposerConfig = {defaultConfig: {}, proposerConfig: {}} + ): Promise { + const validatorStore = new ValidatorStore(modules, valProposerConfig); + + await Promise.all(initialSigners.map((signer) => validatorStore.addSigner(signer, valProposerConfig))); + + return validatorStore; + } + /** Return all known indices from the validatorStore pubkeys */ getAllLocalIndices(): ValidatorIndex[] { return this.indicesService.getAllLocalIndices(); diff --git a/packages/validator/src/validator.ts b/packages/validator/src/validator.ts index cbe2fcc74117..857937b0bfca 100644 --- a/packages/validator/src/validator.ts +++ b/packages/validator/src/validator.ts @@ -22,6 +22,24 @@ import {BeaconHealth, Metrics} from "./metrics.js"; import {MetaDataRepository} from "./repositories/metaDataRepository.js"; import {DoppelgangerService} from "./services/doppelgangerService.js"; +export type ValidatorModules = { + opts: ValidatorOptions; + genesis: Genesis; + validatorStore: ValidatorStore; + slashingProtection: ISlashingProtection; + blockProposingService: BlockProposingService; + attestationService: AttestationService; + syncCommitteeService: SyncCommitteeService; + config: BeaconConfig; + api: Api; + clock: IClock; + chainHeaderTracker: ChainHeaderTracker; + logger: Logger; + db: LodestarValidatorDatabaseController; + metrics: Metrics | null; + controller: AbortController; +}; + export type ValidatorOptions = { slashingProtection: ISlashingProtection; db: LodestarValidatorDatabaseController; @@ -53,6 +71,7 @@ enum Status { * Main class for the Validator client. */ export class Validator { + private readonly genesis: Genesis; readonly validatorStore: ValidatorStore; private readonly slashingProtection: ISlashingProtection; private readonly blockProposingService: BlockProposingService; @@ -67,14 +86,69 @@ export class Validator { private state: Status; private readonly controller: AbortController; - constructor( - opts: ValidatorOptions, - readonly genesis: Genesis, - metrics: Metrics | null = null - ) { + constructor({ + opts, + genesis, + validatorStore, + slashingProtection, + blockProposingService, + attestationService, + syncCommitteeService, + config, + api, + clock, + chainHeaderTracker, + logger, + db, + metrics, + controller, + }: ValidatorModules) { + this.genesis = genesis; + this.validatorStore = validatorStore; + this.slashingProtection = slashingProtection; + this.blockProposingService = blockProposingService; + this.attestationService = attestationService; + this.syncCommitteeService = syncCommitteeService; + this.config = config; + this.api = api; + this.clock = clock; + this.chainHeaderTracker = chainHeaderTracker; + this.logger = logger; + this.controller = controller; + this.db = db; + + if (opts.closed) { + this.state = Status.closed; + } else { + // "start" the validator + // Instantiates block and attestation services and runs them once the chain has been started. + this.state = Status.running; + this.clock.start(this.controller.signal); + this.chainHeaderTracker.start(this.controller.signal); + + if (metrics) { + this.db.setMetrics(metrics.db); + + this.clock.runEverySlot(() => + this.fetchBeaconHealth() + .then((health) => metrics.beaconHealth.set(health)) + .catch((e) => this.logger.error("Error on fetchBeaconHealth", {}, e)) + ); + } + } + } + + get isRunning(): boolean { + return this.state === Status.running; + } + + /** + * Initialize and start a validator client and its varied sub-component services + */ + static async init(opts: ValidatorOptions, genesis: Genesis, metrics: Metrics | null = null): Promise { const {db, config: chainConfig, logger, slashingProtection, signers, valProposerConfig} = opts; const config = createBeaconConfig(chainConfig, genesis.genesisValidatorsRoot); - this.controller = opts.abortController; + const controller = opts.abortController; const clock = new Clock(config, logger, {genesisTime: Number(genesis.genesisTime)}); const loggerVc = getLoggerVc(logger, clock); @@ -88,7 +162,7 @@ export class Validator { // Validator would need the beacon to respond within the slot // See https://github.com/ChainSafe/lodestar/issues/5315 for rationale timeoutMs: config.SECONDS_PER_SLOT * 1000, - getAbortSignal: () => this.controller.signal, + getAbortSignal: () => controller.signal, }, {config, logger, metrics: metrics?.restApiClient} ); @@ -97,6 +171,7 @@ export class Validator { } const indicesService = new IndicesService(logger, api, metrics); + const doppelgangerService = opts.doppelgangerProtection ? new DoppelgangerService( logger, @@ -109,15 +184,16 @@ export class Validator { ) : null; - const validatorStore = new ValidatorStore( - config, - slashingProtection, - indicesService, - doppelgangerService, - metrics, + const validatorStore = await ValidatorStore.init( + { + config, + slashingProtection, + indicesService, + doppelgangerService, + metrics, + }, signers, - valProposerConfig, - genesis.genesisValidatorsRoot + valProposerConfig ); pollPrepareBeaconProposer(config, loggerVc, api, clock, validatorStore, metrics); pollBuilderValidatorRegistration(config, loggerVc, api, clock, validatorStore, metrics); @@ -129,9 +205,9 @@ export class Validator { const chainHeaderTracker = new ChainHeaderTracker(logger, api, emitter); - this.blockProposingService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, metrics); + const blockProposingService = new BlockProposingService(config, loggerVc, api, clock, validatorStore, metrics); - this.attestationService = new AttestationService( + const attestationService = new AttestationService( loggerVc, api, clock, @@ -146,7 +222,7 @@ export class Validator { } ); - this.syncCommitteeService = new SyncCommitteeService( + const syncCommitteeService = new SyncCommitteeService( config, loggerVc, api, @@ -161,40 +237,23 @@ export class Validator { } ); - this.config = config; - this.logger = logger; - this.api = api; - this.db = db; - this.clock = clock; - this.validatorStore = validatorStore; - this.chainHeaderTracker = chainHeaderTracker; - this.slashingProtection = slashingProtection; - - if (metrics) { - db.setMetrics(metrics.db); - } - - if (opts.closed) { - this.state = Status.closed; - } else { - // "start" the validator - // Instantiates block and attestation services and runs them once the chain has been started. - this.state = Status.running; - this.clock.start(this.controller.signal); - this.chainHeaderTracker.start(this.controller.signal); - - if (metrics) { - this.clock.runEverySlot(() => - this.fetchBeaconHealth() - .then((health) => metrics.beaconHealth.set(health)) - .catch((e) => this.logger.error("Error on fetchBeaconHealth", {}, e)) - ); - } - } - } - - get isRunning(): boolean { - return this.state === Status.running; + return new this({ + opts, + genesis, + validatorStore, + slashingProtection, + blockProposingService, + attestationService, + syncCommitteeService, + config, + api, + clock, + chainHeaderTracker, + logger, + db, + metrics, + controller, + }); } /** Waits for genesis and genesis time */ @@ -222,7 +281,7 @@ export class Validator { await assertEqualGenesis(opts, genesis); logger.info("Verified connected beacon node and validator have the same genesisValidatorRoot"); - return new Validator(opts, genesis, metrics); + return Validator.init(opts, genesis, metrics); } removeDutiesForKey(pubkey: PubkeyHex): void { diff --git a/packages/validator/test/e2e/web3signer.test.ts b/packages/validator/test/e2e/web3signer.test.ts index 6b852424f2dd..3cd6db833750 100644 --- a/packages/validator/test/e2e/web3signer.test.ts +++ b/packages/validator/test/e2e/web3signer.test.ts @@ -105,8 +105,8 @@ describe("web3signer signature test", function () { const web3signerUrl = `http://localhost:${startedContainer.getMappedPort(port)}`; // http://localhost:9000/api/v1/eth2/sign/0x8837af2a7452aff5a8b6906c3e5adefce5690e1bba6d73d870b9e679fece096b97a255bae0978e3a344aa832f68c6b47 - validatorStoreRemote = getValidatorStore({type: SignerType.Remote, url: web3signerUrl, pubkey}); - validatorStoreLocal = getValidatorStore({type: SignerType.Local, secretKey}); + validatorStoreRemote = await getValidatorStore({type: SignerType.Remote, url: web3signerUrl, pubkey}); + validatorStoreLocal = await getValidatorStore({type: SignerType.Local, secretKey}); const stream = await startedContainer.logs(); stream @@ -217,7 +217,7 @@ describe("web3signer signature test", function () { } } - function getValidatorStore(signer: Signer): ValidatorStore { + async function getValidatorStore(signer: Signer): Promise { const logger = testLogger(); const api = getClient({baseUrl: "http://localhost:9596"}, {config}); const genesisValidatorsRoot = fromHex(genesisData.mainnet.genesisValidatorsRoot); @@ -226,15 +226,16 @@ describe("web3signer signature test", function () { const valProposerConfig = undefined; const indicesService = new IndicesService(logger, api, metrics); const slashingProtection = new SlashingProtectionDisabled(); - return new ValidatorStore( - createBeaconConfig(config, genesisValidatorsRoot), - slashingProtection, - indicesService, - doppelgangerService, - metrics, + return ValidatorStore.init( + { + config: createBeaconConfig(config, genesisValidatorsRoot), + slashingProtection, + indicesService, + doppelgangerService, + metrics, + }, [signer], - valProposerConfig, - genesisValidatorsRoot + valProposerConfig ); } }); @@ -248,6 +249,10 @@ class SlashingProtectionDisabled implements ISlashingProtection { // } + async hasAttestedInEpoch(): Promise { + return false; + } + async importInterchange(): Promise { // } diff --git a/packages/validator/test/unit/services/attestationDuties.test.ts b/packages/validator/test/unit/services/attestationDuties.test.ts index 864781b94c74..0a4a1b2c2fe9 100644 --- a/packages/validator/test/unit/services/attestationDuties.test.ts +++ b/packages/validator/test/unit/services/attestationDuties.test.ts @@ -36,10 +36,10 @@ describe("AttestationDutiesService", function () { validator: ssz.phase0.Validator.defaultValue(), }; - before(() => { + before(async () => { const secretKeys = [bls.SecretKey.fromBytes(toBufferBE(BigInt(98), 32))]; pubkeys = secretKeys.map((sk) => sk.toPublicKey().toBytes()); - validatorStore = initValidatorStore(secretKeys, api, chainConfig); + validatorStore = await initValidatorStore(secretKeys, api, chainConfig); }); let controller: AbortController; // To stop clock diff --git a/packages/validator/test/unit/services/blockDuties.test.ts b/packages/validator/test/unit/services/blockDuties.test.ts index d6152f6d7b09..93540b0c2794 100644 --- a/packages/validator/test/unit/services/blockDuties.test.ts +++ b/packages/validator/test/unit/services/blockDuties.test.ts @@ -24,10 +24,10 @@ describe("BlockDutiesService", function () { let validatorStore: ValidatorStore; let pubkeys: Uint8Array[]; // Initialize pubkeys in before() so bls is already initialized - before(() => { + before(async () => { const secretKeys = Array.from({length: 3}, (_, i) => bls.SecretKey.fromBytes(toBufferBE(BigInt(i + 1), 32))); pubkeys = secretKeys.map((sk) => sk.toPublicKey().toBytes()); - validatorStore = initValidatorStore(secretKeys, api); + validatorStore = await initValidatorStore(secretKeys, api); }); let controller: AbortController; // To stop clock diff --git a/packages/validator/test/unit/services/doppleganger.test.ts b/packages/validator/test/unit/services/doppleganger.test.ts index 1345b720adc9..ee01615cfa8a 100644 --- a/packages/validator/test/unit/services/doppleganger.test.ts +++ b/packages/validator/test/unit/services/doppleganger.test.ts @@ -5,9 +5,9 @@ import {computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {Api, HttpStatusCode} from "@lodestar/api"; import {DoppelgangerService, DoppelgangerStatus} from "../../../src/services/doppelgangerService.js"; import {IndicesService} from "../../../src/services/indices.js"; +import {SlashingProtectionMock} from "../../utils/slashingProtectionMock.js"; import {testLogger} from "../../utils/logger.js"; import {ClockMock} from "../../utils/clock.js"; -import {SlashingProtectionMock} from "../../utils/slashingProtectionMock.js"; // At genesis start validating immediately diff --git a/packages/validator/test/unit/services/syncCommitteDuties.test.ts b/packages/validator/test/unit/services/syncCommitteDuties.test.ts index 7439f5769b0e..af5734ffdcca 100644 --- a/packages/validator/test/unit/services/syncCommitteDuties.test.ts +++ b/packages/validator/test/unit/services/syncCommitteDuties.test.ts @@ -43,13 +43,13 @@ describe("SyncCommitteeDutiesService", function () { validator: ssz.phase0.Validator.defaultValue(), }; - before(() => { + before(async () => { const secretKeys = [ bls.SecretKey.fromBytes(toBufferBE(BigInt(98), 32)), bls.SecretKey.fromBytes(toBufferBE(BigInt(99), 32)), ]; pubkeys = secretKeys.map((sk) => sk.toPublicKey().toBytes()); - validatorStore = initValidatorStore(secretKeys, api, altair0Config); + validatorStore = await initValidatorStore(secretKeys, api, altair0Config); }); let controller: AbortController; // To stop clock diff --git a/packages/validator/test/unit/validatorStore.test.ts b/packages/validator/test/unit/validatorStore.test.ts index 974f2aef87b2..c08ce0c61244 100644 --- a/packages/validator/test/unit/validatorStore.test.ts +++ b/packages/validator/test/unit/validatorStore.test.ts @@ -21,7 +21,7 @@ describe("ValidatorStore", function () { let valProposerConfig: ValidatorProposerConfig; let signValidatorStub: SinonStubFn; - before(() => { + before(async () => { valProposerConfig = { proposerConfig: { [toHexString(pubkeys[0])]: { @@ -45,7 +45,7 @@ describe("ValidatorStore", function () { }, }; - validatorStore = initValidatorStore(secretKeys, api, chainConfig, valProposerConfig); + validatorStore = await initValidatorStore(secretKeys, api, chainConfig, valProposerConfig); signValidatorStub = sinon.stub(validatorStore, "signValidatorRegistration"); }); diff --git a/packages/validator/test/utils/validatorStore.ts b/packages/validator/test/utils/validatorStore.ts index 4ad959535464..61de1e9371d6 100644 --- a/packages/validator/test/utils/validatorStore.ts +++ b/packages/validator/test/utils/validatorStore.ts @@ -11,12 +11,12 @@ import {SlashingProtectionMock} from "./slashingProtectionMock.js"; /** * Initializes an actual ValidatorStore without stubs */ -export function initValidatorStore( +export async function initValidatorStore( secretKeys: SecretKey[], api: Api, customChainConfig: ChainConfig = chainConfig, valProposerConfig: ValidatorProposerConfig = {defaultConfig: {builder: {}}, proposerConfig: {}} -): ValidatorStore { +): Promise { const logger = testLogger(); const genesisValidatorsRoot = Buffer.alloc(32, 0xdd); @@ -26,15 +26,16 @@ export function initValidatorStore( })); const metrics = null; - const indicesService = new IndicesService(logger, api, metrics); - return new ValidatorStore( - createBeaconConfig(customChainConfig, genesisValidatorsRoot), - new SlashingProtectionMock(), - indicesService, - null, - metrics, + + return ValidatorStore.init( + { + config: createBeaconConfig(customChainConfig, genesisValidatorsRoot), + slashingProtection: new SlashingProtectionMock(), + indicesService: new IndicesService(logger, api, metrics), + doppelgangerService: null, + metrics, + }, signers, - valProposerConfig, - genesisValidatorsRoot + valProposerConfig ); } From 456c9daef4675f5f1489b647e0fb0980cab69a71 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 09:11:36 +0200 Subject: [PATCH 03/15] Add attested in prev epoch doppelganger unit test --- ...pleganger.test.ts => doppelganger.test.ts} | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) rename packages/validator/test/unit/services/{doppleganger.test.ts => doppelganger.test.ts} (84%) diff --git a/packages/validator/test/unit/services/doppleganger.test.ts b/packages/validator/test/unit/services/doppelganger.test.ts similarity index 84% rename from packages/validator/test/unit/services/doppleganger.test.ts rename to packages/validator/test/unit/services/doppelganger.test.ts index ee01615cfa8a..8511a86efb8b 100644 --- a/packages/validator/test/unit/services/doppleganger.test.ts +++ b/packages/validator/test/unit/services/doppelganger.test.ts @@ -134,6 +134,44 @@ describe("doppelganger service", () => { } }); } + + it("attested in prev epoch", async () => { + const index = 0; + const pubkeyHex = "0x" + "aa".repeat(48); + + const beaconApi = getMockBeaconApi(new Map()); + const logger = testLogger(); + + // Register validator to IndicesService for doppelganger to resolve pubkey -> index + const indicesService = new IndicesService(logger, beaconApi, null); + indicesService.index2pubkey.set(index, pubkeyHex); + indicesService.pubkey2index.set(pubkeyHex, index); + + // Initial epoch must be > 0 else doppelganger detection is skipped due to pre-genesis + const initialEpoch = 1; + const clock = new ClockMockMsToSlot(initialEpoch); + + const slashingProtection = new SlashingProtectionMock(); + // Previous epoch attestation exists in slashing protection db + slashingProtection.hasAttestedInEpoch = async () => true; + + const doppelganger = new DoppelgangerService( + logger, + clock, + beaconApi, + indicesService, + slashingProtection, + noop, + null + ); + + // Add validator to doppelganger + await doppelganger.registerValidator(pubkeyHex); + + // Assert doppelganger status right away + const status = doppelganger.getStatus(pubkeyHex); + expect(status).equal(DoppelgangerStatus.VerifiedSafe); + }); }); class MapDef extends Map { From 37fd010ff8a2170cf700192f155d9481f2d3e454 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 09:12:47 +0200 Subject: [PATCH 04/15] Fix typos in doppelganger --- packages/validator/src/services/doppelgangerService.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index 9068591999fe..56595b79c56b 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -12,7 +12,7 @@ import {IndicesService} from "./indices.js"; // The number of epochs that must be checked before we assume that there are // no other duplicate validators on the network const DEFAULT_REMAINING_DETECTION_EPOCHS = 1; -const REMAINING_EPOCHS_IF_DOPPLEGANGER = Infinity; +const REMAINING_EPOCHS_IF_DOPPELGANGER = Infinity; const REMAINING_EPOCHS_IF_SKIPPED = 0; /** Liveness responses for a given epoch */ @@ -198,7 +198,7 @@ export class DoppelgangerService { } if (state.nextEpochToCheck <= epoch) { - // Doppleganger detected + // Doppelganger detected violators.push(response.index); } } @@ -207,7 +207,7 @@ export class DoppelgangerService { if (violators.length > 0) { // If a single doppelganger is detected, enable doppelganger checks on all validators forever for (const state of this.doppelgangerStateByPubkey.values()) { - state.remainingEpochs = REMAINING_EPOCHS_IF_DOPPLEGANGER; + state.remainingEpochs = REMAINING_EPOCHS_IF_DOPPELGANGER; } this.logger.error( @@ -271,7 +271,7 @@ function getStatus(state: DoppelgangerState | undefined): DoppelgangerStatus { return DoppelgangerStatus.Unknown; } else if (state.remainingEpochs <= 0) { return DoppelgangerStatus.VerifiedSafe; - } else if (state.remainingEpochs === REMAINING_EPOCHS_IF_DOPPLEGANGER) { + } else if (state.remainingEpochs === REMAINING_EPOCHS_IF_DOPPELGANGER) { return DoppelgangerStatus.DoppelgangerDetected; } else { return DoppelgangerStatus.Unverified; From e1d4c5fc5136c0533d691004ce74be8e4b2aba16 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 13:34:27 +0200 Subject: [PATCH 05/15] Update doppelganger service register validator logs --- .../validator/src/services/doppelgangerService.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index 56595b79c56b..7a5cee55f4f7 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -67,21 +67,26 @@ export class DoppelgangerService { // Log here to alert that validation won't be active until remainingEpochs == 0 if (remainingEpochs > 0) { - const prevEpoch = currentEpoch - 1; + const previousEpoch = currentEpoch - 1; const attestedInPreviousEpoch = await this.slashingProtection.hasAttestedInEpoch( fromHexString(pubkeyHex), - prevEpoch + previousEpoch ); if (attestedInPreviousEpoch) { remainingEpochs = REMAINING_EPOCHS_IF_SKIPPED; - this.logger.info("Previous epoch attestation in slashing protection db, doppelganger detection skipped", { - prevEpoch, + this.logger.info("Doppelganger detection skipped, previous epoch attestation exists in database", { + previousEpoch, pubkeyHex, }); } else { this.logger.info("Registered validator for doppelganger", {remainingEpochs, nextEpochToCheck, pubkeyHex}); } + } else { + this.logger.info("Doppelganger detection skipped, validator initialized before genesis", { + currentEpoch, + pubkeyHex, + }); } this.doppelgangerStateByPubkey.set(pubkeyHex, { From 9f0ef493476271f86c58793ba311de18b5a717b5 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 13:37:53 +0200 Subject: [PATCH 06/15] Use jsdoc to document doppelganger statuses --- packages/validator/src/services/doppelgangerService.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index 7a5cee55f4f7..616d87ac258b 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -27,13 +27,13 @@ export type DoppelgangerState = { }; export enum DoppelgangerStatus { - // This pubkey is known to the doppelganger service and has been verified safe + /** This pubkey is known to the doppelganger service and has been verified safe */ VerifiedSafe = "VerifiedSafe", - // This pubkey is known to the doppelganger service but has not been verified safe + /** This pubkey is known to the doppelganger service but has not been verified safe */ Unverified = "Unverified", - // This pubkey is unknown to the doppelganger service + /** This pubkey is unknown to the doppelganger service */ Unknown = "Unknown", - // This pubkey has been detected to be active on the network + /** This pubkey has been detected to be active on the network */ DoppelgangerDetected = "DoppelgangerDetected", } From af91f1708a11b1dbb34cac768fdf548c51af9f20 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 14:04:21 +0200 Subject: [PATCH 07/15] Use prettyBytes to print out pubkeys --- .../validator/src/services/doppelgangerService.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index 616d87ac258b..5ae47c161acb 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -1,7 +1,7 @@ import {fromHexString} from "@chainsafe/ssz"; import {Epoch, ValidatorIndex} from "@lodestar/types"; import {Api, ApiError, routes} from "@lodestar/api"; -import {Logger, sleep} from "@lodestar/utils"; +import {Logger, prettyBytes, sleep} from "@lodestar/utils"; import {computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {ISlashingProtection} from "../slashingProtection/index.js"; import {ProcessShutdownCallback, PubkeyHex} from "../types.js"; @@ -77,15 +77,19 @@ export class DoppelgangerService { remainingEpochs = REMAINING_EPOCHS_IF_SKIPPED; this.logger.info("Doppelganger detection skipped, previous epoch attestation exists in database", { previousEpoch, - pubkeyHex, + pubkey: prettyBytes(pubkeyHex), }); } else { - this.logger.info("Registered validator for doppelganger", {remainingEpochs, nextEpochToCheck, pubkeyHex}); + this.logger.info("Registered validator for doppelganger", { + remainingEpochs, + nextEpochToCheck, + pubkey: prettyBytes(pubkeyHex), + }); } } else { this.logger.info("Doppelganger detection skipped, validator initialized before genesis", { currentEpoch, - pubkeyHex, + pubkey: prettyBytes(pubkeyHex), }); } From bc8cf9c77919868401c7af2a55f32c5b31420a55 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 17:34:06 +0200 Subject: [PATCH 08/15] Add comment about doppelganger and signer registration order --- packages/validator/src/services/validatorStore.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/validator/src/services/validatorStore.ts b/packages/validator/src/services/validatorStore.ts index 109078f9ec99..9dbf79b40847 100644 --- a/packages/validator/src/services/validatorStore.ts +++ b/packages/validator/src/services/validatorStore.ts @@ -310,6 +310,7 @@ export class ValidatorStore { const proposerConfig = (valProposerConfig?.proposerConfig ?? {})[pubkey]; if (!this.validators.has(pubkey)) { + // Doppelganger registration must be done before adding validator to signers await this.doppelgangerService?.registerValidator(pubkey); this.pubkeysToDiscover.push(pubkey); From d507898769718efa36cdaa3490dcec850f053173 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 17:38:41 +0200 Subject: [PATCH 09/15] Print out validator pubkey first --- .../src/services/doppelgangerService.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index 5ae47c161acb..ff736d89a704 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -75,21 +75,24 @@ export class DoppelgangerService { if (attestedInPreviousEpoch) { remainingEpochs = REMAINING_EPOCHS_IF_SKIPPED; - this.logger.info("Doppelganger detection skipped, previous epoch attestation exists in database", { - previousEpoch, - pubkey: prettyBytes(pubkeyHex), - }); + this.logger.info( + "Doppelganger detection skipped for validator, previous epoch attestation exists in database", + { + pubkey: prettyBytes(pubkeyHex), + previousEpoch, + } + ); } else { this.logger.info("Registered validator for doppelganger", { + pubkey: prettyBytes(pubkeyHex), remainingEpochs, nextEpochToCheck, - pubkey: prettyBytes(pubkeyHex), }); } } else { - this.logger.info("Doppelganger detection skipped, validator initialized before genesis", { - currentEpoch, + this.logger.info("Doppelganger detection skipped for validator, initialized before genesis", { pubkey: prettyBytes(pubkeyHex), + currentEpoch, }); } @@ -254,7 +257,7 @@ export class DoppelgangerService { if (remainingEpochs <= 0) { this.logger.info("Doppelganger detection complete", {index: response.index, epoch: currentEpoch}); } else { - this.logger.info("Found no doppelganger", {remainingEpochs, nextEpochToCheck, index: response.index}); + this.logger.info("Found no doppelganger", {index: response.index, remainingEpochs, nextEpochToCheck}); } } } From 1525c6edee64c78ed9267672885f838be5a2a16b Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 17:53:50 +0200 Subject: [PATCH 10/15] Update validator client jsdoc --- packages/validator/src/validator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/validator/src/validator.ts b/packages/validator/src/validator.ts index 857937b0bfca..6deb7182339b 100644 --- a/packages/validator/src/validator.ts +++ b/packages/validator/src/validator.ts @@ -143,7 +143,7 @@ export class Validator { } /** - * Initialize and start a validator client and its varied sub-component services + * Initialize and start a validator client */ static async init(opts: ValidatorOptions, genesis: Genesis, metrics: Metrics | null = null): Promise { const {db, config: chainConfig, logger, slashingProtection, signers, valProposerConfig} = opts; From d40f1cd0c0e87bf83fdbf7bbfc626009d447fbe7 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Sun, 1 Oct 2023 20:14:30 +0200 Subject: [PATCH 11/15] Revise doppelganger registration logs --- .../validator/src/services/doppelgangerService.ts | 15 ++++++--------- .../test/unit/services/doppelganger.test.ts | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index ff736d89a704..a4a810de8ae3 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -75,22 +75,19 @@ export class DoppelgangerService { if (attestedInPreviousEpoch) { remainingEpochs = REMAINING_EPOCHS_IF_SKIPPED; - this.logger.info( - "Doppelganger detection skipped for validator, previous epoch attestation exists in database", - { - pubkey: prettyBytes(pubkeyHex), - previousEpoch, - } - ); + this.logger.info("Doppelganger detection skipped, attestation from previous epoch exists in database", { + pubkey: prettyBytes(pubkeyHex), + previousEpoch, + }); } else { - this.logger.info("Registered validator for doppelganger", { + this.logger.info("Registered validator for doppelganger detection", { pubkey: prettyBytes(pubkeyHex), remainingEpochs, nextEpochToCheck, }); } } else { - this.logger.info("Doppelganger detection skipped for validator, initialized before genesis", { + this.logger.info("Doppelganger detection skipped, validator initialized before genesis", { pubkey: prettyBytes(pubkeyHex), currentEpoch, }); diff --git a/packages/validator/test/unit/services/doppelganger.test.ts b/packages/validator/test/unit/services/doppelganger.test.ts index 8511a86efb8b..d21c41eda12c 100644 --- a/packages/validator/test/unit/services/doppelganger.test.ts +++ b/packages/validator/test/unit/services/doppelganger.test.ts @@ -152,7 +152,7 @@ describe("doppelganger service", () => { const clock = new ClockMockMsToSlot(initialEpoch); const slashingProtection = new SlashingProtectionMock(); - // Previous epoch attestation exists in slashing protection db + // Attestation from previous epoch exists in slashing protection db slashingProtection.hasAttestedInEpoch = async () => true; const doppelganger = new DoppelgangerService( From 0fa1682456fbdc410121567ba5d5ac8d9397b9b9 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 2 Oct 2023 10:53:16 +0200 Subject: [PATCH 12/15] Truncate and format bytes as 0x123456789abc --- packages/utils/src/format.ts | 10 ++++++++++ packages/validator/src/services/doppelgangerService.ts | 8 ++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/utils/src/format.ts b/packages/utils/src/format.ts index e3ec2567bec5..6a88ead41490 100644 --- a/packages/utils/src/format.ts +++ b/packages/utils/src/format.ts @@ -17,3 +17,13 @@ export function prettyBytesShort(root: Uint8Array | string): string { const str = typeof root === "string" ? root : toHexString(root); return `${str.slice(0, 6)}…`; } + +/** + * Truncate and format bytes as `0x123456789abc` + * 6 bytes is sufficient to avoid collisions and it allows to easily look up + * values on explorers like beaconcha.in while improving readability of logs + */ +export function truncBytes(root: Uint8Array | string): string { + const str = typeof root === "string" ? root : toHexString(root); + return str.slice(0, 14); +} diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index a4a810de8ae3..fde5b0e5647b 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -1,7 +1,7 @@ import {fromHexString} from "@chainsafe/ssz"; import {Epoch, ValidatorIndex} from "@lodestar/types"; import {Api, ApiError, routes} from "@lodestar/api"; -import {Logger, prettyBytes, sleep} from "@lodestar/utils"; +import {Logger, sleep, truncBytes} from "@lodestar/utils"; import {computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {ISlashingProtection} from "../slashingProtection/index.js"; import {ProcessShutdownCallback, PubkeyHex} from "../types.js"; @@ -76,19 +76,19 @@ export class DoppelgangerService { if (attestedInPreviousEpoch) { remainingEpochs = REMAINING_EPOCHS_IF_SKIPPED; this.logger.info("Doppelganger detection skipped, attestation from previous epoch exists in database", { - pubkey: prettyBytes(pubkeyHex), + pubkey: truncBytes(pubkeyHex), previousEpoch, }); } else { this.logger.info("Registered validator for doppelganger detection", { - pubkey: prettyBytes(pubkeyHex), + pubkey: truncBytes(pubkeyHex), remainingEpochs, nextEpochToCheck, }); } } else { this.logger.info("Doppelganger detection skipped, validator initialized before genesis", { - pubkey: prettyBytes(pubkeyHex), + pubkey: truncBytes(pubkeyHex), currentEpoch, }); } From 8e35ff35ab6218a2a060c948f5c1ea72f3e3244f Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 2 Oct 2023 11:26:30 +0200 Subject: [PATCH 13/15] Add reference to github issue --- packages/validator/src/services/doppelgangerService.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index fde5b0e5647b..394e67524469 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -74,6 +74,8 @@ export class DoppelgangerService { ); if (attestedInPreviousEpoch) { + // It is safe to skip doppelganger detection + // https://github.com/ChainSafe/lodestar/issues/5856 remainingEpochs = REMAINING_EPOCHS_IF_SKIPPED; this.logger.info("Doppelganger detection skipped, attestation from previous epoch exists in database", { pubkey: truncBytes(pubkeyHex), From eb53e4d980f11926182efedae8de1a4431160000 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 2 Oct 2023 16:23:23 +0200 Subject: [PATCH 14/15] Revise logs final --- packages/validator/src/services/doppelgangerService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/validator/src/services/doppelgangerService.ts b/packages/validator/src/services/doppelgangerService.ts index 394e67524469..861db27769e7 100644 --- a/packages/validator/src/services/doppelgangerService.ts +++ b/packages/validator/src/services/doppelgangerService.ts @@ -77,7 +77,7 @@ export class DoppelgangerService { // It is safe to skip doppelganger detection // https://github.com/ChainSafe/lodestar/issues/5856 remainingEpochs = REMAINING_EPOCHS_IF_SKIPPED; - this.logger.info("Doppelganger detection skipped, attestation from previous epoch exists in database", { + this.logger.info("Doppelganger detection skipped for validator because restart was detected", { pubkey: truncBytes(pubkeyHex), previousEpoch, }); @@ -89,7 +89,7 @@ export class DoppelgangerService { }); } } else { - this.logger.info("Doppelganger detection skipped, validator initialized before genesis", { + this.logger.info("Doppelganger detection skipped for validator initialized before genesis", { pubkey: truncBytes(pubkeyHex), currentEpoch, }); From eaf74cda4c43a83ebc184d096e08a599420754b9 Mon Sep 17 00:00:00 2001 From: Nico Flaig Date: Mon, 2 Oct 2023 21:18:32 +0200 Subject: [PATCH 15/15] Update tests to ensure correct epoch is provided --- packages/validator/test/unit/services/doppelganger.test.ts | 6 ++++-- packages/validator/test/utils/slashingProtectionMock.ts | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/validator/test/unit/services/doppelganger.test.ts b/packages/validator/test/unit/services/doppelganger.test.ts index d21c41eda12c..f3507be690f6 100644 --- a/packages/validator/test/unit/services/doppelganger.test.ts +++ b/packages/validator/test/unit/services/doppelganger.test.ts @@ -148,12 +148,14 @@ describe("doppelganger service", () => { indicesService.pubkey2index.set(pubkeyHex, index); // Initial epoch must be > 0 else doppelganger detection is skipped due to pre-genesis - const initialEpoch = 1; + const initialEpoch = 10; const clock = new ClockMockMsToSlot(initialEpoch); const slashingProtection = new SlashingProtectionMock(); // Attestation from previous epoch exists in slashing protection db - slashingProtection.hasAttestedInEpoch = async () => true; + slashingProtection.hasAttestedInEpoch = async (_, epoch: Epoch) => { + return epoch === initialEpoch - 1; + }; const doppelganger = new DoppelgangerService( logger, diff --git a/packages/validator/test/utils/slashingProtectionMock.ts b/packages/validator/test/utils/slashingProtectionMock.ts index 6a714a404f81..ec83fae742cc 100644 --- a/packages/validator/test/utils/slashingProtectionMock.ts +++ b/packages/validator/test/utils/slashingProtectionMock.ts @@ -1,3 +1,4 @@ +import {BLSPubkey, Epoch} from "@lodestar/types"; import {ISlashingProtection} from "../../src/index.js"; /** @@ -10,7 +11,7 @@ export class SlashingProtectionMock implements ISlashingProtection { async checkAndInsertAttestation(): Promise { // } - async hasAttestedInEpoch(): Promise { + async hasAttestedInEpoch(_p: BLSPubkey, _e: Epoch): Promise { return false; } async importInterchange(): Promise {