Skip to content

Commit

Permalink
chore!: make senders fields private (paradigmxyz#13752)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattsse authored Jan 9, 2025
1 parent 66f934b commit bf65ed4
Show file tree
Hide file tree
Showing 15 changed files with 117 additions and 56 deletions.
4 changes: 2 additions & 2 deletions crates/chain-state/src/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ impl<N: NodePrimitives> BlockState<N> {
pub fn sealed_block_with_senders(&self) -> SealedBlockWithSenders<N::Block> {
let block = self.block.block().clone();
let senders = self.block.senders().clone();
SealedBlockWithSenders { block, senders }
SealedBlockWithSenders::new_unchecked(block, senders)
}

/// Returns the hash of executed block that determines the state.
Expand Down Expand Up @@ -840,7 +840,7 @@ impl<N: NodePrimitives> ExecutedBlock<N> {
///
/// Note: this clones the block and senders.
pub fn sealed_block_with_senders(&self) -> SealedBlockWithSenders<N::Block> {
SealedBlockWithSenders { block: (*self.block).clone(), senders: (*self.senders).clone() }
SealedBlockWithSenders::new_unchecked((*self.block).clone(), (*self.senders).clone())
}

/// Returns a reference to the block's execution outcome
Expand Down
5 changes: 3 additions & 2 deletions crates/chain-state/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,10 @@ impl<N: NodePrimitives> TestBlockBuilder<N> {
) -> ExecutedBlock {
let block_with_senders = self.generate_random_block(block_number, parent_hash);

let (block, senders) = block_with_senders.split();
ExecutedBlock::new(
Arc::new(block_with_senders.block.clone()),
Arc::new(block_with_senders.senders),
Arc::new(block),
Arc::new(senders),
Arc::new(ExecutionOutcome::new(
BundleState::default(),
receipts,
Expand Down
12 changes: 5 additions & 7 deletions crates/engine/tree/src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,9 @@ where
.into_iter()
.map(|b| {
let senders = b.senders().unwrap_or_default();
OrderedSealedBlockWithSenders(SealedBlockWithSenders {
block: b,
senders,
})
OrderedSealedBlockWithSenders(SealedBlockWithSenders::new_unchecked(
b, senders,
))
})
.map(Reverse),
);
Expand Down Expand Up @@ -290,14 +289,13 @@ impl<B: Block> Ord for OrderedSealedBlockWithSenders<B> {
impl<B: Block> From<SealedBlockFor<B>> for OrderedSealedBlockWithSenders<B> {
fn from(block: SealedBlockFor<B>) -> Self {
let senders = block.senders().unwrap_or_default();
Self(SealedBlockWithSenders { block, senders })
Self(SealedBlockWithSenders::new_unchecked(block, senders))
}
}

impl<B: Block> From<OrderedSealedBlockWithSenders<B>> for SealedBlockWithSenders<B> {
fn from(value: OrderedSealedBlockWithSenders<B>) -> Self {
let senders = value.0.senders;
Self { block: value.0.block, senders }
value.0
}
}

Expand Down
19 changes: 11 additions & 8 deletions crates/engine/tree/src/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,10 +1597,11 @@ where
return Ok(None)
};

let SealedBlockWithSenders { block, senders } = self
let (block, senders) = self
.provider
.sealed_block_with_senders(hash.into(), TransactionVariant::WithHash)?
.ok_or_else(|| ProviderError::HeaderNotFound(hash.into()))?;
.ok_or_else(|| ProviderError::HeaderNotFound(hash.into()))?
.split();
let execution_output = self
.provider
.get_state(block.number())?
Expand Down Expand Up @@ -2452,7 +2453,7 @@ where

let executed: ExecutedBlock<N> = ExecutedBlock {
block: sealed_block.clone(),
senders: Arc::new(block.senders),
senders: Arc::new(block.senders().to_vec()),
execution_output: Arc::new(ExecutionOutcome::from((output, block_number))),
hashed_state: Arc::new(hashed_state),
trie: Arc::new(trie_output),
Expand Down Expand Up @@ -3002,9 +3003,11 @@ mod tests {
self.persist_blocks(
blocks
.into_iter()
.map(|b| SealedBlockWithSenders {
block: (*b.block).clone(),
senders: b.senders.to_vec(),
.map(|b| {
SealedBlockWithSenders::new_unchecked(
(*b.block).clone(),
b.senders().clone(),
)
})
.collect(),
);
Expand Down Expand Up @@ -3710,7 +3713,7 @@ mod tests {
for block in &chain_a {
test_harness.tree.state.tree_state.insert_executed(ExecutedBlock {
block: Arc::new(block.block.clone()),
senders: Arc::new(block.senders.clone()),
senders: Arc::new(block.senders().to_vec()),
execution_output: Arc::new(ExecutionOutcome::default()),
hashed_state: Arc::new(HashedPostState::default()),
trie: Arc::new(TrieUpdates::default()),
Expand All @@ -3721,7 +3724,7 @@ mod tests {
for block in &chain_b {
test_harness.tree.state.tree_state.insert_executed(ExecutedBlock {
block: Arc::new(block.block.clone()),
senders: Arc::new(block.senders.clone()),
senders: Arc::new(block.senders().to_vec()),
execution_output: Arc::new(ExecutionOutcome::default()),
hashed_state: Arc::new(HashedPostState::default()),
trie: Arc::new(TrieUpdates::default()),
Expand Down
4 changes: 2 additions & 2 deletions crates/evm/execution-types/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,13 +784,13 @@ mod tests {
let block1_hash = B256::new([15; 32]);
block1.set_block_number(1);
block1.set_hash(block1_hash);
block1.senders.push(Address::new([4; 20]));
block1.push_sender(Address::new([4; 20]));

let mut block2: SealedBlockWithSenders = Default::default();
let block2_hash = B256::new([16; 32]);
block2.set_block_number(2);
block2.set_hash(block2_hash);
block2.senders.push(Address::new([4; 20]));
block2.push_sender(Address::new([4; 20]));

let mut block_state_extended = execution_outcome1;
block_state_extended.extend(execution_outcome2);
Expand Down
2 changes: 1 addition & 1 deletion crates/exex/exex/src/backfill/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ where
let execute_start = Instant::now();

// Unseal the block for execution
let (block, senders) = block.into_components();
let (block, senders) = block.split();
let (header, body) = block.split();
let (unsealed_header, hash) = header.split();
let block = P::Block::new(unsealed_header, body).with_senders_unchecked(senders);
Expand Down
16 changes: 8 additions & 8 deletions crates/optimism/evm/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,13 @@ mod tests {

// Attempt to execute a block with one deposit and one non-deposit transaction
executor
.execute_and_verify_one(&BlockWithSenders {
block: Block {
.execute_and_verify_one(&BlockWithSenders::new_unchecked(
Block {
header,
body: BlockBody { transactions: vec![tx, tx_deposit], ..Default::default() },
},
senders: vec![addr, addr],
})
vec![addr, addr],
))
.unwrap();

let receipts = executor.receipts();
Expand Down Expand Up @@ -496,13 +496,13 @@ mod tests {

// attempt to execute an empty block with parent beacon block root, this should not fail
executor
.execute_and_verify_one(&BlockWithSenders {
block: Block {
.execute_and_verify_one(&BlockWithSenders::new_unchecked(
Block {
header,
body: BlockBody { transactions: vec![tx, tx_deposit], ..Default::default() },
},
senders: vec![addr, addr],
})
vec![addr, addr],
))
.expect("Executing a block while canyon is active should not fail");

let receipts = executor.receipts();
Expand Down
54 changes: 49 additions & 5 deletions crates/primitives/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub struct BlockWithSenders<B = Block> {
#[deref_mut]
pub block: B,
/// List of senders that match the transactions in the block
pub senders: Vec<Address>,
senders: Vec<Address>,
}

impl<B: reth_primitives_traits::Block> BlockWithSenders<B> {
Expand All @@ -105,6 +105,16 @@ impl<B: reth_primitives_traits::Block> BlockWithSenders<B> {
(block.body().transactions().len() == senders.len()).then_some(Self { block, senders })
}

/// Returns all senders of the transactions in the block.
pub fn senders(&self) -> &[Address] {
&self.senders
}

/// Returns an iterator over all senders in the block.
pub fn senders_iter(&self) -> impl Iterator<Item = &Address> {
self.senders.iter()
}

/// Seal the block with a known hash.
///
/// WARNING: This method does not perform validation whether the hash is correct.
Expand All @@ -122,7 +132,7 @@ impl<B: reth_primitives_traits::Block> BlockWithSenders<B> {

/// Split Structure to its components
#[inline]
pub fn into_components(self) -> (B, Vec<Address>) {
pub fn split(self) -> (B, Vec<Address>) {
(self.block, self.senders)
}

Expand Down Expand Up @@ -483,7 +493,7 @@ pub struct SealedBlockWithSenders<B: reth_primitives_traits::Block = Block> {
#[serde(bound = "SealedBlock<B::Header, B::Body>: Serialize + serde::de::DeserializeOwned")]
pub block: SealedBlock<B::Header, B::Body>,
/// List of senders that match transactions from block.
pub senders: Vec<Address>,
senders: Vec<Address>,
}

impl<B: reth_primitives_traits::Block> Default for SealedBlockWithSenders<B> {
Expand All @@ -493,23 +503,41 @@ impl<B: reth_primitives_traits::Block> Default for SealedBlockWithSenders<B> {
}

impl<B: reth_primitives_traits::Block> SealedBlockWithSenders<B> {
/// New sealed block with sender
pub const fn new_unchecked(
block: SealedBlock<B::Header, B::Body>,
senders: Vec<Address>,
) -> Self {
Self { block, senders }
}

/// New sealed block with sender. Return none if len of tx and senders does not match
pub fn new(block: SealedBlock<B::Header, B::Body>, senders: Vec<Address>) -> Option<Self> {
(block.body.transactions().len() == senders.len()).then_some(Self { block, senders })
}
}

impl<B: reth_primitives_traits::Block> SealedBlockWithSenders<B> {
/// Returns all senders of the transactions in the block.
pub fn senders(&self) -> &[Address] {
&self.senders
}

/// Returns an iterator over all senders in the block.
pub fn senders_iter(&self) -> impl Iterator<Item = &Address> {
self.senders.iter()
}

/// Split Structure to its components
#[inline]
pub fn into_components(self) -> (SealedBlock<B::Header, B::Body>, Vec<Address>) {
pub fn split(self) -> (SealedBlock<B::Header, B::Body>, Vec<Address>) {
(self.block, self.senders)
}

/// Returns the unsealed [`BlockWithSenders`]
#[inline]
pub fn unseal(self) -> BlockWithSenders<B> {
let (block, senders) = self.into_components();
let (block, senders) = self.split();
let (header, body) = block.split();
let header = header.unseal();
BlockWithSenders::new_unchecked(B::new(header, body), senders)
Expand Down Expand Up @@ -555,6 +583,22 @@ impl<B: reth_primitives_traits::Block> SealedBlockWithSenders<B> {
}
}

#[cfg(any(test, feature = "test-utils"))]
impl<B> SealedBlockWithSenders<B>
where
B: reth_primitives_traits::Block,
{
/// Returns a mutable reference to the recovered senders.
pub fn senders_mut(&mut self) -> &mut Vec<Address> {
&mut self.senders
}

/// Appends the sender to the list of senders.
pub fn push_sender(&mut self, sender: Address) {
self.senders.push(sender);
}
}

#[cfg(any(test, feature = "arbitrary"))]
impl<'a, B> arbitrary::Arbitrary<'a> for SealedBlockWithSenders<B>
where
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-eth-api/src/helpers/pending_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,6 @@ pub trait LoadPendingBlock:
results,
);

Ok((SealedBlockWithSenders { block: block.seal_slow(), senders }, receipts))
Ok((SealedBlockWithSenders::new_unchecked(block.seal_slow(), senders), receipts))
}
}
2 changes: 1 addition & 1 deletion crates/rpc/rpc-types-compat/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ where
// `from_block_with_transactions`, however we need to compute the length before
let block_length = block.block.length();
let transactions = block.block.body().transactions().to_vec();
let transactions_with_senders = transactions.into_iter().zip(block.senders);
let transactions_with_senders = transactions.into_iter().zip(block.senders_iter().copied());
let transactions = transactions_with_senders
.enumerate()
.map(|(idx, (tx, sender))| {
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ where
if self.disallow.contains(&message.proposer_fee_recipient) {
return Err(ValidationApiError::Blacklist(message.proposer_fee_recipient))
}
for (sender, tx) in block.senders.iter().zip(block.transactions()) {
for (sender, tx) in block.senders_iter().zip(block.transactions()) {
if self.disallow.contains(sender) {
return Err(ValidationApiError::Blacklist(*sender))
}
Expand Down
8 changes: 4 additions & 4 deletions crates/storage/provider/src/providers/blockchain_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1211,10 +1211,10 @@ mod tests {

assert_eq!(
provider.pending_block_with_senders()?,
Some(reth_primitives::SealedBlockWithSenders {
block: block.clone(),
senders: block.senders().unwrap()
})
Some(reth_primitives::SealedBlockWithSenders::new_unchecked(
block.clone(),
block.senders().unwrap()
))
);

assert_eq!(provider.pending_block_and_receipts()?, Some((block, vec![])));
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/provider/src/providers/database/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2800,7 +2800,7 @@ impl<TX: DbTxMut + DbTx + 'static, N: NodeTypesForProvider + 'static> BlockWrite

// Ensures we have all the senders for the block's transactions.
for (transaction, sender) in
block.block.body().transactions().iter().zip(block.senders.iter())
block.block.body().transactions().iter().zip(block.senders_iter())
{
let hash = transaction.tx_hash();

Expand Down
25 changes: 20 additions & 5 deletions crates/storage/provider/src/test_utils/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,10 @@ fn block1(number: BlockNumber) -> (SealedBlockWithSenders, ExecutionOutcome) {
header.parent_hash = B256::ZERO;
let block = SealedBlock::new(SealedHeader::seal(header), body);

(SealedBlockWithSenders { block, senders: vec![Address::new([0x30; 20])] }, execution_outcome)
(
SealedBlockWithSenders::new_unchecked(block, vec![Address::new([0x30; 20])]),
execution_outcome,
)
}

/// Block two that points to block 1
Expand Down Expand Up @@ -304,7 +307,10 @@ fn block2(
header.parent_hash = parent_hash;
let block = SealedBlock::new(SealedHeader::seal(header), body);

(SealedBlockWithSenders { block, senders: vec![Address::new([0x31; 20])] }, execution_outcome)
(
SealedBlockWithSenders::new_unchecked(block, vec![Address::new([0x31; 20])]),
execution_outcome,
)
}

/// Block three that points to block 2
Expand Down Expand Up @@ -368,7 +374,10 @@ fn block3(
header.parent_hash = parent_hash;
let block = SealedBlock::new(SealedHeader::seal(header), body);

(SealedBlockWithSenders { block, senders: vec![Address::new([0x31; 20])] }, execution_outcome)
(
SealedBlockWithSenders::new_unchecked(block, vec![Address::new([0x31; 20])]),
execution_outcome,
)
}

/// Block four that points to block 3
Expand Down Expand Up @@ -457,7 +466,10 @@ fn block4(
header.parent_hash = parent_hash;
let block = SealedBlock::new(SealedHeader::seal(header), body);

(SealedBlockWithSenders { block, senders: vec![Address::new([0x31; 20])] }, execution_outcome)
(
SealedBlockWithSenders::new_unchecked(block, vec![Address::new([0x31; 20])]),
execution_outcome,
)
}

/// Block five that points to block 4
Expand Down Expand Up @@ -543,5 +555,8 @@ fn block5(
header.parent_hash = parent_hash;
let block = SealedBlock::new(SealedHeader::seal(header), body);

(SealedBlockWithSenders { block, senders: vec![Address::new([0x31; 20])] }, execution_outcome)
(
SealedBlockWithSenders::new_unchecked(block, vec![Address::new([0x31; 20])]),
execution_outcome,
)
}
Loading

0 comments on commit bf65ed4

Please sign in to comment.