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.

Sign up for GitHub

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

Jump to bottom

fix: fix electra genesis spec test #6764

Merged
merged 5 commits into from
May 14, 2024
Merged

fix: fix electra genesis spec test #6764

merged 5 commits into from
May 14, 2024

Conversation

ensi321
Copy link
Contributor

@ensi321 ensi321 commented May 13, 2024

Fix genesis spec test

   ✓ electra/genesis/initialization/pyspec_tests (7) 813ms
     ✓ electra/genesis/initialization/pyspec_tests/initialize_beacon_state_from_eth1
     ✓ electra/genesis/initialization/pyspec_tests/initialize_beacon_state_one_topup_activation
     ✓ electra/genesis/initialization/pyspec_tests/initialize_beacon_state_random_invalid_genesis
     ✓ electra/genesis/initialization/pyspec_tests/initialize_beacon_state_random_valid_genesis
     ✓ electra/genesis/initialization/pyspec_tests/initialize_beacon_state_some_small_balances 334ms
     ✓ electra/genesis/initialization/pyspec_tests/initialize_post_transition
     ✓ electra/genesis/initialization/pyspec_tests/initialize_pre_transition_empty_payload
   ✓ electra/genesis/validity/pyspec_tests (5)
     ✓ electra/genesis/validity/pyspec_tests/extra_balance
     ✓ electra/genesis/validity/pyspec_tests/full_genesis_deposits
     ✓ electra/genesis/validity/pyspec_tests/invalid_invalid_timestamp
     ✓ electra/genesis/validity/pyspec_tests/invalid_not_enough_validator_count
     ✓ electra/genesis/validity/pyspec_tests/one_more_validator

@ensi321 ensi321 requested a review from a team as a code owner May 13, 2024 18:54
twoeths
twoeths reviewed May 13, 2024
View reviewed changes
packages/state-transition/src/util/genesis.ts
@@ -127,6 +129,7 @@ export function applyTimestamp(config: ChainForkConfig, state: CachedBeaconState
* @returns active validator indices
*/
export function applyDeposits(
fork: ForkSeq,
Copy link
Contributor

Choose a reason for hiding this comment

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

a lot of our code has new "fork" param and it caused a lot of files changed, we can just get the config from state and determine the fork based on state.slot
the good thing is we don't have to keep computing fork in different functions if we pass fork param
I think we can store fork inside the CachedBeaconState so that we don't have to pass fork param around or keep recomputing it in different functions

twoeths
twoeths approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@twoeths twoeths left a comment

Choose a reason for hiding this comment

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

lgtm

@twoeths twoeths merged commit c50d5ef into electra-fork May 14, 2024
2 of 17 checks passed
@twoeths twoeths deleted the nc/genesis-spec-test branch May 14, 2024 02:52
@ensi321 ensi321 mentioned this pull request May 14, 2024
Open
g11tech pushed a commit that referenced this pull request May 24, 2024
@ensi321 @nazarhussain @g11tech
fix: fix electra genesis spec test (#6764)
9e9e3cf 
* process pending deposit from eth1

* Fix the genesis params

* fix

* Fix

* clean up

---------

Co-authored-by: Nazar Hussain <nazarhussain@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twoeths twoeths twoeths approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ensi321 @twoeths @nazarhussain