diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index fcd273af5f..df1c73721e 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -4,7 +4,7 @@ use { account_rent_state::{check_rent_state_with_account, RentState}, accounts_db::{ AccountShrinkThreshold, AccountsAddRootTiming, AccountsDb, AccountsDbConfig, - BankHashInfo, LoadHint, LoadedAccount, ScanStorageResult, + BankHashInfo, IncludeSlotInHash, LoadHint, LoadedAccount, ScanStorageResult, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING, }, accounts_index::{ @@ -1201,6 +1201,7 @@ impl Accounts { durable_nonce: &DurableNonce, lamports_per_signature: u64, preserve_rent_epoch_for_rent_exempt_accounts: bool, + include_slot_in_hash: IncludeSlotInHash, ) { let (accounts_to_store, txn_signatures) = self.collect_accounts_to_store( txs, @@ -1211,8 +1212,10 @@ impl Accounts { lamports_per_signature, preserve_rent_epoch_for_rent_exempt_accounts, ); - self.accounts_db - .store_cached((slot, &accounts_to_store[..]), Some(&txn_signatures)); + self.accounts_db.store_cached( + (slot, &accounts_to_store[..], include_slot_in_hash), + Some(&txn_signatures), + ); } pub fn store_accounts_cached<'a, T: ReadableAccount + Sync + ZeroLamport>( diff --git a/runtime/src/accounts_cache.rs b/runtime/src/accounts_cache.rs index 053764ef7a..d7ecb6d42d 100644 --- a/runtime/src/accounts_cache.rs +++ b/runtime/src/accounts_cache.rs @@ -1,4 +1,5 @@ use { + crate::accounts_db::IncludeSlotInHash, dashmap::DashMap, solana_sdk::{ account::{AccountSharedData, ReadableAccount}, @@ -71,6 +72,7 @@ impl SlotCacheInner { account: AccountSharedData, hash: Option>, slot: Slot, + include_slot_in_hash: IncludeSlotInHash, ) -> CachedAccount { let data_len = account.data().len() as u64; let item = Arc::new(CachedAccountInner { @@ -78,6 +80,7 @@ impl SlotCacheInner { hash: RwLock::new(hash.map(|h| *h.borrow())), slot, pubkey: *pubkey, + include_slot_in_hash, }); if let Some(old) = self.cache.insert(*pubkey, item.clone()) { self.same_account_writes.fetch_add(1, Ordering::Relaxed); @@ -143,6 +146,9 @@ pub struct CachedAccountInner { hash: RwLock>, slot: Slot, pubkey: Pubkey, + /// temporarily here during feature activation + /// since we calculate the hash later, or in the background, we need knowledge of whether this slot uses the slot in the hash or not + pub include_slot_in_hash: IncludeSlotInHash, } impl CachedAccountInner { @@ -156,6 +162,7 @@ impl CachedAccountInner { self.slot, &self.account, &self.pubkey, + self.include_slot_in_hash, ); *self.hash.write().unwrap() = Some(hash); hash @@ -227,6 +234,7 @@ impl AccountsCache { pubkey: &Pubkey, account: AccountSharedData, hash: Option>, + include_slot_in_hash: IncludeSlotInHash, ) -> CachedAccount { let slot_cache = self.slot_cache(slot).unwrap_or_else(|| // DashMap entry.or_insert() returns a RefMut, essentially a write lock, @@ -239,7 +247,7 @@ impl AccountsCache { .or_insert(self.new_inner()) .clone()); - slot_cache.insert(pubkey, account, hash, slot) + slot_cache.insert(pubkey, account, hash, slot, include_slot_in_hash) } pub fn load(&self, slot: Slot, pubkey: &Pubkey) -> Option { @@ -327,7 +335,7 @@ impl AccountsCache { #[cfg(test)] pub mod tests { - use super::*; + use {super::*, crate::accounts_db::INCLUDE_SLOT_IN_HASH_TESTS}; #[test] fn test_remove_slots_le() { @@ -340,6 +348,7 @@ pub mod tests { &Pubkey::new_unique(), AccountSharedData::new(1, 0, &Pubkey::default()), Some(&Hash::default()), + INCLUDE_SLOT_IN_HASH_TESTS, ); // If the cache is told the size limit is 0, it should return the one slot let removed = cache.remove_slots_le(0); @@ -358,6 +367,7 @@ pub mod tests { &Pubkey::new_unique(), AccountSharedData::new(1, 0, &Pubkey::default()), Some(&Hash::default()), + INCLUDE_SLOT_IN_HASH_TESTS, ); // If the cache is told the size limit is 0, it should return nothing, because there's no diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 2d90aadffe..2a30dc7b74 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -137,6 +137,42 @@ const CACHE_VIRTUAL_WRITE_VERSION: StoredMetaWriteVersion = 0; pub(crate) const CACHE_VIRTUAL_OFFSET: Offset = 0; const CACHE_VIRTUAL_STORED_SIZE: StoredSize = 0; +/// temporary enum during feature activation of +/// ignore slot when calculating an account hash #28420 +#[derive(Debug, Clone, Copy)] +pub enum IncludeSlotInHash { + /// this is the status quo, prior to feature activation + /// INCLUDE the slot in the account hash calculation + IncludeSlot, + /// this is the value once feature activation occurs + /// do NOT include the slot in the account hash calculation + RemoveSlot, + /// this option should not be used. + /// If it is, this is a panic worthy event. + /// There are code paths where the feature activation status isn't known, but this value should not possibly be used. + IrrelevantAssertOnUse, +} + +/// used by tests for 'include_slot_in_hash' parameter +/// Tests just need to be self-consistent, so any value should work here. +pub const INCLUDE_SLOT_IN_HASH_TESTS: IncludeSlotInHash = IncludeSlotInHash::IncludeSlot; + +// This value is irrelevant because we are reading from append vecs and the hash is already computed and saved. +// The hash will just be loaded from the append vec as opposed to being calculated initially. +// A shrink-type operation involves reading from an append vec and writing a subset of the read accounts to a new append vec. +// So, by definition, we will just read hashes and write hashes. The hash will not be calculated. +// The 'store' apis are shared, such that the initial store from a bank (where we need to know whether to include the slot) +// must include a feature-based value for 'include_slot_in_hash'. Other uses, specifically shrink, do NOT need to pass this +// parameter, but the shared api requires a value. +pub const INCLUDE_SLOT_IN_HASH_IRRELEVANT_APPEND_VEC_OPERATION: IncludeSlotInHash = + IncludeSlotInHash::IrrelevantAssertOnUse; + +// This value is irrelevant because the the debug-only check_hash debug option is not possible to enable at the moment. +// This has been true for some time now, due to fallout from disabling rewrites. +// The check_hash debug option can be re-enabled once this feature and the 'rent_epoch' features are enabled. +pub const INCLUDE_SLOT_IN_HASH_IRRELEVANT_CHECK_HASH: IncludeSlotInHash = + IncludeSlotInHash::IrrelevantAssertOnUse; + pub enum StoreReclaims { /// normal reclaim mode Default, @@ -564,15 +600,21 @@ impl<'a> LoadedAccount<'a> { } } - pub fn compute_hash(&self, slot: Slot, pubkey: &Pubkey) -> Hash { + pub fn compute_hash( + &self, + slot: Slot, + pubkey: &Pubkey, + include_slot: IncludeSlotInHash, + ) -> Hash { match self { LoadedAccount::Stored(stored_account_meta) => AccountsDb::hash_account( slot, stored_account_meta, &stored_account_meta.meta.pubkey, + include_slot, ), LoadedAccount::Cached(cached_account) => { - AccountsDb::hash_account(slot, &cached_account.account, pubkey) + AccountsDb::hash_account(slot, &cached_account.account, pubkey, include_slot) } } } @@ -2030,7 +2072,11 @@ impl<'a> AppendVecScan for ScanState<'a> { && !AccountsDb::is_filler_account_helper(pubkey, self.filler_account_suffix) { // this will not be supported anymore - let computed_hash = loaded_account.compute_hash(self.current_slot, pubkey); + let computed_hash = loaded_account.compute_hash( + self.current_slot, + pubkey, + INCLUDE_SLOT_IN_HASH_IRRELEVANT_CHECK_HASH, + ); if computed_hash != source_item.hash { info!( "hash mismatch found: computed: {}, loaded: {}, pubkey: {}", @@ -3685,7 +3731,11 @@ impl AccountsDb { // without use of rather wide locks in this whole function, because we're // mutating rooted slots; There should be no writers to them. store_accounts_timing = self.store_accounts_frozen( - (slot, &accounts[..]), + ( + slot, + &accounts[..], + INCLUDE_SLOT_IN_HASH_IRRELEVANT_APPEND_VEC_OPERATION, + ), Some(&hashes), Some(&shrunken_store), Some(Box::new(write_versions.into_iter())), @@ -4103,7 +4153,11 @@ impl AccountsDb { let (accounts, hashes) = accounts.get(storage_selector); self.store_accounts_frozen( - (ancient_slot, accounts), + ( + ancient_slot, + accounts, + INCLUDE_SLOT_IN_HASH_IRRELEVANT_APPEND_VEC_OPERATION, + ), Some(hashes), Some(ancient_store), None, @@ -5942,6 +5996,7 @@ impl AccountsDb { account: &T, pubkey: &Pubkey, rent_epoch: Epoch, + include_slot: IncludeSlotInHash, ) -> Hash { Self::hash_account_data( slot, @@ -5951,10 +6006,16 @@ impl AccountsDb { rent_epoch, account.data(), pubkey, + include_slot, ) } - pub fn hash_account(slot: Slot, account: &T, pubkey: &Pubkey) -> Hash { + pub fn hash_account( + slot: Slot, + account: &T, + pubkey: &Pubkey, + include_slot: IncludeSlotInHash, + ) -> Hash { Self::hash_account_data( slot, account.lamports(), @@ -5963,6 +6024,7 @@ impl AccountsDb { account.rent_epoch(), account.data(), pubkey, + include_slot, ) } @@ -5974,6 +6036,7 @@ impl AccountsDb { rent_epoch: Epoch, data: &[u8], pubkey: &Pubkey, + include_slot: IncludeSlotInHash, ) -> Hash { if lamports == 0 { return Hash::default(); @@ -5983,7 +6046,16 @@ impl AccountsDb { hasher.update(&lamports.to_le_bytes()); - hasher.update(&slot.to_le_bytes()); + match include_slot { + IncludeSlotInHash::IncludeSlot => { + // upon feature activation, stop including slot# in the account hash + hasher.update(&slot.to_le_bytes()); + } + IncludeSlotInHash::RemoveSlot => {} + IncludeSlotInHash::IrrelevantAssertOnUse => { + panic!("IncludeSlotInHash is irrelevant, but we are calculating hash"); + } + } hasher.update(&rent_epoch.to_le_bytes()); @@ -6372,8 +6444,10 @@ impl AccountsDb { // will be able to find the account in storage let flushed_store = self.create_and_insert_store(slot, aligned_total_size, "flush_slot_cache"); + // irrelevant - account will already be hashed since it was used in bank hash previously + let include_slot_in_hash = IncludeSlotInHash::IrrelevantAssertOnUse; self.store_accounts_frozen( - (slot, &accounts[..]), + (slot, &accounts[..], include_slot_in_hash), Some(&hashes), Some(&flushed_store), None, @@ -6391,7 +6465,7 @@ impl AccountsDb { hashes.push(hash); }); self.store_accounts_frozen( - (slot, &accounts[..]), + (slot, &accounts[..], include_slot_in_hash), Some(&hashes), Some(&flushed_store), None, @@ -6491,6 +6565,7 @@ impl AccountsDb { hashes: Option<&[impl Borrow]>, accounts_and_meta_to_store: &[(StoredMeta, Option<&impl ReadableAccount>)], txn_signatures_iter: Box> + 'a>, + include_slot_in_hash: IncludeSlotInHash, ) -> Vec { let len = accounts_and_meta_to_store.len(); let hashes = hashes.map(|hashes| { @@ -6516,7 +6591,13 @@ impl AccountsDb { self.notify_account_at_accounts_update(slot, meta, &account, signature); - let cached_account = self.accounts_cache.store(slot, &meta.pubkey, account, hash); + let cached_account = self.accounts_cache.store( + slot, + &meta.pubkey, + account, + hash, + include_slot_in_hash, + ); // hash this account in the bg match &self.sender_bg_hasher { Some(ref sender) => { @@ -6583,7 +6664,13 @@ impl AccountsDb { } }; - self.write_accounts_to_cache(slot, hashes, &accounts_and_meta_to_store, signature_iter) + self.write_accounts_to_cache( + slot, + hashes, + &accounts_and_meta_to_store, + signature_iter, + accounts.include_slot_in_hash(), + ) } else { match hashes { Some(hashes) => self.write_accounts_to_storage( @@ -6599,7 +6686,12 @@ impl AccountsDb { let mut hashes = Vec::with_capacity(len); for index in 0..accounts.len() { let (pubkey, account) = (accounts.pubkey(index), accounts.account(index)); - let hash = Self::hash_account(slot, account, pubkey); + let hash = Self::hash_account( + slot, + account, + pubkey, + accounts.include_slot_in_hash(), + ); hashes.push(hash); } hash_time.stop(); @@ -6776,7 +6868,7 @@ impl AccountsDb { let balance = loaded_account.lamports(); if config.check_hash && !self.is_filler_account(pubkey) { // this will not be supported anymore let computed_hash = - loaded_account.compute_hash(*slot, pubkey); + loaded_account.compute_hash(*slot, pubkey, INCLUDE_SLOT_IN_HASH_IRRELEVANT_CHECK_HASH); if computed_hash != loaded_hash { info!("hash mismatch found: computed: {}, loaded: {}, pubkey: {}", computed_hash, loaded_hash, pubkey); mismatch_found @@ -8117,7 +8209,12 @@ impl AccountsDb { /// Store the account update. /// only called by tests pub fn store_uncached(&self, slot: Slot, accounts: &[(&Pubkey, &AccountSharedData)]) { - self.store((slot, accounts), false, None, StoreReclaims::Default); + self.store( + (slot, accounts, INCLUDE_SLOT_IN_HASH_TESTS), + false, + None, + StoreReclaims::Default, + ); } fn store<'a, T: ReadableAccount + Sync + ZeroLamport>( @@ -8850,8 +8947,10 @@ impl AccountsDb { .collect::>(); let hashes = (0..filler_entries).map(|_| hash).collect::>(); self.maybe_throttle_index_generation(); + // filler accounts are debug only and their hash is irrelevant anyway, so any value is ok here. + let include_slot_in_hash = INCLUDE_SLOT_IN_HASH_TESTS; self.store_accounts_frozen( - (*slot, &add[..]), + (*slot, &add[..], include_slot_in_hash), Some(&hashes[..]), None, None, @@ -9623,6 +9722,71 @@ pub mod tests { } } + /// This impl exists until this feature is activated: + /// ignore slot when calculating an account hash #28420 + /// For now, all test code will continue to work thanks to this impl + /// Tests will use INCLUDE_SLOT_IN_HASH_TESTS for 'include_slot_in_hash' calls. + impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> for (Slot, &'a [(&'a Pubkey, &'a T)]) { + fn pubkey(&self, index: usize) -> &Pubkey { + self.1[index].0 + } + fn account(&self, index: usize) -> &T { + self.1[index].1 + } + fn slot(&self, _index: usize) -> Slot { + // per-index slot is not unique per slot when per-account slot is not included in the source data + self.target_slot() + } + fn target_slot(&self) -> Slot { + self.0 + } + fn len(&self) -> usize { + self.1.len() + } + fn contains_multiple_slots(&self) -> bool { + false + } + fn include_slot_in_hash(&self) -> IncludeSlotInHash { + INCLUDE_SLOT_IN_HASH_TESTS + } + } + + /// this tuple contains slot info PER account + /// last bool is include_slot_in_hash + impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> + for (Slot, &'a [(&'a Pubkey, &'a T, Slot)]) + { + fn pubkey(&self, index: usize) -> &Pubkey { + self.1[index].0 + } + fn account(&self, index: usize) -> &T { + self.1[index].1 + } + fn slot(&self, index: usize) -> Slot { + // note that this could be different than 'target_slot()' PER account + self.1[index].2 + } + fn target_slot(&self) -> Slot { + self.0 + } + fn len(&self) -> usize { + self.1.len() + } + fn contains_multiple_slots(&self) -> bool { + let len = self.len(); + if len > 0 { + let slot = self.slot(0); + // true if any item has a different slot than the first item + (1..len).any(|i| slot != self.slot(i)) + } else { + false + } + } + fn include_slot_in_hash(&self) -> IncludeSlotInHash { + INCLUDE_SLOT_IN_HASH_TESTS + } + } + #[test] fn test_retain_roots_within_one_epoch_range() { let mut roots = vec![0, 1, 2]; @@ -9703,7 +9867,12 @@ pub mod tests { 1, AccountSharedData::default().owner(), )); - let hash = AccountsDb::hash_account(slot, &raw_accounts[i], &raw_expected[i].pubkey); + let hash = AccountsDb::hash_account( + slot, + &raw_accounts[i], + &raw_expected[i].pubkey, + INCLUDE_SLOT_IN_HASH_TESTS, + ); if slot == 1 { assert_eq!(hash, expected_hashes[i]); } @@ -12157,12 +12326,22 @@ pub mod tests { }; assert_eq!( - AccountsDb::hash_account(slot, &stored_account, &stored_account.meta.pubkey), + AccountsDb::hash_account( + slot, + &stored_account, + &stored_account.meta.pubkey, + INCLUDE_SLOT_IN_HASH_TESTS + ), expected_account_hash, "StoredAccountMeta's data layout might be changed; update hashing if needed." ); assert_eq!( - AccountsDb::hash_account(slot, &account, &stored_account.meta.pubkey), + AccountsDb::hash_account( + slot, + &account, + &stored_account.meta.pubkey, + INCLUDE_SLOT_IN_HASH_TESTS + ), expected_account_hash, "Account-based hashing must be consistent with StoredAccountMeta-based one." ); @@ -12195,6 +12374,8 @@ pub mod tests { assert_eq!(bank_hash.stats.num_executable_accounts, 1); } + // this test tests check_hash=true, which is unsupported behavior at the moment. It cannot be enabled by anything but these tests. + #[ignore] #[test] fn test_calculate_accounts_hash_check_hash_mismatch() { solana_logger::setup(); @@ -12253,6 +12434,8 @@ pub mod tests { } } + // this test tests check_hash=true, which is unsupported behavior at the moment. It cannot be enabled by anything but these tests. + #[ignore] #[test] fn test_calculate_accounts_hash_check_hash() { solana_logger::setup(); @@ -16028,8 +16211,14 @@ pub mod tests { for rent in 0..3 { account.set_rent_epoch(rent); assert_eq!( - AccountsDb::hash_account(slot, &account, &pubkey), - AccountsDb::hash_account_with_rent_epoch(slot, &account, &pubkey, rent) + AccountsDb::hash_account(slot, &account, &pubkey, INCLUDE_SLOT_IN_HASH_TESTS), + AccountsDb::hash_account_with_rent_epoch( + slot, + &account, + &pubkey, + rent, + INCLUDE_SLOT_IN_HASH_TESTS + ) ); } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 9e558d357a..850562923f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -44,7 +44,7 @@ use { TransactionLoadResult, }, accounts_db::{ - AccountShrinkThreshold, AccountsDbConfig, SnapshotStorages, + AccountShrinkThreshold, AccountsDbConfig, IncludeSlotInHash, SnapshotStorages, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING, }, accounts_index::{AccountSecondaryIndexes, IndexKey, ScanConfig, ScanResult, ZeroLamport}, @@ -1154,7 +1154,7 @@ impl StakeReward { } /// allow [StakeReward] to be passed to `StoreAccounts` directly without copies or vec construction -impl<'a> StorableAccounts<'a, AccountSharedData> for (Slot, &'a [StakeReward]) { +impl<'a> StorableAccounts<'a, AccountSharedData> for (Slot, &'a [StakeReward], IncludeSlotInHash) { fn pubkey(&self, index: usize) -> &Pubkey { &self.1[index].stake_pubkey } @@ -1174,6 +1174,9 @@ impl<'a> StorableAccounts<'a, AccountSharedData> for (Slot, &'a [StakeReward]) { fn contains_multiple_slots(&self) -> bool { false } + fn include_slot_in_hash(&self) -> IncludeSlotInHash { + self.2 + } } impl Bank { @@ -2957,7 +2960,7 @@ impl Bank { // store stake account even if stakers_reward is 0 // because credits observed has changed let mut m = Measure::start("store_stake_account"); - self.store_accounts((self.slot(), stake_rewards)); + self.store_accounts((self.slot(), stake_rewards, self.include_slot_in_hash())); m.stop(); metrics .store_stake_accounts_us @@ -4801,6 +4804,7 @@ impl Bank { &durable_nonce, lamports_per_signature, self.preserve_rent_epoch_for_rent_exempt_accounts(), + self.include_slot_in_hash(), ); let rent_debits = self.collect_rent(&execution_results, loaded_txs); @@ -5262,7 +5266,8 @@ impl Bank { let (hash, measure) = measure!(crate::accounts_db::AccountsDb::hash_account( self.slot(), account, - pubkey + pubkey, + self.include_slot_in_hash(), )); time_hashing_skipped_rewrites_us += measure.as_us(); rewrites_skipped.push((*pubkey, hash)); @@ -5293,7 +5298,11 @@ impl Bank { if !accounts_to_store.is_empty() { // TODO: Maybe do not call `store_accounts()` here. Instead return `accounts_to_store` // and have `collect_rent_in_partition()` perform all the stores. - let (_, measure) = measure!(self.store_accounts((self.slot(), &accounts_to_store[..]))); + let (_, measure) = measure!(self.store_accounts(( + self.slot(), + &accounts_to_store[..], + self.include_slot_in_hash() + ))); time_storing_accounts_us += measure.as_us(); } @@ -5308,6 +5317,18 @@ impl Bank { } } + /// true if we should include the slot in account hash + fn include_slot_in_hash(&self) -> IncludeSlotInHash { + if self + .feature_set + .is_active(&feature_set::account_hash_ignore_slot::id()) + { + IncludeSlotInHash::RemoveSlot + } else { + IncludeSlotInHash::IncludeSlot + } + } + /// convert 'partition' to a pubkey range and 'collect_rent_in_range' fn collect_rent_in_partition( &self, @@ -6169,7 +6190,11 @@ impl Bank { pubkey: &Pubkey, account: &T, ) { - self.store_accounts((self.slot(), &[(pubkey, account)][..])) + self.store_accounts(( + self.slot(), + &[(pubkey, account)][..], + self.include_slot_in_hash(), + )) } pub fn store_accounts<'a, T: ReadableAccount + Sync + ZeroLamport>( diff --git a/runtime/src/snapshot_minimizer.rs b/runtime/src/snapshot_minimizer.rs index 6e68bdc4d0..7a710e63a3 100644 --- a/runtime/src/snapshot_minimizer.rs +++ b/runtime/src/snapshot_minimizer.rs @@ -368,9 +368,12 @@ impl<'a> SnapshotMinimizer<'a> { } let (new_storage, _time) = self.accounts_db().get_store_for_shrink(slot, aligned_total); - self.accounts_db().store_accounts_frozen( - (slot, &accounts[..]), + ( + slot, + &accounts[..], + crate::accounts_db::INCLUDE_SLOT_IN_HASH_IRRELEVANT_APPEND_VEC_OPERATION, + ), Some(&hashes), Some(&new_storage), Some(Box::new(write_versions.into_iter())), diff --git a/runtime/src/storable_accounts.rs b/runtime/src/storable_accounts.rs index bfa35cf71c..f6433c0538 100644 --- a/runtime/src/storable_accounts.rs +++ b/runtime/src/storable_accounts.rs @@ -1,5 +1,8 @@ //! trait for abstracting underlying storage of pubkey and account pairs to be written -use solana_sdk::{account::ReadableAccount, clock::Slot, pubkey::Pubkey}; +use { + crate::accounts_db::IncludeSlotInHash, + solana_sdk::{account::ReadableAccount, clock::Slot, pubkey::Pubkey}, +}; /// abstract access to pubkey, account, slot, target_slot of either: /// a. (slot, &[&Pubkey, &ReadableAccount]) @@ -24,6 +27,8 @@ pub trait StorableAccounts<'a, T: ReadableAccount + Sync>: Sync { /// are there accounts from multiple slots /// only used for an assert fn contains_multiple_slots(&self) -> bool; + /// true iff hashing these accounts should include the slot + fn include_slot_in_hash(&self) -> IncludeSlotInHash; } /// accounts that are moving from 'old_slot' to 'target_slot' @@ -36,6 +41,8 @@ pub struct StorableAccountsMovingSlots<'a, T: ReadableAccount + Sync> { pub target_slot: Slot, /// slot where accounts are currently stored pub old_slot: Slot, + /// This is temporarily here until feature activation. + pub include_slot_in_hash: IncludeSlotInHash, } impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> for StorableAccountsMovingSlots<'a, T> { @@ -58,9 +65,16 @@ impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> for StorableAccounts fn contains_multiple_slots(&self) -> bool { false } + fn include_slot_in_hash(&self) -> IncludeSlotInHash { + self.include_slot_in_hash + } } -impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> for (Slot, &'a [(&'a Pubkey, &'a T)]) { +/// The last parameter exists until this feature is activated: +/// ignore slot when calculating an account hash #28420 +impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> + for (Slot, &'a [(&'a Pubkey, &'a T)], IncludeSlotInHash) +{ fn pubkey(&self, index: usize) -> &Pubkey { self.1[index].0 } @@ -80,11 +94,14 @@ impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> for (Slot, &'a [(&'a fn contains_multiple_slots(&self) -> bool { false } + fn include_slot_in_hash(&self) -> IncludeSlotInHash { + self.2 + } } /// this tuple contains slot info PER account impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> - for (Slot, &'a [(&'a Pubkey, &'a T, Slot)]) + for (Slot, &'a [(&'a Pubkey, &'a T, Slot)], IncludeSlotInHash) { fn pubkey(&self, index: usize) -> &Pubkey { self.1[index].0 @@ -112,12 +129,16 @@ impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> false } } + fn include_slot_in_hash(&self) -> IncludeSlotInHash { + self.2 + } } #[cfg(test)] pub mod tests { use { super::*, + crate::accounts_db::INCLUDE_SLOT_IN_HASH_TESTS, solana_sdk::account::{AccountSharedData, WritableAccount}, }; @@ -142,11 +163,13 @@ pub mod tests { let test3 = ( slot, &vec![(&pk, &account, slot), (&pk, &account, slot)][..], + INCLUDE_SLOT_IN_HASH_TESTS, ); assert!(!test3.contains_multiple_slots()); let test3 = ( slot, &vec![(&pk, &account, slot), (&pk, &account, slot + 1)][..], + INCLUDE_SLOT_IN_HASH_TESTS, ); assert!(test3.contains_multiple_slots()); } @@ -178,13 +201,14 @@ pub mod tests { two.push((&raw.0, &raw.1)); // 2 item tuple three.push((&raw.0, &raw.1, raw.2)); // 3 item tuple, including slot }); - let test2 = (target_slot, &two[..]); - let test3 = (target_slot, &three[..]); + let test2 = (target_slot, &two[..], INCLUDE_SLOT_IN_HASH_TESTS); + let test3 = (target_slot, &three[..], INCLUDE_SLOT_IN_HASH_TESTS); let old_slot = starting_slot; let test_moving_slots = StorableAccountsMovingSlots { accounts: &two[..], target_slot, old_slot, + include_slot_in_hash: INCLUDE_SLOT_IN_HASH_TESTS, }; compare(&test2, &test3); compare(&test2, &test_moving_slots); diff --git a/runtime/tests/accounts.rs b/runtime/tests/accounts.rs index fdb55978a6..84eb0c0682 100644 --- a/runtime/tests/accounts.rs +++ b/runtime/tests/accounts.rs @@ -3,7 +3,7 @@ use { rand::{thread_rng, Rng}, rayon::prelude::*, solana_runtime::{ - accounts_db::{AccountsDb, LoadHint}, + accounts_db::{AccountsDb, LoadHint, INCLUDE_SLOT_IN_HASH_TESTS}, ancestors::Ancestors, }, solana_sdk::{ @@ -128,7 +128,7 @@ fn test_bad_bank_hash() { assert_eq!( db.load_account_hash(&ancestors, key, None, LoadHint::Unspecified) .unwrap(), - AccountsDb::hash_account(some_slot, *account, key) + AccountsDb::hash_account(some_slot, *account, key, INCLUDE_SLOT_IN_HASH_TESTS) ); } existing.clear(); diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index bd6e533b0c..c7fe655036 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -490,6 +490,10 @@ pub mod disable_cpi_setting_executable_and_rent_epoch { solana_sdk::declare_id!("B9cdB55u4jQsDNsdTK525yE9dmSc5Ga7YBaBrDFvEhM9"); } +pub mod account_hash_ignore_slot { + solana_sdk::declare_id!("SVn36yVApPLYsa8koK3qUcy14zXDnqkNYWyUh1f4oK1"); +} + pub mod relax_authority_signer_check_for_lookup_table_creation { solana_sdk::declare_id!("FKAcEvNgSY79RpqsPNUV5gDyumopH4cEHqUxyfm8b8Ap"); } @@ -632,6 +636,7 @@ lazy_static! { (preserve_rent_epoch_for_rent_exempt_accounts::id(), "preserve rent epoch for rent exempt accounts #26479"), (enable_bpf_loader_extend_program_ix::id(), "enable bpf upgradeable loader ExtendProgram instruction #25234"), (enable_early_verification_of_account_modifications::id(), "enable early verification of account modifications #25899"), + (account_hash_ignore_slot::id(), "ignore slot when calculating an account hash #28420"), (prevent_crediting_accounts_that_end_rent_paying::id(), "prevent crediting rent paying accounts #26606"), (cap_bpf_program_instruction_accounts::id(), "enforce max number of accounts per bpf program instruction #26628"), (loosen_cpi_size_restriction::id(), "loosen cpi size restrictions #26641"),