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

Electra upgrade #1283

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

Electra upgrade #1283

wants to merge 29 commits into from

Conversation

claravanstaden
Copy link
Contributor

@claravanstaden claravanstaden commented Sep 6, 2024

Changes

  • changes beacon config to include fork versions instead of just a deneb fork value.
  • adds Electra modified and new beacon structures

Testing

  • Beacon client updates for Electra updates work
  • Live switch-over from Deneb to Electra works
  • Smoke tests work

Resolves SNO-1140.

Companion: Snowfork/polkadot-sdk#178

Note: I used a docker image for the relevant Geth dependency, since the source code used for Electra release does not build.

@@ -55,49 +53,3 @@ func DenebExecutionPayloadToScale(e *state.ExecutionPayloadDeneb) (scale.Executi
}, nil
}

func DenebJsonExecutionPayloadHeaderToScale(e *beaconjson.FullExecutionPayloadHeaderJson) (scale.ExecutionPayloadHeaderDeneb, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unused method.

@claravanstaden claravanstaden marked this pull request as ready for review January 16, 2025 10:10
--rest.namespace="*" \
--jwt-secret $config_dir/jwtsecret \
--chain.archiveStateEpochFrequency 1 \
>"$output_dir/lodestar.log" 2>&1 &
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be output_electra_dir?

Comment on lines +15 to +16
if [ "$is_electra" == "true" ]; then
HOST=$(ifconfig | grep -Eo 'inet (addr:)?([0-9]*\.){3}[0-9]*' | grep -Eo '([0-9]*\.){3}[0-9]*' | grep -v '127.0.0.1')
Copy link
Contributor

Choose a reason for hiding this comment

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

This may not work for machines with multi-network interfaces, my PC, e.g.

➜  test git:(electra) ✗ ifconfig | grep -Eo 'inet (addr:)?([0-9]*\.){3}[0-9]*' | grep -Eo '([0-9]*\.){3}[0-9]*' | grep -v '127.0.0.1'
192.168.0.127
26.26.26.1
192.168.64.1

Comment on lines +15 to +17
docker run --rm \
-v "${output_electra_dir}:/mnt" \
docker.io/ethpandaops/geth:lightclient-prague-devnet-4 \
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest not introducing docker for the local setup, can maybe build from source if necessary like lodestar.

Comment on lines +55 to +66
retries := 5
bootstrap, err := s.getCheckpoint()
if err != nil {
return scale.BeaconCheckpoint{}, fmt.Errorf("get finalized checkpoint: %w", err)
for retries > 0 {
retries = retries - 1
bootstrap, err = s.getCheckpoint()
if err != nil {
log.WithError(err).Info("retry bootstrap, sleeping")
time.Sleep(10 * time.Second)
continue
}
break
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious is the retry necessary here?

if isDeneb {
if forkVersion == protocol.Electra {
beaconState = &state.BeaconStateElectra{}
} else if forkVersion == protocol.Deneb {
beaconState = &state.BeaconStateDenebMainnet{}
} else {
beaconState = &state.BeaconStateCapellaMainnet{}
Copy link
Contributor

Choose a reason for hiding this comment

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

The Capella branch can be deleted?

Comment on lines +566 to 569
err = signedBlock.UnmarshalSSZ(blockBytes)
if err != nil {
return scale.HeaderUpdatePayload{}, fmt.Errorf("fetch block: %w", err)
return scale.HeaderUpdatePayload{}, fmt.Errorf("unmarshal block ssz: %w", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool! Much cleaner now.

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

Successfully merging this pull request may close these issues.

2 participants