Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions accounts-db/benches/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,20 +60,22 @@ fn bench_accounts_hash_bank_hash(bencher: &mut Bencher) {
accounts.add_root(slot);
accounts.accounts_db.flush_accounts_cache(true, Some(slot));
bencher.iter(|| {
assert!(accounts.verify_accounts_hash_and_lamports(
0,
total_lamports,
None,
VerifyAccountsHashAndLamportsConfig {
ancestors: &ancestors,
test_hash_calculation: false,
epoch_schedule: &EpochSchedule::default(),
rent_collector: &RentCollector::default(),
ignore_mismatch: false,
store_detailed_debug_info: false,
use_bg_thread_pool: false,
}
))
assert!(accounts
.accounts_db
.verify_accounts_hash_and_lamports_for_tests(
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benchmark needed to be updated too.

0,
total_lamports,
VerifyAccountsHashAndLamportsConfig {
ancestors: &ancestors,
test_hash_calculation: false,
epoch_schedule: &EpochSchedule::default(),
rent_collector: &RentCollector::default(),
ignore_mismatch: false,
store_detailed_debug_info: false,
use_bg_thread_pool: false,
}
)
.is_ok())
});
}

Expand Down
16 changes: 10 additions & 6 deletions accounts-db/src/accounts.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use {
crate::{
accounts_db::{
AccountsAddRootTiming, AccountsDb, LoadHint, LoadedAccount, ScanAccountStorageData,
ScanStorageResult, VerifyAccountsHashAndLamportsConfig,
AccountStorageEntry, AccountsAddRootTiming, AccountsDb, LoadHint, LoadedAccount,
ScanAccountStorageData, ScanStorageResult, VerifyAccountsHashAndLamportsConfig,
},
accounts_index::{IndexKey, ScanConfig, ScanError, ScanResult},
ancestors::Ancestors,
Expand Down Expand Up @@ -297,15 +297,19 @@ impl Accounts {
#[must_use]
pub fn verify_accounts_hash_and_lamports(
&self,
snapshot_storages_and_slots: (&[Arc<AccountStorageEntry>], &[Slot]),
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, base, config)
{
if let Err(err) = self.accounts_db.verify_accounts_hash_and_lamports(
snapshot_storages_and_slots,
slot,
total_lamports,
base,
config,
) {
warn!("verify_accounts_hash failed: {err:?}, slot: {slot}");
false
} else {
Expand Down
46 changes: 36 additions & 10 deletions accounts-db/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7488,6 +7488,7 @@ impl AccountsDb {
/// and then calculate the incremental accounts hash for `(base slot, slot]`.
pub fn verify_accounts_hash_and_lamports(
&self,
snapshot_storages_and_slots: (&[Arc<AccountStorageEntry>], &[Slot]),
slot: Slot,
total_lamports: u64,
base: Option<(Slot, /*capitalization*/ u64)>,
Expand All @@ -7503,11 +7504,21 @@ impl AccountsDb {
let hash_mismatch_is_error = !config.ignore_mismatch;

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);
let sorted_storages =
SortedStorages::new_with_slots(storages.iter().zip(slots), None, None);
self.verify_accounts_hash_and_lamports(
snapshot_storages_and_slots,
base_slot,
base_capitalization,
None,
config,
)?;

let storages_and_slots = snapshot_storages_and_slots
.0
.iter()
.zip(snapshot_storages_and_slots.1.iter())
.filter(|storage_and_slot| *storage_and_slot.1 > base_slot)
.map(|(storage, slot)| (storage, *slot));
let sorted_storages = SortedStorages::new_with_slots(storages_and_slots, None, None);
let calculated_incremental_accounts_hash = self.calculate_incremental_accounts_hash(
&calc_config,
&sorted_storages,
Expand All @@ -7526,9 +7537,13 @@ impl AccountsDb {
}
}
} else {
let (storages, slots) = self.get_snapshot_storages(..=slot);
let sorted_storages =
SortedStorages::new_with_slots(storages.iter().zip(slots), None, None);
let storages_and_slots = snapshot_storages_and_slots
.0
.iter()
.zip(snapshot_storages_and_slots.1.iter())
.filter(|storage_and_slot| *storage_and_slot.1 <= slot)
.map(|(storage, slot)| (storage, *slot));
let sorted_storages = SortedStorages::new_with_slots(storages_and_slots, None, None);
let (calculated_accounts_hash, calculated_lamports) =
self.calculate_accounts_hash(&calc_config, &sorted_storages, HashStats::default());
if calculated_lamports != total_lamports {
Expand Down Expand Up @@ -9448,7 +9463,18 @@ impl AccountsDb {
total_lamports: u64,
config: VerifyAccountsHashAndLamportsConfig,
) -> Result<(), AccountsHashVerificationError> {
self.verify_accounts_hash_and_lamports(slot, total_lamports, None, config)
let snapshot_storages = self.get_snapshot_storages(..);
let snapshot_storages_and_slots = (
snapshot_storages.0.as_slice(),
snapshot_storages.1.as_slice(),
);
self.verify_accounts_hash_and_lamports(
snapshot_storages_and_slots,
slot,
total_lamports,
None,
config,
)
}
}

Expand Down Expand Up @@ -12400,7 +12426,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, None, config.clone()),
db.verify_accounts_hash_and_lamports_for_tests(some_slot, 1, config.clone()),
Copy link
Copy Markdown
Author

@brooksprumo brooksprumo May 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this one in the previous PR.

Ok(_)
);
continue;
Expand Down
31 changes: 27 additions & 4 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ use {
collections::{HashMap, HashSet},
convert::TryFrom,
fmt, mem,
ops::{AddAssign, RangeInclusive},
ops::{AddAssign, RangeFull, RangeInclusive},
path::PathBuf,
slice,
sync::{
Expand Down Expand Up @@ -5620,7 +5620,15 @@ impl Bank {
panic!("cannot verify accounts hash because slot {slot} is not a root");
}
}
let cap = self.capitalization();
// The snapshot storages must be captured *before* starting the background verification.
// Otherwise, it is possible that a delayed call to `get_snapshot_storages()` will *not*
// get the correct storages required to calculate and verify the accounts hashes.
let snapshot_storages = self
.rc
.accounts
.accounts_db
.get_snapshot_storages(RangeFull);
Comment on lines +5623 to +5630
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get the snapshot storages here, before starting the background thread to verify the account hashes.

let capitalization = self.capitalization();
let verify_config = VerifyAccountsHashAndLamportsConfig {
ancestors: &self.ancestors,
epoch_schedule: self.epoch_schedule(),
Expand All @@ -5641,9 +5649,14 @@ impl Bank {
.name("solBgHashVerify".into())
.spawn(move || {
info!("Initial background accounts hash verification has started");
let snapshot_storages_and_slots = (
snapshot_storages.0.as_slice(),
snapshot_storages.1.as_slice(),
);
let result = accounts_.verify_accounts_hash_and_lamports(
snapshot_storages_and_slots,
slot,
cap,
capitalization,
base,
VerifyAccountsHashAndLamportsConfig {
ancestors: &ancestors,
Expand All @@ -5663,7 +5676,17 @@ impl Bank {
});
true // initial result is true. We haven't failed yet. If verification fails, we'll panic from bg thread.
} else {
let result = accounts.verify_accounts_hash_and_lamports(slot, cap, base, verify_config);
let snapshot_storages_and_slots = (
snapshot_storages.0.as_slice(),
snapshot_storages.1.as_slice(),
);
let result = accounts.verify_accounts_hash_and_lamports(
snapshot_storages_and_slots,
slot,
capitalization,
base,
verify_config,
);
self.set_initial_accounts_hash_verification_completed();
result
}
Expand Down