From 77b839ee215c5bf4e31463de479ca17998ef07f3 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Sun, 5 May 2024 11:03:29 +0800 Subject: [PATCH 1/3] Process attestations in block --- .../src/chain/blocks/importBlock.ts | 3 +- .../src/block/processAttestationPhase0.ts | 60 +++++++++--- .../src/block/processAttestations.ts | 4 +- .../src/block/processAttestationsAltair.ts | 7 +- .../state-transition/src/cache/epochCache.ts | 92 ++++++++++++++++--- .../src/signatureSets/indexedAttestation.ts | 5 +- 6 files changed, 136 insertions(+), 35 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index 5b3d69a0e47e..be6936af37ce 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -68,6 +68,7 @@ export async function importBlock( const prevFinalizedEpoch = this.forkChoice.getFinalizedCheckpoint().epoch; const blockDelaySec = (fullyVerifiedBlock.seenTimestampSec - postState.genesisTime) % this.config.SECONDS_PER_SLOT; const recvToValLatency = Date.now() / 1000 - (opts.seenTimestampSec ?? Date.now() / 1000); + const fork = this.config.getForkSeq(blockSlot); // this is just a type assertion since blockinput with blobsPromise type will not end up here if (blockInput.type === BlockInputType.blobsPromise) { @@ -146,7 +147,7 @@ export async function importBlock( for (const attestation of attestations) { try { - const indexedAttestation = postState.epochCtx.getIndexedAttestation(attestation); + const indexedAttestation = postState.epochCtx.getIndexedAttestation(fork, attestation); const {target, beaconBlockRoot} = attestation.data; const attDataRoot = toHexString(ssz.phase0.AttestationData.hashTreeRoot(indexedAttestation.data)); diff --git a/packages/state-transition/src/block/processAttestationPhase0.ts b/packages/state-transition/src/block/processAttestationPhase0.ts index 248ba83b4ed2..8b1890240038 100644 --- a/packages/state-transition/src/block/processAttestationPhase0.ts +++ b/packages/state-transition/src/block/processAttestationPhase0.ts @@ -1,5 +1,5 @@ import {toHexString} from "@chainsafe/ssz"; -import {Slot, phase0, ssz} from "@lodestar/types"; +import {Slot, allForks, electra, phase0, ssz} from "@lodestar/types"; import {MIN_ATTESTATION_INCLUSION_DELAY, SLOTS_PER_EPOCH, ForkSeq} from "@lodestar/params"; import {computeEpochAtSlot} from "../util/index.js"; @@ -51,7 +51,7 @@ export function processAttestationPhase0( state.previousEpochAttestations.push(pendingAttestation); } - if (!isValidIndexedAttestation(state, epochCtx.getIndexedAttestation(attestation), verifySignature)) { + if (!isValidIndexedAttestation(state, epochCtx.getIndexedAttestation(ForkSeq.phase0, attestation), verifySignature)) { throw new Error("Attestation is not valid"); } } @@ -59,19 +59,14 @@ export function processAttestationPhase0( export function validateAttestation( fork: ForkSeq, state: CachedBeaconStateAllForks, - attestation: phase0.Attestation + attestation: allForks.Attestation ): void { const {epochCtx} = state; const slot = state.slot; const data = attestation.data; const computedEpoch = computeEpochAtSlot(data.slot); const committeeCount = epochCtx.getCommitteeCountPerSlot(computedEpoch); - if (!(data.index < committeeCount)) { - throw new Error( - "Attestation committee index not within current committee count: " + - `committeeIndex=${data.index} committeeCount=${committeeCount}` - ); - } + if (!(data.target.epoch === epochCtx.previousShuffling.epoch || data.target.epoch === epochCtx.epoch)) { throw new Error( "Attestation target epoch not in previous or current epoch: " + @@ -93,12 +88,47 @@ export function validateAttestation( ); } - const committee = epochCtx.getBeaconCommittee(data.slot, data.index); - if (attestation.aggregationBits.bitLen !== committee.length) { - throw new Error( - "Attestation aggregation bits length does not match committee length: " + - `aggregationBitsLength=${attestation.aggregationBits.bitLen} committeeLength=${committee.length}` - ); + if (fork >= ForkSeq.electra) { + if (data.index !== 0) { + throw new Error(`AttestationData.index must be zero: index=${data.index}`); + } + const attestationElectra = attestation as electra.Attestation; + const committeeBitsLength = attestationElectra.committeeBits.bitLen; + + if (committeeBitsLength > committeeCount) { + throw new Error( + `Attestation committee bits length are longer than number of committees: committeeBitsLength=${committeeBitsLength} numCommittees=${committeeCount}` + ); + } + + // TODO Electra: this should be obsolete soon when the spec switches to committeeIndices + const committeeIndices = attestationElectra.committeeBits.getTrueBitIndexes(); + + // Get total number of attestation participant of every committee specified + const participantCount = committeeIndices + .map((committeeIndex) => epochCtx.getBeaconCommittee(data.slot, committeeIndex).length) + .reduce((acc, committeeSize) => acc + committeeSize, 0); + + if (attestationElectra.aggregationBits.bitLen !== participantCount) { + throw new Error( + `Attestation aggregation bits length does not match total number of committee participant aggregationBitsLength=${attestation.aggregationBits.bitLen} participantCount=${participantCount}` + ); + } + } else { + if (!(data.index < committeeCount)) { + throw new Error( + "Attestation committee index not within current committee count: " + + `committeeIndex=${data.index} committeeCount=${committeeCount}` + ); + } + + const committee = epochCtx.getBeaconCommittee(data.slot, data.index); + if (attestation.aggregationBits.bitLen !== committee.length) { + throw new Error( + "Attestation aggregation bits length does not match committee length: " + + `aggregationBitsLength=${attestation.aggregationBits.bitLen} committeeLength=${committee.length}` + ); + } } } diff --git a/packages/state-transition/src/block/processAttestations.ts b/packages/state-transition/src/block/processAttestations.ts index 2b132fa22e0b..991ba5621905 100644 --- a/packages/state-transition/src/block/processAttestations.ts +++ b/packages/state-transition/src/block/processAttestations.ts @@ -1,4 +1,4 @@ -import {phase0} from "@lodestar/types"; +import {allForks} from "@lodestar/types"; import {ForkSeq} from "@lodestar/params"; import {CachedBeaconStateAllForks, CachedBeaconStateAltair, CachedBeaconStatePhase0} from "../types.js"; import {processAttestationPhase0} from "./processAttestationPhase0.js"; @@ -10,7 +10,7 @@ import {processAttestationsAltair} from "./processAttestationsAltair.js"; export function processAttestations( fork: ForkSeq, state: CachedBeaconStateAllForks, - attestations: phase0.Attestation[], + attestations: allForks.Attestation[], verifySignatures = true ): void { if (fork === ForkSeq.phase0) { diff --git a/packages/state-transition/src/block/processAttestationsAltair.ts b/packages/state-transition/src/block/processAttestationsAltair.ts index e37629712194..48d304c08133 100644 --- a/packages/state-transition/src/block/processAttestationsAltair.ts +++ b/packages/state-transition/src/block/processAttestationsAltair.ts @@ -1,5 +1,5 @@ import {byteArrayEquals} from "@chainsafe/ssz"; -import {Epoch, phase0} from "@lodestar/types"; +import {Epoch, allForks, phase0} from "@lodestar/types"; import {intSqrt} from "@lodestar/utils"; import { @@ -32,7 +32,7 @@ const SLOTS_PER_EPOCH_SQRT = intSqrt(SLOTS_PER_EPOCH); export function processAttestationsAltair( fork: ForkSeq, state: CachedBeaconStateAltair, - attestations: phase0.Attestation[], + attestations: allForks.Attestation[], verifySignature = true ): void { const {epochCtx} = state; @@ -49,8 +49,7 @@ export function processAttestationsAltair( validateAttestation(fork, state, attestation); // Retrieve the validator indices from the attestation participation bitfield - const committeeIndices = epochCtx.getBeaconCommittee(data.slot, data.index); - const attestingIndices = attestation.aggregationBits.intersectValues(committeeIndices); + const attestingIndices = epochCtx.getAttestingIndices(fork, attestation); // this check is done last because its the most expensive (if signature verification is toggled on) // TODO: Why should we verify an indexed attestation that we just created? If it's just for the signature diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index ade519719fc0..5a3d39235b3a 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -2,7 +2,17 @@ import {CoordType, PublicKey} from "@chainsafe/bls/types"; import bls from "@chainsafe/bls"; import * as immutable from "immutable"; import {fromHexString} from "@chainsafe/ssz"; -import {BLSSignature, CommitteeIndex, Epoch, Slot, ValidatorIndex, phase0, SyncPeriod} from "@lodestar/types"; +import { + BLSSignature, + CommitteeIndex, + Epoch, + Slot, + ValidatorIndex, + phase0, + SyncPeriod, + allForks, + electra, +} from "@lodestar/types"; import {createBeaconConfig, BeaconConfig, ChainConfig} from "@lodestar/config"; import { ATTESTATION_SUBNET_COUNT, @@ -643,15 +653,48 @@ export class EpochCache { * Return the beacon committee at slot for index. */ getBeaconCommittee(slot: Slot, index: CommitteeIndex): Uint32Array { + return this.getBeaconCommittees(slot, [index]); + } + + /** + * Return a single Uint32Array representing concatted committees of indices + */ + getBeaconCommittees(slot: Slot, indices: CommitteeIndex[]): Uint32Array { + + if (indices.length === 0) { + throw new Error("Attempt to get committees without providing CommitteeIndex"); + } + const slotCommittees = this.getShufflingAtSlot(slot).committees[slot % SLOTS_PER_EPOCH]; - if (index >= slotCommittees.length) { - throw new EpochCacheError({ - code: EpochCacheErrorCode.COMMITTEE_INDEX_OUT_OF_RANGE, - index, - maxIndex: slotCommittees.length, - }); + const committees = []; + + for (const index of indices) { + if (index >= slotCommittees.length) { + throw new EpochCacheError({ + code: EpochCacheErrorCode.COMMITTEE_INDEX_OUT_OF_RANGE, + index, + maxIndex: slotCommittees.length, + }); + } + committees.push(slotCommittees[index]); + } + + // Early return if only one index + if (committees.length === 1) { + return committees[0]; + } + + // Create a new Uint32Array to flatten `committees` + const totalLength = committees.reduce((acc, curr) => acc + curr.length, 0); + const result = new Uint32Array(totalLength); + + let offset = 0; + for (const committee of committees) { + result.set(committee, offset); + offset += committee.length; } - return slotCommittees[index]; + + return result; } getCommitteeCountPerSlot(epoch: Epoch): number { @@ -737,10 +780,9 @@ export class EpochCache { /** * Return the indexed attestation corresponding to ``attestation``. */ - getIndexedAttestation(attestation: phase0.Attestation): phase0.IndexedAttestation { - const {aggregationBits, data} = attestation; - const committeeIndices = this.getBeaconCommittee(data.slot, data.index); - const attestingIndices = aggregationBits.intersectValues(committeeIndices); + getIndexedAttestation(fork: ForkSeq, attestation: allForks.Attestation): allForks.IndexedAttestation { + const {data} = attestation; + const attestingIndices = this.getAttestingIndices(fork, attestation); // sort in-place attestingIndices.sort((a, b) => a - b); @@ -751,6 +793,32 @@ export class EpochCache { }; } + /** + * Return indices of validators who attestested in `attestation` + */ + getAttestingIndices(fork: ForkSeq, attestation: allForks.Attestation): number[] { + if (fork < ForkSeq.electra) { + const {aggregationBits, data} = attestation; + const validatorIndices = this.getBeaconCommittee(data.slot, data.index); + + return aggregationBits.intersectValues(validatorIndices); + } else { + const {aggregationBits, committeeBits, data} = attestation as electra.Attestation; + + // There is a naming conflict on the term `committeeIndices` + // In Lodestar it usually means a list of validator indices of participants in a committee + // In the spec it means a list of committee indices according to committeeBits + // This `committeeIndices` refers to the latter + const committeeIndices = committeeBits.getTrueBitIndexes(); + + const validatorIndices = this.getBeaconCommittees(data.slot, committeeIndices); + + const attestingIndices = new Set(aggregationBits.intersectValues(validatorIndices)); + + return Array.from(attestingIndices); + } + } + getCommitteeAssignments( epoch: Epoch, requestedValidatorIndices: ValidatorIndex[] diff --git a/packages/state-transition/src/signatureSets/indexedAttestation.ts b/packages/state-transition/src/signatureSets/indexedAttestation.ts index b5c48a20c9d4..724b186bee40 100644 --- a/packages/state-transition/src/signatureSets/indexedAttestation.ts +++ b/packages/state-transition/src/signatureSets/indexedAttestation.ts @@ -42,6 +42,9 @@ export function getAttestationsSignatureSets( signedBlock: allForks.SignedBeaconBlock ): ISignatureSet[] { return signedBlock.message.body.attestations.map((attestation) => - getIndexedAttestationSignatureSet(state, state.epochCtx.getIndexedAttestation(attestation)) + getIndexedAttestationSignatureSet( + state, + state.epochCtx.getIndexedAttestation(state.epochCtx.config.getForkSeq(signedBlock.message.slot), attestation) + ) ); } From 0de6dfa0147625dd27546fe8b35686070dd38019 Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Sun, 5 May 2024 14:46:30 +0300 Subject: [PATCH 2/3] Fix check-types --- packages/beacon-node/test/spec/presets/fork_choice.test.ts | 2 +- .../chain/validation/attestation/validateAttestation.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/beacon-node/test/spec/presets/fork_choice.test.ts b/packages/beacon-node/test/spec/presets/fork_choice.test.ts index 6e8491138697..6b260eebde1c 100644 --- a/packages/beacon-node/test/spec/presets/fork_choice.test.ts +++ b/packages/beacon-node/test/spec/presets/fork_choice.test.ts @@ -135,7 +135,7 @@ const forkChoiceTest = if (!attestation) throw Error(`No attestation ${step.attestation}`); const headState = chain.getHeadState(); const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(attestation.data)); - chain.forkChoice.onAttestation(headState.epochCtx.getIndexedAttestation(attestation), attDataRootHex); + chain.forkChoice.onAttestation(headState.epochCtx.getIndexedAttestation(ForkSeq[fork], attestation), attDataRootHex); } // attester slashing step diff --git a/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts b/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts index 56aab699f4f7..b4ea2ad9b615 100644 --- a/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts @@ -10,7 +10,7 @@ import { ApiAttestation, GossipAttestation, validateApiAttestation, - validateAttestation, +validateAttestation, } from "../../../../../src/chain/validation/index.js"; import {getAttDataBase64FromAttestationSerialized} from "../../../../../src/util/sszBytes.js"; import {memoOnce} from "../../../../utils/cache.js"; From 4760572d813377cbf34c20de10f4e02d6f995b4f Mon Sep 17 00:00:00 2001 From: Navie Chan Date: Tue, 7 May 2024 21:42:28 +0300 Subject: [PATCH 3/3] Address comments --- .../beacon-node/src/chain/blocks/importBlock.ts | 1 + .../test/spec/presets/fork_choice.test.ts | 5 ++++- .../attestation/validateAttestation.test.ts | 2 +- .../src/block/processAttestationPhase0.ts | 15 +++++++-------- packages/state-transition/src/cache/epochCache.ts | 6 ++---- .../src/signatureSets/indexedAttestation.ts | 1 + 6 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index be6936af37ce..7ea026ca53fc 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -147,6 +147,7 @@ export async function importBlock( for (const attestation of attestations) { try { + // TODO Electra: figure out how to reuse the attesting indices computed from state transition const indexedAttestation = postState.epochCtx.getIndexedAttestation(fork, attestation); const {target, beaconBlockRoot} = attestation.data; diff --git a/packages/beacon-node/test/spec/presets/fork_choice.test.ts b/packages/beacon-node/test/spec/presets/fork_choice.test.ts index 6b260eebde1c..5e237828ef1f 100644 --- a/packages/beacon-node/test/spec/presets/fork_choice.test.ts +++ b/packages/beacon-node/test/spec/presets/fork_choice.test.ts @@ -135,7 +135,10 @@ const forkChoiceTest = if (!attestation) throw Error(`No attestation ${step.attestation}`); const headState = chain.getHeadState(); const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(attestation.data)); - chain.forkChoice.onAttestation(headState.epochCtx.getIndexedAttestation(ForkSeq[fork], attestation), attDataRootHex); + chain.forkChoice.onAttestation( + headState.epochCtx.getIndexedAttestation(ForkSeq[fork], attestation), + attDataRootHex + ); } // attester slashing step diff --git a/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts b/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts index b4ea2ad9b615..56aab699f4f7 100644 --- a/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/attestation/validateAttestation.test.ts @@ -10,7 +10,7 @@ import { ApiAttestation, GossipAttestation, validateApiAttestation, -validateAttestation, + validateAttestation, } from "../../../../../src/chain/validation/index.js"; import {getAttDataBase64FromAttestationSerialized} from "../../../../../src/util/sszBytes.js"; import {memoOnce} from "../../../../utils/cache.js"; diff --git a/packages/state-transition/src/block/processAttestationPhase0.ts b/packages/state-transition/src/block/processAttestationPhase0.ts index 8b1890240038..9cd1823e4dd4 100644 --- a/packages/state-transition/src/block/processAttestationPhase0.ts +++ b/packages/state-transition/src/block/processAttestationPhase0.ts @@ -2,6 +2,7 @@ import {toHexString} from "@chainsafe/ssz"; import {Slot, allForks, electra, phase0, ssz} from "@lodestar/types"; import {MIN_ATTESTATION_INCLUSION_DELAY, SLOTS_PER_EPOCH, ForkSeq} from "@lodestar/params"; +import {assert} from "@lodestar/utils"; import {computeEpochAtSlot} from "../util/index.js"; import {CachedBeaconStatePhase0, CachedBeaconStateAllForks} from "../types.js"; import {isValidIndexedAttestation} from "./index.js"; @@ -89,9 +90,7 @@ export function validateAttestation( } if (fork >= ForkSeq.electra) { - if (data.index !== 0) { - throw new Error(`AttestationData.index must be zero: index=${data.index}`); - } + assert.equal(data.index, 0, `AttestationData.index must be zero: index=${data.index}`); const attestationElectra = attestation as electra.Attestation; const committeeBitsLength = attestationElectra.committeeBits.bitLen; @@ -109,11 +108,11 @@ export function validateAttestation( .map((committeeIndex) => epochCtx.getBeaconCommittee(data.slot, committeeIndex).length) .reduce((acc, committeeSize) => acc + committeeSize, 0); - if (attestationElectra.aggregationBits.bitLen !== participantCount) { - throw new Error( - `Attestation aggregation bits length does not match total number of committee participant aggregationBitsLength=${attestation.aggregationBits.bitLen} participantCount=${participantCount}` - ); - } + assert.equal( + attestationElectra.aggregationBits.bitLen, + participantCount, + `Attestation aggregation bits length does not match total number of committee participant aggregationBitsLength=${attestation.aggregationBits.bitLen} participantCount=${participantCount}` + ); } else { if (!(data.index < committeeCount)) { throw new Error( diff --git a/packages/state-transition/src/cache/epochCache.ts b/packages/state-transition/src/cache/epochCache.ts index 5a3d39235b3a..3d73a58e509b 100644 --- a/packages/state-transition/src/cache/epochCache.ts +++ b/packages/state-transition/src/cache/epochCache.ts @@ -660,7 +660,6 @@ export class EpochCache { * Return a single Uint32Array representing concatted committees of indices */ getBeaconCommittees(slot: Slot, indices: CommitteeIndex[]): Uint32Array { - if (indices.length === 0) { throw new Error("Attempt to get committees without providing CommitteeIndex"); } @@ -809,13 +808,12 @@ export class EpochCache { // In Lodestar it usually means a list of validator indices of participants in a committee // In the spec it means a list of committee indices according to committeeBits // This `committeeIndices` refers to the latter + // TODO Electra: resolve the naming conflicts const committeeIndices = committeeBits.getTrueBitIndexes(); const validatorIndices = this.getBeaconCommittees(data.slot, committeeIndices); - const attestingIndices = new Set(aggregationBits.intersectValues(validatorIndices)); - - return Array.from(attestingIndices); + return aggregationBits.intersectValues(validatorIndices); } } diff --git a/packages/state-transition/src/signatureSets/indexedAttestation.ts b/packages/state-transition/src/signatureSets/indexedAttestation.ts index 724b186bee40..cb4706c645fe 100644 --- a/packages/state-transition/src/signatureSets/indexedAttestation.ts +++ b/packages/state-transition/src/signatureSets/indexedAttestation.ts @@ -41,6 +41,7 @@ export function getAttestationsSignatureSets( state: CachedBeaconStateAllForks, signedBlock: allForks.SignedBeaconBlock ): ISignatureSet[] { + // TODO: figure how to get attesting indices of an attestation once per block processing return signedBlock.message.body.attestations.map((attestation) => getIndexedAttestationSignatureSet( state,