From 112a0b475a7c893d8dc5d323f0d58d0457ded023 Mon Sep 17 00:00:00 2001 From: apfitzge Date: Mon, 2 May 2022 13:46:17 -0500 Subject: [PATCH] Revert "Refactor to use EpochSchedule from within RentCollector struct" (#24893) * Revert "Ran cargo fmt" This reverts commit 9052e41b3272cedcf63b063104fea16269a9749e. * Revert "Fix build error introduced by my editor setup, part 2" This reverts commit 4dfeab3b38d4afc66bbc2768fc132c5cb5a24c4c. * Revert "Fix build error introduced by my editor setup" This reverts commit 87fb78dc561c42babf7fcfb34e4edc394ffcecc2. * Revert "Remove redundant epoch_schedule from AccountsPackage" This reverts commit c2f7f2fff853386a9f765ca868087151dc0f473e. * Revert "Fix a test" This reverts commit 36c0bdaa7894c3d542306a5f26554440efc01d99. * Revert "Fixes to initial code" This reverts commit ed7813e698658fd45dbc684d8ca6ac792c5aed4c. * Revert "Removing redundant EpochSchedule param from fns" This reverts commit 5472d2e605a6105134c4fab4c75b704344b9e554. --- accounts-bench/src/main.rs | 15 ++++-- core/src/accounts_hash_verifier.rs | 3 ++ runtime/benches/accounts.rs | 21 +++++--- runtime/src/accounts.rs | 6 ++- runtime/src/accounts_background_service.rs | 1 + runtime/src/accounts_db.rs | 61 ++++++++++++++++++---- runtime/src/accounts_hash.rs | 2 + runtime/src/bank.rs | 4 ++ runtime/src/snapshot_package.rs | 6 ++- runtime/src/snapshot_utils.rs | 1 + 10 files changed, 97 insertions(+), 23 deletions(-) diff --git a/accounts-bench/src/main.rs b/accounts-bench/src/main.rs index 6e770edb8..987915d8c 100644 --- a/accounts-bench/src/main.rs +++ b/accounts-bench/src/main.rs @@ -15,7 +15,9 @@ use { ancestors::Ancestors, rent_collector::RentCollector, }, - solana_sdk::{genesis_config::ClusterType, pubkey::Pubkey}, + solana_sdk::{ + genesis_config::ClusterType, pubkey::Pubkey, sysvar::epoch_schedule::EpochSchedule, + }, std::{env, fs, path::PathBuf}, }; @@ -118,10 +120,12 @@ fn main() { } else { let mut pubkeys: Vec = vec![]; let mut time = Measure::start("hash"); - let results = - accounts - .accounts_db - .update_accounts_hash(0, &ancestors, &RentCollector::default()); + let results = accounts.accounts_db.update_accounts_hash( + 0, + &ancestors, + &EpochSchedule::default(), + &RentCollector::default(), + ); time.stop(); let mut time_store = Measure::start("hash using store"); let results_store = accounts.accounts_db.update_accounts_hash_with_index_option( @@ -131,6 +135,7 @@ fn main() { &ancestors, None, false, + &EpochSchedule::default(), &RentCollector::default(), false, ); diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index cdd0baef4..6a2f769e3 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -139,6 +139,7 @@ impl AccountsHashVerifier { check_hash: false, ancestors: None, use_write_cache: false, + epoch_schedule: &accounts_package.epoch_schedule, rent_collector: &accounts_package.rent_collector, }, &sorted_storages, @@ -306,6 +307,7 @@ mod tests { genesis_config::ClusterType, hash::hash, signature::{Keypair, Signer}, + sysvar::epoch_schedule::EpochSchedule, }, solana_streamer::socket::SocketAddrSpace, std::str::FromStr, @@ -388,6 +390,7 @@ mod tests { cluster_type: ClusterType::MainnetBeta, snapshot_type: None, accounts: Arc::clone(&accounts), + epoch_schedule: EpochSchedule::default(), rent_collector: RentCollector::default(), }; diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs index 04e944395..24ad6761d 100644 --- a/runtime/benches/accounts.rs +++ b/runtime/benches/accounts.rs @@ -21,6 +21,7 @@ use { hash::Hash, lamports::LamportsError, pubkey::Pubkey, + sysvar::epoch_schedule::EpochSchedule, }, std::{ collections::{HashMap, HashSet}, @@ -108,10 +109,12 @@ fn test_accounts_hash_bank_hash(bencher: &mut Bencher) { let slot = 0; create_test_accounts(&accounts, &mut pubkeys, num_accounts, slot); let ancestors = Ancestors::from(vec![0]); - let (_, total_lamports) = - accounts - .accounts_db - .update_accounts_hash(0, &ancestors, &RentCollector::default()); + let (_, total_lamports) = accounts.accounts_db.update_accounts_hash( + 0, + &ancestors, + &EpochSchedule::default(), + &RentCollector::default(), + ); let test_hash_calculation = false; bencher.iter(|| { assert!(accounts.verify_bank_hash_and_lamports( @@ -119,6 +122,7 @@ fn test_accounts_hash_bank_hash(bencher: &mut Bencher) { &ancestors, total_lamports, test_hash_calculation, + &EpochSchedule::default(), &RentCollector::default() )) }); @@ -138,9 +142,12 @@ fn test_update_accounts_hash(bencher: &mut Bencher) { create_test_accounts(&accounts, &mut pubkeys, 50_000, 0); let ancestors = Ancestors::from(vec![0]); bencher.iter(|| { - accounts - .accounts_db - .update_accounts_hash(0, &ancestors, &RentCollector::default()); + accounts.accounts_db.update_accounts_hash( + 0, + &ancestors, + &EpochSchedule::default(), + &RentCollector::default(), + ); }); } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 0248739f1..529c4060c 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -44,7 +44,7 @@ use { pubkey::Pubkey, slot_hashes::SlotHashes, system_program, - sysvar::{self, instructions::construct_instructions_data}, + sysvar::{self, epoch_schedule::EpochSchedule, instructions::construct_instructions_data}, transaction::{Result, SanitizedTransaction, TransactionAccountLocks, TransactionError}, transaction_context::TransactionAccount, }, @@ -760,6 +760,7 @@ impl Accounts { slot: Slot, can_cached_slot_be_unflushed: bool, debug_verify: bool, + epoch_schedule: &EpochSchedule, rent_collector: &RentCollector, ) -> u64 { let use_index = false; @@ -772,6 +773,7 @@ impl Accounts { ancestors, None, can_cached_slot_be_unflushed, + epoch_schedule, rent_collector, is_startup, ) @@ -786,6 +788,7 @@ impl Accounts { ancestors: &Ancestors, total_lamports: u64, test_hash_calculation: bool, + epoch_schedule: &EpochSchedule, rent_collector: &RentCollector, ) -> bool { if let Err(err) = self.accounts_db.verify_bank_hash_and_lamports_new( @@ -793,6 +796,7 @@ impl Accounts { ancestors, total_lamports, test_hash_calculation, + epoch_schedule, rent_collector, ) { warn!("verify_bank_hash failed: {:?}", err); diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 9732d3df6..19cf6b644 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -233,6 +233,7 @@ impl SnapshotRequestHandler { check_hash, ancestors: None, use_write_cache: false, + epoch_schedule: snapshot_root_bank.epoch_schedule(), rent_collector: snapshot_root_bank.rent_collector(), }, ).unwrap(); diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index ed93d73c7..543f50a8b 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -5297,6 +5297,7 @@ impl AccountsDb { &self, slot: Slot, ancestors: &Ancestors, + epoch_schedule: &EpochSchedule, rent_collector: &RentCollector, ) -> (Hash, u64) { self.update_accounts_hash_with_index_option( @@ -5306,6 +5307,7 @@ impl AccountsDb { ancestors, None, false, + epoch_schedule, rent_collector, false, ) @@ -5319,6 +5321,7 @@ impl AccountsDb { ancestors, None, false, + &EpochSchedule::default(), &RentCollector::default(), false, ) @@ -5620,10 +5623,7 @@ impl AccountsDb { Some(slot), ); - self.mark_old_slots_as_dirty( - &storages, - Some(config.rent_collector.epoch_schedule.slots_per_epoch), - ); + self.mark_old_slots_as_dirty(&storages, Some(config.epoch_schedule.slots_per_epoch)); sort_time.stop(); let mut timings = HashStats { @@ -5677,6 +5677,7 @@ impl AccountsDb { ancestors: &Ancestors, expected_capitalization: Option, can_cached_slot_be_unflushed: bool, + epoch_schedule: &EpochSchedule, rent_collector: &RentCollector, is_startup: bool, ) -> (Hash, u64) { @@ -5691,6 +5692,7 @@ impl AccountsDb { check_hash, ancestors: Some(ancestors), use_write_cache: can_cached_slot_be_unflushed, + epoch_schedule, rent_collector, }, expected_capitalization, @@ -5942,6 +5944,7 @@ impl AccountsDb { ancestors: &Ancestors, total_lamports: u64, test_hash_calculation: bool, + epoch_schedule: &EpochSchedule, rent_collector: &RentCollector, ) -> Result<(), BankHashVerificationError> { self.verify_bank_hash_and_lamports_new( @@ -5949,6 +5952,7 @@ impl AccountsDb { ancestors, total_lamports, test_hash_calculation, + epoch_schedule, rent_collector, ) } @@ -5960,6 +5964,7 @@ impl AccountsDb { ancestors: &Ancestors, total_lamports: u64, test_hash_calculation: bool, + epoch_schedule: &EpochSchedule, rent_collector: &RentCollector, ) -> Result<(), BankHashVerificationError> { use BankHashVerificationError::*; @@ -5978,6 +5983,7 @@ impl AccountsDb { check_hash, ancestors: Some(ancestors), use_write_cache: can_cached_slot_be_unflushed, + epoch_schedule, rent_collector, }, None, @@ -7767,6 +7773,7 @@ pub mod tests { check_hash, ancestors: None, use_write_cache: false, + epoch_schedule: &EpochSchedule::default(), rent_collector: &RentCollector::default(), }, None, @@ -8155,6 +8162,7 @@ pub mod tests { check_hash: false, ancestors: None, use_write_cache: false, + epoch_schedule: &EpochSchedule::default(), rent_collector: &RentCollector::default(), }, &get_storage_refs(&storages), @@ -8183,6 +8191,7 @@ pub mod tests { check_hash: false, ancestors: None, use_write_cache: false, + epoch_schedule: &EpochSchedule::default(), rent_collector: &RentCollector::default(), }, &get_storage_refs(&storages), @@ -8244,6 +8253,7 @@ pub mod tests { check_hash: false, ancestors: None, use_write_cache: false, + epoch_schedule: &EpochSchedule::default(), rent_collector: &RentCollector::default(), }, &get_storage_refs(&storages), @@ -9612,8 +9622,18 @@ pub mod tests { let ancestors = linear_ancestors(latest_slot); assert_eq!( - daccounts.update_accounts_hash(latest_slot, &ancestors, &RentCollector::default()), - accounts.update_accounts_hash(latest_slot, &ancestors, &RentCollector::default()) + daccounts.update_accounts_hash( + latest_slot, + &ancestors, + &EpochSchedule::default(), + &RentCollector::default() + ), + accounts.update_accounts_hash( + latest_slot, + &ancestors, + &EpochSchedule::default(), + &RentCollector::default() + ) ); } @@ -9892,7 +9912,12 @@ pub mod tests { accounts.add_root(current_slot); accounts.print_accounts_stats("pre_f"); - accounts.update_accounts_hash(4, &Ancestors::default(), &RentCollector::default()); + accounts.update_accounts_hash( + 4, + &Ancestors::default(), + &EpochSchedule::default(), + &RentCollector::default(), + ); let accounts = f(accounts, current_slot); @@ -9909,6 +9934,7 @@ pub mod tests { &Ancestors::default(), 1222, true, + &EpochSchedule::default(), &RentCollector::default(), ) .unwrap(); @@ -10220,6 +10246,7 @@ pub mod tests { check_hash, ancestors: Some(&ancestors), use_write_cache: false, + epoch_schedule: &EpochSchedule::default(), rent_collector: &RentCollector::default(), }, ) @@ -10251,6 +10278,7 @@ pub mod tests { check_hash, ancestors: Some(&ancestors), use_write_cache: false, + epoch_schedule: &EpochSchedule::default(), rent_collector: &RentCollector::default(), }, ) @@ -10263,6 +10291,7 @@ pub mod tests { check_hash, ancestors: Some(&ancestors), use_write_cache: false, + epoch_schedule: &EpochSchedule::default(), rent_collector: &RentCollector::default(), }, ) @@ -10291,6 +10320,7 @@ pub mod tests { &ancestors, 1, true, + &EpochSchedule::default(), &RentCollector::default() ), Ok(_) @@ -10303,6 +10333,7 @@ pub mod tests { &ancestors, 1, true, + &EpochSchedule::default(), &RentCollector::default() ), Err(MissingBankHash) @@ -10324,6 +10355,7 @@ pub mod tests { &ancestors, 1, true, + &EpochSchedule::default(), &RentCollector::default() ), Err(MismatchedBankHash) @@ -10351,6 +10383,7 @@ pub mod tests { &ancestors, 1, true, + &EpochSchedule::default(), &RentCollector::default() ), Ok(_) @@ -10371,13 +10404,14 @@ pub mod tests { &ancestors, 2, true, + &EpochSchedule::default(), &RentCollector::default() ), Ok(_) ); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 10, true, &RentCollector::default()), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 10, true, &EpochSchedule::default(), &RentCollector::default()), Err(MismatchedTotalLamports(expected, actual)) if expected == 2 && actual == 10 ); } @@ -10402,6 +10436,7 @@ pub mod tests { &ancestors, 0, true, + &EpochSchedule::default(), &RentCollector::default() ), Ok(_) @@ -10438,6 +10473,7 @@ pub mod tests { &ancestors, 1, true, + &EpochSchedule::default(), &RentCollector::default() ), Err(MismatchedBankHash) @@ -11043,13 +11079,19 @@ pub mod tests { ); let no_ancestors = Ancestors::default(); - accounts.update_accounts_hash(current_slot, &no_ancestors, &RentCollector::default()); + accounts.update_accounts_hash( + current_slot, + &no_ancestors, + &EpochSchedule::default(), + &RentCollector::default(), + ); accounts .verify_bank_hash_and_lamports( current_slot, &no_ancestors, 22300, true, + &EpochSchedule::default(), &RentCollector::default(), ) .unwrap(); @@ -11061,6 +11103,7 @@ pub mod tests { &no_ancestors, 22300, true, + &EpochSchedule::default(), &RentCollector::default(), ) .unwrap(); diff --git a/runtime/src/accounts_hash.rs b/runtime/src/accounts_hash.rs index d1c89df17..2218bcfc0 100644 --- a/runtime/src/accounts_hash.rs +++ b/runtime/src/accounts_hash.rs @@ -6,6 +6,7 @@ use { solana_sdk::{ hash::{Hash, Hasher}, pubkey::Pubkey, + sysvar::epoch_schedule::EpochSchedule, }, std::{ borrow::Borrow, @@ -40,6 +41,7 @@ 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 use_write_cache: bool, + pub epoch_schedule: &'a EpochSchedule, pub rent_collector: &'a RentCollector, } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 2c6bb3813..4542e214e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -6256,6 +6256,7 @@ impl Bank { &self.ancestors, self.capitalization(), test_hash_calculation, + self.epoch_schedule(), &self.rent_collector, ) } @@ -6324,6 +6325,7 @@ impl Bank { self.slot(), can_cached_slot_be_unflushed, debug_verify, + self.epoch_schedule(), &self.rent_collector, ) } @@ -6377,6 +6379,7 @@ impl Bank { &self.ancestors, Some(self.capitalization()), false, + self.epoch_schedule(), &self.rent_collector, is_startup, ); @@ -6402,6 +6405,7 @@ impl Bank { &self.ancestors, Some(self.capitalization()), false, + self.epoch_schedule(), &self.rent_collector, is_startup, ); diff --git a/runtime/src/snapshot_package.rs b/runtime/src/snapshot_package.rs index b038d990f..87bf2038b 100644 --- a/runtime/src/snapshot_package.rs +++ b/runtime/src/snapshot_package.rs @@ -11,7 +11,9 @@ use { }, }, log::*, - solana_sdk::{clock::Slot, genesis_config::ClusterType, hash::Hash}, + solana_sdk::{ + clock::Slot, genesis_config::ClusterType, hash::Hash, sysvar::epoch_schedule::EpochSchedule, + }, std::{ fs, path::{Path, PathBuf}, @@ -43,6 +45,7 @@ pub struct AccountsPackage { pub cluster_type: ClusterType, pub snapshot_type: Option, pub accounts: Arc, + pub epoch_schedule: EpochSchedule, pub rent_collector: RentCollector, } @@ -108,6 +111,7 @@ impl AccountsPackage { cluster_type: bank.cluster_type(), snapshot_type, accounts: bank.accounts(), + epoch_schedule: *bank.epoch_schedule(), rent_collector: bank.rent_collector().clone(), }) } diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 06d235def..e1aeecdc8 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -3461,6 +3461,7 @@ mod tests { cluster_type: solana_sdk::genesis_config::ClusterType::Development, snapshot_type, accounts: Arc::new(crate::accounts::Accounts::default_for_tests()), + epoch_schedule: solana_sdk::epoch_schedule::EpochSchedule::default(), rent_collector: crate::rent_collector::RentCollector::default(), } }