blockworks-foundation
/
solana-with-rpc-optimizations
mirror of https://github.com/blockworks-foundation/solana-with-rpc-optimizations.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[TieredStorage] Deprecate the use of account-hash in HotStorage (#93) 

#### Problem
TieredStorage stores account hash as an optional field inside its HotStorage.
However, the field isn't used and we have already decided to deprecate
the account hash.

#### Summary of Changes
Remove account-hash from the tiered-storage.

#### Test Plan
Existing tiered-storage tests.
Running validators w/ tiered-storage in mainnet-beta w/o storing account-hash.
This commit is contained in:
Yueh-Hsuan Chiang 2024-03-06 18:51:50 -08:00 committed by GitHub
parent f968532d7f
commit 0bf9ec861a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 58 additions and 193 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
accounts-db/src/account_storage/meta.rs
View File

@ -128,7 +128,8 @@ impl<'storage> StoredAccountMeta<'storage> {
pub fn hash(&self) -> &'storage AccountHash {
match self {
Self::AppendVec(av) => av.hash(),
Self::Hot(hot) => hot.hash().unwrap_or(&DEFAULT_ACCOUNT_HASH),
// tiered-storage has deprecated the use of AccountHash
Self::Hot(_) => &DEFAULT_ACCOUNT_HASH,
}
}

8
accounts-db/src/tiered_storage.rs
View File

