From 33e179905f928533e7c921db3c3e850381326a34 Mon Sep 17 00:00:00 2001
From: Yueh-Hsuan Chiang <93241502+yhchiang-sol@users.noreply.github.com>
Date: Mon, 27 Feb 2023 10:03:22 -0800
Subject: [PATCH] getter functions for StoredAccountMeta (#30447)

This PR makes all the StoredAccountMeta fields pub(crate)
and provides getter functions to access its member fields.

This PR is a preparation step for abstracting out StoredAccountMeta.
---
 .../src/accounts_update_notifier.rs           | 12 ++--
 runtime/benches/append_vec.rs                 |  8 +--
 runtime/src/account_storage/meta.rs           | 54 ++++++++++++++++--
 runtime/src/accounts_db.rs                    | 56 ++++++-------------
 .../src/accounts_db/geyser_plugin_utils.rs    |  8 +--
 runtime/src/ancient_append_vecs.rs            |  2 +-
 runtime/src/append_vec.rs                     | 10 ++--
 runtime/src/snapshot_minimizer.rs             |  6 +-
 runtime/src/storable_accounts.rs              | 19 +++----
 runtime/store-tool/src/main.rs                | 22 +++++---
 10 files changed, 109 insertions(+), 88 deletions(-)

diff --git a/geyser-plugin-manager/src/accounts_update_notifier.rs b/geyser-plugin-manager/src/accounts_update_notifier.rs
index 7c502462bf632c..f85f2c97717625 100644
--- a/geyser-plugin-manager/src/accounts_update_notifier.rs
+++ b/geyser-plugin-manager/src/accounts_update_notifier.rs
@@ -129,12 +129,12 @@ impl AccountsUpdateNotifierImpl {
     ) -> Option<ReplicaAccountInfoV3<'a>> {
         Some(ReplicaAccountInfoV3 {
             pubkey: stored_account_meta.pubkey().as_ref(),
-            lamports: stored_account_meta.account_meta.lamports,
-            owner: stored_account_meta.account_meta.owner.as_ref(),
-            executable: stored_account_meta.account_meta.executable,
-            rent_epoch: stored_account_meta.account_meta.rent_epoch,
-            data: stored_account_meta.data,
-            write_version: stored_account_meta.meta.write_version_obsolete,
+            lamports: stored_account_meta.lamports(),
+            owner: stored_account_meta.owner().as_ref(),
+            executable: stored_account_meta.executable(),
+            rent_epoch: stored_account_meta.rent_epoch(),
+            data: stored_account_meta.data(),
+            write_version: stored_account_meta.write_version(),
             txn: None,
         })
     }
diff --git a/runtime/benches/append_vec.rs b/runtime/benches/append_vec.rs
index 19704d4951af14..9d2bd60b683380 100644
--- a/runtime/benches/append_vec.rs
+++ b/runtime/benches/append_vec.rs
@@ -79,7 +79,7 @@ fn append_vec_sequential_read(bencher: &mut Bencher) {
         println!("reading pos {sample} {pos}");
         let (account, _next) = vec.get_account(pos).unwrap();
         let (_meta, test) = create_test_account(sample);
-        assert_eq!(account.data, test.data());
+        assert_eq!(account.data(), test.data());
         indexes.push((sample, pos));
     });
 }
@@ -94,7 +94,7 @@ fn append_vec_random_read(bencher: &mut Bencher) {
         let (sample, pos) = &indexes[random_index];
         let (account, _next) = vec.get_account(*pos).unwrap();
         let (_meta, test) = create_test_account(*sample);
-        assert_eq!(account.data, test.data());
+        assert_eq!(account.data(), test.data());
     });
 }
 
@@ -123,7 +123,7 @@ fn append_vec_concurrent_append_read(bencher: &mut Bencher) {
         let (sample, pos) = *indexes.lock().unwrap().get(random_index).unwrap();
         let (account, _next) = vec.get_account(pos).unwrap();
         let (_meta, test) = create_test_account(sample);
-        assert_eq!(account.data, test.data());
+        assert_eq!(account.data(), test.data());
     });
 }
 
