add test for duplicates in generate_index and fix approx stored count (#33290)

add test

accounts-db/src/accounts_db.rs

@@ -9494,9 +9494,12 @@ impl AccountsDb {
                     count_and_status.0 = entry.count;
                 }
                 store.alive_bytes.store(entry.stored_size, Ordering::SeqCst);
-                store
-                    .approx_store_count
-                    .store(entry.count, Ordering::Relaxed);
+                assert!(
+                    store.approx_stored_count() >= entry.count,
+                    "{}, {}",
+                    store.approx_stored_count(),
+                    entry.count
+                );
             } else {
                 trace!("id: {} clearing count", id);
                 store.count_and_status.write().unwrap().0 = 0;
@@ -10054,6 +10057,74 @@ pub mod tests {
         }
     }
 
+    #[test]
+    fn test_generate_index_duplicates_within_slot() {
+        for reverse in [false, true] {
+            let db = AccountsDb::new_single_for_tests();
+            let slot0 = 0;
+
+            let pubkey = Pubkey::from([1; 32]);
+
+            let append_vec = db.create_and_insert_store(slot0, 1000, "test");
+
+            let mut account_small = AccountSharedData::default();
+            account_small.set_data(vec![1]);
+            account_small.set_lamports(1);
+            let mut account_big = AccountSharedData::default();
+            account_big.set_data(vec![5; 10]);
+            account_big.set_lamports(2);
+            assert_ne!(
+                aligned_stored_size(account_big.data().len()),
+                aligned_stored_size(account_small.data().len())
+            );
+            // same account twice with different data lens
+            // Rules are the last one of each pubkey is the one that ends up in the index.
+            let mut data = vec![(&pubkey, &account_big), (&pubkey, &account_small)];
+            if reverse {
+                data = data.into_iter().rev().collect();
+            }
+            let expected_alive_bytes = if reverse {
+                aligned_stored_size(account_big.data().len())
+            } else {
+                aligned_stored_size(account_small.data().len())
+            };
+            let expected_accounts_data_len = data.last().unwrap().1.data().len();
+            let storable = (slot0, &data[..], INCLUDE_SLOT_IN_HASH_TESTS);
+            let hashes = data.iter().map(|_| Hash::default()).collect::<Vec<_>>();
+            let write_versions = data.iter().map(|_| 0).collect::<Vec<_>>();
+            let append =
+                StorableAccountsWithHashesAndWriteVersions::new_with_hashes_and_write_versions(
+                    &storable,
+                    hashes,
+                    write_versions,
+                );
+
+            // construct append vec with account to generate an index from
+            append_vec.accounts.append_accounts(&append, 0);
+            // append vecs set this at load
+            append_vec
+                .approx_store_count
+                .store(data.len(), Ordering::Relaxed);
+
+            let genesis_config = GenesisConfig::default();
+            assert!(db.accounts_index.get_account_read_entry(&pubkey).is_none());
+            let result = db.generate_index(None, false, &genesis_config);
+            // index entry should only contain a single entry for the pubkey since index cannot hold more than 1 entry per slot
+            let entry = db.accounts_index.get_account_read_entry(&pubkey).unwrap();
+            assert_eq!(entry.slot_list().len(), 1);
+            assert_eq!(append_vec.alive_bytes(), expected_alive_bytes);
+            // total # accounts in append vec
+            assert_eq!(append_vec.approx_stored_count(), 2);
+            // # alive accounts
+            assert_eq!(append_vec.count(), 1);
+            // all account data alive
+            assert_eq!(
+                result.accounts_data_len as usize, expected_accounts_data_len,
+                "reverse: {reverse}"
+            );
+        }
+    }
+
     #[test]
     fn test_maybe_unref_accounts_already_in_ancient() {
         let db = AccountsDb::new_single_for_tests();
@@ -15859,21 +15930,29 @@ pub mod tests {
             count_and_status.0 = 0;
         }
 
+        // count needs to be <= approx stored count in store.
+        // approx stored count is 1 in store since we added a single account.
+        let count = 1;
+
         // populate based on made up hash data
         let dashmap = DashMap::default();
         dashmap.insert(
             0,
             StorageSizeAndCount {
                 stored_size: 2,
-                count: 3,
+                count,
             },
         );
 
+        for (_, store) in accounts.storage.iter() {
+            assert_eq!(store.count_and_status.read().unwrap().0, 0);
+            assert_eq!(store.alive_bytes.load(Ordering::Acquire), 0);
+        }
         accounts.set_storage_count_and_alive_bytes(dashmap, &mut GenerateIndexTimings::default());
         assert_eq!(accounts.storage.len(), 1);
         for (_, store) in accounts.storage.iter() {
             assert_eq!(store.append_vec_id(), 0);
-            assert_eq!(store.count_and_status.read().unwrap().0, 3);
+            assert_eq!(store.count_and_status.read().unwrap().0, count);
             assert_eq!(store.alive_bytes.load(Ordering::Acquire), 2);
         }
     }