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

chore!: make SealedBlock.header field private #13646

Merged
merged 12 commits into from
Jan 7, 2025
Merged
9 changes: 6 additions & 3 deletions crates/chain-state/src/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,11 @@ impl<N: NodePrimitives> CanonicalInMemoryState<N> {
}

/// Returns the header corresponding to the given hash.
pub fn header_by_hash(&self, hash: B256) -> Option<SealedHeader<N::BlockHeader>> {
self.state_by_hash(hash).map(|block| block.block_ref().block.header.clone())
pub fn header_by_hash(
&self,
hash: B256,
) -> Option<SealedBlockFor<<N as NodePrimitives>::Block>> {
self.state_by_hash(hash).map(|block| block.block_ref().block.deref().clone())
}

/// Clears all entries in the in memory state.
Expand Down Expand Up @@ -1321,7 +1324,7 @@ mod tests {
assert_eq!(state.pending_header().unwrap(), block2.block().header().clone());

// Check the pending sealed header
assert_eq!(state.pending_sealed_header().unwrap(), block2.block().header.clone());
assert_eq!(state.pending_sealed_header().unwrap(), block2.block().deref().clone());

// Check the pending block with senders
assert_eq!(
Expand Down
3 changes: 2 additions & 1 deletion crates/evm/execution-types/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use reth_primitives::{
use reth_primitives_traits::{Block, BlockBody, NodePrimitives, SignedTransaction};
use reth_trie::updates::TrieUpdates;
use revm::db::BundleState;
use std::ops::Deref;

/// A chain of blocks and their final state.
///
Expand Down Expand Up @@ -91,7 +92,7 @@ impl<N: NodePrimitives> Chain<N> {

/// Returns an iterator over all headers in the block with increasing block numbers.
pub fn headers(&self) -> impl Iterator<Item = SealedHeader<N::BlockHeader>> + '_ {
self.blocks.values().map(|block| block.header.clone())
self.blocks.values().map(|block| block.as_sealed_header().clone())
}

/// Get cached trie updates for this chain.
Expand Down
3 changes: 2 additions & 1 deletion crates/net/downloaders/src/bodies/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use reth_primitives_traits::InMemorySize;
use std::{
collections::VecDeque,
mem,
ops::Deref,
pin::Pin,
sync::Arc,
task::{ready, Context, Poll},
Expand Down Expand Up @@ -194,7 +195,7 @@ where
// Body is invalid, put the header back and return an error
let hash = block.hash();
let number = block.number();
self.pending_headers.push_front(block.header);
self.pending_headers.push_front(block.deref());
return Err(DownloadError::BodyValidation {
hash,
number,
Expand Down
8 changes: 6 additions & 2 deletions crates/net/downloaders/src/test_utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ use alloy_primitives::B256;
use futures::SinkExt;
use reth_primitives::{BlockBody, SealedHeader};
use reth_testing_utils::generators::{self, random_block_range, BlockRangeParams};
use std::{collections::HashMap, io::SeekFrom, ops::RangeInclusive};
use std::{
collections::HashMap,
io::SeekFrom,
ops::{Deref, RangeInclusive},
};
use tokio::{fs::File, io::AsyncSeekExt};
use tokio_util::codec::FramedWrite;

Expand All @@ -28,7 +32,7 @@ pub(crate) fn generate_bodies(
BlockRangeParams { parent: Some(B256::ZERO), tx_count: 0..2, ..Default::default() },
);

let headers = blocks.iter().map(|block| block.header.clone()).collect();
let headers = blocks.iter().map(|block| block.deref().clone()).collect();
let bodies = blocks.into_iter().map(|block| (block.hash(), block.into_body())).collect();

(headers, bodies)
Expand Down
5 changes: 2 additions & 3 deletions crates/net/p2p/src/bodies/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use alloy_consensus::BlockHeader;
use alloy_primitives::{BlockNumber, U256};
use reth_primitives::{BlockBody, SealedBlock, SealedHeader};
use reth_primitives_traits::InMemorySize;

/// The block response
#[derive(PartialEq, Eq, Debug, Clone)]
pub enum BlockResponse<H, B = BlockBody> {
Expand All @@ -17,9 +16,9 @@ where
H: BlockHeader,
{
/// Return the reference to the response header
pub const fn header(&self) -> &SealedHeader<H> {
pub fn header(&self) -> &SealedHeader<H> {
match self {
Self::Full(block) => &block.header,
Self::Full(block) => block.as_sealed_header(),
Self::Empty(header) => header,
}
}
Expand Down
7 changes: 7 additions & 0 deletions crates/primitives-traits/src/header/sealed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ impl<H> SealedHeader<H> {
pub fn split(self) -> (H, BlockHash) {
(self.header, self.hash)
}

/// Returns a reference to self.
/// This is useful when you need to convert from a dereferenced header back to SealedHeader.
#[inline]
pub const fn as_sealed_header(&self) -> &SealedHeader<H> {
self
}
Copy link
Member

Choose a reason for hiding this comment

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

This method doesn't seem necessary, since it's just returning self?

}

impl<H: Sealable> SealedHeader<H> {
Expand Down
2 changes: 1 addition & 1 deletion crates/primitives/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ pub struct SealedBlock<H = Header, B = BlockBody> {
/// Locked block header.
#[deref]
#[deref_mut]
pub header: SealedHeader<H>,
header: SealedHeader<H>,
/// Block body.
body: B,
}
Expand Down
64 changes: 33 additions & 31 deletions crates/storage/provider/src/providers/blockchain_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ mod tests {
};
use revm::db::BundleState;
use std::{
ops::{Bound, Range, RangeBounds},
ops::{Bound, Deref, Range, RangeBounds},
sync::Arc,
time::Instant,
};
Expand Down Expand Up @@ -952,9 +952,9 @@ mod tests {
let finalized_block = blocks.get(block_count - 3).unwrap();

// Set the canonical head, safe, and finalized blocks
provider.set_canonical_head(canonical_block.header.clone());
provider.set_safe(safe_block.header.clone());
provider.set_finalized(finalized_block.header.clone());
provider.set_canonical_head(canonical_block.as_sealed_header().clone());
provider.set_safe(safe_block.as_sealed_header().clone());
provider.set_finalized(finalized_block.as_sealed_header().clone());

Ok((provider, database_blocks.clone(), in_memory_blocks.clone(), receipts))
}
Expand Down Expand Up @@ -1361,7 +1361,7 @@ mod tests {
let in_memory_block = in_memory_blocks.last().unwrap().clone();
// make sure that the finalized block is on db
let finalized_block = database_blocks.get(database_blocks.len() - 3).unwrap();
provider.set_finalized(finalized_block.header.clone());
provider.set_finalized(finalized_block.deref().clone());

let blocks = [database_blocks, in_memory_blocks].concat();

Expand All @@ -1380,7 +1380,7 @@ mod tests {
blocks
.iter()
.take_while(|header| header.number <= 8)
.map(|b| b.header.clone())
.map(|b| b.deref().clone())
.collect::<Vec<_>>()
);

Expand Down Expand Up @@ -1561,38 +1561,38 @@ mod tests {
let block_number = database_block.number;
assert_eq!(
provider.header_by_number_or_tag(block_number.into()).unwrap(),
Some(database_block.header.clone().unseal())
Some(database_block.deref().clone().unseal())
);
assert_eq!(
provider.sealed_header_by_number_or_tag(block_number.into()).unwrap(),
Some(database_block.header)
provider.sealed_header_by_number_or_tag(block_number.into())?,
Some(database_block.deref().clone())
);

assert_eq!(
provider.header_by_number_or_tag(BlockNumberOrTag::Latest).unwrap(),
Some(canonical_block.header.clone().unseal())
Some(canonical_block.deref().clone().unseal())
);
assert_eq!(
provider.sealed_header_by_number_or_tag(BlockNumberOrTag::Latest).unwrap(),
Some(canonical_block.header)
Some(canonical_block.deref().clone())
);

assert_eq!(
provider.header_by_number_or_tag(BlockNumberOrTag::Safe).unwrap(),
Some(safe_block.header.clone().unseal())
Some(safe_block.deref().clone().unseal())
);
assert_eq!(
provider.sealed_header_by_number_or_tag(BlockNumberOrTag::Safe).unwrap(),
Some(safe_block.header)
Some(safe_block.deref().clone())
);

assert_eq!(
provider.header_by_number_or_tag(BlockNumberOrTag::Finalized).unwrap(),
Some(finalized_block.header.clone().unseal())
Some(finalized_block.deref().clone().unseal())
);
assert_eq!(
provider.sealed_header_by_number_or_tag(BlockNumberOrTag::Finalized).unwrap(),
Some(finalized_block.header)
Some(finalized_block.deref().clone())
);

Ok(())
Expand All @@ -1616,41 +1616,41 @@ mod tests {

assert_eq!(
provider.header_by_id(block_number.into()).unwrap(),
Some(database_block.header.clone().unseal())
Some(database_block.deref().clone().unseal())
);
assert_eq!(
provider.sealed_header_by_id(block_number.into()).unwrap(),
Some(database_block.header.clone())
Some(database_block.deref().clone())
);

assert_eq!(
provider.header_by_id(block_hash.into()).unwrap(),
Some(database_block.header.clone().unseal())
Some(database_block.deref().clone().unseal())
);
assert_eq!(
provider.sealed_header_by_id(block_hash.into()).unwrap(),
Some(database_block.header)
Some(database_block.deref().clone())
);

let block_number = in_memory_block.number;
let block_hash = in_memory_block.hash();

assert_eq!(
provider.header_by_id(block_number.into()).unwrap(),
Some(in_memory_block.header.clone().unseal())
Some(in_memory_block.deref().clone().unseal())
);
assert_eq!(
provider.sealed_header_by_id(block_number.into()).unwrap(),
Some(in_memory_block.header.clone())
Some(in_memory_block.deref().clone())
);

assert_eq!(
provider.header_by_id(block_hash.into()).unwrap(),
Some(in_memory_block.header.clone().unseal())
Some(in_memory_block.deref().clone().unseal())
);
assert_eq!(
provider.sealed_header_by_id(block_hash.into()).unwrap(),
Some(in_memory_block.header)
Some(in_memory_block.deref().clone())
);

Ok(())
Expand Down Expand Up @@ -2036,7 +2036,7 @@ mod tests {
);
// test state by block tag for safe block
let safe_block = in_memory_blocks[in_memory_blocks.len() - 2].clone();
in_memory_provider.canonical_in_memory_state.set_safe(safe_block.header.clone());
in_memory_provider.canonical_in_memory_state.set_safe(safe_block.deref().clone());
assert_eq!(
safe_block.hash(),
in_memory_provider
Expand All @@ -2046,7 +2046,7 @@ mod tests {
);
// test state by block tag for finalized block
let finalized_block = in_memory_blocks[in_memory_blocks.len() - 3].clone();
in_memory_provider.canonical_in_memory_state.set_finalized(finalized_block.header.clone());
in_memory_provider.canonical_in_memory_state.set_finalized(finalized_block.deref().clone());
assert_eq!(
finalized_block.hash(),
in_memory_provider
Expand Down Expand Up @@ -2119,11 +2119,11 @@ mod tests {

// Set the safe block in memory
let safe_block = in_memory_blocks[in_memory_blocks.len() - 2].clone();
provider.canonical_in_memory_state.set_safe(safe_block.header.clone());
provider.canonical_in_memory_state.set_safe(safe_block.deref().clone());

// Set the finalized block in memory
let finalized_block = in_memory_blocks[in_memory_blocks.len() - 3].clone();
provider.canonical_in_memory_state.set_finalized(finalized_block.header.clone());
provider.canonical_in_memory_state.set_finalized(finalized_block.deref().clone());

// Verify the pending block number and hash
assert_eq!(
Expand Down Expand Up @@ -2337,8 +2337,10 @@ mod tests {
// todo(joshie) add canonical_hashes_range below after changing its interface into range
// instead start end
test_by_block_range!([
(headers_range, |block: &SealedBlock| block.header().clone()),
(sealed_headers_range, |block: &SealedBlock| block.header.clone()),
// For headers_range, we need to compare the raw header fields since Header may not
// implement PartialEq
(headers_range, |block: &SealedBlock| block.header().clone().into()),
(sealed_headers_range, |block: &SealedBlock| block.deref().clone()),
(block_range, |block: &SealedBlock| block.clone().unseal()),
(block_with_senders_range, |block: &SealedBlock| block
.clone()
Expand Down Expand Up @@ -2471,7 +2473,7 @@ mod tests {
header_by_number,
|block: &SealedBlock, _: TxNumber, _: B256, _: &Vec<Vec<Receipt>>| (
block.number,
Some(block.header.header().clone())
Some(block.as_sealed_header().clone())
),
u64::MAX
),
Expand All @@ -2480,7 +2482,7 @@ mod tests {
sealed_header,
|block: &SealedBlock, _: TxNumber, _: B256, _: &Vec<Vec<Receipt>>| (
block.number,
Some(block.header.clone())
Some(block.as_sealed_header().clone())
),
u64::MAX
),
Expand Down
12 changes: 6 additions & 6 deletions crates/storage/provider/src/providers/consistent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,15 +632,15 @@ impl<N: ProviderNodeTypes> HeaderProvider for ConsistentProvider<N> {
self.get_in_memory_or_storage_by_block(
(*block_hash).into(),
|db_provider| db_provider.header(block_hash),
|block_state| Ok(Some(block_state.block_ref().block().header.header().clone())),
|block_state| Ok(Some(block_state.block_ref().block().header().clone())),
)
}

fn header_by_number(&self, num: BlockNumber) -> ProviderResult<Option<Self::Header>> {
self.get_in_memory_or_storage_by_block(
num.into(),
|db_provider| db_provider.header_by_number(num),
|block_state| Ok(Some(block_state.block_ref().block().header.header().clone())),
|block_state| Ok(Some(block_state.block_ref().block().header().clone())),
)
}

Expand Down Expand Up @@ -682,7 +682,7 @@ impl<N: ProviderNodeTypes> HeaderProvider for ConsistentProvider<N> {
self.get_in_memory_or_storage_by_block_range_while(
range,
|db_provider, range, _| db_provider.headers_range(range),
|block_state, _| Some(block_state.block_ref().block().header.header().clone()),
|block_state, _| Some(block_state.block_ref().block().header().clone()),
|_| true,
)
}
Expand All @@ -694,7 +694,7 @@ impl<N: ProviderNodeTypes> HeaderProvider for ConsistentProvider<N> {
self.get_in_memory_or_storage_by_block(
number.into(),
|db_provider| db_provider.sealed_header(number),
|block_state| Ok(Some(block_state.block_ref().block().header.clone())),
|block_state| Ok(Some(block_state.block_ref().block().clone())),
)
}

Expand All @@ -705,7 +705,7 @@ impl<N: ProviderNodeTypes> HeaderProvider for ConsistentProvider<N> {
self.get_in_memory_or_storage_by_block_range_while(
range,
|db_provider, range, _| db_provider.sealed_headers_range(range),
|block_state, _| Some(block_state.block_ref().block().header.clone()),
|block_state, _| Some(block_state.block_ref().block().header().clone()),
Copy link
Member

Choose a reason for hiding this comment

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

it looks like the deref coercion causes this to call the SealedHeader::header method, which returns an unsealed header. To fix this we could add a method called header_ref to our SealedBlock type, and then clone.

Copy link
Collaborator

Choose a reason for hiding this comment

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

makes sense

|_| true,
)
}
Expand All @@ -719,7 +719,7 @@ impl<N: ProviderNodeTypes> HeaderProvider for ConsistentProvider<N> {
range,
|db_provider, range, predicate| db_provider.sealed_headers_while(range, predicate),
|block_state, predicate| {
let header = &block_state.block_ref().block().header;
let header = &block_state.block_ref().block().header();
predicate(header).then(|| header.clone())
},
predicate,
Expand Down
Loading
Loading