diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index dc5946eac31016..24166549bb9505 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -8597,6 +8597,7 @@ impl AccountsDb { limit_load_slot_count_from_snapshot: Option, verify: bool, genesis_config: &GenesisConfig, + should_calculate_duplicates_lt_hash: bool, ) -> IndexGenerationInfo { let mut total_time = Measure::start("generate_index"); let mut slots = self.storage.all_slots(); @@ -8853,7 +8854,7 @@ impl AccountsDb { accounts_data_len_from_duplicates: u64, num_duplicate_accounts: u64, uncleaned_roots: IntSet, - duplicates_lt_hash: Box, + duplicates_lt_hash: Option>, } impl DuplicatePubkeysVisitedInfo { fn reduce(mut a: Self, mut b: Self) -> Self { @@ -8870,9 +8871,30 @@ 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); + + match ( + self.duplicates_lt_hash.is_some(), + other.duplicates_lt_hash.is_some(), + ) { + (true, true) => { + // SAFETY: We just checked that both values are Some + self.duplicates_lt_hash + .as_mut() + .unwrap() + .0 + .mix_in(&other.duplicates_lt_hash.as_ref().unwrap().0); + } + (true, false) => { + // nothing to do; `other` doesn't have a duplicates lt hash + } + (false, true) => { + // `self` doesn't have a duplicates lt hash, so pilfer from `other` + self.duplicates_lt_hash = other.duplicates_lt_hash; + } + (false, false) => { + // nothing to do; no duplicates lt hash at all + } + } } } @@ -8909,6 +8931,7 @@ impl AccountsDb { pubkeys, &rent_collector, &timings, + should_calculate_duplicates_lt_hash, ); let intermediate = DuplicatePubkeysVisitedInfo { accounts_data_len_from_duplicates, @@ -8937,7 +8960,7 @@ impl AccountsDb { self.accounts_index .add_uncleaned_roots(uncleaned_roots.into_iter()); accounts_data_len.fetch_sub(accounts_data_len_from_duplicates, Ordering::Relaxed); - if self.is_experimental_accumulator_hash_enabled() { + if let Some(duplicates_lt_hash) = duplicates_lt_hash { let old_val = outer_duplicates_lt_hash.replace(duplicates_lt_hash); assert!(old_val.is_none()); } @@ -9049,11 +9072,13 @@ impl AccountsDb { pubkeys: &[Pubkey], rent_collector: &RentCollector, timings: &GenerateIndexTimings, - ) -> (u64, u64, IntSet, Box) { + should_calculate_duplicates_lt_hash: bool, + ) -> (u64, u64, IntSet, Option>) { 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 duplicates_lt_hash = + should_calculate_duplicates_lt_hash.then(|| Box::new(DuplicatesLtHash::default())); let mut removed_rent_paying = 0; let mut removed_top_off = 0; let mut lt_hash_time = Duration::default(); @@ -9097,7 +9122,7 @@ impl AccountsDb { removed_rent_paying += 1; removed_top_off += lamports_to_top_off; } - if self.is_experimental_accumulator_hash_enabled() { + if let Some(duplicates_lt_hash) = duplicates_lt_hash.as_mut() { let (_, duration) = meas_dur!({ let account_lt_hash = Self::lt_hash_account(&loaded_account, pubkey); @@ -9696,7 +9721,7 @@ pub mod tests { let genesis_config = GenesisConfig::default(); assert!(!db.accounts_index.contains(&pubkey)); - let result = db.generate_index(None, false, &genesis_config); + let result = db.generate_index(None, false, &genesis_config, false); // index entry should only contain a single entry for the pubkey since index cannot hold more than 1 entry per slot let entry = db.accounts_index.get_cloned(&pubkey).unwrap(); assert_eq!(entry.slot_list.read().unwrap().len(), 1); @@ -9736,7 +9761,7 @@ pub mod tests { append_vec.accounts.append_accounts(&storable_accounts, 0); let genesis_config = GenesisConfig::default(); assert!(!db.accounts_index.contains(&pubkey)); - let result = db.generate_index(None, false, &genesis_config); + let result = db.generate_index(None, false, &genesis_config, false); let entry = db.accounts_index.get_cloned(&pubkey).unwrap(); assert_eq!(entry.slot_list.read().unwrap().len(), 1); assert_eq!(append_vec.alive_bytes(), aligned_stored_size(0)); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 46f734099f286a..a72b2ec66284d7 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1731,6 +1731,7 @@ impl Bank { // When there is a feature gate for the accounts lt hash, if the feature is enabled // then it will be *required* that the snapshot contains an accounts lt hash. let accounts_lt_hash = fields.accounts_lt_hash.unwrap_or_else(|| { + info!("Calculating the accounts lt hash..."); let (accounts_lt_hash, duration) = meas_dur!({ thread_pool.install(|| { bank.rc @@ -1742,6 +1743,7 @@ impl Bank { ) }) }); + info!("Calculating the accounts lt hash... Done in {duration:?}"); calculate_accounts_lt_hash_duration = Some(duration); accounts_lt_hash }); @@ -5619,25 +5621,24 @@ impl Bank { .wait_for_complete(); let slot = self.slot(); - 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() { + let verify_kind = match ( + duplicates_lt_hash.is_some(), + self.rc + .accounts + .accounts_db + .is_experimental_accumulator_hash_enabled(), + ) { + (true, _) => VerifyKind::Lattice, + (false, false) => VerifyKind::Merkle, + (false, true) => { + // 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. config.run_in_background = false; + VerifyKind::Lattice } - } + }; if config.require_rooted_bank && !accounts.accounts_db.accounts_index.is_alive_root(slot) { if let Some(parent) = self.parent() { @@ -5678,6 +5679,10 @@ impl Bank { use_bg_thread_pool: config.run_in_background, }; + info!( + "Verifying accounts, in background? {}, verify kind: {verify_kind:?}", + config.run_in_background, + ); if config.run_in_background { let accounts = Arc::clone(accounts); let accounts_ = Arc::clone(&accounts); diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index 7d6f5aa34a6ca6..841408e62e73c5 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -884,6 +884,7 @@ where bank_fields.epoch_accounts_hash, capitalizations, bank_fields.incremental_snapshot_persistence.as_ref(), + bank_fields.accounts_lt_hash.is_some(), )?; let bank_rc = BankRc::new(Accounts::new(Arc::new(accounts_db))); @@ -1049,6 +1050,7 @@ fn reconstruct_accountsdb_from_fields( epoch_accounts_hash: Option, capitalizations: (u64, Option), incremental_snapshot_persistence: Option<&BankIncrementalSnapshotPersistence>, + has_accounts_lt_hash: bool, ) -> Result<(AccountsDb, ReconstructedAccountsDbInfo), Error> where E: SerializableStorage + std::marker::Sync, @@ -1232,6 +1234,11 @@ where }) .unwrap(); + // When generating the index, we want to calculate the duplicates lt hash value (needed to do + // the lattice-based verification of the accounts in the background) optimistically. + // This means, either when the cli arg is set, or when the snapshot has an accounts lt hash. + let is_accounts_lt_hash_enabled = + accounts_db.is_experimental_accumulator_hash_enabled() || has_accounts_lt_hash; let IndexGenerationInfo { accounts_data_len, rent_paying_accounts_by_partition, @@ -1240,6 +1247,7 @@ where limit_load_slot_count_from_snapshot, verify_index, genesis_config, + is_accounts_lt_hash_enabled, ); accounts_db .accounts_index diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index 19df0b52877c74..9fcf3f81818103 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -83,6 +83,7 @@ mod serde_snapshot_tests { None, (u64::default(), None), None, + false, ) .map(|(accounts_db, _)| accounts_db) }