From 8be99c1c7d89c2feadffe8de395bdc5bd4e4e954 Mon Sep 17 00:00:00 2001 From: Brooks Date: Thu, 16 Mar 2023 09:48:06 -0400 Subject: [PATCH] Verifies incremental accounts hash in verify_accounts_hash_and_lamports (#30735) --- runtime/benches/accounts.rs | 1 + runtime/src/accounts.rs | 3 +- runtime/src/accounts_db.rs | 104 ++++++++++++++++++++++++------------ runtime/src/bank.rs | 2 + 4 files changed, 75 insertions(+), 35 deletions(-) diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs index 8ae698214..d88e0529b 100644 --- a/runtime/benches/accounts.rs +++ b/runtime/benches/accounts.rs @@ -104,6 +104,7 @@ fn test_accounts_hash_bank_hash(bencher: &mut Bencher) { assert!(accounts.verify_accounts_hash_and_lamports( 0, total_lamports, + None, VerifyAccountsHashAndLamportsConfig { ancestors: &ancestors, test_hash_calculation, diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 7c9064058..19c2f6449 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -867,11 +867,12 @@ impl Accounts { &self, slot: Slot, total_lamports: u64, + base: Option<(Slot, /*capitalization*/ u64)>, config: VerifyAccountsHashAndLamportsConfig, ) -> bool { if let Err(err) = self.accounts_db - .verify_accounts_hash_and_lamports(slot, total_lamports, config) + .verify_accounts_hash_and_lamports(slot, total_lamports, base, config) { warn!("verify_accounts_hash failed: {err:?}, slot: {slot}"); false diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 979a63022..38478fda7 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -7697,11 +7697,17 @@ impl AccountsDb { .remove_old_historical_roots(min_root, &alive_roots); } - /// Only called from startup or test code. + /// Verify accounts hash at startup (or tests) + /// + /// Calculate accounts hash(es) and compare them to the values set at startup. + /// If `base` is `None`, only calculates the full accounts hash for `[0, slot]`. + /// If `base` is `Some`, calculate the full accounts hash for `[0, base slot]` + /// and then calculate the incremental accounts hash for `(base slot, slot]`. pub fn verify_accounts_hash_and_lamports( &self, slot: Slot, total_lamports: u64, + base: Option<(Slot, /*capitalization*/ u64)>, config: VerifyAccountsHashAndLamportsConfig, ) -> Result<(), AccountsHashVerificationError> { use AccountsHashVerificationError::*; @@ -7713,32 +7719,62 @@ impl AccountsDb { rent_collector: config.rent_collector, store_detailed_debug_info_on_failure: config.store_detailed_debug_info, }; + let hash_mismatch_is_error = !config.ignore_mismatch; - let (calculated_accounts_hash, calculated_lamports) = self - .calculate_accounts_hash_with_verify( - CalcAccountsHashDataSource::Storages, - config.test_hash_calculation, - slot, - calc_config, - None, + if let Some((base_slot, base_capitalization)) = base { + self.verify_accounts_hash_and_lamports(base_slot, base_capitalization, None, config)?; + let (storages, slots) = self.get_snapshot_storages( + base_slot.checked_add(1).unwrap()..=slot, + calc_config.ancestors, + ); + let sorted_storages = + SortedStorages::new_with_slots(storages.iter().zip(slots.into_iter()), None, None); + let calculated_incremental_accounts_hash = self.calculate_incremental_accounts_hash( + &calc_config, + &sorted_storages, + base_slot, + HashStats::default(), )?; + let found_incremental_accounts_hash = self + .get_incremental_accounts_hash(slot) + .ok_or(MissingAccountsHash)?; + if calculated_incremental_accounts_hash != found_incremental_accounts_hash { + warn!( + "mismatched incremental accounts hash for slot {slot}: \ + {calculated_incremental_accounts_hash:?} (calculated) != {found_incremental_accounts_hash:?} (expected)" + ); + if hash_mismatch_is_error { + return Err(MismatchedAccountsHash); + } + } + } else { + let (calculated_accounts_hash, calculated_lamports) = self + .calculate_accounts_hash_with_verify( + CalcAccountsHashDataSource::Storages, + config.test_hash_calculation, + slot, + calc_config, + None, + )?; - if calculated_lamports != total_lamports { - warn!( - "Mismatched total lamports: {} calculated: {}", - total_lamports, calculated_lamports - ); - return Err(MismatchedTotalLamports(calculated_lamports, total_lamports)); - } + if calculated_lamports != total_lamports { + warn!( + "Mismatched total lamports: {} calculated: {}", + total_lamports, calculated_lamports + ); + return Err(MismatchedTotalLamports(calculated_lamports, total_lamports)); + } - let (found_accounts_hash, _) = self.get_accounts_hash(slot).ok_or(MissingAccountsHash)?; - if calculated_accounts_hash != found_accounts_hash { - warn!( - "Mismatched accounts hash for slot {slot}: \ - {calculated_accounts_hash:?} (calculated) != {found_accounts_hash:?} (expected)" - ); - if !config.ignore_mismatch { - return Err(MismatchedAccountsHash); + let (found_accounts_hash, _) = + self.get_accounts_hash(slot).ok_or(MissingAccountsHash)?; + if calculated_accounts_hash != found_accounts_hash { + warn!( + "Mismatched accounts hash for slot {slot}: \ + {calculated_accounts_hash:?} (calculated) != {found_accounts_hash:?} (expected)" + ); + if hash_mismatch_is_error { + return Err(MismatchedAccountsHash); + } } } @@ -12249,7 +12285,7 @@ pub mod tests { ); accounts - .verify_accounts_hash_and_lamports(4, 1222, config) + .verify_accounts_hash_and_lamports(4, 1222, None, config) .unwrap(); } @@ -12674,14 +12710,14 @@ pub mod tests { ); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 1, config.clone()), + db.verify_accounts_hash_and_lamports(some_slot, 1, None, config.clone()), Ok(_) ); db.accounts_hashes.lock().unwrap().remove(&some_slot); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 1, config.clone()), + db.verify_accounts_hash_and_lamports(some_slot, 1, None, config.clone()), Err(MissingAccountsHash) ); @@ -12691,7 +12727,7 @@ pub mod tests { ); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 1, config), + db.verify_accounts_hash_and_lamports(some_slot, 1, None, config), Err(MismatchedAccountsHash) ); } @@ -12722,7 +12758,7 @@ pub mod tests { db.update_accounts_hash_for_tests(some_slot, &ancestors, true, true); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 1, config.clone()), + db.verify_accounts_hash_and_lamports(some_slot, 1, None, config.clone()), Ok(_) ); continue; @@ -12740,12 +12776,12 @@ pub mod tests { db.update_accounts_hash_for_tests(some_slot, &ancestors, true, true); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 2, config.clone()), + db.verify_accounts_hash_and_lamports(some_slot, 2, None, config.clone()), Ok(_) ); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 10, config), + db.verify_accounts_hash_and_lamports(some_slot, 10, None, config), Err(MismatchedTotalLamports(expected, actual)) if expected == 2 && actual == 10 ); } @@ -12771,7 +12807,7 @@ pub mod tests { ); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 0, config), + db.verify_accounts_hash_and_lamports(some_slot, 0, None, config), Ok(_) ); } @@ -12811,7 +12847,7 @@ pub mod tests { ); assert_matches!( - db.verify_accounts_hash_and_lamports(some_slot, 1, config), + db.verify_accounts_hash_and_lamports(some_slot, 1, None, config), Err(MismatchedAccountsHash) ); } @@ -13351,12 +13387,12 @@ pub mod tests { accounts.update_accounts_hash_for_tests(current_slot, &no_ancestors, false, false); accounts - .verify_accounts_hash_and_lamports(current_slot, 22300, config.clone()) + .verify_accounts_hash_and_lamports(current_slot, 22300, None, config.clone()) .unwrap(); let accounts = reconstruct_accounts_db_via_serialization(&accounts, current_slot); accounts - .verify_accounts_hash_and_lamports(current_slot, 22300, config) + .verify_accounts_hash_and_lamports(current_slot, 22300, None, config) .unwrap(); // repeating should be no-op diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 56b25ca63..e95f1b3e4 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -6989,6 +6989,7 @@ impl Bank { let result = accounts_.verify_accounts_hash_and_lamports( slot, cap, + None, VerifyAccountsHashAndLamportsConfig { ancestors: &ancestors, test_hash_calculation: config.test_hash_calculation, @@ -7012,6 +7013,7 @@ impl Bank { let result = accounts.verify_accounts_hash_and_lamports( slot, cap, + None, VerifyAccountsHashAndLamportsConfig { ancestors, test_hash_calculation: config.test_hash_calculation,