From 176f452a46cda5f3b5f9824cea187c879c35563e Mon Sep 17 00:00:00 2001 From: Nazar Hussain Date: Wed, 8 May 2024 17:14:44 +0200 Subject: [PATCH] test: enable spec tests related to eip-7549 (#6741) * initial commit * Update gossip validation * Update attestation gossip validation * aggregateAndProof validation * Extend spec runner to be more flexible * Add missing state attributes for electra * Fix ss data types for electra spec * Make the spec runner more flexible * Fix the bug in process attestation * Update the sepc test version * clean up * Misc * Fix the build erros * feat: get attestations for electra block (#6732) * feat: getAttestationsForBlock() for electra * chore: fix lint * fix: MAX_ATTESTATIONS_PER_GROUP_ELECTRA and address PR comments * chore: unit test aggregateConsolidation * Fix rebase mistake * Address my own comment :) --------- Co-authored-by: Navie Chan * Fix check-types * Address comments * Fix the build erros * Extend spec runner to be more flexible * Add missing state attributes for electra * Fix ss data types for electra spec * Make the spec runner more flexible * Fix the bug in process attestation * Update the sepc test version * Fix rebase issue * Update committee index count check --------- Co-authored-by: NC Co-authored-by: Navie Chan Co-authored-by: tuyennhv --- .../test/spec/presets/operations.test.ts | 4 +-- .../test/spec/specTestVersioning.ts | 2 +- .../test/spec/utils/specTestIterator.ts | 27 +++++++++++++------ .../src/block/processAttestationPhase0.ts | 19 +++++++------ packages/types/src/electra/sszTypes.ts | 20 +++++++------- 5 files changed, 43 insertions(+), 29 deletions(-) diff --git a/packages/beacon-node/test/spec/presets/operations.test.ts b/packages/beacon-node/test/spec/presets/operations.test.ts index 0e6fdafd225..c2934b55dd6 100644 --- a/packages/beacon-node/test/spec/presets/operations.test.ts +++ b/packages/beacon-node/test/spec/presets/operations.test.ts @@ -122,8 +122,8 @@ const operations: TestRunnerFn = (fork, sszTypes: { pre: ssz[fork].BeaconState, post: ssz[fork].BeaconState, - attestation: ssz.phase0.Attestation, - attester_slashing: ssz.phase0.AttesterSlashing, + attestation: fork === ForkName.electra ? ssz.electra.Attestation : ssz.phase0.Attestation, + attester_slashing: fork === ForkName.electra ? ssz.electra.AttesterSlashing : ssz.phase0.AttesterSlashing, block: ssz[fork].BeaconBlock, body: ssz[fork].BeaconBlockBody, deposit: ssz.phase0.Deposit, diff --git a/packages/beacon-node/test/spec/specTestVersioning.ts b/packages/beacon-node/test/spec/specTestVersioning.ts index 06b02ab5304..9195532379a 100644 --- a/packages/beacon-node/test/spec/specTestVersioning.ts +++ b/packages/beacon-node/test/spec/specTestVersioning.ts @@ -15,7 +15,7 @@ import {DownloadTestsOptions} from "@lodestar/spec-test-util/downloadTests"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); export const ethereumConsensusSpecsTests: DownloadTestsOptions = { - specVersion: "v1.5.0-alpha.1", + specVersion: "v1.5.0-alpha.2", // Target directory is the host package root: 'packages/*/spec-tests' outputDir: path.join(__dirname, "../../spec-tests"), specTestsRepoUrl: "https://github.com/ethereum/consensus-spec-tests", diff --git a/packages/beacon-node/test/spec/utils/specTestIterator.ts b/packages/beacon-node/test/spec/utils/specTestIterator.ts index 88d7cbdea9e..b99bc281cf5 100644 --- a/packages/beacon-node/test/spec/utils/specTestIterator.ts +++ b/packages/beacon-node/test/spec/utils/specTestIterator.ts @@ -14,7 +14,8 @@ const ARTIFACT_FILENAMES = new Set([ ]); export interface SkipOpts { - skippedPrefixes?: string[]; + skippedTestSuites?: RegExp[]; + skippedTests?: RegExp[]; skippedForks?: string[]; skippedRunners?: string[]; skippedHandlers?: string[]; @@ -57,14 +58,17 @@ const coveredTestRunners = [ // ], // ``` export const defaultSkipOpts: SkipOpts = { - skippedForks: ["electra", "eip7594"], + skippedForks: ["eip7594"], // TODO: capella // BeaconBlockBody proof in lightclient is the new addition in v1.3.0-rc.2-hotfix // Skip them for now to enable subsequently - skippedPrefixes: [ - "capella/light_client/single_merkle_proof/BeaconBlockBody", - "deneb/light_client/single_merkle_proof/BeaconBlockBody", + skippedTestSuites: [ + /^capella\/light_client\/single_merkle_proof\/BeaconBlockBody.*/, + /^deneb\/light_client\/single_merkle_proof\/BeaconBlockBody.*/, + // /^electra\/(?!operations\/attestations)(?!operations\/attester_slashing)/, + /^electra\/(?!operations\/attestation)/, ], + skippedTests: [], skippedRunners: ["merkle_proof", "networking"], }; @@ -100,7 +104,10 @@ export function specTestIterator( opts: SkipOpts = defaultSkipOpts ): void { for (const forkStr of readdirSyncSpec(configDirpath)) { - if (opts?.skippedForks?.includes(forkStr)) { + if ( + opts?.skippedForks?.includes(forkStr) || + (process.env.SPEC_FILTER_FORK && forkStr !== process.env.SPEC_FILTER_FORK) + ) { continue; } const fork = forkStr as ForkName; @@ -134,7 +141,7 @@ export function specTestIterator( for (const testSuite of readdirSyncSpec(testHandlerDirpath)) { const testId = `${fork}/${testRunnerName}/${testHandler}/${testSuite}`; - if (opts?.skippedPrefixes?.some((skippedPrefix) => testId.startsWith(skippedPrefix))) { + if (opts?.skippedTestSuites?.some((skippedMatch) => testId.match(skippedMatch))) { displaySkipTest(testId); } else if (fork === undefined) { displayFailTest(testId, `Unknown fork ${forkStr}`); @@ -150,7 +157,11 @@ export function specTestIterator( // Generic testRunner else { const {testFunction, options} = testRunner.fn(fork, testHandler, testSuite); - + if (opts.skippedTests && options.shouldSkip === undefined) { + options.shouldSkip = (_testCase: any, name: string, _index: number): boolean => { + return opts?.skippedTests?.some((skippedMatch) => name.match(skippedMatch)) ?? false; + }; + } describeDirectorySpecTest(testId, testSuiteDirpath, testFunction, options); } } diff --git a/packages/state-transition/src/block/processAttestationPhase0.ts b/packages/state-transition/src/block/processAttestationPhase0.ts index 9cd1823e4dd..fb3c9e2da61 100644 --- a/packages/state-transition/src/block/processAttestationPhase0.ts +++ b/packages/state-transition/src/block/processAttestationPhase0.ts @@ -92,17 +92,20 @@ export function validateAttestation( if (fork >= ForkSeq.electra) { assert.equal(data.index, 0, `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(); + if (committeeIndices.length === 0) { + throw Error("Attestation should have at least one committee bit set"); + } else { + const lastCommitteeIndex = committeeIndices[committeeIndices.length - 1]; + if (lastCommitteeIndex >= committeeCount) { + throw new Error( + `Attestation committee index exceeds committee count: lastCommitteeIndex=${lastCommitteeIndex} numCommittees=${committeeCount}` + ); + } + } + // Get total number of attestation participant of every committee specified const participantCount = committeeIndices .map((committeeIndex) => epochCtx.getBeaconCommittee(data.slot, committeeIndex).length) diff --git a/packages/types/src/electra/sszTypes.ts b/packages/types/src/electra/sszTypes.ts index 246c731d838..ee8d2702c65 100644 --- a/packages/types/src/electra/sszTypes.ts +++ b/packages/types/src/electra/sszTypes.ts @@ -19,8 +19,8 @@ import { MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD, MAX_CONSOLIDATIONS, PENDING_BALANCE_DEPOSITS_LIMIT, - PENDING_CONSOLIDATIONS_LIMIT, PENDING_PARTIAL_WITHDRAWALS_LIMIT, + PENDING_CONSOLIDATIONS_LIMIT, } from "@lodestar/params"; import {ssz as primitiveSsz} from "../primitive/index.js"; import {ssz as phase0Ssz} from "../phase0/index.js"; @@ -335,15 +335,15 @@ export const BeaconState = new ContainerType( nextWithdrawalValidatorIndex: capellaSsz.BeaconState.fields.nextWithdrawalValidatorIndex, // Deep history valid from Capella onwards historicalSummaries: capellaSsz.BeaconState.fields.historicalSummaries, - depositRequestsStartIndex: UintBn64, // New in ELECTRA - depositBalanceToConsume: Gwei, // [New in Electra] - exitBalanceToConsume: Gwei, // [New in Electra] - earliestExitEpoch: Epoch, // [New in Electra] - consolidationBalanceToConsume: Gwei, // [New in Electra] - earliestConsolidationEpoch: Epoch, // [New in Electra] - pendingBalanceDeposits: new ListCompositeType(PendingBalanceDeposit, PENDING_BALANCE_DEPOSITS_LIMIT), // [New in Electra] - pendingPartialWithdrawals: new ListCompositeType(PartialWithdrawal, PENDING_PARTIAL_WITHDRAWALS_LIMIT), // [New in Electra] - pendingConsolidations: new ListCompositeType(PendingConsolidation, PENDING_CONSOLIDATIONS_LIMIT), // [New in Electra] + depositRequestsStartIndex: UintBn64, // New in ELECTRA:EIP6110 + depositBalanceToConsume: Gwei, // New in Electra:EIP7251 + exitBalanceToConsume: Gwei, // New in Electra:EIP7251 + earliestExitEpoch: Epoch, // New in Electra:EIP7251 + consolidationBalanceToConsume: Gwei, // New in Electra:EIP7251 + earliestConsolidationEpoch: Epoch, // New in Electra:EIP7251 + pendingBalanceDeposits: new ListCompositeType(PendingBalanceDeposit, Number(PENDING_BALANCE_DEPOSITS_LIMIT)), // new in electra:eip7251 + pendingPartialWithdrawals: new ListCompositeType(PartialWithdrawal, Number(PENDING_PARTIAL_WITHDRAWALS_LIMIT)), // New in Electra:EIP7251 + pendingConsolidations: new ListCompositeType(PendingConsolidation, Number(PENDING_CONSOLIDATIONS_LIMIT)), // new in electra:eip7251 }, {typeName: "BeaconState", jsonCase: "eth2"} );