From 989c80342bdf2eca437dc3cb37f3083062bc21c8 Mon Sep 17 00:00:00 2001 From: Brooks Date: Sun, 19 Mar 2023 21:44:38 -0400 Subject: [PATCH] Adds plumbing and tests for verify_snapshot_bank() with Incremental Accounts Hash (#30789) --- ledger/src/blockstore_processor.rs | 17 +++-- runtime/src/accounts_db.rs | 19 ++++++ runtime/src/bank.rs | 58 ++++++++++++---- runtime/src/bank/tests.rs | 103 +++++++++++++++++++++++++---- runtime/src/snapshot_utils.rs | 1 + 5 files changed, 167 insertions(+), 31 deletions(-) diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 9959f06b5..fb8df9cf7 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -1515,13 +1515,16 @@ 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(VerifyAccountsHashConfig { - test_hash_calculation: false, - 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, - }); + let _ = bank.verify_accounts_hash( + None, + VerifyAccountsHashConfig { + test_hash_calculation: false, + 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, + }, + ); } // `roots` is sorted largest to smallest by root slot diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 3d24327e4..8453d9acb 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -7353,6 +7353,25 @@ impl AccountsDb { (accounts_hash, total_lamports) } + /// Calculate the incremental accounts hash for `storages` and save the results at `slot` + pub fn update_incremental_accounts_hash( + &self, + config: &CalcAccountsHashConfig<'_>, + storages: &SortedStorages<'_>, + slot: Slot, + stats: HashStats, + ) -> Result<(IncrementalAccountsHash, /*capitalization*/ u64), AccountsHashVerificationError> + { + let incremental_accounts_hash = + self.calculate_incremental_accounts_hash(config, storages, stats)?; + let old_incremental_accounts_hash = + self.set_incremental_accounts_hash(slot, incremental_accounts_hash); + if let Some(old_incremental_accounts_hash) = old_incremental_accounts_hash { + warn!("Incremental accounts hash was already set for slot {slot}! old: {old_incremental_accounts_hash:?}, new: {incremental_accounts_hash:?}"); + } + Ok(incremental_accounts_hash) + } + /// Set the accounts hash for `slot` /// /// returns the previous accounts hash for `slot` diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index dc1f64354..6b331fb12 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -49,7 +49,7 @@ use { CalcAccountsHashDataSource, IncludeSlotInHash, VerifyAccountsHashAndLamportsConfig, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING, }, - accounts_hash::{AccountsHash, IncrementalAccountsHash}, + accounts_hash::{AccountsHash, CalcAccountsHashConfig, HashStats, IncrementalAccountsHash}, accounts_index::{AccountSecondaryIndexes, IndexKey, ScanConfig, ScanResult, ZeroLamport}, accounts_update_notifier_interface::AccountsUpdateNotifier, ancestors::{Ancestors, AncestorsForSerialization}, @@ -65,6 +65,7 @@ use { runtime_config::RuntimeConfig, serde_snapshot::{SerdeAccountsHash, SerdeIncrementalAccountsHash}, snapshot_hash::SnapshotHash, + sorted_storages::SortedStorages, stake_account::{self, StakeAccount}, stake_weighted_timestamp::{ calculate_stake_weighted_timestamp, MaxAllowableDrift, @@ -6943,7 +6944,11 @@ impl Bank { /// return true if all is good /// Only called from startup or test code. #[must_use] - pub fn verify_accounts_hash(&self, config: VerifyAccountsHashConfig) -> bool { + pub fn verify_accounts_hash( + &self, + base: Option<(Slot, /*capitalization*/ u64)>, + 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. @@ -6960,7 +6965,7 @@ impl Bank { { if let Some(parent) = self.parent() { info!("{} is not a root, so attempting to verify bank hash on parent bank at slot: {}", self.slot(), parent.slot()); - return parent.verify_accounts_hash(config); + return parent.verify_accounts_hash(base, config); } else { // this will result in mismatch errors // accounts hash calc doesn't include unrooted slots @@ -6988,7 +6993,7 @@ impl Bank { let result = accounts_.verify_accounts_hash_and_lamports( slot, cap, - None, + base, VerifyAccountsHashAndLamportsConfig { ancestors: &ancestors, test_hash_calculation: config.test_hash_calculation, @@ -7012,7 +7017,7 @@ impl Bank { let result = accounts.verify_accounts_hash_and_lamports( slot, cap, - None, + base, VerifyAccountsHashAndLamportsConfig { ancestors, test_hash_calculation: config.test_hash_calculation, @@ -7285,6 +7290,31 @@ impl Bank { self.update_accounts_hash(CalcAccountsHashDataSource::IndexForTests, false, false) } + /// Calculate the incremental accounts hash from `base_slot` to `self` + pub fn update_incremental_accounts_hash(&self, base_slot: Slot) -> IncrementalAccountsHash { + let config = CalcAccountsHashConfig { + use_bg_thread_pool: true, + check_hash: false, + ancestors: None, // does not matter, will not be used + epoch_schedule: &self.epoch_schedule, + rent_collector: &self.rent_collector, + store_detailed_debug_info_on_failure: false, + }; + let storages = self.get_snapshot_storages(Some(base_slot)); + let sorted_storages = SortedStorages::new(&storages); + self.rc + .accounts + .accounts_db + .update_incremental_accounts_hash( + &config, + &sorted_storages, + self.slot(), + HashStats::default(), + ) + .unwrap() // unwrap here will never fail since check_hash = false + .0 + } + /// A snapshot bank should be purged of 0 lamport accounts which are not part of the hash /// calculation and could shield other real accounts. pub fn verify_snapshot_bank( @@ -7292,6 +7322,7 @@ impl Bank { test_hash_calculation: bool, accounts_db_skip_shrink: bool, last_full_snapshot_slot: Slot, + base: Option<(Slot, /*capitalization*/ u64)>, ) -> bool { let (_, clean_time_us) = measure_us!({ let should_clean = !accounts_db_skip_shrink && self.slot() > 0; @@ -7326,13 +7357,16 @@ 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(VerifyAccountsHashConfig { - test_hash_calculation, - ignore_mismatch: false, - require_rooted_bank: false, - run_in_background: true, - store_hash_raw_data_for_debug: false, - }); + let verified = self.verify_accounts_hash( + base, + VerifyAccountsHashConfig { + test_hash_calculation, + ignore_mismatch: false, + require_rooted_bank: false, + run_in_background: true, + store_hash_raw_data_for_debug: false, + }, + ); info!("Verifying accounts... In background."); verified } else { diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 9f952b288..04d3b12c9 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -2739,7 +2739,7 @@ fn test_purge_empty_accounts() { if pass == 0 { add_root_and_flush_write_cache(&bank0); - assert!(bank0.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); + assert!(bank0.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); continue; } @@ -2748,7 +2748,7 @@ fn test_purge_empty_accounts() { bank0.squash(); add_root_and_flush_write_cache(&bank0); if pass == 1 { - assert!(bank0.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); + assert!(bank0.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); continue; } @@ -2756,7 +2756,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(VerifyAccountsHashConfig::default_for_test())); + assert!(bank1.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); // keypair should have 0 tokens on both forks assert_eq!(bank0.get_account(&keypair.pubkey()), None); @@ -2764,7 +2764,7 @@ fn test_purge_empty_accounts() { bank1.clean_accounts_for_tests(); - assert!(bank1.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); + assert!(bank1.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); } } @@ -3918,7 +3918,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(VerifyAccountsHashConfig::default_for_test())); + assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); } #[test] @@ -3948,13 +3948,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(VerifyAccountsHashConfig::default_for_test())); + assert!(bank2.verify_accounts_hash(None, 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(VerifyAccountsHashConfig::default_for_test())); + assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); continue; } if pass == 1 { @@ -3963,7 +3963,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(VerifyAccountsHashConfig::default_for_test())); + assert!(bank3.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); continue; } @@ -3972,10 +3972,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(VerifyAccountsHashConfig::default_for_test())); + assert!(bank2.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); add_root_and_flush_write_cache(&bank3); bank3.update_accounts_hash_for_tests(); - assert!(bank3.verify_accounts_hash(VerifyAccountsHashConfig::default_for_test())); + assert!(bank3.verify_accounts_hash(None, VerifyAccountsHashConfig::default_for_test())); } } @@ -4001,11 +4001,11 @@ fn test_verify_snapshot_bank() { bank.freeze(); add_root_and_flush_write_cache(&bank); bank.update_accounts_hash_for_tests(); - assert!(bank.verify_snapshot_bank(true, false, bank.slot())); + assert!(bank.verify_snapshot_bank(true, false, bank.slot(), None)); // tamper the bank after freeze! bank.increment_signature_count(1); - assert!(!bank.verify_snapshot_bank(true, false, bank.slot())); + assert!(!bank.verify_snapshot_bank(true, false, bank.slot(), None)); } // Test that two bank forks with the same accounts should not hash to the same value. @@ -12835,3 +12835,82 @@ fn test_runtime_feature_enable_with_executor_cache() { // Err(TransactionError::InstructionError(0, InstructionError::InvalidAccountData)) // ); } + +#[test] +fn test_bank_verify_accounts_hash_with_base() { + let GenesisConfigInfo { + mut genesis_config, + mint_keypair: mint, + .. + } = genesis_utils::create_genesis_config_with_leader( + 1_000_000 * LAMPORTS_PER_SOL, + &Pubkey::new_unique(), + 100 * LAMPORTS_PER_SOL, + ); + genesis_config.rent = Rent::default(); + genesis_config.ticks_per_slot = 3; + + let do_transfers = |bank: &Bank| { + let key1 = Keypair::new(); // lamports from mint + let key2 = Keypair::new(); // will end with ZERO lamports + let key3 = Keypair::new(); // lamports from key2 + + let amount = 123_456_789; + let fee = { + let blockhash = bank.last_blockhash(); + let transaction = SanitizedTransaction::from_transaction_for_tests( + system_transaction::transfer(&key2, &key3.pubkey(), amount, blockhash), + ); + bank.get_fee_for_message(transaction.message()).unwrap() + }; + bank.transfer(amount + fee, &mint, &key1.pubkey()).unwrap(); + bank.transfer(amount + fee, &mint, &key2.pubkey()).unwrap(); + bank.transfer(amount + fee, &key2, &key3.pubkey()).unwrap(); + assert_eq!(bank.get_balance(&key2.pubkey()), 0); + + bank.fill_bank_with_ticks_for_tests(); + }; + + let mut bank = Arc::new(Bank::new_for_tests(&genesis_config)); + + // make some banks, do some transactions, ensure there's some zero-lamport accounts + for _ in 0..2 { + bank = Arc::new(Bank::new_from_parent( + &bank, + &Pubkey::new_unique(), + bank.slot() + 1, + )); + do_transfers(&bank); + } + + // update the base accounts hash + bank.squash(); + bank.force_flush_accounts_cache(); + bank.update_accounts_hash(CalcAccountsHashDataSource::Storages, false, 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 { + bank = Arc::new(Bank::new_from_parent( + &bank, + &Pubkey::new_unique(), + bank.slot() + 1, + )); + do_transfers(&bank); + } + + // update the incremental accounts hash + bank.squash(); + bank.force_flush_accounts_cache(); + bank.update_incremental_accounts_hash(base_slot); + + // ensure the accounts hash verifies + assert!(bank.verify_accounts_hash( + Some((base_slot, base_capitalization)), + VerifyAccountsHashConfig { + test_hash_calculation: false, + ..VerifyAccountsHashConfig::default_for_test() + }, + )); +} diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 87825c9e0..eee380407 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -1482,6 +1482,7 @@ pub fn bank_from_snapshot_archives( test_hash_calculation, accounts_db_skip_shrink || !full_snapshot_archive_info.is_remote(), full_snapshot_archive_info.slot(), + None, ) && limit_load_slot_count_from_snapshot.is_none() { panic!("Snapshot bank for slot {} failed to verify", bank.slot());