DB migration for fork choice cleanup #4233
Assignees
Labels
v4.2.0
Q2 2023
Comments
|
Tagging @macladson since he showed some interest in this |
bors bot
pushed a commit
that referenced
this issue
May 15, 2023
## Issue Addressed #4233 ## Proposed Changes Remove the `best_justified_checkpoint` from the `PersistedForkChoiceStore` type as it is now unused. Additionally, remove the `Option`'s wrapping the `justified_checkpoint` and `finalized_checkpoint` fields on `ProtoNode` which were only present to facilitate a previous migration. Include the necessary code to facilitate the migration to a new DB schema.
|
Resolved via #4265 🎉 |
ghost
pushed a commit
to oone-world/lighthouse
that referenced
this issue
Jul 13, 2023
## Issue Addressed sigp#4233 ## Proposed Changes Remove the `best_justified_checkpoint` from the `PersistedForkChoiceStore` type as it is now unused. Additionally, remove the `Option`'s wrapping the `justified_checkpoint` and `finalized_checkpoint` fields on `ProtoNode` which were only present to facilitate a previous migration. Include the necessary code to facilitate the migration to a new DB schema.
Woodpile37
pushed a commit
to Woodpile37/lighthouse
that referenced
this issue
Jan 6, 2024
## Issue Addressed sigp#4233 ## Proposed Changes Remove the `best_justified_checkpoint` from the `PersistedForkChoiceStore` type as it is now unused. Additionally, remove the `Option`'s wrapping the `justified_checkpoint` and `finalized_checkpoint` fields on `ProtoNode` which were only present to facilitate a previous migration. Include the necessary code to facilitate the migration to a new DB schema.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
In #3962 we implemented some spec changes which simplified fork choice. As described in the comment of that issue, I created some DB migration debt. We also had some existing DB migration debt and with soaring interest rates it's never been a better time to pay it off.
There are two debts to pay off:
best_justified_checkpoint(debt from [Merged by Bors] - Fork choice modifications and cleanup #3962)Options around the justified and finalized checkpoints onProtoNode(existing debt introduced in [Merged by Bors] - v1.1.6 Fork Choice changes #2822)Solving (1) is quite straight forward; we don't need the
best_justified_checkpointfield any more so create a newsuperstructvariant ofPersistedForkChoiceStoreand create a migration that ignores that field on any earlier variants.(2) is a little more involved, however not too bad. The justified and finalized checkpoint fields don't need to be an option anymore. I would delete the option and then follow the errors upstream. Since we introduced those options in #2822 (the version at that time was v2.0.1), I don't think there's any need for us to handle the migration. We can just refuse to migrate a database from a pre-merge version to a post-capella version. I think it's unreasonable for anyone to expect that migration to work. We will have to some simple legwork to figure out the DB version in the next release that followed #2822.
So, in summary I think the DB migration can be quite simple. It just needs to drop the
best_justified_checkpointfield and refuse to migrate from the DB version that was released with #2822.The text was updated successfully, but these errors were encountered: