Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Commit

Permalink
verify bank hash on startup with ledger tool option (#17939)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeffwashington authored Jun 15, 2021
1 parent 1b1d34d commit f558b9b
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 39 deletions.
2 changes: 2 additions & 0 deletions core/tests/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,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,
&[],
Expand All @@ -167,6 +168,7 @@ mod tests {
false,
None,
accounts_db::AccountShrinkThreshold::default(),
check_hash_calculation,
)
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions ledger/src/bank_forks_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ fn load_from_snapshot(
process_options.accounts_db_caching_enabled,
process_options.limit_load_slot_count_from_snapshot,
process_options.shrink_ratio,
process_options.accounts_db_test_hash_calculation,
)
.expect("Load from snapshot failed");
if let Some(shrink_paths) = shrink_paths {
Expand Down
10 changes: 9 additions & 1 deletion runtime/benches/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,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]
Expand Down
11 changes: 7 additions & 4 deletions runtime/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,11 +655,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 {
Expand Down
40 changes: 22 additions & 18 deletions runtime/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4866,19 +4866,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!(
Expand Down Expand Up @@ -8215,7 +8219,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();
}

Expand Down Expand Up @@ -8690,13 +8694,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)
);

Expand All @@ -8711,7 +8715,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)
);
}
Expand All @@ -8732,7 +8736,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(_)
);

Expand All @@ -8746,12 +8750,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
);
}
Expand All @@ -8771,7 +8775,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(_)
);
}
Expand Down Expand Up @@ -8801,7 +8805,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)
);
}
Expand Down Expand Up @@ -9370,12 +9374,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
Expand Down
31 changes: 16 additions & 15 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4561,11 +4561,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,
)
}

Expand Down Expand Up @@ -4675,7 +4676,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 {
Expand All @@ -4692,7 +4693,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..");
Expand Down Expand Up @@ -7514,25 +7515,25 @@ 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);
assert_eq!(bank1.get_account(&keypair.pubkey()), None);
bank1.force_flush_accounts_cache();
bank1.clean_accounts(false, false);

assert!(bank1.verify_bank_hash());
assert!(bank1.verify_bank_hash(true));
}

#[test]
Expand Down Expand Up @@ -8338,7 +8339,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]
Expand All @@ -8362,19 +8363,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]
Expand All @@ -8394,11 +8395,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.
Expand Down
3 changes: 2 additions & 1 deletion runtime/src/snapshot_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,7 @@ pub fn bank_from_archive<P: AsRef<Path>>(
accounts_db_caching_enabled: bool,
limit_load_slot_count_from_snapshot: Option<usize>,
shrink_ratio: AccountShrinkThreshold,
test_hash_calculation: bool,
) -> Result<(Bank, BankFromArchiveTimings)> {
let unpack_dir = tempfile::Builder::new()
.prefix(TMP_SNAPSHOT_PREFIX)
Expand Down Expand Up @@ -651,7 +652,7 @@ pub fn bank_from_archive<P: AsRef<Path>>(
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();
Expand Down

0 comments on commit f558b9b

Please sign in to comment.