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

move the overlay transition to Finalize #309

Closed

Conversation

gballet
Copy link
Owner

@gballet gballet commented Nov 23, 2023

After a review with Gary. we came to the conclusion that the overlay transition should be moved to finalize for the moment. This isn't as easy at it looks, because we need to ensure that the transition is activated before the fork block is executed. But the overlay copy itself could be done in prepare or Finalize.

This works fine on kaustinen since the transition is done at genesis time and doesn't use that function. But in order to go further, this needs to be tested with a post-genesis transition.

@@ -904,9 +904,6 @@ func (w *worker) prepareWork(genParams *generateParams) (*environment, error) {
if err != nil {
return nil, err
}
if w.chain.Config().IsPrague(header.Number, header.Time) {
core.OverlayVerkleTransition(state)
}
// Run the consensus preparation with the default or customized consensus engine.
if err := w.engine.Prepare(w.chain, header); err != nil {
Copy link
Owner Author

Choose a reason for hiding this comment

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

it's actually suspicious that the conversion here happens before Prepare when it happens just before Finalize in state_processor.go.

@gballet gballet mentioned this pull request Nov 24, 2023
7 tasks
@gballet gballet added this to the cancun-rebase milestone Jan 29, 2024
@gballet
Copy link
Owner Author

gballet commented Jan 29, 2024

Current state of the PR: it's nice to have, but it would need to be tested on the testnet, so we won't merge it before the relaunch. It also conflicts with the fact that charging the coinbase has been moved to state_transition.go in kaustinen-with-shapella so I would like to compete a rebase first before I try this one out. I'm leaving it open for now, so that I know to recreate one on the rebased/merged branch.

@gballet
Copy link
Owner Author

gballet commented Feb 15, 2024

Already node somewhere else, closing.

@gballet gballet closed this Feb 15, 2024
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.

None yet

1 participant