From 12e92dd59d716939340928d356a7d2caefe8a679 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 16 Jun 2021 14:46:34 +0000 Subject: [PATCH] verify bank hash on startup with ledger tool option (backport #17939) (#17996) * verify bank hash on startup with ledger tool option (#17939) (cherry picked from commit f558b9b6bf88c1fc56dcf93a6c12231cb8eef66d) # Conflicts: # core/tests/snapshots.rs # ledger/src/bank_forks_utils.rs # runtime/src/snapshot_utils.rs * fix merge errors Co-authored-by: Jeff Washington (jwash) <75863576+jeffwashington@users.noreply.github.com> Co-authored-by: Jeff Washington (jwash) --- core/tests/snapshots.rs | 2 ++ ledger/src/bank_forks_utils.rs | 1 + runtime/benches/accounts.rs | 10 ++++++++- runtime/src/accounts.rs | 11 ++++++---- runtime/src/accounts_db.rs | 40 +++++++++++++++++++--------------- runtime/src/bank.rs | 31 +++++++++++++------------- runtime/src/snapshot_utils.rs | 3 ++- 7 files changed, 59 insertions(+), 39 deletions(-) diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index ffd317392a..710e769870 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -145,6 +145,7 @@ mod tests { let old_last_bank = old_bank_forks.get(old_last_slot).unwrap(); + let check_hash_calculation = false; let (deserialized_bank, _timing) = snapshot_utils::bank_from_archive( &account_paths, &[], @@ -165,6 +166,7 @@ mod tests { AccountSecondaryIndexes::default(), false, None, + check_hash_calculation, ) .unwrap(); diff --git a/ledger/src/bank_forks_utils.rs b/ledger/src/bank_forks_utils.rs index 236b4d0d2e..b80209b519 100644 --- a/ledger/src/bank_forks_utils.rs +++ b/ledger/src/bank_forks_utils.rs @@ -142,6 +142,7 @@ fn load_from_snapshot( process_options.account_indexes.clone(), process_options.accounts_db_caching_enabled, process_options.limit_load_slot_count_from_snapshot, + process_options.accounts_db_test_hash_calculation, ) .expect("Load from snapshot failed"); if let Some(shrink_paths) = shrink_paths { diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs index 373301dd71..b83b32863f 100644 --- a/runtime/benches/accounts.rs +++ b/runtime/benches/accounts.rs @@ -110,7 +110,15 @@ fn test_accounts_hash_bank_hash(bencher: &mut Bencher) { create_test_accounts(&accounts, &mut pubkeys, num_accounts, slot); let ancestors = Ancestors::from(vec![0]); let (_, total_lamports) = accounts.accounts_db.update_accounts_hash(0, &ancestors); - bencher.iter(|| assert!(accounts.verify_bank_hash_and_lamports(0, &ancestors, total_lamports))); + let test_hash_calculation = false; + bencher.iter(|| { + assert!(accounts.verify_bank_hash_and_lamports( + 0, + &ancestors, + total_lamports, + test_hash_calculation + )) + }); } #[bench] diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index eb10bc9c72..9606d31553 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -645,11 +645,14 @@ impl Accounts { slot: Slot, ancestors: &Ancestors, total_lamports: u64, + test_hash_calculation: bool, ) -> bool { - if let Err(err) = - self.accounts_db - .verify_bank_hash_and_lamports(slot, ancestors, total_lamports) - { + if let Err(err) = self.accounts_db.verify_bank_hash_and_lamports( + slot, + ancestors, + total_lamports, + test_hash_calculation, + ) { warn!("verify_bank_hash failed: {:?}", err); false } else { diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 0afc7535da..794638b661 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -4690,19 +4690,23 @@ impl AccountsDb { slot: Slot, ancestors: &Ancestors, total_lamports: u64, + test_hash_calculation: bool, ) -> Result<(), BankHashVerificationError> { use BankHashVerificationError::*; let use_index = true; let check_hash = true; let can_cached_slot_be_unflushed = false; - let (calculated_hash, calculated_lamports) = self.calculate_accounts_hash_helper( - use_index, - slot, - ancestors, - check_hash, - can_cached_slot_be_unflushed, - )?; + let (calculated_hash, calculated_lamports) = self + .calculate_accounts_hash_helper_with_verify( + use_index, + test_hash_calculation, + slot, + ancestors, + None, + can_cached_slot_be_unflushed, + check_hash, + )?; if calculated_lamports != total_lamports { warn!( @@ -7955,7 +7959,7 @@ pub mod tests { assert_load_account(&accounts, current_slot, dummy_pubkey, dummy_lamport); accounts - .verify_bank_hash_and_lamports(4, &Ancestors::default(), 1222) + .verify_bank_hash_and_lamports(4, &Ancestors::default(), 1222, true) .unwrap(); } @@ -8430,13 +8434,13 @@ pub mod tests { db.add_root(some_slot); db.update_accounts_hash_test(some_slot, &ancestors); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1, true), Ok(_) ); db.bank_hashes.write().unwrap().remove(&some_slot).unwrap(); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1, true), Err(MissingBankHash) ); @@ -8451,7 +8455,7 @@ pub mod tests { .unwrap() .insert(some_slot, bank_hash_info); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1, true), Err(MismatchedBankHash) ); } @@ -8472,7 +8476,7 @@ pub mod tests { db.add_root(some_slot); db.update_accounts_hash_test(some_slot, &ancestors); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1, true), Ok(_) ); @@ -8486,12 +8490,12 @@ pub mod tests { ); db.update_accounts_hash_test(some_slot, &ancestors); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 2), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 2, true), Ok(_) ); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 10), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 10, true), Err(MismatchedTotalLamports(expected, actual)) if expected == 2 && actual == 10 ); } @@ -8511,7 +8515,7 @@ pub mod tests { db.add_root(some_slot); db.update_accounts_hash_test(some_slot, &ancestors); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 0), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 0, true), Ok(_) ); } @@ -8541,7 +8545,7 @@ pub mod tests { db.store_accounts_unfrozen(some_slot, accounts, Some(&[&some_hash]), false); db.add_root(some_slot); assert_matches!( - db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1), + db.verify_bank_hash_and_lamports(some_slot, &ancestors, 1, true), Err(MismatchedAccountHash) ); } @@ -9110,12 +9114,12 @@ pub mod tests { let no_ancestors = Ancestors::default(); accounts.update_accounts_hash(current_slot, &no_ancestors); accounts - .verify_bank_hash_and_lamports(current_slot, &no_ancestors, 22300) + .verify_bank_hash_and_lamports(current_slot, &no_ancestors, 22300, true) .unwrap(); let accounts = reconstruct_accounts_db_via_serialization(&accounts, current_slot); accounts - .verify_bank_hash_and_lamports(current_slot, &no_ancestors, 22300) + .verify_bank_hash_and_lamports(current_slot, &no_ancestors, 22300, true) .unwrap(); // repeating should be no-op diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ad30c25c0f..addfb0a05e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4530,11 +4530,12 @@ impl Bank { /// Recalculate the hash_internal_state from the account stores. Would be used to verify a /// snapshot. #[must_use] - fn verify_bank_hash(&self) -> bool { + fn verify_bank_hash(&self, test_hash_calculation: bool) -> bool { self.rc.accounts.verify_bank_hash_and_lamports( self.slot(), &self.ancestors, self.capitalization(), + test_hash_calculation, ) } @@ -4644,7 +4645,7 @@ impl Bank { /// 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(&self) -> bool { + pub fn verify_snapshot_bank(&self, test_hash_calculation: bool) -> bool { info!("cleaning.."); let mut clean_time = Measure::start("clean"); if self.slot() > 0 { @@ -4661,7 +4662,7 @@ impl Bank { info!("verify_bank_hash.."); let mut verify_time = Measure::start("verify_bank_hash"); - let mut verify = self.verify_bank_hash(); + let mut verify = self.verify_bank_hash(test_hash_calculation); verify_time.stop(); info!("verify_hash.."); @@ -7483,17 +7484,17 @@ pub(crate) mod tests { assert_eq!(bank0.get_account(&keypair.pubkey()).unwrap().lamports(), 10); assert_eq!(bank1.get_account(&keypair.pubkey()), None); - assert!(bank0.verify_bank_hash()); + assert!(bank0.verify_bank_hash(true)); // Squash and then verify hash_internal value bank0.freeze(); bank0.squash(); - assert!(bank0.verify_bank_hash()); + assert!(bank0.verify_bank_hash(true)); bank1.freeze(); bank1.squash(); bank1.update_accounts_hash(); - assert!(bank1.verify_bank_hash()); + assert!(bank1.verify_bank_hash(true)); // keypair should have 0 tokens on both forks assert_eq!(bank0.get_account(&keypair.pubkey()), None); @@ -7501,7 +7502,7 @@ pub(crate) mod tests { bank1.force_flush_accounts_cache(); bank1.clean_accounts(false, false); - assert!(bank1.verify_bank_hash()); + assert!(bank1.verify_bank_hash(true)); } #[test] @@ -8307,7 +8308,7 @@ pub(crate) mod tests { info!("transfer 2 {}", pubkey2); bank2.transfer(10, &mint_keypair, &pubkey2).unwrap(); bank2.update_accounts_hash(); - assert!(bank2.verify_bank_hash()); + assert!(bank2.verify_bank_hash(true)); } #[test] @@ -8331,19 +8332,19 @@ pub(crate) mod tests { // Checkpointing should never modify the checkpoint's state once frozen let bank0_state = bank0.hash_internal_state(); bank2.update_accounts_hash(); - assert!(bank2.verify_bank_hash()); + assert!(bank2.verify_bank_hash(true)); let bank3 = Bank::new_from_parent(&bank0, &solana_sdk::pubkey::new_rand(), 2); assert_eq!(bank0_state, bank0.hash_internal_state()); - assert!(bank2.verify_bank_hash()); + assert!(bank2.verify_bank_hash(true)); bank3.update_accounts_hash(); - assert!(bank3.verify_bank_hash()); + assert!(bank3.verify_bank_hash(true)); let pubkey2 = solana_sdk::pubkey::new_rand(); info!("transfer 2 {}", pubkey2); bank2.transfer(10, &mint_keypair, &pubkey2).unwrap(); bank2.update_accounts_hash(); - assert!(bank2.verify_bank_hash()); - assert!(bank3.verify_bank_hash()); + assert!(bank2.verify_bank_hash(true)); + assert!(bank3.verify_bank_hash(true)); } #[test] @@ -8363,11 +8364,11 @@ pub(crate) mod tests { bank.transfer(1_000, &mint_keypair, &pubkey).unwrap(); bank.freeze(); bank.update_accounts_hash(); - assert!(bank.verify_snapshot_bank()); + assert!(bank.verify_snapshot_bank(true)); // tamper the bank after freeze! bank.increment_signature_count(1); - assert!(!bank.verify_snapshot_bank()); + assert!(!bank.verify_snapshot_bank(true)); } // Test that two bank forks with the same accounts should not hash to the same value. diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 6b15700612..e4bd019e28 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -612,6 +612,7 @@ pub fn bank_from_archive>( account_indexes: AccountSecondaryIndexes, accounts_db_caching_enabled: bool, limit_load_slot_count_from_snapshot: Option, + test_hash_calculation: bool, ) -> Result<(Bank, BankFromArchiveTimings)> { let unpack_dir = tempfile::Builder::new() .prefix(TMP_SNAPSHOT_PREFIX) @@ -649,7 +650,7 @@ pub fn bank_from_archive>( measure.stop(); let mut verify = Measure::start("verify"); - if !bank.verify_snapshot_bank() { + if !bank.verify_snapshot_bank(test_hash_calculation) { panic!("Snapshot bank for slot {} failed to verify", bank.slot()); } verify.stop();