Эх сурвалжийг харах

Removes merkle-based startup accounts verification (#6975)

Brooks 4 сар өмнө
parent
commit
d045b04bc3
2 өөрчлөгдсөн 64 нэмэгдсэн , 194 устгасан
  1. 53 140
      runtime/src/bank.rs
  2. 11 54
      runtime/src/bank/tests.rs

+ 53 - 140
runtime/src/bank.rs

@@ -76,7 +76,7 @@ use {
         accounts::{AccountAddressFilter, Accounts, PubkeyAccountSlot},
         accounts_db::{
             AccountStorageEntry, AccountsDb, AccountsDbConfig, CalcAccountsHashDataSource,
-            DuplicatesLtHash, PubkeyHashAccount, VerifyAccountsHashAndLamportsConfig,
+            DuplicatesLtHash, PubkeyHashAccount,
         },
         accounts_hash::{
             AccountsHash, AccountsLtHash, CalcAccountsHashConfig, HashStats,
@@ -198,10 +198,8 @@ pub use {partitioned_epoch_rewards::KeyedRewardsAndNumPartitions, solana_reward_
 
 /// params to `verify_accounts_hash`
 struct VerifyAccountsHashConfig {
-    ignore_mismatch: bool,
     require_rooted_bank: bool,
     run_in_background: bool,
-    store_hash_raw_data_for_debug: bool,
 }
 
 mod accounts_lt_hash;
@@ -4580,16 +4578,16 @@ impl Bank {
 
     /// Used by ledger tool to run a final hash calculation once all ledger replay has completed.
     /// This should not be called by validator code.
-    pub fn run_final_hash_calc(&self, on_halt_store_hash_raw_data_for_debug: bool) {
+    pub fn run_final_hash_calc(
+        &self,
+        _on_halt_store_hash_raw_data_for_debug: bool, // will be removed next
+    ) {
         self.force_flush_accounts_cache();
         // note that this slot may not be a root
         _ = self.verify_accounts_hash(
-            None,
             VerifyAccountsHashConfig {
-                ignore_mismatch: true,
                 require_rooted_bank: false,
                 run_in_background: false,
-                store_hash_raw_data_for_debug: on_halt_store_hash_raw_data_for_debug,
             },
             None,
         );
@@ -4601,17 +4599,9 @@ impl Bank {
     #[must_use]
     fn verify_accounts_hash(
         &self,
-        base: Option<(Slot, /*capitalization*/ u64)>,
         mut config: VerifyAccountsHashConfig,
         duplicates_lt_hash: Option<Box<DuplicatesLtHash>>,
     ) -> bool {
-        #[derive(Debug, Eq, PartialEq)]
-        enum VerifyKind {
-            #[allow(dead_code)] // will be removed next
-            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.
@@ -4622,7 +4612,6 @@ impl Bank {
 
         let slot = self.slot();
 
-        let verify_kind = VerifyKind::Lattice;
         if duplicates_lt_hash.is_none() {
             // 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
@@ -4636,13 +4625,11 @@ impl Bank {
                     "slot {slot} is not a root, so verify accounts hash on parent bank at slot {}",
                     parent.slot(),
                 );
-                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).
-                    config.run_in_background = false;
-                }
-                return parent.verify_accounts_hash(base, config, None);
+                // 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(config, None);
             } else {
                 // this will result in mismatch errors
                 // accounts hash calc doesn't include unrooted slots
@@ -4654,25 +4641,14 @@ impl Bank {
         // 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.
         let snapshot_storages = self.rc.accounts.accounts_db.get_storages(RangeFull);
-        let capitalization = self.capitalization();
-        let verify_config = VerifyAccountsHashAndLamportsConfig {
-            ancestors: &self.ancestors,
-            epoch_schedule: self.epoch_schedule(),
-            epoch: self.epoch(),
-            ignore_mismatch: config.ignore_mismatch,
-            store_detailed_debug_info: config.store_hash_raw_data_for_debug,
-            use_bg_thread_pool: config.run_in_background,
-        };
 
         info!(
-            "Verifying accounts, in background? {}, verify kind: {verify_kind:?}",
+            "Verifying accounts, in background? {}",
             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 expected_accounts_lt_hash = self.accounts_lt_hash.lock().unwrap().clone();
             accounts.accounts_db.verify_accounts_hash_in_bg.start(|| {
                 Builder::new()
@@ -4680,71 +4656,32 @@ impl Bank {
                     .spawn(move || {
                         info!("Initial background accounts hash verification has started");
                         let start = Instant::now();
-                        let mut lattice_verify_time = None;
-                        let mut merkle_verify_time = None;
-                        let is_ok = 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(),
-                                            )
-                                    }));
-                                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}",
-                                    );
-                                }
-                                lattice_verify_time = Some(duration);
-                                is_ok
-                            }
-                            VerifyKind::Merkle => {
-                                // accounts lt hash is *disabled* so use merkle-based verification
-                                let snapshot_storages_and_slots = (
+                        let accounts_db = &accounts_.accounts_db;
+                        let (calculated_accounts_lt_hash, lattice_verify_time) =
+                            meas_dur!(accounts_db.thread_pool_hash.install(|| {
+                                accounts_db.calculate_accounts_lt_hash_at_startup_from_storages(
                                     snapshot_storages.0.as_slice(),
-                                    snapshot_storages.1.as_slice(),
-                                );
-                                let (is_ok, duration) = meas_dur!(accounts_
-                                    .verify_accounts_hash_and_lamports(
-                                        snapshot_storages_and_slots,
-                                        slot,
-                                        capitalization,
-                                        base,
-                                        VerifyAccountsHashAndLamportsConfig {
-                                            ancestors: &ancestors,
-                                            epoch_schedule: &epoch_schedule,
-                                            ..verify_config
-                                        },
-                                    ));
-                                merkle_verify_time = Some(duration);
-                                is_ok
-                            }
-                        };
-                        accounts_
-                            .accounts_db
-                            .verify_accounts_hash_in_bg
-                            .background_finished();
+                                    &duplicates_lt_hash.unwrap(),
+                                )
+                            }));
+                        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}",
+                            );
+                        }
+                        accounts_db.verify_accounts_hash_in_bg.background_finished();
                         let total_time = start.elapsed();
                         datapoint_info!(
                             "startup_verify_accounts",
                             ("total_us", total_time.as_micros(), i64),
                             (
                                 "verify_accounts_lt_hash_us",
-                                lattice_verify_time.as_ref().map(Duration::as_micros),
-                                Option<i64>
-                            ),
-                            ("verify_accounts_hash_us",
-                                merkle_verify_time.as_ref().map(Duration::as_micros),
-                                Option<i64>
+                                lattice_verify_time.as_micros(),
+                                i64
                             ),
                         );
                         info!("Initial background accounts hash verification has stopped");
@@ -4754,51 +4691,30 @@ impl Bank {
             });
             true // initial result is true. We haven't failed yet. If verification fails, we'll panic from bg thread.
         } else {
-            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,
-                            )
-                    } else {
-                        accounts
-                            .accounts_db
-                            .calculate_accounts_lt_hash_at_startup_from_index(&self.ancestors, slot)
-                    };
-                    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}",
-                        );
-                    }
-                    self.set_initial_accounts_hash_verification_completed();
-                    is_ok
-                }
-                VerifyKind::Merkle => {
-                    let snapshot_storages_and_slots = (
+            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(),
-                        snapshot_storages.1.as_slice(),
-                    );
-                    let is_ok = accounts.verify_accounts_hash_and_lamports(
-                        snapshot_storages_and_slots,
-                        slot,
-                        capitalization,
-                        base,
-                        verify_config,
-                    );
-                    self.set_initial_accounts_hash_verification_completed();
-                    is_ok
-                }
+                        &duplicates_lt_hash,
+                    )
+            } else {
+                accounts
+                    .accounts_db
+                    .calculate_accounts_lt_hash_at_startup_from_index(&self.ancestors, slot)
+            };
+            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}",
+                );
             }
