Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add SealedBlock in reth-primitives-traits #13735

Merged
merged 107 commits into from
Jan 15, 2025

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Jan 8, 2025

closes #13626

closes #13747

This unifies BlockWithSenders and SealedBlockWithSenders into RecoveredBlock
adds various constructors and conversion between SealedBlock <-> RecoveredBlock

Changes overview

  • unifies BlockWithSenders and SealedBlockWithSenders into RecoveredBlock
  • Make SealedBlock generic over B: Block, but keep the header,body fields
  • Use lazy hashing in SealedHeader

I've kept the aliases but these are now marked as deprecated

@mattsse mattsse added the A-sdk Related to reth's use as a library label Jan 8, 2025
Comment on lines +804 to 806
pub block: Arc<SealedBlock<N::Block>>,
/// Block's senders.
pub senders: Arc<Vec<Address>>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be simplified in a next step

Copy link
Collaborator

@klkvr klkvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! looks good to me, only have nits

crates/evm/execution-types/src/chain.rs Outdated Show resolved Hide resolved
crates/exex/types/src/notification.rs Outdated Show resolved Hide resolved
crates/exex/types/src/notification.rs Outdated Show resolved Hide resolved
crates/exex/types/src/notification.rs Outdated Show resolved Hide resolved
hash: BlockHash,
/// Block hash
#[cfg_attr(feature = "serde", serde(skip))]
hash: OnceLock<BlockHash>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this type completely replace Header usage eventually? given that we now have OnceLock here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I think so

I'm hesitant to integrate this into the actual header object because for this type we want all fields pub but I don't want to expose this lock as pub.

imo this helper type isn't too bad

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I don't think we should add it to actual alloy header, just using SealedHeader as NodePrimitives::Header and avoiding additional conversions where possible

@mattsse mattsse added this pull request to the merge queue Jan 15, 2025
Merged via the queue into main with commit 83b2fb9 Jan 15, 2025
43 checks passed
@mattsse mattsse deleted the matt/introduce-sealed-block-in-reth-primitives-traits branch January 15, 2025 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sdk Related to reth's use as a library M-changelog This change should be included in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify BlockWithSenders types make SealedBlock a wrapper of hash + Block
3 participants