@@ -143,7 +143,7 @@ fn append_vec_concurrent_read_append(bencher: &mut Bencher) {
         let (sample, pos) = *indexes1.lock().unwrap().get(random_index % len).unwrap();
         let (account, _next) = vec1.get_account(pos).unwrap();
         let (_meta, test) = create_test_account(sample);
-        assert_eq!(account.data, test.data());
+        assert_eq!(account.data(), test.data());
     });
     bencher.iter(|| {
         let sample: usize = thread_rng().gen_range(0, 256);
diff --git a/runtime/src/account_storage/meta.rs b/runtime/src/account_storage/meta.rs
index 24b065abefa50b..386148b55acde1 100644
--- a/runtime/src/account_storage/meta.rs
+++ b/runtime/src/account_storage/meta.rs
@@ -98,13 +98,13 @@ impl<'a: 'b, 'b, T: ReadableAccount + Sync + 'b, U: StorableAccounts<'a, T>, V:
 /// (see `StoredAccountMeta::clone_account()`).
 #[derive(PartialEq, Eq, Debug)]
 pub struct StoredAccountMeta<'a> {
-    pub meta: &'a StoredMeta,
+    pub(crate) meta: &'a StoredMeta,
     /// account data
-    pub account_meta: &'a AccountMeta,
-    pub data: &'a [u8],
-    pub offset: usize,
-    pub stored_size: usize,
-    pub hash: &'a Hash,
+    pub(crate) account_meta: &'a AccountMeta,
+    pub(crate) data: &'a [u8],
+    pub(crate) offset: usize,
+    pub(crate) stored_size: usize,
+    pub(crate) hash: &'a Hash,
 }
 
 impl<'a> StoredAccountMeta<'a> {
@@ -123,6 +123,30 @@ impl<'a> StoredAccountMeta<'a> {
         &self.meta.pubkey
     }
 
+    pub fn hash(&self) -> &Hash {
+        self.hash
+    }
+
+    pub fn stored_size(&self) -> usize {
+        self.stored_size
+    }
+
+    pub fn offset(&self) -> usize {
+        self.offset
+    }
+
+    pub fn data(&self) -> &[u8] {
+        self.data
+    }
+
+    pub fn data_len(&self) -> u64 {
+        self.meta.data_len
+    }
+
+    pub fn write_version(&self) -> StoredMetaWriteVersion {
+        self.meta.write_version_obsolete
+    }
+
     pub(crate) fn sanitize(&self) -> bool {
         self.sanitize_executable() && self.sanitize_lamports()
     }
@@ -147,6 +171,24 @@ impl<'a> StoredAccountMeta<'a> {
     }
 }
 
+impl<'a> ReadableAccount for StoredAccountMeta<'a> {
+    fn lamports(&self) -> u64 {
+        self.account_meta.lamports
+    }
+    fn data(&self) -> &[u8] {
+        self.data()
+    }
+    fn owner(&self) -> &Pubkey {
+        &self.account_meta.owner
+    }
+    fn executable(&self) -> bool {
+        self.account_meta.executable
+    }
+    fn rent_epoch(&self) -> Epoch {
+        self.account_meta.rent_epoch
+    }
+}
+
 /// Meta contains enough context to recover the index from storage itself
 /// This struct will be backed by mmaped and snapshotted data files.
 /// So the data layout must be stable and consistent across the entire cluster!
diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs
index 5baecc2235c2e6..66149f3194c739 100644
--- a/runtime/src/accounts_db.rs
+++ b/runtime/src/accounts_db.rs
@@ -237,7 +237,7 @@ impl<'a> ShrinkCollectRefs<'a> for AliveAccounts<'a> {
     }
     fn add(&mut self, _ref_count: u64, account: &'a StoredAccountMeta<'a>) {
         self.accounts.push(account);
-        self.bytes = self.bytes.saturating_add(account.stored_size);
+        self.bytes = self.bytes.saturating_add(account.stored_size());
     }
     fn len(&self) -> usize {
         self.accounts.len()
@@ -882,7 +882,7 @@ pub enum LoadedAccount<'a> {
 impl<'a> LoadedAccount<'a> {
     pub fn loaded_hash(&self) -> Hash {
         match self {
-            LoadedAccount::Stored(stored_account_meta) => *stored_account_meta.hash,
+            LoadedAccount::Stored(stored_account_meta) => *stored_account_meta.hash(),
             LoadedAccount::Cached(cached_account) => cached_account.hash(),
         }
     }
@@ -934,36 +934,32 @@ impl<'a> LoadedAccount<'a> {
 impl<'a> ReadableAccount for LoadedAccount<'a> {
     fn lamports(&self) -> u64 {
         match self {
-            LoadedAccount::Stored(stored_account_meta) => stored_account_meta.account_meta.lamports,
+            LoadedAccount::Stored(stored_account_meta) => stored_account_meta.lamports(),
             LoadedAccount::Cached(cached_account) => cached_account.account.lamports(),
         }
     }
 
     fn data(&self) -> &[u8] {
         match self {
-            LoadedAccount::Stored(stored_account_meta) => stored_account_meta.data,
+            LoadedAccount::Stored(stored_account_meta) => stored_account_meta.data(),
             LoadedAccount::Cached(cached_account) => cached_account.account.data(),
         }
     }
     fn owner(&self) -> &Pubkey {
         match self {
-            LoadedAccount::Stored(stored_account_meta) => &stored_account_meta.account_meta.owner,
+            LoadedAccount::Stored(stored_account_meta) => stored_account_meta.owner(),
             LoadedAccount::Cached(cached_account) => cached_account.account.owner(),
         }
     }
     fn executable(&self) -> bool {
         match self {
-            LoadedAccount::Stored(stored_account_meta) => {
-                stored_account_meta.account_meta.executable
-            }
+            LoadedAccount::Stored(stored_account_meta) => stored_account_meta.executable(),
             LoadedAccount::Cached(cached_account) => cached_account.account.executable(),
         }
     }
     fn rent_epoch(&self) -> Epoch {
         match self {
-            LoadedAccount::Stored(stored_account_meta) => {
-                stored_account_meta.account_meta.rent_epoch
-            }
+            LoadedAccount::Stored(stored_account_meta) => stored_account_meta.rent_epoch(),
             LoadedAccount::Cached(cached_account) => cached_account.account.rent_epoch(),
         }
     }
@@ -2253,24 +2249,6 @@ impl<'a> ZeroLamport for StoredAccountMeta<'a> {
     }
 }
 
-impl<'a> ReadableAccount for StoredAccountMeta<'a> {
-    fn lamports(&self) -> u64 {
-        self.account_meta.lamports
-    }
-    fn data(&self) -> &[u8] {
-        self.data
-    }
-    fn owner(&self) -> &Pubkey {
-        &self.account_meta.owner
-    }
-    fn executable(&self) -> bool {
-        self.account_meta.executable
-    }
-    fn rent_epoch(&self) -> Epoch {
-        self.account_meta.rent_epoch
-    }
-}
-
 struct IndexAccountMapEntry<'a> {
     pub write_version: StoredMetaWriteVersion,
     pub store_id: AppendVecId,
@@ -4330,7 +4308,7 @@ impl AccountsDb {
                 storage
                     .accounts
                     .account_iter()
-                    .map(|account| account.stored_size)
+                    .map(|account| account.stored_size())
                     .collect()
             })
             .unwrap_or_default()
@@ -8599,7 +8577,7 @@ impl AccountsDb {
         let num_accounts = storage.approx_stored_count();
         let mut accounts_map = GenerateIndexAccountsMap::with_capacity(num_accounts);
         storage.accounts.account_iter().for_each(|stored_account| {
-            let this_version = stored_account.meta.write_version_obsolete;
+            let this_version = stored_account.write_version();
             let pubkey = stored_account.pubkey();
             assert!(!self.is_filler_account(pubkey));
             accounts_map.insert(
@@ -8681,8 +8659,8 @@ impl AccountsDb {
                 (
                     pubkey,
                     AccountInfo::new(
-                        StorageLocation::AppendVec(store_id, stored_account.offset), // will never be cached
-                        stored_account.account_meta.lamports,
+                        StorageLocation::AppendVec(store_id, stored_account.offset()), // will never be cached
+                        stored_account.lamports(),
                     ),
                 )
             },
@@ -8914,9 +8892,9 @@ impl AccountsDb {
                                         let ai = AccountInfo::new(
                                             StorageLocation::AppendVec(
                                                 account_info.store_id,
-                                                account_info.stored_account.offset,
+                                                account_info.stored_account.offset(),
                                             ), // will never be cached
-                                            account_info.stored_account.account_meta.lamports,
+                                            account_info.stored_account.lamports(),
                                         );
                                         assert_eq!(&ai, account_info2);
                                     }
@@ -9148,7 +9126,7 @@ impl AccountsDb {
             let mut info = storage_info_local
                 .entry(v.store_id)
                 .or_insert_with(StorageSizeAndCount::default);
-            info.stored_size += v.stored_account.stored_size;
+            info.stored_size += v.stored_account.stored_size();
             info.count += 1;
         }
         storage_size_accounts_map_time.stop();
@@ -9631,7 +9609,7 @@ pub mod tests {
             hash: &hash,
         };
         let map = vec![&account];
-        let alive_total_bytes = account.stored_size;
+        let alive_total_bytes = account.stored_size();
         let to_store = AccountsToStore::new(available_bytes, &map, alive_total_bytes, slot0);
         // Done: setup 'to_store'
 
@@ -14591,7 +14569,7 @@ pub mod tests {
             let reclaims = vec![account_info];
             accounts_db.remove_dead_accounts(reclaims.iter(), None, None, true);
             let after_size = storage0.alive_bytes.load(Ordering::Acquire);
-            assert_eq!(before_size, after_size + account.stored_size);
+            assert_eq!(before_size, after_size + account.stored_size());
         }
     }
 
@@ -17592,7 +17570,7 @@ pub mod tests {
             if let Some(storage) = db.get_storage_for_slot(slot) {
                 storage.accounts.account_iter().for_each(|account| {
                     let info = AccountInfo::new(
-                        StorageLocation::AppendVec(storage.append_vec_id(), account.offset),
+                        StorageLocation::AppendVec(storage.append_vec_id(), account.offset()),
                         account.lamports(),
                     );
                     db.accounts_index.upsert(
diff --git a/runtime/src/accounts_db/geyser_plugin_utils.rs b/runtime/src/accounts_db/geyser_plugin_utils.rs
index 9f0a79a920a92c..5bcafa8f1590a2 100644
--- a/runtime/src/accounts_db/geyser_plugin_utils.rs
+++ b/runtime/src/accounts_db/geyser_plugin_utils.rs
@@ -97,7 +97,7 @@ impl AccountsDb {
         let mut account_len = 0;
         accounts.for_each(|account| {
             account_len += 1;
-            if notified_accounts.contains(&account.meta.pubkey) {
+            if notified_accounts.contains(account.pubkey()) {
                 notify_stats.skipped_accounts += 1;
                 return;
             }
@@ -105,7 +105,7 @@ impl AccountsDb {
             // later entries in the same slot are more recent and override earlier accounts for the same pubkey
             // We can pass an incrementing number here for write_version in the future, if the storage does not have a write_version.
             // As long as all accounts for this slot are in 1 append vec that can be itereated olest to newest.
-            accounts_to_stream.insert(account.meta.pubkey, account);
+            accounts_to_stream.insert(*account.pubkey(), account);
         });
         notify_stats.total_accounts += account_len;
         measure_filter.stop();
@@ -148,7 +148,7 @@ impl AccountsDb {
             notify_stats.total_pure_notify += measure_pure_notify.as_us() as usize;
 
             let mut measure_bookkeep = Measure::start("accountsdb-plugin-notifying-bookeeeping");
-            notified_accounts.insert(account.meta.pubkey);
+            notified_accounts.insert(*account.pubkey());
             measure_bookkeep.stop();
             notify_stats.total_pure_bookeeping += measure_bookkeep.as_us() as usize;
         }
@@ -213,7 +213,7 @@ pub mod tests {
         /// from a snapshot.
         fn notify_account_restore_from_snapshot(&self, slot: Slot, account: &StoredAccountMeta) {
             self.accounts_notified
-                .entry(account.meta.pubkey)
+                .entry(*account.pubkey())
                 .or_default()
                 .push((slot, account.clone_account()));
         }
diff --git a/runtime/src/ancient_append_vecs.rs b/runtime/src/ancient_append_vecs.rs
index 46caf8c974ee33..4d3cd902f84497 100644
--- a/runtime/src/ancient_append_vecs.rs
+++ b/runtime/src/ancient_append_vecs.rs
@@ -706,7 +706,7 @@ impl<'a> AccountsToStore<'a> {
         if alive_total_bytes > available_bytes as usize {
             // not all the alive bytes fit, so we have to find how many accounts fit within available_bytes
             for (i, account) in accounts.iter().enumerate() {
-                let account_size = account.stored_size as u64;
+                let account_size = account.stored_size() as u64;
                 if available_bytes >= account_size {
                     available_bytes = available_bytes.saturating_sub(account_size);
                 } else if index_first_item_overflow == num_accounts {
diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs
index 9d5256cc9f6e7a..3b0444e15a9ffd 100644
--- a/runtime/src/append_vec.rs
+++ b/runtime/src/append_vec.rs
@@ -594,7 +594,7 @@ pub mod tests {
         }
 
         fn get_executable_byte(&self) -> u8 {
-            let executable_bool: bool = self.account_meta.executable;
+            let executable_bool: bool = self.executable();
             // UNSAFE: Force to interpret mmap-backed bool as u8 to really read the actual memory content
             let executable_byte: u8 = unsafe { std::mem::transmute::<bool, u8>(executable_bool) };
             executable_byte
@@ -1065,12 +1065,12 @@ pub mod tests {
         let accounts = av.accounts(0);
         let account = accounts.first().unwrap();
         account.set_data_len_unsafe(crafted_data_len);
-        assert_eq!(account.meta.data_len, crafted_data_len);
+        assert_eq!(account.data_len(), crafted_data_len);
 
         // Reload accounts and observe crafted_data_len
         let accounts = av.accounts(0);
         let account = accounts.first().unwrap();
-        assert_eq!(account.meta.data_len, crafted_data_len);
+        assert_eq!(account.data_len(), crafted_data_len);
 
         av.flush().unwrap();
         let accounts_len = av.len();
@@ -1092,7 +1092,7 @@ pub mod tests {
         let accounts = av.accounts(0);
         let account = accounts.first().unwrap();
         account.set_data_len_unsafe(too_large_data_len);
-        assert_eq!(account.meta.data_len, too_large_data_len);
+        assert_eq!(account.data_len(), too_large_data_len);
 
         // Reload accounts and observe no account with bad offset
         let accounts = av.accounts(0);
@@ -1155,7 +1155,7 @@ pub mod tests {
 
         // we can NOT observe crafted value by value
         {
-            let executable_bool: bool = account.account_meta.executable;
+            let executable_bool: bool = account.executable();
             assert!(!executable_bool);
             assert_eq!(account.get_executable_byte(), 0); // Wow, not crafted_executable!
         }
diff --git a/runtime/src/snapshot_minimizer.rs b/runtime/src/snapshot_minimizer.rs
index b30160cae8cb2a..37e2520a73032e 100644
--- a/runtime/src/snapshot_minimizer.rs
+++ b/runtime/src/snapshot_minimizer.rs
@@ -319,7 +319,7 @@ impl<'a> SnapshotMinimizer<'a> {
             let mut purge_pubkeys = Vec::with_capacity(CHUNK_SIZE);
             chunk.iter().for_each(|account| {
                 if self.minimized_account_set.contains(account.pubkey()) {
-                    chunk_bytes += account.stored_size;
+                    chunk_bytes += account.stored_size();
                     keep_accounts.push(account);
                 } else if self
                     .accounts_db()
@@ -361,8 +361,8 @@ impl<'a> SnapshotMinimizer<'a> {
 
             for alive_account in keep_accounts {
                 accounts.push(alive_account);
-                hashes.push(alive_account.hash);
-                write_versions.push(alive_account.meta.write_version_obsolete);
+                hashes.push(alive_account.hash());
+                write_versions.push(alive_account.write_version());
             }
 
             shrink_in_progress = Some(self.accounts_db().get_store_for_shrink(slot, aligned_total));
diff --git a/runtime/src/storable_accounts.rs b/runtime/src/storable_accounts.rs
index 6348a83e759034..a6b335a203f6f8 100644
--- a/runtime/src/storable_accounts.rs
+++ b/runtime/src/storable_accounts.rs
@@ -148,10 +148,10 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>>
         true
     }
     fn hash(&self, index: usize) -> &Hash {
-        self.account(index).hash
+        self.account(index).hash()
     }
     fn write_version(&self, index: usize) -> u64 {
-        self.account(index).meta.write_version_obsolete
+        self.account(index).write_version()
     }
 }
 
@@ -252,10 +252,10 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>> for StorableAccountsBySlot<
         true
     }
     fn hash(&self, index: usize) -> &Hash {
-        self.account(index).hash
+        self.account(index).hash()
     }
     fn write_version(&self, index: usize) -> u64 {
-        self.account(index).meta.write_version_obsolete
+        self.account(index).write_version()
     }
 }
 
@@ -292,10 +292,10 @@ impl<'a> StorableAccounts<'a, StoredAccountMeta<'a>>
         true
     }
     fn hash(&self, index: usize) -> &Hash {
-        self.account(index).hash
+        self.account(index).hash()
     }
     fn write_version(&self, index: usize) -> u64 {
-        self.account(index).meta.write_version_obsolete
+        self.account(index).write_version()
     }
 }
 
@@ -556,12 +556,9 @@ pub mod tests {
                             let index = index as usize;
                             assert_eq!(storable.account(index), &raw2[index]);
                             assert_eq!(storable.pubkey(index), raw2[index].pubkey());
-                            assert_eq!(storable.hash(index), raw2[index].hash);
+                            assert_eq!(storable.hash(index), raw2[index].hash());
                             assert_eq!(storable.slot(index), expected_slots[index]);
-                            assert_eq!(
-                                storable.write_version(index),
-                                raw2[index].meta.write_version_obsolete
-                            );
+                            assert_eq!(storable.write_version(index), raw2[index].write_version());
                         })
                     }
                 }
diff --git a/runtime/store-tool/src/main.rs b/runtime/store-tool/src/main.rs
index 0fe6a84b5d15d6..a8879a690d72ed 100644
--- a/runtime/store-tool/src/main.rs
+++ b/runtime/store-tool/src/main.rs
@@ -2,7 +2,11 @@ use {
     clap::{crate_description, crate_name, value_t, value_t_or_exit, App, Arg},
     log::*,
     solana_runtime::{account_storage::meta::StoredAccountMeta, append_vec::AppendVec},
-    solana_sdk::{account::AccountSharedData, hash::Hash, pubkey::Pubkey},
+    solana_sdk::{
+        account::{AccountSharedData, ReadableAccount},
+        hash::Hash,
+        pubkey::Pubkey,
+    },
 };
 
 fn main() {
@@ -42,13 +46,13 @@ fn main() {
         info!(
             "  account: {:?} version: {} lamports: {} data: {} hash: {:?}",
             account.pubkey(),
-            account.meta.write_version_obsolete,
-            account.account_meta.lamports,
-            account.meta.data_len,
-            account.hash
+            account.write_version(),
+            account.lamports(),
+            account.data_len(),
+            account.hash()
         );
         num_accounts = num_accounts.saturating_add(1);
-        stored_accounts_len = stored_accounts_len.saturating_add(account.stored_size);
+        stored_accounts_len = stored_accounts_len.saturating_add(account.stored_size());
     }
     info!(
         "num_accounts: {} stored_accounts_len: {}",
@@ -57,9 +61,9 @@ fn main() {
 }
 
 fn is_account_zeroed(account: &StoredAccountMeta) -> bool {
-    account.hash == &Hash::default()
-        && account.meta.data_len == 0
-        && account.meta.write_version_obsolete == 0
+    account.hash() == &Hash::default()
+        && account.data_len() == 0
+        && account.write_version() == 0
         && account.pubkey() == &Pubkey::default()
         && account.clone_account() == AccountSharedData::default()
 }