From a66b023f2a1c0250c4d23ab15793eee1c46f1832 Mon Sep 17 00:00:00 2001 From: Brooks Date: Mon, 14 Oct 2024 12:16:09 -0400 Subject: [PATCH] Verifies accounts lt hash at startup (#3145) --- accounts-db/src/accounts_db.rs | 96 ++++++++++++- accounts-db/src/accounts_file.rs | 5 +- runtime/src/bank.rs | 82 ++++++++++- runtime/src/bank/accounts_lt_hash.rs | 207 +++++++++++++++++++++++++-- runtime/src/bank/serde_snapshot.rs | 6 +- runtime/src/bank/tests.rs | 61 ++++++-- runtime/src/serde_snapshot.rs | 45 ++++-- runtime/src/snapshot_bank_utils.rs | 35 +++-- 8 files changed, 485 insertions(+), 52 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 7ed43e238eb546..29582ab7e1ff7f 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -80,7 +80,7 @@ use { seqlock::SeqLock, smallvec::SmallVec, solana_lattice_hash::lt_hash::LtHash, - solana_measure::{measure::Measure, measure_us}, + solana_measure::{meas_dur, measure::Measure, measure_us}, solana_nohash_hasher::{IntMap, IntSet}, solana_rayon_threadlimit::get_thread_count, solana_sdk::{ @@ -654,6 +654,9 @@ pub enum ScanStorageResult { pub struct IndexGenerationInfo { pub accounts_data_len: u64, pub rent_paying_accounts_by_partition: RentPayingAccountsByPartition, + /// The lt hash of the old/duplicate accounts identified during index generation. + /// Will be used when verifying the accounts lt hash, after rebuilding a Bank. + pub duplicates_lt_hash: Box, } #[derive(Debug, Default)] @@ -666,6 +669,21 @@ struct SlotIndexGenerationInfo { rent_paying_accounts_by_partition: Vec, } +/// The lt hash of old/duplicate accounts +/// +/// Accumulation of all the duplicate accounts found during index generation. +/// These accounts need to have their lt hashes mixed *out*. +/// This is the final value, that when applied to all the storages at startup, +/// will produce the correct accounts lt hash. +#[derive(Debug, Clone)] +pub struct DuplicatesLtHash(pub LtHash); + +impl Default for DuplicatesLtHash { + fn default() -> Self { + Self(LtHash::identity()) + } +} + #[derive(Default, Debug)] struct GenerateIndexTimings { pub total_time_us: u64, @@ -687,6 +705,7 @@ struct GenerateIndexTimings { pub populate_duplicate_keys_us: u64, pub total_slots: u64, pub slots_to_clean: u64, + pub par_duplicates_lt_hash_us: AtomicU64, } #[derive(Default, Debug, PartialEq, Eq)] @@ -763,6 +782,11 @@ impl GenerateIndexTimings { startup_stats.copy_data_us.swap(0, Ordering::Relaxed), i64 ), + ( + "par_duplicates_lt_hash_us", + self.par_duplicates_lt_hash_us.load(Ordering::Relaxed), + i64 + ), ); } } @@ -6467,7 +6491,7 @@ impl AccountsDb { /// /// Only intended to be called at startup (or by tests). /// Only intended to be used while testing the experimental accumulator hash. - pub fn calculate_accounts_lt_hash_at_startup( + pub fn calculate_accounts_lt_hash_at_startup_from_index( &self, ancestors: &Ancestors, startup_slot: Slot, @@ -6528,6 +6552,39 @@ impl AccountsDb { AccountsLtHash(lt_hash) } + /// Calculates the accounts lt hash + /// + /// Intended to be used to verify the accounts lt hash at startup. + /// + /// The `duplicates_lt_hash` is the old/duplicate accounts to mix *out* of the storages. + /// This value comes from index generation. + pub fn calculate_accounts_lt_hash_at_startup_from_storages( + &self, + storages: &[Arc], + duplicates_lt_hash: &DuplicatesLtHash, + ) -> AccountsLtHash { + debug_assert!(self.is_experimental_accumulator_hash_enabled()); + + let mut lt_hash = storages + .par_iter() + .fold(LtHash::identity, |mut accum, storage| { + storage.accounts.scan_accounts(|stored_account_meta| { + let account_lt_hash = + Self::lt_hash_account(&stored_account_meta, stored_account_meta.pubkey()); + accum.mix_in(&account_lt_hash.0); + }); + accum + }) + .reduce(LtHash::identity, |mut accum, elem| { + accum.mix_in(&elem); + accum + }); + + lt_hash.mix_out(&duplicates_lt_hash.0); + + AccountsLtHash(lt_hash) + } + /// This is only valid to call from tests. /// run the accounts hash calculation and store the results pub fn update_accounts_hash_for_tests( @@ -8351,6 +8408,7 @@ impl AccountsDb { let rent_paying_accounts_by_partition = Mutex::new(RentPayingAccountsByPartition::new(schedule)); + let mut outer_duplicates_lt_hash = None; // pass == 0 always runs and generates the index // pass == 1 only runs if verify == true. @@ -8554,6 +8612,7 @@ impl AccountsDb { accounts_data_len_from_duplicates: u64, num_duplicate_accounts: u64, uncleaned_roots: IntSet, + duplicates_lt_hash: Box, } impl DuplicatePubkeysVisitedInfo { fn reduce(mut a: Self, mut b: Self) -> Self { @@ -8570,6 +8629,9 @@ impl AccountsDb { other.accounts_data_len_from_duplicates; self.num_duplicate_accounts += other.num_duplicate_accounts; self.uncleaned_roots.extend(other.uncleaned_roots); + self.duplicates_lt_hash + .0 + .mix_in(&other.duplicates_lt_hash.0); } } @@ -8580,6 +8642,7 @@ impl AccountsDb { accounts_data_len_from_duplicates, num_duplicate_accounts, uncleaned_roots, + duplicates_lt_hash, } = unique_pubkeys_by_bin .par_iter() .fold( @@ -8592,6 +8655,7 @@ impl AccountsDb { accounts_data_len_from_duplicates, accounts_duplicates_num, uncleaned_roots, + duplicates_lt_hash, ) = self.visit_duplicate_pubkeys_during_startup( pubkeys, &rent_collector, @@ -8601,6 +8665,7 @@ impl AccountsDb { accounts_data_len_from_duplicates, num_duplicate_accounts: accounts_duplicates_num, uncleaned_roots, + duplicates_lt_hash, }; DuplicatePubkeysVisitedInfo::reduce(accum, intermediate) }) @@ -8623,6 +8688,8 @@ impl AccountsDb { self.accounts_index .add_uncleaned_roots(uncleaned_roots.into_iter()); accounts_data_len.fetch_sub(accounts_data_len_from_duplicates, Ordering::Relaxed); + let old_val = outer_duplicates_lt_hash.replace(duplicates_lt_hash); + assert!(old_val.is_none()); info!( "accounts data len: {}", accounts_data_len.load(Ordering::Relaxed) @@ -8649,6 +8716,7 @@ impl AccountsDb { rent_paying_accounts_by_partition: rent_paying_accounts_by_partition .into_inner() .unwrap(), + duplicates_lt_hash: outer_duplicates_lt_hash.unwrap(), } } @@ -8678,20 +8746,28 @@ impl AccountsDb { /// 1. get the _duplicate_ accounts data len from the given pubkeys /// 2. get the slots that contained duplicate pubkeys /// 3. update rent stats + /// 4. build up the duplicates lt hash /// /// Note this should only be used when ALL entries in the accounts index are roots. - /// returns (data len sum of all older duplicates, number of duplicate accounts, slots that contained duplicate pubkeys) + /// + /// returns tuple of: + /// - data len sum of all older duplicates + /// - number of duplicate accounts + /// - slots that contained duplicate pubkeys + /// - lt hash of duplicates fn visit_duplicate_pubkeys_during_startup( &self, pubkeys: &[Pubkey], rent_collector: &RentCollector, timings: &GenerateIndexTimings, - ) -> (u64, u64, IntSet) { + ) -> (u64, u64, IntSet, Box) { let mut accounts_data_len_from_duplicates = 0; let mut num_duplicate_accounts = 0_u64; let mut uncleaned_slots = IntSet::default(); + let mut duplicates_lt_hash = Box::new(DuplicatesLtHash::default()); let mut removed_rent_paying = 0; let mut removed_top_off = 0; + let mut lt_hash_time = Duration::default(); self.accounts_index.scan( pubkeys.iter(), |pubkey, slots_refs, _entry| { @@ -8730,6 +8806,14 @@ impl AccountsDb { removed_rent_paying += 1; removed_top_off += lamports_to_top_off; } + if self.is_experimental_accumulator_hash_enabled() { + let (_, duration) = meas_dur!({ + let account_lt_hash = + Self::lt_hash_account(&loaded_account, pubkey); + duplicates_lt_hash.0.mix_in(&account_lt_hash.0); + }); + lt_hash_time += duration; + } }); }); } @@ -8746,10 +8830,14 @@ impl AccountsDb { timings .amount_to_top_off_rent .fetch_sub(removed_top_off, Ordering::Relaxed); + timings + .par_duplicates_lt_hash_us + .fetch_add(lt_hash_time.as_micros() as u64, Ordering::Relaxed); ( accounts_data_len_from_duplicates as u64, num_duplicate_accounts, uncleaned_slots, + duplicates_lt_hash, ) } diff --git a/accounts-db/src/accounts_file.rs b/accounts-db/src/accounts_file.rs index 3602f47c88fbec..d087aecbe0b871 100644 --- a/accounts-db/src/accounts_file.rs +++ b/accounts-db/src/accounts_file.rs @@ -210,10 +210,7 @@ impl AccountsFile { } /// Iterate over all accounts and call `callback` with each account. - pub(crate) fn scan_accounts( - &self, - callback: impl for<'local> FnMut(StoredAccountMeta<'local>), - ) { + pub fn scan_accounts(&self, callback: impl for<'local> FnMut(StoredAccountMeta<'local>)) { match self { Self::AppendVec(av) => av.scan_accounts(callback), Self::TieredStorage(ts) => { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index bd19c71fbb99bc..60347c5dde4e00 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -75,7 +75,8 @@ use { accounts::{AccountAddressFilter, Accounts, PubkeyAccountSlot}, accounts_db::{ AccountShrinkThreshold, AccountStorageEntry, AccountsDb, AccountsDbConfig, - CalcAccountsHashDataSource, PubkeyHashAccount, VerifyAccountsHashAndLamportsConfig, + CalcAccountsHashDataSource, DuplicatesLtHash, PubkeyHashAccount, + VerifyAccountsHashAndLamportsConfig, }, accounts_hash::{ AccountHash, AccountsHash, AccountsLtHash, CalcAccountsHashConfig, HashStats, @@ -1724,7 +1725,7 @@ impl Bank { .rc .accounts .accounts_db - .calculate_accounts_lt_hash_at_startup(&bank.ancestors, bank.slot()); + .calculate_accounts_lt_hash_at_startup_from_index(&bank.ancestors, bank.slot()); }); duration }); @@ -5548,6 +5549,7 @@ impl Bank { run_in_background: false, store_hash_raw_data_for_debug: on_halt_store_hash_raw_data_for_debug, }, + None, ); } @@ -5558,7 +5560,8 @@ impl Bank { fn verify_accounts_hash( &self, base: Option<(Slot, /*capitalization*/ u64)>, - config: VerifyAccountsHashConfig, + mut config: VerifyAccountsHashConfig, + duplicates_lt_hash: Option<&DuplicatesLtHash>, ) -> bool { let accounts = &self.rc.accounts; // Wait until initial hash calc is complete before starting a new hash calc. @@ -5569,19 +5572,36 @@ impl Bank { .wait_for_complete(); let slot = self.slot(); + let is_accounts_lt_hash_enabled = self.is_accounts_lt_hash_enabled(); if config.require_rooted_bank && !accounts.accounts_db.accounts_index.is_alive_root(slot) { if let Some(parent) = self.parent() { info!( "slot {slot} is not a root, so verify accounts hash on parent bank at slot {}", parent.slot(), ); - return parent.verify_accounts_hash(base, config); + if is_accounts_lt_hash_enabled { + // The duplicates_lt_hash is only valid for the current slot, so we must fall + // back to verifying the accounts lt hash with the index (which also means we + // cannot run in the background). + config.run_in_background = false; + } + return parent.verify_accounts_hash(base, config, None); } else { // this will result in mismatch errors // accounts hash calc doesn't include unrooted slots panic!("cannot verify accounts hash because slot {slot} is not a root"); } } + + if is_accounts_lt_hash_enabled { + // Calculating the accounts lt hash from storages *requires* a duplicates_lt_hash. + // If it is None here, then we must use the index instead, which also means we + // cannot run in the background. + if duplicates_lt_hash.is_none() { + config.run_in_background = false; + } + } + // 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. @@ -5600,17 +5620,43 @@ impl Bank { store_detailed_debug_info: config.store_hash_raw_data_for_debug, use_bg_thread_pool: config.run_in_background, }; + if config.run_in_background { let accounts = Arc::clone(accounts); 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(); + let duplicates_lt_hash = duplicates_lt_hash.cloned(); accounts.accounts_db.verify_accounts_hash_in_bg.start(|| { Builder::new() .name("solBgHashVerify".into()) .spawn(move || { info!("Initial background accounts hash verification has started"); + if is_accounts_lt_hash_enabled { + let accounts_db = &accounts_.accounts_db; + let (calculated_accounts_lt_hash, duration) = meas_dur!(accounts_db.thread_pool_hash.install(|| { + accounts_db + .calculate_accounts_lt_hash_at_startup_from_storages( + snapshot_storages.0.as_slice(), + &duplicates_lt_hash.unwrap(), + ) + })); + if calculated_accounts_lt_hash != expected_accounts_lt_hash { + error!( + "Verifying accounts lt hash failed: hashes do not match, expected: {}, calculated: {}", + expected_accounts_lt_hash.0.checksum(), + calculated_accounts_lt_hash.0.checksum(), + ); + return false; + } + datapoint_info!( + "startup_verify_accounts", + ("verify_accounts_lt_hash_us", duration.as_micros(), i64) + ); + } + let snapshot_storages_and_slots = ( snapshot_storages.0.as_slice(), snapshot_storages.1.as_slice(), @@ -5638,6 +5684,32 @@ impl Bank { }); true // initial result is true. We haven't failed yet. If verification fails, we'll panic from bg thread. } else { + if is_accounts_lt_hash_enabled { + let expected_accounts_lt_hash = self.accounts_lt_hash.lock().unwrap().clone(); + let calculated_accounts_lt_hash = + if let Some(duplicates_lt_hash) = duplicates_lt_hash { + accounts + .accounts_db + .calculate_accounts_lt_hash_at_startup_from_storages( + snapshot_storages.0.as_slice(), + duplicates_lt_hash, + ) + } else { + accounts + .accounts_db + .calculate_accounts_lt_hash_at_startup_from_index(&self.ancestors, slot) + }; + if calculated_accounts_lt_hash != expected_accounts_lt_hash { + error!( + "Verifying accounts lt hash failed: hashes do not match, expected: {}, calculated: {}", + expected_accounts_lt_hash.0.checksum(), + calculated_accounts_lt_hash.0.checksum(), + ); + return false; + } + // if we get here then the accounts lt hash is correct + } + let snapshot_storages_and_slots = ( snapshot_storages.0.as_slice(), snapshot_storages.1.as_slice(), @@ -5953,6 +6025,7 @@ impl Bank { force_clean: bool, latest_full_snapshot_slot: Slot, base: Option<(Slot, /*capitalization*/ u64)>, + duplicates_lt_hash: Option<&DuplicatesLtHash>, ) -> bool { let (_, clean_time_us) = measure_us!({ let should_clean = force_clean || (!skip_shrink && self.slot() > 0); @@ -6002,6 +6075,7 @@ impl Bank { run_in_background: true, store_hash_raw_data_for_debug: false, }, + duplicates_lt_hash, ); info!("Verifying accounts... In background."); verified diff --git a/runtime/src/bank/accounts_lt_hash.rs b/runtime/src/bank/accounts_lt_hash.rs index 372c65f08751a9..2fe0fceb3f3072 100644 --- a/runtime/src/bank/accounts_lt_hash.rs +++ b/runtime/src/bank/accounts_lt_hash.rs @@ -282,8 +282,18 @@ pub enum InitialStateOfAccount { mod tests { use { super::*, - crate::bank::tests::new_bank_from_parent_with_bank_forks, - solana_accounts_db::accounts::Accounts, + crate::{ + bank::tests::new_bank_from_parent_with_bank_forks, runtime_config::RuntimeConfig, + snapshot_bank_utils, snapshot_config::SnapshotConfig, snapshot_utils, + }, + solana_accounts_db::{ + accounts::Accounts, + accounts_db::{ + AccountShrinkThreshold, AccountsDbConfig, DuplicatesLtHash, + ACCOUNTS_DB_CONFIG_FOR_TESTING, + }, + accounts_index::AccountSecondaryIndexes, + }, solana_sdk::{ account::{ReadableAccount as _, WritableAccount as _}, fee_calculator::FeeRateGovernor, @@ -293,7 +303,8 @@ mod tests { signature::Signer as _, signer::keypair::Keypair, }, - std::{cmp, str::FromStr as _, sync::Arc}, + std::{cmp, collections::HashMap, ops::RangeFull, str::FromStr as _, sync::Arc}, + tempfile::TempDir, }; #[test] @@ -581,7 +592,7 @@ mod tests { } #[test] - fn test_calculate_accounts_lt_hash_at_startup() { + fn test_calculate_accounts_lt_hash_at_startup_from_index() { let (genesis_config, mint_keypair) = create_genesis_config(123_456_789 * LAMPORTS_PER_SOL); let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); bank.rc @@ -624,10 +635,190 @@ mod tests { .rc .accounts .accounts_db - .calculate_accounts_lt_hash_at_startup(&bank.ancestors, bank.slot()); + .calculate_accounts_lt_hash_at_startup_from_index(&bank.ancestors, bank.slot()); + assert_eq!(expected_accounts_lt_hash, calculated_accounts_lt_hash); + } - let expected = expected_accounts_lt_hash.0.checksum(); - let actual = calculated_accounts_lt_hash.0.checksum(); - assert_eq!(expected, actual, "expected: {expected}, actual: {actual}"); + #[test] + fn test_calculate_accounts_lt_hash_at_startup_from_storages() { + let (genesis_config, mint_keypair) = create_genesis_config(123_456_789 * LAMPORTS_PER_SOL); + let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); + bank.rc + .accounts + .accounts_db + .set_is_experimental_accumulator_hash_enabled(true); + + // ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything... + assert!(bank.is_accounts_lt_hash_enabled()); + + let amount = cmp::max( + bank.get_minimum_balance_for_rent_exemption(0), + LAMPORTS_PER_SOL, + ); + + // Write to this pubkey multiple times, so there are guaranteed duplicates in the storages. + let duplicate_pubkey = pubkey::new_rand(); + + // create some banks with some modified accounts so that there are stored accounts + // (note: the number of banks and transfers are arbitrary) + for _ in 0..7 { + let slot = bank.slot() + 1; + bank = + new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), slot); + for _ in 0..9 { + bank.register_unique_recent_blockhash_for_test(); + // note: use a random pubkey here to ensure accounts + // are spread across all the index bins + // (and calculating the accounts lt hash from storages requires no duplicates) + bank.transfer(amount, &mint_keypair, &pubkey::new_rand()) + .unwrap(); + + bank.register_unique_recent_blockhash_for_test(); + bank.transfer(amount, &mint_keypair, &duplicate_pubkey) + .unwrap(); + } + + // flush the write cache each slot to ensure there are account duplicates in the storages + bank.squash(); + bank.force_flush_accounts_cache(); + } + let expected_accounts_lt_hash = bank.accounts_lt_hash.lock().unwrap().clone(); + + // go through the storages to find the duplicates + let (mut storages, _slots) = bank + .rc + .accounts + .accounts_db + .get_snapshot_storages(RangeFull); + // sort the storages in slot-descending order + // this makes skipping the latest easier + storages.sort_unstable_by_key(|storage| cmp::Reverse(storage.slot())); + let storages = storages.into_boxed_slice(); + + // get all the lt hashes for each version of all accounts + let mut stored_accounts_map = HashMap::<_, Vec<_>>::new(); + for storage in &storages { + storage.accounts.scan_accounts(|stored_account_meta| { + let pubkey = stored_account_meta.pubkey(); + let account_lt_hash = AccountsDb::lt_hash_account(&stored_account_meta, pubkey); + stored_accounts_map + .entry(*pubkey) + .or_default() + .push(account_lt_hash) + }); + } + + // calculate the duplicates lt hash by skipping the first version (latest) of each account, + // and then mixing together all the rest + let duplicates_lt_hash = stored_accounts_map + .values() + .map(|lt_hashes| { + // the first element in the vec is the latest; all the rest are duplicates + <_hashes[1..] + }) + .fold(LtHash::identity(), |mut accum, duplicate_lt_hashes| { + for duplicate_lt_hash in duplicate_lt_hashes { + accum.mix_in(&duplicate_lt_hash.0); + } + accum + }); + let duplicates_lt_hash = DuplicatesLtHash(duplicates_lt_hash); + + // ensure that calculating the accounts lt hash from storages is correct + let calculated_accounts_lt_hash_from_storages = bank + .rc + .accounts + .accounts_db + .calculate_accounts_lt_hash_at_startup_from_storages(&storages, &duplicates_lt_hash); + assert_eq!( + expected_accounts_lt_hash, + calculated_accounts_lt_hash_from_storages + ); + } + + #[test] + fn test_verify_accounts_lt_hash_at_startup() { + let (genesis_config, mint_keypair) = create_genesis_config(123_456_789 * LAMPORTS_PER_SOL); + let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); + bank.rc + .accounts + .accounts_db + .set_is_experimental_accumulator_hash_enabled(true); + + // ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything... + assert!(bank.is_accounts_lt_hash_enabled()); + + let amount = cmp::max( + bank.get_minimum_balance_for_rent_exemption(0), + LAMPORTS_PER_SOL, + ); + + // Write to this pubkey multiple times, so there are guaranteed duplicates in the storages. + let duplicate_pubkey = pubkey::new_rand(); + + // create some banks with some modified accounts so that there are stored accounts + // (note: the number of banks and transfers are arbitrary) + for _ in 0..9 { + let slot = bank.slot() + 1; + bank = + new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), slot); + for _ in 0..3 { + bank.register_unique_recent_blockhash_for_test(); + bank.transfer(amount, &mint_keypair, &pubkey::new_rand()) + .unwrap(); + bank.register_unique_recent_blockhash_for_test(); + bank.transfer(amount, &mint_keypair, &duplicate_pubkey) + .unwrap(); + } + + // flush the write cache to disk to ensure there are duplicates across the storages + bank.fill_bank_with_ticks_for_tests(); + bank.squash(); + bank.force_flush_accounts_cache(); + } + + // verification happens at startup, so mimic the behavior by loading from a snapshot + let snapshot_config = SnapshotConfig::default(); + let bank_snapshots_dir = TempDir::new().unwrap(); + let snapshot_archives_dir = TempDir::new().unwrap(); + let snapshot = snapshot_bank_utils::bank_to_full_snapshot_archive( + &bank_snapshots_dir, + &bank, + Some(snapshot_config.snapshot_version), + &snapshot_archives_dir, + &snapshot_archives_dir, + snapshot_config.archive_format, + ) + .unwrap(); + let (_accounts_tempdir, accounts_dir) = snapshot_utils::create_tmp_accounts_dir_for_tests(); + let accounts_db_config = AccountsDbConfig { + enable_experimental_accumulator_hash: true, + ..ACCOUNTS_DB_CONFIG_FOR_TESTING + }; + let (roundtrip_bank, _) = snapshot_bank_utils::bank_from_snapshot_archives( + &[accounts_dir], + &bank_snapshots_dir, + &snapshot, + None, + &genesis_config, + &RuntimeConfig::default(), + None, + None, + AccountSecondaryIndexes::default(), + None, + AccountShrinkThreshold::default(), + false, + false, + false, + false, + Some(accounts_db_config), + None, + Arc::default(), + ) + .unwrap(); + + // Wait for the startup verification to complete. If we don't panic, then we're good! + roundtrip_bank.wait_for_initial_accounts_hash_verification_completed_for_tests(); + assert_eq!(roundtrip_bank, *bank); } } diff --git a/runtime/src/bank/serde_snapshot.rs b/runtime/src/bank/serde_snapshot.rs index a088979e7bf429..72c97f7acc8999 100644 --- a/runtime/src/bank/serde_snapshot.rs +++ b/runtime/src/bank/serde_snapshot.rs @@ -238,7 +238,7 @@ mod tests { full_snapshot_stream: &mut reader, incremental_snapshot_stream: None, }; - let dbank = serde_snapshot::bank_from_streams( + let (dbank, _) = serde_snapshot::bank_from_streams( &mut snapshot_streams, &dbank_paths, storage_and_next_append_vec_id, @@ -352,7 +352,7 @@ mod tests { storage_access, ) .unwrap(); - let dbank = crate::serde_snapshot::bank_from_streams( + let (dbank, _) = crate::serde_snapshot::bank_from_streams( &mut snapshot_streams, &dbank_paths, storage_and_next_append_vec_id, @@ -487,7 +487,7 @@ mod tests { storage_access, ) .unwrap(); - let dbank = crate::serde_snapshot::bank_from_streams( + let (dbank, _) = crate::serde_snapshot::bank_from_streams( &mut snapshot_streams, &dbank_paths, storage_and_next_append_vec_id, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 13fb573bc8afad..454aa32215de11 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -2220,7 +2220,11 @@ fn test_purge_empty_accounts() { if pass == 0 { add_root_and_flush_write_cache(&bank0); - assert!(bank0.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank0.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); continue; } @@ -2229,7 +2233,11 @@ fn test_purge_empty_accounts() { bank0.squash(); add_root_and_flush_write_cache(&bank0); if pass == 1 { - assert!(bank0.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank0.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); continue; } @@ -2237,7 +2245,11 @@ fn test_purge_empty_accounts() { bank1.squash(); add_root_and_flush_write_cache(&bank1); bank1.update_accounts_hash_for_tests(); - assert!(bank1.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank1.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); // keypair should have 0 tokens on both forks assert_eq!(bank0.get_account(&keypair.pubkey()), None); @@ -2245,7 +2257,11 @@ fn test_purge_empty_accounts() { bank1.clean_accounts_for_tests(); - assert!(bank1.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank1.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); } } @@ -3440,7 +3456,7 @@ fn test_bank_hash_internal_state() { add_root_and_flush_write_cache(&bank1); add_root_and_flush_write_cache(&bank2); bank2.update_accounts_hash_for_tests(); - assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test(), None,)); } #[test] @@ -3475,7 +3491,11 @@ fn test_bank_hash_internal_state_verify() { // we later modify bank 2, so this flush is destructive to the test add_root_and_flush_write_cache(&bank2); bank2.update_accounts_hash_for_tests(); - assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank2.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); } let bank3 = new_bank_from_parent_with_bank_forks( bank_forks.as_ref(), @@ -3486,7 +3506,11 @@ fn test_bank_hash_internal_state_verify() { assert_eq!(bank0_state, bank0.hash_internal_state()); if pass == 0 { // this relies on us having set the bank hash in the pass==0 if above - assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank2.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); continue; } if pass == 1 { @@ -3495,7 +3519,11 @@ fn test_bank_hash_internal_state_verify() { // Doing so throws an assert. So, we can't flush 3 until 2 is flushed. add_root_and_flush_write_cache(&bank3); bank3.update_accounts_hash_for_tests(); - assert!(bank3.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank3.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); continue; } @@ -3504,10 +3532,18 @@ fn test_bank_hash_internal_state_verify() { bank2.transfer(amount, &mint_keypair, &pubkey2).unwrap(); add_root_and_flush_write_cache(&bank2); bank2.update_accounts_hash_for_tests(); - assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank2.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); add_root_and_flush_write_cache(&bank3); bank3.update_accounts_hash_for_tests(); - assert!(bank3.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); + assert!(bank3.verify_accounts_hash( + None, + VerifyAccountsHashConfig::default_for_test(), + None, + )); } } @@ -3533,11 +3569,11 @@ fn test_verify_snapshot_bank() { bank.freeze(); add_root_and_flush_write_cache(&bank); bank.update_accounts_hash_for_tests(); - assert!(bank.verify_snapshot_bank(true, false, false, bank.slot(), None)); + assert!(bank.verify_snapshot_bank(true, false, false, bank.slot(), None, None,)); // tamper the bank after freeze! bank.increment_signature_count(1); - assert!(!bank.verify_snapshot_bank(true, false, false, bank.slot(), None)); + assert!(!bank.verify_snapshot_bank(true, false, false, bank.slot(), None, None,)); } // Test that two bank forks with the same accounts should not hash to the same value. @@ -12157,6 +12193,7 @@ fn test_bank_verify_accounts_hash_with_base() { test_hash_calculation: false, ..VerifyAccountsHashConfig::default_for_test() }, + None, )); } diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index e12b9c5ec124a7..0b299ea5d42185 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -20,7 +20,8 @@ use { accounts::Accounts, accounts_db::{ stats::BankHashStats, AccountShrinkThreshold, AccountStorageEntry, AccountsDb, - AccountsDbConfig, AccountsFileId, AtomicAccountsFileId, IndexGenerationInfo, + AccountsDbConfig, AccountsFileId, AtomicAccountsFileId, DuplicatesLtHash, + IndexGenerationInfo, }, accounts_file::{AccountsFile, StorageAccess}, accounts_hash::{AccountsDeltaHash, AccountsHash}, @@ -545,6 +546,12 @@ pub(crate) fn fields_from_streams( Ok((snapshot_bank_fields, snapshot_accounts_db_fields)) } +/// This struct contains side-info while reconstructing the bank from streams +#[derive(Debug)] +pub struct BankFromStreamsInfo { + pub duplicates_lt_hash: Box, +} + #[allow(clippy::too_many_arguments)] pub(crate) fn bank_from_streams( snapshot_streams: &mut SnapshotStreams, @@ -561,12 +568,12 @@ pub(crate) fn bank_from_streams( accounts_db_config: Option, accounts_update_notifier: Option, exit: Arc, -) -> std::result::Result +) -> std::result::Result<(Bank, BankFromStreamsInfo), Error> where R: Read, { let (bank_fields, accounts_db_fields) = fields_from_streams(snapshot_streams)?; - reconstruct_bank_from_fields( + let (bank, info) = reconstruct_bank_from_fields( bank_fields, accounts_db_fields, genesis_config, @@ -582,7 +589,13 @@ where accounts_db_config, accounts_update_notifier, exit, - ) + )?; + Ok(( + bank, + BankFromStreamsInfo { + duplicates_lt_hash: info.duplicates_lt_hash, + }, + )) } #[cfg(test)] @@ -826,6 +839,12 @@ impl<'a> Serialize for SerializableAccountsDb<'a> { #[cfg(feature = "frozen-abi")] impl<'a> solana_frozen_abi::abi_example::TransparentAsHelper for SerializableAccountsDb<'a> {} +/// This struct contains side-info while reconstructing the bank from fields +#[derive(Debug)] +struct ReconstructedBankInfo { + duplicates_lt_hash: Box, +} + #[allow(clippy::too_many_arguments)] fn reconstruct_bank_from_fields( bank_fields: SnapshotBankFields, @@ -843,7 +862,7 @@ fn reconstruct_bank_from_fields( accounts_db_config: Option, accounts_update_notifier: Option, exit: Arc, -) -> Result +) -> Result<(Bank, ReconstructedBankInfo), Error> where E: SerializableStorage + std::marker::Sync, { @@ -890,7 +909,12 @@ where info!("rent_collector: {:?}", bank.rent_collector()); - Ok(bank) + Ok(( + bank, + ReconstructedBankInfo { + duplicates_lt_hash: reconstructed_accounts_db_info.duplicates_lt_hash, + }, + )) } pub(crate) fn reconstruct_single_storage( @@ -1010,9 +1034,10 @@ pub(crate) fn remap_and_reconstruct_single_storage( } /// This struct contains side-info while reconstructing the accounts DB from fields. -#[derive(Debug, Default, Copy, Clone)] +#[derive(Debug, Default, Clone)] pub struct ReconstructedAccountsDbInfo { pub accounts_data_len: u64, + pub duplicates_lt_hash: Box, } #[allow(clippy::too_many_arguments)] @@ -1220,6 +1245,7 @@ where let IndexGenerationInfo { accounts_data_len, rent_paying_accounts_by_partition, + duplicates_lt_hash, } = accounts_db.generate_index( limit_load_slot_count_from_snapshot, verify_index, @@ -1241,7 +1267,10 @@ where Ok(( Arc::try_unwrap(accounts_db).unwrap(), - ReconstructedAccountsDbInfo { accounts_data_len }, + ReconstructedAccountsDbInfo { + accounts_data_len, + duplicates_lt_hash, + }, )) } diff --git a/runtime/src/snapshot_bank_utils.rs b/runtime/src/snapshot_bank_utils.rs index cc65a12f9c63c9..2547935c52841d 100644 --- a/runtime/src/snapshot_bank_utils.rs +++ b/runtime/src/snapshot_bank_utils.rs @@ -28,7 +28,7 @@ use { solana_accounts_db::{ accounts_db::{ AccountShrinkThreshold, AccountStorageEntry, AccountsDbConfig, AtomicAccountsFileId, - CalcAccountsHashDataSource, + CalcAccountsHashDataSource, DuplicatesLtHash, }, accounts_file::StorageAccess, accounts_index::AccountSecondaryIndexes, @@ -180,7 +180,7 @@ pub fn bank_from_snapshot_archives( }; let mut measure_rebuild = Measure::start("rebuild bank from snapshots"); - let bank = rebuild_bank_from_unarchived_snapshots( + let (bank, info) = rebuild_bank_from_unarchived_snapshots( &unarchived_full_snapshot.unpacked_snapshots_dir_and_version, unarchived_incremental_snapshot .as_ref() @@ -235,6 +235,7 @@ pub fn bank_from_snapshot_archives( accounts_db_force_initial_clean, full_snapshot_archive_info.slot(), base, + Some(&info.duplicates_lt_hash), ) && limit_load_slot_count_from_snapshot.is_none() { panic!("Snapshot bank for slot {} failed to verify", bank.slot()); @@ -386,7 +387,7 @@ pub fn bank_from_snapshot_dir( storage, next_append_vec_id, }; - let (bank, measure_rebuild_bank) = measure_time!( + let ((bank, _info), measure_rebuild_bank) = measure_time!( rebuild_bank_from_snapshot( bank_snapshot, account_paths, @@ -544,6 +545,12 @@ fn deserialize_status_cache( }) } +/// This struct contains side-info from rebuilding the bank +#[derive(Debug)] +struct RebuiltBankInfo { + duplicates_lt_hash: Box, +} + #[allow(clippy::too_many_arguments)] fn rebuild_bank_from_unarchived_snapshots( full_snapshot_unpacked_snapshots_dir_and_version: &UnpackedSnapshotsDirAndVersion, @@ -563,7 +570,7 @@ fn rebuild_bank_from_unarchived_snapshots( accounts_db_config: Option, accounts_update_notifier: Option, exit: Arc, -) -> snapshot_utils::Result { +) -> snapshot_utils::Result<(Bank, RebuiltBankInfo)> { let (full_snapshot_version, full_snapshot_root_paths) = verify_unpacked_snapshots_dir_and_version( full_snapshot_unpacked_snapshots_dir_and_version, @@ -593,7 +600,7 @@ fn rebuild_bank_from_unarchived_snapshots( .map(|root_paths| root_paths.snapshot_path()), }; - let bank = deserialize_snapshot_data_files(&snapshot_root_paths, |snapshot_streams| { + let (bank, info) = deserialize_snapshot_data_files(&snapshot_root_paths, |snapshot_streams| { Ok( match incremental_snapshot_version.unwrap_or(full_snapshot_version) { SnapshotVersion::V1_2_0 => bank_from_streams( @@ -641,7 +648,12 @@ fn rebuild_bank_from_unarchived_snapshots( bank.status_cache.write().unwrap().append(&slot_deltas); info!("Rebuilt bank for slot: {}", bank.slot()); - Ok(bank) + Ok(( + bank, + RebuiltBankInfo { + duplicates_lt_hash: info.duplicates_lt_hash, + }, + )) } #[allow(clippy::too_many_arguments)] @@ -660,7 +672,7 @@ fn rebuild_bank_from_snapshot( accounts_db_config: Option, accounts_update_notifier: Option, exit: Arc, -) -> snapshot_utils::Result { +) -> snapshot_utils::Result<(Bank, RebuiltBankInfo)> { info!( "Rebuilding bank from snapshot {}", bank_snapshot.snapshot_dir.display(), @@ -671,7 +683,7 @@ fn rebuild_bank_from_snapshot( incremental_snapshot_root_file_path: None, }; - let bank = deserialize_snapshot_data_files(&snapshot_root_paths, |snapshot_streams| { + let (bank, info) = deserialize_snapshot_data_files(&snapshot_root_paths, |snapshot_streams| { Ok(bank_from_streams( snapshot_streams, account_paths, @@ -702,7 +714,12 @@ fn rebuild_bank_from_snapshot( bank.status_cache.write().unwrap().append(&slot_deltas); info!("Rebuilt bank for slot: {}", bank.slot()); - Ok(bank) + Ok(( + bank, + RebuiltBankInfo { + duplicates_lt_hash: info.duplicates_lt_hash, + }, + )) } /// Verify that the snapshot's slot deltas are not corrupt/invalid