Conversation
eserilev
changed the title
eserilev
changed the title
…oas-data-availability-checker
| pub struct DataColumnsByRootRequest<E: EthSpec> { | ||
| /// The list of beacon block roots and column indices being requested. | ||
| pub data_column_ids: RuntimeVariableList<DataColumnsByRootIdentifier<E>>, | ||
| pub fork_name: ForkName, |
There was a problem hiding this comment.
Maybe add a comment here saying that this isn't part of the consensus object?
Mainly to avoid serializing the fork_name when sending it over the wire.
| ); | ||
|
|
||
| // If a block is in the da_checker, sync maybe awaiting for an event when block is finally | ||
| // imported. A block can become imported both after processing a block or data column. If a |
There was a problem hiding this comment.
| // imported. A block can become imported both after processing a block or data column. If a | |
| // imported. A block can become imported both after processing a block or data column. If |
There was a problem hiding this comment.
Any specific reason why we moved it from below to here other than this being cleaner?
There was a problem hiding this comment.
yeah just to clean it up a bit
| /// this trait. The associated types differ: | ||
| /// - V1: Returns `Availability<E>` containing `AvailableExecutedBlock<E>` | ||
| /// - V2: Returns `Availability<E>` containing `(Hash256, DataColumnSidecarList<E>)` (block root + columns) | ||
| pub trait AvailabilityCache<T: BeaconChainTypes>: Send + Sync { |
There was a problem hiding this comment.
Given that the da checker holds both v1 and v2 variants and we are explicitly dispatching to v1 and v2, the trait doesn't seem necessary to me.
Right now, it just seems to be documenting shared behaviour between both variants. Do you think its required?
There was a problem hiding this comment.
Yeah the trait was made to just be explicit about what functionality is shared between v1 and v2. Might be a lot of overhead just for documentation. I don't mind deleting it
3 participants
TODO
Add more details