chore: add comment to PendingComponents #7979
Conversation
| /// Note: from this, one can immediately see that `verified_blobs` and `verified_data_columns` | ||
| /// are mutually exclusive. i.e. If we are verifying columns to determine a block's availability | ||
| /// we are ignoring the `verified_blobs` field. | ||
| pub struct PendingComponents<E: EthSpec> { |
There was a problem hiding this comment.
IIUC This structure will work across fork boundaries because a lot of the logic will explicitly handle the different forks at runtime. So merge_blobs if called during peerDAS epoch, will do nothing for example.
If that's correct then I think something like this:
pub enum PendingComponents<E: EthSpec> {
PostDeneb {
block_root: Hash256,
executed_block: Option<DietAvailabilityPendingExecutedBlock<E>>,
verified_blobs: RuntimeFixedVector<Option<KzgVerifiedBlob<E>>>,
span: Span,
},
PostPeerDAS {
block_root: Hash256,
executed_block: Option<DietAvailabilityPendingExecutedBlock<E>>,
verified_data_columns: Vec<KzgVerifiedCustodyDataColumn<E>>,
reconstruction_started: bool,
span: Span,
},Might express the intentions clearer and then it would be possible to panic if PendingComponents is PostPeerDAS but merge_blobs has been called for example.
There was a problem hiding this comment.
Thanks @kevaundray - Agree the above is cleaner, although we may not refactor this further at this stage as we're in the process of redesigning DA checker but we'll keep this in mind, thanks!
There was a problem hiding this comment.
Ah this is great to know -- is there currently a draft branch or doc for this refactor?
jimmygchen
left a comment
jimmygchen
left a comment
There was a problem hiding this comment.
Thanks, the added comments look good.
|
@mergify requeue |
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically |
Adds doc comment Co-Authored-By: Kevaundray Wedderburn <[email protected]> Co-Authored-By: Jimmy Chen <[email protected]>
Adds doc comment Co-Authored-By: Kevaundray Wedderburn <[email protected]> Co-Authored-By: Jimmy Chen <[email protected]>
Adds doc comment Co-Authored-By: Kevaundray Wedderburn <[email protected]> Co-Authored-By: Jimmy Chen <[email protected]>
2 participants
Issue Addressed
Adds doc comment