Update Blob Storage Structure

[ethDreamer]

Issue Addressed

• Change blob storage structure in database #4099

[ethDreamer]

I'm not familiar enough with how gossip / fork choice works to refactor blob_verification.rs yet. Taking a look at it briefly, I think the new structure would resemble something like:

enum AvailableBlockInner<E: EthSpec> {
    Block(Arc<SignedBeaconBlock<E>>),
    BlockAndBlob(
        SignedBeaconBlock<E>,
        SignedBlobSidecarList<E>,
    ),
}

but when I tried to do that I got a bit lost in the changes to fork choice and gossip verification. I can keep going down this rabbit hole but perhaps @realbigsean or @divagant-martian have some ideas about this?

[divagant-martian]

This makes sense to me, but @pawanjay176 is working on something that looks a bit different in #4092 (AvailableBlockInner was removed there, for instance). I liked the approach of having AvailableBlockInner requiring checks to be built, so it seems we ought to have a lil chat to decide how to continue here. Beside that, I think changing the db structure could be glued in some potentially ugly way -as long as we are all on the same page- so that blob verification can be handled in pawan's pr and not this one. What do you all think?

[ethDreamer]

I've updated this PR to make it easier to merge into other PR's without conflicts

[divagant-martian]

I see why here it makes more sense to arc the list and not the individual blobs. Before storing them in the db we have blobs arc'd individually (since they arrive individually in gossip and rpc and need to use them in a couple of places)

(Question, not suggestion) Should we still store this as an arc'd list of un-arc'd elements?

[divagant-martian]

this won't allocate so I think it's fine? Also better to differentiate between blobs not found and blobs not required

[ethDreamer]

@pawanjay176's pr seems to remove this entirely.. not sure why, perhaps we no longer have empty blobs?

[divagant-martian]

he has a enum that has a variant Blobs::None (if I remember correctly)

[realbigsean]

We no longer have to serve empty blobs via RPC (this was a relic of devnet 4) so we no longer need to do the ugly empty blob reconstruction logic. I would leave this as is and I can clean it up in pawan's PR once this is merged

[divagant-martian]

if left like this we still won't serve empty blobs, this is just an empty list. What would you do differently after pawan's?

[realbigsean]

In that PR we're removing the fallback logic completely because it includes a db read for the block to check whether we should have blobs (and can return the correct Blobs enum variant). So I think the method just returns None. I don't think it was a good thing to always fallback to the block db read, at the end of the day if we don't have the blobs the data's not available. If we do want that fallback-to-block db read logic somewhere we can add it in a new method here somewhere. But I'm not sure where it's useful anymore now that we don't have to reconstruct empty blobs.