Skip to content

Commit

Permalink
[TieredStorage] Deprecate the use of account-hash in HotStorage (#93)
Browse files Browse the repository at this point in the history 
#### 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.
  • Loading branch information
@yhchiang-sol @willhickey
yhchiang-sol authored and willhickey committed Mar 9, 2024
1 parent a8d527b commit 782591e
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 193 deletions.
3 changes: 2 additions & 1 deletion accounts-db/src/account_storage/meta.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
8 changes: 4 additions & 4 deletions accounts-db/src/tiered_storage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
25 changes: 3 additions & 22 deletions accounts-db/src/tiered_storage/byte_block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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>();
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
60 changes: 12 additions & 48 deletions accounts-db/src/tiered_storage/hot.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -574,15 +554,15 @@ 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)?,
};

// 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(),
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand All @@ -779,15 +750,13 @@ 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;
const TEST_RENT_EPOCH: Epoch = 7;

let optional_fields = AccountMetaOptionalFields {
rent_epoch: Some(TEST_RENT_EPOCH),
account_hash: Some(&acc_hash),
};

let flags = AccountMetaFlags::new_from(&optional_fields);
Expand All @@ -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>()..];
Expand All @@ -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]
Expand Down Expand Up @@ -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);
}
Expand All @@ -1352,18 +1316,18 @@ 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
let accounts = hot_storage.accounts(IndexOffset(0)).unwrap();

// 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
Expand Down
Loading

0 comments on commit 782591e

Please sign in to comment.