diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 8dd9179e86d1d2..5ef448bb7c4625 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -38,10 +38,7 @@ use { AccountsStats, CleanAccountsStats, FlushStats, ObsoleteAccountsStats, PurgeStats, ShrinkAncientStats, ShrinkStats, ShrinkStatsSub, StoreAccountsTiming, }, - accounts_file::{ - AccountsFile, AccountsFileError, AccountsFileProvider, MatchAccountOwnerError, - StorageAccess, - }, + accounts_file::{AccountsFile, AccountsFileError, AccountsFileProvider, StorageAccess}, accounts_hash::{AccountLtHash, AccountsLtHash, ZERO_LAMPORT_ACCOUNT_LT_HASH}, accounts_index::{ in_mem_accounts_index::StartupStats, AccountSecondaryIndexes, AccountsIndex, @@ -831,36 +828,6 @@ impl LoadedAccountAccessor<'_> { } } } - - fn account_matches_owners(&self, owners: &[Pubkey]) -> Result { - match self { - LoadedAccountAccessor::Cached(cached_account) => cached_account - .as_ref() - .and_then(|cached_account| { - if cached_account.account.is_zero_lamport() { - None - } else { - owners - .iter() - .position(|entry| cached_account.account.owner() == entry) - } - }) - .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() - .map(|(storage_entry, offset)| { - storage_entry - .accounts - .account_matches_owners(*offset, owners) - }) - .unwrap_or(Err(MatchAccountOwnerError::UnableToLoad)) - } - } - } } pub enum LoadedAccount<'a> { @@ -4095,47 +4062,6 @@ impl AccountsDb { self.do_load(ancestors, pubkey, None, load_hint, LoadZeroLamports::None) } - /// Return Ok(index_of_matching_owner) if the account owner at `offset` is one of the pubkeys in `owners`. - /// Return Err(MatchAccountOwnerError::NoMatch) if the account has 0 lamports or the owner is not one of - /// the pubkeys in `owners`. - /// Return Err(MatchAccountOwnerError::UnableToLoad) if the account could not be accessed. - pub fn account_matches_owners( - &self, - ancestors: &Ancestors, - account: &Pubkey, - owners: &[Pubkey], - ) -> Result { - let (slot, storage_location, _maybe_account_accessor) = 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 if account.is_zero_lamport() { - Err(MatchAccountOwnerError::NoMatch) - } else { - owners - .iter() - .position(|entry| account.owner() == entry) - .ok_or(MatchAccountOwnerError::NoMatch) - }; - } - } - - 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) - } - /// load the account with `pubkey` into the read only accounts cache. /// The goal is to make subsequent loads (which caller expects to occur) to find the account quickly. pub fn load_account_into_read_cache(&self, ancestors: &Ancestors, pubkey: &Pubkey) { diff --git a/accounts-db/src/accounts_db/tests.rs b/accounts-db/src/accounts_db/tests.rs index 66fb63beb5f72e..24f19569672a92 100644 --- a/accounts-db/src/accounts_db/tests.rs +++ b/accounts-db/src/accounts_db/tests.rs @@ -3144,100 +3144,6 @@ fn test_load_with_read_only_accounts_cache() { assert_eq!(slot, 2); } -#[test] -fn test_account_matches_owners() { - let db = Arc::new(AccountsDb::new_single_for_tests()); - - let owners: Vec = (0..2).map(|_| Pubkey::new_unique()).collect(); - - let account1_key = Pubkey::new_unique(); - let account1 = AccountSharedData::new(321, 10, &owners[0]); - - let account2_key = Pubkey::new_unique(); - let account2 = AccountSharedData::new(1, 1, &owners[1]); - - let account3_key = Pubkey::new_unique(); - let account3 = AccountSharedData::new(1, 1, &Pubkey::new_unique()); - - // Account with 0 lamports - let account4_key = Pubkey::new_unique(); - let account4 = AccountSharedData::new(0, 1, &owners[1]); - - db.store_cached((0, &[(&account1_key, &account1)][..])); - db.store_cached((1, &[(&account2_key, &account2)][..])); - db.store_cached((2, &[(&account3_key, &account3)][..])); - db.store_cached((3, &[(&account4_key, &account4)][..])); - - db.add_root(0); - db.add_root(1); - db.add_root(2); - db.add_root(3); - - // Set the latest full snapshot slot to one that is *older* than the slot account4 is in. - // This is required to ensure account4 is not purged during `clean`, - // which is required to have account_matches_owners() return NoMatch. - db.set_latest_full_snapshot_slot(2); - - // Flush the cache so that the account meta will be read from the storage - db.flush_accounts_cache(true, None); - db.clean_accounts_for_tests(); - - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &account1_key, &owners), - Ok(0) - ); - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &account2_key, &owners), - Ok(1) - ); - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &account3_key, &owners), - Err(MatchAccountOwnerError::NoMatch) - ); - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &account4_key, &owners), - Err(MatchAccountOwnerError::NoMatch) - ); - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &Pubkey::new_unique(), &owners), - Err(MatchAccountOwnerError::UnableToLoad) - ); - - // Flush the cache and load account1 (so that it's in the cache) - db.flush_accounts_cache(true, None); - db.clean_accounts_for_tests(); - let _ = db - .do_load( - &Ancestors::default(), - &account1_key, - Some(0), - LoadHint::Unspecified, - LoadZeroLamports::SomeWithZeroLamportAccountForTests, - ) - .unwrap(); - - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &account1_key, &owners), - Ok(0) - ); - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &account2_key, &owners), - Ok(1) - ); - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &account3_key, &owners), - Err(MatchAccountOwnerError::NoMatch) - ); - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &account4_key, &owners), - Err(MatchAccountOwnerError::NoMatch) - ); - assert_eq!( - db.account_matches_owners(&Ancestors::default(), &Pubkey::new_unique(), &owners), - Err(MatchAccountOwnerError::UnableToLoad) - ); -} - /// a test that will accept either answer const LOAD_ZERO_LAMPORTS_ANY_TESTS: LoadZeroLamports = LoadZeroLamports::None; diff --git a/accounts-db/src/accounts_file.rs b/accounts-db/src/accounts_file.rs index ac21f6293ab70a..6ffdfbbf899bd7 100644 --- a/accounts-db/src/accounts_file.rs +++ b/accounts-db/src/accounts_file.rs @@ -47,14 +47,6 @@ pub enum AccountsFileError { TieredStorageError(#[from] TieredStorageError), } -#[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, -} - #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub enum StorageAccess { /// storages should be accessed by Mmap @@ -263,28 +255,6 @@ impl AccountsFile { } } - pub fn account_matches_owners( - &self, - offset: usize, - owners: &[Pubkey], - ) -> std::result::Result { - match self { - Self::AppendVec(av) => av.account_matches_owners(offset, owners), - // Note: The conversion here is needed as the AccountsDB currently - // assumes all offsets are multiple of 8 while TieredStorage uses - // IndexOffset that is equivalent to AccountInfo::reduced_offset. - Self::TieredStorage(ts) => { - let Some(reader) = ts.reader() else { - return Err(MatchAccountOwnerError::UnableToLoad); - }; - reader.account_matches_owners( - IndexOffset(AccountInfo::get_reduced_offset(offset)), - owners, - ) - } - } - } - /// Return the path of the underlying account file. pub fn path(&self) -> &Path { match self { diff --git a/accounts-db/src/append_vec.rs b/accounts-db/src/append_vec.rs index 5f261f0ed1f5bf..b0119f17524c9e 100644 --- a/accounts-db/src/append_vec.rs +++ b/accounts-db/src/append_vec.rs @@ -19,8 +19,7 @@ use { account_info::Offset, account_storage::stored_account_info::{StoredAccountInfo, StoredAccountInfoWithoutData}, accounts_file::{ - AccountsFileError, InternalsForArchive, MatchAccountOwnerError, Result, StorageAccess, - StoredAccountsInfo, + AccountsFileError, InternalsForArchive, Result, StorageAccess, StoredAccountsInfo, }, buffered_reader::{ BufReaderWithOverflow, BufferedReader, FileBufRead as _, RequiredLenBufFileRead, @@ -893,28 +892,6 @@ impl AppendVec { } } - /// Return Ok(index_of_matching_owner) if the account owner at `offset` is one of the pubkeys in `owners`. - /// Return Err(MatchAccountOwnerError::NoMatch) if the account has 0 lamports or the owner is not one of - /// the pubkeys in `owners`. - /// Return Err(MatchAccountOwnerError::UnableToLoad) if the `offset` value causes a data overrun. - pub fn account_matches_owners( - &self, - offset: usize, - owners: &[Pubkey], - ) -> std::result::Result { - self.get_stored_account_no_data_callback(offset, |stored_account_meta| { - if stored_account_meta.lamports() == 0 { - Err(MatchAccountOwnerError::NoMatch) - } else { - owners - .iter() - .position(|entry| stored_account_meta.owner() == entry) - .ok_or(MatchAccountOwnerError::NoMatch) - } - }) - .unwrap_or(Err(MatchAccountOwnerError::UnableToLoad)) - } - #[cfg(test)] pub fn get_account_test( &self, @@ -1591,46 +1568,6 @@ pub mod tests { assert_eq!(av.get_account_test(index1).unwrap(), account1); } - #[test] - fn test_account_matches_owners() { - let path = get_append_vec_path("test_append_data"); - let av = AppendVec::new(&path.path, true, 1024 * 1024); - let owners: Vec = (0..2).map(|_| Pubkey::new_unique()).collect(); - - 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), Ok(0)); - - 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), Ok(1)); - assert_eq!(av.account_matches_owners(index, &owners), Ok(0)); - - 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), - Err(MatchAccountOwnerError::NoMatch) - ); - - // tests for overflow - assert_eq!( - av.account_matches_owners(usize::MAX - mem::size_of::(), &owners), - Err(MatchAccountOwnerError::UnableToLoad) - ); - - assert_eq!( - av.account_matches_owners( - usize::MAX - mem::size_of::() - mem::size_of::() + 1, - &owners - ), - Err(MatchAccountOwnerError::UnableToLoad) - ); - } - impl AppendVec { /// return how many accounts in the storage fn accounts_count(&self) -> usize { diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 3651679a99b0a5..b6b221246aefad 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -4,7 +4,7 @@ use { crate::{ account_info::{AccountInfo, Offset}, account_storage::stored_account_info::{StoredAccountInfo, StoredAccountInfoWithoutData}, - accounts_file::{MatchAccountOwnerError, StoredAccountsInfo}, + accounts_file::StoredAccountsInfo, tiered_storage::{ byte_block, file::{TieredReadableFile, TieredWritableFile}, @@ -440,38 +440,6 @@ impl HotStorageReader { .get_owner_address(&self.mmap, &self.footer, owner_offset) } - /// Returns Ok(index_of_matching_owner) if the account owner at - /// `account_offset` is one of the pubkeys in `owners`. - /// - /// Returns Err(MatchAccountOwnerError::NoMatch) if the account has 0 - /// lamports or the owner is not one of the pubkeys in `owners`. - /// - /// Returns Err(MatchAccountOwnerError::UnableToLoad) if there is any internal - /// error that causes the data unable to load, including `account_offset` - /// causes a data overrun. - pub fn account_matches_owners( - &self, - account_offset: HotAccountOffset, - owners: &[Pubkey], - ) -> Result { - let account_meta = self - .get_account_meta_from_offset(account_offset) - .map_err(|_| MatchAccountOwnerError::UnableToLoad)?; - - if account_meta.lamports() == 0 { - Err(MatchAccountOwnerError::NoMatch) - } else { - let account_owner = self - .get_owner_address(account_meta.owner_offset()) - .map_err(|_| MatchAccountOwnerError::UnableToLoad)?; - - owners - .iter() - .position(|candidate| account_owner == candidate) - .ok_or(MatchAccountOwnerError::NoMatch) - } - } - /// Returns the size of the account block based on its account offset /// and index offset. /// @@ -870,7 +838,7 @@ mod tests { }, assert_matches::assert_matches, memoffset::offset_of, - rand::{seq::SliceRandom, Rng}, + rand::Rng, solana_account::ReadableAccount, solana_clock::{Epoch, Slot}, solana_hash::Hash, @@ -1358,115 +1326,6 @@ mod tests { } } - #[test] - fn test_account_matches_owners() { - // Generate a new temp path that is guaranteed to NOT already have a file. - let temp_dir = TempDir::new().unwrap(); - let path = temp_dir.path().join("test_hot_storage_get_owner_address"); - const NUM_OWNERS: u32 = 10; - - let owner_addresses: Vec<_> = std::iter::repeat_with(Pubkey::new_unique) - .take(NUM_OWNERS as usize) - .collect(); - - const NUM_ACCOUNTS: u32 = 30; - let mut rng = rand::thread_rng(); - - let hot_account_metas: Vec<_> = std::iter::repeat_with({ - || { - HotAccountMeta::new() - .with_lamports(rng.gen_range(1..u64::MAX)) - .with_owner_offset(OwnerOffset(rng.gen_range(0..NUM_OWNERS))) - } - }) - .take(NUM_ACCOUNTS as usize) - .collect(); - let mut footer = TieredStorageFooter { - account_meta_format: AccountMetaFormat::Hot, - account_entry_count: NUM_ACCOUNTS, - owner_count: NUM_OWNERS, - ..TieredStorageFooter::default() - }; - let account_offsets: Vec<_>; - - { - let mut file = TieredWritableFile::new(&path).unwrap(); - let mut current_offset = 0; - - account_offsets = hot_account_metas - .iter() - .map(|meta| { - let prev_offset = current_offset; - current_offset += file.write_pod(meta).unwrap(); - HotAccountOffset::new(prev_offset).unwrap() - }) - .collect(); - footer.index_block_offset = current_offset as u64; - // Typically, the owners block is stored after index block, but - // since we don't write index block in this test, so we have - // the owners_block_offset set to the end of the accounts blocks. - footer.owners_block_offset = footer.index_block_offset; - - let mut owners_table = OwnersTable::default(); - owner_addresses.iter().for_each(|owner_address| { - owners_table.insert(owner_address); - }); - footer - .owners_block_format - .write_owners_block(&mut file, &owners_table) - .unwrap(); - - // while the test only focuses on account metas, writing a footer - // here is necessary to make it a valid tiered-storage file. - footer.write_footer_block(&mut file).unwrap(); - } - - let file = TieredReadableFile::new(&path).unwrap(); - let hot_storage = HotStorageReader::new(file).unwrap(); - - // First, verify whether we can find the expected owners. - let mut owner_candidates = owner_addresses.clone(); - owner_candidates.shuffle(&mut rng); - - for (account_offset, account_meta) in account_offsets.iter().zip(hot_account_metas.iter()) { - let index = hot_storage - .account_matches_owners(*account_offset, &owner_candidates) - .unwrap(); - assert_eq!( - owner_candidates[index], - owner_addresses[account_meta.owner_offset().0 as usize] - ); - } - - // Second, verify the MatchAccountOwnerError::NoMatch case - const NUM_UNMATCHED_OWNERS: usize = 20; - let unmatched_candidates: Vec<_> = std::iter::repeat_with(Pubkey::new_unique) - .take(NUM_UNMATCHED_OWNERS) - .collect(); - - for account_offset in account_offsets.iter() { - assert_eq!( - hot_storage.account_matches_owners(*account_offset, &unmatched_candidates), - Err(MatchAccountOwnerError::NoMatch) - ); - } - - // Thirdly, we mixed two candidates and make sure we still find the - // matched owner. - owner_candidates.extend(unmatched_candidates); - owner_candidates.shuffle(&mut rng); - - for (account_offset, account_meta) in account_offsets.iter().zip(hot_account_metas.iter()) { - let index = hot_storage - .account_matches_owners(*account_offset, &owner_candidates) - .unwrap(); - assert_eq!( - owner_candidates[index], - owner_addresses[account_meta.owner_offset().0 as usize] - ); - } - } - #[test] fn test_get_stored_account_without_data_callback() { const NUM_ACCOUNTS: usize = 20; diff --git a/accounts-db/src/tiered_storage/readable.rs b/accounts-db/src/tiered_storage/readable.rs index 7b2aeb9cab094f..056b37878029eb 100644 --- a/accounts-db/src/tiered_storage/readable.rs +++ b/accounts-db/src/tiered_storage/readable.rs @@ -2,7 +2,6 @@ use { crate::{ account_info::Offset, account_storage::stored_account_info::{StoredAccountInfo, StoredAccountInfoWithoutData}, - accounts_file::MatchAccountOwnerError, tiered_storage::{ file::TieredReadableFile, footer::{AccountMetaFormat, TieredStorageFooter}, @@ -110,30 +109,6 @@ impl TieredStorageReader { } } - /// Returns Ok(index_of_matching_owner) if the account owner at - /// `account_offset` is one of the pubkeys in `owners`. - /// - /// Returns Err(MatchAccountOwnerError::NoMatch) if the account has 0 - /// lamports or the owner is not one of the pubkeys in `owners`. - /// - /// Returns Err(MatchAccountOwnerError::UnableToLoad) if there is any internal - /// error that causes the data unable to load, including `account_offset` - /// causes a data overrun. - pub fn account_matches_owners( - &self, - index_offset: IndexOffset, - owners: &[Pubkey], - ) -> Result { - match self { - Self::Hot(hot) => { - let account_offset = hot - .get_account_offset(index_offset) - .map_err(|_| MatchAccountOwnerError::UnableToLoad)?; - hot.account_matches_owners(account_offset, owners) - } - } - } - /// iterate over all pubkeys pub fn scan_pubkeys(&self, callback: impl FnMut(&Pubkey)) -> TieredStorageResult<()> { match self {