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
8 changes: 5 additions & 3 deletions accounts-db/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10215,14 +10215,16 @@ pub mod tests {
);

#[test]
#[should_panic(expected = "MismatchedAccountsHash")]
fn test_accountsdb_scan_snapshot_stores_check_hash() {
solana_logger::setup();
let accounts_db = AccountsDb::new_single_for_tests();
let (storages, _raw_expected) = sample_storages_and_accounts(&accounts_db);
let max_slot = storages.iter().map(|storage| storage.slot()).max().unwrap();

let hash =
// bogus_hash is ignored during appendvec store. When we verify hashes
// later during scan, the test will recompute the "correct" hash and
// should not mismatch.
let bogus_hash =
AccountHash(Hash::from_str("7JcmM6TFZMkcDkZe6RKVkGaWwN5dXciGC4fa3RxvqQc9").unwrap());

// replace the sample storages, storing bogus hash values so that we trigger the hash mismatch
Expand All @@ -10242,7 +10244,7 @@ pub mod tests {
.collect::<Vec<_>>();
let slice = &accounts[..];
let account_data = (slot, slice);
let hashes = (0..account_data.len()).map(|_| &hash).collect();
let hashes = (0..account_data.len()).map(|_| &bogus_hash).collect();
let storable_accounts =
StorableAccountsWithHashes::new_with_hashes(&account_data, hashes);
copied_storage
Expand Down
7 changes: 4 additions & 3 deletions accounts-db/src/append_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use {
solana_sdk::{
account::{AccountSharedData, ReadableAccount, WritableAccount},
clock::Slot,
hash::Hash,
pubkey::Pubkey,
stake_history::Epoch,
},
Expand Down Expand Up @@ -741,8 +742,8 @@ impl AppendVec {
skip: usize,
) -> Option<Vec<StoredAccountInfo>> {
let _lock = self.append_lock.lock().unwrap();
let default_hash: Hash = Hash::default(); // [0_u8; 32];
let mut offset = self.len();

let len = accounts.accounts.len();
// Here we have `len - skip` number of accounts. The +1 extra capacity
// is for storing the aligned offset of the last entry to that is used
Expand All @@ -754,7 +755,7 @@ impl AppendVec {
if stop {
break;
}
accounts.get(i, |account, hash| {
accounts.get(i, |account, _hash| {
let account_meta = AccountMeta {
lamports: account.lamports(),
owner: *account.owner(),
Expand All @@ -771,7 +772,7 @@ impl AppendVec {
let account_meta_ptr = &account_meta as *const AccountMeta;
let data_len = stored_meta.data_len as usize;
let data_ptr = account.data().as_ptr();
let hash_ptr = bytemuck::bytes_of(hash).as_ptr();
let hash_ptr = bytemuck::bytes_of(&default_hash).as_ptr();
Comment thread
brooksprumo marked this conversation as resolved.
let ptrs = [
(meta_ptr as *const u8, mem::size_of::<StoredMeta>()),
(account_meta_ptr as *const u8, mem::size_of::<AccountMeta>()),
Expand Down
28 changes: 24 additions & 4 deletions runtime/tests/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ use {
rayon::prelude::*,
solana_accounts_db::{
accounts_db::{AccountsDb, LoadHint},
accounts_hash::AccountHash,
ancestors::Ancestors,
},
solana_sdk::{
account::{AccountSharedData, ReadableAccount, WritableAccount},
clock::Slot,
hash::Hash,
pubkey::Pubkey,
sysvar::epoch_schedule::EpochSchedule,
},
Expand Down Expand Up @@ -128,11 +130,29 @@ fn test_bad_bank_hash() {
db.store_cached((some_slot, &account_refs[..]), None);
for pass in 0..2 {
for (key, account) in &account_refs {
assert_eq!(
db.load_account_hash(&ancestors, key, Some(some_slot), LoadHint::Unspecified)
if pass == 1 {
assert_eq!(
db.load_account_hash(
&ancestors,
key,
Some(some_slot),
LoadHint::Unspecified
)
.unwrap(),
AccountsDb::hash_account(*account, key)
);
AccountHash(Hash::default())
);
} else {
assert_eq!(
db.load_account_hash(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should we ever be saving/loading a hash now? not sure about this test.

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.

I think the intention of this test is to test load hash from cache vs storage.
What this test does is to make sure that when we load from the write cache, we should have the correct hash (pass=0), but when we load from the storage, we should have default hash (pass=1).

&ancestors,
key,
Some(some_slot),
LoadHint::Unspecified
)
.unwrap(),
AccountsDb::hash_account(*account, key)
);
}
}
if pass == 0 {
// flush the write cache so we're reading from append vecs on the next iteration
Expand Down