diff --git a/accounts-db/src/tiered_storage/error.rs b/accounts-db/src/tiered_storage/error.rs index 145334574b4ea3..9295d43632cc64 100644 --- a/accounts-db/src/tiered_storage/error.rs +++ b/accounts-db/src/tiered_storage/error.rs @@ -1,4 +1,7 @@ -use {super::footer::SanitizeFooterError, std::path::PathBuf, thiserror::Error}; +use { + super::footer::SanitizeFooterError, solana_sdk::hash::Hash, std::path::PathBuf, + thiserror::Error, +}; #[derive(Error, Debug)] pub enum TieredStorageError { @@ -31,4 +34,7 @@ pub enum TieredStorageError { #[error("OffsetAlignmentError: offset {0} must be multiple of {1}")] OffsetAlignmentError(usize, usize), + + #[error("FileHashMismatch: {0} {1}")] + FileHashMismatch(Hash, Hash), } diff --git a/accounts-db/src/tiered_storage/file.rs b/accounts-db/src/tiered_storage/file.rs index e6ea4a7c65d15d..3c8765e877138f 100644 --- a/accounts-db/src/tiered_storage/file.rs +++ b/accounts-db/src/tiered_storage/file.rs @@ -1,6 +1,7 @@ use { - super::{error::TieredStorageError, TieredStorageResult}, + super::{error::TieredStorageError, footer::FOOTER_TAIL_SIZE, TieredStorageResult}, bytemuck::{AnyBitPattern, NoUninit, Pod, Zeroable}, + solana_sdk::hash::Hash, std::{ fs::{File, OpenOptions}, io::{BufWriter, Read, Result as IoResult, Seek, SeekFrom, Write}, @@ -38,17 +39,37 @@ impl TieredReadableFile { ); file.check_magic_number()?; + file.check_file_hash()?; Ok(file) } - pub fn new_writable(file_path: impl AsRef) -> IoResult { - Ok(Self( - OpenOptions::new() - .create_new(true) - .write(true) - .open(file_path)?, - )) + fn check_file_hash(&self) -> TieredStorageResult<()> { + self.seek(0)?; + + let len = self.0.metadata()?.len() as usize; + let hashed_len = len - std::mem::size_of::() - FOOTER_TAIL_SIZE; + let mut hasher = blake3::Hasher::new(); + + const BLOCK_SIZE: usize = 4096; + let mut buffer = [0u8; BLOCK_SIZE]; + let mut offset = 0; + while offset < hashed_len { + let block_size = std::cmp::min(BLOCK_SIZE, hashed_len - offset); + self.read_bytes(&mut buffer[0..block_size])?; + hasher.update(&buffer[0..block_size]); + + offset += block_size; + } + let hash = Hash::new_from_array(hasher.finalize().into()); + + let mut raw_hash_from_file = [0u8; 32]; + self.read_bytes(&mut raw_hash_from_file)?; + let hash_from_file = Hash::new_from_array(raw_hash_from_file); + + (hash == hash_from_file) + .then(|| Ok(())) + .unwrap_or_else(|| Err(TieredStorageError::FileHashMismatch(hash, hash_from_file))) } fn check_magic_number(&self) -> TieredStorageResult<()> { @@ -105,16 +126,22 @@ impl TieredReadableFile { } #[derive(Debug)] -pub struct TieredWritableFile(pub BufWriter); +pub struct TieredWritableFile { + file: BufWriter, + hasher: blake3::Hasher, +} impl TieredWritableFile { pub fn new(file_path: impl AsRef) -> IoResult { - Ok(Self(BufWriter::new( - OpenOptions::new() - .create_new(true) - .write(true) - .open(file_path)?, - ))) + Ok(Self { + file: BufWriter::new( + OpenOptions::new() + .create_new(true) + .write(true) + .open(file_path)?, + ), + hasher: blake3::Hasher::new(), + }) } /// Writes `value` to the file. @@ -142,18 +169,23 @@ impl TieredWritableFile { } pub fn seek(&mut self, offset: u64) -> IoResult { - self.0.seek(SeekFrom::Start(offset)) + self.file.seek(SeekFrom::Start(offset)) } pub fn seek_from_end(&mut self, offset: i64) -> IoResult { - self.0.seek(SeekFrom::End(offset)) + self.file.seek(SeekFrom::End(offset)) } pub fn write_bytes(&mut self, bytes: &[u8]) -> IoResult { - self.0.write_all(bytes)?; + self.file.write_all(bytes)?; + self.hasher.update(bytes); Ok(bytes.len()) } + + pub fn hash(&self) -> Hash { + Hash::new_from_array(self.hasher.finalize().into()) + } } #[cfg(test)] @@ -161,14 +193,47 @@ mod tests { use { crate::tiered_storage::{ error::TieredStorageError, - file::{TieredReadableFile, TieredWritableFile, FILE_MAGIC_NUMBER}, + file::{ + TieredReadableFile, TieredStorageMagicNumber, TieredWritableFile, FILE_MAGIC_NUMBER, + }, + footer::TieredStorageFooter, }, std::path::Path, tempfile::TempDir, }; + // Only write the footer and update the hash without writing the magic number + fn write_footer_only(footer: &mut TieredStorageFooter, file: &mut TieredWritableFile) { + // SAFETY: The footer does not contain any uninitialized bytes. + unsafe { + file.write_type(&footer.account_meta_format).unwrap(); + file.write_type(&footer.owners_block_format).unwrap(); + file.write_type(&footer.index_block_format).unwrap(); + file.write_type(&footer.account_block_format).unwrap(); + } + + file.write_pod(&footer.account_entry_count).unwrap(); + file.write_pod(&footer.account_meta_entry_size).unwrap(); + file.write_pod(&footer.account_block_size).unwrap(); + file.write_pod(&footer.owner_count).unwrap(); + file.write_pod(&footer.owner_entry_size).unwrap(); + file.write_pod(&footer.index_block_offset).unwrap(); + file.write_pod(&footer.owners_block_offset).unwrap(); + file.write_pod(&footer.min_account_address).unwrap(); + file.write_pod(&footer.max_account_address).unwrap(); + + // everything before the FooterTail will be hashed + footer.hash = file.hash(); + file.write_pod(&footer.hash).unwrap(); + + file.write_pod(&footer.format_version).unwrap(); + file.write_pod(&footer.footer_size).unwrap(); + } + fn generate_test_file_with_number(path: impl AsRef, number: u64) { let mut file = TieredWritableFile::new(path).unwrap(); + let mut footer = TieredStorageFooter::default(); + write_footer_only(&mut footer, &mut file); file.write_pod(&number).unwrap(); } @@ -190,4 +255,56 @@ mod tests { Err(TieredStorageError::MagicNumberMismatch(_, _)) )); } + + #[test] + fn test_file_hash() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().join("test_file_hash_mismatch"); + let mut expected_footer = TieredStorageFooter { + account_entry_count: 300, + account_meta_entry_size: 24, + account_block_size: 4096, + owner_count: 250, + owner_entry_size: 32, + index_block_offset: 1069600, + owners_block_offset: 1081200, + ..TieredStorageFooter::default() + }; + + // Manually persist the footer without updating the hash + { + let mut file = TieredWritableFile::new(&path).unwrap(); + // SAFETY: the footer does not contain any uninitialized bytes + unsafe { + file.write_type(&expected_footer).unwrap(); + } + file.write_pod(&TieredStorageMagicNumber::default()) + .unwrap(); + } + + // Reopen the same storage and expect FileHashMismatch + { + let result = TieredReadableFile::new(&path); + assert!(matches!( + result, + Err(TieredStorageError::FileHashMismatch(_, _)) + )); + } + + // Rewrite the same footer into a different file, but this time using + // the standard way to persist the footer that will also update the + // file hash. + let path = temp_dir.path().join("test_file_hash"); + { + let mut file = TieredWritableFile::new(&path).unwrap(); + expected_footer.write_footer_block(&mut file).unwrap() + } + + // Reopen the same storage and expect the footer matches + { + let file = TieredReadableFile::new(&path).unwrap(); + let footer = TieredStorageFooter::new_from_footer_block(&file).unwrap(); + assert_eq!(expected_footer, footer); + } + } } diff --git a/accounts-db/src/tiered_storage/footer.rs b/accounts-db/src/tiered_storage/footer.rs index 89e671d121cce6..57942c93bebb4e 100644 --- a/accounts-db/src/tiered_storage/footer.rs +++ b/accounts-db/src/tiered_storage/footer.rs @@ -15,7 +15,7 @@ use { thiserror::Error, }; -pub const FOOTER_FORMAT_VERSION: u64 = 1; +pub const FOOTER_FORMAT_VERSION: u64 = 2; /// The size of the footer struct + the magic number at the end. pub const FOOTER_SIZE: usize = @@ -159,9 +159,9 @@ impl Default for TieredStorageFooter { owner_entry_size: 0, index_block_offset: 0, owners_block_offset: 0, - hash: Hash::new_unique(), min_account_address: Pubkey::default(), max_account_address: Pubkey::default(), + hash: Hash::default(), format_version: FOOTER_FORMAT_VERSION, footer_size: FOOTER_SIZE as u64, } @@ -174,9 +174,33 @@ impl TieredStorageFooter { Self::new_from_footer_block(&file) } - pub fn write_footer_block(&self, file: &mut TieredWritableFile) -> TieredStorageResult<()> { + /// Presist the footer to the specified TieredStorageWritableFile. + /// The `hash` field of the footer will also be updated. + pub fn write_footer_block(&mut self, file: &mut TieredWritableFile) -> TieredStorageResult<()> { // SAFETY: The footer does not contain any uninitialized bytes. - unsafe { file.write_type(self)? }; + unsafe { + file.write_type(&self.account_meta_format)?; + file.write_type(&self.owners_block_format)?; + file.write_type(&self.index_block_format)?; + file.write_type(&self.account_block_format)?; + } + + file.write_pod(&self.account_entry_count)?; + file.write_pod(&self.account_meta_entry_size)?; + file.write_pod(&self.account_block_size)?; + file.write_pod(&self.owner_count)?; + file.write_pod(&self.owner_entry_size)?; + file.write_pod(&self.index_block_offset)?; + file.write_pod(&self.owners_block_offset)?; + file.write_pod(&self.min_account_address)?; + file.write_pod(&self.max_account_address)?; + + // everything before the FooterTail will be hashed + self.hash = file.hash(); + file.write_pod(&self.hash)?; + + file.write_pod(&self.format_version)?; + file.write_pod(&self.footer_size)?; file.write_pod(&TieredStorageMagicNumber::default())?; Ok(()) @@ -313,13 +337,12 @@ mod tests { append_vec::test_utils::get_append_vec_path, tiered_storage::file::TieredWritableFile, }, memoffset::offset_of, - solana_sdk::hash::Hash, }; #[test] fn test_footer() { let path = get_append_vec_path("test_file_footer"); - let expected_footer = TieredStorageFooter { + let mut expected_footer = TieredStorageFooter { account_meta_format: AccountMetaFormat::Hot, owners_block_format: OwnersBlockFormat::AddressesOnly, index_block_format: IndexBlockFormat::AddressesThenOffsets, @@ -331,12 +354,15 @@ mod tests { owner_entry_size: 32, index_block_offset: 1069600, owners_block_offset: 1081200, - hash: Hash::new_unique(), min_account_address: Pubkey::default(), max_account_address: Pubkey::new_unique(), format_version: FOOTER_FORMAT_VERSION, footer_size: FOOTER_SIZE as u64, + ..TieredStorageFooter::default() }; + // Copy the original footer as the we will update the hash field of + // the expected footer. + let raw_footer = expected_footer; // Persist the expected footer. { @@ -349,6 +375,29 @@ mod tests { { let footer = TieredStorageFooter::new_from_path(&path.path).unwrap(); assert_eq!(expected_footer, footer); + + // Expect everything is the same as the original footer except the hash + assert_eq!(footer.account_meta_format, raw_footer.account_meta_format); + assert_eq!(footer.owners_block_format, raw_footer.owners_block_format); + assert_eq!(footer.index_block_format, raw_footer.index_block_format); + assert_eq!(footer.account_block_format, raw_footer.account_block_format); + assert_eq!(footer.account_entry_count, raw_footer.account_entry_count); + assert_eq!( + footer.account_meta_entry_size, + raw_footer.account_meta_entry_size + ); + assert_eq!(footer.account_block_size, raw_footer.account_block_size); + assert_eq!(footer.owner_count, raw_footer.owner_count); + assert_eq!(footer.owner_entry_size, raw_footer.owner_entry_size); + assert_eq!(footer.index_block_offset, raw_footer.index_block_offset); + assert_eq!(footer.owners_block_offset, raw_footer.owners_block_offset); + assert_eq!(footer.min_account_address, raw_footer.min_account_address); + assert_eq!(footer.max_account_address, raw_footer.max_account_address); + assert_eq!(footer.format_version, raw_footer.format_version); + assert_eq!(footer.footer_size, raw_footer.footer_size); + + // Expect hash mismatch as the footer should include an updated hash + assert_ne!(footer.hash, raw_footer.hash); } } diff --git a/accounts-db/src/tiered_storage/hot.rs b/accounts-db/src/tiered_storage/hot.rs index 260548897f66e2..67bba4e83f83f2 100644 --- a/accounts-db/src/tiered_storage/hot.rs +++ b/accounts-db/src/tiered_storage/hot.rs @@ -555,34 +555,34 @@ impl HotStorageReader { } } -fn write_optional_fields( - file: &mut TieredWritableFile, - opt_fields: &AccountMetaOptionalFields, -) -> TieredStorageResult { - let mut size = 0; - if let Some(rent_epoch) = opt_fields.rent_epoch { - size += file.write_pod(&rent_epoch)?; - } - - debug_assert_eq!(size, opt_fields.size()); - - Ok(size) -} - /// The writer that creates a hot accounts file. #[derive(Debug)] pub struct HotStorageWriter { - storage: TieredWritableFile, + file: TieredWritableFile, } impl HotStorageWriter { /// Create a new HotStorageWriter with the specified path. pub fn new(file_path: impl AsRef) -> TieredStorageResult { Ok(Self { - storage: TieredWritableFile::new(file_path)?, + file: TieredWritableFile::new(file_path)?, }) } + fn write_optional_fields( + &mut self, + opt_fields: &AccountMetaOptionalFields, + ) -> TieredStorageResult { + let mut size = 0; + if let Some(rent_epoch) = opt_fields.rent_epoch { + size += self.file.write_pod(&rent_epoch)?; + } + + debug_assert_eq!(size, opt_fields.size()); + + Ok(size) + } + /// Persists an account with the specified information and returns /// the stored size of the account. fn write_account( @@ -608,12 +608,12 @@ impl HotStorageWriter { let mut stored_size = 0; - stored_size += self.storage.write_pod(&meta)?; - stored_size += self.storage.write_bytes(account_data)?; + stored_size += self.file.write_pod(&meta)?; + stored_size += self.file.write_bytes(account_data)?; stored_size += self - .storage + .file .write_bytes(&PADDING_BUFFER[0..(padding_len as usize)])?; - stored_size += write_optional_fields(&mut self.storage, &optional_fields)?; + stored_size += self.write_optional_fields(&optional_fields)?; Ok(stored_size) } @@ -693,13 +693,13 @@ impl HotStorageWriter { footer.index_block_offset = cursor as u64; cursor += footer .index_block_format - .write_index_block(&mut self.storage, &index)?; + .write_index_block(&mut self.file, &index)?; if cursor % HOT_BLOCK_ALIGNMENT != 0 { // In case it is not yet aligned, it is due to the fact that // the index block has an odd number of entries. In such case, // we expect the amount off is equal to 4. assert_eq!(cursor % HOT_BLOCK_ALIGNMENT, 4); - cursor += self.storage.write_pod(&0u32)?; + cursor += self.file.write_pod(&0u32)?; } // writing owners block @@ -708,10 +708,10 @@ impl HotStorageWriter { footer.owner_count = owners_table.len() as u32; footer .owners_block_format - .write_owners_block(&mut self.storage, &owners_table)?; + .write_owners_block(&mut self.file, &owners_table)?; footer.min_account_address = *address_range.min; footer.max_account_address = *address_range.max; - footer.write_footer_block(&mut self.storage)?; + footer.write_footer_block(&mut self.file)?; Ok(stored_infos) } @@ -724,7 +724,10 @@ pub mod tests { crate::tiered_storage::{ byte_block::ByteBlockWriter, file::{TieredStorageMagicNumber, TieredWritableFile}, - footer::{AccountBlockFormat, AccountMetaFormat, TieredStorageFooter, FOOTER_SIZE}, + footer::{ + AccountBlockFormat, AccountMetaFormat, TieredStorageFooter, FOOTER_FORMAT_VERSION, + FOOTER_SIZE, + }, hot::{HotAccountMeta, HotStorageReader}, index::{AccountIndexWriterEntry, IndexBlockFormat, IndexOffset}, meta::{AccountMetaFlags, AccountMetaOptionalFields, TieredAccountMeta}, @@ -732,6 +735,7 @@ pub mod tests { test_utils::{create_test_account, verify_test_account}, }, assert_matches::assert_matches, + blake3::traits::digest::Digest, memoffset::offset_of, rand::{seq::SliceRandom, Rng}, solana_sdk::{ @@ -889,7 +893,7 @@ pub mod tests { // Generate a new temp path that is guaranteed to NOT already have a file. let temp_dir = TempDir::new().unwrap(); let path = temp_dir.path().join("test_hot_storage_footer"); - let expected_footer = TieredStorageFooter { + let mut expected_footer = TieredStorageFooter { account_meta_format: AccountMetaFormat::Hot, owners_block_format: OwnersBlockFormat::AddressesOnly, index_block_format: IndexBlockFormat::AddressesThenOffsets, @@ -901,11 +905,11 @@ pub mod tests { owner_entry_size: 32, index_block_offset: 1069600, owners_block_offset: 1081200, - hash: Hash::new_unique(), + hash: Hash::new_from_array(blake3::Hasher::new().finalize().into()), min_account_address: Pubkey::default(), max_account_address: Pubkey::new_unique(), footer_size: FOOTER_SIZE as u64, - format_version: 1, + format_version: FOOTER_FORMAT_VERSION, }; { @@ -983,7 +987,7 @@ pub mod tests { .path() .join("test_get_acount_meta_from_offset_out_of_bounds"); - let footer = TieredStorageFooter { + let mut footer = TieredStorageFooter { account_meta_format: AccountMetaFormat::Hot, index_block_offset: 160, ..TieredStorageFooter::default() @@ -1072,7 +1076,7 @@ pub mod tests { .take(NUM_OWNERS) .collect(); - let footer = TieredStorageFooter { + let mut footer = TieredStorageFooter { account_meta_format: AccountMetaFormat::Hot, // meta/data nor index block in this test owners_block_offset: 0, @@ -1300,7 +1304,6 @@ pub mod tests { .owners_block_format .write_owners_block(&mut file, &owners_table) .unwrap(); - footer.write_footer_block(&mut file).unwrap(); } @@ -1332,11 +1335,11 @@ pub mod tests { } #[test] - fn test_hot_storage_writer_twice_on_same_path() { + fn test_hot_storage_file_twice_on_same_path() { let temp_dir = TempDir::new().unwrap(); let path = temp_dir .path() - .join("test_hot_storage_writer_twice_on_same_path"); + .join("test_hot_storage_file_twice_on_same_path"); // Expect the first returns Ok assert_matches!(HotStorageWriter::new(&path), Ok(_)); diff --git a/accounts-db/src/tiered_storage/index.rs b/accounts-db/src/tiered_storage/index.rs index 82dbb9332c7550..f0b4551aa2f557 100644 --- a/accounts-db/src/tiered_storage/index.rs +++ b/accounts-db/src/tiered_storage/index.rs @@ -216,7 +216,7 @@ mod tests { .path() .join("test_get_account_address_out_of_bounds"); - let footer = TieredStorageFooter { + let mut footer = TieredStorageFooter { account_entry_count: 100, index_block_format: IndexBlockFormat::AddressesThenOffsets, ..TieredStorageFooter::default() @@ -249,7 +249,7 @@ mod tests { .path() .join("test_get_account_address_exceeds_index_block_boundary"); - let footer = TieredStorageFooter { + let mut footer = TieredStorageFooter { account_entry_count: 100, index_block_format: IndexBlockFormat::AddressesThenOffsets, index_block_offset: 1024, @@ -287,7 +287,7 @@ mod tests { .path() .join("test_get_account_offset_out_of_bounds"); - let footer = TieredStorageFooter { + let mut footer = TieredStorageFooter { account_entry_count: 100, index_block_format: IndexBlockFormat::AddressesThenOffsets, ..TieredStorageFooter::default() @@ -324,7 +324,7 @@ mod tests { .path() .join("test_get_account_offset_exceeds_index_block_boundary"); - let footer = TieredStorageFooter { + let mut footer = TieredStorageFooter { account_entry_count: 100, index_block_format: IndexBlockFormat::AddressesThenOffsets, index_block_offset: 1024, diff --git a/accounts-db/src/tiered_storage/owners.rs b/accounts-db/src/tiered_storage/owners.rs index fa42ffaca97dac..92bac3dc8d1bd2 100644 --- a/accounts-db/src/tiered_storage/owners.rs +++ b/accounts-db/src/tiered_storage/owners.rs @@ -131,7 +131,7 @@ mod tests { .take(NUM_OWNERS as usize) .collect(); - let footer = TieredStorageFooter { + let mut footer = TieredStorageFooter { // Set owners_block_offset to 0 as we didn't write any account // meta/data nor index block. owners_block_offset: 0,