From d3db999ab09ddfc9bb3f169249ca1d31a5e13e79 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Tue, 18 Jul 2023 11:14:25 +0700 Subject: [PATCH 01/16] feat: validate gossip attestations same att data in batch --- .../src/chain/errors/attestationError.ts | 5 +- .../src/chain/validation/attestation.ts | 200 ++++++++++++--- .../src/metrics/metrics/lodestar.ts | 22 +- .../src/network/gossip/interface.ts | 53 ++-- .../src/network/processor/gossipHandlers.ts | 165 +++++++++---- .../network/processor/gossipQueues/index.ts | 64 ++--- .../network/processor/gossipQueues/types.ts | 2 +- .../network/processor/gossipValidatorFn.ts | 85 ++++++- .../src/network/processor/index.ts | 100 +++++--- .../src/network/processor/types.ts | 2 + packages/beacon-node/src/util/wrapError.ts | 2 +- .../test/e2e/network/gossipsub.test.ts | 24 +- .../beacon-node/test/perf/bls/bls.test.ts | 58 ++++- .../perf/chain/validation/attestation.test.ts | 87 +++++-- .../unit/chain/validation/attestation.test.ts | 227 ++++++++++++++++-- .../test/utils/validationData/attestation.ts | 9 +- .../src/util/signatureSets.ts | 32 +-- packages/state-transition/test/perf/util.ts | 14 +- 18 files changed, 891 insertions(+), 260 deletions(-) diff --git a/packages/beacon-node/src/chain/errors/attestationError.ts b/packages/beacon-node/src/chain/errors/attestationError.ts index a93f5b42e439..913827d4ca38 100644 --- a/packages/beacon-node/src/chain/errors/attestationError.ts +++ b/packages/beacon-node/src/chain/errors/attestationError.ts @@ -132,6 +132,8 @@ export enum AttestationErrorCode { INVALID_SERIALIZED_BYTES = "ATTESTATION_ERROR_INVALID_SERIALIZED_BYTES", /** Too many skipped slots. */ TOO_MANY_SKIPPED_SLOTS = "ATTESTATION_ERROR_TOO_MANY_SKIPPED_SLOTS", + /** attDataBase64 is not available */ + NO_INDEXED_DATA = "ATTESTATION_ERROR_NO_INDEXED_DATA", } export type AttestationErrorType = @@ -166,7 +168,8 @@ export type AttestationErrorType = | {code: AttestationErrorCode.INVALID_AGGREGATOR} | {code: AttestationErrorCode.INVALID_INDEXED_ATTESTATION} | {code: AttestationErrorCode.INVALID_SERIALIZED_BYTES} - | {code: AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS; headBlockSlot: Slot; attestationSlot: Slot}; + | {code: AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS; headBlockSlot: Slot; attestationSlot: Slot} + | {code: AttestationErrorCode.NO_INDEXED_DATA}; export class AttestationError extends GossipActionError { getMetadata(): Record { diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index bea4d060e48a..6e0efa34beeb 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -1,13 +1,14 @@ import {toHexString} from "@chainsafe/ssz"; +import bls from "@chainsafe/bls"; import {phase0, Epoch, Root, Slot, RootHex, ssz} from "@lodestar/types"; import {ProtoBlock} from "@lodestar/fork-choice"; import {ATTESTATION_SUBNET_COUNT, SLOTS_PER_EPOCH, ForkName, ForkSeq} from "@lodestar/params"; import { computeEpochAtSlot, CachedBeaconStateAllForks, - ISignatureSet, getAttestationDataSigningRoot, createSingleSignatureSetFromComponents, + SingleSignatureSet, } from "@lodestar/state-transition"; import {IBeaconChain} from ".."; import {AttestationError, AttestationErrorCode, GossipAction} from "../errors/index.js"; @@ -16,11 +17,19 @@ import {RegenCaller} from "../regen/index.js"; import { AttDataBase64, getAggregationBitsFromAttestationSerialized, - getAttDataBase64FromAttestationSerialized, getSignatureFromAttestationSerialized, } from "../../util/sszBytes.js"; import {AttestationDataCacheEntry} from "../seenCache/seenAttestationData.js"; import {sszDeserializeAttestation} from "../../network/gossip/topic.js"; +import {Result, wrapError} from "../../util/wrapError.js"; +import {MIN_SIGNATURE_SETS_TO_BATCH_VERIFY} from "../../network/processor/gossipQueues/index.js"; +import {signatureFromBytesNoCheck} from "../opPools/utils.js"; + +export type BatchResult = { + results: Result[]; + batchableBls: boolean; + fallbackBls: boolean; +}; export type AttestationValidationResult = { attestation: phase0.Attestation; @@ -40,6 +49,12 @@ export type GossipAttestation = { serializedData: Uint8Array; // available in NetworkProcessor since we check for unknown block root attestations attSlot: Slot; + attDataBase64?: string | null; +}; + +export type Phase0Result = AttestationValidationResult & { + signatureSet: SingleSignatureSet; + validatorIndex: number; }; /** @@ -49,11 +64,13 @@ export type GossipAttestation = { const SHUFFLING_LOOK_AHEAD_EPOCHS = 1; /** - * Validate attestations from gossip - * - Only deserialize the attestation if needed, use the cached AttestationData instead - * - This is to avoid deserializing similar attestation multiple times which could help the gc - * - subnet is required - * - do not prioritize bls signature set + * Verify gossip attestations of the same attestation data. + * - If there are less than 32 signatures, verify each signature individually with batchable = true + * - If there are not less than 32 signatures + * - do a quick verify by aggregate all signatures and pubkeys, this takes 4.6ms for 32 signatures and 7.6ms for 64 signatures + * - if one of the signature is invalid, do a fallback verify by verify each signature individually with batchable = false + * - subnet is required + * - do not prioritize bls signature set */ export async function validateGossipAttestation( fork: ForkName, @@ -65,6 +82,112 @@ export async function validateGossipAttestation( return validateAttestation(fork, chain, attestationOrBytes, subnet); } +export async function validateGossipAttestationsSameAttData( + fork: ForkName, + chain: IBeaconChain, + attestationOrBytesArr: AttestationOrBytes[], + subnet: number, + // for unit test, consumers do not need to pass this + phase0ValidationFn = validateGossipAttestationNoSignatureCheck +): Promise { + if (attestationOrBytesArr.length === 0) { + return {results: [], batchableBls: false, fallbackBls: false}; + } + + // phase0: do all verifications except for signature verification + const phase0ResultOrErrors = await Promise.all( + attestationOrBytesArr.map((attestationOrBytes) => + wrapError(phase0ValidationFn(fork, chain, attestationOrBytes, subnet)) + ) + ); + + // phase1: verify signatures of all valid attestations + // map new index to index in resultOrErrors + const newIndexToOldIndex = new Map(); + const signatureSets: SingleSignatureSet[] = []; + let newIndex = 0; + const phase0Results: Phase0Result[] = []; + for (const [i, resultOrError] of phase0ResultOrErrors.entries()) { + if (resultOrError.err) { + continue; + } + phase0Results.push(resultOrError.result); + newIndexToOldIndex.set(newIndex, i); + signatureSets.push(resultOrError.result.signatureSet); + newIndex++; + } + + let signatureValids: boolean[]; + let batchableBls = false; + let fallbackBls = false; + if (signatureSets.length >= MIN_SIGNATURE_SETS_TO_BATCH_VERIFY) { + // all signature sets should have same signing root since we filtered in network processor + const aggregatedPubkey = bls.PublicKey.aggregate(signatureSets.map((set) => set.pubkey)); + const aggregatedSignature = bls.Signature.aggregate( + // no need to check signature, will do a final verify later + signatureSets.map((set) => signatureFromBytesNoCheck(set.signature)) + ); + + // quick check, it's likely this is valid most of the time + batchableBls = true; + const isAllValid = aggregatedSignature.verify(aggregatedPubkey, signatureSets[0].signingRoot); + fallbackBls = !isAllValid; + signatureValids = isAllValid + ? new Array(signatureSets.length).fill(true) + : // batchable is false because one of the signature is invalid + await Promise.all(signatureSets.map((set) => chain.bls.verifySignatureSets([set], {batchable: false}))); + } else { + batchableBls = false; + // don't want to block the main thread if there are too few signatures + signatureValids = await Promise.all( + signatureSets.map((set) => chain.bls.verifySignatureSets([set], {batchable: true})) + ); + } + + // phase0 post validation + for (const [i, isValid] of signatureValids.entries()) { + const oldIndex = newIndexToOldIndex.get(i); + if (oldIndex == null) { + // should not happen + throw Error(`Cannot get old index for index ${i}`); + } + + const {validatorIndex, attestation} = phase0Results[i]; + const targetEpoch = attestation.data.target.epoch; + if (isValid) { + // Now that the attestation has been fully verified, store that we have received a valid attestation from this validator. + // + // It's important to double check that the attestation still hasn't been observed, since + // there can be a race-condition if we receive two attestations at the same time and + // process them in different threads. + if (chain.seenAttesters.isKnown(targetEpoch, validatorIndex)) { + phase0ResultOrErrors[oldIndex] = { + err: new AttestationError(GossipAction.IGNORE, { + code: AttestationErrorCode.ATTESTATION_ALREADY_KNOWN, + targetEpoch, + validatorIndex, + }), + }; + } + + // valid + chain.seenAttesters.add(targetEpoch, validatorIndex); + } else { + phase0ResultOrErrors[oldIndex] = { + err: new AttestationError(GossipAction.IGNORE, { + code: AttestationErrorCode.INVALID_SIGNATURE, + }), + }; + } + } + + return { + results: phase0ResultOrErrors, + batchableBls, + fallbackBls, + }; +} + /** * Validate attestations from api * - no need to deserialize attestation @@ -81,17 +204,42 @@ export async function validateApiAttestation( } /** - * Only deserialize the attestation if needed, use the cached AttestationData instead - * This is to avoid deserializing similar attestation multiple times which could help the gc + * Validate a single unaggregated attestation + * subnet is null for api attestations */ -async function validateAttestation( +export async function validateAttestation( fork: ForkName, chain: IBeaconChain, attestationOrBytes: AttestationOrBytes, - /** Optional, to allow verifying attestations through API with unknown subnet */ subnet: number | null, prioritizeBls = false ): Promise { + const phase0Result = await validateGossipAttestationNoSignatureCheck(fork, chain, attestationOrBytes, subnet); + const {attestation, signatureSet, validatorIndex} = phase0Result; + const isValid = await chain.bls.verifySignatureSets([signatureSet], {batchable: true, priority: prioritizeBls}); + + if (isValid) { + const targetEpoch = attestation.data.target.epoch; + chain.seenAttesters.add(targetEpoch, validatorIndex); + return phase0Result; + } else { + throw new AttestationError(GossipAction.IGNORE, { + code: AttestationErrorCode.INVALID_SIGNATURE, + }); + } +} + +/** + * Only deserialize the attestation if needed, use the cached AttestationData instead + * This is to avoid deserializing similar attestation multiple times which could help the gc + */ +async function validateGossipAttestationNoSignatureCheck( + fork: ForkName, + chain: IBeaconChain, + attestationOrBytes: AttestationOrBytes, + /** Optional, to allow verifying attestations through API with unknown subnet */ + subnet: number | null +): Promise { // Do checks in this order: // - do early checks (w/o indexed attestation) // - > obtain indexed attestation and committes per slot @@ -108,8 +256,13 @@ async function validateAttestation( let attDataBase64: AttDataBase64 | null; if (attestationOrBytes.serializedData) { // gossip - attDataBase64 = getAttDataBase64FromAttestationSerialized(attestationOrBytes.serializedData); const attSlot = attestationOrBytes.attSlot; + if (!attestationOrBytes.attDataBase64) { + throw new AttestationError(GossipAction.IGNORE, { + code: AttestationErrorCode.NO_INDEXED_DATA, + }); + } + attDataBase64 = attestationOrBytes.attDataBase64; const cachedAttData = attDataBase64 !== null ? chain.seenAttestationDatas.get(attSlot, attDataBase64) : null; if (cachedAttData === null) { const attestation = sszDeserializeAttestation(attestationOrBytes.serializedData); @@ -263,7 +416,7 @@ async function validateAttestation( // [REJECT] The signature of attestation is valid. const attestingIndices = [validatorIndex]; - let signatureSet: ISignatureSet; + let signatureSet: SingleSignatureSet; let attDataRootHex: RootHex; const signature = attestationOrCache.attestation ? attestationOrCache.attestation.signature @@ -304,24 +457,7 @@ async function validateAttestation( } } - if (!(await chain.bls.verifySignatureSets([signatureSet], {batchable: true, priority: prioritizeBls}))) { - throw new AttestationError(GossipAction.REJECT, {code: AttestationErrorCode.INVALID_SIGNATURE}); - } - - // Now that the attestation has been fully verified, store that we have received a valid attestation from this validator. - // - // It's important to double check that the attestation still hasn't been observed, since - // there can be a race-condition if we receive two attestations at the same time and - // process them in different threads. - if (chain.seenAttesters.isKnown(targetEpoch, validatorIndex)) { - throw new AttestationError(GossipAction.IGNORE, { - code: AttestationErrorCode.ATTESTATION_ALREADY_KNOWN, - targetEpoch, - validatorIndex, - }); - } - - chain.seenAttesters.add(targetEpoch, validatorIndex); + // no signature check, leave that for phase1 const indexedAttestation: phase0.IndexedAttestation = { attestingIndices, @@ -336,7 +472,7 @@ async function validateAttestation( data: attData, signature, }; - return {attestation, indexedAttestation, subnet: expectedSubnet, attDataRootHex}; + return {attestation, indexedAttestation, subnet: expectedSubnet, attDataRootHex, signatureSet, validatorIndex}; } /** diff --git a/packages/beacon-node/src/metrics/metrics/lodestar.ts b/packages/beacon-node/src/metrics/metrics/lodestar.ts index 8b3410a6eadf..aae4ae0fe6ba 100644 --- a/packages/beacon-node/src/metrics/metrics/lodestar.ts +++ b/packages/beacon-node/src/metrics/metrics/lodestar.ts @@ -38,9 +38,9 @@ export function createLodestarMetrics( help: "Count of total gossip validation queue length", labelNames: ["topic"], }), - dropRatio: register.gauge<"topic">({ - name: "lodestar_gossip_validation_queue_current_drop_ratio", - help: "Current drop ratio of gossip validation queue", + keySize: register.gauge<"topic">({ + name: "lodestar_gossip_validation_queue_key_size", + help: "Count of total gossip validation queue key size", labelNames: ["topic"], }), droppedJobs: register.gauge<"topic">({ @@ -575,6 +575,22 @@ export function createLodestarMetrics( labelNames: ["caller"], buckets: [0, 1, 2, 4, 8, 16, 32, 64], }), + attestationBatchCount: register.gauge({ + name: "lodestar_gossip_attestation_verified_in_batch_count", + help: "Count of attestations verified in batch", + }), + attestationNonBatchCount: register.gauge({ + name: "lodestar_gossip_attestation_verified_non_batch_count", + help: "Count of attestations NOT verified in batch", + }), + totalBatch: register.gauge({ + name: "lodestar_gossip_attestation_total_batch_count", + help: "Total number of attestation batches", + }), + totalBatchFallbackBlsCheck: register.gauge({ + name: "lodestar_gossip_attestation_total_batch_fallback_bls_check_count", + help: "Total number of attestation batches that fallback to checking each signature separately", + }), }, // Gossip block diff --git a/packages/beacon-node/src/network/gossip/interface.ts b/packages/beacon-node/src/network/gossip/interface.ts index 58961bd6db2f..825178532ee8 100644 --- a/packages/beacon-node/src/network/gossip/interface.ts +++ b/packages/beacon-node/src/network/gossip/interface.ts @@ -7,6 +7,7 @@ import {BeaconConfig} from "@lodestar/config"; import {Logger} from "@lodestar/utils"; import {IBeaconChain} from "../../chain/index.js"; import {JobItemQueue} from "../../util/queue/index.js"; +import {AttestationError} from "../../chain/errors/attestationError.js"; export enum GossipType { beacon_block = "beacon_block", @@ -124,13 +125,18 @@ export type GossipModules = { * * js-libp2p-gossipsub expects validation functions that look like this */ -export type GossipValidatorFn = ( - topic: GossipTopic, - msg: Message, - propagationSource: PeerIdStr, - seenTimestampSec: number, - msgSlot: Slot | null -) => Promise; +export type GossipMessageInfo = { + topic: GossipTopic; + msg: Message; + propagationSource: PeerIdStr; + seenTimestampSec: number; + msgSlot: Slot | null; + indexed?: string; +}; + +export type GossipValidatorFn = (messageInfo: GossipMessageInfo) => Promise; + +export type GossipValidatorBatchFn = (messageInfos: GossipMessageInfo[]) => Promise; export type ValidatorFnsByType = {[K in GossipType]: GossipValidatorFn}; @@ -141,22 +147,31 @@ export type GossipJobQueues = { export type GossipData = { serializedData: Uint8Array; msgSlot?: Slot | null; + indexed?: string; +}; + +export type GossipHandlerParam = { + gossipData: GossipData; + topic: GossipTopicMap[GossipType]; + peerIdStr: string; + seenTimestampSec: number; }; -export type GossipHandlerFn = ( - gossipData: GossipData, - topic: GossipTopicMap[GossipType], - peerIdStr: string, - seenTimestampSec: number -) => Promise; +export type GossipHandlerFn = (gossipHandlerParam: GossipHandlerParam) => Promise; + +export type BatchGossipHandlerFn = (gossipHandlerParam: GossipHandlerParam[]) => Promise<(null | AttestationError)[]>; + +export type GossipHandlerParamGeneric = { + gossipData: GossipData; + topic: GossipTopicMap[T]; + peerIdStr: string; + seenTimestampSec: number; +}; export type GossipHandlers = { - [K in GossipType]: ( - gossipData: GossipData, - topic: GossipTopicMap[K], - peerIdStr: string, - seenTimestampSec: number - ) => Promise; + [K in GossipType]: + | ((gossipHandlerParam: GossipHandlerParamGeneric) => Promise) + | ((gossipHandlerParams: GossipHandlerParamGeneric[]) => Promise<(null | AttestationError)[]>); }; // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index c0a91a0ae812..c08c9198ff8f 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -17,10 +17,9 @@ import { GossipActionError, SyncCommitteeError, } from "../../chain/errors/index.js"; -import {GossipHandlers, GossipType} from "../gossip/interface.js"; +import {GossipHandlerParamGeneric, GossipHandlers, GossipType} from "../gossip/interface.js"; import { validateGossipAggregateAndProof, - validateGossipAttestation, validateGossipAttesterSlashing, validateGossipBlock, validateGossipProposerSlashing, @@ -28,8 +27,9 @@ import { validateSyncCommitteeGossipContributionAndProof, validateGossipVoluntaryExit, validateGossipBlsToExecutionChange, - AttestationValidationResult, AggregateAndProofValidationResult, + validateGossipAttestationsSameAttData, + AttestationOrBytes, } from "../../chain/validation/index.js"; import {NetworkEvent, NetworkEventBus} from "../events.js"; import {PeerAction} from "../peers/index.js"; @@ -243,7 +243,14 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH } return { - [GossipType.beacon_block]: async ({serializedData}, topic, peerIdStr, seenTimestampSec) => { + [GossipType.beacon_block]: async ({ + gossipData, + topic, + peerIdStr, + seenTimestampSec, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; + const signedBlock = sszDeserialize(topic, serializedData); const blockInput = await validateBeaconBlock( signedBlock, @@ -263,7 +270,13 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH } }, - [GossipType.blob_sidecar]: async ({serializedData}, topic, peerIdStr, seenTimestampSec) => { + [GossipType.blob_sidecar]: async ({ + gossipData, + topic, + peerIdStr, + seenTimestampSec, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const signedBlob = sszDeserialize(topic, serializedData); if (config.getForkSeq(signedBlob.message.slot) < ForkSeq.deneb) { throw new GossipActionError(GossipAction.REJECT, {code: "PRE_DENEB_BLOCK"}); @@ -280,7 +293,12 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH } }, - [GossipType.beacon_aggregate_and_proof]: async ({serializedData}, topic, _peer, seenTimestampSec) => { + [GossipType.beacon_aggregate_and_proof]: async ({ + gossipData, + topic, + seenTimestampSec, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; let validationResult: AggregateAndProofValidationResult; const signedAggregateAndProof = sszDeserialize(topic, serializedData); const {fork} = topic; @@ -320,56 +338,79 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH chain.emitter.emit(routes.events.EventType.attestation, signedAggregateAndProof.message.aggregate); }, - - [GossipType.beacon_attestation]: async ({serializedData, msgSlot}, topic, _peer, seenTimestampSec) => { - if (msgSlot == undefined) { - throw Error("msgSlot is undefined for beacon_attestation topic"); + [GossipType.beacon_attestation]: async ( + gossipHandlerParams: GossipHandlerParamGeneric[] + ) => { + const results: (null | AttestationError)[] = []; + if (gossipHandlerParams.length === 0) { + return results; } - const {subnet, fork} = topic; - - // do not deserialize gossipSerializedData here, it's done in validateGossipAttestation only if needed - let validationResult: AttestationValidationResult; - try { - validationResult = await validateGossipAttestation( - fork, - chain, - {attestation: null, serializedData, attSlot: msgSlot}, - subnet - ); - } catch (e) { - if (e instanceof AttestationError && e.action === GossipAction.REJECT) { - chain.persistInvalidSszBytes(ssz.phase0.Attestation.typeName, serializedData, "gossip_reject"); + // all attestations should have same attestation data as filtered by network processor + const {subnet, fork} = gossipHandlerParams[0].topic; + const validationParams = gossipHandlerParams.map((param) => ({ + attestation: null, + serializedData: param.gossipData.serializedData, + attSlot: param.gossipData.msgSlot, + attDataBase64: param.gossipData.indexed, + })) as AttestationOrBytes[]; + const { + results: validationResults, + batchableBls, + fallbackBls, + } = await validateGossipAttestationsSameAttData(fork, chain, validationParams, subnet); + for (const [i, validationResult] of validationResults.entries()) { + if (validationResult.err) { + results.push(validationResult.err as AttestationError); + continue; } - throw e; - } - // Handler - const {indexedAttestation, attDataRootHex, attestation} = validationResult; - metrics?.registerGossipUnaggregatedAttestation(seenTimestampSec, indexedAttestation); + results.push(null); - try { - // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages - // but don't add to attestation pool, to save CPU and RAM - if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { - const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); - metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); - } - } catch (e) { - logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); - } + // Handler + const {indexedAttestation, attDataRootHex, attestation} = validationResult.result; + metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation); - if (!options.dontSendGossipAttestationsToForkchoice) { try { - chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); + // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages + // but don't add to attestation pool, to save CPU and RAM + if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { + const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); + metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); + } } catch (e) { - logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); + logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); + } + + if (!options.dontSendGossipAttestationsToForkchoice) { + try { + chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); + } catch (e) { + logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); + } } + + chain.emitter.emit(routes.events.EventType.attestation, attestation); + } + + if (batchableBls) { + metrics?.gossipAttestation.totalBatch.inc(); + metrics?.gossipAttestation.attestationBatchCount.inc(gossipHandlerParams.length); + } else { + metrics?.gossipAttestation.attestationNonBatchCount.inc(gossipHandlerParams.length); + } + + if (fallbackBls) { + metrics?.gossipAttestation.totalBatchFallbackBlsCheck.inc(); } - chain.emitter.emit(routes.events.EventType.attestation, attestation); + return results; }, - [GossipType.attester_slashing]: async ({serializedData}, topic) => { + [GossipType.attester_slashing]: async ({ + gossipData, + topic, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const attesterSlashing = sszDeserialize(topic, serializedData); await validateGossipAttesterSlashing(chain, attesterSlashing); @@ -383,7 +424,11 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH } }, - [GossipType.proposer_slashing]: async ({serializedData}, topic) => { + [GossipType.proposer_slashing]: async ({ + gossipData, + topic, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const proposerSlashing = sszDeserialize(topic, serializedData); await validateGossipProposerSlashing(chain, proposerSlashing); @@ -396,7 +441,8 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH } }, - [GossipType.voluntary_exit]: async ({serializedData}, topic) => { + [GossipType.voluntary_exit]: async ({gossipData, topic}: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const voluntaryExit = sszDeserialize(topic, serializedData); await validateGossipVoluntaryExit(chain, voluntaryExit); @@ -411,7 +457,11 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH chain.emitter.emit(routes.events.EventType.voluntaryExit, voluntaryExit); }, - [GossipType.sync_committee_contribution_and_proof]: async ({serializedData}, topic) => { + [GossipType.sync_committee_contribution_and_proof]: async ({ + gossipData, + topic, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const contributionAndProof = sszDeserialize(topic, serializedData); const {syncCommitteeParticipantIndices} = await validateSyncCommitteeGossipContributionAndProof( chain, @@ -435,7 +485,8 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH chain.emitter.emit(routes.events.EventType.contributionAndProof, contributionAndProof); }, - [GossipType.sync_committee]: async ({serializedData}, topic) => { + [GossipType.sync_committee]: async ({gossipData, topic}: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const syncCommittee = sszDeserialize(topic, serializedData); const {subnet} = topic; let indexInSubcommittee = 0; @@ -458,18 +509,30 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH } }, - [GossipType.light_client_finality_update]: async ({serializedData}, topic) => { + [GossipType.light_client_finality_update]: async ({ + gossipData, + topic, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const lightClientFinalityUpdate = sszDeserialize(topic, serializedData); validateLightClientFinalityUpdate(config, chain, lightClientFinalityUpdate); }, - [GossipType.light_client_optimistic_update]: async ({serializedData}, topic) => { + [GossipType.light_client_optimistic_update]: async ({ + gossipData, + topic, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const lightClientOptimisticUpdate = sszDeserialize(topic, serializedData); validateLightClientOptimisticUpdate(config, chain, lightClientOptimisticUpdate); }, // blsToExecutionChange is to be generated and validated against GENESIS_FORK_VERSION - [GossipType.bls_to_execution_change]: async ({serializedData}, topic) => { + [GossipType.bls_to_execution_change]: async ({ + gossipData, + topic, + }: GossipHandlerParamGeneric) => { + const {serializedData} = gossipData; const blsToExecutionChange = sszDeserialize(topic, serializedData); await validateGossipBlsToExecutionChange(chain, blsToExecutionChange); diff --git a/packages/beacon-node/src/network/processor/gossipQueues/index.ts b/packages/beacon-node/src/network/processor/gossipQueues/index.ts index c85b2560ebce..37eb354f8bb9 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/index.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/index.ts @@ -1,33 +1,36 @@ import {mapValues} from "@lodestar/utils"; import {GossipType} from "../../gossip/interface.js"; +import {PendingGossipsubMessage} from "../types.js"; +import {getAttDataBase64FromAttestationSerialized} from "../../../util/sszBytes.js"; import {LinearGossipQueue} from "./linear.js"; +import { + DropType, + GossipQueue, + GossipQueueOpts, + QueueType, + isIndexedGossipQueueAvgTimeOpts, + isIndexedGossipQueueMinSizeOpts, +} from "./types.js"; +import {IndexedGossipQueueMinSize} from "./indexed.js"; +import {IndexedGossipQueueAvgTime} from "./indexedAvgTime.js"; -export enum QueueType { - FIFO = "FIFO", - LIFO = "LIFO", -} - -export enum DropType { - count = "count", - ratio = "ratio", -} +/** + * In normal condition, the higher this value the more efficient the signature verification. + * However, if at least 1 signature is invalid, we need to verify each signature separately. + */ +const MAX_GOSSIP_ATTESTATION_BATCH_SIZE = 128; -type DropOpts = - | { - type: DropType.count; - count: number; - } - | { - type: DropType.ratio; - start: number; - step: number; - }; +/** + * Batching too few signatures and verifying them on main thread is not worth it, + * we should only batch verify when there are at least 32 signatures. + */ +export const MIN_SIGNATURE_SETS_TO_BATCH_VERIFY = 32; /** * Numbers from https://github.com/sigp/lighthouse/blob/b34a79dc0b02e04441ba01fd0f304d1e203d877d/beacon_node/network/src/beacon_processor/mod.rs#L69 */ const gossipQueueOpts: { - [K in GossipType]: GossipQueueOpts; + [K in GossipType]: GossipQueueOpts; } = { // validation gossip block asap [GossipType.beacon_block]: {maxLength: 1024, type: QueueType.FIFO, dropOpts: {type: DropType.count, count: 1}}, @@ -49,8 +52,9 @@ const gossipQueueOpts: { // start with dropping 1% of the queue, then increase 1% more each time. Reset when queue is empty [GossipType.beacon_attestation]: { maxLength: 24576, - type: QueueType.LIFO, - dropOpts: {type: DropType.ratio, start: 0.01, step: 0.01}, + indexFn: (item: PendingGossipsubMessage) => getAttDataBase64FromAttestationSerialized(item.msg.data), + minChunkSize: MIN_SIGNATURE_SETS_TO_BATCH_VERIFY, + maxChunkSize: MAX_GOSSIP_ATTESTATION_BATCH_SIZE, }, [GossipType.voluntary_exit]: {maxLength: 4096, type: QueueType.FIFO, dropOpts: {type: DropType.count, count: 1}}, [GossipType.proposer_slashing]: {maxLength: 4096, type: QueueType.FIFO, dropOpts: {type: DropType.count, count: 1}}, @@ -79,12 +83,6 @@ const gossipQueueOpts: { }, }; -type GossipQueueOpts = { - type: QueueType; - maxLength: number; - dropOpts: DropOpts; -}; - /** * Wraps a GossipValidatorFn with a queue, to limit the processing of gossip objects by type. * @@ -101,8 +99,14 @@ type GossipQueueOpts = { * By topic is too specific, so by type groups all similar objects in the same queue. All in the same won't allow * to customize different queue behaviours per object type (see `gossipQueueOpts`). */ -export function createGossipQueues(): {[K in GossipType]: LinearGossipQueue} { +export function createGossipQueues(): {[K in GossipType]: GossipQueue} { return mapValues(gossipQueueOpts, (opts) => { - return new LinearGossipQueue(opts); + if (isIndexedGossipQueueMinSizeOpts(opts)) { + return new IndexedGossipQueueMinSize(opts); + } else if (isIndexedGossipQueueAvgTimeOpts(opts)) { + return new IndexedGossipQueueAvgTime(opts); + } else { + return new LinearGossipQueue(opts); + } }); } diff --git a/packages/beacon-node/src/network/processor/gossipQueues/types.ts b/packages/beacon-node/src/network/processor/gossipQueues/types.ts index 074eebfb5219..9d6e41c7579e 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/types.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/types.ts @@ -1,4 +1,4 @@ -export type GossipQueueOpts = LinearGossipQueueOpts | IndexedGossipQueueOpts; +export type GossipQueueOpts = LinearGossipQueueOpts | IndexedGossipQueueOpts | IndexedGossipQueueMinSizeOpts; export type LinearGossipQueueOpts = { type: QueueType; diff --git a/packages/beacon-node/src/network/processor/gossipValidatorFn.ts b/packages/beacon-node/src/network/processor/gossipValidatorFn.ts index a3f4a4aa25b4..89c40b7482f0 100644 --- a/packages/beacon-node/src/network/processor/gossipValidatorFn.ts +++ b/packages/beacon-node/src/network/processor/gossipValidatorFn.ts @@ -2,8 +2,15 @@ import {TopicValidatorResult} from "@libp2p/interface/pubsub"; import {ChainForkConfig} from "@lodestar/config"; import {Logger} from "@lodestar/utils"; import {Metrics} from "../../metrics/index.js"; -import {GossipValidatorFn, GossipHandlers, GossipHandlerFn} from "../gossip/interface.js"; -import {GossipActionError, GossipAction} from "../../chain/errors/index.js"; +import { + GossipValidatorFn, + GossipHandlers, + GossipHandlerFn, + GossipValidatorBatchFn, + BatchGossipHandlerFn, + GossipMessageInfo, +} from "../gossip/interface.js"; +import {GossipActionError, GossipAction, AttestationError} from "../../chain/errors/index.js"; export type ValidatorFnModules = { config: ChainForkConfig; @@ -11,6 +18,68 @@ export type ValidatorFnModules = { metrics: Metrics | null; }; +/** + * Similar to getGossipValidatorFn but return a function to accept a batch of beacon_attestation messages + * with the same attestation data + */ +export function getGossipValidatorBatchFn( + gossipHandlers: GossipHandlers, + modules: ValidatorFnModules +): GossipValidatorBatchFn { + const {logger, metrics} = modules; + + return async function gossipValidatorBatchFn(messageInfos: GossipMessageInfo[]) { + // all messageInfos have same topic + const {topic} = messageInfos[0]; + const type = topic.type; + try { + const results = await (gossipHandlers[type] as BatchGossipHandlerFn)( + messageInfos.map((messageInfo) => ({ + gossipData: { + serializedData: messageInfo.msg.data, + msgSlot: messageInfo.msgSlot, + indexed: messageInfo.indexed, + }, + topic, + peerIdStr: messageInfo.propagationSource, + seenTimestampSec: messageInfo.seenTimestampSec, + })) + ); + + return results.map((e) => { + if (e == null) { + return TopicValidatorResult.Accept; + } + + if (!(e instanceof AttestationError)) { + logger.debug(`Gossip batch validation ${type} threw a non-AttestationError`, {}, e as Error); + metrics?.networkProcessor.gossipValidationIgnore.inc({topic: type}); + return TopicValidatorResult.Ignore; + } + + switch (e.action) { + case GossipAction.IGNORE: + metrics?.networkProcessor.gossipValidationIgnore.inc({topic: type}); + return TopicValidatorResult.Ignore; + + case GossipAction.REJECT: + metrics?.networkProcessor.gossipValidationReject.inc({topic: type}); + logger.debug(`Gossip validation ${type} rejected`, {}, e); + return TopicValidatorResult.Reject; + } + }); + } catch (e) { + // Don't expect error here + logger.debug(`Gossip batch validation ${type} threw an error`, {}, e as Error); + const results: TopicValidatorResult[] = []; + for (let i = 0; i < messageInfos.length; i++) { + results.push(TopicValidatorResult.Ignore); + } + return results; + } + }; +} + /** * Returns a GossipSub validator function from a GossipHandlerFn. GossipHandlerFn may throw GossipActionError if one * or more validation conditions from the consensus-specs#p2p-interface are not satisfied. @@ -28,16 +97,16 @@ export type ValidatorFnModules = { export function getGossipValidatorFn(gossipHandlers: GossipHandlers, modules: ValidatorFnModules): GossipValidatorFn { const {logger, metrics} = modules; - return async function gossipValidatorFn(topic, msg, propagationSource, seenTimestampSec, msgSlot) { + return async function gossipValidatorFn({topic, msg, propagationSource, seenTimestampSec, msgSlot}) { const type = topic.type; try { - await (gossipHandlers[type] as GossipHandlerFn)( - {serializedData: msg.data, msgSlot}, + await (gossipHandlers[type] as GossipHandlerFn)({ + gossipData: {serializedData: msg.data, msgSlot}, topic, - propagationSource, - seenTimestampSec - ); + peerIdStr: propagationSource, + seenTimestampSec, + }); metrics?.networkProcessor.gossipValidationAccept.inc({topic: type}); diff --git a/packages/beacon-node/src/network/processor/index.ts b/packages/beacon-node/src/network/processor/index.ts index 332a8577887d..2692d692a22f 100644 --- a/packages/beacon-node/src/network/processor/index.ts +++ b/packages/beacon-node/src/network/processor/index.ts @@ -8,13 +8,19 @@ import {Metrics} from "../../metrics/metrics.js"; import {IBeaconDb} from "../../db/interface.js"; import {ClockEvent} from "../../util/clock.js"; import {NetworkEvent, NetworkEventBus} from "../events.js"; -import {GossipHandlers, GossipType, GossipValidatorFn} from "../gossip/interface.js"; +import { + GossipHandlers, + GossipMessageInfo, + GossipType, + GossipValidatorBatchFn, + GossipValidatorFn, +} from "../gossip/interface.js"; import {PeerIdStr} from "../peers/index.js"; import {createGossipQueues} from "./gossipQueues/index.js"; import {PendingGossipsubMessage} from "./types.js"; import {ValidatorFnsModules, GossipHandlerOpts, getGossipHandlers} from "./gossipHandlers.js"; import {createExtractBlockSlotRootFns} from "./extractSlotRootFns.js"; -import {ValidatorFnModules, getGossipValidatorFn} from "./gossipValidatorFn.js"; +import {ValidatorFnModules, getGossipValidatorBatchFn, getGossipValidatorFn} from "./gossipValidatorFn.js"; export * from "./types.js"; @@ -142,7 +148,8 @@ export class NetworkProcessor { private readonly logger: Logger; private readonly metrics: Metrics | null; private readonly gossipValidatorFn: GossipValidatorFn; - private readonly gossipQueues = createGossipQueues(); + private readonly gossipValidatorBatchFn: GossipValidatorBatchFn; + private readonly gossipQueues = createGossipQueues(); private readonly gossipTopicConcurrency = mapValues(this.gossipQueues, () => 0); private readonly extractBlockSlotRootFns = createExtractBlockSlotRootFns(); // we may not receive the block for Attestation and SignedAggregateAndProof messages, in that case PendingGossipsubMessage needs @@ -163,6 +170,10 @@ export class NetworkProcessor { this.logger = logger; this.events = events; this.gossipValidatorFn = getGossipValidatorFn(modules.gossipHandlers ?? getGossipHandlers(modules, opts), modules); + this.gossipValidatorBatchFn = getGossipValidatorBatchFn( + modules.gossipHandlers ?? getGossipHandlers(modules, opts), + modules + ); events.on(NetworkEvent.pendingGossipsubMessage, this.onPendingGossipsubMessage.bind(this)); this.chain.emitter.on(routes.events.EventType.block, this.onBlockProcessed.bind(this)); @@ -179,7 +190,7 @@ export class NetworkProcessor { metrics.gossipValidationQueue.length.addCollect(() => { for (const topic of executeGossipWorkOrder) { metrics.gossipValidationQueue.length.set({topic}, this.gossipQueues[topic].length); - metrics.gossipValidationQueue.dropRatio.set({topic}, this.gossipQueues[topic].dropRatio); + metrics.gossipValidationQueue.keySize.set({topic}, this.gossipQueues[topic].keySize); metrics.gossipValidationQueue.concurrency.set({topic}, this.gossipTopicConcurrency[topic]); } metrics.reprocessGossipAttestations.countPerSlot.set(this.unknownBlockGossipsubMessagesCount); @@ -363,13 +374,14 @@ export class NetworkProcessor { } const item = this.gossipQueues[topic].next(); + const numMessages = Array.isArray(item) ? item.length : 1; if (item) { - this.gossipTopicConcurrency[topic]++; + this.gossipTopicConcurrency[topic] += numMessages; this.processPendingGossipsubMessage(item) - .finally(() => this.gossipTopicConcurrency[topic]--) + .finally(() => (this.gossipTopicConcurrency[topic] -= numMessages)) .catch((e) => this.logger.error("processGossipAttestations must not throw", {}, e)); - jobsSubmitted++; + jobsSubmitted += numMessages; // Attempt to find more work, but check canAcceptWork() again and run executeGossipWorkOrder priorization continue job_loop; } @@ -384,40 +396,68 @@ export class NetworkProcessor { } } - private async processPendingGossipsubMessage(message: PendingGossipsubMessage): Promise { - message.startProcessUnixSec = Date.now() / 1000; + private async processPendingGossipsubMessage( + messageOrArray: PendingGossipsubMessage | PendingGossipsubMessage[] + ): Promise { + const nowSec = Date.now() / 1000; + if (Array.isArray(messageOrArray)) { + messageOrArray.forEach((msg) => (msg.startProcessUnixSec = nowSec)); + } else { + messageOrArray.startProcessUnixSec = nowSec; + } - const acceptance = await this.gossipValidatorFn( - message.topic, - message.msg, - message.propagationSource, - message.seenTimestampSec, - message.msgSlot ?? null - ); + const acceptanceArr = Array.isArray(messageOrArray) + ? // for beacon_attestation topic, process attestations with same attestation data + // we always have msgSlot in beaccon_attestation topic so the type conversion is safe + await this.gossipValidatorBatchFn(messageOrArray as GossipMessageInfo[]) + : [ + // for other topics + await this.gossipValidatorFn({...messageOrArray, msgSlot: messageOrArray.msgSlot ?? null}), + ]; + + if (Array.isArray(messageOrArray)) { + messageOrArray.forEach((msg) => this.trackJobTime(msg, messageOrArray.length)); + } else { + this.trackJobTime(messageOrArray, 1); + } + // Use setTimeout to yield to the macro queue + // This is mostly due to too many attestation messages, and a gossipsub RPC may + // contain multiple of them. This helps avoid the I/O lag issue. + + if (Array.isArray(messageOrArray)) { + for (const [i, msg] of messageOrArray.entries()) { + setTimeout(() => { + this.events.emit(NetworkEvent.gossipMessageValidationResult, { + msgId: msg.msgId, + propagationSource: msg.propagationSource, + acceptance: acceptanceArr[i], + }); + }, 0); + } + } else { + setTimeout(() => { + this.events.emit(NetworkEvent.gossipMessageValidationResult, { + msgId: messageOrArray.msgId, + propagationSource: messageOrArray.propagationSource, + acceptance: acceptanceArr[0], + }); + }, 0); + } + } + + private trackJobTime(message: PendingGossipsubMessage, numJob: number): void { if (message.startProcessUnixSec !== null) { this.metrics?.gossipValidationQueue.jobWaitTime.observe( {topic: message.topic.type}, message.startProcessUnixSec - message.seenTimestampSec ); + // if it takes 64ms to process 64 jobs, the average job time is 1ms this.metrics?.gossipValidationQueue.jobTime.observe( {topic: message.topic.type}, - Date.now() / 1000 - message.startProcessUnixSec + (Date.now() / 1000 - message.startProcessUnixSec) / numJob ); } - - // Use setTimeout to yield to the macro queue - // This is mostly due to too many attestation messages, and a gossipsub RPC may - // contain multiple of them. This helps avoid the I/O lag issue. - setTimeout( - () => - this.events.emit(NetworkEvent.gossipMessageValidationResult, { - msgId: message.msgId, - propagationSource: message.propagationSource, - acceptance, - }), - 0 - ); } /** diff --git a/packages/beacon-node/src/network/processor/types.ts b/packages/beacon-node/src/network/processor/types.ts index fcb3fd90b366..c571e7ab4870 100644 --- a/packages/beacon-node/src/network/processor/types.ts +++ b/packages/beacon-node/src/network/processor/types.ts @@ -12,6 +12,8 @@ export type PendingGossipsubMessage = { msg: Message; // only available for beacon_attestation and aggregate_and_proof msgSlot?: Slot; + // indexed data if any, only available for beacon_attestation as a result of getAttDataBase64FromAttestationSerialized + indexed?: string; msgId: string; // TODO: Refactor into accepting string (requires gossipsub changes) for easier multi-threading propagationSource: PeerIdStr; diff --git a/packages/beacon-node/src/util/wrapError.ts b/packages/beacon-node/src/util/wrapError.ts index f64661b926a8..3b25da203c47 100644 --- a/packages/beacon-node/src/util/wrapError.ts +++ b/packages/beacon-node/src/util/wrapError.ts @@ -1,4 +1,4 @@ -type Result = {err: null; result: T} | {err: Error}; +export type Result = {err: null; result: T} | {err: Error}; /** * Wraps a promise to return either an error or result diff --git a/packages/beacon-node/test/e2e/network/gossipsub.test.ts b/packages/beacon-node/test/e2e/network/gossipsub.test.ts index b943c356ff00..49ba6c42d90e 100644 --- a/packages/beacon-node/test/e2e/network/gossipsub.test.ts +++ b/packages/beacon-node/test/e2e/network/gossipsub.test.ts @@ -4,7 +4,7 @@ import {sleep} from "@lodestar/utils"; import {computeStartSlotAtEpoch} from "@lodestar/state-transition"; import {ssz} from "@lodestar/types"; import {Network} from "../../../src/network/index.js"; -import {GossipType, GossipHandlers} from "../../../src/network/gossip/index.js"; +import {GossipType, GossipHandlers, GossipHandlerParamGeneric} from "../../../src/network/gossip/index.js"; import {connect, onPeerConnect, getNetworkForTest} from "../../utils/network.js"; describe("gossipsub / main thread", function () { @@ -57,8 +57,8 @@ function runTests(this: Mocha.Suite, {useWorker}: {useWorker: boolean}): void { const onVoluntaryExitPromise = new Promise((resolve) => (onVoluntaryExit = resolve)); const {netA, netB} = await mockModules({ - [GossipType.voluntary_exit]: async ({serializedData}) => { - onVoluntaryExit(serializedData); + [GossipType.voluntary_exit]: async ({gossipData}: GossipHandlerParamGeneric) => { + onVoluntaryExit(gossipData.serializedData); }, }); @@ -90,8 +90,10 @@ function runTests(this: Mocha.Suite, {useWorker}: {useWorker: boolean}): void { const onBlsToExecutionChangePromise = new Promise((resolve) => (onBlsToExecutionChange = resolve)); const {netA, netB} = await mockModules({ - [GossipType.bls_to_execution_change]: async ({serializedData}) => { - onBlsToExecutionChange(serializedData); + [GossipType.bls_to_execution_change]: async ({ + gossipData, + }: GossipHandlerParamGeneric) => { + onBlsToExecutionChange(gossipData.serializedData); }, }); @@ -124,8 +126,10 @@ function runTests(this: Mocha.Suite, {useWorker}: {useWorker: boolean}): void { ); const {netA, netB} = await mockModules({ - [GossipType.light_client_optimistic_update]: async ({serializedData}) => { - onLightClientOptimisticUpdate(serializedData); + [GossipType.light_client_optimistic_update]: async ({ + gossipData, + }: GossipHandlerParamGeneric) => { + onLightClientOptimisticUpdate(gossipData.serializedData); }, }); @@ -161,8 +165,10 @@ function runTests(this: Mocha.Suite, {useWorker}: {useWorker: boolean}): void { ); const {netA, netB} = await mockModules({ - [GossipType.light_client_finality_update]: async ({serializedData}) => { - onLightClientFinalityUpdate(serializedData); + [GossipType.light_client_finality_update]: async ({ + gossipData, + }: GossipHandlerParamGeneric) => { + onLightClientFinalityUpdate(gossipData.serializedData); }, }); diff --git a/packages/beacon-node/test/perf/bls/bls.test.ts b/packages/beacon-node/test/perf/bls/bls.test.ts index c73723c22d94..bd6a26a3f692 100644 --- a/packages/beacon-node/test/perf/bls/bls.test.ts +++ b/packages/beacon-node/test/perf/bls/bls.test.ts @@ -1,20 +1,26 @@ +import crypto from "node:crypto"; import {itBench} from "@dapplion/benchmark"; import bls from "@chainsafe/bls"; -import type {PublicKey, SecretKey, Signature} from "@chainsafe/bls/types"; +import {CoordType, type PublicKey, type SecretKey} from "@chainsafe/bls/types"; import {linspace} from "../../../src/util/numpy.js"; describe("BLS ops", function () { type Keypair = {publicKey: PublicKey; secretKey: SecretKey}; - type BlsSet = {publicKey: PublicKey; message: Uint8Array; signature: Signature}; + // signature needs to be in Uint8Array to match real situation + type BlsSet = {publicKey: PublicKey; message: Uint8Array; signature: Uint8Array}; // Create and cache (on demand) crypto data to benchmark const sets = new Map(); + const sameMessageSets = new Map(); const keypairs = new Map(); function getKeypair(i: number): Keypair { let keypair = keypairs.get(i); if (!keypair) { - const secretKey = bls.SecretKey.fromBytes(Buffer.alloc(32, i + 1)); + const bytes = new Uint8Array(32); + const dataView = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); + dataView.setUint32(0, i + 1, true); + const secretKey = bls.SecretKey.fromBytes(bytes); const publicKey = secretKey.toPublicKey(); keypair = {secretKey, publicKey}; keypairs.set(i, keypair); @@ -27,26 +33,64 @@ describe("BLS ops", function () { if (!set) { const {secretKey, publicKey} = getKeypair(i); const message = Buffer.alloc(32, i + 1); - set = {publicKey, message: message, signature: secretKey.sign(message)}; + set = {publicKey, message: message, signature: secretKey.sign(message).toBytes()}; sets.set(i, set); } return set; } + const seedMessage = crypto.randomBytes(32); + function getSetSameMessage(i: number): BlsSet { + const message = new Uint8Array(32); + message.set(seedMessage); + let set = sameMessageSets.get(i); + if (!set) { + const {secretKey, publicKey} = getKeypair(i); + set = {publicKey, message, signature: secretKey.sign(message).toBytes()}; + sameMessageSets.set(i, set); + } + return set; + } + // Note: getSet() caches the value, does not re-compute every time itBench({id: `BLS verify - ${bls.implementation}`, beforeEach: () => getSet(0)}, (set) => { - const isValid = set.signature.verify(set.publicKey, set.message); + const isValid = bls.Signature.fromBytes(set.signature).verify(set.publicKey, set.message); if (!isValid) throw Error("Invalid"); }); // An aggregate and proof object has 3 signatures. // We may want to bundle up to 32 sets in a single batch. - for (const count of [3, 8, 32]) { + for (const count of [3, 8, 32, 64, 128]) { itBench({ id: `BLS verifyMultipleSignatures ${count} - ${bls.implementation}`, beforeEach: () => linspace(0, count - 1).map((i) => getSet(i)), fn: (sets) => { - const isValid = bls.Signature.verifyMultipleSignatures(sets); + const isValid = bls.Signature.verifyMultipleSignatures( + sets.map((set) => ({ + publicKey: set.publicKey, + message: set.message, + signature: bls.Signature.fromBytes(set.signature), + })) + ); + if (!isValid) throw Error("Invalid"); + }, + }); + } + + // An aggregate and proof object has 3 signatures. + // We may want to bundle up to 32 sets in a single batch. + // TODO: figure out why it does not work with 256 or more + for (const count of [3, 8, 32, 64, 128]) { + itBench({ + id: `BLS verifyMultipleSignatures - same message - ${count} - ${bls.implementation}`, + beforeEach: () => linspace(0, count - 1).map((i) => getSetSameMessage(i)), + fn: (sets) => { + // aggregate and verify aggregated signatures + const aggregatedPubkey = bls.PublicKey.aggregate(sets.map((set) => set.publicKey)); + const aggregatedSignature = bls.Signature.aggregate( + sets.map((set) => bls.Signature.fromBytes(set.signature, CoordType.affine, false)) + ); + const isValid = aggregatedSignature.verify(aggregatedPubkey, sets[0].message); if (!isValid) throw Error("Invalid"); }, }); diff --git a/packages/beacon-node/test/perf/chain/validation/attestation.test.ts b/packages/beacon-node/test/perf/chain/validation/attestation.test.ts index 84b423dd7e62..f0a4f7c3e89d 100644 --- a/packages/beacon-node/test/perf/chain/validation/attestation.test.ts +++ b/packages/beacon-node/test/perf/chain/validation/attestation.test.ts @@ -1,40 +1,83 @@ -import {itBench} from "@dapplion/benchmark"; -import {ssz} from "@lodestar/types"; // eslint-disable-next-line import/no-relative-packages -import {generateTestCachedBeaconStateOnlyValidators} from "../../../../../state-transition/test/perf/util.js"; -import {validateApiAttestation, validateGossipAttestation} from "../../../../src/chain/validation/index.js"; +import {itBench, setBenchOpts} from "@dapplion/benchmark"; +import {expect} from "chai"; +import {ssz} from "@lodestar/types"; +import {generateTestCachedBeaconStateOnlyValidators} from "@lodestar/state-transition/test/perf/util.js"; +import {validateAttestation, validateGossipAttestationsSameAttData} from "../../../../src/chain/validation/index.js"; import {getAttestationValidData} from "../../../utils/validationData/attestation.js"; +import {getAttDataBase64FromAttestationSerialized} from "../../../../src/util/sszBytes.js"; -describe("validate attestation", () => { - const vc = 64; +describe("validate gossip attestation", () => { + setBenchOpts({ + minMs: 30_000, + }); + + const vc = 640_000; const stateSlot = 100; + const state = generateTestCachedBeaconStateOnlyValidators({vc, slot: stateSlot}); - const {chain, attestation, subnet} = getAttestationValidData({ + const { + chain, + attestation: attestation0, + subnet: subnet0, + } = getAttestationValidData({ currentSlot: stateSlot, - state: generateTestCachedBeaconStateOnlyValidators({vc, slot: stateSlot}), + state, + bitIndex: 0, + // enable this in local environment to match production + // blsVerifyAllMainThread: false, + }); + + const attSlot = attestation0.data.slot; + const serializedData = ssz.phase0.Attestation.serialize(attestation0); + const fork = chain.config.getForkName(stateSlot); + itBench({ + id: `validate gossip attestation - vc ${vc}`, + beforeEach: () => chain.seenAttesters["validatorIndexesByEpoch"].clear(), + fn: async () => { + await validateAttestation( + fork, + chain, + { + attestation: null, + serializedData, + attSlot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, + subnet0 + ); + }, }); - const attStruct = attestation; + for (const chunkSize of [32, 64, 128, 256]) { + const attestations = [attestation0]; + for (let i = 1; i < chunkSize; i++) { + const {attestation, subnet} = getAttestationValidData({ + currentSlot: stateSlot, + state, + bitIndex: i, + }); + expect(subnet).to.be.equal(subnet0); + attestations.push(attestation); + } - for (const [id, att] of Object.entries({struct: attStruct})) { - const serializedData = ssz.phase0.Attestation.serialize(att); - const slot = attestation.data.slot; - itBench({ - id: `validate api attestation - ${id}`, - beforeEach: () => chain.seenAttesters["validatorIndexesByEpoch"].clear(), - fn: async () => { - const fork = chain.config.getForkName(stateSlot); - await validateApiAttestation(fork, chain, {attestation: att, serializedData: null}); - }, + const attestationOrBytesArr = attestations.map((att) => { + const serializedData = ssz.phase0.Attestation.serialize(att); + return { + attestation: null, + serializedData, + attSlot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }; }); itBench({ - id: `validate gossip attestation - ${id}`, + id: `batch validate gossip attestation - vc ${vc} - chunk ${chunkSize}`, beforeEach: () => chain.seenAttesters["validatorIndexesByEpoch"].clear(), fn: async () => { - const fork = chain.config.getForkName(stateSlot); - await validateGossipAttestation(fork, chain, {attestation: null, serializedData, attSlot: slot}, subnet); + await validateGossipAttestationsSameAttData(fork, chain, attestationOrBytesArr, subnet0); }, + runsFactor: chunkSize, }); } }); diff --git a/packages/beacon-node/test/unit/chain/validation/attestation.test.ts b/packages/beacon-node/test/unit/chain/validation/attestation.test.ts index f28f62abc229..24dc073891e5 100644 --- a/packages/beacon-node/test/unit/chain/validation/attestation.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/attestation.test.ts @@ -1,21 +1,30 @@ import sinon, {SinonStubbedInstance} from "sinon"; import {expect} from "chai"; import {BitArray} from "@chainsafe/ssz"; -import {SLOTS_PER_EPOCH} from "@lodestar/params"; -import {computeEpochAtSlot, computeStartSlotAtEpoch, processSlots} from "@lodestar/state-transition"; +import type {PublicKey, SecretKey} from "@chainsafe/bls/types"; +import bls from "@chainsafe/bls"; +import {ForkName, SLOTS_PER_EPOCH} from "@lodestar/params"; import {defaultChainConfig, createChainForkConfig, BeaconConfig} from "@lodestar/config"; -import {Slot, ssz} from "@lodestar/types"; import {ProtoBlock} from "@lodestar/fork-choice"; // eslint-disable-next-line import/no-relative-packages -import {generateTestCachedBeaconStateOnlyValidators} from "../../../../../state-transition/test/perf/util.js"; +import {SignatureSetType, computeEpochAtSlot, computeStartSlotAtEpoch, processSlots} from "@lodestar/state-transition"; +import {Slot, ssz} from "@lodestar/types"; +import {generateTestCachedBeaconStateOnlyValidators} from "@lodestar/state-transition/test/perf/util.js"; import {IBeaconChain} from "../../../../src/chain/index.js"; -import {AttestationErrorCode, GossipErrorCode} from "../../../../src/chain/errors/index.js"; +import { + AttestationError, + AttestationErrorCode, + GossipAction, + GossipErrorCode, +} from "../../../../src/chain/errors/index.js"; import { ApiAttestation, GossipAttestation, getStateForAttestationVerification, validateApiAttestation, - validateGossipAttestation, + Phase0Result, + validateAttestation, + validateGossipAttestationsSameAttData, } from "../../../../src/chain/validation/index.js"; import {expectRejectedWithLodestarError} from "../../../utils/errors.js"; import {memoOnce} from "../../../utils/cache.js"; @@ -25,7 +34,120 @@ import {StateRegenerator} from "../../../../src/chain/regen/regen.js"; import {ZERO_HASH_HEX} from "../../../../src/constants/constants.js"; import {QueuedStateRegenerator} from "../../../../src/chain/regen/queued.js"; -describe("chain / validation / attestation", () => { +import {BlsSingleThreadVerifier} from "../../../../src/chain/bls/singleThread.js"; +import {SeenAttesters} from "../../../../src/chain/seenCache/seenAttesters.js"; +import {getAttDataBase64FromAttestationSerialized} from "../../../../src/util/sszBytes.js"; + +describe("validateGossipAttestationsSameAttData", () => { + // phase0Result specifies whether the attestation is valid in phase0 + // phase1Result specifies signature verification + const testCases: {phase0Result: boolean[]; phase1Result: boolean[]; seenAttesters: number[]}[] = [ + { + phase0Result: [true, true, true, true, true], + phase1Result: [true, true, true, true, true], + seenAttesters: [0, 1, 2, 3, 4], + }, + { + phase0Result: [false, true, true, true, true], + phase1Result: [true, false, true, true, true], + seenAttesters: [2, 3, 4], + }, + { + phase0Result: [false, false, true, true, true], + phase1Result: [true, false, false, true, true], + seenAttesters: [3, 4], + }, + { + phase0Result: [false, false, true, true, true], + phase1Result: [true, false, false, true, false], + seenAttesters: [3], + }, + { + phase0Result: [false, false, true, true, true], + phase1Result: [true, true, false, false, false], + seenAttesters: [], + }, + ]; + + type Keypair = {publicKey: PublicKey; secretKey: SecretKey}; + const keypairs = new Map(); + function getKeypair(i: number): Keypair { + let keypair = keypairs.get(i); + if (!keypair) { + const bytes = new Uint8Array(32); + const dataView = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength); + dataView.setUint32(0, i + 1, true); + const secretKey = bls.SecretKey.fromBytes(bytes); + const publicKey = secretKey.toPublicKey(); + keypair = {secretKey, publicKey}; + keypairs.set(i, keypair); + } + return keypair; + } + + let chain: IBeaconChain; + const signingRoot = Buffer.alloc(32, 1); + + beforeEach(() => { + chain = { + bls: new BlsSingleThreadVerifier({metrics: null}), + seenAttesters: new SeenAttesters(), + } as Partial as IBeaconChain; + }); + + for (const [testCaseIndex, testCase] of testCases.entries()) { + const {phase0Result, phase1Result, seenAttesters} = testCase; + it(`test case ${testCaseIndex}`, async () => { + const phase0Results: Promise[] = []; + for (const [i, isValid] of phase0Result.entries()) { + const signatureSet = { + type: SignatureSetType.single, + pubkey: getKeypair(i).publicKey, + signingRoot, + signature: getKeypair(i).secretKey.sign(signingRoot).toBytes(), + }; + if (isValid) { + if (!phase1Result[i]) { + // invalid signature + signatureSet.signature = getKeypair(2023).secretKey.sign(signingRoot).toBytes(); + } + phase0Results.push( + Promise.resolve({ + attestation: ssz.phase0.Attestation.defaultValue(), + signatureSet, + validatorIndex: i, + } as Partial as Phase0Result) + ); + } else { + phase0Results.push( + Promise.reject( + new AttestationError(GossipAction.REJECT, { + code: AttestationErrorCode.BAD_TARGET_EPOCH, + }) + ) + ); + } + } + + let callIndex = 0; + const phase0ValidationFn = (): Promise => { + const result = phase0Results[callIndex]; + callIndex++; + return result; + }; + await validateGossipAttestationsSameAttData(ForkName.phase0, chain, new Array(5).fill({}), 0, phase0ValidationFn); + for (let validatorIndex = 0; validatorIndex < phase0Result.length; validatorIndex++) { + if (seenAttesters.includes(validatorIndex)) { + expect(chain.seenAttesters.isKnown(0, validatorIndex)).to.be.true; + } else { + expect(chain.seenAttesters.isKnown(0, validatorIndex)).to.be.false; + } + } + }); // end test case + } +}); + +describe("validateAttestation", () => { const vc = 64; const stateSlot = 100; @@ -60,7 +182,7 @@ describe("chain / validation / attestation", () => { const {chain, subnet} = getValidData(); await expectGossipError( chain, - {attestation: null, serializedData: Buffer.alloc(0), attSlot: 0}, + {attestation: null, serializedData: Buffer.alloc(0), attSlot: 0, attDataBase64: "invalid"}, subnet, GossipErrorCode.INVALID_SERIALIZED_BYTES_ERROR_CODE ); @@ -76,7 +198,12 @@ describe("chain / validation / attestation", () => { await expectApiError(chain, {attestation, serializedData: null}, AttestationErrorCode.BAD_TARGET_EPOCH); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.BAD_TARGET_EPOCH ); @@ -90,7 +217,12 @@ describe("chain / validation / attestation", () => { await expectApiError(chain, {attestation, serializedData: null}, AttestationErrorCode.PAST_SLOT); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.PAST_SLOT ); @@ -104,7 +236,12 @@ describe("chain / validation / attestation", () => { await expectApiError(chain, {attestation, serializedData: null}, AttestationErrorCode.FUTURE_SLOT); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.FUTURE_SLOT ); @@ -124,7 +261,12 @@ describe("chain / validation / attestation", () => { ); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.NOT_EXACTLY_ONE_AGGREGATION_BIT_SET ); @@ -139,7 +281,12 @@ describe("chain / validation / attestation", () => { await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.NOT_EXACTLY_ONE_AGGREGATION_BIT_SET ); @@ -158,7 +305,12 @@ describe("chain / validation / attestation", () => { ); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT ); @@ -173,7 +325,12 @@ describe("chain / validation / attestation", () => { await expectApiError(chain, {attestation, serializedData: null}, AttestationErrorCode.INVALID_TARGET_ROOT); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.INVALID_TARGET_ROOT ); @@ -197,7 +354,12 @@ describe("chain / validation / attestation", () => { ); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.NO_COMMITTEE_FOR_SLOT_AND_INDEX ); @@ -219,7 +381,12 @@ describe("chain / validation / attestation", () => { ); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.WRONG_NUMBER_OF_AGGREGATION_BITS ); @@ -233,7 +400,12 @@ describe("chain / validation / attestation", () => { await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, invalidSubnet, AttestationErrorCode.INVALID_SUBNET_ID ); @@ -248,7 +420,12 @@ describe("chain / validation / attestation", () => { await expectApiError(chain, {attestation, serializedData: null}, AttestationErrorCode.ATTESTATION_ALREADY_KNOWN); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.ATTESTATION_ALREADY_KNOWN ); @@ -265,7 +442,12 @@ describe("chain / validation / attestation", () => { await expectApiError(chain, {attestation, serializedData: null}, AttestationErrorCode.INVALID_SIGNATURE); await expectGossipError( chain, - {attestation: null, serializedData, attSlot: attestation.data.slot}, + { + attestation: null, + serializedData, + attSlot: attestation.data.slot, + attDataBase64: getAttDataBase64FromAttestationSerialized(serializedData), + }, subnet, AttestationErrorCode.INVALID_SIGNATURE ); @@ -288,10 +470,7 @@ describe("chain / validation / attestation", () => { errorCode: string ): Promise { const fork = chain.config.getForkName(stateSlot); - await expectRejectedWithLodestarError( - validateGossipAttestation(fork, chain, attestationOrBytes, subnet), - errorCode - ); + await expectRejectedWithLodestarError(validateAttestation(fork, chain, attestationOrBytes, subnet), errorCode); } }); diff --git a/packages/beacon-node/test/utils/validationData/attestation.ts b/packages/beacon-node/test/utils/validationData/attestation.ts index 314d3d255b62..6f768227e5cd 100644 --- a/packages/beacon-node/test/utils/validationData/attestation.ts +++ b/packages/beacon-node/test/utils/validationData/attestation.ts @@ -14,12 +14,13 @@ import {IBeaconChain} from "../../../src/chain/index.js"; import {IStateRegenerator} from "../../../src/chain/regen/index.js"; import {ZERO_HASH, ZERO_HASH_HEX} from "../../../src/constants/index.js"; import {SeenAttesters} from "../../../src/chain/seenCache/index.js"; -import {BlsSingleThreadVerifier} from "../../../src/chain/bls/index.js"; +import {BlsMultiThreadWorkerPool, BlsSingleThreadVerifier} from "../../../src/chain/bls/index.js"; import {signCached} from "../cache.js"; import {ClockStatic} from "../clock.js"; import {SeenAggregatedAttestations} from "../../../src/chain/seenCache/seenAggregateAndProof.js"; import {SeenAttestationDatas} from "../../../src/chain/seenCache/seenAttestationData.js"; import {defaultChainOptions} from "../../../src/chain/options.js"; +import {testLogger} from "../logger.js"; export type AttestationValidDataOpts = { currentSlot?: Slot; @@ -28,6 +29,7 @@ export type AttestationValidDataOpts = { bitIndex?: number; targetRoot?: Uint8Array; beaconBlockRoot?: Uint8Array; + blsVerifyAllMainThread?: boolean; state: ReturnType; }; @@ -46,6 +48,7 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { const bitIndex = opts.bitIndex ?? 0; const targetRoot = opts.targetRoot ?? ZERO_HASH; const beaconBlockRoot = opts.beaconBlockRoot ?? ZERO_HASH; + const blsVerifyAllMainThread = opts.blsVerifyAllMainThread ?? true; // Create cached state const state = opts.state; @@ -124,7 +127,9 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { seenAttesters: new SeenAttesters(), seenAggregatedAttestations: new SeenAggregatedAttestations(null), seenAttestationDatas: new SeenAttestationDatas(null, 0, 0), - bls: new BlsSingleThreadVerifier({metrics: null}), + bls: blsVerifyAllMainThread + ? new BlsSingleThreadVerifier({metrics: null}) + : new BlsMultiThreadWorkerPool({}, {logger: testLogger(), metrics: null}), waitForBlock: () => Promise.resolve(false), index2pubkey: state.epochCtx.index2pubkey, opts: defaultChainOptions, diff --git a/packages/state-transition/src/util/signatureSets.ts b/packages/state-transition/src/util/signatureSets.ts index 770f66e7ecd9..3aa3480cba88 100644 --- a/packages/state-transition/src/util/signatureSets.ts +++ b/packages/state-transition/src/util/signatureSets.ts @@ -7,19 +7,21 @@ export enum SignatureSetType { aggregate = "aggregate", } -export type ISignatureSet = - | { - type: SignatureSetType.single; - pubkey: PublicKey; - signingRoot: Root; - signature: Uint8Array; - } - | { - type: SignatureSetType.aggregate; - pubkeys: PublicKey[]; - signingRoot: Root; - signature: Uint8Array; - }; +export type SingleSignatureSet = { + type: SignatureSetType.single; + pubkey: PublicKey; + signingRoot: Root; + signature: Uint8Array; +}; + +export type AggregatedSignatureSet = { + type: SignatureSetType.aggregate; + pubkeys: PublicKey[]; + signingRoot: Root; + signature: Uint8Array; +}; + +export type ISignatureSet = SingleSignatureSet | AggregatedSignatureSet; export function verifySignatureSet(signatureSet: ISignatureSet): boolean { // All signatures are not trusted and must be group checked (p2.subgroup_check) @@ -41,7 +43,7 @@ export function createSingleSignatureSetFromComponents( pubkey: PublicKey, signingRoot: Root, signature: Uint8Array -): ISignatureSet { +): SingleSignatureSet { return { type: SignatureSetType.single, pubkey, @@ -54,7 +56,7 @@ export function createAggregateSignatureSetFromComponents( pubkeys: PublicKey[], signingRoot: Root, signature: Uint8Array -): ISignatureSet { +): AggregatedSignatureSet { return { type: SignatureSetType.aggregate, pubkeys, diff --git a/packages/state-transition/test/perf/util.ts b/packages/state-transition/test/perf/util.ts index 316860fef447..169b205ce5c6 100644 --- a/packages/state-transition/test/perf/util.ts +++ b/packages/state-transition/test/perf/util.ts @@ -424,9 +424,13 @@ export function generateTestCachedBeaconStateOnlyValidators({ throw Error(`Wrong number of validators in the state: ${state.validators.length} !== ${vc}`); } - return createCachedBeaconState(state, { - config: createBeaconConfig(config, state.genesisValidatorsRoot), - pubkey2index, - index2pubkey, - }); + return createCachedBeaconState( + state, + { + config: createBeaconConfig(config, state.genesisValidatorsRoot), + pubkey2index, + index2pubkey, + }, + {skipSyncPubkeys: true} + ); } From f8200b8b107b4e6c5da1be1b5cd2309401205521 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 17 Aug 2023 09:04:46 +0700 Subject: [PATCH 02/16] feat: consume bls verifySignatureSetsSameMessage api --- .../src/chain/validation/attestation.ts | 31 +++++-------------- .../src/metrics/metrics/lodestar.ts | 7 ++--- .../src/network/processor/gossipHandlers.ts | 17 +++++----- .../network/processor/gossipQueues/index.ts | 5 +-- .../perf/chain/validation/attestation.test.ts | 3 +- .../unit/chain/validation/attestation.test.ts | 3 +- 6 files changed, 24 insertions(+), 42 deletions(-) diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index 6e0efa34beeb..290391cf2fa4 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -23,12 +23,10 @@ import {AttestationDataCacheEntry} from "../seenCache/seenAttestationData.js"; import {sszDeserializeAttestation} from "../../network/gossip/topic.js"; import {Result, wrapError} from "../../util/wrapError.js"; import {MIN_SIGNATURE_SETS_TO_BATCH_VERIFY} from "../../network/processor/gossipQueues/index.js"; -import {signatureFromBytesNoCheck} from "../opPools/utils.js"; export type BatchResult = { results: Result[]; batchableBls: boolean; - fallbackBls: boolean; }; export type AttestationValidationResult = { @@ -91,7 +89,7 @@ export async function validateGossipAttestationsSameAttData( phase0ValidationFn = validateGossipAttestationNoSignatureCheck ): Promise { if (attestationOrBytesArr.length === 0) { - return {results: [], batchableBls: false, fallbackBls: false}; + return {results: [], batchableBls: false}; } // phase0: do all verifications except for signature verification @@ -118,26 +116,14 @@ export async function validateGossipAttestationsSameAttData( } let signatureValids: boolean[]; - let batchableBls = false; - let fallbackBls = false; - if (signatureSets.length >= MIN_SIGNATURE_SETS_TO_BATCH_VERIFY) { + const batchableBls = signatureSets.length >= MIN_SIGNATURE_SETS_TO_BATCH_VERIFY; + if (batchableBls) { // all signature sets should have same signing root since we filtered in network processor - const aggregatedPubkey = bls.PublicKey.aggregate(signatureSets.map((set) => set.pubkey)); - const aggregatedSignature = bls.Signature.aggregate( - // no need to check signature, will do a final verify later - signatureSets.map((set) => signatureFromBytesNoCheck(set.signature)) + signatureValids = await chain.bls.verifySignatureSetsSameMessage( + signatureSets.map((set) => ({publicKey: set.pubkey, signature: set.signature})), + signatureSets[0].signingRoot ); - - // quick check, it's likely this is valid most of the time - batchableBls = true; - const isAllValid = aggregatedSignature.verify(aggregatedPubkey, signatureSets[0].signingRoot); - fallbackBls = !isAllValid; - signatureValids = isAllValid - ? new Array(signatureSets.length).fill(true) - : // batchable is false because one of the signature is invalid - await Promise.all(signatureSets.map((set) => chain.bls.verifySignatureSets([set], {batchable: false}))); } else { - batchableBls = false; // don't want to block the main thread if there are too few signatures signatureValids = await Promise.all( signatureSets.map((set) => chain.bls.verifySignatureSets([set], {batchable: true})) @@ -145,7 +131,7 @@ export async function validateGossipAttestationsSameAttData( } // phase0 post validation - for (const [i, isValid] of signatureValids.entries()) { + for (const [i, sigValid] of signatureValids.entries()) { const oldIndex = newIndexToOldIndex.get(i); if (oldIndex == null) { // should not happen @@ -154,7 +140,7 @@ export async function validateGossipAttestationsSameAttData( const {validatorIndex, attestation} = phase0Results[i]; const targetEpoch = attestation.data.target.epoch; - if (isValid) { + if (sigValid) { // Now that the attestation has been fully verified, store that we have received a valid attestation from this validator. // // It's important to double check that the attestation still hasn't been observed, since @@ -184,7 +170,6 @@ export async function validateGossipAttestationsSameAttData( return { results: phase0ResultOrErrors, batchableBls, - fallbackBls, }; } diff --git a/packages/beacon-node/src/metrics/metrics/lodestar.ts b/packages/beacon-node/src/metrics/metrics/lodestar.ts index aae4ae0fe6ba..c8e9c4f6207b 100644 --- a/packages/beacon-node/src/metrics/metrics/lodestar.ts +++ b/packages/beacon-node/src/metrics/metrics/lodestar.ts @@ -575,9 +575,10 @@ export function createLodestarMetrics( labelNames: ["caller"], buckets: [0, 1, 2, 4, 8, 16, 32, 64], }), - attestationBatchCount: register.gauge({ + attestationBatchCount: register.histogram({ name: "lodestar_gossip_attestation_verified_in_batch_count", help: "Count of attestations verified in batch", + buckets: [1, 2, 4, 8, 16, 32, 64, 128], }), attestationNonBatchCount: register.gauge({ name: "lodestar_gossip_attestation_verified_non_batch_count", @@ -587,10 +588,6 @@ export function createLodestarMetrics( name: "lodestar_gossip_attestation_total_batch_count", help: "Total number of attestation batches", }), - totalBatchFallbackBlsCheck: register.gauge({ - name: "lodestar_gossip_attestation_total_batch_fallback_bls_check_count", - help: "Total number of attestation batches that fallback to checking each signature separately", - }), }, // Gossip block diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index c08c9198ff8f..5034bc37f69c 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -353,11 +353,12 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH attSlot: param.gossipData.msgSlot, attDataBase64: param.gossipData.indexed, })) as AttestationOrBytes[]; - const { - results: validationResults, - batchableBls, - fallbackBls, - } = await validateGossipAttestationsSameAttData(fork, chain, validationParams, subnet); + const {results: validationResults, batchableBls} = await validateGossipAttestationsSameAttData( + fork, + chain, + validationParams, + subnet + ); for (const [i, validationResult] of validationResults.entries()) { if (validationResult.err) { results.push(validationResult.err as AttestationError); @@ -394,15 +395,11 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH if (batchableBls) { metrics?.gossipAttestation.totalBatch.inc(); - metrics?.gossipAttestation.attestationBatchCount.inc(gossipHandlerParams.length); + metrics?.gossipAttestation.attestationBatchCount.observe(gossipHandlerParams.length); } else { metrics?.gossipAttestation.attestationNonBatchCount.inc(gossipHandlerParams.length); } - if (fallbackBls) { - metrics?.gossipAttestation.totalBatchFallbackBlsCheck.inc(); - } - return results; }, diff --git a/packages/beacon-node/src/network/processor/gossipQueues/index.ts b/packages/beacon-node/src/network/processor/gossipQueues/index.ts index 37eb354f8bb9..a028981a630c 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/index.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/index.ts @@ -21,8 +21,9 @@ import {IndexedGossipQueueAvgTime} from "./indexedAvgTime.js"; const MAX_GOSSIP_ATTESTATION_BATCH_SIZE = 128; /** - * Batching too few signatures and verifying them on main thread is not worth it, - * we should only batch verify when there are at least 32 signatures. + * Batching signatures have the cost of signature aggregation which blocks the main thread. + * We should only batch verify when there are at least 32 signatures. + * TODO: make this configurable */ export const MIN_SIGNATURE_SETS_TO_BATCH_VERIFY = 32; diff --git a/packages/beacon-node/test/perf/chain/validation/attestation.test.ts b/packages/beacon-node/test/perf/chain/validation/attestation.test.ts index f0a4f7c3e89d..ed5c17ed890b 100644 --- a/packages/beacon-node/test/perf/chain/validation/attestation.test.ts +++ b/packages/beacon-node/test/perf/chain/validation/attestation.test.ts @@ -2,7 +2,8 @@ import {itBench, setBenchOpts} from "@dapplion/benchmark"; import {expect} from "chai"; import {ssz} from "@lodestar/types"; -import {generateTestCachedBeaconStateOnlyValidators} from "@lodestar/state-transition/test/perf/util.js"; +// eslint-disable-next-line import/no-relative-packages +import {generateTestCachedBeaconStateOnlyValidators} from "../../../../../state-transition/test/perf/util.js"; import {validateAttestation, validateGossipAttestationsSameAttData} from "../../../../src/chain/validation/index.js"; import {getAttestationValidData} from "../../../utils/validationData/attestation.js"; import {getAttDataBase64FromAttestationSerialized} from "../../../../src/util/sszBytes.js"; diff --git a/packages/beacon-node/test/unit/chain/validation/attestation.test.ts b/packages/beacon-node/test/unit/chain/validation/attestation.test.ts index 24dc073891e5..3dffb67bb35b 100644 --- a/packages/beacon-node/test/unit/chain/validation/attestation.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/attestation.test.ts @@ -9,7 +9,8 @@ import {ProtoBlock} from "@lodestar/fork-choice"; // eslint-disable-next-line import/no-relative-packages import {SignatureSetType, computeEpochAtSlot, computeStartSlotAtEpoch, processSlots} from "@lodestar/state-transition"; import {Slot, ssz} from "@lodestar/types"; -import {generateTestCachedBeaconStateOnlyValidators} from "@lodestar/state-transition/test/perf/util.js"; +// eslint-disable-next-line import/no-relative-packages +import {generateTestCachedBeaconStateOnlyValidators} from "../../../../../state-transition/test/perf/util.js"; import {IBeaconChain} from "../../../../src/chain/index.js"; import { AttestationError, From dba7af0212793042e33b155b79715331724431c0 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 17 Aug 2023 09:14:04 +0700 Subject: [PATCH 03/16] chore: metrics to count percent of attestations batched --- packages/beacon-node/src/metrics/metrics/lodestar.ts | 8 ++++++-- .../beacon-node/src/network/processor/gossipHandlers.ts | 8 +++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/beacon-node/src/metrics/metrics/lodestar.ts b/packages/beacon-node/src/metrics/metrics/lodestar.ts index c8e9c4f6207b..2fa3c4c2ff8d 100644 --- a/packages/beacon-node/src/metrics/metrics/lodestar.ts +++ b/packages/beacon-node/src/metrics/metrics/lodestar.ts @@ -575,10 +575,14 @@ export function createLodestarMetrics( labelNames: ["caller"], buckets: [0, 1, 2, 4, 8, 16, 32, 64], }), - attestationBatchCount: register.histogram({ + attestationBatchHistogram: register.histogram({ + name: "lodestar_gossip_attestation_verified_in_batch_histogram", + help: "Number of attestations verified in batch", + buckets: [1, 2, 4, 8, 16, 32, 64, 128], + }), + attestationBatchCount: register.gauge({ name: "lodestar_gossip_attestation_verified_in_batch_count", help: "Count of attestations verified in batch", - buckets: [1, 2, 4, 8, 16, 32, 64, 128], }), attestationNonBatchCount: register.gauge({ name: "lodestar_gossip_attestation_verified_non_batch_count", diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index 5034bc37f69c..55b29663a9e9 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -342,7 +342,8 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH gossipHandlerParams: GossipHandlerParamGeneric[] ) => { const results: (null | AttestationError)[] = []; - if (gossipHandlerParams.length === 0) { + const attestationCount = gossipHandlerParams.length; + if (attestationCount === 0) { return results; } // all attestations should have same attestation data as filtered by network processor @@ -395,9 +396,10 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH if (batchableBls) { metrics?.gossipAttestation.totalBatch.inc(); - metrics?.gossipAttestation.attestationBatchCount.observe(gossipHandlerParams.length); + metrics?.gossipAttestation.attestationBatchCount.inc(attestationCount); + metrics?.gossipAttestation.attestationBatchHistogram.observe(gossipHandlerParams.length); } else { - metrics?.gossipAttestation.attestationNonBatchCount.inc(gossipHandlerParams.length); + metrics?.gossipAttestation.attestationNonBatchCount.inc(attestationCount); } return results; From d7050744d8e658b902c8b8502edf1956818ac123 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 17 Aug 2023 09:32:39 +0700 Subject: [PATCH 04/16] feat: add minSameMessageSignatureSetsToBatch flag --- packages/beacon-node/src/chain/options.ts | 5 +++++ .../beacon-node/src/chain/validation/attestation.ts | 2 +- .../src/network/processor/gossipQueues/index.ts | 1 - packages/cli/src/options/beaconNodeOptions/chain.ts | 11 +++++++++++ .../cli/test/unit/options/beaconNodeOptions.test.ts | 2 ++ 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index 93947f105c94..b487b6b43105 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -26,6 +26,7 @@ export type IChainOptions = BlockProcessOpts & /** Option to load a custom kzg trusted setup in txt format */ trustedSetup?: string; broadcastValidationStrictness?: string; + minSameMessageSignatureSetsToBatch: number; }; export type BlockProcessOpts = { @@ -83,4 +84,8 @@ export const defaultChainOptions: IChainOptions = { // for attestation validation, having this value ensures we don't have to regen states most of the time maxSkipSlots: 32, broadcastValidationStrictness: "warn", + // should be less than or equal to MIN_SIGNATURE_SETS_TO_BATCH_VERIFY + // batching too much may block the I/O thread + // this value is conservative, can decrease it if useWorker = true + minSameMessageSignatureSetsToBatch: 32, }; diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index 290391cf2fa4..94b985e835a2 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -116,7 +116,7 @@ export async function validateGossipAttestationsSameAttData( } let signatureValids: boolean[]; - const batchableBls = signatureSets.length >= MIN_SIGNATURE_SETS_TO_BATCH_VERIFY; + const batchableBls = signatureSets.length >= chain.opts.minSameMessageSignatureSetsToBatch; if (batchableBls) { // all signature sets should have same signing root since we filtered in network processor signatureValids = await chain.bls.verifySignatureSetsSameMessage( diff --git a/packages/beacon-node/src/network/processor/gossipQueues/index.ts b/packages/beacon-node/src/network/processor/gossipQueues/index.ts index a028981a630c..7dc7931d2639 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/index.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/index.ts @@ -23,7 +23,6 @@ const MAX_GOSSIP_ATTESTATION_BATCH_SIZE = 128; /** * Batching signatures have the cost of signature aggregation which blocks the main thread. * We should only batch verify when there are at least 32 signatures. - * TODO: make this configurable */ export const MIN_SIGNATURE_SETS_TO_BATCH_VERIFY = 32; diff --git a/packages/cli/src/options/beaconNodeOptions/chain.ts b/packages/cli/src/options/beaconNodeOptions/chain.ts index 9c7e1edc49c6..359b77740b00 100644 --- a/packages/cli/src/options/beaconNodeOptions/chain.ts +++ b/packages/cli/src/options/beaconNodeOptions/chain.ts @@ -23,6 +23,7 @@ export type ChainArgs = { "chain.archiveStateEpochFrequency": number; emitPayloadAttributes?: boolean; broadcastValidationStrictness?: string; + "chain.minSameMessageSignatureSetsToBatch"?: number; }; export function parseArgs(args: ChainArgs): IBeaconNodeOptions["chain"] { @@ -46,6 +47,8 @@ export function parseArgs(args: ChainArgs): IBeaconNodeOptions["chain"] { archiveStateEpochFrequency: args["chain.archiveStateEpochFrequency"], emitPayloadAttributes: args["emitPayloadAttributes"], broadcastValidationStrictness: args["broadcastValidationStrictness"], + minSameMessageSignatureSetsToBatch: + args["chain.minSameMessageSignatureSetsToBatch"] ?? defaultOptions.chain.minSameMessageSignatureSetsToBatch, }; } @@ -182,4 +185,12 @@ Will double processing times. Use only for debugging purposes.", type: "string", default: "warn", }, + + "chain.minSameMessageSignatureSetsToBatch": { + hidden: true, + description: "Minimum number of same message signature sets to batch", + type: "number", + default: defaultOptions.chain.minSameMessageSignatureSetsToBatch, + group: "chain", + }, }; diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index 31f11cf4a79d..94f5bf5a2e9a 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -33,6 +33,7 @@ describe("options / beaconNodeOptions", () => { "safe-slots-to-import-optimistically": 256, "chain.archiveStateEpochFrequency": 1024, "chain.trustedSetup": "", + "chain.minSameMessageSignatureSetsToBatch": 32, emitPayloadAttributes: false, eth1: true, @@ -131,6 +132,7 @@ describe("options / beaconNodeOptions", () => { archiveStateEpochFrequency: 1024, emitPayloadAttributes: false, trustedSetup: "", + minSameMessageSignatureSetsToBatch: 32, }, eth1: { enabled: true, From f1c5043009c218a821eeca4402a0fecb5e949571 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 17 Aug 2023 13:38:44 +0700 Subject: [PATCH 05/16] chore: remove unused metrics --- packages/beacon-node/src/chain/validation/attestation.ts | 2 -- packages/beacon-node/src/metrics/metrics/lodestar.ts | 8 -------- .../beacon-node/src/network/processor/gossipHandlers.ts | 4 +--- .../test/perf/chain/verifyImportBlocks.test.ts | 1 + packages/beacon-node/test/utils/network.ts | 1 + 5 files changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index 94b985e835a2..ec67c8e9b828 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -1,5 +1,4 @@ import {toHexString} from "@chainsafe/ssz"; -import bls from "@chainsafe/bls"; import {phase0, Epoch, Root, Slot, RootHex, ssz} from "@lodestar/types"; import {ProtoBlock} from "@lodestar/fork-choice"; import {ATTESTATION_SUBNET_COUNT, SLOTS_PER_EPOCH, ForkName, ForkSeq} from "@lodestar/params"; @@ -22,7 +21,6 @@ import { import {AttestationDataCacheEntry} from "../seenCache/seenAttestationData.js"; import {sszDeserializeAttestation} from "../../network/gossip/topic.js"; import {Result, wrapError} from "../../util/wrapError.js"; -import {MIN_SIGNATURE_SETS_TO_BATCH_VERIFY} from "../../network/processor/gossipQueues/index.js"; export type BatchResult = { results: Result[]; diff --git a/packages/beacon-node/src/metrics/metrics/lodestar.ts b/packages/beacon-node/src/metrics/metrics/lodestar.ts index 2fa3c4c2ff8d..eed69ba74533 100644 --- a/packages/beacon-node/src/metrics/metrics/lodestar.ts +++ b/packages/beacon-node/src/metrics/metrics/lodestar.ts @@ -580,18 +580,10 @@ export function createLodestarMetrics( help: "Number of attestations verified in batch", buckets: [1, 2, 4, 8, 16, 32, 64, 128], }), - attestationBatchCount: register.gauge({ - name: "lodestar_gossip_attestation_verified_in_batch_count", - help: "Count of attestations verified in batch", - }), attestationNonBatchCount: register.gauge({ name: "lodestar_gossip_attestation_verified_non_batch_count", help: "Count of attestations NOT verified in batch", }), - totalBatch: register.gauge({ - name: "lodestar_gossip_attestation_total_batch_count", - help: "Total number of attestation batches", - }), }, // Gossip block diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index 55b29663a9e9..fe4edd510cf8 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -395,9 +395,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH } if (batchableBls) { - metrics?.gossipAttestation.totalBatch.inc(); - metrics?.gossipAttestation.attestationBatchCount.inc(attestationCount); - metrics?.gossipAttestation.attestationBatchHistogram.observe(gossipHandlerParams.length); + metrics?.gossipAttestation.attestationBatchHistogram.observe(attestationCount); } else { metrics?.gossipAttestation.attestationNonBatchCount.inc(attestationCount); } diff --git a/packages/beacon-node/test/perf/chain/verifyImportBlocks.test.ts b/packages/beacon-node/test/perf/chain/verifyImportBlocks.test.ts index 8db7deb5d3c3..21b70c69a425 100644 --- a/packages/beacon-node/test/perf/chain/verifyImportBlocks.test.ts +++ b/packages/beacon-node/test/perf/chain/verifyImportBlocks.test.ts @@ -88,6 +88,7 @@ describe.skip("verify+import blocks - range sync perf test", () => { suggestedFeeRecipient: defaultValidatorOptions.suggestedFeeRecipient, skipCreateStateCacheIfAvailable: true, archiveStateEpochFrequency: 1024, + minSameMessageSignatureSetsToBatch: 32, }, { config: state.config, diff --git a/packages/beacon-node/test/utils/network.ts b/packages/beacon-node/test/utils/network.ts index 367f49d59ed0..02e8c66879fb 100644 --- a/packages/beacon-node/test/utils/network.ts +++ b/packages/beacon-node/test/utils/network.ts @@ -82,6 +82,7 @@ export async function getNetworkForTest( disableArchiveOnCheckpoint: true, disableLightClientServerOnImportBlockHead: true, disablePrepareNextSlot: true, + minSameMessageSignatureSetsToBatch: 32, }, { config: beaconConfig, From f7400f292122bd1e02a9116f27d22360cc62c00b Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 17 Aug 2023 17:03:08 +0700 Subject: [PATCH 06/16] feat: chain.beaconAttestationBatchValidation cli option --- packages/beacon-node/src/chain/options.ts | 7 +++--- .../network/processor/gossipQueues/index.ts | 25 +++++++++++++++---- .../src/network/processor/index.ts | 6 +++-- .../unit/chain/validation/attestation.test.ts | 3 +++ .../src/options/beaconNodeOptions/chain.ts | 9 +++++++ .../unit/options/beaconNodeOptions.test.ts | 2 ++ 6 files changed, 42 insertions(+), 10 deletions(-) diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index b487b6b43105..c0a18e578ff7 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -26,6 +26,7 @@ export type IChainOptions = BlockProcessOpts & /** Option to load a custom kzg trusted setup in txt format */ trustedSetup?: string; broadcastValidationStrictness?: string; + beaconAttestationBatchValidation?: boolean; minSameMessageSignatureSetsToBatch: number; }; @@ -85,7 +86,7 @@ export const defaultChainOptions: IChainOptions = { maxSkipSlots: 32, broadcastValidationStrictness: "warn", // should be less than or equal to MIN_SIGNATURE_SETS_TO_BATCH_VERIFY - // batching too much may block the I/O thread - // this value is conservative, can decrease it if useWorker = true - minSameMessageSignatureSetsToBatch: 32, + // batching too much may block the I/O thread so if useWorker=false, suggest this value to be 32 + // since this batch attestation work is designed to work with useWorker=true, make this the lowest value + minSameMessageSignatureSetsToBatch: 2, }; diff --git a/packages/beacon-node/src/network/processor/gossipQueues/index.ts b/packages/beacon-node/src/network/processor/gossipQueues/index.ts index 7dc7931d2639..d2e98b18bfe7 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/index.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/index.ts @@ -29,7 +29,7 @@ export const MIN_SIGNATURE_SETS_TO_BATCH_VERIFY = 32; /** * Numbers from https://github.com/sigp/lighthouse/blob/b34a79dc0b02e04441ba01fd0f304d1e203d877d/beacon_node/network/src/beacon_processor/mod.rs#L69 */ -const gossipQueueOpts: { +const defaultGossipQueueOpts: { [K in GossipType]: GossipQueueOpts; } = { // validation gossip block asap @@ -52,9 +52,8 @@ const gossipQueueOpts: { // start with dropping 1% of the queue, then increase 1% more each time. Reset when queue is empty [GossipType.beacon_attestation]: { maxLength: 24576, - indexFn: (item: PendingGossipsubMessage) => getAttDataBase64FromAttestationSerialized(item.msg.data), - minChunkSize: MIN_SIGNATURE_SETS_TO_BATCH_VERIFY, - maxChunkSize: MAX_GOSSIP_ATTESTATION_BATCH_SIZE, + type: QueueType.LIFO, + dropOpts: {type: DropType.ratio, start: 0.01, step: 0.01}, }, [GossipType.voluntary_exit]: {maxLength: 4096, type: QueueType.FIFO, dropOpts: {type: DropType.count, count: 1}}, [GossipType.proposer_slashing]: {maxLength: 4096, type: QueueType.FIFO, dropOpts: {type: DropType.count, count: 1}}, @@ -83,6 +82,17 @@ const gossipQueueOpts: { }, }; +const indexedGossipQueueOpts: { + [K in GossipType]?: GossipQueueOpts; +} = { + [GossipType.beacon_attestation]: { + maxLength: 24576, + indexFn: (item: PendingGossipsubMessage) => getAttDataBase64FromAttestationSerialized(item.msg.data), + minChunkSize: MIN_SIGNATURE_SETS_TO_BATCH_VERIFY, + maxChunkSize: MAX_GOSSIP_ATTESTATION_BATCH_SIZE, + }, +}; + /** * Wraps a GossipValidatorFn with a queue, to limit the processing of gossip objects by type. * @@ -99,7 +109,12 @@ const gossipQueueOpts: { * By topic is too specific, so by type groups all similar objects in the same queue. All in the same won't allow * to customize different queue behaviours per object type (see `gossipQueueOpts`). */ -export function createGossipQueues(): {[K in GossipType]: GossipQueue} { +export function createGossipQueues(beaconAttestationBatchValidation = false): { + [K in GossipType]: GossipQueue; +} { + const gossipQueueOpts = beaconAttestationBatchValidation + ? {...defaultGossipQueueOpts, ...indexedGossipQueueOpts} + : defaultGossipQueueOpts; return mapValues(gossipQueueOpts, (opts) => { if (isIndexedGossipQueueMinSizeOpts(opts)) { return new IndexedGossipQueueMinSize(opts); diff --git a/packages/beacon-node/src/network/processor/index.ts b/packages/beacon-node/src/network/processor/index.ts index 2692d692a22f..35655a5f22aa 100644 --- a/packages/beacon-node/src/network/processor/index.ts +++ b/packages/beacon-node/src/network/processor/index.ts @@ -149,8 +149,8 @@ export class NetworkProcessor { private readonly metrics: Metrics | null; private readonly gossipValidatorFn: GossipValidatorFn; private readonly gossipValidatorBatchFn: GossipValidatorBatchFn; - private readonly gossipQueues = createGossipQueues(); - private readonly gossipTopicConcurrency = mapValues(this.gossipQueues, () => 0); + private readonly gossipQueues: ReturnType; + private readonly gossipTopicConcurrency: {[K in GossipType]: number}; private readonly extractBlockSlotRootFns = createExtractBlockSlotRootFns(); // we may not receive the block for Attestation and SignedAggregateAndProof messages, in that case PendingGossipsubMessage needs // to be stored in this Map and reprocessed once the block comes @@ -169,6 +169,8 @@ export class NetworkProcessor { this.metrics = metrics; this.logger = logger; this.events = events; + this.gossipQueues = createGossipQueues(this.chain.opts.beaconAttestationBatchValidation); + this.gossipTopicConcurrency = mapValues(this.gossipQueues, () => 0); this.gossipValidatorFn = getGossipValidatorFn(modules.gossipHandlers ?? getGossipHandlers(modules, opts), modules); this.gossipValidatorBatchFn = getGossipValidatorBatchFn( modules.gossipHandlers ?? getGossipHandlers(modules, opts), diff --git a/packages/beacon-node/test/unit/chain/validation/attestation.test.ts b/packages/beacon-node/test/unit/chain/validation/attestation.test.ts index 3dffb67bb35b..c348e701d47f 100644 --- a/packages/beacon-node/test/unit/chain/validation/attestation.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/attestation.test.ts @@ -93,6 +93,9 @@ describe("validateGossipAttestationsSameAttData", () => { chain = { bls: new BlsSingleThreadVerifier({metrics: null}), seenAttesters: new SeenAttesters(), + opts: { + minSameMessageSignatureSetsToBatch: 2, + } as IBeaconChain["opts"], } as Partial as IBeaconChain; }); diff --git a/packages/cli/src/options/beaconNodeOptions/chain.ts b/packages/cli/src/options/beaconNodeOptions/chain.ts index 359b77740b00..b526760f4b99 100644 --- a/packages/cli/src/options/beaconNodeOptions/chain.ts +++ b/packages/cli/src/options/beaconNodeOptions/chain.ts @@ -23,6 +23,7 @@ export type ChainArgs = { "chain.archiveStateEpochFrequency": number; emitPayloadAttributes?: boolean; broadcastValidationStrictness?: string; + "chain.beaconAttestationBatchValidation"?: boolean; "chain.minSameMessageSignatureSetsToBatch"?: number; }; @@ -47,6 +48,7 @@ export function parseArgs(args: ChainArgs): IBeaconNodeOptions["chain"] { archiveStateEpochFrequency: args["chain.archiveStateEpochFrequency"], emitPayloadAttributes: args["emitPayloadAttributes"], broadcastValidationStrictness: args["broadcastValidationStrictness"], + beaconAttestationBatchValidation: args["chain.beaconAttestationBatchValidation"], minSameMessageSignatureSetsToBatch: args["chain.minSameMessageSignatureSetsToBatch"] ?? defaultOptions.chain.minSameMessageSignatureSetsToBatch, }; @@ -186,6 +188,13 @@ Will double processing times. Use only for debugging purposes.", default: "warn", }, + "chain.beaconAttestationBatchValidation": { + hidden: true, + description: "Enable beacon attestation batch validation", + type: "boolean", + group: "chain", + }, + "chain.minSameMessageSignatureSetsToBatch": { hidden: true, description: "Minimum number of same message signature sets to batch", diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index 94f5bf5a2e9a..16997d4fbd59 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -33,6 +33,7 @@ describe("options / beaconNodeOptions", () => { "safe-slots-to-import-optimistically": 256, "chain.archiveStateEpochFrequency": 1024, "chain.trustedSetup": "", + "chain.beaconAttestationBatchValidation": true, "chain.minSameMessageSignatureSetsToBatch": 32, emitPayloadAttributes: false, @@ -132,6 +133,7 @@ describe("options / beaconNodeOptions", () => { archiveStateEpochFrequency: 1024, emitPayloadAttributes: false, trustedSetup: "", + beaconAttestationBatchValidation: true, minSameMessageSignatureSetsToBatch: 32, }, eth1: { From b6a44eb421a40333851066e746774f00777edade Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Thu, 17 Aug 2023 17:54:48 +0700 Subject: [PATCH 07/16] fix: create worker with defaultPoolSize - 1 --- packages/beacon-node/src/chain/bls/multithread/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/beacon-node/src/chain/bls/multithread/index.ts b/packages/beacon-node/src/chain/bls/multithread/index.ts index 81990a97a019..56593d2c176b 100644 --- a/packages/beacon-node/src/chain/bls/multithread/index.ts +++ b/packages/beacon-node/src/chain/bls/multithread/index.ts @@ -133,7 +133,8 @@ export class BlsMultiThreadWorkerPool implements IBlsVerifier { // THe worker is not able to deserialize from uncompressed // `Error: err _wrapDeserialize` this.format = implementation === "blst-native" ? PointFormat.uncompressed : PointFormat.compressed; - this.workers = this.createWorkers(implementation, defaultPoolSize); + // 1 worker for the main thread + this.workers = this.createWorkers(implementation, defaultPoolSize - 1); if (metrics) { metrics.blsThreadPool.queueLength.addCollect(() => { From 280f56266bd1e7435c66808b437b609c28b93d69 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 18 Aug 2023 11:20:30 +0700 Subject: [PATCH 08/16] feat: enforce each queue item 50ms old --- .../network/processor/gossipQueues/indexed.ts | 97 +++++++++++++++---- .../processor/gossipQueues/indexed.test.ts | 17 +++- 2 files changed, 94 insertions(+), 20 deletions(-) diff --git a/packages/beacon-node/src/network/processor/gossipQueues/indexed.ts b/packages/beacon-node/src/network/processor/gossipQueues/indexed.ts index c909ada27dad..cf56b9cad52e 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/indexed.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/indexed.ts @@ -3,6 +3,18 @@ import {OrderedSet} from "../../../util/set.js"; import {OrderedMap} from "../../../util/map.js"; import {GossipQueue, IndexedGossipQueueMinSizeOpts} from "./types.js"; +type QueueItem = { + listItems: LinkedList; + firstSeenMs: number; +}; + +/** + * Enforce minimum wait time for each key. On a mainnet node, wait time for beacon_attestation + * is more than 500ms, it's worth to take 1/10 of that to help batch more items. + * This is only needed for key item < minChunkSize. + */ +const MINIMUM_WAIT_TIME_MS = 50; + /** * This implementation tries to get the most items with same key: * - index items by indexFn using a map @@ -18,16 +30,22 @@ import {GossipQueue, IndexedGossipQueueMinSizeOpts} from "./types.js"; */ export class IndexedGossipQueueMinSize implements GossipQueue { private _length = 0; - private indexedItems: OrderedMap>; + // TODO: may switch to regular map if we don't need to get last key + private indexedItems: OrderedMap>; // keys with at least minChunkSize items // we want to process the last key with minChunkSize first, similar to LIFO private minChunkSizeKeys = new OrderedSet(); + // wait time for the next() function to prevent having to search for items >=MINIMUM_WAIT_TIME_MS old repeatedly + // this value is <= MINIMUM_WAIT_TIME_MS + private nextWaitTimeMs: number | null = null; + // the last time we checked for items >=MINIMUM_WAIT_TIME_MS old + private lastWaitTimeCheckedMs = 0; constructor(private readonly opts: IndexedGossipQueueMinSizeOpts) { const {minChunkSize, maxChunkSize} = opts; if (minChunkSize < 0 || maxChunkSize < 0 || minChunkSize > maxChunkSize) { throw Error(`Unexpected min chunk size ${minChunkSize}, max chunk size ${maxChunkSize}}`); } - this.indexedItems = new OrderedMap>(); + this.indexedItems = new OrderedMap>(); } get length(): number { @@ -56,13 +74,13 @@ export class IndexedGossipQueueMinSize implements return 0; } item.indexed = key; - let list = this.indexedItems.get(key); - if (list == null) { - list = new LinkedList(); - this.indexedItems.set(key, list); + let queueItem = this.indexedItems.get(key); + if (queueItem == null) { + queueItem = {firstSeenMs: Date.now(), listItems: new LinkedList()}; + this.indexedItems.set(key, queueItem); } - list.push(item); - if (list.length >= this.opts.minChunkSize) { + queueItem.listItems.push(item); + if (queueItem.listItems.length >= this.opts.minChunkSize) { this.minChunkSizeKeys.add(key); } this._length++; @@ -76,19 +94,19 @@ export class IndexedGossipQueueMinSize implements if (firstKey == null) { return 0; } - const firstList = this.indexedItems.get(firstKey); + const firstQueueItem = this.indexedItems.get(firstKey); // should not happen - if (firstList == null) { + if (firstQueueItem == null) { return 0; } - const deletedItem = firstList.shift(); + const deletedItem = firstQueueItem.listItems.shift(); if (deletedItem != null) { this._length--; - if (firstList.length === 0) { + if (firstQueueItem.listItems.length === 0) { // it's faster to search for deleted item from the head in this case this.indexedItems.delete(firstKey, true); } - if (firstList.length < this.opts.minChunkSize) { + if (firstQueueItem.listItems.length < this.opts.minChunkSize) { // it's faster to search for deleted item from the head in this case this.minChunkSizeKeys.delete(firstKey, true); } @@ -100,24 +118,25 @@ export class IndexedGossipQueueMinSize implements /** * Try to get items of last key with minChunkSize first. - * If not, pick the last key + * If not, pick the last key with MINIMUM_WAIT_TIME_MS old */ next(): T[] | null { let key: string | null = this.minChunkSizeKeys.last(); if (key == null) { - key = this.indexedItems.lastKey(); + key = this.lastMinWaitKey(); } if (key == null) { return null; } - const list = this.indexedItems.get(key); - if (list == null) { + const queueItem = this.indexedItems.get(key); + if (queueItem == null) { // should not happen return null; } + const list = queueItem.listItems; const result: T[] = []; while (list.length > 0 && result.length < this.opts.maxChunkSize) { const t = list.pop(); @@ -142,7 +161,7 @@ export class IndexedGossipQueueMinSize implements getAll(): T[] { const result: T[] = []; for (const key of this.indexedItems.keys()) { - const array = this.indexedItems.get(key)?.toArray(); + const array = this.indexedItems.get(key)?.listItems.toArray(); if (array) { result.push(...array); } @@ -150,4 +169,46 @@ export class IndexedGossipQueueMinSize implements return result; } + + /** + * `indexedItems` is already sorted by key, so we can just iterate through it + * Search for the last key with >= MINIMUM_WAIT_TIME_MS old + * Do not search again if we already searched recently + */ + private lastMinWaitKey(): string | null { + const now = Date.now(); + // searched recently, skip + if (this.nextWaitTimeMs != null && now - this.lastWaitTimeCheckedMs < this.nextWaitTimeMs) { + return null; + } + + this.lastWaitTimeCheckedMs = now; + this.nextWaitTimeMs = null; + let resultedKey: string | null = null; + for (const key of this.indexedItems.keys()) { + const queueItem = this.indexedItems.get(key); + if (queueItem == null) { + // should not happen + continue; + } + if (now - queueItem.firstSeenMs >= MINIMUM_WAIT_TIME_MS) { + // found, do not return to find the last key with >= MINIMUM_WAIT_TIME_MS old + this.nextWaitTimeMs = null; + resultedKey = key; + } else { + // if a key is not at least MINIMUM_WAIT_TIME_MS old, all remaining keys are not either + break; + } + } + + if (resultedKey == null) { + // all items are not old enough, set nextWaitTimeMs to avoid searching again + const firstValue = this.indexedItems.firstValue(); + if (firstValue != null) { + this.nextWaitTimeMs = Math.max(0, MINIMUM_WAIT_TIME_MS - (now - firstValue.firstSeenMs)); + } + } + + return resultedKey; + } } diff --git a/packages/beacon-node/test/unit/network/processor/gossipQueues/indexed.test.ts b/packages/beacon-node/test/unit/network/processor/gossipQueues/indexed.test.ts index 58afa8bcf79d..ff3985e476cd 100644 --- a/packages/beacon-node/test/unit/network/processor/gossipQueues/indexed.test.ts +++ b/packages/beacon-node/test/unit/network/processor/gossipQueues/indexed.test.ts @@ -1,4 +1,5 @@ import {expect} from "chai"; +import sinon from "sinon"; import {IndexedGossipQueueMinSize} from "../../../../../src/network/processor/gossipQueues/indexed.js"; type Item = { @@ -14,7 +15,7 @@ function toIndexedItem(key: string): Item { return {key, indexed: key.substring(0, 1)}; } -describe("IndexedGossipQueues", () => { +describe("IndexedGossipQueueMinSize", () => { const gossipQueue = new IndexedGossipQueueMinSize({ maxLength: 12, indexFn: (item: Item) => item.key.substring(0, 1), @@ -22,6 +23,9 @@ describe("IndexedGossipQueues", () => { maxChunkSize: 3, }); + const sandbox = sinon.createSandbox(); + sandbox.useFakeTimers(); + beforeEach(() => { gossipQueue.clear(); for (const letter of ["a", "b", "c"]) { @@ -31,6 +35,10 @@ describe("IndexedGossipQueues", () => { } }); + afterEach(() => { + sandbox.restore(); + }); + it("should return items with minChunkSize", () => { expect(gossipQueue.next()).to.be.deep.equal(["c3", "c2", "c1"].map(toIndexedItem)); expect(gossipQueue.length).to.be.equal(9); @@ -38,7 +46,12 @@ describe("IndexedGossipQueues", () => { expect(gossipQueue.length).to.be.equal(6); expect(gossipQueue.next()).to.be.deep.equal(["a3", "a2", "a1"].map(toIndexedItem)); expect(gossipQueue.length).to.be.equal(3); - // no more keys with min chunk size, pick the last key + // no more keys with min chunk size but not enough wait time + expect(gossipQueue.next()).to.be.null; + sandbox.clock.tick(20); + expect(gossipQueue.next()).to.be.null; + sandbox.clock.tick(30); + // should pick items of the last key expect(gossipQueue.next()).to.be.deep.equal(["c0"].map(toIndexedItem)); expect(gossipQueue.length).to.be.equal(2); expect(gossipQueue.next()).to.be.deep.equal(["b0"].map(toIndexedItem)); From e862ed763f55fe28dd61603e59336fa045e9d293 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 18 Aug 2023 11:31:51 +0700 Subject: [PATCH 09/16] chore: switch back to regular Map in IndexedGossipQueueMinSize --- .../network/processor/gossipQueues/indexed.ts | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/beacon-node/src/network/processor/gossipQueues/indexed.ts b/packages/beacon-node/src/network/processor/gossipQueues/indexed.ts index cf56b9cad52e..77e52869839c 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/indexed.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/indexed.ts @@ -1,6 +1,5 @@ import {LinkedList} from "../../../util/array.js"; import {OrderedSet} from "../../../util/set.js"; -import {OrderedMap} from "../../../util/map.js"; import {GossipQueue, IndexedGossipQueueMinSizeOpts} from "./types.js"; type QueueItem = { @@ -30,8 +29,7 @@ const MINIMUM_WAIT_TIME_MS = 50; */ export class IndexedGossipQueueMinSize implements GossipQueue { private _length = 0; - // TODO: may switch to regular map if we don't need to get last key - private indexedItems: OrderedMap>; + private indexedItems: Map>; // keys with at least minChunkSize items // we want to process the last key with minChunkSize first, similar to LIFO private minChunkSizeKeys = new OrderedSet(); @@ -45,7 +43,7 @@ export class IndexedGossipQueueMinSize implements if (minChunkSize < 0 || maxChunkSize < 0 || minChunkSize > maxChunkSize) { throw Error(`Unexpected min chunk size ${minChunkSize}, max chunk size ${maxChunkSize}}`); } - this.indexedItems = new OrderedMap>(); + this.indexedItems = new Map>(); } get length(): number { @@ -53,11 +51,11 @@ export class IndexedGossipQueueMinSize implements } get keySize(): number { - return this.indexedItems.size(); + return this.indexedItems.size; } clear(): void { - this.indexedItems = new OrderedMap(); + this.indexedItems = new Map(); this._length = 0; this.minChunkSizeKeys = new OrderedSet(); } @@ -89,7 +87,7 @@ export class IndexedGossipQueueMinSize implements } // overload, need to drop more items - const firstKey = this.indexedItems.firstKey(); + const firstKey = this.indexedItems.keys().next().value as string | undefined; // there should be at least 1 key if (firstKey == null) { return 0; @@ -103,8 +101,7 @@ export class IndexedGossipQueueMinSize implements if (deletedItem != null) { this._length--; if (firstQueueItem.listItems.length === 0) { - // it's faster to search for deleted item from the head in this case - this.indexedItems.delete(firstKey, true); + this.indexedItems.delete(firstKey); } if (firstQueueItem.listItems.length < this.opts.minChunkSize) { // it's faster to search for deleted item from the head in this case @@ -146,8 +143,7 @@ export class IndexedGossipQueueMinSize implements } if (list.length === 0) { - // it's faster to search for deleted item from the tail in this case - this.indexedItems.delete(key, false); + this.indexedItems.delete(key); } if (list.length < this.opts.minChunkSize) { // it's faster to search for deleted item from the tail in this case @@ -203,7 +199,7 @@ export class IndexedGossipQueueMinSize implements if (resultedKey == null) { // all items are not old enough, set nextWaitTimeMs to avoid searching again - const firstValue = this.indexedItems.firstValue(); + const firstValue = this.indexedItems.values().next().value as QueueItem | undefined; if (firstValue != null) { this.nextWaitTimeMs = Math.max(0, MINIMUM_WAIT_TIME_MS - (now - firstValue.firstSeenMs)); } From 36db4a694fb3c24a488a2c0bf2497fbf6b00a1a8 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 18 Aug 2023 14:21:35 +0700 Subject: [PATCH 10/16] fix: network.beaconAttestationBatchValidation flag --- packages/beacon-node/src/chain/options.ts | 1 - .../src/chain/validation/attestation.ts | 17 +- .../src/network/processor/gossipHandlers.ts | 191 ++++++++++++------ .../network/processor/gossipQueues/index.ts | 7 +- .../src/network/processor/index.ts | 2 +- .../src/options/beaconNodeOptions/chain.ts | 9 - .../src/options/beaconNodeOptions/network.ts | 9 + .../unit/options/beaconNodeOptions.test.ts | 4 +- 8 files changed, 148 insertions(+), 92 deletions(-) diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index c0a18e578ff7..9f826d1a2403 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -26,7 +26,6 @@ export type IChainOptions = BlockProcessOpts & /** Option to load a custom kzg trusted setup in txt format */ trustedSetup?: string; broadcastValidationStrictness?: string; - beaconAttestationBatchValidation?: boolean; minSameMessageSignatureSetsToBatch: number; }; diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index ec67c8e9b828..573bfa5c227c 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -9,7 +9,6 @@ import { createSingleSignatureSetFromComponents, SingleSignatureSet, } from "@lodestar/state-transition"; -import {IBeaconChain} from ".."; import {AttestationError, AttestationErrorCode, GossipAction} from "../errors/index.js"; import {MAXIMUM_GOSSIP_CLOCK_DISPARITY_SEC} from "../../constants/index.js"; import {RegenCaller} from "../regen/index.js"; @@ -21,6 +20,7 @@ import { import {AttestationDataCacheEntry} from "../seenCache/seenAttestationData.js"; import {sszDeserializeAttestation} from "../../network/gossip/topic.js"; import {Result, wrapError} from "../../util/wrapError.js"; +import {IBeaconChain} from "../interface.js"; export type BatchResult = { results: Result[]; @@ -59,15 +59,6 @@ export type Phase0Result = AttestationValidationResult & { */ const SHUFFLING_LOOK_AHEAD_EPOCHS = 1; -/** - * Verify gossip attestations of the same attestation data. - * - If there are less than 32 signatures, verify each signature individually with batchable = true - * - If there are not less than 32 signatures - * - do a quick verify by aggregate all signatures and pubkeys, this takes 4.6ms for 32 signatures and 7.6ms for 64 signatures - * - if one of the signature is invalid, do a fallback verify by verify each signature individually with batchable = false - * - subnet is required - * - do not prioritize bls signature set - */ export async function validateGossipAttestation( fork: ForkName, chain: IBeaconChain, @@ -78,6 +69,12 @@ export async function validateGossipAttestation( return validateAttestation(fork, chain, attestationOrBytes, subnet); } +/** + * Verify gossip attestations of the same attestation data. The main advantage is we can batch verify bls signatures + * through verifySignatureSetsSameMessage bls api to improve performance. + * - If there are less than 2 signatures (minSameMessageSignatureSetsToBatch), verify each signature individually with batchable = true + * - do not prioritize bls signature set + */ export async function validateGossipAttestationsSameAttData( fork: ForkName, chain: IBeaconChain, diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index fe4edd510cf8..ee706dbb7bd4 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -6,7 +6,6 @@ import {ForkName, ForkSeq} from "@lodestar/params"; import {routes} from "@lodestar/api"; import {Metrics} from "../../metrics/index.js"; import {OpSource} from "../../metrics/validatorMonitor.js"; -import {IBeaconChain} from "../../chain/index.js"; import { AttestationError, AttestationErrorCode, @@ -29,7 +28,9 @@ import { validateGossipBlsToExecutionChange, AggregateAndProofValidationResult, validateGossipAttestationsSameAttData, + validateGossipAttestation, AttestationOrBytes, + AttestationValidationResult, } from "../../chain/validation/index.js"; import {NetworkEvent, NetworkEventBus} from "../events.js"; import {PeerAction} from "../peers/index.js"; @@ -40,6 +41,7 @@ import {BlockInput, BlockSource, getBlockInput, GossipedInputType} from "../../c import {sszDeserialize} from "../gossip/topic.js"; import {INetworkCore} from "../core/index.js"; import {INetwork} from "../interface.js"; +import {IBeaconChain} from "../../chain/interface.js"; import {AggregatorTracker} from "./aggregatorTracker.js"; /** @@ -48,6 +50,8 @@ import {AggregatorTracker} from "./aggregatorTracker.js"; export type GossipHandlerOpts = { /** By default pass gossip attestations to forkchoice */ dontSendGossipAttestationsToForkchoice?: boolean; + /** By default don't validate gossip attestations in batch */ + beaconAttestationBatchValidation?: boolean; }; export type ValidatorFnsModules = { @@ -242,6 +246,124 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH }); } + async function beaconAttestationBatchHandler( + gossipHandlerParams: GossipHandlerParamGeneric[] + ): Promise<(null | AttestationError)[]> { + const results: (null | AttestationError)[] = []; + const attestationCount = gossipHandlerParams.length; + if (attestationCount === 0) { + return results; + } + // all attestations should have same attestation data as filtered by network processor + const {subnet, fork} = gossipHandlerParams[0].topic; + const validationParams = gossipHandlerParams.map((param) => ({ + attestation: null, + serializedData: param.gossipData.serializedData, + attSlot: param.gossipData.msgSlot, + attDataBase64: param.gossipData.indexed, + })) as AttestationOrBytes[]; + const {results: validationResults, batchableBls} = await validateGossipAttestationsSameAttData( + fork, + chain, + validationParams, + subnet + ); + for (const [i, validationResult] of validationResults.entries()) { + if (validationResult.err) { + results.push(validationResult.err as AttestationError); + continue; + } + + results.push(null); + + // Handler + const {indexedAttestation, attDataRootHex, attestation} = validationResult.result; + metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation); + + try { + // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages + // but don't add to attestation pool, to save CPU and RAM + if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { + const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); + metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); + } + } catch (e) { + logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); + } + + if (!options.dontSendGossipAttestationsToForkchoice) { + try { + chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); + } catch (e) { + logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); + } + } + + chain.emitter.emit(routes.events.EventType.attestation, attestation); + } + + if (batchableBls) { + metrics?.gossipAttestation.attestationBatchHistogram.observe(attestationCount); + } else { + metrics?.gossipAttestation.attestationNonBatchCount.inc(attestationCount); + } + + return results; + } + + async function beaconAttestationHandler({ + gossipData, + topic, + seenTimestampSec, + }: GossipHandlerParamGeneric): Promise { + const {serializedData, msgSlot} = gossipData; + if (msgSlot == undefined) { + throw Error("msgSlot is undefined for beacon_attestation topic"); + } + const {subnet, fork} = topic; + + // do not deserialize gossipSerializedData here, it's done in validateGossipAttestation only if needed + let validationResult: AttestationValidationResult; + try { + validationResult = await validateGossipAttestation( + fork, + chain, + {attestation: null, serializedData, attSlot: msgSlot}, + subnet + ); + } catch (e) { + if (e instanceof AttestationError && e.action === GossipAction.REJECT) { + chain.persistInvalidSszBytes(ssz.phase0.Attestation.typeName, serializedData, "gossip_reject"); + } + throw e; + } + + // Handler + const {indexedAttestation, attDataRootHex, attestation} = validationResult; + metrics?.registerGossipUnaggregatedAttestation(seenTimestampSec, indexedAttestation); + + try { + // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages + // but don't add to attestation pool, to save CPU and RAM + if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { + const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); + metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); + } + } catch (e) { + logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); + } + + if (!options.dontSendGossipAttestationsToForkchoice) { + try { + chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); + } catch (e) { + logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); + } + } + + chain.emitter.emit(routes.events.EventType.attestation, attestation); + } + return { [GossipType.beacon_block]: async ({ gossipData, @@ -338,70 +460,9 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH chain.emitter.emit(routes.events.EventType.attestation, signedAggregateAndProof.message.aggregate); }, - [GossipType.beacon_attestation]: async ( - gossipHandlerParams: GossipHandlerParamGeneric[] - ) => { - const results: (null | AttestationError)[] = []; - const attestationCount = gossipHandlerParams.length; - if (attestationCount === 0) { - return results; - } - // all attestations should have same attestation data as filtered by network processor - const {subnet, fork} = gossipHandlerParams[0].topic; - const validationParams = gossipHandlerParams.map((param) => ({ - attestation: null, - serializedData: param.gossipData.serializedData, - attSlot: param.gossipData.msgSlot, - attDataBase64: param.gossipData.indexed, - })) as AttestationOrBytes[]; - const {results: validationResults, batchableBls} = await validateGossipAttestationsSameAttData( - fork, - chain, - validationParams, - subnet - ); - for (const [i, validationResult] of validationResults.entries()) { - if (validationResult.err) { - results.push(validationResult.err as AttestationError); - continue; - } - - results.push(null); - - // Handler - const {indexedAttestation, attDataRootHex, attestation} = validationResult.result; - metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation); - - try { - // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages - // but don't add to attestation pool, to save CPU and RAM - if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { - const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); - metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); - } - } catch (e) { - logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); - } - - if (!options.dontSendGossipAttestationsToForkchoice) { - try { - chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); - } catch (e) { - logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); - } - } - - chain.emitter.emit(routes.events.EventType.attestation, attestation); - } - - if (batchableBls) { - metrics?.gossipAttestation.attestationBatchHistogram.observe(attestationCount); - } else { - metrics?.gossipAttestation.attestationNonBatchCount.inc(attestationCount); - } - - return results; - }, + [GossipType.beacon_attestation]: options.beaconAttestationBatchValidation + ? beaconAttestationBatchHandler + : beaconAttestationHandler, [GossipType.attester_slashing]: async ({ gossipData, diff --git a/packages/beacon-node/src/network/processor/gossipQueues/index.ts b/packages/beacon-node/src/network/processor/gossipQueues/index.ts index d2e98b18bfe7..c97eb4016ab3 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/index.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/index.ts @@ -18,13 +18,12 @@ import {IndexedGossipQueueAvgTime} from "./indexedAvgTime.js"; * In normal condition, the higher this value the more efficient the signature verification. * However, if at least 1 signature is invalid, we need to verify each signature separately. */ -const MAX_GOSSIP_ATTESTATION_BATCH_SIZE = 128; +const MAX_GOSSIP_ATTESTATION_BATCH_SIZE = 64; /** - * Batching signatures have the cost of signature aggregation which blocks the main thread. - * We should only batch verify when there are at least 32 signatures. + * Minimum signature sets to batch verify without waiting for 50ms. */ -export const MIN_SIGNATURE_SETS_TO_BATCH_VERIFY = 32; +export const MIN_SIGNATURE_SETS_TO_BATCH_VERIFY = 16; /** * Numbers from https://github.com/sigp/lighthouse/blob/b34a79dc0b02e04441ba01fd0f304d1e203d877d/beacon_node/network/src/beacon_processor/mod.rs#L69 diff --git a/packages/beacon-node/src/network/processor/index.ts b/packages/beacon-node/src/network/processor/index.ts index 35655a5f22aa..ee3dc4616c48 100644 --- a/packages/beacon-node/src/network/processor/index.ts +++ b/packages/beacon-node/src/network/processor/index.ts @@ -169,7 +169,7 @@ export class NetworkProcessor { this.metrics = metrics; this.logger = logger; this.events = events; - this.gossipQueues = createGossipQueues(this.chain.opts.beaconAttestationBatchValidation); + this.gossipQueues = createGossipQueues(this.opts.beaconAttestationBatchValidation); this.gossipTopicConcurrency = mapValues(this.gossipQueues, () => 0); this.gossipValidatorFn = getGossipValidatorFn(modules.gossipHandlers ?? getGossipHandlers(modules, opts), modules); this.gossipValidatorBatchFn = getGossipValidatorBatchFn( diff --git a/packages/cli/src/options/beaconNodeOptions/chain.ts b/packages/cli/src/options/beaconNodeOptions/chain.ts index b526760f4b99..359b77740b00 100644 --- a/packages/cli/src/options/beaconNodeOptions/chain.ts +++ b/packages/cli/src/options/beaconNodeOptions/chain.ts @@ -23,7 +23,6 @@ export type ChainArgs = { "chain.archiveStateEpochFrequency": number; emitPayloadAttributes?: boolean; broadcastValidationStrictness?: string; - "chain.beaconAttestationBatchValidation"?: boolean; "chain.minSameMessageSignatureSetsToBatch"?: number; }; @@ -48,7 +47,6 @@ export function parseArgs(args: ChainArgs): IBeaconNodeOptions["chain"] { archiveStateEpochFrequency: args["chain.archiveStateEpochFrequency"], emitPayloadAttributes: args["emitPayloadAttributes"], broadcastValidationStrictness: args["broadcastValidationStrictness"], - beaconAttestationBatchValidation: args["chain.beaconAttestationBatchValidation"], minSameMessageSignatureSetsToBatch: args["chain.minSameMessageSignatureSetsToBatch"] ?? defaultOptions.chain.minSameMessageSignatureSetsToBatch, }; @@ -188,13 +186,6 @@ Will double processing times. Use only for debugging purposes.", default: "warn", }, - "chain.beaconAttestationBatchValidation": { - hidden: true, - description: "Enable beacon attestation batch validation", - type: "boolean", - group: "chain", - }, - "chain.minSameMessageSignatureSetsToBatch": { hidden: true, description: "Minimum number of same message signature sets to batch", diff --git a/packages/cli/src/options/beaconNodeOptions/network.ts b/packages/cli/src/options/beaconNodeOptions/network.ts index 06a46cd849e2..ed2e0a6a429c 100644 --- a/packages/cli/src/options/beaconNodeOptions/network.ts +++ b/packages/cli/src/options/beaconNodeOptions/network.ts @@ -26,6 +26,7 @@ export type NetworkArgs = { "network.connectToDiscv5Bootnodes"?: boolean; "network.discv5FirstQueryDelayMs"?: number; "network.dontSendGossipAttestationsToForkchoice"?: boolean; + "network.beaconAttestationBatchValidation"?: boolean; "network.allowPublishToZeroPeers"?: boolean; "network.gossipsubD"?: number; "network.gossipsubDLow"?: number; @@ -142,6 +143,7 @@ export function parseArgs(args: NetworkArgs): IBeaconNodeOptions["network"] { connectToDiscv5Bootnodes: args["network.connectToDiscv5Bootnodes"], discv5FirstQueryDelayMs: args["network.discv5FirstQueryDelayMs"], dontSendGossipAttestationsToForkchoice: args["network.dontSendGossipAttestationsToForkchoice"], + beaconAttestationBatchValidation: args["network.beaconAttestationBatchValidation"], allowPublishToZeroPeers: args["network.allowPublishToZeroPeers"], gossipsubD: args["network.gossipsubD"], gossipsubDLow: args["network.gossipsubDLow"], @@ -323,6 +325,13 @@ export const options: CliCommandOptions = { group: "network", }, + "network.beaconAttestationBatchValidation": { + hidden: true, + type: "boolean", + description: "Validate gossip attestations in batches", + group: "network", + }, + "network.allowPublishToZeroPeers": { hidden: true, type: "boolean", diff --git a/packages/cli/test/unit/options/beaconNodeOptions.test.ts b/packages/cli/test/unit/options/beaconNodeOptions.test.ts index 16997d4fbd59..442b4c529d9b 100644 --- a/packages/cli/test/unit/options/beaconNodeOptions.test.ts +++ b/packages/cli/test/unit/options/beaconNodeOptions.test.ts @@ -33,7 +33,6 @@ describe("options / beaconNodeOptions", () => { "safe-slots-to-import-optimistically": 256, "chain.archiveStateEpochFrequency": 1024, "chain.trustedSetup": "", - "chain.beaconAttestationBatchValidation": true, "chain.minSameMessageSignatureSetsToBatch": 32, emitPayloadAttributes: false, @@ -88,6 +87,7 @@ describe("options / beaconNodeOptions", () => { "network.blockCountPeerLimit": 500, "network.rateTrackerTimeoutMs": 60000, "network.dontSendGossipAttestationsToForkchoice": true, + "network.beaconAttestationBatchValidation": true, "network.allowPublishToZeroPeers": true, "network.gossipsubD": 4, "network.gossipsubDLow": 2, @@ -133,7 +133,6 @@ describe("options / beaconNodeOptions", () => { archiveStateEpochFrequency: 1024, emitPayloadAttributes: false, trustedSetup: "", - beaconAttestationBatchValidation: true, minSameMessageSignatureSetsToBatch: 32, }, eth1: { @@ -190,6 +189,7 @@ describe("options / beaconNodeOptions", () => { connectToDiscv5Bootnodes: true, discv5FirstQueryDelayMs: 1000, dontSendGossipAttestationsToForkchoice: true, + beaconAttestationBatchValidation: true, allowPublishToZeroPeers: true, gossipsubD: 4, gossipsubDLow: 2, From d6cf1cdb44fdb62e5ab4a1c48dba1d7a8c9b2b72 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 18 Aug 2023 16:17:43 +0700 Subject: [PATCH 11/16] fix: useFakeTimer in beforeEach --- .../test/unit/network/processor/gossipQueues/indexed.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/beacon-node/test/unit/network/processor/gossipQueues/indexed.test.ts b/packages/beacon-node/test/unit/network/processor/gossipQueues/indexed.test.ts index ff3985e476cd..feee05172379 100644 --- a/packages/beacon-node/test/unit/network/processor/gossipQueues/indexed.test.ts +++ b/packages/beacon-node/test/unit/network/processor/gossipQueues/indexed.test.ts @@ -24,9 +24,9 @@ describe("IndexedGossipQueueMinSize", () => { }); const sandbox = sinon.createSandbox(); - sandbox.useFakeTimers(); beforeEach(() => { + sandbox.useFakeTimers(); gossipQueue.clear(); for (const letter of ["a", "b", "c"]) { for (let i = 0; i < 4; i++) { From 37cfb6e77c35f27cc7b170bc95c490ee80fa7e85 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Fri, 18 Aug 2023 19:36:50 +0700 Subject: [PATCH 12/16] fix: address attDataBase64 for both linear & indexed queues --- .../src/chain/errors/attestationError.ts | 5 +---- .../beacon-node/src/chain/validation/attestation.ts | 13 ++++++------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/beacon-node/src/chain/errors/attestationError.ts b/packages/beacon-node/src/chain/errors/attestationError.ts index 913827d4ca38..a93f5b42e439 100644 --- a/packages/beacon-node/src/chain/errors/attestationError.ts +++ b/packages/beacon-node/src/chain/errors/attestationError.ts @@ -132,8 +132,6 @@ export enum AttestationErrorCode { INVALID_SERIALIZED_BYTES = "ATTESTATION_ERROR_INVALID_SERIALIZED_BYTES", /** Too many skipped slots. */ TOO_MANY_SKIPPED_SLOTS = "ATTESTATION_ERROR_TOO_MANY_SKIPPED_SLOTS", - /** attDataBase64 is not available */ - NO_INDEXED_DATA = "ATTESTATION_ERROR_NO_INDEXED_DATA", } export type AttestationErrorType = @@ -168,8 +166,7 @@ export type AttestationErrorType = | {code: AttestationErrorCode.INVALID_AGGREGATOR} | {code: AttestationErrorCode.INVALID_INDEXED_ATTESTATION} | {code: AttestationErrorCode.INVALID_SERIALIZED_BYTES} - | {code: AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS; headBlockSlot: Slot; attestationSlot: Slot} - | {code: AttestationErrorCode.NO_INDEXED_DATA}; + | {code: AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS; headBlockSlot: Slot; attestationSlot: Slot}; export class AttestationError extends GossipActionError { getMetadata(): Record { diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index 573bfa5c227c..e47c795f0243 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -15,6 +15,7 @@ import {RegenCaller} from "../regen/index.js"; import { AttDataBase64, getAggregationBitsFromAttestationSerialized, + getAttDataBase64FromAttestationSerialized, getSignatureFromAttestationSerialized, } from "../../util/sszBytes.js"; import {AttestationDataCacheEntry} from "../seenCache/seenAttestationData.js"; @@ -233,16 +234,14 @@ async function validateGossipAttestationNoSignatureCheck( let attestationOrCache: | {attestation: phase0.Attestation; cache: null} | {attestation: null; cache: AttestationDataCacheEntry; serializedData: Uint8Array}; - let attDataBase64: AttDataBase64 | null; + let attDataBase64: AttDataBase64 | null = null; if (attestationOrBytes.serializedData) { // gossip const attSlot = attestationOrBytes.attSlot; - if (!attestationOrBytes.attDataBase64) { - throw new AttestationError(GossipAction.IGNORE, { - code: AttestationErrorCode.NO_INDEXED_DATA, - }); - } - attDataBase64 = attestationOrBytes.attDataBase64; + // for old LIFO linear gossip queue we don't have attDataBase64 + // for indexed gossip queue we have attDataBase64 + attDataBase64 = + attestationOrBytes.attDataBase64 ?? getAttDataBase64FromAttestationSerialized(attestationOrBytes.serializedData); const cachedAttData = attDataBase64 !== null ? chain.seenAttestationDatas.get(attSlot, attDataBase64) : null; if (cachedAttData === null) { const attestation = sszDeserializeAttestation(attestationOrBytes.serializedData); From 11169432cc87490e4ab5b84bc9e4210d2d50debc Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 19 Aug 2023 08:18:50 +0700 Subject: [PATCH 13/16] refactor: add getBatchHandlers --- .../src/network/gossip/interface.ts | 1 + .../src/network/processor/gossipHandlers.ts | 259 ++++++++++-------- 2 files changed, 139 insertions(+), 121 deletions(-) diff --git a/packages/beacon-node/src/network/gossip/interface.ts b/packages/beacon-node/src/network/gossip/interface.ts index 825178532ee8..f61008b50570 100644 --- a/packages/beacon-node/src/network/gossip/interface.ts +++ b/packages/beacon-node/src/network/gossip/interface.ts @@ -171,6 +171,7 @@ export type GossipHandlerParamGeneric = { export type GossipHandlers = { [K in GossipType]: | ((gossipHandlerParam: GossipHandlerParamGeneric) => Promise) + // TODO: make it generic | ((gossipHandlerParams: GossipHandlerParamGeneric[]) => Promise<(null | AttestationError)[]>); }; diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index ee706dbb7bd4..5e476c9ad413 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -81,6 +81,19 @@ const MAX_UNKNOWN_BLOCK_ROOT_RETRIES = 1; * - Ethereum Consensus gossipsub protocol strictly defined a single topic for message */ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipHandlerOpts): GossipHandlers { + const defaultHandlers = getDefaultHandlers(modules, options); + if (options.beaconAttestationBatchValidation) { + const batchHandlers = getBatchHandlers(modules, options); + return {...defaultHandlers, ...batchHandlers}; + } + return defaultHandlers; +} + +/** + * Default handlers validate gossip messages one by one. + * We only have a choice to do batch validation for beacon_attestation topic. + */ +function getDefaultHandlers(modules: ValidatorFnsModules, options: GossipHandlerOpts): GossipHandlers { const {chain, config, metrics, events, logger, core, aggregatorTracker} = modules; async function validateBeaconBlock( @@ -246,124 +259,6 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH }); } - async function beaconAttestationBatchHandler( - gossipHandlerParams: GossipHandlerParamGeneric[] - ): Promise<(null | AttestationError)[]> { - const results: (null | AttestationError)[] = []; - const attestationCount = gossipHandlerParams.length; - if (attestationCount === 0) { - return results; - } - // all attestations should have same attestation data as filtered by network processor - const {subnet, fork} = gossipHandlerParams[0].topic; - const validationParams = gossipHandlerParams.map((param) => ({ - attestation: null, - serializedData: param.gossipData.serializedData, - attSlot: param.gossipData.msgSlot, - attDataBase64: param.gossipData.indexed, - })) as AttestationOrBytes[]; - const {results: validationResults, batchableBls} = await validateGossipAttestationsSameAttData( - fork, - chain, - validationParams, - subnet - ); - for (const [i, validationResult] of validationResults.entries()) { - if (validationResult.err) { - results.push(validationResult.err as AttestationError); - continue; - } - - results.push(null); - - // Handler - const {indexedAttestation, attDataRootHex, attestation} = validationResult.result; - metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation); - - try { - // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages - // but don't add to attestation pool, to save CPU and RAM - if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { - const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); - metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); - } - } catch (e) { - logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); - } - - if (!options.dontSendGossipAttestationsToForkchoice) { - try { - chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); - } catch (e) { - logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); - } - } - - chain.emitter.emit(routes.events.EventType.attestation, attestation); - } - - if (batchableBls) { - metrics?.gossipAttestation.attestationBatchHistogram.observe(attestationCount); - } else { - metrics?.gossipAttestation.attestationNonBatchCount.inc(attestationCount); - } - - return results; - } - - async function beaconAttestationHandler({ - gossipData, - topic, - seenTimestampSec, - }: GossipHandlerParamGeneric): Promise { - const {serializedData, msgSlot} = gossipData; - if (msgSlot == undefined) { - throw Error("msgSlot is undefined for beacon_attestation topic"); - } - const {subnet, fork} = topic; - - // do not deserialize gossipSerializedData here, it's done in validateGossipAttestation only if needed - let validationResult: AttestationValidationResult; - try { - validationResult = await validateGossipAttestation( - fork, - chain, - {attestation: null, serializedData, attSlot: msgSlot}, - subnet - ); - } catch (e) { - if (e instanceof AttestationError && e.action === GossipAction.REJECT) { - chain.persistInvalidSszBytes(ssz.phase0.Attestation.typeName, serializedData, "gossip_reject"); - } - throw e; - } - - // Handler - const {indexedAttestation, attDataRootHex, attestation} = validationResult; - metrics?.registerGossipUnaggregatedAttestation(seenTimestampSec, indexedAttestation); - - try { - // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages - // but don't add to attestation pool, to save CPU and RAM - if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { - const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); - metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); - } - } catch (e) { - logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); - } - - if (!options.dontSendGossipAttestationsToForkchoice) { - try { - chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); - } catch (e) { - logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); - } - } - - chain.emitter.emit(routes.events.EventType.attestation, attestation); - } - return { [GossipType.beacon_block]: async ({ gossipData, @@ -460,9 +355,58 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH chain.emitter.emit(routes.events.EventType.attestation, signedAggregateAndProof.message.aggregate); }, - [GossipType.beacon_attestation]: options.beaconAttestationBatchValidation - ? beaconAttestationBatchHandler - : beaconAttestationHandler, + [GossipType.beacon_attestation]: async ({ + gossipData, + topic, + seenTimestampSec, + }: GossipHandlerParamGeneric): Promise => { + const {serializedData, msgSlot} = gossipData; + if (msgSlot == undefined) { + throw Error("msgSlot is undefined for beacon_attestation topic"); + } + const {subnet, fork} = topic; + + // do not deserialize gossipSerializedData here, it's done in validateGossipAttestation only if needed + let validationResult: AttestationValidationResult; + try { + validationResult = await validateGossipAttestation( + fork, + chain, + {attestation: null, serializedData, attSlot: msgSlot}, + subnet + ); + } catch (e) { + if (e instanceof AttestationError && e.action === GossipAction.REJECT) { + chain.persistInvalidSszBytes(ssz.phase0.Attestation.typeName, serializedData, "gossip_reject"); + } + throw e; + } + + // Handler + const {indexedAttestation, attDataRootHex, attestation} = validationResult; + metrics?.registerGossipUnaggregatedAttestation(seenTimestampSec, indexedAttestation); + + try { + // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages + // but don't add to attestation pool, to save CPU and RAM + if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { + const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); + metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); + } + } catch (e) { + logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); + } + + if (!options.dontSendGossipAttestationsToForkchoice) { + try { + chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); + } catch (e) { + logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); + } + } + + chain.emitter.emit(routes.events.EventType.attestation, attestation); + }, [GossipType.attester_slashing]: async ({ gossipData, @@ -606,6 +550,79 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH }; } +/** + * For now, only beacon_attestation topic is batched. + */ +function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOpts): Partial { + const {chain, metrics, logger, aggregatorTracker} = modules; + return { + [GossipType.beacon_attestation]: async ( + gossipHandlerParams: GossipHandlerParamGeneric[] + ): Promise<(null | AttestationError)[]> => { + const results: (null | AttestationError)[] = []; + const attestationCount = gossipHandlerParams.length; + if (attestationCount === 0) { + return results; + } + // all attestations should have same attestation data as filtered by network processor + const {subnet, fork} = gossipHandlerParams[0].topic; + const validationParams = gossipHandlerParams.map((param) => ({ + attestation: null, + serializedData: param.gossipData.serializedData, + attSlot: param.gossipData.msgSlot, + attDataBase64: param.gossipData.indexed, + })) as AttestationOrBytes[]; + const {results: validationResults, batchableBls} = await validateGossipAttestationsSameAttData( + fork, + chain, + validationParams, + subnet + ); + for (const [i, validationResult] of validationResults.entries()) { + if (validationResult.err) { + results.push(validationResult.err as AttestationError); + continue; + } + + results.push(null); + + // Handler + const {indexedAttestation, attDataRootHex, attestation} = validationResult.result; + metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation); + + try { + // Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages + // but don't add to attestation pool, to save CPU and RAM + if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) { + const insertOutcome = chain.attestationPool.add(attestation, attDataRootHex); + metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome}); + } + } catch (e) { + logger.error("Error adding unaggregated attestation to pool", {subnet}, e as Error); + } + + if (!options.dontSendGossipAttestationsToForkchoice) { + try { + chain.forkChoice.onAttestation(indexedAttestation, attDataRootHex); + } catch (e) { + logger.debug("Error adding gossip unaggregated attestation to forkchoice", {subnet}, e as Error); + } + } + + chain.emitter.emit(routes.events.EventType.attestation, attestation); + } + + if (batchableBls) { + metrics?.gossipAttestation.attestationBatchHistogram.observe(attestationCount); + } else { + metrics?.gossipAttestation.attestationNonBatchCount.inc(attestationCount); + } + + return results; + }, + }; +} + /** * Retry a function if it throws error code UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT */ From b7c4505ee50b0f733a4acf0e089a3053f58eb496 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 19 Aug 2023 17:07:42 +0700 Subject: [PATCH 14/16] fix: block event handler in the next event loop --- .../beacon-node/src/chain/blocks/importBlock.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index 63cabc225290..55d798df8e3b 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -89,11 +89,14 @@ export async function importBlock( this.metrics?.importBlock.bySource.inc({source}); this.logger.verbose("Added block to forkchoice and state cache", {slot: block.message.slot, root: blockRootHex}); - this.emitter.emit(routes.events.EventType.block, { - block: toHexString(this.config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message)), - slot: block.message.slot, - executionOptimistic: blockSummary != null && isOptimisticBlock(blockSummary), - }); + // We want to import block asap so call all event handler in the next event loop + setTimeout(() => { + this.emitter.emit(routes.events.EventType.block, { + block: toHexString(this.config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message)), + slot: block.message.slot, + executionOptimistic: blockSummary != null && isOptimisticBlock(blockSummary), + }); + }, 0); // 3. Import attestations to fork choice // From 62d49f68bd87ef4c7e4d1907a96b1a9651253261 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sat, 19 Aug 2023 20:07:18 +0700 Subject: [PATCH 15/16] chore: refactor GossipHandlers type --- .../src/network/gossip/interface.ts | 24 +++++++++++++++---- .../src/network/processor/gossipHandlers.ts | 12 +++++++--- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/packages/beacon-node/src/network/gossip/interface.ts b/packages/beacon-node/src/network/gossip/interface.ts index f61008b50570..8e9013487a06 100644 --- a/packages/beacon-node/src/network/gossip/interface.ts +++ b/packages/beacon-node/src/network/gossip/interface.ts @@ -7,7 +7,8 @@ import {BeaconConfig} from "@lodestar/config"; import {Logger} from "@lodestar/utils"; import {IBeaconChain} from "../../chain/index.js"; import {JobItemQueue} from "../../util/queue/index.js"; -import {AttestationError} from "../../chain/errors/attestationError.js"; +import {AttestationError, AttestationErrorType} from "../../chain/errors/attestationError.js"; +import {GossipActionError} from "../../chain/errors/gossipValidation.js"; export enum GossipType { beacon_block = "beacon_block", @@ -169,10 +170,23 @@ export type GossipHandlerParamGeneric = { }; export type GossipHandlers = { - [K in GossipType]: - | ((gossipHandlerParam: GossipHandlerParamGeneric) => Promise) - // TODO: make it generic - | ((gossipHandlerParams: GossipHandlerParamGeneric[]) => Promise<(null | AttestationError)[]>); + [K in GossipType]: DefaultGossipHandler | BatchGossipHandler; +}; + +export type DefaultGossipHandler = ( + gossipHandlerParam: GossipHandlerParamGeneric +) => Promise; + +export type DefaultGossipHandlers = { + [K in GossipType]: DefaultGossipHandler; +}; + +export type BatchGossipHandler = ( + gossipHandlerParams: GossipHandlerParamGeneric[] +) => Promise<(null | GossipActionError)[]>; + +export type BatchGossipHandlers = { + [K in GossipType]?: BatchGossipHandler; }; // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index 5e476c9ad413..0600aa7220d4 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -16,7 +16,13 @@ import { GossipActionError, SyncCommitteeError, } from "../../chain/errors/index.js"; -import {GossipHandlerParamGeneric, GossipHandlers, GossipType} from "../gossip/interface.js"; +import { + BatchGossipHandlers, + DefaultGossipHandlers, + GossipHandlerParamGeneric, + GossipHandlers, + GossipType, +} from "../gossip/interface.js"; import { validateGossipAggregateAndProof, validateGossipAttesterSlashing, @@ -93,7 +99,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH * Default handlers validate gossip messages one by one. * We only have a choice to do batch validation for beacon_attestation topic. */ -function getDefaultHandlers(modules: ValidatorFnsModules, options: GossipHandlerOpts): GossipHandlers { +function getDefaultHandlers(modules: ValidatorFnsModules, options: GossipHandlerOpts): DefaultGossipHandlers { const {chain, config, metrics, events, logger, core, aggregatorTracker} = modules; async function validateBeaconBlock( @@ -553,7 +559,7 @@ function getDefaultHandlers(modules: ValidatorFnsModules, options: GossipHandler /** * For now, only beacon_attestation topic is batched. */ -function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOpts): Partial { +function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOpts): Partial { const {chain, metrics, logger, aggregatorTracker} = modules; return { [GossipType.beacon_attestation]: async ( From e0fe66afdb451600ab4514eafdbb779bc3f8ddf3 Mon Sep 17 00:00:00 2001 From: Tuyen Nguyen Date: Sun, 20 Aug 2023 07:00:52 +0700 Subject: [PATCH 16/16] chore: set min and max batch to 32 & 128 --- .../beacon-node/src/network/processor/gossipQueues/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/src/network/processor/gossipQueues/index.ts b/packages/beacon-node/src/network/processor/gossipQueues/index.ts index c97eb4016ab3..366b23b30679 100644 --- a/packages/beacon-node/src/network/processor/gossipQueues/index.ts +++ b/packages/beacon-node/src/network/processor/gossipQueues/index.ts @@ -18,12 +18,12 @@ import {IndexedGossipQueueAvgTime} from "./indexedAvgTime.js"; * In normal condition, the higher this value the more efficient the signature verification. * However, if at least 1 signature is invalid, we need to verify each signature separately. */ -const MAX_GOSSIP_ATTESTATION_BATCH_SIZE = 64; +const MAX_GOSSIP_ATTESTATION_BATCH_SIZE = 128; /** * Minimum signature sets to batch verify without waiting for 50ms. */ -export const MIN_SIGNATURE_SETS_TO_BATCH_VERIFY = 16; +export const MIN_SIGNATURE_SETS_TO_BATCH_VERIFY = 32; /** * Numbers from https://github.com/sigp/lighthouse/blob/b34a79dc0b02e04441ba01fd0f304d1e203d877d/beacon_node/network/src/beacon_processor/mod.rs#L69