Skip to content

Remove EthSpec#9229

Open
macladson wants to merge 1 commit intosigp:unstablefrom
macladson:remove-ethspec
Open

Remove EthSpec#9229
macladson wants to merge 1 commit intosigp:unstablefrom
macladson:remove-ethspec

Conversation

@macladson
Copy link
Copy Markdown
Member

@macladson macladson commented Apr 29, 2026

Motivation

Nearly all consensus-related types throughout Lighthouse need the runtime trait EthSpec to provide access to preset spec related constants.

The decision to make this a trait on all consensus types was made to allow runtime selection of preset specs without needing to recompile. This also had the added benefit of allowing a single testing binary to test multiple presets at once (e.g. ef-tests).

However, this has created a poor developer experience. Adding what looks like a simple feature can often require threading EthSpec into multiple new types, methods, associated functions, etc. My argument is that this tradeoff was incorrectly made and that spec related consts should be set at compile time.

Proposed Changes

Remove EthSpec and replace it with a compile-time selected global Spec type.

Implications of this Change

Firstly, running different presets (mainnet, minimal, gnosis) will now require different binaries.
From a UX perspective, I think this won't be noticeable for the vast majority of users. We will continue to provide Mainnet-only binaries which will accomodate 99% of users including mainnet-preset networks (holesky, sepolia, etc). For users running Gnosis, they will either have to compile from source (with --gnosis or --spec-gnosis) or we can optionally also provide Gnosis-specific binaries. The Minimal spec is only used for testing and so is unlikely to affect any users.

Docker is similarly tricky, we usually ship our docker image to support all presets. Now we will have to provide a separate gnosis docker image and perhaps a minimal one too.

The bigger implication is in testing. We can no longer test minimal-spec tests and mainnet-spec tests in the same binary. This makes the testing framework a bit more difficult to reason about. Some existing tests were written with baked-in assumptions from 1 preset (usually mainnet). These tests need to be gated to prevent them running on minimal or gnosis test runs. Other tests were written only for minimal, usually for speed reasons as the minimal preset only has 8 slots per epoch.

Finally, EthSpec is gone. No more missing trait bounds, needing to thread EthSpec through multiple types for a simple change, etc. This is a huge win for DevEX imo

Reviewer Guide

The vast majority of the 14k+ line changes are mechanical changes. These largely follow these patterns:

  1. Removing E: EthSpec and <E> from types, methods and functions
  2. Changing E::SomeConst to Spec::SOME_CONST
  3. Swapping MainnetEthSpec/MinimalEthSpec to Spec in tests (this makes the test spec-agnostic provided that the test doesn't contain any spec-specific assumptions.

The changes which should be most carefully reviewed are changes in .github, Makefile, consensus/types/src/core/spec.rs, lcli,testing/ef_tests and network tests.

Some tests also are slightly altered to make them spec agnostic when it was trivial to do so.

What Will Developers Need to do Once These Changes are Merged?

The primary dev burden will be that when making tests you have to ensure the test follows one of these three patterns:

  1. Test is spec agnostic. This is the ideal case.
  2. Test is minimal only for speed purposes. Gate the test with #[cfg(feature = "spec-minimal"] and ensure it is available as a unit test or as an integration test module inside a tests/spec-minimal.rs target.
  3. Test is mainnet only for complexity/convenience. This is the least ideal case. The test will need to be gated with #[cfg(not(features = "spec-non-mainnet"))].

Final Notes

This PR was written with assistance from Opus 4.6 and GPT 5.5. However I have personally manually reviewed all 14k+ lines.
This PR basically conflicts with everything and so keeping this free from conflicts will be tricky and time-consuming. If this gets buy-in from devs a quick review cycle would be ideal.
For now I have added a CI target for the smallest possible set of spec-minimal and spec-gnosis tests. That is, the tests which are only compiled for specific specs. This keeps the CI test suite most similar to what we are used to, although there is some duplication in unit tests. Later we might decide we actually want to run the full test suite under minimal as well. We might need to add some extra makefile targets in that case. For now, any test you want to run under minimal can be done e.g. TEST_FEATURES="spec-minimal" make test-beacon-chain

@macladson macladson added work-in-progress PR is a work-in-progress code-quality backwards-incompat Backwards-incompatible API change labels Apr 29, 2026
@macladson macladson force-pushed the remove-ethspec branch 2 times, most recently from 231d560 to fba42c8 Compare April 30, 2026 08:41
pub attestations: VariableList<AttestationElectra<E>, E::MaxAttestationsElectra>,
pub deposits: VariableList<Deposit, E::MaxDeposits>,
pub voluntary_exits: VariableList<SignedVoluntaryExit, E::MaxVoluntaryExits>,
pub attestations: VariableList<AttestationElectra, U<{ Spec::MAX_ATTESTATIONS_ELECTRA }>>,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Another benefit which I forgot to mention. Now that spec constants are actually consts and not typenum types, we can actually move to migrate our SSZ stack to const generics and remove typenum altogether. Here we use the typenum provided U adapter for backwards compatiblity but once the SSZ stack is using const generics we will be able to useSpec::MAX_ATTESTATIONS_ELECTRA directly

@macladson macladson added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels May 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards-incompat Backwards-incompatible API change code-quality ready-for-review The code is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant