From 17b48edd7b863dada0a4ebacf3e161d2dea78fad Mon Sep 17 00:00:00 2001 From: Brooks Date: Fri, 10 Mar 2023 20:02:14 -0500 Subject: [PATCH] Renames types to be consistent with verify_accounts_hash (#30674) --- ledger/src/blockstore_processor.rs | 4 +- runtime/benches/accounts.rs | 5 +- runtime/src/accounts.rs | 7 +-- runtime/src/accounts_db.rs | 73 +++++++++++++++--------------- runtime/src/bank.rs | 14 +++--- runtime/src/bank/tests.rs | 22 ++++----- 6 files changed, 63 insertions(+), 62 deletions(-) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 3ba390ec06..55322eb128 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -24,7 +24,7 @@ use { accounts_update_notifier_interface::AccountsUpdateNotifier, bank::{ Bank, RentDebits, TransactionBalancesSet, TransactionExecutionDetails, - TransactionExecutionResult, TransactionResults, VerifyBankHash, + TransactionExecutionResult, TransactionResults, VerifyAccountsHashConfig, }, bank_forks::BankForks, bank_utils, @@ -1533,7 +1533,7 @@ fn load_frozen_forks( fn run_final_hash_calc(bank: &Bank, on_halt_store_hash_raw_data_for_debug: bool) { bank.force_flush_accounts_cache(); // note that this slot may not be a root - let _ = bank.verify_accounts_hash(VerifyBankHash { + let _ = bank.verify_accounts_hash(VerifyAccountsHashConfig { test_hash_calculation: false, ignore_mismatch: true, require_rooted_bank: false, diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs index 49a6de6e57..8ae698214d 100644 --- a/runtime/benches/accounts.rs +++ b/runtime/benches/accounts.rs @@ -10,7 +10,8 @@ use { solana_runtime::{ accounts::{AccountAddressFilter, Accounts}, accounts_db::{ - test_utils::create_test_accounts, AccountShrinkThreshold, BankHashLamportsVerifyConfig, + test_utils::create_test_accounts, AccountShrinkThreshold, + VerifyAccountsHashAndLamportsConfig, }, accounts_index::{AccountSecondaryIndexes, ScanConfig}, ancestors::Ancestors, @@ -103,7 +104,7 @@ fn test_accounts_hash_bank_hash(bencher: &mut Bencher) { assert!(accounts.verify_accounts_hash_and_lamports( 0, total_lamports, - BankHashLamportsVerifyConfig { + VerifyAccountsHashAndLamportsConfig { ancestors: &ancestors, test_hash_calculation, epoch_schedule: &EpochSchedule::default(), diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index b6a84af0d9..7c9064058d 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -4,8 +4,9 @@ use { account_rent_state::{check_rent_state_with_account, RentState}, accounts_db::{ AccountShrinkThreshold, AccountsAddRootTiming, AccountsDb, AccountsDbConfig, - BankHashLamportsVerifyConfig, IncludeSlotInHash, LoadHint, LoadedAccount, - ScanStorageResult, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING, + IncludeSlotInHash, LoadHint, LoadedAccount, ScanStorageResult, + VerifyAccountsHashAndLamportsConfig, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, + ACCOUNTS_DB_CONFIG_FOR_TESTING, }, accounts_index::{ AccountSecondaryIndexes, IndexKey, ScanConfig, ScanError, ScanResult, ZeroLamport, @@ -866,7 +867,7 @@ impl Accounts { &self, slot: Slot, total_lamports: u64, - config: BankHashLamportsVerifyConfig, + config: VerifyAccountsHashAndLamportsConfig, ) -> bool { if let Err(err) = self.accounts_db diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 10aae9c060..64867ec1c5 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -196,9 +196,9 @@ pub(crate) struct ShrinkCollectAliveSeparatedByRefs<'a> { pub(crate) many_refs: AliveAccounts<'a>, } -/// Configuration Parameters for running bank hash and total lamports verification +/// Configuration Parameters for running accounts hash and total lamports verification #[derive(Debug, Clone)] -pub struct BankHashLamportsVerifyConfig<'a> { +pub struct VerifyAccountsHashAndLamportsConfig<'a> { /// bank ancestors pub ancestors: &'a Ancestors, /// true to verify hash calculation @@ -985,10 +985,9 @@ impl<'a> ReadableAccount for LoadedAccount<'a> { } #[derive(Debug)] -pub enum BankHashVerificationError { - MismatchedAccountHash, - MismatchedBankHash, - MissingBankHash, +pub enum AccountsHashVerificationError { + MissingAccountsHash, + MismatchedAccountsHash, MismatchedTotalLamports(u64, u64), } @@ -6856,8 +6855,8 @@ impl AccountsDb { &self, max_slot: Slot, config: &CalcAccountsHashConfig<'_>, - ) -> Result<(AccountsHash, u64), BankHashVerificationError> { - use BankHashVerificationError::*; + ) -> Result<(AccountsHash, u64), AccountsHashVerificationError> { + use AccountsHashVerificationError::*; let mut collect = Measure::start("collect"); let keys: Vec<_> = self .accounts_index @@ -6953,7 +6952,7 @@ impl AccountsDb { "{} mismatched account hash(es) found", mismatch_found.load(Ordering::Relaxed) ); - return Err(MismatchedAccountHash); + return Err(MismatchedAccountsHash); } scan.stop(); @@ -7255,7 +7254,7 @@ impl AccountsDb { data_source: CalcAccountsHashDataSource, slot: Slot, config: &CalcAccountsHashConfig<'_>, - ) -> Result<(AccountsHash, u64), BankHashVerificationError> { + ) -> Result<(AccountsHash, u64), AccountsHashVerificationError> { match data_source { CalcAccountsHashDataSource::Storages => { if self.accounts_cache.contains_any_slots(slot) { @@ -7298,7 +7297,7 @@ impl AccountsDb { slot: Slot, config: CalcAccountsHashConfig<'_>, expected_capitalization: Option, - ) -> Result<(AccountsHash, u64), BankHashVerificationError> { + ) -> Result<(AccountsHash, u64), AccountsHashVerificationError> { let (accounts_hash, total_lamports) = self.calculate_accounts_hash(data_source, slot, &config)?; if debug_verify { @@ -7385,7 +7384,7 @@ impl AccountsDb { bin_range: &Range, config: &CalcAccountsHashConfig<'_>, filler_account_suffix: Option<&Pubkey>, - ) -> Result, BankHashVerificationError> { + ) -> Result, AccountsHashVerificationError> { let bin_calculator = PubkeyBinCalculator24::new(bins); assert!(bin_range.start < bins && bin_range.end <= bins && bin_range.start < bin_range.end); let mut time = Measure::start("scan all accounts"); @@ -7424,7 +7423,7 @@ impl AccountsDb { "{} mismatched account hash(es) found", mismatch_found.load(Ordering::Relaxed) ); - return Err(BankHashVerificationError::MismatchedAccountHash); + return Err(AccountsHashVerificationError::MismatchedAccountsHash); } time.stop(); @@ -7489,7 +7488,7 @@ impl AccountsDb { config: &CalcAccountsHashConfig<'_>, storages: &SortedStorages<'_>, stats: HashStats, - ) -> Result<(AccountsHash, u64), BankHashVerificationError> { + ) -> Result<(AccountsHash, u64), AccountsHashVerificationError> { let (accounts_hash, capitalization) = self._calculate_accounts_hash_from_storages( config, storages, @@ -7518,7 +7517,7 @@ impl AccountsDb { storages: &SortedStorages<'_>, base_slot: Slot, stats: HashStats, - ) -> Result<(IncrementalAccountsHash, /* capitalization */ u64), BankHashVerificationError> + ) -> Result<(IncrementalAccountsHash, /* capitalization */ u64), AccountsHashVerificationError> { assert!(storages.range().start > base_slot, "The storages for calculating an incremental accounts hash must all be higher than the base slot"); let (accounts_hash, capitalization) = self._calculate_accounts_hash_from_storages( @@ -7541,7 +7540,7 @@ impl AccountsDb { mut stats: HashStats, flavor: CalcAccountsHashFlavor, accounts_hash_cache_path: PathBuf, - ) -> Result<(AccountsHashEnum, u64), BankHashVerificationError> { + ) -> Result<(AccountsHashEnum, u64), AccountsHashVerificationError> { let _guard = self.active_stats.activate(ActiveStatItem::Hash); stats.oldest_root = storages.range().start; @@ -7644,9 +7643,9 @@ impl AccountsDb { &self, slot: Slot, total_lamports: u64, - config: BankHashLamportsVerifyConfig, - ) -> Result<(), BankHashVerificationError> { - use BankHashVerificationError::*; + config: VerifyAccountsHashAndLamportsConfig, + ) -> Result<(), AccountsHashVerificationError> { + use AccountsHashVerificationError::*; let check_hash = false; // this will not be supported anymore let (calculated_accounts_hash, calculated_lamports) = self @@ -7683,10 +7682,10 @@ impl AccountsDb { "mismatched accounts hash for slot {slot}: \ {calculated_accounts_hash:?} (calculated) != {found_accounts_hash:?} (expected)" ); - Err(MismatchedBankHash) + Err(MismatchedAccountsHash) } } else { - Err(MissingBankHash) + Err(MissingAccountsHash) } } @@ -9498,7 +9497,7 @@ pub mod tests { bins: usize, bin_range: &Range, check_hash: bool, - ) -> Result, BankHashVerificationError> { + ) -> Result, AccountsHashVerificationError> { let temp_dir = TempDir::new().unwrap(); let accounts_hash_cache_path = temp_dir.path().to_path_buf(); self.scan_snapshot_stores_with_cache( @@ -9617,13 +9616,13 @@ pub mod tests { } } - impl<'a> BankHashLamportsVerifyConfig<'a> { + impl<'a> VerifyAccountsHashAndLamportsConfig<'a> { fn new_for_test( ancestors: &'a Ancestors, epoch_schedule: &'a EpochSchedule, rent_collector: &'a RentCollector, - ) -> BankHashLamportsVerifyConfig<'a> { - BankHashLamportsVerifyConfig { + ) -> VerifyAccountsHashAndLamportsConfig<'a> { + VerifyAccountsHashAndLamportsConfig { ancestors, test_hash_calculation: true, epoch_schedule, @@ -12189,7 +12188,7 @@ pub mod tests { let ancestors = Ancestors::default(); let epoch_schedule = EpochSchedule::default(); let rent_collector = RentCollector::default(); - let config = BankHashLamportsVerifyConfig::new_for_test( + let config = VerifyAccountsHashAndLamportsConfig::new_for_test( &ancestors, &epoch_schedule, &rent_collector, @@ -12597,7 +12596,7 @@ pub mod tests { #[test] fn test_verify_bank_hash() { - use BankHashVerificationError::*; + use AccountsHashVerificationError::*; solana_logger::setup(); let db = AccountsDb::new(Vec::new(), &ClusterType::Development); @@ -12613,7 +12612,7 @@ pub mod tests { db.add_root_and_flush_write_cache(some_slot); db.update_accounts_hash_for_tests(some_slot, &ancestors, true, true); - let config = BankHashLamportsVerifyConfig::new_for_test( + let config = VerifyAccountsHashAndLamportsConfig::new_for_test( &ancestors, &epoch_schedule, &rent_collector, @@ -12628,21 +12627,21 @@ pub mod tests { assert_matches!( db.verify_accounts_hash_and_lamports(some_slot, 1, config.clone()), - Err(MissingBankHash) + Err(MissingAccountsHash) ); db.set_accounts_hash(some_slot, AccountsHash(Hash::new(&[0xca; HASH_BYTES]))); assert_matches!( db.verify_accounts_hash_and_lamports(some_slot, 1, config), - Err(MismatchedBankHash) + Err(MismatchedAccountsHash) ); } #[test] fn test_verify_bank_capitalization() { for pass in 0..2 { - use BankHashVerificationError::*; + use AccountsHashVerificationError::*; solana_logger::setup(); let db = AccountsDb::new(Vec::new(), &ClusterType::Development); @@ -12653,7 +12652,7 @@ pub mod tests { let ancestors = vec![(some_slot, 0)].into_iter().collect(); let epoch_schedule = EpochSchedule::default(); let rent_collector = RentCollector::default(); - let config = BankHashLamportsVerifyConfig::new_for_test( + let config = VerifyAccountsHashAndLamportsConfig::new_for_test( &ancestors, &epoch_schedule, &rent_collector, @@ -12707,7 +12706,7 @@ pub mod tests { let epoch_schedule = EpochSchedule::default(); let rent_collector = RentCollector::default(); - let config = BankHashLamportsVerifyConfig::new_for_test( + let config = VerifyAccountsHashAndLamportsConfig::new_for_test( &ancestors, &epoch_schedule, &rent_collector, @@ -12721,7 +12720,7 @@ pub mod tests { #[test] fn test_verify_bank_hash_bad_account_hash() { - use BankHashVerificationError::*; + use AccountsHashVerificationError::*; solana_logger::setup(); let db = AccountsDb::new(Vec::new(), &ClusterType::Development); @@ -12747,7 +12746,7 @@ pub mod tests { let epoch_schedule = EpochSchedule::default(); let rent_collector = RentCollector::default(); - let config = BankHashLamportsVerifyConfig::new_for_test( + let config = VerifyAccountsHashAndLamportsConfig::new_for_test( &ancestors, &epoch_schedule, &rent_collector, @@ -12755,7 +12754,7 @@ pub mod tests { assert_matches!( db.verify_accounts_hash_and_lamports(some_slot, 1, config), - Err(MismatchedBankHash) + Err(MismatchedAccountsHash) ); } @@ -13286,7 +13285,7 @@ pub mod tests { let epoch_schedule = EpochSchedule::default(); let rent_collector = RentCollector::default(); - let config = BankHashLamportsVerifyConfig::new_for_test( + let config = VerifyAccountsHashAndLamportsConfig::new_for_test( &no_ancestors, &epoch_schedule, &rent_collector, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index eb2f39957d..f138451ecb 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -46,7 +46,7 @@ use { }, accounts_db::{ AccountShrinkThreshold, AccountStorageEntry, AccountsDbConfig, - BankHashLamportsVerifyConfig, CalcAccountsHashDataSource, IncludeSlotInHash, + CalcAccountsHashDataSource, IncludeSlotInHash, VerifyAccountsHashAndLamportsConfig, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING, }, accounts_hash::AccountsHash, @@ -181,8 +181,8 @@ use { }, }; -/// params to `verify_bank_hash` -pub struct VerifyBankHash { +/// params to `verify_accounts_hash` +pub struct VerifyAccountsHashConfig { pub test_hash_calculation: bool, pub ignore_mismatch: bool, pub require_rooted_bank: bool, @@ -6944,7 +6944,7 @@ impl Bank { /// return true if all is good /// Only called from startup or test code. #[must_use] - pub fn verify_accounts_hash(&self, config: VerifyBankHash) -> bool { + pub fn verify_accounts_hash(&self, config: VerifyAccountsHashConfig) -> bool { 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. @@ -6989,7 +6989,7 @@ impl Bank { let result = accounts_.verify_accounts_hash_and_lamports( slot, cap, - BankHashLamportsVerifyConfig { + VerifyAccountsHashAndLamportsConfig { ancestors: &ancestors, test_hash_calculation: config.test_hash_calculation, epoch_schedule: &epoch_schedule, @@ -7012,7 +7012,7 @@ impl Bank { let result = accounts.verify_accounts_hash_and_lamports( slot, cap, - BankHashLamportsVerifyConfig { + VerifyAccountsHashAndLamportsConfig { ancestors, test_hash_calculation: config.test_hash_calculation, epoch_schedule, @@ -7307,7 +7307,7 @@ impl Bank { let should_verify_accounts = !self.rc.accounts.accounts_db.skip_initial_hash_calc; if should_verify_accounts { info!("Verifying accounts..."); - let verified = self.verify_accounts_hash(VerifyBankHash { + let verified = self.verify_accounts_hash(VerifyAccountsHashConfig { test_hash_calculation, ignore_mismatch: false, require_rooted_bank: false, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 09ab30dea1..d2f5b2d23a 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -2661,7 +2661,7 @@ fn test_bank_update_rewards_determinism() { } } -impl VerifyBankHash { +impl VerifyAccountsHashConfig { fn default_for_test() -> Self { Self { test_hash_calculation: true, @@ -2744,7 +2744,7 @@ fn test_purge_empty_accounts() { if pass == 0 { add_root_and_flush_write_cache(&bank0); - assert!(bank0.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank0.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); continue; } @@ -2753,7 +2753,7 @@ fn test_purge_empty_accounts() { bank0.squash(); add_root_and_flush_write_cache(&bank0); if pass == 1 { - assert!(bank0.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank0.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); continue; } @@ -2761,7 +2761,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(VerifyBankHash::default_for_test())); + assert!(bank1.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); // keypair should have 0 tokens on both forks assert_eq!(bank0.get_account(&keypair.pubkey()), None); @@ -2769,7 +2769,7 @@ fn test_purge_empty_accounts() { bank1.clean_accounts_for_tests(); - assert!(bank1.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank1.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); } } @@ -3923,7 +3923,7 @@ fn test_bank_hash_internal_state() { add_root_and_flush_write_cache(&bank1); add_root_and_flush_write_cache(&bank2); bank2.update_accounts_hash_for_tests(); - assert!(bank2.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank2.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); } #[test] @@ -3953,13 +3953,13 @@ fn test_bank_hash_internal_state_verify() { // we later modify bank 2, so this flush is destructive to the test add_root_and_flush_write_cache(&bank2); bank2.update_accounts_hash_for_tests(); - assert!(bank2.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank2.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); } let bank3 = Bank::new_from_parent(&bank0, &solana_sdk::pubkey::new_rand(), 2); assert_eq!(bank0_state, bank0.hash_internal_state()); if pass == 0 { // this relies on us having set the bank hash in the pass==0 if above - assert!(bank2.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank2.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); continue; } if pass == 1 { @@ -3968,7 +3968,7 @@ fn test_bank_hash_internal_state_verify() { // Doing so throws an assert. So, we can't flush 3 until 2 is flushed. add_root_and_flush_write_cache(&bank3); bank3.update_accounts_hash_for_tests(); - assert!(bank3.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank3.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); continue; } @@ -3977,10 +3977,10 @@ fn test_bank_hash_internal_state_verify() { bank2.transfer(amount, &mint_keypair, &pubkey2).unwrap(); add_root_and_flush_write_cache(&bank2); bank2.update_accounts_hash_for_tests(); - assert!(bank2.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank2.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); add_root_and_flush_write_cache(&bank3); bank3.update_accounts_hash_for_tests(); - assert!(bank3.verify_accounts_hash(VerifyBankHash::default_for_test())); + assert!(bank3.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); } }