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: relax BodyStage bounds #12539

Merged
merged 2 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 4 additions & 65 deletions crates/primitives-traits/src/block/body.rs
Copy link
Collaborator

Choose a reason for hiding this comment

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

this makes sense for now imo

it's easier to gradually add them back,

could you open an issue with the removed code snippet so we have it at hand and can track what else we might want to add?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, tracking in #12544

Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
//! Block body abstraction.

use alloc::{fmt, vec::Vec};
use alloc::fmt;

use alloy_consensus::{BlockHeader, Transaction, TxType};
use alloy_eips::{eip4895::Withdrawal, eip7685::Requests};
use alloy_primitives::{Address, B256};
use alloy_consensus::Transaction;

use crate::InMemorySize;

Expand All @@ -26,67 +24,8 @@ pub trait BlockBody:
{
/// Ordered list of signed transactions as committed in block.
// todo: requires trait for signed transaction
type SignedTransaction: Transaction;

/// Header type (uncle blocks).
type Header: BlockHeader;

/// Withdrawals in block.
type Withdrawals: Iterator<Item = Withdrawal>;
type Transaction: Transaction;

/// Returns reference to transactions in block.
fn transactions(&self) -> &[Self::SignedTransaction];

/// Returns `Withdrawals` in the block, if any.
// todo: branch out into extension trait
fn withdrawals(&self) -> Option<&Self::Withdrawals>;

/// Returns reference to uncle block headers.
fn ommers(&self) -> &[Self::Header];

/// Returns [`Requests`] in block, if any.
fn requests(&self) -> Option<&Requests>;

/// Calculate the transaction root for the block body.
fn calculate_tx_root(&self) -> B256;

/// Calculate the ommers root for the block body.
fn calculate_ommers_root(&self) -> B256;

/// Calculate the withdrawals root for the block body, if withdrawals exist. If there are no
/// withdrawals, this will return `None`.
// todo: can be default impl if `calculate_withdrawals_root` made into a method on
// `Withdrawals` and `Withdrawals` moved to alloy
fn calculate_withdrawals_root(&self) -> Option<B256>;

/// Recover signer addresses for all transactions in the block body.
fn recover_signers(&self) -> Option<Vec<Address>>;

/// Returns whether or not the block body contains any blob transactions.
fn has_blob_transactions(&self) -> bool {
self.transactions().iter().any(|tx| tx.ty() == TxType::Eip4844 as u8)
}

/// Returns whether or not the block body contains any EIP-7702 transactions.
fn has_eip7702_transactions(&self) -> bool {
self.transactions().iter().any(|tx| tx.ty() == TxType::Eip7702 as u8)
}

/// Returns an iterator over all blob transactions of the block
fn blob_transactions_iter(&self) -> impl Iterator<Item = &Self::SignedTransaction> + '_ {
self.transactions().iter().filter(|tx| tx.ty() == TxType::Eip4844 as u8)
}

/// Returns only the blob transactions, if any, from the block body.
fn blob_transactions(&self) -> Vec<&Self::SignedTransaction> {
self.blob_transactions_iter().collect()
}

/// Returns an iterator over all blob versioned hashes from the block body.
fn blob_versioned_hashes_iter(&self) -> impl Iterator<Item = &B256> + '_;

