chore(sdk): blanket impl Header for Block types
#12660
Conversation
emhane
added
C-debt
emhane
changed the title
Header for Block typesHeader for Block types
| @@ -49,3 +49,53 @@ impl<T> BlockHeader for T where | |||
| + MaybeSerde | |||
| { | |||
| } | |||
|
|
|||
| /// Helper trait to implement [`BlockHeader`] functionality for all [`Block`](crate::Block) types. | |||
| pub trait Header { | |||
There was a problem hiding this comment.
isn't this identical to alloy_consensus::Header?
There was a problem hiding this comment.
yes. not possible to impl foreign trait for generic T: Block.
There was a problem hiding this comment.
if this really simplifies things a lot I'd prefer those as Block trait methods with default implementations
although doing block.header().number() is not that bad imo
There was a problem hiding this comment.
if you default impl these, you lose the ability to inline them afaik. would inline all getters, ref alloy-rs/alloy#1642
There was a problem hiding this comment.
but sure, can have identical trait methods on Block
There was a problem hiding this comment.
the whole point though is for rollups to have to avoid implementing lots of boilerplate code.
there is a reason we avoided calling .header() everywhere in the original code I believe.
emhane
force-pushed
the
emhane/block-header
branch
from
77326fb
to
cdebd41
Compare
| @@ -49,3 +49,53 @@ impl<T> BlockHeader for T where | |||
| + MaybeSerde | |||
| { | |||
| } | |||
|
|
|||
| /// Helper trait to implement [`BlockHeader`] functionality for all [`Block`](crate::Block) types. | |||
| pub trait Header { | |||
There was a problem hiding this comment.
introducing a new trait just for this feels like weird to me
Ref #12575
Since we currently rely on
Blocktype to deref toHeader, this helper trait avoids explicitly having to insertblock.header()everywhere with #12454