diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 919ae08b6..5d77e932f 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -13,7 +13,7 @@ use { accounts_update_notifier_interface::AccountsUpdateNotifier, ancestors::Ancestors, bank::{ - Bank, NonceFull, NonceInfo, RentDebits, Rewrites, TransactionCheckResult, + Bank, NonceFull, NonceInfo, RentDebits, TransactionCheckResult, TransactionExecutionResult, }, blockhash_queue::BlockhashQueue, @@ -1209,10 +1209,8 @@ impl Accounts { } } - pub fn bank_hash_info_at(&self, slot: Slot, rewrites: &Rewrites) -> BankHashInfo { - let accounts_delta_hash = self - .accounts_db - .get_accounts_delta_hash_with_rewrites(slot, rewrites); + pub fn bank_hash_info_at(&self, slot: Slot) -> BankHashInfo { + let accounts_delta_hash = self.accounts_db.get_accounts_delta_hash(slot); let bank_hashes = self.accounts_db.bank_hashes.read().unwrap(); let mut bank_hash_info = bank_hashes .get(&slot) @@ -2629,7 +2627,7 @@ mod tests { AccountSecondaryIndexes::default(), AccountShrinkThreshold::default(), ); - accounts.bank_hash_info_at(1, &Rewrites::default()); + accounts.bank_hash_info_at(1); } #[test] diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index f27a5147f..db4725c80 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -45,7 +45,6 @@ use { StoredAccountMeta, StoredMeta, StoredMetaWriteVersion, APPEND_VEC_MMAPPED_FILES_OPEN, STORE_META_OVERHEAD, }, - bank::Rewrites, cache_hash_data::{CacheHashData, CacheHashDataFile}, contains::Contains, epoch_accounts_hash::EpochAccountsHashManager, @@ -7733,10 +7732,6 @@ impl AccountsDb { } } - pub fn get_accounts_delta_hash(&self, slot: Slot) -> Hash { - self.get_accounts_delta_hash_with_rewrites(slot, &Rewrites::default()) - } - /// helper to return /// 1. pubkey, hash pairs for the slot /// 2. us spent scanning @@ -7781,11 +7776,7 @@ impl AccountsDb { (hashes, scan.as_us(), accumulate) } - pub fn get_accounts_delta_hash_with_rewrites( - &self, - slot: Slot, - skipped_rewrites: &Rewrites, - ) -> Hash { + pub fn get_accounts_delta_hash(&self, slot: Slot) -> Hash { let (mut hashes, scan_us, mut accumulate) = self.get_pubkey_hash_for_slot(slot); let dirty_keys = hashes.iter().map(|(pubkey, _hash)| *pubkey).collect(); @@ -7794,8 +7785,6 @@ impl AccountsDb { hashes.retain(|(pubkey, _hash)| !self.is_filler_account(pubkey)); } - self.extend_hashes_with_skipped_rewrites(&mut hashes, skipped_rewrites); - let ret = AccountsHasher::accumulate_account_hashes(hashes); accumulate.stop(); let mut uncleaned_time = Measure::start("uncleaned_index"); @@ -7815,38 +7804,6 @@ impl AccountsDb { ret } - /// add all items from 'skipped_rewrites' to 'hashes' where the pubkey doesn't already exist in 'hashes' - fn extend_hashes_with_skipped_rewrites( - &self, - hashes: &mut Vec<(Pubkey, Hash)>, - skipped_rewrites: &Rewrites, - ) { - let mut skipped_rewrites = skipped_rewrites.read().unwrap().clone(); - if skipped_rewrites.is_empty() { - // if there are no skipped rewrites, then there is nothing futher to do - return; - } - hashes.iter().for_each(|(key, _)| { - skipped_rewrites.remove(key); - }); - - if self.filler_accounts_enabled() { - // simulate the time we would normally spend hashing the filler accounts - // this is an over approximation but at least takes a stab at simulating what the validator would spend time doing - let _ = AccountsHasher::accumulate_account_hashes( - skipped_rewrites - .iter() - .map(|(k, v)| (*k, *v)) - .collect::>(), - ); - - // filler accounts do not get their updated hash values hashed into the delta hash - skipped_rewrites.retain(|key, _| !self.is_filler_account(key)); - } - - hashes.extend(skipped_rewrites.into_iter()); - } - fn update_index<'a, T: ReadableAccount + Sync>( &self, infos: Vec, @@ -16548,28 +16505,6 @@ pub mod tests { } } - #[test] - fn test_extend_hashes_with_skipped_rewrites() { - let db = AccountsDb::new_single_for_tests(); - let mut hashes = Vec::default(); - let rewrites = Rewrites::default(); - db.extend_hashes_with_skipped_rewrites(&mut hashes, &rewrites); - assert!(hashes.is_empty()); - let pubkey = Pubkey::new(&[1; 32]); - let hash = Hash::new(&[2; 32]); - rewrites.write().unwrap().insert(pubkey, hash); - db.extend_hashes_with_skipped_rewrites(&mut hashes, &rewrites); - assert_eq!(hashes, vec![(pubkey, hash)]); - // pubkey is already in hashes, will not be added a second time - db.extend_hashes_with_skipped_rewrites(&mut hashes, &rewrites); - assert_eq!(hashes, vec![(pubkey, hash)]); - let pubkey2 = Pubkey::new(&[2; 32]); - let hash2 = Hash::new(&[3; 32]); - rewrites.write().unwrap().insert(pubkey2, hash2); - db.extend_hashes_with_skipped_rewrites(&mut hashes, &rewrites); - assert_eq!(hashes, vec![(pubkey, hash), (pubkey2, hash2)]); - } - #[test] fn test_calc_alive_ancient_historical_roots() { let db = AccountsDb::new_single_for_tests(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ac596fbf3..ec6ada469 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -210,8 +210,6 @@ pub const SECONDS_PER_YEAR: f64 = 365.25 * 24.0 * 60.0 * 60.0; pub const MAX_LEADER_SCHEDULE_STAKES: Epoch = 5; -pub type Rewrites = RwLock>; - #[derive(Default)] struct RentMetrics { hold_range_us: AtomicU64, @@ -862,7 +860,6 @@ impl PartialEq for Bank { freeze_started: _, vote_only_bank: _, cost_tracker: _, - rewrites_skipped_this_slot: _, sysvar_cache: _, accounts_data_size_initial: _, accounts_data_size_delta_on_chain: _, @@ -1110,9 +1107,6 @@ pub struct Bank { sysvar_cache: RwLock, - /// (Pubkey, account Hash) for each account that would have been rewritten in rent collection for this slot - pub rewrites_skipped_this_slot: Rewrites, - /// The initial accounts data size at the start of this Bank, before processing any transactions/etc accounts_data_size_initial: u64, /// The change to accounts data size in this Bank, due on-chain events (i.e. transactions) @@ -1270,7 +1264,6 @@ impl Bank { fn default_with_accounts(accounts: Accounts) -> Self { let mut bank = Self { incremental_snapshot_persistence: None, - rewrites_skipped_this_slot: Rewrites::default(), rc: BankRc::new(accounts, Slot::default()), status_cache: Arc::>::default(), blockhash_queue: RwLock::::default(), @@ -1574,7 +1567,6 @@ impl Bank { let accounts_data_size_initial = parent.load_accounts_data_size(); let mut new = Bank { incremental_snapshot_persistence: None, - rewrites_skipped_this_slot: Rewrites::default(), rc, status_cache, slot, @@ -1958,7 +1950,6 @@ impl Bank { let feature_set = new(); let mut bank = Self { incremental_snapshot_persistence: fields.incremental_snapshot_persistence, - rewrites_skipped_this_slot: Rewrites::default(), rc: bank_rc, status_cache: new(), blockhash_queue: RwLock::new(fields.blockhash_queue), @@ -5299,11 +5290,6 @@ impl Bank { "collect_rent_eagerly", ("accounts", rent_metrics.count.load(Relaxed), i64), ("partitions", count, i64), - ( - "skipped_rewrites", - self.rewrites_skipped_this_slot.read().unwrap().len(), - i64 - ), ("total_time_us", measure.as_us(), i64), ( "hold_range_us", @@ -5431,7 +5417,6 @@ impl Bank { CollectRentFromAccountsInfo { rent_collected_info: total_rent_collected_info, rent_rewards: rent_debits.into_unordered_rewards_iter().collect(), - rewrites_skipped, time_collecting_rent_us, time_hashing_skipped_rewrites_us, time_storing_accounts_us, @@ -5572,7 +5557,6 @@ impl Bank { .write() .unwrap() .append(&mut results.rent_rewards); - self.remember_skipped_rewrites(results.rewrites_skipped); metrics .load_us @@ -5590,16 +5574,6 @@ impl Bank { }); } - // put 'rewrites_skipped' into 'self.rewrites_skipped_this_slot' - fn remember_skipped_rewrites(&self, rewrites_skipped: Vec<(Pubkey, Hash)>) { - if !rewrites_skipped.is_empty() { - let mut rewrites_skipped_this_slot = self.rewrites_skipped_this_slot.write().unwrap(); - rewrites_skipped.into_iter().for_each(|(pubkey, hash)| { - rewrites_skipped_this_slot.insert(pubkey, hash); - }); - } - } - /// return true iff storing this account is just a rewrite and can be skipped fn skip_rewrite(rent_amount: u64, account: &AccountSharedData) -> bool { // if rent was != 0 @@ -6720,10 +6694,7 @@ impl Bank { /// of the delta of the ledger since the last vote and up to now fn hash_internal_state(&self) -> Hash { // If there are no accounts, return the hash of the previous state and the latest blockhash - let bank_hash_info = self - .rc - .accounts - .bank_hash_info_at(self.slot(), &self.rewrites_skipped_this_slot); + let bank_hash_info = self.rc.accounts.bank_hash_info_at(self.slot()); let mut signature_count_buf = [0u8; 8]; LittleEndian::write_u64(&mut signature_count_buf[..], self.signature_count()); @@ -7877,7 +7848,6 @@ enum ApplyFeatureActivationsCaller { struct CollectRentFromAccountsInfo { rent_collected_info: CollectedInfo, rent_rewards: Vec<(Pubkey, RewardInfo)>, - rewrites_skipped: Vec<(Pubkey, Hash)>, time_collecting_rent_us: u64, time_hashing_skipped_rewrites_us: u64, time_storing_accounts_us: u64, @@ -7891,7 +7861,6 @@ struct CollectRentInPartitionInfo { rent_collected: u64, accounts_data_size_reclaimed: u64, rent_rewards: Vec<(Pubkey, RewardInfo)>, - rewrites_skipped: Vec<(Pubkey, Hash)>, time_loading_accounts_us: u64, time_collecting_rent_us: u64, time_hashing_skipped_rewrites_us: u64, @@ -7908,7 +7877,6 @@ impl CollectRentInPartitionInfo { rent_collected: info.rent_collected_info.rent_amount, accounts_data_size_reclaimed: info.rent_collected_info.account_data_len_reclaimed, rent_rewards: info.rent_rewards, - rewrites_skipped: info.rewrites_skipped, time_loading_accounts_us: time_loading_accounts.as_micros() as u64, time_collecting_rent_us: info.time_collecting_rent_us, time_hashing_skipped_rewrites_us: info.time_hashing_skipped_rewrites_us, @@ -7929,7 +7897,6 @@ impl CollectRentInPartitionInfo { .accounts_data_size_reclaimed .saturating_add(rhs.accounts_data_size_reclaimed), rent_rewards: [lhs.rent_rewards, rhs.rent_rewards].concat(), - rewrites_skipped: [lhs.rewrites_skipped, rhs.rewrites_skipped].concat(), time_loading_accounts_us: lhs .time_loading_accounts_us .saturating_add(rhs.time_loading_accounts_us), @@ -10024,25 +9991,7 @@ pub(crate) mod tests { ); assert_eq!(bank.collected_rent.load(Relaxed), 0); - assert!(bank.rewrites_skipped_this_slot.read().unwrap().is_empty()); bank.collect_rent_in_partition((0, 0, 1), true, &RentMetrics::default()); - { - let rewrites_skipped = bank.rewrites_skipped_this_slot.read().unwrap(); - // `rewrites_skipped.len()` is the number of non-rent paying accounts in the slot. - // 'collect_rent_in_partition' fills 'rewrites_skipped_this_slot' with rewrites that - // were skipped during rent collection but should still be considered in the slot's - // bank hash. If the slot is also written in the append vec, then the bank hash calc - // code ignores the contents of this list. This assert is confirming that the expected # - // of accounts were included in 'rewrites_skipped' by the call to - // 'collect_rent_in_partition(..., true)' above. - assert_eq!(rewrites_skipped.len(), 1); - // should not have skipped 'rent_exempt_pubkey' - // Once preserve_rent_epoch_for_rent_exempt_accounts is activated, - // rewrite-skip is irrelevant to rent-exempt accounts. - assert!(!rewrites_skipped.contains_key(&rent_exempt_pubkey)); - // should NOT have skipped 'rent_due_pubkey' - assert!(!rewrites_skipped.contains_key(&rent_due_pubkey)); - } assert_eq!(bank.collected_rent.load(Relaxed), 0); bank.collect_rent_in_partition((0, 0, 1), false, &RentMetrics::default()); // all range @@ -10114,13 +10063,12 @@ pub(crate) mod tests { let just_rewrites = true; // loaded from previous slot, so we skip rent collection on it - let result = later_bank.collect_rent_from_accounts( + let _result = later_bank.collect_rent_from_accounts( vec![(zero_lamport_pubkey, account, later_slot - 1)], just_rewrites, None, PartitionIndex::default(), ); - assert!(result.rewrites_skipped[0].0 == zero_lamport_pubkey); } #[test] @@ -19611,53 +19559,6 @@ pub(crate) mod tests { } } - #[test] - fn test_remember_skipped_rewrites() { - let GenesisConfigInfo { - mut genesis_config, .. - } = create_genesis_config_with_leader(1_000_000_000, &Pubkey::new_unique(), 42); - genesis_config.rent = Rent::default(); - activate_all_features(&mut genesis_config); - - let bank = Bank::new_for_tests(&genesis_config); - - assert!(bank.rewrites_skipped_this_slot.read().unwrap().is_empty()); - bank.remember_skipped_rewrites(Vec::default()); - assert!(bank.rewrites_skipped_this_slot.read().unwrap().is_empty()); - - // bank's map is initially empty - let mut test = vec![(Pubkey::new(&[4; 32]), Hash::new(&[5; 32]))]; - bank.remember_skipped_rewrites(test.clone()); - assert_eq!( - *bank.rewrites_skipped_this_slot.read().unwrap(), - test.clone().into_iter().collect() - ); - - // now there is already some stuff in the bank's map - test.push((Pubkey::new(&[6; 32]), Hash::new(&[7; 32]))); - bank.remember_skipped_rewrites(test[1..].to_vec()); - assert_eq!( - *bank.rewrites_skipped_this_slot.read().unwrap(), - test.clone().into_iter().collect() - ); - - // all contents are already in map - bank.remember_skipped_rewrites(test[1..].to_vec()); - assert_eq!( - *bank.rewrites_skipped_this_slot.read().unwrap(), - test.clone().into_iter().collect() - ); - - // all contents are already in map, but we're changing hash values - test[0].1 = Hash::new(&[8; 32]); - test[1].1 = Hash::new(&[9; 32]); - bank.remember_skipped_rewrites(test.to_vec()); - assert_eq!( - *bank.rewrites_skipped_this_slot.read().unwrap(), - test.into_iter().collect() - ); - } - #[test] fn test_inner_instructions_list_from_instruction_trace() { let instruction_trace = [1, 2, 1, 1, 2, 3, 2]; diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index 4f65f0117..791f4a6ad 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -7,7 +7,7 @@ use { accounts_db::{get_temp_accounts_paths, AccountShrinkThreshold, AccountStorageMap}, accounts_hash::AccountsHash, append_vec::AppendVec, - bank::{Bank, BankTestConfig, Rewrites}, + bank::{Bank, BankTestConfig}, epoch_accounts_hash, genesis_utils::{self, activate_all_features, activate_feature}, snapshot_utils::ArchiveFormat, @@ -209,12 +209,8 @@ fn test_accounts_serialize_style(serde_style: SerdeStyle) { ); check_accounts(&daccounts, &pubkeys, 100); assert_eq!( - accounts - .bank_hash_info_at(0, &Rewrites::default()) - .accounts_delta_hash, - daccounts - .bank_hash_info_at(0, &Rewrites::default()) - .accounts_delta_hash + accounts.bank_hash_info_at(0).accounts_delta_hash, + daccounts.bank_hash_info_at(0).accounts_delta_hash ); }