pr comments

anza-xyz · Jul 18, 2024 · 98f1f45 · 98f1f45
1 parent 2b9b717
commit 98f1f45
Showing 1 changed file with 56 additions and 55 deletions.
diff --git a/accounts-db/src/append_vec.rs b/accounts-db/src/append_vec.rs
@@ -1867,12 +1867,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) {
-        test_scan_index_helper(storage_access, |_, size| size);
-    }
-
     #[test_case(StorageAccess::Mmap)]
     #[test_case(StorageAccess::File)]
     fn test_get_account_sizes(storage_access: StorageAccess) {
@@ -1924,7 +1918,7 @@ pub mod tests {
     /// `check_fn` performs the check for the scan.
     fn test_scan_helper(
         storage_access: StorageAccess,
-        callback: impl Fn(&PathBuf, usize) -> usize,
+        modify_fn: impl Fn(&PathBuf, usize) -> usize,
         check_fn: impl Fn(&AppendVec, &[Pubkey], &[usize], &[AccountSharedData]),
     ) {
         const NUM_ACCOUNTS: usize = 37;
@@ -1959,30 +1953,26 @@ pub mod tests {
             stored_accounts_info.offsets
         };
 
-        let assert_scan = |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 = ManuallyDrop::new(
-                AppendVec::new_from_file_unchecked(&temp_file.path, stored_size, storage_access)
-                    .unwrap(),
-            );
-
-            check_fn(&append_vec, &pubkeys, &account_offsets, &accounts);
-        };
+        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 = ManuallyDrop::new(
+            AppendVec::new_from_file_unchecked(&temp_file.path, total_stored_size, storage_access)
+                .unwrap(),
+        );
 
-        let total_stored_size = callback(&temp_file.path, total_stored_size);
-        assert_scan(total_stored_size);
+        check_fn(&append_vec, &pubkeys, &account_offsets, &accounts);
     }
 
     /// A helper fn to test `scan_pubkeys`.
     fn test_scan_pubkeys_helper(
         storage_access: StorageAccess,
-        callback: impl Fn(&PathBuf, usize) -> usize,
+        modify_fn: impl Fn(&PathBuf, usize) -> usize,
     ) {
         test_scan_helper(
             storage_access,
-            callback,
-            |append_vec, pubkeys, _account_offset, _accounts| {
+            modify_fn,
+            |append_vec, pubkeys, _account_offsets, _accounts| {
                 let mut i = 0;
                 append_vec.scan_pubkeys(|pubkey| {
                     assert_eq!(pubkey, pubkeys.get(i).unwrap());
@@ -2020,30 +2010,54 @@ pub mod tests {
     /// Test `scan_pubkey` for storage with garbage data that cause overflow in next element offset calculation.
     #[test_case(StorageAccess::Mmap)]
     #[test_case(StorageAccess::File)]
-    #[should_panic(expected = "stored size cannot overflow")]
-    fn test_scan_pubkeys_overflow(storage_access: StorageAccess) {
+    fn test_scan_pubkeys_missing_account_data(storage_access: StorageAccess) {
         test_scan_pubkeys_helper(storage_access, |path, size| {
-            // Append a garbage AccountMeta to simulate next_offset overflow.
-            {
-                let mut f = OpenOptions::new()
-                    .read(true)
-                    .append(true)
-                    .open(path)
-                    .unwrap();
-                f.write_all(&[0xFF; STORE_META_OVERHEAD]).unwrap();
-            }
-            size + STORE_META_OVERHEAD
+            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::<StoredMeta>(),
+                )
+            };
+            let account_meta_slice: &[u8] = unsafe {
+                std::slice::from_raw_parts(
+                    (&fake_account_meta as *const AccountMeta) as *const u8,
+                    mem::size_of::<AccountMeta>(),
+                )
+            };
+
+            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::<StoredMeta>() + mem::size_of::<AccountMeta>()
         });
     }
 
     /// A helper fn to test scan_index
     fn test_scan_index_helper(
         storage_access: StorageAccess,
-        callback: impl Fn(&PathBuf, usize) -> usize,
+        modify_fn: impl Fn(&PathBuf, usize) -> usize,
     ) {
         test_scan_helper(
             storage_access,
-            callback,
+            modify_fn,
             |append_vec, pubkeys, account_offsets, accounts| {
                 let mut i = 0;
                 append_vec.scan_index(|index_info| {
@@ -2069,6 +2083,12 @@ pub mod tests {
         )
     }
 
+    #[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)]
@@ -2128,23 +2148,4 @@ pub mod tests {
             size + mem::size_of::<StoredMeta>() + mem::size_of::<AccountMeta>()
         });
     }
-
-    /// Test `scan_index` for storage with garbage data that cause overflow in next element offset calculation.
-    #[test_case(StorageAccess::Mmap)]
-    #[test_case(StorageAccess::File)]
-    #[should_panic(expected = "stored size cannot overflow")]
-    fn test_scan_index_overflow(storage_access: StorageAccess) {
-        test_scan_index_helper(storage_access, |path, size| {
-            // Append a garbage AccountMeta to simulate next_offset overflow.
-            {
-                let mut f = OpenOptions::new()
-                    .read(true)
-                    .append(true)
-                    .open(path)
-                    .unwrap();
-                f.write_all(&[0xFF; STORE_META_OVERHEAD]).unwrap();
-            }
-            size + STORE_META_OVERHEAD
-        });
-    }
 }