From bd345378b6e81bfd72aab74dccae5d1909cc3ce0 Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Mon, 6 Jan 2025 16:15:09 +0000 Subject: [PATCH] chore: remove unused `WithdrawalsProvider::latest_withdrawal` (#13671) --- Cargo.lock | 73 ------ crates/consensus/common/Cargo.toml | 3 - crates/consensus/common/src/validation.rs | 210 +----------------- .../src/providers/blockchain_provider.rs | 15 +- .../provider/src/providers/consistent.rs | 23 +- .../provider/src/providers/database/mod.rs | 9 +- .../src/providers/database/provider.rs | 12 +- crates/storage/provider/src/providers/mod.rs | 7 +- .../src/providers/static_file/manager.rs | 11 +- .../storage/provider/src/test_utils/mock.rs | 8 +- crates/storage/storage-api/src/noop.rs | 8 +- crates/storage/storage-api/src/withdrawals.rs | 10 +- 12 files changed, 15 insertions(+), 374 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3dd60fd9d..89064e4e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2605,12 +2605,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aac81fa3e28d21450aa4d2ac065992ba96a1d7303efbce51a95f4fd175b67562" -[[package]] -name = "downcast" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1435fa1053d8b2fbbe9be7e97eca7f33d37b28409959813daefc1446a14247f1" - [[package]] name = "dunce" version = "1.0.5" @@ -3318,12 +3312,6 @@ dependencies = [ "percent-encoding", ] -[[package]] -name = "fragile" -version = "2.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c2141d6d6c8512188a7891b4b01590a45f6dac67afb4f255c4124dbb86d4eaa" - [[package]] name = "fsevent-sys" version = "4.1.0" @@ -5038,32 +5026,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "mockall" -version = "0.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "39a6bfcc6c8c7eed5ee98b9c3e33adc726054389233e201c95dab2d41a3839d2" -dependencies = [ - "cfg-if", - "downcast", - "fragile", - "mockall_derive", - "predicates", - "predicates-tree", -] - -[[package]] -name = "mockall_derive" -version = "0.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25ca3004c2efe9011bd4e461bd8256445052b9615405b4f7ea43fc8ca5c20898" -dependencies = [ - "cfg-if", - "proc-macro2", - "quote", - "syn 2.0.94", -] - [[package]] name = "modular-bitfield" version = "0.11.2" @@ -5874,32 +5836,6 @@ dependencies = [ "zerocopy", ] -[[package]] -name = "predicates" -version = "3.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a5d19ee57562043d37e82899fade9a22ebab7be9cef5026b07fda9cdd4293573" -dependencies = [ - "anstyle", - "predicates-core", -] - -[[package]] -name = "predicates-core" -version = "1.0.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "727e462b119fe9c93fd0eb1429a5f7647394014cf3c04ab2c0350eeb09095ffa" - -[[package]] -name = "predicates-tree" -version = "1.0.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72dd2d6d381dfb73a193c7fca536518d7caee39fc8503f74e7dc0be0531b425c" -dependencies = [ - "predicates-core", - "termtree", -] - [[package]] name = "pretty_assertions" version = "1.4.1" @@ -6919,14 +6855,11 @@ dependencies = [ "alloy-consensus", "alloy-eips", "alloy-primitives", - "mockall", "rand 0.8.5", "reth-chainspec", "reth-consensus", - "reth-ethereum-primitives", "reth-primitives", "reth-primitives-traits", - "reth-storage-api", ] [[package]] @@ -10824,12 +10757,6 @@ dependencies = [ "windows-sys 0.59.0", ] -[[package]] -name = "termtree" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" - [[package]] name = "test-fuzz" version = "6.0.0" diff --git a/crates/consensus/common/Cargo.toml b/crates/consensus/common/Cargo.toml index c7640ff69..e551c64cc 100644 --- a/crates/consensus/common/Cargo.toml +++ b/crates/consensus/common/Cargo.toml @@ -24,8 +24,5 @@ alloy-eips.workspace = true [dev-dependencies] alloy-consensus.workspace = true -reth-storage-api.workspace = true -reth-ethereum-primitives.workspace = true rand.workspace = true -mockall = "0.13" diff --git a/crates/consensus/common/src/validation.rs b/crates/consensus/common/src/validation.rs index a6f5b2f94..67ecc886e 100644 --- a/crates/consensus/common/src/validation.rs +++ b/crates/consensus/common/src/validation.rs @@ -335,123 +335,12 @@ pub fn validate_against_parent_4844( #[cfg(test)] mod tests { use super::*; - use alloy_consensus::{Header, TxEip4844, EMPTY_OMMER_ROOT_HASH, EMPTY_ROOT_HASH}; - use alloy_eips::{ - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, - }; - use alloy_primitives::{ - hex_literal::hex, Address, BlockHash, BlockNumber, Bytes, PrimitiveSignature as Signature, - U256, - }; - use mockall::mock; + use alloy_consensus::{Header, TxEip4844}; + use alloy_eips::eip4895::Withdrawals; + use alloy_primitives::{Address, Bytes, PrimitiveSignature as Signature, U256}; use rand::Rng; use reth_chainspec::ChainSpecBuilder; - use reth_ethereum_primitives::{Transaction, TransactionSigned}; - use reth_primitives::{proofs, Account, BlockBody}; - use reth_storage_api::{ - errors::provider::ProviderResult, AccountReader, HeaderProvider, WithdrawalsProvider, - }; - use std::ops::RangeBounds; - - mock! { - WithdrawalsProvider {} - - impl WithdrawalsProvider for WithdrawalsProvider { - fn latest_withdrawal(&self) -> ProviderResult> ; - - fn withdrawals_by_block( - &self, - _id: BlockHashOrNumber, - _timestamp: u64, - ) -> ProviderResult> ; - } - } - - struct Provider { - is_known: bool, - parent: Option
, - account: Option, - withdrawals_provider: MockWithdrawalsProvider, - } - - impl Provider { - /// New provider with parent - fn new(parent: Option
) -> Self { - Self { - is_known: false, - parent, - account: None, - withdrawals_provider: MockWithdrawalsProvider::new(), - } - } - } - - impl AccountReader for Provider { - fn basic_account(&self, _address: &Address) -> ProviderResult> { - Ok(self.account) - } - } - - impl HeaderProvider for Provider { - type Header = Header; - - fn is_known(&self, _block_hash: &BlockHash) -> ProviderResult { - Ok(self.is_known) - } - - fn header(&self, _block_number: &BlockHash) -> ProviderResult> { - Ok(self.parent.clone()) - } - - fn header_by_number(&self, _num: u64) -> ProviderResult> { - Ok(self.parent.clone()) - } - - fn header_td(&self, _hash: &BlockHash) -> ProviderResult> { - Ok(None) - } - - fn header_td_by_number(&self, _number: BlockNumber) -> ProviderResult> { - Ok(None) - } - - fn headers_range( - &self, - _range: impl RangeBounds, - ) -> ProviderResult> { - Ok(vec![]) - } - - fn sealed_header( - &self, - _block_number: BlockNumber, - ) -> ProviderResult> { - Ok(None) - } - - fn sealed_headers_while( - &self, - _range: impl RangeBounds, - _predicate: impl FnMut(&SealedHeader) -> bool, - ) -> ProviderResult> { - Ok(vec![]) - } - } - - impl WithdrawalsProvider for Provider { - fn withdrawals_by_block( - &self, - _id: BlockHashOrNumber, - _timestamp: u64, - ) -> ProviderResult> { - self.withdrawals_provider.withdrawals_by_block(_id, _timestamp) - } - - fn latest_withdrawal(&self) -> ProviderResult> { - self.withdrawals_provider.latest_withdrawal() - } - } + use reth_primitives::{proofs, BlockBody, Transaction, TransactionSigned}; fn mock_blob_tx(nonce: u64, num_blobs: usize) -> TransactionSigned { let mut rng = rand::thread_rng(); @@ -474,97 +363,6 @@ mod tests { TransactionSigned::new_unhashed(request, signature) } - /// got test block - fn mock_block() -> (SealedBlock, Header) { - // https://etherscan.io/block/15867168 where transaction root and receipts root are cleared - // empty merkle tree: 0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421 - - let header = Header { - parent_hash: hex!("859fad46e75d9be177c2584843501f2270c7e5231711e90848290d12d7c6dcdd").into(), - ommers_hash: EMPTY_OMMER_ROOT_HASH, - beneficiary: hex!("4675c7e5baafbffbca748158becba61ef3b0a263").into(), - state_root: hex!("8337403406e368b3e40411138f4868f79f6d835825d55fd0c2f6e17b1a3948e9").into(), - transactions_root: EMPTY_ROOT_HASH, - receipts_root: EMPTY_ROOT_HASH, - logs_bloom: hex!("002400000000004000220000800002000000000000000000000000000000100000000000000000100000000000000021020000000800000006000000002100040000000c0004000000000008000008200000000000000000000000008000000001040000020000020000002000000800000002000020000000022010000000000000010002001000000000020200000000000001000200880000004000000900020000000000020000000040000000000000000000000000000080000000000001000002000000000000012000200020000000000000001000000000000020000010321400000000100000000000000000000000000000400000000000000000").into(), - difficulty: U256::ZERO, // total difficulty: 0xc70d815d562d3cfa955).into(), - number: 0xf21d20, - gas_limit: 0x1c9c380, - gas_used: 0x6e813, - timestamp: 0x635f9657, - extra_data: hex!("")[..].into(), - mix_hash: hex!("0000000000000000000000000000000000000000000000000000000000000000").into(), - nonce: 0x0000000000000000u64.into(), - base_fee_per_gas: 0x28f0001df.into(), - withdrawals_root: None, - blob_gas_used: None, - excess_blob_gas: None, - parent_beacon_block_root: None, - requests_hash: None, - }; - // size: 0x9b5 - - let mut parent = header.clone(); - parent.gas_used = 17763076; - parent.gas_limit = 30000000; - parent.base_fee_per_gas = Some(0x28041f7f5); - parent.number -= 1; - parent.timestamp -= 1; - - let ommers = Vec::new(); - let transactions = Vec::new(); - - ( - SealedBlock::new( - SealedHeader::seal(header), - BlockBody { transactions, ommers, withdrawals: None }, - ), - parent, - ) - } - - #[test] - fn valid_withdrawal_index() { - let chain_spec = ChainSpecBuilder::mainnet().shanghai_activated().build(); - - let create_block_with_withdrawals = |indexes: &[u64]| { - let withdrawals = Withdrawals::new( - indexes - .iter() - .map(|idx| Withdrawal { index: *idx, ..Default::default() }) - .collect(), - ); - - let header = Header { - withdrawals_root: Some(proofs::calculate_withdrawals_root(&withdrawals)), - ..Default::default() - }; - - SealedBlock::new( - SealedHeader::seal(header), - BlockBody { withdrawals: Some(withdrawals), ..Default::default() }, - ) - }; - - // Single withdrawal - let block: SealedBlock = create_block_with_withdrawals(&[1]); - assert_eq!(validate_block_pre_execution(&block, &chain_spec), Ok(())); - - // Multiple increasing withdrawals - let block = create_block_with_withdrawals(&[1, 2, 3]); - assert_eq!(validate_block_pre_execution(&block, &chain_spec), Ok(())); - let block = create_block_with_withdrawals(&[5, 6, 7, 8, 9]); - assert_eq!(validate_block_pre_execution(&block, &chain_spec), Ok(())); - let (_, parent) = mock_block(); - - // Withdrawal index should be the last withdrawal index + 1 - let mut provider = Provider::new(Some(parent)); - provider - .withdrawals_provider - .expect_latest_withdrawal() - .return_const(Ok(Some(Withdrawal { index: 2, ..Default::default() }))); - } - #[test] fn cancun_block_incorrect_blob_gas_used() { let chain_spec = ChainSpecBuilder::mainnet().cancun_activated().build(); diff --git a/crates/storage/provider/src/providers/blockchain_provider.rs b/crates/storage/provider/src/providers/blockchain_provider.rs index 1f66c7ab5..20c371cdf 100644 --- a/crates/storage/provider/src/providers/blockchain_provider.rs +++ b/crates/storage/provider/src/providers/blockchain_provider.rs @@ -461,10 +461,6 @@ impl WithdrawalsProvider for BlockchainProvider2 { ) -> ProviderResult> { self.consistent_provider()?.withdrawals_by_block(id, timestamp) } - - fn latest_withdrawal(&self) -> ProviderResult> { - self.consistent_provider()?.latest_withdrawal() - } } impl OmmersProvider for BlockchainProvider2 { @@ -1459,7 +1455,7 @@ mod tests { "Expected withdrawals_by_block to return empty list if block does not exist" ); - for block in blocks.clone() { + for block in blocks { assert_eq!( provider .withdrawals_by_block( @@ -1472,15 +1468,6 @@ mod tests { ); } - let canonical_block_num = provider.best_block_number().unwrap(); - let canonical_block = blocks.get(canonical_block_num as usize).unwrap(); - - assert_eq!( - Some(provider.latest_withdrawal()?.unwrap()), - canonical_block.body().withdrawals.clone().unwrap().pop(), - "Expected latest withdrawal to be equal to last withdrawal entry in canonical block" - ); - Ok(()) } diff --git a/crates/storage/provider/src/providers/consistent.rs b/crates/storage/provider/src/providers/consistent.rs index 46f7d7a9c..f2872352b 100644 --- a/crates/storage/provider/src/providers/consistent.rs +++ b/crates/storage/provider/src/providers/consistent.rs @@ -8,9 +8,8 @@ use crate::{ }; use alloy_consensus::{transaction::TransactionMeta, BlockHeader}; use alloy_eips::{ - eip2718::Encodable2718, - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, BlockId, BlockNumHash, BlockNumberOrTag, HashOrNumber, + eip2718::Encodable2718, eip4895::Withdrawals, BlockHashOrNumber, BlockId, BlockNumHash, + BlockNumberOrTag, HashOrNumber, }; use alloy_primitives::{ map::{hash_map, HashMap}, @@ -1132,24 +1131,6 @@ impl WithdrawalsProvider for ConsistentProvider { |block_state| Ok(block_state.block_ref().block().body().withdrawals().cloned()), ) } - - fn latest_withdrawal(&self) -> ProviderResult> { - let best_block_num = self.best_block_number()?; - - self.get_in_memory_or_storage_by_block( - best_block_num.into(), - |db_provider| db_provider.latest_withdrawal(), - |block_state| { - Ok(block_state - .block_ref() - .block() - .body() - .withdrawals() - .cloned() - .and_then(|mut w| w.pop())) - }, - ) - } } impl OmmersProvider for ConsistentProvider { diff --git a/crates/storage/provider/src/providers/database/mod.rs b/crates/storage/provider/src/providers/database/mod.rs index 8ff8ef3b7..57ec9c059 100644 --- a/crates/storage/provider/src/providers/database/mod.rs +++ b/crates/storage/provider/src/providers/database/mod.rs @@ -8,10 +8,7 @@ use crate::{ TransactionVariant, TransactionsProvider, WithdrawalsProvider, }; use alloy_consensus::transaction::TransactionMeta; -use alloy_eips::{ - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, -}; +use alloy_eips::{eip4895::Withdrawals, BlockHashOrNumber}; use alloy_primitives::{Address, BlockHash, BlockNumber, TxHash, TxNumber, B256, U256}; use core::fmt; use reth_chainspec::{ChainInfo, EthereumHardforks}; @@ -556,10 +553,6 @@ impl WithdrawalsProvider for ProviderFactory { ) -> ProviderResult> { self.provider()?.withdrawals_by_block(id, timestamp) } - - fn latest_withdrawal(&self) -> ProviderResult> { - self.provider()?.latest_withdrawal() - } } impl OmmersProvider for ProviderFactory { diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index 16c62f7c3..89b2ae5b6 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -20,11 +20,7 @@ use crate::{ TransactionsProviderExt, TrieWriter, WithdrawalsProvider, }; use alloy_consensus::{transaction::TransactionMeta, BlockHeader, Header}; -use alloy_eips::{ - eip2718::Encodable2718, - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, -}; +use alloy_eips::{eip2718::Encodable2718, eip4895::Withdrawals, BlockHashOrNumber}; use alloy_primitives::{ keccak256, map::{hash_map, B256HashMap, HashMap, HashSet}, @@ -1593,12 +1589,6 @@ impl> Withdrawals } Ok(None) } - - fn latest_withdrawal(&self) -> ProviderResult> { - let latest_block_withdrawal = self.tx.cursor_read::()?.last()?; - Ok(latest_block_withdrawal - .and_then(|(_, mut block_withdrawal)| block_withdrawal.withdrawals.pop())) - } } impl OmmersProvider for DatabaseProvider { diff --git a/crates/storage/provider/src/providers/mod.rs b/crates/storage/provider/src/providers/mod.rs index 0d37f1143..b8c7ce0c8 100644 --- a/crates/storage/provider/src/providers/mod.rs +++ b/crates/storage/provider/src/providers/mod.rs @@ -11,8 +11,7 @@ use crate::{ }; use alloy_consensus::{transaction::TransactionMeta, Header}; use alloy_eips::{ - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, BlockId, BlockNumHash, BlockNumberOrTag, + eip4895::Withdrawals, BlockHashOrNumber, BlockId, BlockNumHash, BlockNumberOrTag, }; use alloy_primitives::{Address, BlockHash, BlockNumber, TxHash, TxNumber, B256, U256}; use alloy_rpc_types_engine::ForkchoiceState; @@ -570,10 +569,6 @@ impl WithdrawalsProvider for BlockchainProvider { ) -> ProviderResult> { self.database.withdrawals_by_block(id, timestamp) } - - fn latest_withdrawal(&self) -> ProviderResult> { - self.database.latest_withdrawal() - } } impl OmmersProvider for BlockchainProvider { diff --git a/crates/storage/provider/src/providers/static_file/manager.rs b/crates/storage/provider/src/providers/static_file/manager.rs index 7f8b3e1b9..cb8be0f92 100644 --- a/crates/storage/provider/src/providers/static_file/manager.rs +++ b/crates/storage/provider/src/providers/static_file/manager.rs @@ -8,11 +8,7 @@ use crate::{ TransactionsProviderExt, WithdrawalsProvider, }; use alloy_consensus::{transaction::TransactionMeta, Header}; -use alloy_eips::{ - eip2718::Encodable2718, - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, -}; +use alloy_eips::{eip2718::Encodable2718, eip4895::Withdrawals, BlockHashOrNumber}; use alloy_primitives::{keccak256, Address, BlockHash, BlockNumber, TxHash, TxNumber, B256, U256}; use dashmap::DashMap; use notify::{RecommendedWatcher, RecursiveMode, Watcher}; @@ -1675,11 +1671,6 @@ impl WithdrawalsProvider for StaticFileProvider { // Required data not present in static_files Err(ProviderError::UnsupportedProvider) } - - fn latest_withdrawal(&self) -> ProviderResult> { - // Required data not present in static_files - Err(ProviderError::UnsupportedProvider) - } } impl> OmmersProvider for StaticFileProvider { diff --git a/crates/storage/provider/src/test_utils/mock.rs b/crates/storage/provider/src/test_utils/mock.rs index d3196b819..362bbc32d 100644 --- a/crates/storage/provider/src/test_utils/mock.rs +++ b/crates/storage/provider/src/test_utils/mock.rs @@ -6,10 +6,7 @@ use crate::{ StateRootProvider, TransactionVariant, TransactionsProvider, WithdrawalsProvider, }; use alloy_consensus::{constants::EMPTY_ROOT_HASH, transaction::TransactionMeta, Header}; -use alloy_eips::{ - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, BlockId, BlockNumberOrTag, -}; +use alloy_eips::{eip4895::Withdrawals, BlockHashOrNumber, BlockId, BlockNumberOrTag}; use alloy_primitives::{ keccak256, map::{B256HashMap, HashMap}, @@ -766,9 +763,6 @@ impl WithdrawalsProvider for MockEthProvider { ) -> ProviderResult> { Ok(None) } - fn latest_withdrawal(&self) -> ProviderResult> { - Ok(None) - } } impl OmmersProvider for MockEthProvider { diff --git a/crates/storage/storage-api/src/noop.rs b/crates/storage/storage-api/src/noop.rs index f58c1e176..3e6ed7dbd 100644 --- a/crates/storage/storage-api/src/noop.rs +++ b/crates/storage/storage-api/src/noop.rs @@ -9,10 +9,7 @@ use crate::{ TransactionVariant, TransactionsProvider, WithdrawalsProvider, }; use alloy_consensus::transaction::TransactionMeta; -use alloy_eips::{ - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, BlockId, BlockNumberOrTag, -}; +use alloy_eips::{eip4895::Withdrawals, BlockHashOrNumber, BlockId, BlockNumberOrTag}; use alloy_primitives::{ map::{B256HashMap, HashMap}, Address, BlockHash, BlockNumber, Bytes, StorageKey, StorageValue, TxHash, TxNumber, B256, U256, @@ -543,9 +540,6 @@ impl WithdrawalsProvider for NoopProvider ProviderResult> { Ok(None) } - fn latest_withdrawal(&self) -> ProviderResult> { - Ok(None) - } } impl OmmersProvider for NoopProvider { diff --git a/crates/storage/storage-api/src/withdrawals.rs b/crates/storage/storage-api/src/withdrawals.rs index 47aa49444..fdfb27aa7 100644 --- a/crates/storage/storage-api/src/withdrawals.rs +++ b/crates/storage/storage-api/src/withdrawals.rs @@ -1,10 +1,7 @@ -use alloy_eips::{ - eip4895::{Withdrawal, Withdrawals}, - BlockHashOrNumber, -}; +use alloy_eips::{eip4895::Withdrawals, BlockHashOrNumber}; use reth_storage_errors::provider::ProviderResult; -/// Client trait for fetching [Withdrawal] related data. +/// Client trait for fetching [`alloy_eips::eip4895::Withdrawal`] related data. #[auto_impl::auto_impl(&, Arc)] pub trait WithdrawalsProvider: Send + Sync { /// Get withdrawals by block id. @@ -13,7 +10,4 @@ pub trait WithdrawalsProvider: Send + Sync { id: BlockHashOrNumber, timestamp: u64, ) -> ProviderResult>; - - /// Get latest withdrawal from this block or earlier . - fn latest_withdrawal(&self) -> ProviderResult>; }