-
Notifications
You must be signed in to change notification settings - Fork 44
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
Executor block by block #909
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really nice and clean. Great work on chopping out a lot of the cruft from the original WIP branch and moving logic back into the legacy executor verifier code, far cleaner and makes a lot more sense having it there anyway.
I've left some comments for some bits that I didn't quite follow to open up the discussion. The part on handling the l1 info tree during unwinds is a hot one that we definitely want to nail down before merging, happy to help there if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small thing around the logging
* l1infotree - highest seen + tiny refactor * l1infotreeindexprogress * remove useless param * update how l1infotreeindexprogress is stored
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good after reviewing the latest commits
No description provided.