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

holocene: operator fee #130

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

holocene: operator fee #130

wants to merge 4 commits into from

Conversation

yuwen01
Copy link

@yuwen01 yuwen01 commented Sep 30, 2024

Overview

Updates the L1BlockInfoHolocene in op-alloy-protocol to include the new operator fee scalar and operator fee constant.

Additional Context

Design Doc
Spec.

@yuwen01 yuwen01 marked this pull request as ready for review September 30, 2024 18:56
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this looks correct to me,

pending @refcell sanity check

Copy link
Collaborator

@refcell refcell left a comment

Choose a reason for hiding this comment

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

Isthmus is a different hardfork than Holocene. We'll need to add an entirely new L1BlockInfo type for Isthmus.

This should be done by:

  • Leaving holocene types unchanged.
  • Introducing a new variant in the L1BlockInfo enum called Isthmus that wraps a new L1BlockInfoIsthmus struct.
  • Add a is_isthmus_active method to the RollupConfig with isthmus_time field.
  • Add the operator fee parsing inside try_new when creating a new L1BlockInfoTx behind a if rollup_config.is_isthmus_active(l2_block_time) branch above the holocene check since it follows the holocene hardfork.

@yuwen01
Copy link
Author

yuwen01 commented Sep 30, 2024

Sorry for the miscommunication -- since this feature is almost ready, I have verbal confirmation from tynes that it's very likely to be included in the Holocene hardfork. I'll update the spec shortly.

@refcell
Copy link
Collaborator

refcell commented Sep 30, 2024

Sorry for the miscommunication -- since this feature is almost ready, I have verbal confirmation from tynes that it's very likely to be included in the Holocene hardfork. I'll update the spec shortly.

I don't believe tynes is correct here. The Holocene feature set has been frozen for a little while now with implementation well on the way. It's most likely these changes will need to be a part of Isthmus. We have channels in our discord to discuss hardfork changes. I'd recommend joining the discord and kicking off a thread in #hf-next channel about adding these changes and linking your spec pr.

@tynes
Copy link

tynes commented Oct 1, 2024

We have general consensus on these changes being included in a future hardfork when they are ready. We should all make sure we agree on the design before saying anything is ready for inclusion

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.

4 participants