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

Migrate from SealedBlock to alloy_primitives::Sealed<T> #11449

Open
Tracked by #12576
emhane opened this issue Oct 3, 2024 · 2 comments
Open
Tracked by #12576

Migrate from SealedBlock to alloy_primitives::Sealed<T> #11449

emhane opened this issue Oct 3, 2024 · 2 comments
Labels
A-dependencies Pull requests or issues that are about dependencies A-sdk Related to reth's use as a library C-debt Refactor of code section that is hard to understand or maintain S-blocked This cannot more forward until something else changes

Comments

@emhane
Copy link
Member

emhane commented Oct 3, 2024

Describe the feature

Similar to #11123, remove type reth_primitives::SealedBlock in favour of using alloy_primitives::Sealed<T>. The new Block trait will automatically be implemented for SealedBlock, since it derefs to the inner type T. Hence any redundant methods from Block trait and impl of reth_primitives::SealedBlock should be removed. Same goes for methods in impl of Sealed<T> and alloy_primitives:Sealable. Probably there are no unique methods left on reth_primitives::SealedBlock, if there is we may need a wrapper type in reth, however we should be able to replace the reth type with

pub type SealedBlock = alloy_primitives::Sealed<reth_primitives::Block>;

/// Abstraction of block data type.
pub trait Block:
fmt::Debug
+ Clone
+ PartialEq
+ Eq
+ Default
+ serde::Serialize
+ for<'a> serde::Deserialize<'a>
+ From<(Self::Header, Self::Body)>
+ Into<(Self::Header, Self::Body)>
{
/// Header part of the block.
type Header: BlockHeader + Sealable;
/// The block's body contains the transactions in the block.
type Body: BlockBody;
/// Returns reference to [`BlockHeader`] type.
fn header(&self) -> &Self::Header;
/// Returns reference to [`BlockBody`] type.
fn body(&self) -> &Self::Body;
/// Calculate the header hash and seal the block so that it can't be changed.
fn seal_slow(self) -> SealedBlock<Self::Header, Self::Body> {
let (header, body) = self.into();
let sealed = header.seal_slow();
let (header, seal) = sealed.into_parts();
SealedBlock { header: SealedHeader::new(header, seal), body }
}
/// Seal the block with a known hash.
///
/// WARNING: This method does not perform validation whether the hash is correct.
fn seal(self, hash: B256) -> SealedBlock<Self::Header, Self::Body> {
let (header, body) = self.into();
SealedBlock { header: SealedHeader::new(header, hash), body }
}
/// Expensive operation that recovers transaction signer. See
/// [`SealedBlockWithSenders`](reth_primitives::SealedBlockWithSenders).
fn senders(&self) -> Option<Vec<Address>> {
self.body().recover_signers()
}
/// Transform into a [`BlockWithSenders`].
///
/// # Panics
///
/// If the number of senders does not match the number of transactions in the block
/// and the signer recovery for one of the transactions fails.
///
/// Note: this is expected to be called with blocks read from disk.
#[track_caller]
fn with_senders_unchecked(self, senders: Vec<Address>) -> BlockWithSenders<Self> {
self.try_with_senders_unchecked(senders).expect("stored block is valid")
}
/// Transform into a [`BlockWithSenders`] using the given senders.
///
/// If the number of senders does not match the number of transactions in the block, this falls
/// back to manually recovery, but _without ensuring that the signature has a low `s` value_.
/// See also [`TransactionSigned::recover_signer_unchecked`]
///
/// Returns an error if a signature is invalid.
#[track_caller]
fn try_with_senders_unchecked(
self,
senders: Vec<Address>,
) -> Result<BlockWithSenders<Self>, Self> {
let senders = if self.body().transactions().len() == senders.len() {
senders
} else {
let Some(senders) = self.body().recover_signers() else { return Err(self) };
senders
};
Ok(BlockWithSenders { block: self, senders })
}
/// **Expensive**. Transform into a [`BlockWithSenders`] by recovering senders in the contained
/// transactions.
///
/// Returns `None` if a transaction is invalid.
fn with_recovered_senders(self) -> Option<BlockWithSenders<Self>> {
let senders = self.senders()?;
Some(BlockWithSenders { block: self, senders })
}
/// Calculates a heuristic for the in-memory size of the [`Block`].
fn size(&self) -> usize;
}
impl<T> Block for T
where
T: ops::Deref<Target: Block>
+ fmt::Debug
+ Clone
+ PartialEq
+ Eq
+ Default
+ serde::Serialize
+ for<'a> serde::Deserialize<'a>
+ From<(<T::Target as Block>::Header, <T::Target as Block>::Body)>
+ Into<(<T::Target as Block>::Header, <T::Target as Block>::Body)>,
{
type Header = <T::Target as Block>::Header;
type Body = <T::Target as Block>::Body;
#[inline]
fn header(&self) -> &Self::Header {
self.deref().header()
}
#[inline]
fn body(&self) -> &Self::Body {
self.deref().body()
}
#[inline]
fn size(&self) -> usize {
self.deref().size()
}
}

This PR will follow #11430, which only temporarily fixes this (unblocking import of Block trait into reth_node_types).

For problems that will be encountered with Comapct on Sealed<T> ref #11442

Additional context

No response

@emhane emhane added C-debt Refactor of code section that is hard to understand or maintain A-dependencies Pull requests or issues that are about dependencies A-sdk Related to reth's use as a library labels Oct 3, 2024
@emhane emhane added the S-blocked This cannot more forward until something else changes label Oct 3, 2024
Copy link
Contributor

This issue is stale because it has been open for 21 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Oct 25, 2024
@emhane emhane removed the S-stale This issue/PR is stale and will close with no further activity label Oct 25, 2024
Copy link
Contributor

This issue is stale because it has been open for 21 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Nov 16, 2024
@emhane emhane removed the S-stale This issue/PR is stale and will close with no further activity label Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Pull requests or issues that are about dependencies A-sdk Related to reth's use as a library C-debt Refactor of code section that is hard to understand or maintain S-blocked This cannot more forward until something else changes
Projects
Status: Todo
Development

No branches or pull requests

1 participant