Bank::new_from_fields() always gets the accounts lt hash from the sna…

…pshot (#3519)
anza-xyz · Nov 7, 2024 · 7688d91 · 7688d91
1 parent 2a618b5
commit 7688d91
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 15 deletions.
diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs
@@ -6677,8 +6677,6 @@ impl AccountsDb {
         ancestors: &Ancestors,
         startup_slot: Slot,
     ) -> AccountsLtHash {
-        debug_assert!(self.is_experimental_accumulator_hash_enabled());
-
         // This impl iterates over all the index bins in parallel, and computes the lt hash
         // sequentially per bin.  Then afterwards reduces to a single lt hash.
         // This implementation is quite fast.  Runtime is about 150 seconds on mnb as of 10/2/2024.
@@ -6744,8 +6742,6 @@ impl AccountsDb {
         storages: &[Arc<AccountStorageEntry>],
         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| {

diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
@@ -1730,11 +1730,13 @@ impl Bank {
         bank.rebuild_skipped_rewrites();
 
         let mut calculate_accounts_lt_hash_duration = None;
-        if bank.is_accounts_lt_hash_enabled() {
+        if let Some(accounts_lt_hash) = fields.accounts_lt_hash {
+            *bank.accounts_lt_hash.get_mut().unwrap() = accounts_lt_hash;
+        } else {
             // Use the accounts lt hash from the snapshot, if present, otherwise calculate it.
             // 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(|| {
+            if bank.is_accounts_lt_hash_enabled() {
                 info!("Calculating the accounts lt hash...");
                 let (ancestors, slot) = if bank.is_frozen() {
                     // Loading from a snapshot necessarily means this slot was rooted, and thus
@@ -1762,11 +1764,10 @@ impl Bank {
                             .calculate_accounts_lt_hash_at_startup_from_index(&ancestors, slot)
                     })
                 });
-                info!("Calculating the accounts lt hash... Done in {duration:?}");
                 calculate_accounts_lt_hash_duration = Some(duration);
-                accounts_lt_hash
-            });
-            *bank.accounts_lt_hash.get_mut().unwrap() = accounts_lt_hash;
+                *bank.accounts_lt_hash.get_mut().unwrap() = accounts_lt_hash;
+                info!("Calculating the accounts lt hash... Done in {duration:?}");
+            }
         }
 
         // Sanity assertions between bank snapshot and genesis config

diff --git a/runtime/src/bank/accounts_lt_hash.rs b/runtime/src/bank/accounts_lt_hash.rs
@@ -316,7 +316,7 @@ mod tests {
         },
         std::{cmp, collections::HashMap, ops::RangeFull, str::FromStr as _, sync::Arc},
         tempfile::TempDir,
-        test_case::test_case,
+        test_case::{test_case, test_matrix},
     };
 
     /// What features should be enabled?
@@ -328,6 +328,15 @@ mod tests {
         All,
     }
 
+    /// Should the experimental accumulator hash cli arg be enabled?
+    #[derive(Debug, Copy, Clone, Eq, PartialEq)]
+    enum Cli {
+        /// Do not enable the cli arg
+        Off,
+        /// Enable the cli arg
+        On,
+    }
+
     /// Creates a genesis config with `features` enabled
     fn genesis_config_with(features: Features) -> (GenesisConfig, Keypair) {
         let mint_lamports = 123_456_789 * LAMPORTS_PER_SOL;
@@ -812,9 +821,11 @@ mod tests {
         );
     }
 
-    #[test_case(Features::None; "no features")]
-    #[test_case(Features::All; "all features")]
-    fn test_verify_accounts_lt_hash_at_startup(features: Features) {
+    #[test_matrix(
+        [Features::None, Features::All],
+        [Cli::Off, Cli::On]
+    )]
+    fn test_verify_accounts_lt_hash_at_startup(features: Features, verify_cli: Cli) {
         let (genesis_config, mint_keypair) = genesis_config_with(features);
         let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);
         bank.rc
@@ -869,7 +880,10 @@ mod tests {
         .unwrap();
         let (_accounts_tempdir, accounts_dir) = snapshot_utils::create_tmp_accounts_dir_for_tests();
         let accounts_db_config = AccountsDbConfig {
-            enable_experimental_accumulator_hash: true,
+            enable_experimental_accumulator_hash: match verify_cli {
+                Cli::Off => false,
+                Cli::On => true,
+            },
             ..ACCOUNTS_DB_CONFIG_FOR_TESTING
         };
         let (roundtrip_bank, _) = snapshot_bank_utils::bank_from_snapshot_archives(