-
Notifications
You must be signed in to change notification settings - Fork 1k
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
E2E from Phase0 or Bellatrix #11802
E2E from Phase0 or Bellatrix #11802
Conversation
@@ -197,7 +197,26 @@ func (node *BeaconNode) generateGenesis(ctx context.Context) (state.BeaconState, | |||
DepositCount: 0, | |||
BlockHash: gb.Hash().Bytes(), | |||
} | |||
v := e2etypes.GenesisFork() | |||
switch v { | |||
case version.Bellatrix: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets add altair here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do!
account = common.BytesToAddress(fr) | ||
// If the beacon chain has transitioned to Bellatrix, but the EL hasn't hit TTD, we could see a few slots | ||
// of blocks with empty payloads. | ||
if bytes.Equal(fr.Block.Body.ExecutionPayload.BlockHash, make([]byte, 32)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
PR #11746 changed e2e to start from bellatrix, allowing faster testnet startup by skipping earlier forks and removing the need to create on chain deposits so that deposit events would be seen by the deposit tracker. However that PR hardcoded starting up from bellatrix and broke starting from phase0. This PR fixes that problem and allows e2e to start from phase0, and changes phase0 to be the default starting point.
Which issues(s) does this PR fix?
Fixes #11750
Other notes for review
The method to create the pre-mined BeaconState is a mess of copy/pasta, and there are some other things I want to clean up. I'm going to work on refactoring that code, but I also want to merge this ASAP, so I'm putting it up in its current form. I'm hoping to get things cleaned up before someone reviews but we can also do that cleanup in a follow-up PR in the interest of starting e2e from phase0 and un-breaking scenario tests asap.
Shout-out to @nisdas for help debugging how too low of a TTD value was impacting the e2e run.
NB: we are going to merge this without altair support, follow up PR to refactor premined genesis codepaths with altair support coming next.