Skip to content

Conversation

@rolfyone
Copy link
Contributor

  • added electra required fields
  • added missing fields from late block reorg which are phase0
  • added missing MAX_BLOBS_PER_BLOCK which should have been in deneb.

Left MAX_PAYLOAD_SIZE and removal of MAX_CHUNK_SIZE for #97
Left the electra fork epoch for #98

the REORG_* fields don't affect some clients as they haven't implemented late block reorg, but they should definitely be included in a config for the testnet. I'm not sure how we got away without MAX_BLOBS_PER_BLOCK for deneb.

Copy link

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Looks good!

I'm not sure how we got away without MAX_BLOBS_PER_BLOCK for deneb.

Just for context: for Lighthouse we maintain our own copies of the testnet configs in our repo. We add the fields we need there, and we also have defaults for all the new fork fields so that we can maintain backwards compatibility with old configs that omit them. So, unless a param is changed from default, we don't really need it.

@rolfyone
Copy link
Contributor Author

Just for context: for Lighthouse we maintain our own copies of the testnet configs in our repo. We add the fields we need there, and we also have defaults for all the new fork fields so that we can maintain backwards compatibility with old configs that omit them. So, unless a param is changed from default, we don't really need it.

We're almost ready for late block reorg so not having these fields would definitely be sub optimal, but we can maintain our own config, it just seems pointless to have an incomplete testnet config on a repo supposed to house the testnet config...

@michaelsproul
Copy link

Yeah, I agree we should merge it. Just wanted to give context for why LH isn't broken without these changes

@tersec tersec merged commit ca07b8f into eth-clients:main Feb 24, 2025
1 check passed
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