diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs index eab2b5ef80..5ed513e7fc 100644 --- a/runtime/benches/accounts.rs +++ b/runtime/benches/accounts.rs @@ -59,12 +59,12 @@ fn test_accounts_squash(bencher: &mut Bencher) { } #[bench] -fn test_accounts_hash_internal_state(bencher: &mut Bencher) { +fn test_accounts_hash_bank_hash(bencher: &mut Bencher) { let accounts = Accounts::new(vec![PathBuf::from("bench_accounts_hash_internal")]); let mut pubkeys: Vec = vec![]; create_test_accounts(&accounts, &mut pubkeys, 60000, 0); let ancestors = vec![(0, 0)].into_iter().collect(); bencher.iter(|| { - accounts.verify_hash_internal_state(0, &ancestors); + accounts.verify_bank_hash(0, &ancestors); }); } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index d3075e5dff..71157b58c8 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -345,8 +345,8 @@ impl Accounts { }) } - pub fn verify_hash_internal_state(&self, slot: Slot, ancestors: &HashMap) -> bool { - self.accounts_db.verify_hash_internal_state(slot, ancestors) + pub fn verify_bank_hash(&self, slot: Slot, ancestors: &HashMap) -> bool { + self.accounts_db.verify_bank_hash(slot, ancestors).is_ok() } pub fn load_by_program( @@ -476,7 +476,7 @@ impl Accounts { } } - pub fn hash_internal_state(&self, slot_id: Slot) -> BankHash { + pub fn bank_hash_at(&self, slot_id: Slot) -> BankHash { let slot_hashes = self.accounts_db.slot_hashes.read().unwrap(); *slot_hashes .get(&slot_id) @@ -1164,17 +1164,17 @@ mod tests { #[test] #[should_panic] - fn test_accounts_empty_hash_internal_state() { + fn test_accounts_empty_bank_hash() { let accounts = Accounts::new(Vec::new()); - accounts.hash_internal_state(0); + accounts.bank_hash_at(0); } #[test] #[should_panic] - fn test_accounts_empty_account_hash_internal_state() { + fn test_accounts_empty_account_bank_hash() { let accounts = Accounts::new(Vec::new()); accounts.store_slow(0, &Pubkey::default(), &Account::new(1, 0, &sysvar::id())); - accounts.hash_internal_state(0); + accounts.bank_hash_at(0); } fn check_accounts(accounts: &Accounts, pubkeys: &Vec, num: usize) { @@ -1221,10 +1221,7 @@ mod tests { .accounts_from_stream(&mut reader, &daccounts_paths, copied_accounts.path()) .is_ok()); check_accounts(&daccounts, &pubkeys, 100); - assert_eq!( - accounts.hash_internal_state(0), - daccounts.hash_internal_state(0) - ); + assert_eq!(accounts.bank_hash_at(0), daccounts.bank_hash_at(0)); } #[test] diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 502f4d7fcb..abbc952822 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -175,6 +175,13 @@ pub enum AccountStorageStatus { Candidate = 2, } +#[derive(Debug)] +pub enum BankHashVerificatonError { + MismatchedAccountHash, + MismatchedBankHash, + MissingBankHash, +} + /// Persistent storage structure holding the accounts #[derive(Debug, Serialize, Deserialize)] pub struct AccountStorageEntry { @@ -354,13 +361,11 @@ pub struct AccountsDB { /// Keeps tracks of index into AppendVec on a per slot basis pub accounts_index: RwLock>, - /// Account storage pub storage: RwLock, /// distribute the accounts across storage lists pub next_id: AtomicUsize, - /// write version write_version: AtomicUsize, /// Set of storage paths to pick from @@ -379,7 +384,6 @@ pub struct AccountsDB { /// the accounts min_num_stores: usize, - /// slot to BankHash and a status flag to indicate if the hash has been initialized or not pub slot_hashes: RwLock>, } @@ -969,28 +973,49 @@ impl AccountsDB { datapoint_info!("accounts_db-stores", ("total_count", total_count, i64)); } - pub fn verify_hash_internal_state(&self, slot: Slot, ancestors: &HashMap) -> bool { - let mut hash_state = BankHash::default(); - let hashes: Vec<_> = self.scan_accounts( + pub fn verify_bank_hash( + &self, + slot: Slot, + ancestors: &HashMap, + ) -> Result<(), BankHashVerificatonError> { + use BankHashVerificatonError::*; + + let (hashes, mismatch_found) = self.scan_accounts( ancestors, - |collector: &mut Vec, option: Option<(&Pubkey, Account, Slot)>| { + |(collector, mismatch_found): &mut (Vec, bool), + option: Option<(&Pubkey, Account, Slot)>| { if let Some((pubkey, account, slot)) = option { if !sysvar::check_id(&account.owner) { - let hash = BankHash::from_hash(&Self::hash_account(slot, &account, pubkey)); + let hash = Self::hash_account(slot, &account, pubkey); + if hash != account.hash { + *mismatch_found = true; + } + if *mismatch_found { + return; + } + let hash = BankHash::from_hash(&hash); debug!("xoring..{} key: {}", hash, pubkey); collector.push(hash); } } }, ); + if mismatch_found { + return Err(MismatchedAccountHash); + } + let mut calculated_hash = BankHash::default(); for hash in hashes { - hash_state.xor(hash); + calculated_hash.xor(hash); } let slot_hashes = self.slot_hashes.read().unwrap(); - if let Some(state) = slot_hashes.get(&slot) { - hash_state == *state + if let Some(found_hash) = slot_hashes.get(&slot) { + if calculated_hash == *found_hash { + Ok(()) + } else { + Err(MismatchedBankHash) + } } else { - false + Err(BankHashVerificatonError::MissingBankHash) } } @@ -1120,9 +1145,12 @@ impl AccountsDB { /// Store the account update. pub fn store(&self, slot_id: Slot, accounts: &[(&Pubkey, &Account)]) { let hashes = self.hash_accounts(slot_id, accounts); + self.store_with_hashes(slot_id, accounts, &hashes); + } + fn store_with_hashes(&self, slot_id: Slot, accounts: &[(&Pubkey, &Account)], hashes: &[Hash]) { let mut store_accounts = Measure::start("store::store_accounts"); - let infos = self.store_accounts(slot_id, accounts, &hashes); + let infos = self.store_accounts(slot_id, accounts, hashes); store_accounts.stop(); let mut update_index = Measure::start("store::update_index"); @@ -1234,9 +1262,11 @@ pub mod tests { // TODO: all the bank tests are bank specific, issue: 2194 use super::*; use crate::append_vec::AccountMeta; + use assert_matches::assert_matches; use bincode::serialize_into; use rand::{thread_rng, Rng}; use solana_sdk::account::Account; + use solana_sdk::hash::HASH_BYTES; use std::fs; use std::str::FromStr; use tempfile::TempDir; @@ -2195,4 +2225,78 @@ pub mod tests { "Account-based hashing must be consistent with StoredAccount-based one." ); } + + #[test] + fn test_verify_bank_hash() { + use BankHashVerificatonError::*; + solana_logger::setup(); + let db = AccountsDB::new(Vec::new()); + + let key = Pubkey::default(); + let some_data_len = 0; + let some_slot: Slot = 0; + let account = Account::new(1, some_data_len, &key); + let ancestors = vec![(some_slot, 0)].into_iter().collect(); + + db.store(some_slot, &[(&key, &account)]); + db.add_root(some_slot); + assert_matches!(db.verify_bank_hash(some_slot, &ancestors), Ok(_)); + + db.slot_hashes.write().unwrap().remove(&some_slot).unwrap(); + assert_matches!( + db.verify_bank_hash(some_slot, &ancestors), + Err(MissingBankHash) + ); + + let some_bank_hash = BankHash::from_hash(&Hash::new(&[0xca; HASH_BYTES])); + db.slot_hashes + .write() + .unwrap() + .insert(some_slot, some_bank_hash); + assert_matches!( + db.verify_bank_hash(some_slot, &ancestors), + Err(MismatchedBankHash) + ); + } + + #[test] + fn test_verify_bank_hash_no_account() { + solana_logger::setup(); + let db = AccountsDB::new(Vec::new()); + + let some_slot: Slot = 0; + let ancestors = vec![(some_slot, 0)].into_iter().collect(); + + db.slot_hashes + .write() + .unwrap() + .insert(some_slot, BankHash::default()); + db.add_root(some_slot); + assert_matches!(db.verify_bank_hash(some_slot, &ancestors), Ok(_)); + } + + #[test] + fn test_verify_bank_hash_bad_account_hash() { + use BankHashVerificatonError::*; + solana_logger::setup(); + let db = AccountsDB::new(Vec::new()); + + let key = Pubkey::default(); + let some_data_len = 0; + let some_slot: Slot = 0; + let account = Account::new(1, some_data_len, &key); + let ancestors = vec![(some_slot, 0)].into_iter().collect(); + + let accounts = &[(&key, &account)]; + // update AccountsDB's bank hash but discard real account hashes + db.hash_accounts(some_slot, accounts); + // provide bogus account hashes + let some_hash = Hash::new(&[0xca; HASH_BYTES]); + db.store_with_hashes(some_slot, accounts, &vec![some_hash]); + db.add_root(some_slot); + assert_matches!( + db.verify_bank_hash(some_slot, &ancestors), + Err(MismatchedAccountHash) + ); + } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 882e66a9ba..d5f8e90d9a 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -1595,7 +1595,7 @@ impl Bank { /// of the delta of the ledger since the last vote and up to now fn hash_internal_state(&self) -> Hash { // If there are no accounts, return the hash of the previous state and the latest blockhash - let accounts_delta_hash = self.rc.accounts.hash_internal_state(self.slot()); + let accounts_delta_hash = self.rc.accounts.bank_hash_at(self.slot()); let mut signature_count_buf = [0u8; 8]; LittleEndian::write_u64(&mut signature_count_buf[..], self.signature_count() as u64); hashv(&[ @@ -1611,7 +1611,7 @@ impl Bank { pub fn verify_hash_internal_state(&self) -> bool { self.rc .accounts - .verify_hash_internal_state(self.slot(), &self.ancestors) + .verify_bank_hash(self.slot(), &self.ancestors) } /// A snapshot bank should be purged of 0 lamport accounts which are not part of the hash @@ -1620,7 +1620,7 @@ impl Bank { self.purge_zero_lamport_accounts(); self.rc .accounts - .verify_hash_internal_state(self.slot(), &self.ancestors) + .verify_bank_hash(self.slot(), &self.ancestors) } /// Return the number of hashes per tick @@ -1806,8 +1806,8 @@ impl Bank { let dsc = dbank.src.status_cache.read().unwrap(); assert_eq!(*sc, *dsc); assert_eq!( - self.rc.accounts.hash_internal_state(self.slot), - dbank.rc.accounts.hash_internal_state(dbank.slot) + self.rc.accounts.bank_hash_at(self.slot), + dbank.rc.accounts.bank_hash_at(dbank.slot) ); } diff --git a/sdk/src/hash.rs b/sdk/src/hash.rs index 4ab26c851e..e1a86e5253 100644 --- a/sdk/src/hash.rs +++ b/sdk/src/hash.rs @@ -7,9 +7,10 @@ use std::fmt; use std::mem; use std::str::FromStr; +pub const HASH_BYTES: usize = 32; #[derive(Serialize, Deserialize, Clone, Copy, Default, Eq, PartialEq, Ord, PartialOrd, Hash)] #[repr(transparent)] -pub struct Hash([u8; 32]); +pub struct Hash([u8; HASH_BYTES]); #[derive(Clone, Default)] pub struct Hasher { @@ -28,7 +29,7 @@ impl Hasher { pub fn result(self) -> Hash { // At the time of this writing, the sha2 library is stuck on an old version // of generic_array (0.9.0). Decouple ourselves with a clone to our version. - Hash(<[u8; 32]>::try_from(self.hasher.result().as_slice()).unwrap()) + Hash(<[u8; HASH_BYTES]>::try_from(self.hasher.result().as_slice()).unwrap()) } } @@ -73,7 +74,7 @@ impl FromStr for Hash { impl Hash { pub fn new(hash_slice: &[u8]) -> Self { - Hash(<[u8; 32]>::try_from(hash_slice).unwrap()) + Hash(<[u8; HASH_BYTES]>::try_from(hash_slice).unwrap()) } }