diff --git a/accounts-db/benches/accounts.rs b/accounts-db/benches/accounts.rs index e17871d2bd3..1f013aff807 100644 --- a/accounts-db/benches/accounts.rs +++ b/accounts-db/benches/accounts.rs @@ -60,20 +60,22 @@ fn bench_accounts_hash_bank_hash(bencher: &mut Bencher) { accounts.add_root(slot); accounts.accounts_db.flush_accounts_cache(true, Some(slot)); bencher.iter(|| { - assert!(accounts.verify_accounts_hash_and_lamports( - 0, - total_lamports, - None, - VerifyAccountsHashAndLamportsConfig { - ancestors: &ancestors, - test_hash_calculation: false, - epoch_schedule: &EpochSchedule::default(), - rent_collector: &RentCollector::default(), - ignore_mismatch: false, - store_detailed_debug_info: false, - use_bg_thread_pool: false, - } - )) + assert!(accounts + .accounts_db + .verify_accounts_hash_and_lamports_for_tests( + 0, + total_lamports, + VerifyAccountsHashAndLamportsConfig { + ancestors: &ancestors, + test_hash_calculation: false, + epoch_schedule: &EpochSchedule::default(), + rent_collector: &RentCollector::default(), + ignore_mismatch: false, + store_detailed_debug_info: false, + use_bg_thread_pool: false, + } + ) + .is_ok()) }); } diff --git a/accounts-db/src/accounts.rs b/accounts-db/src/accounts.rs index 3ecdd270867..ea95448abcf 100644 --- a/accounts-db/src/accounts.rs +++ b/accounts-db/src/accounts.rs @@ -1,8 +1,8 @@ use { crate::{ accounts_db::{ - AccountsAddRootTiming, AccountsDb, LoadHint, LoadedAccount, ScanAccountStorageData, - ScanStorageResult, VerifyAccountsHashAndLamportsConfig, + AccountStorageEntry, AccountsAddRootTiming, AccountsDb, LoadHint, LoadedAccount, + ScanAccountStorageData, ScanStorageResult, VerifyAccountsHashAndLamportsConfig, }, accounts_index::{IndexKey, ScanConfig, ScanError, ScanResult}, ancestors::Ancestors, @@ -297,15 +297,19 @@ impl Accounts { #[must_use] pub fn verify_accounts_hash_and_lamports( &self, + snapshot_storages_and_slots: (&[Arc], &[Slot]), slot: Slot, total_lamports: u64, base: Option<(Slot, /*capitalization*/ u64)>, config: VerifyAccountsHashAndLamportsConfig, ) -> bool { - if let Err(err) = - self.accounts_db - .verify_accounts_hash_and_lamports(slot, total_lamports, base, config) - { + if let Err(err) = self.accounts_db.verify_accounts_hash_and_lamports( + snapshot_storages_and_slots, + slot, + total_lamports, + base, + config, + ) { warn!("verify_accounts_hash failed: {err:?}, slot: {slot}"); false } else { diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 5dd14632fb5..d16020ecc40 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -7488,6 +7488,7 @@ impl AccountsDb { /// and then calculate the incremental accounts hash for `(base slot, slot]`. pub fn verify_accounts_hash_and_lamports( &self, + snapshot_storages_and_slots: (&[Arc], &[Slot]), slot: Slot, total_lamports: u64, base: Option<(Slot, /*capitalization*/ u64)>, @@ -7503,11 +7504,21 @@ impl AccountsDb { let hash_mismatch_is_error = !config.ignore_mismatch; if let Some((base_slot, base_capitalization)) = base { - self.verify_accounts_hash_and_lamports(base_slot, base_capitalization, None, config)?; - let (storages, slots) = - self.get_snapshot_storages(base_slot.checked_add(1).unwrap()..=slot); - let sorted_storages = - SortedStorages::new_with_slots(storages.iter().zip(slots), None, None); + self.verify_accounts_hash_and_lamports( + snapshot_storages_and_slots, + base_slot, + base_capitalization, + None, + config, + )?; + + let storages_and_slots = snapshot_storages_and_slots + .0 + .iter() + .zip(snapshot_storages_and_slots.1.iter()) + .filter(|storage_and_slot| *storage_and_slot.1 > base_slot) + .map(|(storage, slot)| (storage, *slot)); + let sorted_storages = SortedStorages::new_with_slots(storages_and_slots, None, None); let calculated_incremental_accounts_hash = self.calculate_incremental_accounts_hash( &calc_config, &sorted_storages, @@ -7526,9 +7537,13 @@ impl AccountsDb { } } } else { - let (storages, slots) = self.get_snapshot_storages(..=slot); - let sorted_storages = - SortedStorages::new_with_slots(storages.iter().zip(slots), None, None); + let storages_and_slots = snapshot_storages_and_slots + .0 + .iter() + .zip(snapshot_storages_and_slots.1.iter()) + .filter(|storage_and_slot| *storage_and_slot.1 <= slot) + .map(|(storage, slot)| (storage, *slot)); + let sorted_storages = SortedStorages::new_with_slots(storages_and_slots, None, None); let (calculated_accounts_hash, calculated_lamports) = self.calculate_accounts_hash(&calc_config, &sorted_storages, HashStats::default()); if calculated_lamports != total_lamports { @@ -9448,7 +9463,18 @@ impl AccountsDb { total_lamports: u64, config: VerifyAccountsHashAndLamportsConfig, ) -> Result<(), AccountsHashVerificationError> { - self.verify_accounts_hash_and_lamports(slot, total_lamports, None, config) + let snapshot_storages = self.get_snapshot_storages(..); + let snapshot_storages_and_slots = ( + snapshot_storages.0.as_slice(), + snapshot_storages.1.as_slice(), + ); + self.verify_accounts_hash_and_lamports( + snapshot_storages_and_slots, + slot, + total_lamports, + None, + config, + ) } } @@ -12400,7 +12426,7 @@ pub mod tests { db.update_accounts_hash_for_tests(some_slot, &ancestors, true, true); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 1, None, config.clone()), + db.verify_accounts_hash_and_lamports_for_tests(some_slot, 1, config.clone()), Ok(_) ); continue; diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ab0864c000a..36f6714dd08 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -185,7 +185,7 @@ use { collections::{HashMap, HashSet}, convert::TryFrom, fmt, mem, - ops::{AddAssign, RangeInclusive}, + ops::{AddAssign, RangeFull, RangeInclusive}, path::PathBuf, slice, sync::{ @@ -5620,7 +5620,15 @@ impl Bank { panic!("cannot verify accounts hash because slot {slot} is not a root"); } } - let cap = self.capitalization(); + // The snapshot storages must be captured *before* starting the background verification. + // Otherwise, it is possible that a delayed call to `get_snapshot_storages()` will *not* + // get the correct storages required to calculate and verify the accounts hashes. + let snapshot_storages = self + .rc + .accounts + .accounts_db + .get_snapshot_storages(RangeFull); + let capitalization = self.capitalization(); let verify_config = VerifyAccountsHashAndLamportsConfig { ancestors: &self.ancestors, epoch_schedule: self.epoch_schedule(), @@ -5641,9 +5649,14 @@ impl Bank { .name("solBgHashVerify".into()) .spawn(move || { info!("Initial background accounts hash verification has started"); + let snapshot_storages_and_slots = ( + snapshot_storages.0.as_slice(), + snapshot_storages.1.as_slice(), + ); let result = accounts_.verify_accounts_hash_and_lamports( + snapshot_storages_and_slots, slot, - cap, + capitalization, base, VerifyAccountsHashAndLamportsConfig { ancestors: &ancestors, @@ -5663,7 +5676,17 @@ impl Bank { }); true // initial result is true. We haven't failed yet. If verification fails, we'll panic from bg thread. } else { - let result = accounts.verify_accounts_hash_and_lamports(slot, cap, base, verify_config); + let snapshot_storages_and_slots = ( + snapshot_storages.0.as_slice(), + snapshot_storages.1.as_slice(), + ); + let result = accounts.verify_accounts_hash_and_lamports( + snapshot_storages_and_slots, + slot, + capitalization, + base, + verify_config, + ); self.set_initial_accounts_hash_verification_completed(); result }