@ -362,15 +362,15 @@ mod tests {
let mut expected_accounts_map = HashMap::new();
for i in 0..num_accounts {
let (account, address, account_hash, _write_version) = storable_accounts.get(i);
expected_accounts_map.insert(address, (account, account_hash));
let (account, address, _account_hash, _write_version) = storable_accounts.get(i);
expected_accounts_map.insert(address, account);
}
let mut index_offset = IndexOffset(0);
let mut verified_accounts = HashSet::new();
while let Some((stored_meta, next)) = reader.get_account(index_offset).unwrap() {
if let Some((account, account_hash)) = expected_accounts_map.get(stored_meta.pubkey()) {
verify_test_account(&stored_meta, *account, stored_meta.pubkey(), account_hash);
if let Some(account) = expected_accounts_map.get(stored_meta.pubkey()) {
verify_test_account(&stored_meta, *account, stored_meta.pubkey());
verified_accounts.insert(stored_meta.pubkey());
}
index_offset = next;

25
accounts-db/src/tiered_storage/byte_block.rs
View File

@ -95,9 +95,6 @@ impl ByteBlockWriter {
if let Some(rent_epoch) = opt_fields.rent_epoch {
size += self.write_pod(&rent_epoch)?;
}
if let Some(hash) = opt_fields.account_hash {
size += self.write_pod(hash)?;
}
debug_assert_eq!(size, opt_fields.size());
@ -191,11 +188,7 @@ impl ByteBlockReader {
#[cfg(test)]
mod tests {
use {
super::*,
crate::accounts_hash::AccountHash,
solana_sdk::{hash::Hash, stake_history::Epoch},
};
use {super::*, solana_sdk::stake_history::Epoch};
fn read_type_unaligned<T>(buffer: &[u8], offset: usize) -> (T, usize) {
let size = std::mem::size_of::<T>();
@ -352,19 +345,13 @@ mod tests {
let mut writer = ByteBlockWriter::new(format);
let mut opt_fields_vec = vec![];
let mut some_count = 0;
let acc_hash = AccountHash(Hash::new_unique());
// prepare a vector of optional fields that contains all combinations
// of Some and None.
for rent_epoch in [None, Some(test_epoch)] {
for account_hash in [None, Some(&acc_hash)] {
some_count += rent_epoch.iter().count() + account_hash.iter().count();
some_count += rent_epoch.iter().count();
opt_fields_vec.push(AccountMetaOptionalFields {
rent_epoch,
account_hash,
});
}
opt_fields_vec.push(AccountMetaOptionalFields { rent_epoch });
test_epoch += 1;
}
@ -396,12 +383,6 @@ mod tests {
verified_count += 1;
offset += std::mem::size_of::<Epoch>();
}
if let Some(expected_hash) = opt_fields.account_hash {
let hash = read_pod::<AccountHash>(&decoded_buffer, offset).unwrap();
assert_eq!(hash, expected_hash);
verified_count += 1;
offset += std::mem::size_of::<AccountHash>();
}
}
// make sure the number of Some fields matches the number of fields we

60
accounts-db/src/tiered_storage/hot.rs
View File

@ -242,19 +242,6 @@ impl TieredAccountMeta for HotAccountMeta {
.flatten()
}
/// Returns the account hash by parsing the specified account block. None
/// will be returned if this account does not persist this optional field.
fn account_hash<'a>(&self, account_block: &'a [u8]) -> Option<&'a AccountHash> {
self.flags()
.has_account_hash()
.then(|| {
let offset = self.optional_fields_offset(account_block)
+ AccountMetaOptionalFields::account_hash_offset(self.flags());
byte_block::read_pod::<AccountHash>(account_block, offset)
})
.flatten()
}
/// Returns the offset of the optional fields based on the specified account
/// block.
fn optional_fields_offset(&self, account_block: &[u8]) -> usize {
@ -488,9 +475,6 @@ fn write_optional_fields(
if let Some(rent_epoch) = opt_fields.rent_epoch {
size += file.write_pod(&rent_epoch)?;
}
if let Some(hash) = opt_fields.account_hash {
size += file.write_pod(hash)?;
}
debug_assert_eq!(size, opt_fields.size());
@ -520,12 +504,8 @@ impl HotStorageWriter {
account_data: &[u8],
executable: bool,
rent_epoch: Option<Epoch>,
account_hash: Option<&AccountHash>,
) -> TieredStorageResult<usize> {
let optional_fields = AccountMetaOptionalFields {
rent_epoch,
account_hash,
};
let optional_fields = AccountMetaOptionalFields { rent_epoch };
let mut flags = AccountMetaFlags::new_from(&optional_fields);
flags.set_executable(executable);
@ -574,7 +554,7 @@ impl HotStorageWriter {
let total_input_accounts = len - skip;
let mut stored_infos = Vec::with_capacity(total_input_accounts);
for i in skip..len {
let (account, address, account_hash, _write_version) = accounts.get(i);
let (account, address, _account_hash, _write_version) = accounts.get(i);
let index_entry = AccountIndexWriterEntry {
address,
offset: HotAccountOffset::new(cursor)?,
@ -582,7 +562,7 @@ impl HotStorageWriter {
// Obtain necessary fields from the account, or default fields
// for a zero-lamport account in the None case.
let (lamports, owner, data, executable, rent_epoch, account_hash) = account
let (lamports, owner, data, executable, rent_epoch) = account
.map(|acc| {
(
acc.lamports(),
@ -591,19 +571,12 @@ impl HotStorageWriter {
acc.executable(),
// only persist rent_epoch for those rent-paying accounts
(acc.rent_epoch() != RENT_EXEMPT_RENT_EPOCH).then_some(acc.rent_epoch()),
Some(account_hash),
)
})
.unwrap_or((0, &OWNER_NO_OWNER, &[], false, None, None));
.unwrap_or((0, &OWNER_NO_OWNER, &[], false, None));
let owner_offset = owners_table.insert(owner);
let stored_size = self.write_account(
lamports,
owner_offset,
data,
executable,
rent_epoch,
account_hash,
)?;
let stored_size =
self.write_account(lamports, owner_offset, data, executable, rent_epoch)?;
cursor += stored_size;
stored_infos.push(StoredAccountInfo {
@ -755,11 +728,9 @@ pub mod tests {
const TEST_PADDING: u8 = 5;
const TEST_OWNER_OFFSET: OwnerOffset = OwnerOffset(0x1fef_1234);
const TEST_RENT_EPOCH: Epoch = 7;
let acc_hash = AccountHash(Hash::new_unique());
let optional_fields = AccountMetaOptionalFields {
rent_epoch: Some(TEST_RENT_EPOCH),
account_hash: Some(&acc_hash),
};
let flags = AccountMetaFlags::new_from(&optional_fields);
@ -779,7 +750,6 @@ pub mod tests {
fn test_hot_account_meta_full() {
let account_data = [11u8; 83];
let padding = [0u8; 5];
let acc_hash = AccountHash(Hash::new_unique());
const TEST_LAMPORT: u64 = 2314232137;
const OWNER_OFFSET: u32 = 0x1fef_1234;
@ -787,7 +757,6 @@ pub mod tests {
let optional_fields = AccountMetaOptionalFields {
rent_epoch: Some(TEST_RENT_EPOCH),
account_hash: Some(&acc_hash),
};
let flags = AccountMetaFlags::new_from(&optional_fields);
@ -810,7 +779,6 @@ pub mod tests {
let meta = byte_block::read_pod::<HotAccountMeta>(&buffer, 0).unwrap();
assert_eq!(expected_meta, *meta);
assert!(meta.flags().has_rent_epoch());
assert!(meta.flags().has_account_hash());
assert_eq!(meta.account_data_padding() as usize, padding.len());
let account_block = &buffer[std::mem::size_of::<HotAccountMeta>()..];
@ -823,10 +791,6 @@ pub mod tests {
assert_eq!(account_data.len(), meta.account_data_size(account_block));
assert_eq!(account_data, meta.account_data(account_block));
assert_eq!(meta.rent_epoch(account_block), optional_fields.rent_epoch);
assert_eq!(
(meta.account_hash(account_block).unwrap()),
optional_fields.account_hash.unwrap()
);
}
#[test]
@ -1334,8 +1298,8 @@ pub mod tests {
.unwrap()
.unwrap();
let (account, address, account_hash, _write_version) = storable_accounts.get(i);
verify_test_account(&stored_meta, account, address, account_hash);
let (account, address, _account_hash, _write_version) = storable_accounts.get(i);
verify_test_account(&stored_meta, account, address);
assert_eq!(i + 1, next.0 as usize);
}
@ -1352,9 +1316,9 @@ pub mod tests {
.unwrap()
.unwrap();
let (account, address, account_hash, _write_version) =
let (account, address, _account_hash, _write_version) =
storable_accounts.get(stored_info.offset);
verify_test_account(&stored_meta, account, address, account_hash);
verify_test_account(&stored_meta, account, address);
}
// verify get_accounts
@ -1362,8 +1326,8 @@ pub mod tests {
// first, we verify everything
for (i, stored_meta) in accounts.iter().enumerate() {
let (account, address, account_hash, _write_version) = storable_accounts.get(i);
verify_test_account(stored_meta, account, address, account_hash);
let (account, address, _account_hash, _write_version) = storable_accounts.get(i);
verify_test_account(stored_meta, account, address);
}
// second, we verify various initial position

128
accounts-db/src/tiered_storage/meta.rs
View File

@ -1,7 +1,7 @@
//! The account meta and related structs for the tiered storage.
use {
crate::{accounts_hash::AccountHash, tiered_storage::owners::OwnerOffset},
crate::tiered_storage::owners::OwnerOffset,
bytemuck::{Pod, Zeroable},
modular_bitfield::prelude::*,
solana_sdk::stake_history::Epoch,
@ -14,12 +14,10 @@ use {
pub struct AccountMetaFlags {
/// whether the account meta has rent epoch
pub has_rent_epoch: bool,
/// whether the account meta has account hash
pub has_account_hash: bool,
/// whether the account is executable
pub executable: bool,
/// the reserved bits.
reserved: B29,
reserved: B30,
}
// Ensure there are no implicit padding bytes
@ -70,10 +68,6 @@ pub trait TieredAccountMeta: Sized {
/// does not persist this optional field.
fn rent_epoch(&self, _account_block: &[u8]) -> Option<Epoch>;
/// Returns the account hash by parsing the specified account block. None
/// will be returned if this account does not persist this optional field.
fn account_hash<'a>(&self, _account_block: &'a [u8]) -> Option<&'a AccountHash>;
/// Returns the offset of the optional fields based on the specified account
/// block.
fn optional_fields_offset(&self, _account_block: &[u8]) -> usize;
@ -91,7 +85,6 @@ impl AccountMetaFlags {
pub fn new_from(optional_fields: &AccountMetaOptionalFields) -> Self {
let mut flags = AccountMetaFlags::default();
flags.set_has_rent_epoch(optional_fields.rent_epoch.is_some());
flags.set_has_account_hash(optional_fields.account_hash.is_some());
flags.set_executable(false);
flags
}
@ -102,20 +95,15 @@ impl AccountMetaFlags {
/// Note that the storage representation of the optional fields might be
/// different from its in-memory representation.
#[derive(Debug, PartialEq, Eq, Clone)]
pub struct AccountMetaOptionalFields<'a> {
pub struct AccountMetaOptionalFields {
/// the epoch at which its associated account will next owe rent
pub rent_epoch: Option<Epoch>,
/// the hash of its associated account
pub account_hash: Option<&'a AccountHash>,
}
impl<'a> AccountMetaOptionalFields<'a> {
impl AccountMetaOptionalFields {
/// The size of the optional fields in bytes (excluding the boolean flags).
pub fn size(&self) -> usize {
self.rent_epoch.map_or(0, |_| std::mem::size_of::<Epoch>())
+ self
.account_hash
.map_or(0, |_| std::mem::size_of::<AccountHash>())
}
/// Given the specified AccountMetaFlags, returns the size of its
@ -125,9 +113,6 @@ impl<'a> AccountMetaOptionalFields<'a> {
if flags.has_rent_epoch() {
fields_size += std::mem::size_of::<Epoch>();
}
if flags.has_account_hash() {
fields_size += std::mem::size_of::<AccountHash>();
}
fields_size
}
@ -137,29 +122,17 @@ impl<'a> AccountMetaOptionalFields<'a> {
pub fn rent_epoch_offset(_flags: &AccountMetaFlags) -> usize {
0
}
/// Given the specified AccountMetaFlags, returns the relative offset
/// of its account_hash field to the offset of its optional fields entry.
pub fn account_hash_offset(flags: &AccountMetaFlags) -> usize {
let mut offset = Self::rent_epoch_offset(flags);
// rent_epoch is the previous field to account hash
if flags.has_rent_epoch() {
offset += std::mem::size_of::<Epoch>();
}
offset
}
}
#[cfg(test)]
pub mod tests {
use {super::*, solana_sdk::hash::Hash};
use super::*;
#[test]
fn test_account_meta_flags_new() {
let flags = AccountMetaFlags::new();
assert!(!flags.has_rent_epoch());
assert!(!flags.has_account_hash());
assert_eq!(flags.reserved(), 0u32);
assert_eq!(
@ -179,20 +152,11 @@ pub mod tests {
flags.set_has_rent_epoch(true);
assert!(flags.has_rent_epoch());
assert!(!flags.has_account_hash());
assert!(!flags.executable());
verify_flags_serialization(&flags);
flags.set_has_account_hash(true);
assert!(flags.has_rent_epoch());
assert!(flags.has_account_hash());
assert!(!flags.executable());
verify_flags_serialization(&flags);
flags.set_executable(true);
assert!(flags.has_rent_epoch());
assert!(flags.has_account_hash());
assert!(flags.executable());
verify_flags_serialization(&flags);
@ -203,84 +167,58 @@ pub mod tests {
fn update_and_verify_flags(opt_fields: &AccountMetaOptionalFields) {
let flags: AccountMetaFlags = AccountMetaFlags::new_from(opt_fields);
assert_eq!(flags.has_rent_epoch(), opt_fields.rent_epoch.is_some());
assert_eq!(flags.has_account_hash(), opt_fields.account_hash.is_some());
assert_eq!(flags.reserved(), 0u32);
}
#[test]
fn test_optional_fields_update_flags() {
let test_epoch = 5432312;
let acc_hash = AccountHash(Hash::new_unique());
for rent_epoch in [None, Some(test_epoch)] {
for account_hash in [None, Some(&acc_hash)] {
update_and_verify_flags(&AccountMetaOptionalFields {
rent_epoch,
account_hash,
});
}
update_and_verify_flags(&AccountMetaOptionalFields { rent_epoch });
}
}
#[test]
fn test_optional_fields_size() {
let test_epoch = 5432312;
let acc_hash = AccountHash(Hash::new_unique());
for rent_epoch in [None, Some(test_epoch)] {
for account_hash in [None, Some(&acc_hash)] {
let opt_fields = AccountMetaOptionalFields {
rent_epoch,
account_hash,
};
assert_eq!(
opt_fields.size(),
rent_epoch.map_or(0, |_| std::mem::size_of::<Epoch>())
+ account_hash.map_or(0, |_| std::mem::size_of::<AccountHash>())
);
assert_eq!(
opt_fields.size(),
AccountMetaOptionalFields::size_from_flags(&AccountMetaFlags::new_from(
&opt_fields
))
);
}
let opt_fields = AccountMetaOptionalFields { rent_epoch };
assert_eq!(
opt_fields.size(),
rent_epoch.map_or(0, |_| std::mem::size_of::<Epoch>()),
);
assert_eq!(
opt_fields.size(),
AccountMetaOptionalFields::size_from_flags(&AccountMetaFlags::new_from(
&opt_fields
))
);
}
}
#[test]
fn test_optional_fields_offset() {
let test_epoch = 5432312;
let acc_hash = AccountHash(Hash::new_unique());
for rent_epoch in [None, Some(test_epoch)] {
for account_hash in [None, Some(&acc_hash)] {
let rent_epoch_offset = 0;
let account_hash_offset =
rent_epoch_offset + rent_epoch.as_ref().map(std::mem::size_of_val).unwrap_or(0);
let derived_size = account_hash_offset
+ account_hash
.as_ref()
.map(|acc_hash| std::mem::size_of_val(*acc_hash))
.unwrap_or(0);
let opt_fields = AccountMetaOptionalFields {
rent_epoch,
account_hash,
};
let flags = AccountMetaFlags::new_from(&opt_fields);
assert_eq!(
AccountMetaOptionalFields::rent_epoch_offset(&flags),
rent_epoch_offset
);
assert_eq!(
AccountMetaOptionalFields::account_hash_offset(&flags),
account_hash_offset
);
assert_eq!(
AccountMetaOptionalFields::size_from_flags(&flags),
derived_size
);
}
let rent_epoch_offset = 0;
let derived_size = if rent_epoch.is_some() {
std::mem::size_of::<Epoch>()
} else {
0
};
let opt_fields = AccountMetaOptionalFields { rent_epoch };
let flags = AccountMetaFlags::new_from(&opt_fields);
assert_eq!(
AccountMetaOptionalFields::rent_epoch_offset(&flags),
rent_epoch_offset
);
assert_eq!(
AccountMetaOptionalFields::size_from_flags(&flags),
derived_size
);
}
}
}

6
accounts-db/src/tiered_storage/readable.rs
View File

@ -2,7 +2,6 @@ use {
crate::{
account_storage::meta::StoredAccountMeta,
accounts_file::MatchAccountOwnerError,
accounts_hash::AccountHash,
tiered_storage::{
footer::{AccountMetaFormat, TieredStorageFooter},
hot::HotStorageReader,
@ -40,11 +39,6 @@ impl<'accounts_file, M: TieredAccountMeta> TieredReadableAccount<'accounts_file,
self.address
}
/// Returns the hash of this account.
pub fn hash(&self) -> Option<&'accounts_file AccountHash> {
self.meta.account_hash(self.account_block)
}
/// Returns the index to this account in its AccountsFile.
pub fn index(&self) -> IndexOffset {
self.index

21
accounts-db/src/tiered_storage/test_utils.rs
View File

@ -48,20 +48,10 @@ pub(super) fn verify_test_account(
stored_meta: &StoredAccountMeta<'_>,
account: Option<&impl ReadableAccount>,
address: &Pubkey,
account_hash: &AccountHash,
) {
let (lamports, owner, data, executable, account_hash) = account
.map(|acc| {
(
acc.lamports(),
acc.owner(),
acc.data(),
acc.executable(),
// only persist rent_epoch for those rent-paying accounts
Some(*account_hash),
)
})
.unwrap_or((0, &OWNER_NO_OWNER, &[], false, None));
let (lamports, owner, data, executable) = account
.map(|acc| (acc.lamports(), acc.owner(), acc.data(), acc.executable()))
.unwrap_or((0, &OWNER_NO_OWNER, &[], false));
assert_eq!(stored_meta.lamports(), lamports);
assert_eq!(stored_meta.data().len(), data.len());
@ -69,8 +59,5 @@ pub(super) fn verify_test_account(
assert_eq!(stored_meta.executable(), executable);
assert_eq!(stored_meta.owner(), owner);
assert_eq!(stored_meta.pubkey(), address);
assert_eq!(
*stored_meta.hash(),
account_hash.unwrap_or(AccountHash(Hash::default()))
);
assert_eq!(*stored_meta.hash(), AccountHash(Hash::default()));
}