Add merge indicator to fork choice struct #2627
Conversation
* Remove unchecked arith from ssz_derive * Address clippy lints in block_verfication * Use safe math for is_valid_gas_limit
…dd-merge-indicator-to-fork-choice-struct
| @@ -73,6 +75,28 @@ pub fn migrate_schema<T: BeaconChainTypes>( | |||
|
|
|||
| Ok(()) | |||
| } | |||
| // Migration for adding `is_merge_complete` field to the fork choice store. | |||
| (SchemaVersion(4), SchemaVersion(5)) => { | |||
There was a problem hiding this comment.
There's a schema 5 in unstable already for checkpoint sync, so this should be 5->6 once merge-f2f gets updated for the latest unstable
paulhauner
force-pushed
the
merge-f2f
branch
from
cce6f63 to
78fd388
Compare
|
Rebasing onto I suggest doing a |
|
Whilst working on #2635, I noticed that we need access to the execution block hash for finalized block when we're setting the head. We don't have this on hand, so there's two options:
Going with (2) seems appealing. Whilst (1) only involves block read (not a state read) from the database, it's still IO that I think would be nice to avoid. Instead of storing the What are your thoughts @realbigsean? Sorry for the last minute info. |
- Add database migration for the persisted fork choice
* Update to v1.1.0-beta.4 (squash of sigp#2548) * SSZ, cached tree hash, EF tests
* Remove unchecked arith from ssz_derive * Address clippy lints in block_verfication * Use safe math for is_valid_gas_limit
…ithub.com/realbigsean/lighthouse into add-merge-indicator-to-fork-choice-struct
This reverts commit 076e624.
This reverts commit 08407b1.
|
@paulhauner I rebased as you suggested, but I'm a little confused because I'm seeing a7e866f show up in this PR's diff but it already should be in the I implemented (2) here, it wasn't much to change on my end and it seems reasonable to me 👍 |
realbigsean
added
ready-for-review
|
Moving to #2643 |
Issue Addressed
ExecutionPayload. In order to avoid loading a parent state for every block we see in gossip, this PR adds anis_merge_completeboolto theProtoNodestructso we can call
fork_choice.get_block(block.parent_root).is_merge_completeProposed Changes
PersistedForkChoiceAdditional Info
To facilitate the database migration I added the new field to
ProtoNodeand created aLegacyProtoNodestruct (which doesn't include the new field) andLegacySszContainserstruct. I initially tried to usesuperstructto create variants with and without the field but it didn't seem worth the complexity because the old struct is only going to be used during the database migration.