Refactors accounts lt hash startup verification (#3397)

accounts-db/src/accounts_db.rs

@@ -8597,6 +8597,7 @@ impl AccountsDb {
         limit_load_slot_count_from_snapshot: Option<usize>,
         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<Slot>,
-                    duplicates_lt_hash: Box<DuplicatesLtHash>,
+                    duplicates_lt_hash: Option<Box<DuplicatesLtHash>>,
                 }
                 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<Slot>, Box<DuplicatesLtHash>) {
+        should_calculate_duplicates_lt_hash: bool,
+    ) -> (u64, u64, IntSet<Slot>, Option<Box<DuplicatesLtHash>>) {
         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));

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);

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<E>(
     epoch_accounts_hash: Option<Hash>,
     capitalizations: (u64, Option<u64>),
     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

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)
     }