diff --git a/crates/storage/provider/src/providers/database/mod.rs b/crates/storage/provider/src/providers/database/mod.rs index 4273964c136e..e9da00b85059 100644 --- a/crates/storage/provider/src/providers/database/mod.rs +++ b/crates/storage/provider/src/providers/database/mod.rs @@ -402,10 +402,10 @@ mod tests { }; use reth_interfaces::test_utils::{generators, generators::random_block}; use reth_primitives::{ - hex_literal::hex, ChainSpecBuilder, PruneMode, PruneModes, SealedBlock, H256, + hex_literal::hex, ChainSpecBuilder, PruneMode, PruneModes, SealedBlock, TxNumber, H256, }; use reth_rlp::Decodable; - use std::sync::Arc; + use std::{ops::RangeInclusive, sync::Arc}; #[test] fn common_history_provider() { @@ -497,14 +497,26 @@ mod tests { let mut rng = generators::rng(); let block = random_block(&mut rng, 0, None, Some(3), None); - { + let tx_ranges: Vec> = vec![0..=0, 1..=1, 2..=2, 0..=1, 1..=2]; + for range in tx_ranges { let provider = factory.provider_rw().unwrap(); assert_matches!(provider.insert_block(block.clone(), None, None), Ok(_)); - let senders = provider.get_or_take::(0..=0); - assert_eq!(senders, Ok(vec![(0, block.body[0].recover_signer().unwrap())])); - assert_eq!(provider.transaction_sender(0), Ok(None)); + let senders = provider.get_or_take::(range.clone()); + assert_eq!( + senders, + Ok(range + .clone() + .map(|tx_number| ( + tx_number, + block.body[tx_number as usize].recover_signer().unwrap() + )) + .collect()) + ); + + let db_senders = provider.senders_by_tx_range(range); + assert_eq!(db_senders, Ok(vec![])); let result = provider.get_take_block_transaction_range::(0..=0); assert_eq!( diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index e54268530180..d811e9ab3ccc 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -34,7 +34,7 @@ use reth_primitives::{ ChainInfo, ChainSpec, Hardfork, Head, Header, PruneCheckpoint, PruneModes, PrunePart, Receipt, SealedBlock, SealedBlockWithSenders, SealedHeader, StorageEntry, TransactionMeta, TransactionSigned, TransactionSignedEcRecovered, TransactionSignedNoHash, TxHash, TxNumber, - Withdrawal, H160, H256, U256, + Withdrawal, H256, U256, }; use reth_revm_primitives::{ config::revm_spec, @@ -435,26 +435,63 @@ impl<'this, TX: DbTxMut<'this> + DbTx<'this>> DatabaseProvider<'this, TX> { self.get_or_take::(first_transaction..=last_transaction)?; // Recover senders manually if not found in db - let senders_len = senders.len(); - let transactions_len = transactions.len(); - let missing_senders = transactions_len - senders_len; - let mut senders_recovered: Vec<(u64, H160)> = (first_transaction.. - first_transaction + missing_senders as u64) - .zip( - TransactionSigned::recover_signers( - transactions.iter().take(missing_senders).map(|(_, tx)| tx).collect::>(), - missing_senders, - ) - .ok_or(BlockExecutionError::Validation( - BlockValidationError::SenderRecoveryError, - ))?, + // SAFETY: Transactions are always guaranteed to be in the database whereas + // senders might be pruned. + if senders.len() != transactions.len() { + senders.reserve(transactions.len() - senders.len()); + // Find all missing senders, their corresponding tx numbers and indexes to the original + // `senders` vector at which the recovered senders will be inserted. + let mut missing_senders = Vec::with_capacity(transactions.len() - senders.len()); + { + let mut senders = senders.iter().peekable(); + + // `transactions` contain all entries. `senders` contain _some_ of the senders for + // these transactions. Both are sorted and indexed by `TxNumber`. + // + // The general idea is to iterate on both `transactions` and `senders`, and advance + // the `senders` iteration only if it matches the current `transactions` entry's + // `TxNumber`. Otherwise, add the transaction to the list of missing senders. + for (i, (tx_number, transaction)) in transactions.iter().enumerate() { + if let Some((sender_tx_number, _)) = senders.peek() { + if sender_tx_number == tx_number { + // If current sender's `TxNumber` matches current transaction's + // `TxNumber`, advance the senders iterator. + senders.next(); + } else { + // If current sender's `TxNumber` doesn't match current transaction's + // `TxNumber`, add it to missing senders. + missing_senders.push((i, tx_number, transaction)); + } + } else { + // If there's no more senders left, but we're still iterating over + // transactions, add them to missing senders + missing_senders.push((i, tx_number, transaction)); + } + } + } + + // Recover senders + let recovered_senders = TransactionSigned::recover_signers( + missing_senders.iter().map(|(_, _, tx)| *tx).collect::>(), + missing_senders.len(), ) - .collect(); - // It's only possible to have missing senders at the beginning of the range, and not in the - // middle or in the end, so it's safe to do `senders_recovered.extend(senders.iter())` - senders_recovered.extend(senders.iter()); - senders = senders_recovered; - debug_assert_eq!(senders.len(), transactions_len, "missing one or more senders"); + .ok_or(BlockExecutionError::Validation(BlockValidationError::SenderRecoveryError))?; + + // Insert recovered senders along with tx numbers at the corresponding indexes to the + // original `senders` vector + for ((i, tx_number, _), sender) in missing_senders.into_iter().zip(recovered_senders) { + // Insert will put recovered senders at necessary positions and shift the rest + senders.insert(i, (*tx_number, sender)); + } + + // Debug assertions which are triggered during the test to ensure that all senders are + // present and sorted + debug_assert_eq!(senders.len(), transactions.len(), "missing one or more senders"); + debug_assert!( + senders.iter().tuple_windows().all(|(a, b)| a.0 < b.0), + "senders not sorted" + ); + } if TAKE { // Remove TxHashNumber diff --git a/crates/transaction-pool/src/pool/best.rs b/crates/transaction-pool/src/pool/best.rs index 5b0ab07c5684..2a57f2f22017 100644 --- a/crates/transaction-pool/src/pool/best.rs +++ b/crates/transaction-pool/src/pool/best.rs @@ -70,7 +70,7 @@ pub(crate) struct BestTransactions { pub(crate) independent: BTreeSet>, /// There might be the case where a yielded transactions is invalid, this will track it. pub(crate) invalid: HashSet, - /// Used to recieve any new pending transactions that have been added to the pool after this + /// Used to receive any new pending transactions that have been added to the pool after this /// iterator was snapshotted /// /// These new pending transactions are inserted into this iterator's pool before yielding the