From cb61ce435ee3accee75252462d48ffb28b31f0e7 Mon Sep 17 00:00:00 2001 From: Yueh-Hsuan Chiang <93241502+yhchiang-sol@users.noreply.github.com> Date: Fri, 9 Feb 2024 16:35:40 -0800 Subject: [PATCH] [TieredStorage] Put commonly used test functions into test_utils.rs (#35065) #### Problem There're some test functions that have been used in different mod in TieredStorage. It's better to have one same place for all tiere-storage related test functions. #### Summary of Changes Created test_utils.rs under /tiered_storage and move test-related functions into it. #### Test Plan Existing tests. --- accounts-db/src/tiered_storage.rs | 67 ++------------ accounts-db/src/tiered_storage/hot.rs | 92 +++----------------- accounts-db/src/tiered_storage/test_utils.rs | 76 ++++++++++++++++ 3 files changed, 95 insertions(+), 140 deletions(-) create mode 100644 accounts-db/src/tiered_storage/test_utils.rs diff --git a/accounts-db/src/tiered_storage.rs b/accounts-db/src/tiered_storage.rs index f0a23150e..335e93c72 100644 --- a/accounts-db/src/tiered_storage.rs +++ b/accounts-db/src/tiered_storage.rs @@ -10,6 +10,7 @@ pub mod meta; pub mod mmap_utils; pub mod owners; pub mod readable; +mod test_utils; pub mod writer; use { @@ -160,17 +161,12 @@ impl TieredStorage { mod tests { use { super::*, - crate::account_storage::meta::{StoredAccountMeta, StoredMeta, StoredMetaWriteVersion}, + crate::account_storage::meta::StoredMetaWriteVersion, footer::{TieredStorageFooter, TieredStorageMagicNumber}, hot::HOT_FORMAT, index::IndexOffset, - owners::OWNER_NO_OWNER, solana_sdk::{ - account::{Account, AccountSharedData}, - clock::Slot, - hash::Hash, - pubkey::Pubkey, - rent_collector::RENT_EXEMPT_RENT_EPOCH, + account::AccountSharedData, clock::Slot, hash::Hash, pubkey::Pubkey, system_instruction::MAX_PERMITTED_DATA_LENGTH, }, std::{ @@ -178,6 +174,7 @@ mod tests { mem::ManuallyDrop, }, tempfile::tempdir, + test_utils::{create_test_account, verify_test_account}, }; impl TieredStorage { @@ -310,58 +307,6 @@ mod tests { assert!(!tiered_storage_path.try_exists().unwrap()); } - /// Create a test account based on the specified seed. - fn create_account(seed: u64) -> (StoredMeta, AccountSharedData) { - let data_byte = seed as u8; - let account = Account { - lamports: seed, - data: std::iter::repeat(data_byte).take(seed as usize).collect(), - owner: Pubkey::new_unique(), - executable: seed % 2 > 0, - rent_epoch: if seed % 3 > 0 { - seed - } else { - RENT_EXEMPT_RENT_EPOCH - }, - }; - - let stored_meta = StoredMeta { - write_version_obsolete: StoredMetaWriteVersion::default(), - pubkey: Pubkey::new_unique(), - data_len: seed, - }; - (stored_meta, AccountSharedData::from(account)) - } - - fn verify_account( - stored_meta: &StoredAccountMeta<'_>, - account: Option<&impl ReadableAccount>, - account_hash: &AccountHash, - ) { - let (lamports, owner, data, executable, account_hash) = account - .map(|acc| { - ( - acc.lamports(), - acc.owner(), - acc.data(), - acc.executable(), - // only persist rent_epoch for those rent-paying accounts - Some(*account_hash), - ) - }) - .unwrap_or((0, &OWNER_NO_OWNER, &[], false, None)); - - assert_eq!(stored_meta.lamports(), lamports); - assert_eq!(stored_meta.data().len(), data.len()); - assert_eq!(stored_meta.data(), data); - assert_eq!(stored_meta.executable(), executable); - assert_eq!(stored_meta.owner(), owner); - assert_eq!( - *stored_meta.hash(), - account_hash.unwrap_or(AccountHash(Hash::default())) - ); - } - /// The helper function for all write_accounts tests. /// Currently only supports hot accounts. fn do_test_write_accounts( @@ -371,7 +316,7 @@ mod tests { ) { let accounts: Vec<_> = account_data_sizes .iter() - .map(|size| create_account(*size)) + .map(|size| create_test_account(*size)) .collect(); let account_refs: Vec<_> = accounts @@ -415,7 +360,7 @@ mod tests { let mut verified_accounts = HashSet::new(); while let Some((stored_meta, next)) = reader.get_account(index_offset).unwrap() { if let Some((account, account_hash)) = expected_accounts_map.get(stored_meta.pubkey()) { - verify_account(&stored_meta, *account, account_hash); + verify_test_account(&stored_meta, *account, stored_meta.pubkey(), account_hash); verified_accounts.insert(stored_meta.pubkey()); } index_offset = next; diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 7db9e90d6..f662c2e06 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -657,26 +657,21 @@ impl HotStorageWriter { pub mod tests { use { super::*, - crate::{ - account_storage::meta::StoredMeta, - tiered_storage::{ - byte_block::ByteBlockWriter, - file::TieredStorageFile, - footer::{AccountBlockFormat, AccountMetaFormat, TieredStorageFooter, FOOTER_SIZE}, - hot::{HotAccountMeta, HotStorageReader}, - index::{AccountIndexWriterEntry, IndexBlockFormat, IndexOffset}, - meta::{AccountMetaFlags, AccountMetaOptionalFields, TieredAccountMeta}, - owners::{OwnersBlockFormat, OwnersTable}, - }, + crate::tiered_storage::{ + byte_block::ByteBlockWriter, + file::TieredStorageFile, + footer::{AccountBlockFormat, AccountMetaFormat, TieredStorageFooter, FOOTER_SIZE}, + hot::{HotAccountMeta, HotStorageReader}, + index::{AccountIndexWriterEntry, IndexBlockFormat, IndexOffset}, + meta::{AccountMetaFlags, AccountMetaOptionalFields, TieredAccountMeta}, + owners::{OwnersBlockFormat, OwnersTable}, + test_utils::{create_test_account, verify_test_account}, }, assert_matches::assert_matches, memoffset::offset_of, rand::{seq::SliceRandom, Rng}, solana_sdk::{ - account::{Account, AccountSharedData, ReadableAccount}, - hash::Hash, - pubkey::Pubkey, - slot_history::Slot, + account::ReadableAccount, hash::Hash, pubkey::Pubkey, slot_history::Slot, stake_history::Epoch, }, tempfile::TempDir, @@ -1288,67 +1283,6 @@ pub mod tests { assert_matches!(HotStorageWriter::new(&path), Err(_)); } - /// Create a test account based on the specified seed. - /// The created test account might have default rent_epoch - /// and write_version. - /// - /// When the seed is zero, then a zero-lamport test account will be - /// created. - fn create_test_account(seed: u64) -> (StoredMeta, AccountSharedData) { - let data_byte = seed as u8; - let owner_byte = u8::MAX - data_byte; - let account = Account { - lamports: seed, - data: std::iter::repeat(data_byte).take(seed as usize).collect(), - // this will allow some test account sharing the same owner. - owner: [owner_byte; 32].into(), - executable: seed % 2 > 0, - rent_epoch: if seed % 3 > 0 { - seed - } else { - RENT_EXEMPT_RENT_EPOCH - }, - }; - - let stored_meta = StoredMeta { - write_version_obsolete: u64::MAX, - pubkey: Pubkey::new_unique(), - data_len: seed, - }; - (stored_meta, AccountSharedData::from(account)) - } - - fn verify_account( - stored_meta: &StoredAccountMeta<'_>, - account: Option<&impl ReadableAccount>, - address: &Pubkey, - account_hash: &AccountHash, - ) { - let (lamports, owner, data, executable, account_hash) = account - .map(|acc| { - ( - acc.lamports(), - acc.owner(), - acc.data(), - acc.executable(), - // only persist rent_epoch for those rent-paying accounts - Some(*account_hash), - ) - }) - .unwrap_or((0, &OWNER_NO_OWNER, &[], false, None)); - - assert_eq!(stored_meta.lamports(), lamports); - assert_eq!(stored_meta.data().len(), data.len()); - assert_eq!(stored_meta.data(), data); - assert_eq!(stored_meta.executable(), executable); - assert_eq!(stored_meta.owner(), owner); - assert_eq!(stored_meta.pubkey(), address); - assert_eq!( - *stored_meta.hash(), - account_hash.unwrap_or(AccountHash(Hash::default())) - ); - } - #[test] fn test_write_account_and_index_blocks() { let account_data_sizes = &[ @@ -1401,7 +1335,7 @@ pub mod tests { .unwrap(); let (account, address, account_hash, _write_version) = storable_accounts.get(i); - verify_account(&stored_meta, account, address, account_hash); + verify_test_account(&stored_meta, account, address, account_hash); assert_eq!(i + 1, next.0 as usize); } @@ -1420,7 +1354,7 @@ pub mod tests { let (account, address, account_hash, _write_version) = storable_accounts.get(stored_info.offset); - verify_account(&stored_meta, account, address, account_hash); + verify_test_account(&stored_meta, account, address, account_hash); } // verify get_accounts @@ -1429,7 +1363,7 @@ pub mod tests { // first, we verify everything for (i, stored_meta) in accounts.iter().enumerate() { let (account, address, account_hash, _write_version) = storable_accounts.get(i); - verify_account(stored_meta, account, address, account_hash); + verify_test_account(stored_meta, account, address, account_hash); } // second, we verify various initial position diff --git a/accounts-db/src/tiered_storage/test_utils.rs b/accounts-db/src/tiered_storage/test_utils.rs new file mode 100644 index 000000000..2ed2399f3 --- /dev/null +++ b/accounts-db/src/tiered_storage/test_utils.rs @@ -0,0 +1,76 @@ +#![cfg(test)] +//! Helper functions for TieredStorage tests +use { + crate::{ + account_storage::meta::{StoredAccountMeta, StoredMeta}, + accounts_hash::AccountHash, + tiered_storage::owners::OWNER_NO_OWNER, + }, + solana_sdk::{ + account::{Account, AccountSharedData, ReadableAccount}, + hash::Hash, + pubkey::Pubkey, + rent_collector::RENT_EXEMPT_RENT_EPOCH, + }, +}; + +/// Create a test account based on the specified seed. +/// The created test account might have default rent_epoch +/// and write_version. +/// +/// When the seed is zero, then a zero-lamport test account will be +/// created. +pub(super) fn create_test_account(seed: u64) -> (StoredMeta, AccountSharedData) { + let data_byte = seed as u8; + let owner_byte = u8::MAX - data_byte; + let account = Account { + lamports: seed, + data: std::iter::repeat(data_byte).take(seed as usize).collect(), + // this will allow some test account sharing the same owner. + owner: [owner_byte; 32].into(), + executable: seed % 2 > 0, + rent_epoch: if seed % 3 > 0 { + seed + } else { + RENT_EXEMPT_RENT_EPOCH + }, + }; + + let stored_meta = StoredMeta { + write_version_obsolete: u64::MAX, + pubkey: Pubkey::new_unique(), + data_len: seed, + }; + (stored_meta, AccountSharedData::from(account)) +} + +pub(super) fn verify_test_account( + stored_meta: &StoredAccountMeta<'_>, + account: Option<&impl ReadableAccount>, + address: &Pubkey, + account_hash: &AccountHash, +) { + let (lamports, owner, data, executable, account_hash) = account + .map(|acc| { + ( + acc.lamports(), + acc.owner(), + acc.data(), + acc.executable(), + // only persist rent_epoch for those rent-paying accounts + Some(*account_hash), + ) + }) + .unwrap_or((0, &OWNER_NO_OWNER, &[], false, None)); + + assert_eq!(stored_meta.lamports(), lamports); + assert_eq!(stored_meta.data().len(), data.len()); + assert_eq!(stored_meta.data(), data); + assert_eq!(stored_meta.executable(), executable); + assert_eq!(stored_meta.owner(), owner); + assert_eq!(stored_meta.pubkey(), address); + assert_eq!( + *stored_meta.hash(), + account_hash.unwrap_or(AccountHash(Hash::default())) + ); +}