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

Remove the global state for HotShot height #210

Merged
merged 9 commits into from
Aug 28, 2024

Conversation

ImJeremyHe
Copy link
Member

@ImJeremyHe ImJeremyHe commented Aug 27, 2024

Remove the extra global state that we added for the hotshot block height. We don't need this anymore for following reasons:

  • Since the L1 arbitrator and block validator have the ability to look back to the previous messages, we don't need to store the hotshot height in the global state for checking the continuity of hotshot block
  • Removing the extra global state helps the nitro integration keep in line with the upstream so that we can utilize their tools, such as arbitrum-sdk, without any change.
  • This change is also friendly to the migration

This PR:

  • Remove the extra global state.
  • Remove the continuity check of hotshot block for now, this is not necessary in sovereign sequencer.
  • Also remove the staker check which is largely based on this global state. We have to figure out how to do the similar things later.

@ImJeremyHe ImJeremyHe changed the title Remove the global state for HotShot Height Remove the global state for HotShot height Aug 27, 2024
@ImJeremyHe ImJeremyHe force-pushed the jh/remove-global-state branch 5 times, most recently from 384dd42 to ded5276 Compare August 27, 2024 06:18
@ImJeremyHe ImJeremyHe marked this pull request as ready for review August 27, 2024 09:13
Copy link

@Sneh1999 Sneh1999 left a comment

Choose a reason for hiding this comment

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

@ImJeremyHe I would suggest we merge my PR first. This would make the changes in this PR cleaner and the tests will also pass for this PR

@sveitser sveitser self-requested a review August 28, 2024 11:54
Copy link
Collaborator

@sveitser sveitser left a comment

Choose a reason for hiding this comment

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

LGTM but one question. I'm also fine with not addressing this right now because we can't panic due to an error in the Message in the replay binary anyway so we will probably have to make significant changes in that region anyway.

@ImJeremyHe ImJeremyHe merged commit ae84633 into integration Aug 28, 2024
8 checks passed
@ImJeremyHe ImJeremyHe deleted the jh/remove-global-state branch August 28, 2024 14:49
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