Include rent_epoch and executable into account hash (#7415)

* Clean a bit

* Add rent_epoch and executable into account hashing

* Remove comment and instead create an issue

runtime/src/accounts_db.rs

@@ -34,7 +34,7 @@ use solana_measure::measure::Measure;
 use solana_rayon_threadlimit::get_thread_count;
 use solana_sdk::account::Account;
 use solana_sdk::bank_hash::BankHash;
-use solana_sdk::clock::Slot;
+use solana_sdk::clock::{Epoch, Slot};
 use solana_sdk::hash::{Hash, Hasher};
 use solana_sdk::pubkey::Pubkey;
 use solana_sdk::sysvar;
@@ -820,16 +820,32 @@ impl AccountsDB {
         Self::hash_account_data(
             slot,
             account.account_meta.lamports,
+            account.account_meta.executable,
+            account.account_meta.rent_epoch,
             account.data,
             &account.meta.pubkey,
         )
     }
 
     pub fn hash_account(slot: Slot, account: &Account, pubkey: &Pubkey) -> Hash {
-        Self::hash_account_data(slot, account.lamports, &account.data, pubkey)
+        Self::hash_account_data(
+            slot,
+            account.lamports,
+            account.executable,
+            account.rent_epoch,
+            &account.data,
+            pubkey,
+        )
     }
 
-    pub fn hash_account_data(slot: Slot, lamports: u64, data: &[u8], pubkey: &Pubkey) -> Hash {
+    pub fn hash_account_data(
+        slot: Slot,
+        lamports: u64,
+        executable: bool,
+        rent_epoch: Epoch,
+        data: &[u8],
+        pubkey: &Pubkey,
+    ) -> Hash {
         if lamports == 0 {
             return Hash::default();
         }
@@ -843,8 +859,17 @@ impl AccountsDB {
         LittleEndian::write_u64(&mut buf[..], slot);
         hasher.hash(&buf);
 
+        LittleEndian::write_u64(&mut buf[..], rent_epoch);
+        hasher.hash(&buf);
+
         hasher.hash(&data);
 
+        if executable {
+            hasher.hash(&[1u8; 1]);
+        } else {
+            hasher.hash(&[0u8; 1]);
+        }
+
         hasher.hash(&pubkey.as_ref());
 
         hasher.result()
@@ -1201,10 +1226,12 @@ impl AccountsDB {
 pub mod tests {
     // TODO: all the bank tests are bank specific, issue: 2194
     use super::*;
+    use crate::append_vec::AccountMeta;
     use bincode::serialize_into;
     use rand::{thread_rng, Rng};
     use solana_sdk::account::Account;
     use std::fs;
+    use std::str::FromStr;
     use tempfile::TempDir;
 
     #[test]
@@ -2109,4 +2136,56 @@ pub mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn test_hash_stored_account() {
+        // This test uses some UNSAFE trick to detect most of account's field
+        // addition and deletion without changing the hash code
+
+        const ACCOUNT_DATA_LEN: usize = 3;
+        // the type of InputTuple elements must not contain references;
+        // they should be simple scalars or data blobs
+        type InputTuple = (
+            Slot,
+            StoredMeta,
+            AccountMeta,
+            [u8; ACCOUNT_DATA_LEN],
+            usize, // for StoredAccount::offset
+            Hash,
+        );
+        const INPUT_LEN: usize = std::mem::size_of::<InputTuple>();
+        type InputBlob = [u8; INPUT_LEN];
+        let mut blob: InputBlob = [0u8; INPUT_LEN];
+
+        // spray memory with decreasing counts so that, data layout can be detected.
+        for (i, byte) in blob.iter_mut().enumerate() {
+            *byte = (INPUT_LEN - i) as u8;
+        }
+
+        //UNSAFE: forcibly cast the special byte pattern to actual account fields.
+        let (slot, meta, account_meta, data, offset, hash): InputTuple =
+            unsafe { std::mem::transmute::<InputBlob, InputTuple>(blob) };
+
+        let stored_account = StoredAccount {
+            meta: &meta,
+            account_meta: &account_meta,
+            data: &data,
+            offset,
+            hash: &hash,
+        };
+        let account = stored_account.clone_account();
+        let expected_account_hash =
+            Hash::from_str("GGTsxvxwnMsNfN6yYbBVQaRgvbVLfxeWnGXNyB8iXDyE").unwrap();
+
+        assert_eq!(
+            AccountsDB::hash_stored_account(slot, &stored_account),
+            expected_account_hash,
+            "StoredAccount's data layout might be changed; update hashing if needed."
+        );
+        assert_eq!(
+            AccountsDB::hash_account(slot, &account, &stored_account.meta.pubkey),
+            expected_account_hash,
+            "Account-based hashing must be consistent with StoredAccount-based one."
+        );
+    }
 }

runtime/src/append_vec.rs

@@ -22,6 +22,8 @@ macro_rules! align_up {
 }
 
 /// 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!
 #[derive(Clone, PartialEq, Debug)]
 pub struct StoredMeta {
     /// global write version
@@ -31,6 +33,8 @@ pub struct StoredMeta {
     pub data_len: u64,
 }
 
+/// This struct will be backed by mmaped and snapshotted data files.
+/// So the data layout must be stable and consistent across the entire cluster!
 #[derive(Serialize, Deserialize, Clone, Debug, Default, Eq, PartialEq)]
 pub struct AccountMeta {
     /// lamports in the account
@@ -187,9 +191,7 @@ impl AppendVec {
     }
 
     fn get_slice(&self, offset: usize, size: usize) -> Option<(&[u8], usize)> {
-        let len = self.len();
-
-        if len < offset + size {
+        if offset + size > self.len() {
             return None;
         }
         let data = &self.map[offset..offset + size];