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

remove all but truly stub support for SHARDING_FORK_{EPOCH,VERSION} #4385

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

tersec
Copy link
Contributor

@tersec tersec commented Dec 2, 2022

Currently, its only appearances in the consensus-specs repository are

consensus-specs$ rg SHARDING
specs/das/fork-choice.md
40:    if compute_epoch_at_slot(block.slot) < SHARDING_FORK_EPOCH:

tests/core/pyspec/eth2spec/test/context.py
14:    PHASE0, ALTAIR, BELLATRIX, CAPELLA, EIP4844, SHARDING,
311:    elif _check_current_version(spec, state, SHARDING):
315:        overrides['SHARDING_FORK_EPOCH'] = spec.GENESIS_EPOCH

tests/core/pyspec/eth2spec/test/sharding/unittests/test_get_start_shard.py
5:from eth2spec.test.helpers.constants import SHARDING
9:@with_phases([SHARDING])
26:@with_phases([SHARDING])
42:@with_phases([SHARDING])
60:@with_phases([SHARDING])
79:@with_phases([SHARDING])

tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py
54:    assert spec.config.SHARDING_FORK_EPOCH == spec.GENESIS_EPOCH
55:    if state.fork.current_version == spec.config.SHARDING_FORK_VERSION:

tests/core/pyspec/eth2spec/test/phase0/unittests/fork_choice/test_on_attestation.py
27:    # elif spec.fork == SHARDING: TODO: check if vote count for shard blob increased as expected

tests/core/pyspec/eth2spec/test/phase0/sanity/test_blocks.py
576:# TODO All AttesterSlashing tests should be adopted for SHARDING and later but helper support is not yet there
821:    # if spec.fork == SHARDING:
898:# After SHARDING is enabled, a committee is computed for SHARD_COMMITTEE_PERIOD slots ago,
901:# TODO: when integrating SHARDING tests, voluntary-exit tests may need to change.

tests/core/pyspec/eth2spec/test/helpers/state.py
47:    Transition to slot `compute_epoch_at_slot(spec.config.SHARDING_FORK_EPOCH) + 1`
48:    and fork at `compute_epoch_at_slot(spec.config.SHARDING_FORK_EPOCH)`.
50:    transition_to(spec, state, spec.compute_epoch_at_slot(spec.config.SHARDING_FORK_EPOCH))

tests/core/pyspec/eth2spec/test/helpers/attestations.py
85:    # if spec.fork == SHARDING  # TODO: add extra data for shard voting
198:        # if spec.fork == SHARDING:  TODO: add shard data to attestation, include shard headers in block

tests/core/pyspec/eth2spec/test/helpers/constants.py
14:SHARDING = SpecForkName('sharding')

That is, it does not have any defined semantics, and it exists solely as a set of TODOs and boundary conditions.

https://github.com/eth-clients/eth2-networks and https://github.com/eth-clients/merge-testnets network definitions do to contain it, so this PR does not remove the spec/presets RuntimeConfig support for reading in that field from a such a network configuration. However, checkForkConsistency then asserts unconditionally that SHARDING_FORK_EPOCH is FAR_FUTURE_EPOCH and otherwise ignores it.

This PR was prompted by #4365 discovering that there's a spurious conflict between certain definitions of EIP4844_FORK_VERSION and SHARDING_FORK_VERSION, when the latter is irrelevant.

@github-actions
Copy link

github-actions bot commented Dec 2, 2022

Unit Test Results

         9 files  ±0       873 suites  ±0   24m 50s ⏱️ +20s
  2 704 tests ±0    2 511 ✔️ ±0  193 💤 ±0  0 ±0 
11 783 runs  ±0  11 564 ✔️ ±0  219 💤 ±0  0 ±0 

Results for commit d0b58cc. ± Comparison against base commit 4e71e77.

@arnetheduck arnetheduck merged commit 5c16062 into unstable Dec 2, 2022
@arnetheduck arnetheduck deleted the ni9 branch December 2, 2022 12:33
@etan-status
Copy link
Contributor

RuntimeConfig parser has funtionality to ignore an irrelevant constant btw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants