remove slot# from account hash (#28405)

* remove slot# from account hash

* add feature

* fix tests

* constants to help clarify 'irrelevant' changes

* move to enum for enforcing irrelevancy

* ignore unsupported tests
This commit is contained in:
Jeff Washington (jwash) 2022-10-18 08:03:37 -07:00 committed by GitHub
parent 682999a423
commit fd2e671861
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 299 additions and 40 deletions

View File

@ -4,7 +4,7 @@ use {
account_rent_state::{check_rent_state_with_account, RentState}, account_rent_state::{check_rent_state_with_account, RentState},
accounts_db::{ accounts_db::{
AccountShrinkThreshold, AccountsAddRootTiming, AccountsDb, AccountsDbConfig, AccountShrinkThreshold, AccountsAddRootTiming, AccountsDb, AccountsDbConfig,
BankHashInfo, LoadHint, LoadedAccount, ScanStorageResult, BankHashInfo, IncludeSlotInHash, LoadHint, LoadedAccount, ScanStorageResult,
ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING,
}, },
accounts_index::{ accounts_index::{
@ -1201,6 +1201,7 @@ impl Accounts {
durable_nonce: &DurableNonce, durable_nonce: &DurableNonce,
lamports_per_signature: u64, lamports_per_signature: u64,
preserve_rent_epoch_for_rent_exempt_accounts: bool, preserve_rent_epoch_for_rent_exempt_accounts: bool,
include_slot_in_hash: IncludeSlotInHash,
) { ) {
let (accounts_to_store, txn_signatures) = self.collect_accounts_to_store( let (accounts_to_store, txn_signatures) = self.collect_accounts_to_store(
txs, txs,
@ -1211,8 +1212,10 @@ impl Accounts {
lamports_per_signature, lamports_per_signature,
preserve_rent_epoch_for_rent_exempt_accounts, preserve_rent_epoch_for_rent_exempt_accounts,
); );
self.accounts_db self.accounts_db.store_cached(
.store_cached((slot, &accounts_to_store[..]), Some(&txn_signatures)); (slot, &accounts_to_store[..], include_slot_in_hash),
Some(&txn_signatures),
);
} }
pub fn store_accounts_cached<'a, T: ReadableAccount + Sync + ZeroLamport>( pub fn store_accounts_cached<'a, T: ReadableAccount + Sync + ZeroLamport>(

View File

@ -1,4 +1,5 @@
use { use {
crate::accounts_db::IncludeSlotInHash,
dashmap::DashMap, dashmap::DashMap,
solana_sdk::{ solana_sdk::{
account::{AccountSharedData, ReadableAccount}, account::{AccountSharedData, ReadableAccount},
@ -71,6 +72,7 @@ impl SlotCacheInner {
account: AccountSharedData, account: AccountSharedData,
hash: Option<impl Borrow<Hash>>, hash: Option<impl Borrow<Hash>>,
slot: Slot, slot: Slot,
include_slot_in_hash: IncludeSlotInHash,
) -> CachedAccount { ) -> CachedAccount {
let data_len = account.data().len() as u64; let data_len = account.data().len() as u64;
let item = Arc::new(CachedAccountInner { let item = Arc::new(CachedAccountInner {
@ -78,6 +80,7 @@ impl SlotCacheInner {
hash: RwLock::new(hash.map(|h| *h.borrow())), hash: RwLock::new(hash.map(|h| *h.borrow())),
slot, slot,
pubkey: *pubkey, pubkey: *pubkey,
include_slot_in_hash,
}); });
if let Some(old) = self.cache.insert(*pubkey, item.clone()) { if let Some(old) = self.cache.insert(*pubkey, item.clone()) {
self.same_account_writes.fetch_add(1, Ordering::Relaxed); self.same_account_writes.fetch_add(1, Ordering::Relaxed);
@ -143,6 +146,9 @@ pub struct CachedAccountInner {
hash: RwLock<Option<Hash>>, hash: RwLock<Option<Hash>>,
slot: Slot, slot: Slot,
pubkey: Pubkey, 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 { impl CachedAccountInner {
@ -156,6 +162,7 @@ impl CachedAccountInner {
self.slot, self.slot,
&self.account, &self.account,
&self.pubkey, &self.pubkey,
self.include_slot_in_hash,
); );
*self.hash.write().unwrap() = Some(hash); *self.hash.write().unwrap() = Some(hash);
hash hash
@ -227,6 +234,7 @@ impl AccountsCache {
pubkey: &Pubkey, pubkey: &Pubkey,
account: AccountSharedData, account: AccountSharedData,
hash: Option<impl Borrow<Hash>>, hash: Option<impl Borrow<Hash>>,
include_slot_in_hash: IncludeSlotInHash,
) -> CachedAccount { ) -> CachedAccount {
let slot_cache = self.slot_cache(slot).unwrap_or_else(|| let slot_cache = self.slot_cache(slot).unwrap_or_else(||
// DashMap entry.or_insert() returns a RefMut, essentially a write lock, // DashMap entry.or_insert() returns a RefMut, essentially a write lock,
@ -239,7 +247,7 @@ impl AccountsCache {
.or_insert(self.new_inner()) .or_insert(self.new_inner())
.clone()); .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<CachedAccount> { pub fn load(&self, slot: Slot, pubkey: &Pubkey) -> Option<CachedAccount> {
@ -327,7 +335,7 @@ impl AccountsCache {
#[cfg(test)] #[cfg(test)]
pub mod tests { pub mod tests {
use super::*; use {super::*, crate::accounts_db::INCLUDE_SLOT_IN_HASH_TESTS};
#[test] #[test]
fn test_remove_slots_le() { fn test_remove_slots_le() {
@ -340,6 +348,7 @@ pub mod tests {
&Pubkey::new_unique(), &Pubkey::new_unique(),
AccountSharedData::new(1, 0, &Pubkey::default()), AccountSharedData::new(1, 0, &Pubkey::default()),
Some(&Hash::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 // If the cache is told the size limit is 0, it should return the one slot
let removed = cache.remove_slots_le(0); let removed = cache.remove_slots_le(0);
@ -358,6 +367,7 @@ pub mod tests {
&Pubkey::new_unique(), &Pubkey::new_unique(),
AccountSharedData::new(1, 0, &Pubkey::default()), AccountSharedData::new(1, 0, &Pubkey::default()),
Some(&Hash::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 // If the cache is told the size limit is 0, it should return nothing, because there's no

View File

@ -137,6 +137,42 @@ const CACHE_VIRTUAL_WRITE_VERSION: StoredMetaWriteVersion = 0;
pub(crate) const CACHE_VIRTUAL_OFFSET: Offset = 0; pub(crate) const CACHE_VIRTUAL_OFFSET: Offset = 0;
const CACHE_VIRTUAL_STORED_SIZE: StoredSize = 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 { pub enum StoreReclaims {
/// normal reclaim mode /// normal reclaim mode
Default, 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 { match self {
LoadedAccount::Stored(stored_account_meta) => AccountsDb::hash_account( LoadedAccount::Stored(stored_account_meta) => AccountsDb::hash_account(
slot, slot,
stored_account_meta, stored_account_meta,
&stored_account_meta.meta.pubkey, &stored_account_meta.meta.pubkey,
include_slot,
), ),
LoadedAccount::Cached(cached_account) => { 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) && !AccountsDb::is_filler_account_helper(pubkey, self.filler_account_suffix)
{ {
// this will not be supported anymore // 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 { if computed_hash != source_item.hash {
info!( info!(
"hash mismatch found: computed: {}, loaded: {}, pubkey: {}", "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 // without use of rather wide locks in this whole function, because we're
// mutating rooted slots; There should be no writers to them. // mutating rooted slots; There should be no writers to them.
store_accounts_timing = self.store_accounts_frozen( store_accounts_timing = self.store_accounts_frozen(
(slot, &accounts[..]), (
slot,
&accounts[..],
INCLUDE_SLOT_IN_HASH_IRRELEVANT_APPEND_VEC_OPERATION,
),
Some(&hashes), Some(&hashes),
Some(&shrunken_store), Some(&shrunken_store),
Some(Box::new(write_versions.into_iter())), Some(Box::new(write_versions.into_iter())),
@ -4103,7 +4153,11 @@ impl AccountsDb {
let (accounts, hashes) = accounts.get(storage_selector); let (accounts, hashes) = accounts.get(storage_selector);
self.store_accounts_frozen( self.store_accounts_frozen(
(ancient_slot, accounts), (
ancient_slot,
accounts,
INCLUDE_SLOT_IN_HASH_IRRELEVANT_APPEND_VEC_OPERATION,
),
Some(hashes), Some(hashes),
Some(ancient_store), Some(ancient_store),
None, None,
@ -5942,6 +5996,7 @@ impl AccountsDb {
account: &T, account: &T,
pubkey: &Pubkey, pubkey: &Pubkey,
rent_epoch: Epoch, rent_epoch: Epoch,
include_slot: IncludeSlotInHash,
) -> Hash { ) -> Hash {
Self::hash_account_data( Self::hash_account_data(
slot, slot,
@ -5951,10 +6006,16 @@ impl AccountsDb {
rent_epoch, rent_epoch,
account.data(), account.data(),
pubkey, pubkey,
include_slot,
) )
} }
pub fn hash_account<T: ReadableAccount>(slot: Slot, account: &T, pubkey: &Pubkey) -> Hash { pub fn hash_account<T: ReadableAccount>(
slot: Slot,
account: &T,
pubkey: &Pubkey,
include_slot: IncludeSlotInHash,
) -> Hash {
Self::hash_account_data( Self::hash_account_data(
slot, slot,
account.lamports(), account.lamports(),
@ -5963,6 +6024,7 @@ impl AccountsDb {
account.rent_epoch(), account.rent_epoch(),
account.data(), account.data(),
pubkey, pubkey,
include_slot,
) )
} }
@ -5974,6 +6036,7 @@ impl AccountsDb {
rent_epoch: Epoch, rent_epoch: Epoch,
data: &[u8], data: &[u8],
pubkey: &Pubkey, pubkey: &Pubkey,
include_slot: IncludeSlotInHash,
) -> Hash { ) -> Hash {
if lamports == 0 { if lamports == 0 {
return Hash::default(); return Hash::default();
@ -5983,7 +6046,16 @@ impl AccountsDb {
hasher.update(&lamports.to_le_bytes()); 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()); hasher.update(&rent_epoch.to_le_bytes());
@ -6372,8 +6444,10 @@ impl AccountsDb {
// will be able to find the account in storage // will be able to find the account in storage
let flushed_store = let flushed_store =
self.create_and_insert_store(slot, aligned_total_size, "flush_slot_cache"); 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( self.store_accounts_frozen(
(slot, &accounts[..]), (slot, &accounts[..], include_slot_in_hash),
Some(&hashes), Some(&hashes),
Some(&flushed_store), Some(&flushed_store),
None, None,
@ -6391,7 +6465,7 @@ impl AccountsDb {
hashes.push(hash); hashes.push(hash);
}); });
self.store_accounts_frozen( self.store_accounts_frozen(
(slot, &accounts[..]), (slot, &accounts[..], include_slot_in_hash),
Some(&hashes), Some(&hashes),
Some(&flushed_store), Some(&flushed_store),
None, None,
@ -6491,6 +6565,7 @@ impl AccountsDb {
hashes: Option<&[impl Borrow<Hash>]>, hashes: Option<&[impl Borrow<Hash>]>,
accounts_and_meta_to_store: &[(StoredMeta, Option<&impl ReadableAccount>)], accounts_and_meta_to_store: &[(StoredMeta, Option<&impl ReadableAccount>)],
txn_signatures_iter: Box<dyn std::iter::Iterator<Item = &Option<&Signature>> + 'a>, txn_signatures_iter: Box<dyn std::iter::Iterator<Item = &Option<&Signature>> + 'a>,
include_slot_in_hash: IncludeSlotInHash,
) -> Vec<AccountInfo> { ) -> Vec<AccountInfo> {
let len = accounts_and_meta_to_store.len(); let len = accounts_and_meta_to_store.len();
let hashes = hashes.map(|hashes| { let hashes = hashes.map(|hashes| {
@ -6516,7 +6591,13 @@ impl AccountsDb {
self.notify_account_at_accounts_update(slot, meta, &account, signature); 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 // hash this account in the bg
match &self.sender_bg_hasher { match &self.sender_bg_hasher {
Some(ref sender) => { 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 { } else {
match hashes { match hashes {
Some(hashes) => self.write_accounts_to_storage( Some(hashes) => self.write_accounts_to_storage(
@ -6599,7 +6686,12 @@ impl AccountsDb {
let mut hashes = Vec::with_capacity(len); let mut hashes = Vec::with_capacity(len);
for index in 0..accounts.len() { for index in 0..accounts.len() {
let (pubkey, account) = (accounts.pubkey(index), accounts.account(index)); 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); hashes.push(hash);
} }
hash_time.stop(); hash_time.stop();
@ -6776,7 +6868,7 @@ impl AccountsDb {
let balance = loaded_account.lamports(); let balance = loaded_account.lamports();
if config.check_hash && !self.is_filler_account(pubkey) { // this will not be supported anymore if config.check_hash && !self.is_filler_account(pubkey) { // this will not be supported anymore
let computed_hash = 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 { if computed_hash != loaded_hash {
info!("hash mismatch found: computed: {}, loaded: {}, pubkey: {}", computed_hash, loaded_hash, pubkey); info!("hash mismatch found: computed: {}, loaded: {}, pubkey: {}", computed_hash, loaded_hash, pubkey);
mismatch_found mismatch_found
@ -8117,7 +8209,12 @@ impl AccountsDb {
/// Store the account update. /// Store the account update.
/// only called by tests /// only called by tests
pub fn store_uncached(&self, slot: Slot, accounts: &[(&Pubkey, &AccountSharedData)]) { 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>( fn store<'a, T: ReadableAccount + Sync + ZeroLamport>(
@ -8850,8 +8947,10 @@ impl AccountsDb {
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let hashes = (0..filler_entries).map(|_| hash).collect::<Vec<_>>(); let hashes = (0..filler_entries).map(|_| hash).collect::<Vec<_>>();
self.maybe_throttle_index_generation(); 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( self.store_accounts_frozen(
(*slot, &add[..]), (*slot, &add[..], include_slot_in_hash),
Some(&hashes[..]), Some(&hashes[..]),
None, None,
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] #[test]
fn test_retain_roots_within_one_epoch_range() { fn test_retain_roots_within_one_epoch_range() {
let mut roots = vec![0, 1, 2]; let mut roots = vec![0, 1, 2];
@ -9703,7 +9867,12 @@ pub mod tests {
1, 1,
AccountSharedData::default().owner(), 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 { if slot == 1 {
assert_eq!(hash, expected_hashes[i]); assert_eq!(hash, expected_hashes[i]);
} }
@ -12157,12 +12326,22 @@ pub mod tests {
}; };
assert_eq!( 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, expected_account_hash,
"StoredAccountMeta's data layout might be changed; update hashing if needed." "StoredAccountMeta's data layout might be changed; update hashing if needed."
); );
assert_eq!( 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, expected_account_hash,
"Account-based hashing must be consistent with StoredAccountMeta-based one." "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); 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] #[test]
fn test_calculate_accounts_hash_check_hash_mismatch() { fn test_calculate_accounts_hash_check_hash_mismatch() {
solana_logger::setup(); 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] #[test]
fn test_calculate_accounts_hash_check_hash() { fn test_calculate_accounts_hash_check_hash() {
solana_logger::setup(); solana_logger::setup();
@ -16028,8 +16211,14 @@ pub mod tests {
for rent in 0..3 { for rent in 0..3 {
account.set_rent_epoch(rent); account.set_rent_epoch(rent);
assert_eq!( assert_eq!(
AccountsDb::hash_account(slot, &account, &pubkey), AccountsDb::hash_account(slot, &account, &pubkey, INCLUDE_SLOT_IN_HASH_TESTS),
AccountsDb::hash_account_with_rent_epoch(slot, &account, &pubkey, rent) AccountsDb::hash_account_with_rent_epoch(
slot,
&account,
&pubkey,
rent,
INCLUDE_SLOT_IN_HASH_TESTS
)
); );
} }
} }

View File

@ -44,7 +44,7 @@ use {
TransactionLoadResult, TransactionLoadResult,
}, },
accounts_db::{ accounts_db::{
AccountShrinkThreshold, AccountsDbConfig, SnapshotStorages, AccountShrinkThreshold, AccountsDbConfig, IncludeSlotInHash, SnapshotStorages,
ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING, ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS, ACCOUNTS_DB_CONFIG_FOR_TESTING,
}, },
accounts_index::{AccountSecondaryIndexes, IndexKey, ScanConfig, ScanResult, ZeroLamport}, 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 /// 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 { fn pubkey(&self, index: usize) -> &Pubkey {
&self.1[index].stake_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 { fn contains_multiple_slots(&self) -> bool {
false false
} }
fn include_slot_in_hash(&self) -> IncludeSlotInHash {
self.2
}
} }
impl Bank { impl Bank {
@ -2957,7 +2960,7 @@ impl Bank {
// store stake account even if stakers_reward is 0 // store stake account even if stakers_reward is 0
// because credits observed has changed // because credits observed has changed
let mut m = Measure::start("store_stake_account"); 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(); m.stop();
metrics metrics
.store_stake_accounts_us .store_stake_accounts_us
@ -4801,6 +4804,7 @@ impl Bank {
&durable_nonce, &durable_nonce,
lamports_per_signature, lamports_per_signature,
self.preserve_rent_epoch_for_rent_exempt_accounts(), self.preserve_rent_epoch_for_rent_exempt_accounts(),
self.include_slot_in_hash(),
); );
let rent_debits = self.collect_rent(&execution_results, loaded_txs); 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( let (hash, measure) = measure!(crate::accounts_db::AccountsDb::hash_account(
self.slot(), self.slot(),
account, account,
pubkey pubkey,
self.include_slot_in_hash(),
)); ));
time_hashing_skipped_rewrites_us += measure.as_us(); time_hashing_skipped_rewrites_us += measure.as_us();
rewrites_skipped.push((*pubkey, hash)); rewrites_skipped.push((*pubkey, hash));
@ -5293,7 +5298,11 @@ impl Bank {
if !accounts_to_store.is_empty() { if !accounts_to_store.is_empty() {
// TODO: Maybe do not call `store_accounts()` here. Instead return `accounts_to_store` // TODO: Maybe do not call `store_accounts()` here. Instead return `accounts_to_store`
// and have `collect_rent_in_partition()` perform all the stores. // 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(); 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' /// convert 'partition' to a pubkey range and 'collect_rent_in_range'
fn collect_rent_in_partition( fn collect_rent_in_partition(
&self, &self,
@ -6169,7 +6190,11 @@ impl Bank {
pubkey: &Pubkey, pubkey: &Pubkey,
account: &T, 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>( pub fn store_accounts<'a, T: ReadableAccount + Sync + ZeroLamport>(

View File

@ -368,9 +368,12 @@ impl<'a> SnapshotMinimizer<'a> {
} }
let (new_storage, _time) = self.accounts_db().get_store_for_shrink(slot, aligned_total); let (new_storage, _time) = self.accounts_db().get_store_for_shrink(slot, aligned_total);
self.accounts_db().store_accounts_frozen( self.accounts_db().store_accounts_frozen(
(slot, &accounts[..]), (
slot,
&accounts[..],
crate::accounts_db::INCLUDE_SLOT_IN_HASH_IRRELEVANT_APPEND_VEC_OPERATION,
),
Some(&hashes), Some(&hashes),
Some(&new_storage), Some(&new_storage),
Some(Box::new(write_versions.into_iter())), Some(Box::new(write_versions.into_iter())),

View File

@ -1,5 +1,8 @@
//! trait for abstracting underlying storage of pubkey and account pairs to be written //! 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: /// abstract access to pubkey, account, slot, target_slot of either:
/// a. (slot, &[&Pubkey, &ReadableAccount]) /// a. (slot, &[&Pubkey, &ReadableAccount])
@ -24,6 +27,8 @@ pub trait StorableAccounts<'a, T: ReadableAccount + Sync>: Sync {
/// are there accounts from multiple slots /// are there accounts from multiple slots
/// only used for an assert /// only used for an assert
fn contains_multiple_slots(&self) -> bool; 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' /// 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, pub target_slot: Slot,
/// slot where accounts are currently stored /// slot where accounts are currently stored
pub old_slot: Slot, 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> { 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 { fn contains_multiple_slots(&self) -> bool {
false 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 { fn pubkey(&self, index: usize) -> &Pubkey {
self.1[index].0 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 { fn contains_multiple_slots(&self) -> bool {
false false
} }
fn include_slot_in_hash(&self) -> IncludeSlotInHash {
self.2
}
} }
/// this tuple contains slot info PER account /// this tuple contains slot info PER account
impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T> 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 { fn pubkey(&self, index: usize) -> &Pubkey {
self.1[index].0 self.1[index].0
@ -112,12 +129,16 @@ impl<'a, T: ReadableAccount + Sync> StorableAccounts<'a, T>
false false
} }
} }
fn include_slot_in_hash(&self) -> IncludeSlotInHash {
self.2
}
} }
#[cfg(test)] #[cfg(test)]
pub mod tests { pub mod tests {
use { use {
super::*, super::*,
crate::accounts_db::INCLUDE_SLOT_IN_HASH_TESTS,
solana_sdk::account::{AccountSharedData, WritableAccount}, solana_sdk::account::{AccountSharedData, WritableAccount},
}; };
@ -142,11 +163,13 @@ pub mod tests {
let test3 = ( let test3 = (
slot, slot,
&vec![(&pk, &account, slot), (&pk, &account, slot)][..], &vec![(&pk, &account, slot), (&pk, &account, slot)][..],
INCLUDE_SLOT_IN_HASH_TESTS,
); );
assert!(!test3.contains_multiple_slots()); assert!(!test3.contains_multiple_slots());
let test3 = ( let test3 = (
slot, slot,
&vec![(&pk, &account, slot), (&pk, &account, slot + 1)][..], &vec![(&pk, &account, slot), (&pk, &account, slot + 1)][..],
INCLUDE_SLOT_IN_HASH_TESTS,
); );
assert!(test3.contains_multiple_slots()); assert!(test3.contains_multiple_slots());
} }
@ -178,13 +201,14 @@ pub mod tests {
two.push((&raw.0, &raw.1)); // 2 item tuple two.push((&raw.0, &raw.1)); // 2 item tuple
three.push((&raw.0, &raw.1, raw.2)); // 3 item tuple, including slot three.push((&raw.0, &raw.1, raw.2)); // 3 item tuple, including slot
}); });
let test2 = (target_slot, &two[..]); let test2 = (target_slot, &two[..], INCLUDE_SLOT_IN_HASH_TESTS);
let test3 = (target_slot, &three[..]); let test3 = (target_slot, &three[..], INCLUDE_SLOT_IN_HASH_TESTS);
let old_slot = starting_slot; let old_slot = starting_slot;
let test_moving_slots = StorableAccountsMovingSlots { let test_moving_slots = StorableAccountsMovingSlots {
accounts: &two[..], accounts: &two[..],
target_slot, target_slot,
old_slot, old_slot,
include_slot_in_hash: INCLUDE_SLOT_IN_HASH_TESTS,
}; };
compare(&test2, &test3); compare(&test2, &test3);
compare(&test2, &test_moving_slots); compare(&test2, &test_moving_slots);

View File

@ -3,7 +3,7 @@ use {
rand::{thread_rng, Rng}, rand::{thread_rng, Rng},
rayon::prelude::*, rayon::prelude::*,
solana_runtime::{ solana_runtime::{
accounts_db::{AccountsDb, LoadHint}, accounts_db::{AccountsDb, LoadHint, INCLUDE_SLOT_IN_HASH_TESTS},
ancestors::Ancestors, ancestors::Ancestors,
}, },
solana_sdk::{ solana_sdk::{
@ -128,7 +128,7 @@ fn test_bad_bank_hash() {
assert_eq!( assert_eq!(
db.load_account_hash(&ancestors, key, None, LoadHint::Unspecified) db.load_account_hash(&ancestors, key, None, LoadHint::Unspecified)
.unwrap(), .unwrap(),
AccountsDb::hash_account(some_slot, *account, key) AccountsDb::hash_account(some_slot, *account, key, INCLUDE_SLOT_IN_HASH_TESTS)
); );
} }
existing.clear(); existing.clear();

View File

@ -490,6 +490,10 @@ pub mod disable_cpi_setting_executable_and_rent_epoch {
solana_sdk::declare_id!("B9cdB55u4jQsDNsdTK525yE9dmSc5Ga7YBaBrDFvEhM9"); 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 { pub mod relax_authority_signer_check_for_lookup_table_creation {
solana_sdk::declare_id!("FKAcEvNgSY79RpqsPNUV5gDyumopH4cEHqUxyfm8b8Ap"); 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"), (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_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"), (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"), (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"), (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"), (loosen_cpi_size_restriction::id(), "loosen cpi size restrictions #26641"),