return Result instead of Option

Author: pgarg66

runtime/src/accounts_db.rs

@@ -42,9 +42,9 @@ use {
             get_ancient_append_vec_capacity, is_ancient, AccountsToStore, StorageSelector,
         },
         append_vec::{
-            aligned_stored_size, AppendVec, StorableAccountsWithHashesAndWriteVersions,
-            StoredAccountMeta, StoredMetaWriteVersion, APPEND_VEC_MMAPPED_FILES_OPEN,
-            STORE_META_OVERHEAD,
+            aligned_stored_size, AppendVec, MatchAccountOwnerError,
+            StorableAccountsWithHashesAndWriteVersions, StoredAccountMeta, StoredMetaWriteVersion,
+            APPEND_VEC_MMAPPED_FILES_OPEN, STORE_META_OVERHEAD,
         },
         cache_hash_data::{CacheHashData, CacheHashDataFile},
         contains::Contains,
@@ -816,22 +816,28 @@ impl<'a> LoadedAccountAccessor<'a> {
         }
     }
 
-    fn account_matches_owners(&self, owners: &[&Pubkey]) -> Option<bool> {
+    fn account_matches_owners(&self, owners: &[&Pubkey]) -> Result<(), MatchAccountOwnerError> {
         match self {
             LoadedAccountAccessor::Cached(cached_account) => cached_account
                 .as_ref()
-                .map(|cached_account| owners.contains(&cached_account.account.owner())),
+                .and_then(|cached_account| {
+                    owners
+                        .contains(&cached_account.account.owner())
+                        .then_some(())
+                })
+                .ok_or(MatchAccountOwnerError::NoMatch),
             LoadedAccountAccessor::Stored(maybe_storage_entry) => {
                 // storage entry may not be present if slot was cleaned up in
                 // between reading the accounts index and calling this function to
                 // get account meta from the storage entry here
                 maybe_storage_entry
                     .as_ref()
-                    .and_then(|(storage_entry, offset)| {
+                    .map(|(storage_entry, offset)| {
                         storage_entry
                             .accounts
                             .account_matches_owners(*offset, owners)
                     })
+                    .unwrap_or(Err(MatchAccountOwnerError::UnableToLoad))
             }
         }
     }
@@ -4943,25 +4949,31 @@ impl AccountsDb {
         ancestors: &Ancestors,
         account: &Pubkey,
         owners: &[&Pubkey],
-    ) -> Option<bool> {
-        let (slot, storage_location, _maybe_account_accesor) =
-            self.read_index_for_accessor_or_load_slow(ancestors, account, None, false)?;
+    ) -> Result<(), MatchAccountOwnerError> {
+        let (slot, storage_location, _maybe_account_accesor) = self
+            .read_index_for_accessor_or_load_slow(ancestors, account, None, false)
+            .ok_or(MatchAccountOwnerError::UnableToLoad)?;
 
         if !storage_location.is_cached() {
             let result = self.read_only_accounts_cache.load(*account, slot);
             if let Some(account) = result {
-                return Some(owners.contains(&account.owner()));
+                return owners
+                    .contains(&account.owner())
+                    .then_some(())
+                    .ok_or(MatchAccountOwnerError::NoMatch);
             }
         }
 
-        let (account_accessor, _slot) = self.retry_to_get_account_accessor(
-            slot,
-            storage_location,
-            ancestors,
-            account,
-            None,
-            LoadHint::Unspecified,
-        )?;
+        let (account_accessor, _slot) = self
+            .retry_to_get_account_accessor(
+                slot,
+                storage_location,
+                ancestors,
+                account,
+                None,
+                LoadHint::Unspecified,
+            )
+            .ok_or(MatchAccountOwnerError::UnableToLoad)?;
         account_accessor.account_matches_owners(owners)
     }
 
@@ -14150,19 +14162,19 @@ pub mod tests {
 
         assert_eq!(
             db.account_matches_owners(&Ancestors::default(), &account1_key, &owners_refs),
-            Some(true)
+            Ok(())
         );
         assert_eq!(
             db.account_matches_owners(&Ancestors::default(), &account2_key, &owners_refs),
-            Some(true)
+            Ok(())
         );
         assert_eq!(
             db.account_matches_owners(&Ancestors::default(), &account3_key, &owners_refs),
-            Some(false)
+            Err(MatchAccountOwnerError::NoMatch)
         );
         assert_eq!(
             db.account_matches_owners(&Ancestors::default(), &Pubkey::new_unique(), &owners_refs),
-            None
+            Err(MatchAccountOwnerError::UnableToLoad)
         );
 
         // Flush the cache and load account1 (so that it's in the cache)
@@ -14180,19 +14192,19 @@ pub mod tests {
 
         assert_eq!(
             db.account_matches_owners(&Ancestors::default(), &account1_key, &owners_refs),
-            Some(true)
+            Ok(())
         );
         assert_eq!(
             db.account_matches_owners(&Ancestors::default(), &account2_key, &owners_refs),
-            Some(true)
+            Ok(())
         );
         assert_eq!(
             db.account_matches_owners(&Ancestors::default(), &account3_key, &owners_refs),
-            Some(false)
+            Err(MatchAccountOwnerError::NoMatch)
         );
         assert_eq!(
             db.account_matches_owners(&Ancestors::default(), &Pubkey::new_unique(), &owners_refs),
-            None
+            Err(MatchAccountOwnerError::UnableToLoad)
         );
     }
 

runtime/src/append_vec.rs

@@ -28,6 +28,7 @@ use {
             Mutex,
         },
     },
+    thiserror::Error,
 };
 
 pub mod test_utils;
@@ -270,6 +271,14 @@ impl<'a> Iterator for AppendVecAccountsIter<'a> {
     }
 }
 
+#[derive(Error, Debug, PartialEq, Eq)]
+pub enum MatchAccountOwnerError {
+    #[error("The account owner does not match with the provided list")]
+    NoMatch,
+    #[error("Unable to load the account")]
+    UnableToLoad,
+}
+
 /// A thread-safe, file-backed block of memory used to store `Account` instances. Append operations
 /// are serialized such that only one thread updates the internal `append_lock` at a time. No
 /// restrictions are placed on reading. That is, one may read items from one thread while another
@@ -594,17 +603,30 @@ impl AppendVec {
         ))
     }
 