/// Returns all blob versioned hashes from the block body.
fn blob_versioned_hashes(&self) -> Vec<&B256> {
self.blob_versioned_hashes_iter().collect()
}
fn transactions(&self) -> &[Self::Transaction];
}
2 changes: 1 addition & 1 deletion crates/primitives-traits/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl NodePrimitives for () {
/// Helper trait that sets trait bounds on [`NodePrimitives`].
pub trait FullNodePrimitives: Send + Sync + Unpin + Clone + Default + fmt::Debug {
/// Block primitive.
type Block: FullBlock<Body: BlockBody<SignedTransaction = Self::SignedTx>>;
type Block: FullBlock<Body: BlockBody<Transaction = Self::SignedTx>>;
/// Signed version of the transaction type.
type SignedTx: FullSignedTx;
/// Transaction envelope type ID.
Expand Down
8 changes: 8 additions & 0 deletions crates/primitives/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,14 @@ impl InMemorySize for BlockBody {
}
}

impl reth_primitives_traits::BlockBody for BlockBody {
type Transaction = TransactionSigned;

fn transactions(&self) -> &[Self::Transaction] {
&self.transactions
}
}

impl From<Block> for BlockBody {
fn from(block: Block) -> Self {
Self {
Expand Down
16 changes: 16 additions & 0 deletions crates/primitives/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,22 @@ impl reth_codecs::Compact for TransactionSignedNoHash {
}
}

#[cfg(any(test, feature = "reth-codec"))]
impl reth_codecs::Compact for TransactionSigned {
Comment on lines +1061 to +1062
Copy link
Collaborator

Choose a reason for hiding this comment

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

imo this makes sense

especially if we do

#12471

this also isn't more expensive because this just does the clone somewhere else

fn to_compact<B>(&self, buf: &mut B) -> usize
where
B: bytes::BufMut + AsMut<[u8]>,
{
let tx: TransactionSignedNoHash = self.clone().into();
tx.to_compact(buf)
}

fn from_compact(buf: &[u8], len: usize) -> (Self, &[u8]) {
let (tx, buf) = TransactionSignedNoHash::from_compact(buf, len);
(tx.into(), buf)
}
}

impl From<TransactionSignedNoHash> for TransactionSigned {
fn from(tx: TransactionSignedNoHash) -> Self {
tx.with_hash()
Expand Down
16 changes: 8 additions & 8 deletions crates/stages/stages/src/stages/bodies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use std::{
};

use futures_util::TryStreamExt;
use reth_codecs::Compact;
use reth_primitives_traits::BlockBody;
use tracing::*;

use alloy_primitives::TxNumber;
Expand Down Expand Up @@ -75,8 +77,8 @@ where
+ StaticFileProviderFactory
+ StatsReader
+ BlockReader
+ BlockWriter,
D: BodyDownloader<Body = reth_primitives::BlockBody>,
+ BlockWriter<Body = D::Body>,
D: BodyDownloader<Body: BlockBody<Transaction: Compact>>,
{
/// Return the id of the stage
fn id(&self) -> StageId {
Expand Down Expand Up @@ -190,9 +192,9 @@ where
match response {
BlockResponse::Full(block) => {
// Write transactions
for transaction in &block.body.transactions {
let appended_tx_number = static_file_producer
.append_transaction(next_tx_num, &transaction.clone().into())?;
for transaction in block.body.transactions() {
let appended_tx_number =
static_file_producer.append_transaction(next_tx_num, transaction)?;

if appended_tx_number != next_tx_num {
// This scenario indicates a critical error in the logic of adding new
Expand Down Expand Up @@ -702,9 +704,7 @@ mod tests {

body.tx_num_range().try_for_each(|tx_num| {
let transaction = random_signed_tx(&mut rng);
static_file_producer
.append_transaction(tx_num, &transaction.into())
.map(drop)
static_file_producer.append_transaction(tx_num, &transaction).map(drop)
})?;

if body.tx_count != 0 {
Expand Down
2 changes: 1 addition & 1 deletion crates/stages/stages/src/test_utils/test_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ impl TestStageDB {

let res = block.body.transactions.iter().try_for_each(|body_tx| {
if let Some(txs_writer) = &mut txs_writer {
txs_writer.append_transaction(next_tx_num, &body_tx.clone().into())?;
txs_writer.append_transaction(next_tx_num, body_tx)?;
} else {
tx.put::<tables::Transactions>(next_tx_num, body_tx.clone().into())?
}
Expand Down
2 changes: 2 additions & 0 deletions crates/storage/provider/src/providers/database/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3110,6 +3110,8 @@ impl<TX: DbTxMut + DbTx + 'static, N: NodeTypes<ChainSpec: EthereumHardforks> +
impl<TX: DbTxMut + DbTx + 'static, N: NodeTypes<ChainSpec: EthereumHardforks> + 'static> BlockWriter
for DatabaseProvider<TX, N>
{
type Body = BlockBody;

/// Inserts the block into the database, always modifying the following tables:
/// * [`CanonicalHeaders`](tables::CanonicalHeaders)
/// * [`Headers`](tables::Headers)
Expand Down
4 changes: 2 additions & 2 deletions crates/storage/provider/src/providers/static_file/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use reth_db_api::models::CompactU256;
use reth_nippy_jar::{NippyJar, NippyJarError, NippyJarWriter};
use reth_primitives::{
static_file::{SegmentHeader, SegmentRangeInclusive},
Receipt, StaticFileSegment, TransactionSignedNoHash,
Receipt, StaticFileSegment,
};
use reth_storage_errors::provider::{ProviderError, ProviderResult};
use std::{
Expand Down Expand Up @@ -544,7 +544,7 @@ impl StaticFileProviderRW {
pub fn append_transaction(
&mut self,
tx_num: TxNumber,
tx: &TransactionSignedNoHash,
tx: impl Compact,
) -> ProviderResult<TxNumber> {
let start = Instant::now();
self.ensure_no_queued_prune()?;
Expand Down
7 changes: 5 additions & 2 deletions crates/storage/provider/src/traits/block.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use alloy_primitives::BlockNumber;
use reth_db_api::models::StoredBlockBodyIndices;
use reth_execution_types::{Chain, ExecutionOutcome};
use reth_primitives::{BlockBody, SealedBlockWithSenders};
use reth_primitives::SealedBlockWithSenders;
use reth_storage_errors::provider::ProviderResult;
use reth_trie::{updates::TrieUpdates, HashedPostStateSorted};
use std::ops::RangeInclusive;
Expand Down Expand Up @@ -32,6 +32,9 @@ pub trait StateReader: Send + Sync {
/// Block Writer
#[auto_impl::auto_impl(&, Arc, Box)]
pub trait BlockWriter: Send + Sync {
/// The body this writer can write.
type Body: Send + Sync;

/// Insert full block and make it canonical. Parent tx num and transition id is taken from
/// parent block in database.
///
Expand All @@ -47,7 +50,7 @@ pub trait BlockWriter: Send + Sync {
/// Bodies are passed as [`Option`]s, if body is `None` the corresponding block is empty.
fn append_block_bodies(
&self,
bodies: impl Iterator<Item = (BlockNumber, Option<BlockBody>)>,
bodies: impl Iterator<Item = (BlockNumber, Option<Self::Body>)>,
) -> ProviderResult<()>;

/// Appends a batch of sealed blocks to the blockchain, including sender information, and
Expand Down
Loading