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(reth_primitives): Use trait for size methods in primitive types #12201

Merged
merged 15 commits into from
Nov 11, 2024
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion crates/net/downloaders/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ reth-consensus.workspace = true
reth-network-p2p.workspace = true
reth-network-peers.workspace = true
reth-primitives.workspace = true
reth-primitives-traits.workspace = true
reth-storage-api.workspace = true
reth-tasks.workspace = true

Expand Down Expand Up @@ -80,5 +81,6 @@ test-utils = [
"reth-chainspec/test-utils",
"reth-primitives/test-utils",
"reth-db-api?/test-utils",
"reth-provider/test-utils"
"reth-provider/test-utils",
"reth-primitives-traits/test-utils"
]
1 change: 1 addition & 0 deletions crates/net/downloaders/src/bodies/bodies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use reth_network_p2p::{
error::{DownloadError, DownloadResult},
};
use reth_primitives::SealedHeader;
use reth_primitives_traits::size::HeuristicSize;
use reth_storage_api::HeaderProvider;
use reth_tasks::{TaskSpawner, TokioTaskExecutor};
use std::{
Expand Down
1 change: 1 addition & 0 deletions crates/net/downloaders/src/bodies/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use reth_network_p2p::{
};
use reth_network_peers::{PeerId, WithPeerId};
use reth_primitives::{BlockBody, GotExpected, SealedBlock, SealedHeader};
use reth_primitives_traits::HeuristicSize;
use std::{
collections::VecDeque,
mem,
Expand Down
8 changes: 5 additions & 3 deletions crates/net/p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ workspace = true
[dependencies]
# reth
reth-primitives.workspace = true
reth-primitives-traits.workspace = true
reth-eth-wire-types.workspace = true
reth-consensus.workspace = true
reth-network-peers.workspace = true
Expand All @@ -32,7 +33,6 @@ tokio = { workspace = true, features = ["sync"] }
auto_impl.workspace = true
tracing.workspace = true
derive_more.workspace = true

parking_lot = { workspace = true, optional = true }

[dev-dependencies]
Expand All @@ -47,11 +47,13 @@ test-utils = [
"reth-consensus/test-utils",
"parking_lot",
"reth-network-types/test-utils",
"reth-primitives/test-utils"
"reth-primitives/test-utils",
"reth-primitives-traits/test-utils"
]
std = [
"reth-consensus/std",
"reth-primitives/std",
"alloy-eips/std",
"alloy-primitives/std"
"alloy-primitives/std",
"reth-primitives-traits/std"
]
21 changes: 12 additions & 9 deletions crates/net/p2p/src/bodies/response.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use alloy_primitives::{BlockNumber, U256};
use reth_primitives::{SealedBlock, SealedHeader};
use reth_primitives_traits::HeuristicSize;

/// The block response
#[derive(PartialEq, Eq, Debug, Clone)]
Expand All @@ -19,15 +20,6 @@ impl BlockResponse {
}
}

/// Calculates a heuristic for the in-memory size of the [`BlockResponse`].
#[inline]
pub fn size(&self) -> usize {
match self {
Self::Full(block) => SealedBlock::size(block),
Self::Empty(header) => SealedHeader::size(header),
}
}

/// Return the block number
pub fn block_number(&self) -> BlockNumber {
self.header().number
Expand All @@ -41,3 +33,14 @@ impl BlockResponse {
}
}
}

impl HeuristicSize for BlockResponse {
/// Calculates a heuristic for the in-memory size of the [`BlockResponse`].
#[inline]
fn size(&self) -> usize {
match self {
Self::Full(block) => SealedBlock::size(block),
Self::Empty(header) => SealedHeader::size(header),
}
}
}
6 changes: 5 additions & 1 deletion crates/primitives-traits/src/header/sealed.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use crate::HeuristicSize;

use super::Header;
use alloy_eips::BlockNumHash;
use alloy_primitives::{keccak256, BlockHash, Sealable};
Expand Down Expand Up @@ -58,10 +60,12 @@ impl SealedHeader {
pub fn num_hash(&self) -> BlockNumHash {
BlockNumHash::new(self.number, self.hash)
}
}