-    /// Return Some(true) if the account owner at `offset` is one of the pubkeys in `owners`.
-    /// Return Some(false) if the account owner is not one of the pubkeys in `owners`.
-    /// It returns None if the `offset` value causes a data overrun.
-    pub fn account_matches_owners(&self, offset: usize, owners: &[&Pubkey]) -> Option<bool> {
+    fn get_account_meta<'a>(&'a self, offset: usize) -> Option<&'a AccountMeta> {
         // Skip over StoredMeta data in the account
         let offset = offset.checked_add(mem::size_of::<StoredMeta>())?;
         // u64_align! does an unchecked add for alignment. Check that it won't cause an overflow.
         offset.checked_add(ALIGN_BOUNDARY_OFFSET - 1)?;
         let (account_meta, _): (&AccountMeta, _) = self.get_type(u64_align!(offset))?;
+        Some(account_meta)
+    }
 
-        Some(owners.contains(&&account_meta.owner))
+    /// Return Some(true) if the account owner at `offset` is one of the pubkeys in `owners`.
+    /// Return Some(false) if the account owner is not one of the pubkeys in `owners`.
+    /// It returns None if the `offset` value causes a data overrun.
+    pub fn account_matches_owners(
+        &self,
+        offset: usize,
+        owners: &[&Pubkey],
+    ) -> Result<(), MatchAccountOwnerError> {
+        let account_meta = self
+            .get_account_meta(offset)
+            .ok_or(MatchAccountOwnerError::UnableToLoad)?;
+        owners
+            .contains(&&account_meta.owner)
+            .then_some(())
+            .ok_or(MatchAccountOwnerError::NoMatch)
     }
 
     #[cfg(test)]
@@ -1070,31 +1092,34 @@ pub mod tests {
         let mut account = create_test_account(5);
         account.1.set_owner(owners[0]);
         let index = av.append_account_test(&account).unwrap();
-        assert_eq!(av.account_matches_owners(index, &owners_refs), Some(true));
+        assert_eq!(av.account_matches_owners(index, &owners_refs), Ok(()));
 
         let mut account1 = create_test_account(6);
         account1.1.set_owner(owners[1]);
         let index1 = av.append_account_test(&account1).unwrap();
-        assert_eq!(av.account_matches_owners(index1, &owners_refs), Some(true));
-        assert_eq!(av.account_matches_owners(index, &owners_refs), Some(true));
+        assert_eq!(av.account_matches_owners(index1, &owners_refs), Ok(()));
+        assert_eq!(av.account_matches_owners(index, &owners_refs), Ok(()));
 
         let mut account2 = create_test_account(6);
         account2.1.set_owner(Pubkey::new_unique());
         let index2 = av.append_account_test(&account2).unwrap();
-        assert_eq!(av.account_matches_owners(index2, &owners_refs), Some(false));
+        assert_eq!(
+            av.account_matches_owners(index2, &owners_refs),
+            Err(MatchAccountOwnerError::NoMatch)
+        );
 
         // tests for overflow
         assert_eq!(
             av.account_matches_owners(usize::MAX - mem::size_of::<StoredMeta>(), &owners_refs),
-            None
+            Err(MatchAccountOwnerError::UnableToLoad)
         );
 
         assert_eq!(
             av.account_matches_owners(
                 usize::MAX - mem::size_of::<StoredMeta>() - mem::size_of::<AccountMeta>() + 1,
                 &owners_refs
             ),
-            None
+            Err(MatchAccountOwnerError::UnableToLoad)
         );
     }