diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index 3fd6d81533..6a2f769e31 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -96,7 +96,7 @@ impl AccountsHashVerifier { fault_injection_rate_slots: u64, snapshot_config: Option<&SnapshotConfig>, ) { - Self::verify_accounts_package_hash(&accounts_package); + let accounts_hash = Self::calculate_and_verify_accounts_hash(&accounts_package); Self::push_accounts_hashes_to_cluster( &accounts_package, @@ -106,55 +106,63 @@ impl AccountsHashVerifier { hashes, exit, fault_injection_rate_slots, + accounts_hash, ); - Self::submit_for_packaging(accounts_package, pending_snapshot_package, snapshot_config); + Self::submit_for_packaging( + accounts_package, + pending_snapshot_package, + snapshot_config, + accounts_hash, + ); } - fn verify_accounts_package_hash(accounts_package: &AccountsPackage) { + /// returns calculated accounts hash + fn calculate_and_verify_accounts_hash(accounts_package: &AccountsPackage) -> Hash { let mut measure_hash = Measure::start("hash"); + let mut sort_time = Measure::start("sort_storages"); + let sorted_storages = SortedStorages::new(&accounts_package.snapshot_storages); + sort_time.stop(); + + let mut timings = HashStats { + storage_sort_us: sort_time.as_us(), + ..HashStats::default() + }; + timings.calc_storage_size_quartiles(&accounts_package.snapshot_storages); + + let (accounts_hash, lamports) = accounts_package + .accounts + .accounts_db + .calculate_accounts_hash_without_index( + &CalcAccountsHashConfig { + use_bg_thread_pool: true, + check_hash: false, + ancestors: None, + use_write_cache: false, + epoch_schedule: &accounts_package.epoch_schedule, + rent_collector: &accounts_package.rent_collector, + }, + &sorted_storages, + timings, + ) + .unwrap(); + + assert_eq!(accounts_package.expected_capitalization, lamports); if let Some(expected_hash) = accounts_package.accounts_hash_for_testing { - let mut sort_time = Measure::start("sort_storages"); - let sorted_storages = SortedStorages::new(&accounts_package.snapshot_storages); - sort_time.stop(); - - let mut timings = HashStats { - storage_sort_us: sort_time.as_us(), - ..HashStats::default() - }; - timings.calc_storage_size_quartiles(&accounts_package.snapshot_storages); - - let (hash, lamports) = accounts_package - .accounts - .accounts_db - .calculate_accounts_hash_without_index( - &CalcAccountsHashConfig { - use_bg_thread_pool: true, - check_hash: false, - ancestors: None, - use_write_cache: false, - epoch_schedule: &accounts_package.epoch_schedule, - rent_collector: &accounts_package.rent_collector, - }, - &sorted_storages, - timings, - ) - .unwrap(); - - assert_eq!(accounts_package.expected_capitalization, lamports); - assert_eq!(expected_hash, hash); + assert_eq!(expected_hash, accounts_hash); }; measure_hash.stop(); solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash( accounts_package.snapshot_links.path(), accounts_package.slot, - &accounts_package.accounts_hash, + &accounts_hash, ); datapoint_info!( "accounts_hash_verifier", ("calculate_hash", measure_hash.as_us(), i64), ); + accounts_hash } fn push_accounts_hashes_to_cluster( @@ -165,8 +173,8 @@ impl AccountsHashVerifier { hashes: &mut Vec<(Slot, Hash)>, exit: &Arc, fault_injection_rate_slots: u64, + accounts_hash: Hash, ) { - let hash = accounts_package.accounts_hash; if fault_injection_rate_slots != 0 && accounts_package.slot % fault_injection_rate_slots == 0 { @@ -177,10 +185,10 @@ impl AccountsHashVerifier { }; warn!("inserting fault at slot: {}", accounts_package.slot); let rand = thread_rng().gen_range(0, 10); - let hash = extend_and_hash(&hash, &[rand]); + let hash = extend_and_hash(&accounts_hash, &[rand]); hashes.push((accounts_package.slot, hash)); } else { - hashes.push((accounts_package.slot, hash)); + hashes.push((accounts_package.slot, accounts_hash)); } while hashes.len() > MAX_SNAPSHOT_HASHES { @@ -204,6 +212,7 @@ impl AccountsHashVerifier { accounts_package: AccountsPackage, pending_snapshot_package: Option<&PendingSnapshotPackage>, snapshot_config: Option<&SnapshotConfig>, + accounts_hash: Hash, ) { if accounts_package.snapshot_type.is_none() || pending_snapshot_package.is_none() @@ -212,7 +221,7 @@ impl AccountsHashVerifier { return; }; - let snapshot_package = SnapshotPackage::from(accounts_package); + let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash); let pending_snapshot_package = pending_snapshot_package.unwrap(); let _snapshot_config = snapshot_config.unwrap(); @@ -301,6 +310,7 @@ mod tests { sysvar::epoch_schedule::EpochSchedule, }, solana_streamer::socket::SocketAddrSpace, + std::str::FromStr, }; fn new_test_cluster_info(contact_info: ContactInfo) -> ClusterInfo { @@ -364,6 +374,7 @@ mod tests { ..SnapshotConfig::default() }; let accounts = Arc::new(solana_runtime::accounts::Accounts::default_for_tests()); + let expected_hash = Hash::from_str("GKot5hBsd81kMupNCXHaqbhv3huEbxAFMLnpcX2hniwn").unwrap(); for i in 0..MAX_SNAPSHOT_HASHES + 1 { let accounts_package = AccountsPackage { slot: full_snapshot_archive_interval_slots + i as u64, @@ -371,7 +382,6 @@ mod tests { slot_deltas: vec![], snapshot_links: TempDir::new().unwrap(), snapshot_storages: vec![], - accounts_hash: hash(&[i as u8]), archive_format: ArchiveFormat::TarBzip2, snapshot_version: SnapshotVersion::default(), snapshot_archives_dir: PathBuf::default(), @@ -409,13 +419,13 @@ mod tests { assert_eq!(cluster_hashes.len(), MAX_SNAPSHOT_HASHES); assert_eq!( cluster_hashes[0], - (full_snapshot_archive_interval_slots + 1, hash(&[1])) + (full_snapshot_archive_interval_slots + 1, expected_hash) ); assert_eq!( cluster_hashes[MAX_SNAPSHOT_HASHES - 1], ( full_snapshot_archive_interval_slots + MAX_SNAPSHOT_HASHES as u64, - hash(&[MAX_SNAPSHOT_HASHES as u8]) + expected_hash ) ); } diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 2d76be4e09..6aee704552 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -288,7 +288,8 @@ mod tests { accounts_package.slot, &last_bank.get_accounts_hash(), ); - let snapshot_package = SnapshotPackage::from(accounts_package); + let snapshot_package = + SnapshotPackage::new(accounts_package, last_bank.get_accounts_hash()); snapshot_utils::archive_snapshot_package( &snapshot_package, snapshot_config.maximum_full_snapshot_archives_to_retain, @@ -391,7 +392,6 @@ mod tests { let tx = system_transaction::transfer(mint_keypair, &key1, 1, genesis_config.hash()); assert_eq!(bank.process_transaction(&tx), Ok(())); bank.squash(); - let accounts_hash = bank.update_accounts_hash(); let pending_accounts_package = { if slot == saved_slot as u64 { @@ -461,7 +461,9 @@ mod tests { saved_archive_path = Some(snapshot_utils::build_full_snapshot_archive_path( snapshot_archives_dir, slot, - &accounts_hash, + // this needs to match the hash value that we reserialize with later. It is complicated, so just use default. + // This hash value is just used to build the file name. Since this is mocked up test code, it is sufficient to pass default here. + &Hash::default(), ArchiveFormat::TarBzip2, )); } @@ -515,7 +517,7 @@ mod tests { accounts_package.slot, &Hash::default(), ); - let snapshot_package = SnapshotPackage::from(accounts_package); + let snapshot_package = SnapshotPackage::new(accounts_package, Hash::default()); pending_snapshot_package .lock() .unwrap() diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 49c3dce3d4..4b339a0fb8 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -4,6 +4,7 @@ use { crate::{ + accounts_hash::CalcAccountsHashConfig, bank::{Bank, BankSlotDelta, DropCallback}, bank_forks::BankForks, snapshot_config::SnapshotConfig, @@ -109,7 +110,7 @@ impl SnapshotRequestHandler { let previous_hash = if test_hash_calculation { // We have to use the index version here. - // We cannot calculate the non-index way because cache has not been flushed and stores don't match reality. + // We cannot calculate the non-index way because cache has not been flushed and stores don't match reality. This comment is out of date and can be re-evaluated. snapshot_root_bank.update_accounts_hash_with_index_option(true, false, false) } else { Hash::default() @@ -144,20 +145,27 @@ impl SnapshotRequestHandler { } flush_accounts_cache_time.stop(); - let use_index_hash_calculation = false; - let mut hash_time = Measure::start("hash_time"); - let this_hash = snapshot_root_bank.update_accounts_hash_with_index_option( - use_index_hash_calculation, - test_hash_calculation, - false, - ); let hash_for_testing = if test_hash_calculation { + let use_index_hash_calculation = false; + let check_hash = false; + + let (this_hash, _cap) = snapshot_root_bank.accounts().accounts_db.calculate_accounts_hash_helper( + use_index_hash_calculation, + snapshot_root_bank.slot(), + &CalcAccountsHashConfig { + use_bg_thread_pool: true, + check_hash, + ancestors: None, + use_write_cache: false, + epoch_schedule: snapshot_root_bank.epoch_schedule(), + rent_collector: &snapshot_root_bank.rent_collector(), + }, + ).unwrap(); assert_eq!(previous_hash, this_hash); - Some(snapshot_root_bank.get_accounts_hash()) + Some(this_hash) } else { None }; - hash_time.stop(); let mut clean_time = Measure::start("clean_time"); // Don't clean the slot we're snapshotting because it may have zero-lamport @@ -234,7 +242,6 @@ impl SnapshotRequestHandler { datapoint_info!( "handle_snapshot_requests-timing", - ("hash_time", hash_time.as_us(), i64), ( "flush_accounts_cache_time", flush_accounts_cache_time.as_us(), diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index f57a9e32d8..5bcdb5aa1a 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -5505,7 +5505,7 @@ impl AccountsDb { } } - fn calculate_accounts_hash_helper( + pub(crate) fn calculate_accounts_hash_helper( &self, use_index: bool, slot: Slot, diff --git a/runtime/src/snapshot_package.rs b/runtime/src/snapshot_package.rs index 991f6cabf6..95f5aacb4e 100644 --- a/runtime/src/snapshot_package.rs +++ b/runtime/src/snapshot_package.rs @@ -37,7 +37,6 @@ pub struct AccountsPackage { pub slot_deltas: Vec, pub snapshot_links: TempDir, pub snapshot_storages: SnapshotStorages, - pub accounts_hash: Hash, // temporarily here while we still have to calculate hash before serializing bank pub archive_format: ArchiveFormat, pub snapshot_version: SnapshotVersion, pub snapshot_archives_dir: PathBuf, @@ -104,7 +103,6 @@ impl AccountsPackage { slot_deltas, snapshot_links, snapshot_storages, - accounts_hash: bank.get_accounts_hash(), archive_format, snapshot_version, snapshot_archives_dir: snapshot_archives_dir.as_ref().to_path_buf(), @@ -129,8 +127,8 @@ pub struct SnapshotPackage { pub snapshot_type: SnapshotType, } -impl From for SnapshotPackage { - fn from(accounts_package: AccountsPackage) -> Self { +impl SnapshotPackage { + pub fn new(accounts_package: AccountsPackage, accounts_hash: Hash) -> Self { assert!( accounts_package.snapshot_type.is_some(), "Cannot make a SnapshotPackage from an AccountsPackage when SnapshotType is None!" @@ -141,7 +139,7 @@ impl From for SnapshotPackage { SnapshotType::FullSnapshot => snapshot_utils::build_full_snapshot_archive_path( accounts_package.snapshot_archives_dir, accounts_package.slot, - &accounts_package.accounts_hash, + &accounts_hash, accounts_package.archive_format, ), SnapshotType::IncrementalSnapshot(incremental_snapshot_base_slot) => { @@ -161,7 +159,7 @@ impl From for SnapshotPackage { accounts_package.snapshot_archives_dir, incremental_snapshot_base_slot, accounts_package.slot, - &accounts_package.accounts_hash, + &accounts_hash, accounts_package.archive_format, ) } @@ -171,7 +169,7 @@ impl From for SnapshotPackage { snapshot_archive_info: SnapshotArchiveInfo { path: snapshot_archive_path, slot: accounts_package.slot, - hash: accounts_package.accounts_hash, + hash: accounts_hash, archive_format: accounts_package.archive_format, }, block_height: accounts_package.block_height, diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 6402e02ea4..1aee970466 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -1924,7 +1924,7 @@ pub fn package_and_archive_full_snapshot( &bank.get_accounts_hash(), ); - let snapshot_package = SnapshotPackage::from(accounts_package); + let snapshot_package = SnapshotPackage::new(accounts_package, bank.get_accounts_hash()); archive_snapshot_package( &snapshot_package, maximum_full_snapshot_archives_to_retain, @@ -1971,7 +1971,7 @@ pub fn package_and_archive_incremental_snapshot( &bank.get_accounts_hash(), ); - let snapshot_package = SnapshotPackage::from(accounts_package); + let snapshot_package = SnapshotPackage::new(accounts_package, bank.get_accounts_hash()); archive_snapshot_package( &snapshot_package, maximum_full_snapshot_archives_to_retain, @@ -3443,7 +3443,6 @@ mod tests { slot_deltas: Vec::default(), snapshot_links: TempDir::new().unwrap(), snapshot_storages: SnapshotStorages::default(), - accounts_hash: Hash::default(), archive_format: ArchiveFormat::Tar, snapshot_version: SnapshotVersion::default(), snapshot_archives_dir: PathBuf::default(),