+            self.set_initial_accounts_hash_verification_completed();
+            is_ok
         }
     }
 
@@ -5102,7 +5018,7 @@ impl Bank {
         skip_shrink: bool,
         force_clean: bool,
         latest_full_snapshot_slot: Slot,
-        base: Option<(Slot, /*capitalization*/ u64)>,
+        _base: Option<(Slot, /*capitalization*/ u64)>, // will be removed next
         duplicates_lt_hash: Option<Box<DuplicatesLtHash>>,
     ) -> bool {
         // If we verify the accounts using the lattice-based hash *and* with storages (as opposed
@@ -5117,12 +5033,9 @@ impl Bank {
             if should_verify_accounts {
                 info!("Verifying accounts...");
                 let verified = self.verify_accounts_hash(
-                    base,
                     VerifyAccountsHashConfig {
-                        ignore_mismatch: false,
                         require_rooted_bank: false,
                         run_in_background: true,
-                        store_hash_raw_data_for_debug: false,
                     },
                     duplicates_lt_hash,
                 );

+ 11 - 54
runtime/src/bank/tests.rs

@@ -943,10 +943,8 @@ fn test_bank_update_rewards_determinism() {
 impl VerifyAccountsHashConfig {
     fn default_for_test() -> Self {
         Self {
-            ignore_mismatch: false,
             require_rooted_bank: false,
             run_in_background: false,
-            store_hash_raw_data_for_debug: false,
         }
     }
 }
@@ -1020,11 +1018,7 @@ 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(),
-                None,
-            ));
+            assert!(bank0.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
             continue;
         }
 
@@ -1033,11 +1027,7 @@ 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(),
-                None,
-            ));
+            assert!(bank0.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
             continue;
         }
 