impl HeuristicSize for SealedHeader {
/// Calculates a heuristic for the in-memory size of the [`SealedHeader`].
#[inline]
pub fn size(&self) -> usize {
fn size(&self) -> usize {
self.header.size() + mem::size_of::<BlockHash>()
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/primitives-traits/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,7 @@ pub use header::{BlockHeader, Header, HeaderError, SealedHeader};
pub mod serde_bincode_compat {
pub use super::header::{serde_bincode_compat as header, serde_bincode_compat::*};
}

/// Heuristic size trait
pub mod size;
pub use size::HeuristicSize;
5 changes: 5 additions & 0 deletions crates/primitives-traits/src/size.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/// Trait for calculating a heuristic for the in-memory size of a struct.
pub trait HeuristicSize {
Copy link
Member

Choose a reason for hiding this comment

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

maybe name it SizeHeuristic, or InMemorySize or something like that

/// Returns a heuristic for the in-memory size of a struct.
fn size(&self) -> usize;
}
23 changes: 15 additions & 8 deletions crates/primitives/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use alloy_rlp::{Decodable, Encodable, RlpDecodable, RlpEncodable};
use derive_more::{Deref, DerefMut};
#[cfg(any(test, feature = "arbitrary"))]
pub use reth_primitives_traits::test_utils::{generate_valid_header, valid_header_strategy};
use reth_primitives_traits::HeuristicSize;
use serde::{Deserialize, Serialize};

/// Ethereum full block.
Expand Down Expand Up @@ -89,10 +90,12 @@ impl Block {
let senders = self.senders()?;
Some(BlockWithSenders { block: self, senders })
}
}

impl HeuristicSize for Block {
/// Calculates a heuristic for the in-memory size of the [`Block`].
#[inline]
pub fn size(&self) -> usize {
fn size(&self) -> usize {
self.header.size() + self.body.size()
}
}
Expand Down Expand Up @@ -380,12 +383,6 @@ impl SealedBlock {
Block { header: self.header.unseal(), body: self.body }
}

/// Calculates a heuristic for the in-memory size of the [`SealedBlock`].
#[inline]
pub fn size(&self) -> usize {
self.header.size() + self.body.size()
}

/// Calculates the total gas used by blob transactions in the sealed block.
pub fn blob_gas_used(&self) -> u64 {
self.blob_transactions().iter().filter_map(|tx| tx.blob_gas_used()).sum()
Expand Down Expand Up @@ -435,6 +432,14 @@ impl SealedBlock {
}
}

impl HeuristicSize for SealedBlock {
/// Calculates a heuristic for the in-memory size of the [`SealedBlock`].
#[inline]
fn size(&self) -> usize {
self.header.size() + self.body.size()
}
}

impl From<SealedBlock> for Block {
fn from(block: SealedBlock) -> Self {
block.unseal()
Expand Down Expand Up @@ -608,10 +613,12 @@ impl BlockBody {
pub fn transactions(&self) -> impl Iterator<Item = &TransactionSigned> + '_ {
self.transactions.iter()
}
}

impl HeuristicSize for BlockBody {
/// Calculates a heuristic for the in-memory size of the [`BlockBody`].
#[inline]
pub fn size(&self) -> usize {
fn size(&self) -> usize {
self.transactions.iter().map(TransactionSigned::size).sum::<usize>() +
self.transactions.capacity() * core::mem::size_of::<TransactionSigned>() +
self.ommers.iter().map(Header::size).sum::<usize>() +
Expand Down
31 changes: 17 additions & 14 deletions crates/primitives/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use once_cell as _;
#[cfg(not(feature = "std"))]
use once_cell::sync::Lazy as LazyLock;
use rayon::prelude::{IntoParallelIterator, ParallelIterator};
use reth_primitives_traits::HeuristicSize;
use serde::{Deserialize, Serialize};
use signature::{decode_with_eip155_chain_id, with_eip155_parity};
#[cfg(feature = "std")]
Expand Down Expand Up @@ -486,20 +487,6 @@ impl Transaction {
}
}

/// Calculates a heuristic for the in-memory size of the [Transaction].
#[inline]
pub fn size(&self) -> usize {
match self {
Self::Legacy(tx) => tx.size(),
Self::Eip2930(tx) => tx.size(),
Self::Eip1559(tx) => tx.size(),
Self::Eip4844(tx) => tx.size(),
Self::Eip7702(tx) => tx.size(),
#[cfg(feature = "optimism")]
Self::Deposit(tx) => tx.size(),
}
}

/// Returns true if the transaction is a legacy transaction.
#[inline]
pub const fn is_legacy(&self) -> bool {
Expand Down Expand Up @@ -571,6 +558,22 @@ impl Transaction {
}
}

impl HeuristicSize for Transaction {
/// Calculates a heuristic for the in-memory size of the [Transaction].
#[inline]
fn size(&self) -> usize {
match self {
Self::Legacy(tx) => tx.size(),
Self::Eip2930(tx) => tx.size(),
Self::Eip1559(tx) => tx.size(),
Self::Eip4844(tx) => tx.size(),
Self::Eip7702(tx) => tx.size(),
#[cfg(feature = "optimism")]
Self::Deposit(tx) => tx.size(),
}
}
}

#[cfg(any(test, feature = "reth-codec"))]
impl reth_codecs::Compact for Transaction {
// Serializes the TxType to the buffer if necessary, returning 2 bits of the type as an
Expand Down
1 change: 1 addition & 0 deletions crates/prune/prune/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ reth-errors.workspace = true
reth-provider.workspace = true
reth-tokio-util.workspace = true
reth-config.workspace = true
reth-primitives-traits.workspace = true
reth-prune-types.workspace = true
reth-static-file-types.workspace = true

Expand Down
4 changes: 3 additions & 1 deletion crates/prune/prune/src/segments/user/receipts_by_logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ use crate::{
PrunerError,
};
use reth_db::{tables, transaction::DbTxMut};
#[allow(unused_imports)]
Copy link
Member

Choose a reason for hiding this comment

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

should remove this

use reth_primitives_traits::HeuristicSize;
use reth_provider::{BlockReader, DBProvider, PruneCheckpointWriter, TransactionsProvider};
use reth_prune_types::{
PruneCheckpoint, PruneMode, PruneProgress, PrunePurpose, PruneSegment, ReceiptsLogPruneConfig,
SegmentOutput, MINIMUM_PRUNING_DISTANCE,
};
use tracing::{instrument, trace};

#[derive(Debug)]
pub struct ReceiptsByLogs {
config: ReceiptsLogPruneConfig,
Expand Down Expand Up @@ -223,6 +224,7 @@ mod tests {
use assert_matches::assert_matches;
use reth_db::tables;
use reth_db_api::{cursor::DbCursorRO, transaction::DbTx};
use reth_primitives_traits::HeuristicSize;
use reth_provider::{DatabaseProviderFactory, PruneCheckpointReader, TransactionsProvider};
use reth_prune_types::{PruneLimiter, PruneMode, PruneSegment, ReceiptsLogPruneConfig};
use reth_stages::test_utils::{StorageKind, TestStageDB};
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc-types-compat/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ alloy-consensus.workspace = true
serde.workspace = true

[dev-dependencies]
serde_json.workspace = true
serde_json.workspace = true
11 changes: 11 additions & 0 deletions crates/rpc/rpc-types-compat/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,14 @@ pub fn transaction_to_call_request(tx: TransactionSignedEcRecovered) -> Transact
authorization_list,
}
}

/// Convert [`reth_primitives::TxType`] to [`alloy_consensus::TxType`]
/// Will panics if the `TxType` is `TxType::Deposit` (Optimism Deposit transaction)
pub fn tx_type_to_alloy(tx_type: TxType) -> alloy_consensus::TxType {
u8::from(tx_type).try_into().unwrap()
}

/// Convert [`alloy_consensus::TxType`] to [`reth_primitives::TxType`]
pub fn tx_type_from_alloy(tx_type: alloy_consensus::TxType) -> TxType {
u8::from(tx_type).try_into().unwrap()
}
Copy link
Member

Choose a reason for hiding this comment

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

these changes are unrelated right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah sorry I messed up my local branch

7 changes: 5 additions & 2 deletions crates/transaction-pool/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ reth-chain-state.workspace = true
reth-chainspec.workspace = true
reth-eth-wire-types.workspace = true
reth-primitives = { workspace = true, features = ["c-kzg", "secp256k1"] }
reth-primitives-traits.workspace = true
reth-execution-types.workspace = true
reth-fs-util.workspace = true
reth-storage-api.workspace = true
Expand Down Expand Up @@ -94,7 +95,8 @@ test-utils = [
"reth-chainspec/test-utils",
"reth-primitives/test-utils",
"reth-provider/test-utils",
"revm/test-utils"
"revm/test-utils",
"reth-primitives-traits/test-utils"
]
arbitrary = [
"proptest",
Expand All @@ -107,7 +109,8 @@ arbitrary = [
"alloy-primitives/arbitrary",
"bitflags/arbitrary",
"revm/arbitrary",
"smallvec/arbitrary"
"smallvec/arbitrary",
"reth-primitives-traits/arbitrary"
]

[[bench]]
Expand Down
1 change: 1 addition & 0 deletions crates/transaction-pool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ use aquamarine as _;
use reth_eth_wire_types::HandleMempoolData;
use reth_execution_types::ChangedAccount;
use reth_primitives::{BlobTransactionSidecar, PooledTransactionsElement};
use reth_primitives_traits as _;
Copy link
Member

Choose a reason for hiding this comment

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

should remove this

use reth_storage_api::StateProviderFactory;
use std::{collections::HashSet, sync::Arc};
use tokio::sync::mpsc::Receiver;
Expand Down
4 changes: 3 additions & 1 deletion crates/transaction-pool/src/test_utils/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ use reth_primitives::{
BlobTransactionValidationError, PooledTransactionsElementEcRecovered, Signature, Transaction,
TransactionSigned, TransactionSignedEcRecovered, TxType,
};

use std::{ops::Range, sync::Arc, time::Instant, vec::IntoIter};

#[cfg(any(test, feature = "arbitrary"))]
use reth_primitives_traits::HeuristicSize;
Copy link
Member

Choose a reason for hiding this comment

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

if the use reth_primitives_traits as _; is required since this is feature gated, I'd try to instead make primitives-traits an optional dep, including it by default as a dev-dependency, and when arbitrary is enabled


/// A transaction pool implementation using [`MockOrdering`] for transaction ordering.
///
/// This type is an alias for [`TxPool<MockOrdering>`].
Expand Down
Loading