Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: enable spec tests related to eip-7549 #6741

Merged
merged 27 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
6ddf100
initial commit
ensi321 Apr 16, 2024
14e6fd8
Update gossip validation
ensi321 Apr 24, 2024
32e8ae0
Update attestation gossip validation
ensi321 Apr 25, 2024
f491628
aggregateAndProof validation
ensi321 Apr 26, 2024
39b259e
Extend spec runner to be more flexible
nazarhussain May 7, 2024
2ba9a22
Add missing state attributes for electra
nazarhussain May 7, 2024
8061a4d
Fix ss data types for electra spec
nazarhussain May 7, 2024
c509eb0
Make the spec runner more flexible
nazarhussain May 7, 2024
c7f4b26
Fix the bug in process attestation
nazarhussain May 7, 2024
0ff25d9
Update the sepc test version
nazarhussain May 8, 2024
cd74406
clean up
ensi321 Apr 26, 2024
a4761e8
Misc
ensi321 Apr 29, 2024
2cf273d
Fix the build erros
nazarhussain May 6, 2024
316a500
feat: get attestations for electra block (#6732)
twoeths May 7, 2024
295642f
Fix check-types
ensi321 May 5, 2024
13878fa
Address comments
ensi321 May 7, 2024
bddcf2b
Fix the build erros
nazarhussain May 6, 2024
4285f75
Extend spec runner to be more flexible
nazarhussain May 7, 2024
8bcc864
Add missing state attributes for electra
nazarhussain May 7, 2024
6610dc1
Fix ss data types for electra spec
nazarhussain May 7, 2024
3d88a33
Make the spec runner more flexible
nazarhussain May 7, 2024
8fc72da
Fix the bug in process attestation
nazarhussain May 7, 2024
8be57b1
Update the sepc test version
nazarhussain May 8, 2024
323e74c
Fix rebase issue
ensi321 May 8, 2024
40b7eb7
Merge branch 'nh/5749-spec-tests' of github.com:ChainSafe/lodestar in…
nazarhussain May 8, 2024
3c6e2e6
Update committee index count check
nazarhussain May 8, 2024
ff0e397
Merge branch 'electra-fork' into nh/5749-spec-tests
ensi321 May 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/beacon-node/test/spec/presets/operations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ const operations: TestRunnerFn<OperationsTestCase, BeaconStateAllForks> = (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,
Expand Down
2 changes: 1 addition & 1 deletion packages/beacon-node/test/spec/specTestVersioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
27 changes: 19 additions & 8 deletions packages/beacon-node/test/spec/utils/specTestIterator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ const ARTIFACT_FILENAMES = new Set([
]);

export interface SkipOpts {
skippedPrefixes?: string[];
skippedTestSuites?: RegExp[];
skippedTests?: RegExp[];
skippedForks?: string[];
skippedRunners?: string[];
skippedHandlers?: string[];
Expand Down Expand Up @@ -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)/,
ensi321 marked this conversation as resolved.
Show resolved Hide resolved
/^electra\/(?!operations\/attestation)/,
],
skippedTests: [],
skippedRunners: ["merkle_proof", "networking"],
};

Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why we would want to specify fork that we want to skip in a env variable SPEC_FILTER_FORK. What's the use case for this? I think defaultSkipOpts is serving quite well

Copy link
Contributor Author

@nazarhussain nazarhussain May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quickly run the spec tests for some forks. Whilde debugging, if we run for all forks it take too much time.

SPEC_FILTER_FORK=electra yarn test:spec:minimal

) {
continue;
}
const fork = forkStr as ForkName;
Expand Down Expand Up @@ -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}`);
Expand All @@ -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);
}
}
Expand Down
19 changes: 11 additions & 8 deletions packages/state-transition/src/block/processAttestationPhase0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions packages/types/src/electra/sszTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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"}
);
Expand Down
Loading