@@ -1045,11 +1035,7 @@ 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(),
-            None,
-        ));
+        assert!(bank1.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
 
         // keypair should have 0 tokens on both forks
         assert_eq!(bank0.get_account(&keypair.pubkey()), None);
@@ -1057,11 +1043,7 @@ fn test_purge_empty_accounts() {
 
         bank1.clean_accounts_for_tests();
 
-        assert!(bank1.verify_accounts_hash(
-            None,
-            VerifyAccountsHashConfig::default_for_test(),
-            None,
-        ));
+        assert!(bank1.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
     }
 }
 
@@ -2198,7 +2180,7 @@ fn test_bank_hash_internal_state() {
     bank2.squash();
     bank2.force_flush_accounts_cache();
     bank2.update_accounts_hash(CalcAccountsHashDataSource::Storages, false);
-    assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test(), None,));
+    assert!(bank2.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
 }
 
 #[test]
@@ -2233,11 +2215,7 @@ fn test_bank_hash_internal_state_verify() {
             bank2.freeze();
             add_root_and_flush_write_cache(&bank2);
             bank2.update_accounts_hash_for_tests();
-            assert!(bank2.verify_accounts_hash(
-                None,
-                VerifyAccountsHashConfig::default_for_test(),
-                None,
-            ));
+            assert!(bank2.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
         }
         let bank3 = new_bank_from_parent_with_bank_forks(
             &bank_forks,
@@ -2248,11 +2226,7 @@ fn test_bank_hash_internal_state_verify() {
         assert_eq!(bank0_state, bank0.hash_internal_state());
         if pass == 0 {
             // this relies on us having set bank2's accounts hash in the pass==0 if above
-            assert!(bank2.verify_accounts_hash(
-                None,
-                VerifyAccountsHashConfig::default_for_test(),
-                None,
-            ));
+            assert!(bank2.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
             continue;
         }
         if pass == 1 {
@@ -2262,11 +2236,7 @@ fn test_bank_hash_internal_state_verify() {
             bank3.freeze();
             add_root_and_flush_write_cache(&bank3);
             bank3.update_accounts_hash_for_tests();
-            assert!(bank3.verify_accounts_hash(
-                None,
-                VerifyAccountsHashConfig::default_for_test(),
-                None,
-            ));
+            assert!(bank3.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
             continue;
         }
 
@@ -2276,11 +2246,7 @@ fn test_bank_hash_internal_state_verify() {
         if pass == 2 {
             add_root_and_flush_write_cache(&bank2);
             bank2.update_accounts_hash_for_tests();
-            assert!(bank2.verify_accounts_hash(
-                None,
-                VerifyAccountsHashConfig::default_for_test(),
-                None,
-            ));
+            assert!(bank2.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
 
             // Verifying the accounts lt hash is only intended to be called at startup, and
             // normally in the background.  Since here we're *not* at startup, and doing it
@@ -2296,11 +2262,7 @@ fn test_bank_hash_internal_state_verify() {
         bank3.freeze();
         add_root_and_flush_write_cache(&bank3);
         bank3.update_accounts_hash_for_tests();
-        assert!(bank3.verify_accounts_hash(
-            None,
-            VerifyAccountsHashConfig::default_for_test(),
-            None,
-        ));
+        assert!(bank3.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
     }
 }
 
@@ -10877,7 +10839,6 @@ fn test_bank_verify_accounts_hash_with_base() {
     bank.force_flush_accounts_cache();
     bank.update_accounts_hash(CalcAccountsHashDataSource::Storages, false);
     let base_slot = bank.slot();
-    let base_capitalization = bank.capitalization();
 
     // make more banks, do more transactions, ensure there's more zero-lamport accounts
     for _ in 0..2 {
@@ -10897,11 +10858,7 @@ fn test_bank_verify_accounts_hash_with_base() {
     bank.update_incremental_accounts_hash(base_slot);
 
     // ensure the accounts hash verifies
-    assert!(bank.verify_accounts_hash(
-        Some((base_slot, base_capitalization)),
-        VerifyAccountsHashConfig::default_for_test(),
-        None,
-    ));
+    assert!(bank.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test(), None));
 }
 
 #[test]