From 93e837e28a77a890f145f95be6a75ec5a66dfb19 Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Fri, 20 Oct 2023 00:56:01 +0200 Subject: [PATCH] feat(storage): add an option to return hashed transactions in `block_with_senders` (#5095) Co-authored-by: Matthias Seitz --- crates/stages/src/stages/execution.rs | 6 ++++-- crates/storage/provider/src/lib.rs | 3 ++- .../provider/src/providers/database/mod.rs | 10 +++++++--- .../src/providers/database/provider.rs | 13 +++++++------ crates/storage/provider/src/providers/mod.rs | 14 +++++++++----- crates/storage/provider/src/test_utils/mock.rs | 8 ++++++-- crates/storage/provider/src/test_utils/noop.rs | 4 +++- crates/storage/provider/src/traits/block.rs | 18 +++++++++++++++++- crates/storage/provider/src/traits/mod.rs | 5 ++++- 9 files changed, 59 insertions(+), 22 deletions(-) diff --git a/crates/stages/src/stages/execution.rs b/crates/stages/src/stages/execution.rs index feb490015..a78631b84 100644 --- a/crates/stages/src/stages/execution.rs +++ b/crates/stages/src/stages/execution.rs @@ -19,7 +19,7 @@ use reth_primitives::{ }; use reth_provider::{ BlockReader, DatabaseProviderRW, ExecutorFactory, HeaderProvider, LatestStateProviderRef, - OriginalValuesKnown, ProviderError, + OriginalValuesKnown, ProviderError, TransactionVariant, }; use std::{ ops::RangeInclusive, @@ -144,8 +144,10 @@ impl ExecutionStage { let td = provider .header_td_by_number(block_number)? .ok_or_else(|| ProviderError::HeaderNotFound(block_number.into()))?; + + // we need the block's transactions but we don't need the transaction hashes let block = provider - .block_with_senders(block_number)? + .block_with_senders(block_number, TransactionVariant::NoHash)? .ok_or_else(|| ProviderError::BlockNotFound(block_number.into()))?; fetch_block_duration += time.elapsed(); diff --git a/crates/storage/provider/src/lib.rs b/crates/storage/provider/src/lib.rs index 02f093664..c100d5a1e 100644 --- a/crates/storage/provider/src/lib.rs +++ b/crates/storage/provider/src/lib.rs @@ -24,7 +24,8 @@ pub use traits::{ HashingWriter, HeaderProvider, HistoryWriter, PrunableBlockExecutor, PruneCheckpointReader, PruneCheckpointWriter, ReceiptProvider, ReceiptProviderIdExt, StageCheckpointReader, StageCheckpointWriter, StateProvider, StateProviderBox, StateProviderFactory, - StateRootProvider, StorageReader, TransactionsProvider, WithdrawalsProvider, + StateRootProvider, StorageReader, TransactionVariant, TransactionsProvider, + WithdrawalsProvider, }; /// Provider trait implementations. diff --git a/crates/storage/provider/src/providers/database/mod.rs b/crates/storage/provider/src/providers/database/mod.rs index fc1c56162..140de2a2f 100644 --- a/crates/storage/provider/src/providers/database/mod.rs +++ b/crates/storage/provider/src/providers/database/mod.rs @@ -3,7 +3,7 @@ use crate::{ traits::{BlockSource, ReceiptProvider}, BlockHashReader, BlockNumReader, BlockReader, ChainSpecProvider, EvmEnvProvider, HeaderProvider, ProviderError, PruneCheckpointReader, StageCheckpointReader, StateProviderBox, - TransactionsProvider, WithdrawalsProvider, + TransactionVariant, TransactionsProvider, WithdrawalsProvider, }; use reth_db::{database::Database, init_db, models::StoredBlockBodyIndices, DatabaseEnv}; use reth_interfaces::{db::LogLevel, RethError, RethResult}; @@ -246,8 +246,12 @@ impl BlockReader for ProviderFactory { self.provider()?.block_body_indices(number) } - fn block_with_senders(&self, number: BlockNumber) -> RethResult> { - self.provider()?.block_with_senders(number) + fn block_with_senders( + &self, + number: BlockNumber, + transaction_kind: TransactionVariant, + ) -> RethResult> { + self.provider()?.block_with_senders(number, transaction_kind) } fn block_range(&self, range: RangeInclusive) -> RethResult> { diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index 7208c12d7..913503f09 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -6,7 +6,7 @@ use crate::{ AccountReader, BlockExecutionWriter, BlockHashReader, BlockNumReader, BlockReader, BlockWriter, Chain, EvmEnvProvider, HashingWriter, HeaderProvider, HistoryWriter, OriginalValuesKnown, ProviderError, PruneCheckpointReader, PruneCheckpointWriter, StageCheckpointReader, - StorageReader, TransactionsProvider, WithdrawalsProvider, + StorageReader, TransactionVariant, TransactionsProvider, WithdrawalsProvider, }; use itertools::{izip, Itertools}; use reth_db::{ @@ -1039,6 +1039,7 @@ impl BlockReader for DatabaseProvider { fn block_with_senders( &self, block_number: BlockNumber, + transaction_kind: TransactionVariant, ) -> RethResult> { let Some(header) = self.header_by_number(block_number)? else { return Ok(None) }; @@ -1066,14 +1067,14 @@ impl BlockReader for DatabaseProvider { let body = transactions .into_iter() - .map(|tx| { - TransactionSigned { - // TODO: This is the fastest way right now to make everything just work with - // a dummy transaction hash. + .map(|tx| match transaction_kind { + TransactionVariant::NoHash => TransactionSigned { + // Caller explicitly asked for no hash, so we don't calculate it hash: Default::default(), signature: tx.signature, transaction: tx.transaction, - } + }, + TransactionVariant::WithHash => tx.with_hash(), }) .collect(); diff --git a/crates/storage/provider/src/providers/mod.rs b/crates/storage/provider/src/providers/mod.rs index a865f5a63..f07f9f5a0 100644 --- a/crates/storage/provider/src/providers/mod.rs +++ b/crates/storage/provider/src/providers/mod.rs @@ -4,7 +4,7 @@ use crate::{ CanonStateNotifications, CanonStateSubscriptions, ChainSpecProvider, ChangeSetReader, EvmEnvProvider, HeaderProvider, ProviderError, PruneCheckpointReader, ReceiptProvider, ReceiptProviderIdExt, StageCheckpointReader, StateProviderBox, StateProviderFactory, - TransactionsProvider, WithdrawalsProvider, + TransactionVariant, TransactionsProvider, WithdrawalsProvider, }; use reth_db::{database::Database, models::StoredBlockBodyIndices}; use reth_interfaces::{ @@ -262,12 +262,16 @@ where /// Returns the block with senders with matching number from database. /// - /// **NOTE: The transactions have invalid hashes, since they would need to be calculated on the - /// spot, and we want fast querying.** + /// **NOTE: If [TransactionVariant::NoHash] is provided then the transactions have invalid + /// hashes, since they would need to be calculated on the spot, and we want fast querying.** /// /// Returns `None` if block is not found. - fn block_with_senders(&self, number: BlockNumber) -> RethResult> { - self.database.provider()?.block_with_senders(number) + fn block_with_senders( + &self, + number: BlockNumber, + transaction_kind: TransactionVariant, + ) -> RethResult> { + self.database.provider()?.block_with_senders(number, transaction_kind) } fn block_range(&self, range: RangeInclusive) -> RethResult> { diff --git a/crates/storage/provider/src/test_utils/mock.rs b/crates/storage/provider/src/test_utils/mock.rs index 032048c62..c0d1c4129 100644 --- a/crates/storage/provider/src/test_utils/mock.rs +++ b/crates/storage/provider/src/test_utils/mock.rs @@ -4,7 +4,7 @@ use crate::{ AccountReader, BlockHashReader, BlockIdReader, BlockNumReader, BlockReader, BlockReaderIdExt, BundleStateDataProvider, ChainSpecProvider, EvmEnvProvider, HeaderProvider, ReceiptProviderIdExt, StateProvider, StateProviderBox, StateProviderFactory, StateRootProvider, - TransactionsProvider, WithdrawalsProvider, + TransactionVariant, TransactionsProvider, WithdrawalsProvider, }; use parking_lot::Mutex; use reth_db::models::StoredBlockBodyIndices; @@ -437,7 +437,11 @@ impl BlockReader for MockEthProvider { Ok(None) } - fn block_with_senders(&self, _number: BlockNumber) -> RethResult> { + fn block_with_senders( + &self, + _number: BlockNumber, + _transaction_kind: TransactionVariant, + ) -> RethResult> { Ok(None) } diff --git a/crates/storage/provider/src/test_utils/noop.rs b/crates/storage/provider/src/test_utils/noop.rs index b4e2c6b85..1ff22f98b 100644 --- a/crates/storage/provider/src/test_utils/noop.rs +++ b/crates/storage/provider/src/test_utils/noop.rs @@ -4,7 +4,8 @@ use crate::{ AccountReader, BlockHashReader, BlockIdReader, BlockNumReader, BlockReader, BlockReaderIdExt, ChainSpecProvider, ChangeSetReader, EvmEnvProvider, HeaderProvider, PruneCheckpointReader, ReceiptProviderIdExt, StageCheckpointReader, StateProvider, StateProviderBox, - StateProviderFactory, StateRootProvider, TransactionsProvider, WithdrawalsProvider, + StateProviderFactory, StateRootProvider, TransactionVariant, TransactionsProvider, + WithdrawalsProvider, }; use reth_db::models::{AccountBeforeTx, StoredBlockBodyIndices}; use reth_interfaces::RethResult; @@ -94,6 +95,7 @@ impl BlockReader for NoopProvider { fn block_with_senders( &self, _number: BlockNumber, + _transaction_kind: TransactionVariant, ) -> RethResult> { Ok(None) } diff --git a/crates/storage/provider/src/traits/block.rs b/crates/storage/provider/src/traits/block.rs index 122955f45..6378f8513 100644 --- a/crates/storage/provider/src/traits/block.rs +++ b/crates/storage/provider/src/traits/block.rs @@ -12,6 +12,16 @@ use reth_primitives::{ }; use std::ops::RangeInclusive; +/// Enum to control transaction hash inclusion. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)] +pub enum TransactionVariant { + /// Indicates that transactions should be processed without including their hashes. + NoHash, + /// Indicates that transactions should be processed along with their hashes. + #[default] + WithHash, +} + /// A helper enum that represents the origin of the requested block. /// /// This helper type's sole purpose is to give the caller more control over from where blocks can be @@ -104,8 +114,14 @@ pub trait BlockReader: /// Returns the block with senders with matching number from database. /// + /// Returns the block's transactions in the requested variant. + /// /// Returns `None` if block is not found. - fn block_with_senders(&self, number: BlockNumber) -> RethResult>; + fn block_with_senders( + &self, + number: BlockNumber, + transaction_kind: TransactionVariant, + ) -> RethResult>; /// Returns all blocks in the given inclusive range. /// diff --git a/crates/storage/provider/src/traits/mod.rs b/crates/storage/provider/src/traits/mod.rs index 474549577..ae6d36387 100644 --- a/crates/storage/provider/src/traits/mod.rs +++ b/crates/storage/provider/src/traits/mod.rs @@ -7,7 +7,10 @@ mod storage; pub use storage::StorageReader; mod block; -pub use block::{BlockExecutionWriter, BlockReader, BlockReaderIdExt, BlockSource, BlockWriter}; +pub use block::{ + BlockExecutionWriter, BlockReader, BlockReaderIdExt, BlockSource, BlockWriter, + TransactionVariant, +}; mod block_hash; pub use block_hash::BlockHashReader;