feat: check continuity of payload attributes#346
Conversation
| attributes.push(WithL1FinalizedBlockNumber::new( | ||
| batch.block_number, | ||
| WithL2BlockNumber::new( | ||
| block.context.number, | ||
| WithBatchInfo::new(batch.index, batch.hash, attribute), | ||
| ), | ||
| )); |
There was a problem hiding this comment.
The update adds a little work when initializing the structures, but the information attached to the attribute is more explicit.
| // we reset `latest_processed_l2_block_number` to 0. | ||
| self.latest_processed_l2_block_number = 0; |
There was a problem hiding this comment.
resetting to 0 means the next batch that we receive won't be checked for contiguity.
frisitano
left a comment
frisitano
left a comment
There was a problem hiding this comment.
Left some comments inline.
crates/engine/src/future/mod.rs
Outdated
| payload_attributes_with_batch_info.index, | ||
| payload_attributes_with_batch_info.hash, | ||
| ); | ||
| let mut payload_attributes = payload_attributes_with_batch_info.inner.inner.inner.clone(); |
There was a problem hiding this comment.
This type is getting unwieldy with the requirement of .inner.inner.inner. Should we consider refactoring the base types to include the additional metadata?
crates/primitives/src/block.rs
Outdated
| } | ||
| } | ||
| /// Type alias for a wrapper type with the full L2 metadata. | ||
| pub type WithFullL2Meta<T> = WithL1FinalizedBlockNumber<WithL2BlockNumber<WithBatchInfo<T>>>; |
There was a problem hiding this comment.
Same comment as above, this is a complex type. Should we push the additional metadata into the inner type T.
There was a problem hiding this comment.
can do, I like the idea of having these composable wrappers, so we can just quickly attach some extra information to structures. But I can push this info to the T
|
@frisitano what do you think about bumping to rust edition 2024? Reth bumped it recently. |
greged93
changed the title
|
@greged93 can we close this and open a new implementation based on the new derivation pipeline pattern? |
|
closing as stale |
2 participants
Resolves #328