diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index d69cd895ccafd8..e38664a70c965d 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -691,7 +691,7 @@ pub struct IndexGenerationInfo { 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, + pub duplicates_lt_hash: Option>, } #[derive(Debug, Default)] @@ -8791,8 +8791,10 @@ 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()); + if self.is_experimental_accumulator_hash_enabled() { + 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) @@ -8830,7 +8832,7 @@ impl AccountsDb { rent_paying_accounts_by_partition: rent_paying_accounts_by_partition .into_inner() .unwrap(), - duplicates_lt_hash: outer_duplicates_lt_hash.unwrap(), + duplicates_lt_hash: outer_duplicates_lt_hash, } } diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 6e17f5a9cfb0f2..abda76928dc3dd 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -6,7 +6,7 @@ use { itertools::Itertools, log::{info, trace}, solana_accounts_db::{ - accounts_db::{self, ACCOUNTS_DB_CONFIG_FOR_TESTING}, + accounts_db::{self, AccountsDbConfig, ACCOUNTS_DB_CONFIG_FOR_TESTING}, accounts_index::AccountSecondaryIndexes, epoch_accounts_hash::EpochAccountsHash, }, @@ -55,7 +55,7 @@ use { time::{Duration, Instant}, }, tempfile::TempDir, - test_case::test_case, + test_case::{test_case, test_matrix}, }; struct SnapshotTestConfig { @@ -629,14 +629,22 @@ fn restore_from_snapshots_and_check_banks_are_equal( Ok(()) } -/// Spin up the background services fully and test taking snapshots -#[test_case(V1_2_0, Development)] -#[test_case(V1_2_0, Devnet)] -#[test_case(V1_2_0, Testnet)] -#[test_case(V1_2_0, MainnetBeta)] +#[derive(Debug, Eq, PartialEq)] +enum VerifyAccountsKind { + Merkle, + Lattice, +} + +/// Spin up the background services fully then test taking & verifying snapshots +#[test_matrix( + V1_2_0, + [Development, Devnet, Testnet, MainnetBeta], + [VerifyAccountsKind::Merkle, VerifyAccountsKind::Lattice] +)] fn test_snapshots_with_background_services( snapshot_version: SnapshotVersion, cluster_type: ClusterType, + verify_accounts_kind: VerifyAccountsKind, ) { solana_logger::setup(); @@ -821,6 +829,10 @@ fn test_snapshots_with_background_services( // Load the snapshot and ensure it matches what's in BankForks let (_tmp_dir, temporary_accounts_dir) = create_tmp_accounts_dir_for_tests(); + let accounts_db_config = AccountsDbConfig { + enable_experimental_accumulator_hash: verify_accounts_kind == VerifyAccountsKind::Lattice, + ..ACCOUNTS_DB_CONFIG_FOR_TESTING + }; let (deserialized_bank, ..) = snapshot_bank_utils::bank_from_latest_snapshot_archives( &snapshot_test_config.snapshot_config.bank_snapshots_dir, &snapshot_test_config @@ -841,7 +853,7 @@ fn test_snapshots_with_background_services( false, false, false, - Some(ACCOUNTS_DB_CONFIG_FOR_TESTING), + Some(accounts_db_config), None, exit.clone(), ) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index de8923f378b10a..01638b913cec97 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5606,8 +5606,14 @@ impl Bank { &self, base: Option<(Slot, /*capitalization*/ u64)>, mut config: VerifyAccountsHashConfig, - duplicates_lt_hash: Option<&DuplicatesLtHash>, + duplicates_lt_hash: Option>, ) -> bool { + #[derive(Debug, Eq, PartialEq)] + enum VerifyKind { + Merkle, + Lattice, + } + let accounts = &self.rc.accounts; // Wait until initial hash calc is complete before starting a new hash calc. // This should only occur when we halt at a slot in ledger-tool. @@ -5617,14 +5623,33 @@ impl Bank { .wait_for_complete(); let slot = self.slot(); - let is_accounts_lt_hash_enabled = self.is_accounts_lt_hash_enabled(); + let verify_kind = if self + .rc + .accounts + .accounts_db + .is_experimental_accumulator_hash_enabled() + { + VerifyKind::Lattice + } else { + VerifyKind::Merkle + }; + + if verify_kind == VerifyKind::Lattice { + // 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; + } + } + 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(), ); - if is_accounts_lt_hash_enabled { + if verify_kind == VerifyKind::Lattice { // 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). @@ -5638,15 +5663,6 @@ impl Bank { } } - 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. @@ -5673,7 +5689,6 @@ impl Bank { 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()) @@ -5682,49 +5697,53 @@ impl Bank { let start = Instant::now(); let mut lattice_verify_time = None; let mut merkle_verify_time = None; - if is_accounts_lt_hash_enabled { - // accounts lt hash is *enabled* so use lattice-based verification - 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; + match verify_kind { + VerifyKind::Lattice => { + // accounts lt hash is *enabled* so use lattice-based verification + 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 { + let expected = expected_accounts_lt_hash.0.checksum(); + let calculated = calculated_accounts_lt_hash.0.checksum(); + error!( + "Verifying accounts failed: accounts lattice hashes do not \ + match, expected: {expected}, calculated: {calculated}", + ); + return false; + } + lattice_verify_time = Some(duration); } - lattice_verify_time = Some(duration); - } else { - // accounts lt hash is *disabled* so use merkle-based verification - let snapshot_storages_and_slots = ( - snapshot_storages.0.as_slice(), - snapshot_storages.1.as_slice(), - ); - let (result, duration) = meas_dur!(accounts_ - .verify_accounts_hash_and_lamports( - snapshot_storages_and_slots, - slot, - capitalization, - base, - VerifyAccountsHashAndLamportsConfig { - ancestors: &ancestors, - epoch_schedule: &epoch_schedule, - rent_collector: &rent_collector, - ..verify_config - }, - )); - if !result { - return false; + VerifyKind::Merkle => { + // accounts lt hash is *disabled* so use merkle-based verification + let snapshot_storages_and_slots = ( + snapshot_storages.0.as_slice(), + snapshot_storages.1.as_slice(), + ); + let (result, duration) = meas_dur!(accounts_ + .verify_accounts_hash_and_lamports( + snapshot_storages_and_slots, + slot, + capitalization, + base, + VerifyAccountsHashAndLamportsConfig { + ancestors: &ancestors, + epoch_schedule: &epoch_schedule, + rent_collector: &rent_collector, + ..verify_config + }, + )); + if !result { + return false; + } + merkle_verify_time = Some(duration); } - merkle_verify_time = Some(duration); } accounts_ .accounts_db @@ -5751,45 +5770,50 @@ 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 { + match verify_kind { + VerifyKind::Lattice => { + 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, + &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(), + let is_ok = calculated_accounts_lt_hash == expected_accounts_lt_hash; + if !is_ok { + let expected = expected_accounts_lt_hash.0.checksum(); + let calculated = calculated_accounts_lt_hash.0.checksum(); + error!( + "Verifying accounts failed: accounts lattice hashes do not \ + match, expected: {expected}, calculated: {calculated}", + ); + } + is_ok + } + VerifyKind::Merkle => { + 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, ); - return false; + self.set_initial_accounts_hash_verification_completed(); + result } - // 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(), - ); - 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 } } @@ -6108,7 +6132,7 @@ impl Bank { force_clean: bool, latest_full_snapshot_slot: Slot, base: Option<(Slot, /*capitalization*/ u64)>, - duplicates_lt_hash: Option<&DuplicatesLtHash>, + duplicates_lt_hash: Option>, ) -> bool { let (_, clean_time_us) = measure_us!({ let should_clean = force_clean || (!skip_shrink && self.slot() > 0); diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index b6517961f681b8..11ac433507b6fb 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -549,7 +549,7 @@ pub(crate) fn fields_from_streams( /// This struct contains side-info while reconstructing the bank from streams #[derive(Debug)] pub struct BankFromStreamsInfo { - pub duplicates_lt_hash: Box, + pub duplicates_lt_hash: Option>, } #[allow(clippy::too_many_arguments)] @@ -842,7 +842,7 @@ impl<'a> solana_frozen_abi::abi_example::TransparentAsHelper for SerializableAcc /// This struct contains side-info while reconstructing the bank from fields #[derive(Debug)] struct ReconstructedBankInfo { - duplicates_lt_hash: Box, + duplicates_lt_hash: Option>, } #[allow(clippy::too_many_arguments)] @@ -1037,7 +1037,7 @@ pub(crate) fn remap_and_reconstruct_single_storage( #[derive(Debug, Default, Clone)] pub struct ReconstructedAccountsDbInfo { pub accounts_data_len: u64, - pub duplicates_lt_hash: Box, + pub duplicates_lt_hash: Option>, } #[allow(clippy::too_many_arguments)] diff --git a/runtime/src/snapshot_bank_utils.rs b/runtime/src/snapshot_bank_utils.rs index 9d6546a9e8cf18..f86ccda9d57446 100644 --- a/runtime/src/snapshot_bank_utils.rs +++ b/runtime/src/snapshot_bank_utils.rs @@ -235,7 +235,7 @@ pub fn bank_from_snapshot_archives( accounts_db_force_initial_clean, full_snapshot_archive_info.slot(), base, - Some(&info.duplicates_lt_hash), + info.duplicates_lt_hash, ) && limit_load_slot_count_from_snapshot.is_none() { panic!("Snapshot bank for slot {} failed to verify", bank.slot()); @@ -548,7 +548,7 @@ fn deserialize_status_cache( /// This struct contains side-info from rebuilding the bank #[derive(Debug)] struct RebuiltBankInfo { - duplicates_lt_hash: Box, + duplicates_lt_hash: Option>, } #[allow(clippy::too_many_arguments)]