From 70efe819bcf70476926f06c473f29015b3269536 Mon Sep 17 00:00:00 2001 From: HaoranYi <219428+HaoranYi@users.noreply.github.com> Date: Sun, 21 Jul 2024 01:08:23 -0500 Subject: [PATCH] Accounts-db: Add scan tests to cover bad accounts storage files (#2123) * Add scan tests * pr: shared test_scan_index code * pr comments * Update accounts-db/src/append_vec.rs Co-authored-by: Brooks * Update accounts-db/src/append_vec.rs Co-authored-by: Brooks * pr comments --------- Co-authored-by: HaoranYi Co-authored-by: Brooks --- accounts-db/src/append_vec.rs | 282 ++++++++++++++++++++++++---------- 1 file changed, 205 insertions(+), 77 deletions(-) diff --git a/accounts-db/src/append_vec.rs b/accounts-db/src/append_vec.rs index fd9f3409cb4b7e..d0dcee18cb3ac5 100644 --- a/accounts-db/src/append_vec.rs +++ b/accounts-db/src/append_vec.rs @@ -1860,68 +1860,6 @@ pub mod tests { assert!(result.is_none()); // Expect None to be returned. } - #[test_case(StorageAccess::Mmap)] - #[test_case(StorageAccess::File)] - fn test_scan_index(storage_access: StorageAccess) { - const NUM_ACCOUNTS: usize = 37; - let pubkeys: Vec<_> = std::iter::repeat_with(solana_sdk::pubkey::new_rand) - .take(NUM_ACCOUNTS) - .collect(); - - let mut rng = thread_rng(); - let mut accounts = Vec::with_capacity(pubkeys.len()); - let mut total_stored_size = 0; - for _ in &pubkeys { - let lamports = rng.gen(); - let data_len = rng.gen_range(0..MAX_PERMITTED_DATA_LENGTH) as usize; - let account = AccountSharedData::new(lamports, data_len, &Pubkey::default()); - accounts.push(account); - total_stored_size += aligned_stored_size(data_len); - } - let accounts = accounts; - let total_stored_size = total_stored_size; - - let temp_file = get_append_vec_path("test_scan_index"); - let account_offsets = { - let append_vec = AppendVec::new(&temp_file.path, true, total_stored_size); - // wrap AppendVec in ManuallyDrop to ensure we do not remove the backing file when dropped - let append_vec = ManuallyDrop::new(append_vec); - let slot = 55; // the specific slot does not matter - let storable_accounts: Vec<_> = std::iter::zip(&pubkeys, &accounts).collect(); - let stored_accounts_info = append_vec - .append_accounts(&(slot, storable_accounts.as_slice()), 0) - .unwrap(); - append_vec.flush().unwrap(); - stored_accounts_info.offsets - }; - - // now open the append vec with the given storage access method - // then scan_index and ensure it is correct - let (append_vec, _) = - AppendVec::new_from_file(&temp_file.path, total_stored_size, storage_access).unwrap(); - - let mut i = 0; - append_vec.scan_index(|index_info| { - let pubkey = pubkeys.get(i).unwrap(); - let account = accounts.get(i).unwrap(); - let offset = account_offsets.get(i).unwrap(); - - assert_eq!( - index_info.stored_size_aligned, - aligned_stored_size(account.data().len()), - ); - assert_eq!(index_info.index_info.offset, *offset); - assert_eq!(index_info.index_info.pubkey, *pubkey); - assert_eq!(index_info.index_info.lamports, account.lamports()); - assert_eq!(index_info.index_info.rent_epoch, account.rent_epoch()); - assert_eq!(index_info.index_info.executable, account.executable()); - assert_eq!(index_info.index_info.data_len, account.data().len() as u64); - - i += 1; - }); - assert_eq!(i, accounts.len()); - } - #[test_case(StorageAccess::Mmap)] #[test_case(StorageAccess::File)] fn test_get_account_sizes(storage_access: StorageAccess) { @@ -1967,9 +1905,15 @@ pub mod tests { assert_eq!(account_sizes, stored_sizes); } - #[test_case(StorageAccess::Mmap)] - #[test_case(StorageAccess::File)] - fn test_scan_pubkeys(storage_access: StorageAccess) { + /// A helper function for testing different scenario for scan_*. + /// + /// `modify_fn` is used to (optionally) modify the append vec before checks are performed. + /// `check_fn` performs the check for the scan. + fn test_scan_helper( + storage_access: StorageAccess, + modify_fn: impl Fn(&PathBuf, usize) -> usize, + check_fn: impl Fn(&AppendVec, &[Pubkey], &[usize], &[AccountSharedData]), + ) { const NUM_ACCOUNTS: usize = 37; let pubkeys: Vec<_> = std::iter::repeat_with(solana_sdk::pubkey::new_rand) .take(NUM_ACCOUNTS) @@ -1988,29 +1932,213 @@ pub mod tests { let accounts = accounts; let total_stored_size = total_stored_size; - let temp_file = get_append_vec_path("test_scan_pubkeys"); - { + let temp_file = get_append_vec_path("test_scan"); + let account_offsets = { // wrap AppendVec in ManuallyDrop to ensure we do not remove the backing file when dropped let append_vec = ManuallyDrop::new(AppendVec::new(&temp_file.path, true, total_stored_size)); let slot = 42; // the specific slot does not matter let storable_accounts: Vec<_> = std::iter::zip(&pubkeys, &accounts).collect(); - append_vec + let stored_accounts_info = append_vec .append_accounts(&(slot, storable_accounts.as_slice()), 0) .unwrap(); append_vec.flush().unwrap(); - } + stored_accounts_info.offsets + }; + let total_stored_size = modify_fn(&temp_file.path, total_stored_size); // now open the append vec with the given storage access method - // then scan the pubkeys to ensure they are correct - let (append_vec, _) = - AppendVec::new_from_file(&temp_file.path, total_stored_size, storage_access).unwrap(); + // then perform the scan and check it is correct + let append_vec = ManuallyDrop::new( + AppendVec::new_from_file_unchecked(&temp_file.path, total_stored_size, storage_access) + .unwrap(), + ); + + check_fn(&append_vec, &pubkeys, &account_offsets, &accounts); + } + + /// A helper fn to test `scan_pubkeys`. + fn test_scan_pubkeys_helper( + storage_access: StorageAccess, + modify_fn: impl Fn(&PathBuf, usize) -> usize, + ) { + test_scan_helper( + storage_access, + modify_fn, + |append_vec, pubkeys, _account_offsets, _accounts| { + let mut i = 0; + append_vec.scan_pubkeys(|pubkey| { + assert_eq!(pubkey, pubkeys.get(i).unwrap()); + i += 1; + }); + assert_eq!(i, pubkeys.len()); + }, + ) + } + + /// Test `scan_pubkey` for a valid account storage. + #[test_case(StorageAccess::Mmap)] + #[test_case(StorageAccess::File)] + fn test_scan_pubkeys(storage_access: StorageAccess) { + test_scan_pubkeys_helper(storage_access, |_, size| size); + } + + /// Test `scan_pubkey` for storage with incomplete account meta data. + #[test_case(StorageAccess::Mmap)] + #[test_case(StorageAccess::File)] + fn test_scan_pubkeys_incomplete_data(storage_access: StorageAccess) { + test_scan_pubkeys_helper(storage_access, |path, size| { + // Append 1 byte of data at the end of the storage file to simulate + // incomplete account's meta data. + let mut f = OpenOptions::new() + .read(true) + .append(true) + .open(path) + .unwrap(); + f.write_all(&[0xFF]).unwrap(); + size + 1 + }); + } + + /// Test `scan_pubkey` for storage which is missing the last account data + #[test_case(StorageAccess::Mmap)] + #[test_case(StorageAccess::File)] + fn test_scan_pubkeys_missing_account_data(storage_access: StorageAccess) { + test_scan_pubkeys_helper(storage_access, |path, size| { + let fake_stored_meta = StoredMeta { + write_version_obsolete: 0, + data_len: 100, + pubkey: solana_sdk::pubkey::new_rand(), + }; + let fake_account_meta = AccountMeta { + lamports: 100, + rent_epoch: 10, + owner: solana_sdk::pubkey::new_rand(), + executable: false, + }; + + let stored_meta_slice: &[u8] = unsafe { + std::slice::from_raw_parts( + (&fake_stored_meta as *const StoredMeta) as *const u8, + mem::size_of::(), + ) + }; + let account_meta_slice: &[u8] = unsafe { + std::slice::from_raw_parts( + (&fake_account_meta as *const AccountMeta) as *const u8, + mem::size_of::(), + ) + }; + + let mut f = OpenOptions::new() + .read(true) + .append(true) + .open(path) + .unwrap(); + + f.write_all(stored_meta_slice).unwrap(); + f.write_all(account_meta_slice).unwrap(); + + size + mem::size_of::() + mem::size_of::() + }); + } + + /// A helper fn to test scan_index + fn test_scan_index_helper( + storage_access: StorageAccess, + modify_fn: impl Fn(&PathBuf, usize) -> usize, + ) { + test_scan_helper( + storage_access, + modify_fn, + |append_vec, pubkeys, account_offsets, accounts| { + let mut i = 0; + append_vec.scan_index(|index_info| { + let pubkey = pubkeys.get(i).unwrap(); + let account = accounts.get(i).unwrap(); + let offset = account_offsets.get(i).unwrap(); + + assert_eq!( + index_info.stored_size_aligned, + aligned_stored_size(account.data().len()), + ); + assert_eq!(index_info.index_info.offset, *offset); + assert_eq!(index_info.index_info.pubkey, *pubkey); + assert_eq!(index_info.index_info.lamports, account.lamports()); + assert_eq!(index_info.index_info.rent_epoch, account.rent_epoch()); + assert_eq!(index_info.index_info.executable, account.executable()); + assert_eq!(index_info.index_info.data_len, account.data().len() as u64); + + i += 1; + }); + assert_eq!(i, accounts.len()); + }, + ) + } + + #[test_case(StorageAccess::Mmap)] + #[test_case(StorageAccess::File)] + fn test_scan_index(storage_access: StorageAccess) { + test_scan_index_helper(storage_access, |_, size| size); + } + + /// Test `scan_index` for storage with incomplete account meta data. + #[test_case(StorageAccess::Mmap)] + #[test_case(StorageAccess::File)] + fn test_scan_index_incomplete_data(storage_access: StorageAccess) { + test_scan_index_helper(storage_access, |path, size| { + // Append 1 byte of data at the end of the storage file to simulate + // incomplete account's meta data. + let mut f = OpenOptions::new() + .read(true) + .append(true) + .open(path) + .unwrap(); + f.write_all(&[0xFF]).unwrap(); + size + 1 + }); + } + + /// Test `scan_index` for storage which is missing the last account data + #[test_case(StorageAccess::Mmap)] + #[test_case(StorageAccess::File)] + fn test_scan_index_missing_account_data(storage_access: StorageAccess) { + test_scan_index_helper(storage_access, |path, size| { + let fake_stored_meta = StoredMeta { + write_version_obsolete: 0, + data_len: 100, + pubkey: solana_sdk::pubkey::new_rand(), + }; + let fake_account_meta = AccountMeta { + lamports: 100, + rent_epoch: 10, + owner: solana_sdk::pubkey::new_rand(), + executable: false, + }; + + let stored_meta_slice: &[u8] = unsafe { + std::slice::from_raw_parts( + (&fake_stored_meta as *const StoredMeta) as *const u8, + mem::size_of::(), + ) + }; + let account_meta_slice: &[u8] = unsafe { + std::slice::from_raw_parts( + (&fake_account_meta as *const AccountMeta) as *const u8, + mem::size_of::(), + ) + }; + + let mut f = OpenOptions::new() + .read(true) + .append(true) + .open(path) + .unwrap(); + + f.write_all(stored_meta_slice).unwrap(); + f.write_all(account_meta_slice).unwrap(); - let mut i = 0; - append_vec.scan_pubkeys(|pubkey| { - assert_eq!(pubkey, pubkeys.get(i).unwrap()); - i += 1; + size + mem::size_of::() + mem::size_of::() }); - assert_eq!(i, pubkeys.len()); } }