From d7063cee15bc059c56e95f606a5626a659298f18 Mon Sep 17 00:00:00 2001 From: Jon C Date: Mon, 30 Jun 2025 22:09:46 +0200 Subject: [PATCH] accounts-db: Remove usage of RentCollector #### Problem The RentCollector type is included in accounts-db types, but it's not actually used for anything. The only place it *is* used is to figure out the number of slots in an epoch, but it looks like that call site only ever passes in `0` anyway, from `RentCollector::default()`. #### Summary of changes Remove RentCollector from types and interfaces in accounts-db. --- accounts-bench/src/main.rs | 2 - accounts-db/benches/accounts.rs | 4 +- accounts-db/src/accounts_db.rs | 16 ++++---- .../src/accounts_db/scan_account_storage.rs | 5 +-- accounts-db/src/accounts_db/tests.rs | 40 ++++++------------- accounts-db/src/accounts_hash.rs | 6 +-- core/src/accounts_hash_verifier.rs | 10 ++++- core/tests/epoch_accounts_hash.rs | 2 +- ledger-tool/src/main.rs | 1 - runtime/src/accounts_background_service.rs | 2 +- runtime/src/bank.rs | 9 +---- runtime/src/serde_snapshot/tests.rs | 16 +++----- runtime/src/snapshot_bank_utils.rs | 2 +- 13 files changed, 45 insertions(+), 70 deletions(-) diff --git a/accounts-bench/src/main.rs b/accounts-bench/src/main.rs index 8af7b9a0b54aa8..0b2052065963b6 100644 --- a/accounts-bench/src/main.rs +++ b/accounts-bench/src/main.rs @@ -16,7 +16,6 @@ use { solana_epoch_schedule::EpochSchedule, solana_measure::measure::Measure, solana_pubkey::Pubkey, - solana_rent_collector::RentCollector, std::{env, fs, path::PathBuf, sync::Arc}, }; @@ -131,7 +130,6 @@ fn main() { &ancestors, None, &EpochSchedule::default(), - &RentCollector::default(), true, ); time_store.stop(); diff --git a/accounts-db/benches/accounts.rs b/accounts-db/benches/accounts.rs index c9151fe1903263..bbab6a5c1d88ff 100644 --- a/accounts-db/benches/accounts.rs +++ b/accounts-db/benches/accounts.rs @@ -18,9 +18,9 @@ use { accounts_index::ScanConfig, ancestors::Ancestors, }, + solana_clock::Epoch, solana_hash::Hash, solana_pubkey::Pubkey, - solana_rent_collector::RentCollector, solana_sysvar::epoch_schedule::EpochSchedule, std::{ collections::{HashMap, HashSet}, @@ -64,7 +64,7 @@ fn bench_accounts_hash_bank_hash(bencher: &mut Bencher) { ancestors: &ancestors, test_hash_calculation: false, epoch_schedule: &EpochSchedule::default(), - rent_collector: &RentCollector::default(), + epoch: Epoch::default(), ignore_mismatch: false, store_detailed_debug_info: false, use_bg_thread_pool: false, diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 7c1fb9d50408f3..bc5c7609e04976 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -91,7 +91,6 @@ use { solana_nohash_hasher::{BuildNoHashHasher, IntMap, IntSet}, solana_pubkey::Pubkey, solana_rayon_threadlimit::get_thread_count, - solana_rent_collector::RentCollector, solana_transaction::sanitized::SanitizedTransaction, std::{ borrow::Cow, @@ -185,8 +184,8 @@ pub struct VerifyAccountsHashAndLamportsConfig<'a> { pub test_hash_calculation: bool, /// epoch_schedule pub epoch_schedule: &'a EpochSchedule, - /// rent_collector - pub rent_collector: &'a RentCollector, + /// epoch + pub epoch: Epoch, /// true to ignore mismatches pub ignore_mismatch: bool, /// true to dump debug log if mismatch happens @@ -6256,7 +6255,6 @@ impl AccountsDb { ancestors, None, &EpochSchedule::default(), - &RentCollector::default(), is_startup, ) } @@ -6417,9 +6415,9 @@ impl AccountsDb { ancestors: &Ancestors, expected_capitalization: Option, epoch_schedule: &EpochSchedule, - rent_collector: &RentCollector, is_startup: bool, ) -> (AccountsHash, u64) { + let epoch = epoch_schedule.get_epoch(slot); let (accounts_hash, total_lamports) = self.calculate_accounts_hash_with_verify_from( data_source, debug_verify, @@ -6428,7 +6426,7 @@ impl AccountsDb { use_bg_thread_pool: !is_startup, ancestors: Some(ancestors), epoch_schedule, - rent_collector, + epoch, store_detailed_debug_info_on_failure: false, }, expected_capitalization, @@ -6772,7 +6770,7 @@ impl AccountsDb { use_bg_thread_pool: config.use_bg_thread_pool, ancestors: Some(config.ancestors), epoch_schedule: config.epoch_schedule, - rent_collector: config.rent_collector, + epoch: config.epoch, store_detailed_debug_info_on_failure: config.store_detailed_debug_info, }; let hash_mismatch_is_error = !config.ignore_mismatch; @@ -8788,13 +8786,13 @@ impl<'a> VerifyAccountsHashAndLamportsConfig<'a> { pub fn new_for_test( ancestors: &'a Ancestors, epoch_schedule: &'a EpochSchedule, - rent_collector: &'a RentCollector, + epoch: Epoch, ) -> Self { Self { ancestors, test_hash_calculation: true, epoch_schedule, - rent_collector, + epoch, ignore_mismatch: false, store_detailed_debug_info: false, use_bg_thread_pool: false, diff --git a/accounts-db/src/accounts_db/scan_account_storage.rs b/accounts-db/src/accounts_db/scan_account_storage.rs index 53d718e910022f..335883e058382c 100644 --- a/accounts-db/src/accounts_db/scan_account_storage.rs +++ b/accounts-db/src/accounts_db/scan_account_storage.rs @@ -224,10 +224,7 @@ impl AccountsDb { config.epoch_schedule, snapshot_storages.max_slot_inclusive(), ); - let slots_per_epoch = config - .rent_collector - .epoch_schedule - .get_slots_in_epoch(config.rent_collector.epoch); + let slots_per_epoch = config.epoch_schedule.get_slots_in_epoch(config.epoch); let one_epoch_old = snapshot_storages .range() .end diff --git a/accounts-db/src/accounts_db/tests.rs b/accounts-db/src/accounts_db/tests.rs index 594ba050d0442f..5276cf784c9da9 100644 --- a/accounts-db/src/accounts_db/tests.rs +++ b/accounts-db/src/accounts_db/tests.rs @@ -2105,8 +2105,6 @@ fn test_hash_stored_account() { pub static EPOCH_SCHEDULE: std::sync::LazyLock = std::sync::LazyLock::new(EpochSchedule::default); -pub static RENT_COLLECTOR: std::sync::LazyLock = - std::sync::LazyLock::new(RentCollector::default); impl CalcAccountsHashConfig<'_> { pub(crate) fn default() -> Self { @@ -2114,7 +2112,7 @@ impl CalcAccountsHashConfig<'_> { use_bg_thread_pool: false, ancestors: None, epoch_schedule: &EPOCH_SCHEDULE, - rent_collector: &RENT_COLLECTOR, + epoch: 0, store_detailed_debug_info_on_failure: false, } } @@ -2131,17 +2129,14 @@ fn test_verify_accounts_hash() { let account = AccountSharedData::new(1, some_data_len, &key); let ancestors = vec![(some_slot, 0)].into_iter().collect(); let epoch_schedule = EpochSchedule::default(); - let rent_collector = RentCollector::default(); + let epoch = Epoch::default(); db.store_for_tests(some_slot, &[(&key, &account)]); db.add_root_and_flush_write_cache(some_slot); let (_, capitalization) = db.update_accounts_hash_for_tests(some_slot, &ancestors, true, true); - let config = VerifyAccountsHashAndLamportsConfig::new_for_test( - &ancestors, - &epoch_schedule, - &rent_collector, - ); + let config = + VerifyAccountsHashAndLamportsConfig::new_for_test(&ancestors, &epoch_schedule, epoch); assert_matches!( db.verify_accounts_hash_and_lamports_for_tests(some_slot, 1, config.clone()), @@ -2181,12 +2176,9 @@ fn test_verify_bank_capitalization() { let account = AccountSharedData::new(1, some_data_len, &key); let ancestors = vec![(some_slot, 0)].into_iter().collect(); let epoch_schedule = EpochSchedule::default(); - let rent_collector = RentCollector::default(); - let config = VerifyAccountsHashAndLamportsConfig::new_for_test( - &ancestors, - &epoch_schedule, - &rent_collector, - ); + let epoch = Epoch::default(); + let config = + VerifyAccountsHashAndLamportsConfig::new_for_test(&ancestors, &epoch_schedule, epoch); db.store_for_tests(some_slot, &[(&key, &account)]); if pass == 0 { @@ -2235,12 +2227,9 @@ fn test_verify_accounts_hash_no_account() { db.update_accounts_hash_for_tests(some_slot, &ancestors, true, true); let epoch_schedule = EpochSchedule::default(); - let rent_collector = RentCollector::default(); - let config = VerifyAccountsHashAndLamportsConfig::new_for_test( - &ancestors, - &epoch_schedule, - &rent_collector, - ); + let epoch = Epoch::default(); + let config = + VerifyAccountsHashAndLamportsConfig::new_for_test(&ancestors, &epoch_schedule, epoch); assert_matches!( db.verify_accounts_hash_and_lamports_for_tests(some_slot, 0, config), @@ -2267,12 +2256,9 @@ fn test_verify_accounts_hash_bad_account_hash() { db.add_root_and_flush_write_cache(some_slot); let epoch_schedule = EpochSchedule::default(); - let rent_collector = RentCollector::default(); - let config = VerifyAccountsHashAndLamportsConfig::new_for_test( - &ancestors, - &epoch_schedule, - &rent_collector, - ); + let epoch = Epoch::default(); + let config = + VerifyAccountsHashAndLamportsConfig::new_for_test(&ancestors, &epoch_schedule, epoch); assert_matches!( db.verify_accounts_hash_and_lamports_for_tests(some_slot, 1, config), diff --git a/accounts-db/src/accounts_hash.rs b/accounts-db/src/accounts_hash.rs index a892c72ed8720f..b960118c372fb1 100644 --- a/accounts-db/src/accounts_hash.rs +++ b/accounts-db/src/accounts_hash.rs @@ -8,12 +8,11 @@ use { bytemuck_derive::{Pod, Zeroable}, log::*, rayon::prelude::*, - solana_clock::Slot, + solana_clock::{Epoch, Slot}, solana_hash::{Hash, HASH_BYTES}, solana_lattice_hash::lt_hash::LtHash, solana_measure::{measure::Measure, measure_us}, solana_pubkey::Pubkey, - solana_rent_collector::RentCollector, solana_sha256_hasher::Hasher, solana_sysvar::epoch_schedule::EpochSchedule, std::{ @@ -97,9 +96,10 @@ pub struct CalcAccountsHashConfig<'a> { /// does hash calc need to consider account data that exists in the write cache? /// if so, 'ancestors' will be used for this purpose as well as storages. pub epoch_schedule: &'a EpochSchedule, - pub rent_collector: &'a RentCollector, /// used for tracking down hash mismatches after the fact pub store_detailed_debug_info_on_failure: bool, + /// used to calculate the number of slots in the given epoch + pub epoch: Epoch, } // smallest, 3 quartiles, largest, average diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index a2c64d62fa17c4..e824fb74eb8605 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -335,11 +335,14 @@ impl AccountsHashVerifier { }; timings.calc_storage_size_quartiles(&accounts_package.snapshot_storages); + let epoch = accounts_package + .epoch_schedule + .get_epoch(accounts_package.slot); let calculate_accounts_hash_config = CalcAccountsHashConfig { use_bg_thread_pool: true, ancestors: None, epoch_schedule: &accounts_package.epoch_schedule, - rent_collector: &accounts_package.rent_collector, + epoch, store_detailed_debug_info_on_failure: false, }; @@ -403,11 +406,14 @@ impl AccountsHashVerifier { }); let sorted_storages = SortedStorages::new_with_slots(incremental_storages, None, None); + let epoch = accounts_package + .epoch_schedule + .get_epoch(accounts_package.slot); let calculate_accounts_hash_config = CalcAccountsHashConfig { use_bg_thread_pool: true, ancestors: None, epoch_schedule: &accounts_package.epoch_schedule, - rent_collector: &accounts_package.rent_collector, + epoch, store_detailed_debug_info_on_failure: false, }; diff --git a/core/tests/epoch_accounts_hash.rs b/core/tests/epoch_accounts_hash.rs index 9f33a1cdf81030..3afebaf9cd9f19 100755 --- a/core/tests/epoch_accounts_hash.rs +++ b/core/tests/epoch_accounts_hash.rs @@ -321,7 +321,7 @@ fn test_epoch_accounts_hash_basic(test_environment: TestEnvironment) { use_bg_thread_pool: false, ancestors: Some(&bank.ancestors), epoch_schedule: bank.epoch_schedule(), - rent_collector: bank.rent_collector(), + epoch: bank.epoch(), store_detailed_debug_info_on_failure: false, }, ); diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index f13ea3b53600cd..b2069893b48b4e 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -3118,7 +3118,6 @@ fn main() { assert_capitalization(&bank); println!("Inflation: {:?}", bank.inflation()); - println!("RentCollector: {:?}", bank.rent_collector()); println!("Capitalization: {}", Sol(bank.capitalization())); } } diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 2e0b7ccdd348ca..c7c8d654e74a40 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -338,7 +338,7 @@ impl SnapshotRequestHandler { use_bg_thread_pool: true, ancestors: None, epoch_schedule: snapshot_root_bank.epoch_schedule(), - rent_collector: snapshot_root_bank.rent_collector(), + epoch: snapshot_root_bank.epoch(), store_detailed_debug_info_on_failure: false, }, ); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 8ccc5073523e9b..c7dc8caa797f1c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5262,7 +5262,7 @@ impl Bank { let verify_config = VerifyAccountsHashAndLamportsConfig { ancestors: &self.ancestors, epoch_schedule: self.epoch_schedule(), - rent_collector: self.rent_collector(), + epoch: self.epoch(), test_hash_calculation: config.test_hash_calculation, ignore_mismatch: config.ignore_mismatch, store_detailed_debug_info: config.store_hash_raw_data_for_debug, @@ -5278,7 +5278,6 @@ impl Bank { let accounts_ = Arc::clone(&accounts); let ancestors = self.ancestors.clone(); let epoch_schedule = self.epoch_schedule().clone(); - let rent_collector = self.rent_collector().clone(); let expected_accounts_lt_hash = self.accounts_lt_hash.lock().unwrap().clone(); accounts.accounts_db.verify_accounts_hash_in_bg.start(|| { Builder::new() @@ -5328,7 +5327,6 @@ impl Bank { VerifyAccountsHashAndLamportsConfig { ancestors: &ancestors, epoch_schedule: &epoch_schedule, - rent_collector: &rent_collector, ..verify_config }, )); @@ -5533,7 +5531,6 @@ impl Bank { &self.ancestors, None, self.epoch_schedule(), - &self.rent_collector, is_startup, ) .1 @@ -5668,7 +5665,6 @@ impl Bank { &self.ancestors, Some(self.capitalization()), self.epoch_schedule(), - &self.rent_collector, is_startup, ); if total_lamports != self.capitalization() { @@ -5692,7 +5688,6 @@ impl Bank { &self.ancestors, Some(self.capitalization()), self.epoch_schedule(), - &self.rent_collector, is_startup, ); @@ -5712,7 +5707,7 @@ impl Bank { use_bg_thread_pool: true, ancestors: None, // does not matter, will not be used epoch_schedule: &self.epoch_schedule, - rent_collector: &self.rent_collector, + epoch: self.epoch, store_detailed_debug_info_on_failure: false, }; let storages = self.get_snapshot_storages(Some(base_slot)); diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index eb8e041cbe31f3..9fd5c085d88b47 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -25,12 +25,11 @@ mod serde_snapshot_tests { accounts_hash::AccountsHash, ancestors::Ancestors, }, - solana_clock::Slot, + solana_clock::{Epoch, Slot}, solana_epoch_schedule::EpochSchedule, solana_hash::Hash, solana_nohash_hasher::BuildNoHashHasher, solana_pubkey::Pubkey, - solana_rent_collector::RentCollector, std::{ fs::File, io::{self, BufReader, Cursor, Read, Write}, @@ -524,12 +523,9 @@ mod serde_snapshot_tests { let ancestors = Ancestors::default(); let epoch_schedule = EpochSchedule::default(); - let rent_collector = RentCollector::default(); - let config = VerifyAccountsHashAndLamportsConfig::new_for_test( - &ancestors, - &epoch_schedule, - &rent_collector, - ); + let epoch = Epoch::default(); + let config = + VerifyAccountsHashAndLamportsConfig::new_for_test(&ancestors, &epoch_schedule, epoch); accounts .verify_accounts_hash_and_lamports_for_tests(4, 1222, config) @@ -819,11 +815,11 @@ mod serde_snapshot_tests { let no_ancestors = Ancestors::default(); let epoch_schedule = EpochSchedule::default(); - let rent_collector = RentCollector::default(); + let epoch = Epoch::default(); let config = VerifyAccountsHashAndLamportsConfig::new_for_test( &no_ancestors, &epoch_schedule, - &rent_collector, + epoch, ); accounts.update_accounts_hash_for_tests(current_slot, &no_ancestors, false, false); diff --git a/runtime/src/snapshot_bank_utils.rs b/runtime/src/snapshot_bank_utils.rs index 6f4c282acfd248..5758890ea42722 100644 --- a/runtime/src/snapshot_bank_utils.rs +++ b/runtime/src/snapshot_bank_utils.rs @@ -2191,7 +2191,7 @@ mod tests { use_bg_thread_pool: false, ancestors: None, epoch_schedule: deserialized_bank.epoch_schedule(), - rent_collector: deserialized_bank.rent_collector(), + epoch: deserialized_bank.epoch(), store_detailed_debug_info_on_failure: false, }, &SortedStorages::new(&other_incremental_snapshot_storages),