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: set sepolia genesis delay #6961

Merged
merged 2 commits into from
Jul 16, 2024
Merged

fix: set sepolia genesis delay #6961

merged 2 commits into from
Jul 16, 2024

Conversation

domiwei
Copy link
Contributor

@domiwei domiwei commented Jul 16, 2024 •
edited by nflaig
Loading

Motivation

GENESIS_DELAY should be 86400 on sepolia

Ref: https://github.com/eth-clients/sepolia/blob/main/metadata/config.yaml#L11

@domiwei domiwei requested a review from a team as a code owner July 16, 2024 06:11
@CLAassistant
Copy link

CLAassistant commented Jul 16, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@domiwei domiwei changed the title fix sepolia genesis delay fix: sepolia genesis delay Jul 16, 2024
nflaig
nflaig reviewed Jul 16, 2024
View reviewed changes
Copy link
Member

@nflaig nflaig left a comment

Choose a reason for hiding this comment

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

LGTM - curious, does this cause any issues? since genesis happened a long time ago I would expect this to not really matter now?

packages/config/src/chainConfig/networks/sepolia.ts Show resolved Hide resolved
@nflaig
Copy link
Member

nflaig commented Jul 16, 2024

curious, does this cause any issues? since genesis happened a long time ago I would expect this to not really matter now?

Ah yes, we cross-check the spec on the validator client and beacon node, this would fail if Lodestar is paired with another client on sepolia.

lodestar/packages/validator/src/util/params.ts

Line 93 in e57e4b7

GENESIS_DELAY: true,

nflaig
nflaig approved these changes Jul 16, 2024
View reviewed changes
Copy link
Member

@nflaig nflaig left a comment

Choose a reason for hiding this comment

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

Thanks @domiwei

@nflaig nflaig enabled auto-merge (squash) July 16, 2024 09:23
@nflaig nflaig changed the title fix: sepolia genesis delay fix: set sepolia genesis delay Jul 16, 2024
@nflaig nflaig merged commit 565bb3a into ChainSafe:unstable Jul 16, 2024
21 checks passed
@domiwei
Copy link
Contributor Author

domiwei commented Jul 17, 2024

sorry for late reply.
I am from Erigon team. Someone reported me that our CL client (caplin) can't work properly with lodestar VC, so I tried to do experiments and discovered this minor issue.

@nflaig
Copy link
Member

nflaig commented Jul 17, 2024

I tried to do experiments and discovered this minor issue.

Thanks for looking into it and fixing it on our end. This would fail with any other CL client as well on sepolia

@wemeetagain
Copy link
Member

🎉 This PR is included in v1.21.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nflaig nflaig nflaig 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.
4 participants
@domiwei @CLAassistant @nflaig @wemeetagain