Fork choice `Store` contains objects cross the spec versions

[hwwhww]

Fork choice Store.blocks and Store.states

• The Store storage is an abstract data structure that clients can design their own implementations.
  • The Store.blocks filed is type of Dict[Root, BeaconBlock].
• In our spec presentation, BeaconBlock means the BeaconBlock of each spec.
  • However, right after the fork boundary, store contains both phase0.BeaconBlock and altair.BeaconBlock.
• In the view of implementation, there should be only one head store object.
  • p.s. pyspec tests could choose to execute phase0.on_block or altair.on_block by the given block slot. Both with Altair store.

Proposed solution

1. Define "Union" SignedBeaconBlock and BeaconBlock. Since it's fork choice storage and not consensus container, it's not necessary to be the SSZ Union type. Python Union might be fine enough.
2. Add upgrade_to_altair_store(store: phase0.Store) -> altair.Store helper to handle the fork boundary transition.

[ericsson49]

As far as I remember, fork choice logic depends rather on BeaconBlockHeader and doesn't really need BeaconBlockBody contents, just its hash_tree_root. And the former is less likely to change between phases (in an "incompatible" way).

There are still problems. The Store contains BeaconState too, which is altered in superseding phases, sometimes in an incompatible way (e.g. replacing fields and w/o subclassing a prior version). However, fork choice logic itself doesn't really need entire BeaconState contents, but rather balances, checkpoints, slot, perhaps something else. A more problematic here is that state_transition is invoked during fork choice, which does access BeaconState fields. And it differs from phase to phase.

This looks like a more serious problem to me, though there are methods like upgrade_to_altair. I expect that Store has to be converted when transitioning to newer phases. Basically, blocks can be children of blocks from prior phases, so corresponding BeaconStates should be retrieved and state_transition should somehow handle it.

In my opinion, in the context of a specification, converting Store with one shot looks like a better approach. Lazy handling is more like an optimization and can be tricky to implement.

In a broader perspective, I have encountered more problems which are quite similar. Basically, superseding phases refer to classes/methods from previous phases. And it can be really tricky to handle, if one wants to enforce typing rules while not hurting readability too much. At least within nominal subtyping (default in MyPy). The problem doesn't manifest itself with altair and merge, but is rather severe with sharding ("circular" dependencies between methods and classes of different phases). I expect it will be even more prominent with later phases. I'm writing an issue (perhaps a couple) about that currently.

One possible solution can be to employ structural sub-typing, which MyPy has support for, via Protocols. One problem with structural subtyping is that it misses problems like a Slot used as an Epoch (e.g. #1962), which I consider to be a severe problem. Hopefully, MyPy can handle both approaches at the same time. More powerful static analysis can model Python dynamic behavior more precisely and I'm going to explore it during my research too.

[leolara]

I am closing this issue because it seems stale. Please, do not hesitate to reopen it if this is a mistake