is_data_available should be in fork-choice

[djrtwo]

assert is_data_available should not livein the state transition validation logic. This seems very much a fork-choice consideration. Both places have the same general effect but:

1. This puts a hidden input on the state transition function (BlobSideCar) instead of it being a function of only (pre_state, block). So as a function call, we've degraded it's integrity and mixed layers/concerns which we've actively avoided in all other past considerations in the beacon chain functionality.
2. it's also weird to have a temporally aware condition in the state transition function. That is -- you only need to validate the sidecar availability once. After that, you don't necessarily validate it again nor can! (e.g. if now outside of the prune window).

This shouldn't affect client implementations at all, but this more appropriately (in sidecar and DAS form) lives in the fork-choice in on_block.

[terencechain]

I suppose another place for this is in the p2p-interface spec, where we don't allow block import unless is_data_available is true. I don't have a strong opinion on where to put this as long as it's not part of state transition. Putting it in fork-choice feels future-proof and nicer with danksharding

[mkalinin]

The on_block handler would the right place for the check as DA property is aside to the state transition validity. Also, there is a consensus around not applying a block to the fork choice unless data is available which can be stated in a form of comment in the on_block spec.

[terencechain]

That assumes we are ok to gossip blocks with unavailable blobs which is ok. I think this mostly is for syncing by range

[mkalinin]

That assumes we are ok to gossip blocks with unavailable blobs which is ok. I think this mostly is for syncing by range

It might even be a plus given that we may end up with uncoupled block and blob gossip

[djrtwo]

There was no opposition on the prior CL call -- I'd like to get this into the release this friday

[Inphi]

That assumes we are ok to gossip blocks with unavailable blobs which is ok. I think this mostly is for syncing by range

It might even be a plus given that we may end up with uncoupled block and blob gossip

Sounds like this will be the case if we move da checks to fc. This seems pretty consequential so I'd like to get one last round of comments on this during the call.

[djrtwo]

closed via #3185