From 65681ef2b8e8a71bcef92f4152f4905414931c6f Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 10 Dec 2019 15:40:12 +0900 Subject: [PATCH 1/3] Clean a bit --- runtime/src/append_vec.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index fa26e60363f198..efa9bd6f75018b 100644 --- a/runtime/src/append_vec.rs +++ b/runtime/src/append_vec.rs @@ -187,9 +187,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]; From ff504c33322dcc61953a32968da7bf4e65d4f3a8 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Wed, 11 Dec 2019 04:30:03 +0900 Subject: [PATCH 2/3] Add rent_epoch and executable into account hashing --- runtime/src/accounts_db.rs | 87 ++++++++++++++++++++++++++++++++++++-- runtime/src/append_vec.rs | 4 ++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 5c5a79c6e67147..ad98405655a049 100644 --- a/runtime/src/accounts_db.rs +++ b/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; @@ -191,6 +191,8 @@ pub struct AccountStorageEntry { /// any accounts in it /// status corresponding to the storage, lets us know that /// the append_vec, once maxed out, then emptied, can be reclaimed + /// Currently serialized count isn't used at all. Rather, we recalculate it + /// when deserializing. count_and_status: RwLock<(usize, AccountStorageStatus)>, } @@ -808,16 +810,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(); } @@ -831,8 +849,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() @@ -1189,10 +1216,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] @@ -2103,4 +2132,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::(); + 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::(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." + ); + } } diff --git a/runtime/src/append_vec.rs b/runtime/src/append_vec.rs index efa9bd6f75018b..703f530a0a7a94 100644 --- a/runtime/src/append_vec.rs +++ b/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 From ae6e54ec801168e706de7c3fd981ee1647b823ce Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Thu, 12 Dec 2019 12:08:08 +0900 Subject: [PATCH 3/3] Remove comment and instead create an issue --- runtime/src/accounts_db.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index ad98405655a049..fe8a18429673c3 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -191,8 +191,6 @@ pub struct AccountStorageEntry { /// any accounts in it /// status corresponding to the storage, lets us know that /// the append_vec, once maxed out, then emptied, can be reclaimed - /// Currently serialized count isn't used at all. Rather, we recalculate it - /// when deserializing. count_and_status: RwLock<(usize, AccountStorageStatus)>, }