From 37507a2de636efec8bec0b38e794e22230c8d183 Mon Sep 17 00:00:00 2001 From: Brooks Prumo Date: Mon, 31 Oct 2022 09:43:17 -0400 Subject: [PATCH] Removes EAH parameter from serde_snapshot::reserialize_bank() (#28669) --- core/src/accounts_hash_verifier.rs | 1 - core/tests/snapshots.rs | 3 -- runtime/src/serde_snapshot.rs | 5 --- runtime/src/serde_snapshot/newer.rs | 3 +- runtime/src/serde_snapshot/tests.rs | 49 ++++++++--------------------- runtime/src/snapshot_utils.rs | 2 -- 6 files changed, 14 insertions(+), 49 deletions(-) diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index ae315ce49b..483fca6a22 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -303,7 +303,6 @@ impl AccountsHashVerifier { accounts_package.slot, &accounts_hash, None, - None, ); datapoint_info!( "accounts_hash_verifier", diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 48410863d3..3c05e079ec 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -280,7 +280,6 @@ fn run_bank_forks_snapshot_n( accounts_package.slot, &last_bank.get_accounts_hash(), None, - None, ); let snapshot_package = SnapshotPackage::new(accounts_package, last_bank.get_accounts_hash()); snapshot_utils::archive_snapshot_package( @@ -518,7 +517,6 @@ fn test_concurrent_snapshot_packaging( accounts_package.slot, &Hash::default(), None, - None, ); let snapshot_package = SnapshotPackage::new(accounts_package, Hash::default()); pending_snapshot_package @@ -563,7 +561,6 @@ fn test_concurrent_snapshot_packaging( saved_slot, &Hash::default(), None, - None, ); snapshot_utils::verify_snapshot_archive( diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index fa3fee4a08..b3b0238415 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -194,7 +194,6 @@ trait TypeContext<'a>: PartialEq { stream_writer: &mut BufWriter, accounts_hash: &Hash, incremental_snapshot_persistence: Option<&BankIncrementalSnapshotPersistence>, - epoch_accounts_hash: Option<&Hash>, ) -> std::result::Result<(), Box> where R: Read, @@ -394,7 +393,6 @@ fn reserialize_bank_fields_with_new_hash( stream_writer: &mut BufWriter, accounts_hash: &Hash, incremental_snapshot_persistence: Option<&BankIncrementalSnapshotPersistence>, - epoch_accounts_hash: Option<&Hash>, ) -> Result<(), Error> where W: Write, @@ -405,7 +403,6 @@ where stream_writer, accounts_hash, incremental_snapshot_persistence, - epoch_accounts_hash, ) } @@ -419,7 +416,6 @@ pub fn reserialize_bank_with_new_accounts_hash( slot: Slot, accounts_hash: &Hash, incremental_snapshot_persistence: Option<&BankIncrementalSnapshotPersistence>, - epoch_accounts_hash: Option<&Hash>, ) -> bool { let bank_post = snapshot_utils::get_bank_snapshots_dir(bank_snapshots_dir, slot); let bank_post = bank_post.join(snapshot_utils::get_snapshot_file_name(slot)); @@ -438,7 +434,6 @@ pub fn reserialize_bank_with_new_accounts_hash( &mut BufWriter::new(file_out), accounts_hash, incremental_snapshot_persistence, - epoch_accounts_hash, ) .unwrap(); } diff --git a/runtime/src/serde_snapshot/newer.rs b/runtime/src/serde_snapshot/newer.rs index c61061dccc..9a1503cade 100644 --- a/runtime/src/serde_snapshot/newer.rs +++ b/runtime/src/serde_snapshot/newer.rs @@ -347,7 +347,6 @@ impl<'a> TypeContext<'a> for Context { stream_writer: &mut BufWriter, accounts_hash: &Hash, incremental_snapshot_persistence: Option<&BankIncrementalSnapshotPersistence>, - epoch_accounts_hash: Option<&Hash>, ) -> std::result::Result<(), Box> where R: Read, @@ -360,7 +359,7 @@ impl<'a> TypeContext<'a> for Context { let blockhash_queue = RwLock::new(std::mem::take(&mut rhs.blockhash_queue)); let hard_forks = RwLock::new(std::mem::take(&mut rhs.hard_forks)); let lamports_per_signature = rhs.fee_rate_governor.lamports_per_signature; - let epoch_accounts_hash = epoch_accounts_hash.or(rhs.epoch_accounts_hash.as_ref()); + let epoch_accounts_hash = rhs.epoch_accounts_hash.as_ref(); let bank = SerializableVersionedBank { blockhash_queue: &blockhash_queue, diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index f8acca86d5..0ab77e0ba5 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -219,7 +219,6 @@ fn test_bank_serialize_style( reserialize_accounts_hash: bool, update_accounts_hash: bool, incremental_snapshot_persistence: bool, - epoch_accounts_hash: bool, initial_epoch_accounts_hash: bool, ) { solana_logger::setup(); @@ -294,7 +293,7 @@ fn test_bank_serialize_style( incremental_capitalization: 32, }); - if reserialize_accounts_hash || incremental_snapshot_persistence || epoch_accounts_hash { + if reserialize_accounts_hash || incremental_snapshot_persistence { let temp_dir = TempDir::new().unwrap(); let slot_dir = temp_dir.path().join(slot.to_string()); let post_path = slot_dir.join(slot.to_string()); @@ -306,19 +305,11 @@ fn test_bank_serialize_style( f.write_all(&buf).unwrap(); } - let reserialized_epoch_accounts_hash = if epoch_accounts_hash { - expected_epoch_accounts_hash = Some(Hash::new(&[3; 32])); - expected_epoch_accounts_hash - } else { - None - }; - assert!(reserialize_bank_with_new_accounts_hash( temp_dir.path(), slot, &accounts_hash, incremental.as_ref(), - reserialized_epoch_accounts_hash.as_ref(), )); let mut buf_reserialized; { @@ -333,15 +324,6 @@ fn test_bank_serialize_style( } else { // no change 0 - } - + if epoch_accounts_hash && !initial_epoch_accounts_hash { - // previously saved a none (size 1), now added a Some - let sizeof_epoch_accounts_hash_persistence = - std::mem::size_of::>(); - sizeof_epoch_accounts_hash_persistence - 1 - } else { - // no change - 0 }; // +1: larger buffer than expected to make sure the file isn't larger than expected @@ -352,11 +334,10 @@ fn test_bank_serialize_style( assert_eq!( size, expected, - "(reserialize_accounts_hash, incremental_snapshot_persistence, epoch_accounts_hash, update_accounts_hash, initial_epoch_accounts_hash): {:?}, previous_len: {previous_len}", + "(reserialize_accounts_hash, incremental_snapshot_persistence, update_accounts_hash, initial_epoch_accounts_hash): {:?}, previous_len: {previous_len}", ( reserialize_accounts_hash, incremental_snapshot_persistence, - epoch_accounts_hash, update_accounts_hash, initial_epoch_accounts_hash, ) @@ -370,7 +351,7 @@ fn test_bank_serialize_style( // But, we can guarantee that the buffer is different if we change the hash! assert_ne!(buf, buf_reserialized); } - if update_accounts_hash || incremental_snapshot_persistence || epoch_accounts_hash { + if update_accounts_hash || incremental_snapshot_persistence { buf = buf_reserialized; } } @@ -416,12 +397,11 @@ fn test_bank_serialize_style( assert_eq!(dbank.get_accounts_hash(), accounts_hash); assert!(bank2 == dbank); assert_eq!(dbank.incremental_snapshot_persistence, incremental); - assert_eq!(dbank.rc.accounts.accounts_db.epoch_accounts_hash_manager.try_get_epoch_accounts_hash().map(|hash| *hash.as_ref()), expected_epoch_accounts_hash, - "(reserialize_accounts_hash, incremental_snapshot_persistence, epoch_accounts_hash, update_accounts_hash, initial_epoch_accounts_hash): {:?}", + assert_eq!(dbank.get_epoch_accounts_hash_to_serialize(), expected_epoch_accounts_hash, + "(reserialize_accounts_hash, incremental_snapshot_persistence, update_accounts_hash, initial_epoch_accounts_hash): {:?}", ( reserialize_accounts_hash, incremental_snapshot_persistence, - epoch_accounts_hash, update_accounts_hash, initial_epoch_accounts_hash, ) @@ -485,17 +465,14 @@ fn test_bank_serialize_newer() { [false].to_vec() }; for incremental_snapshot_persistence in parameters.clone() { - for epoch_accounts_hash in parameters.clone() { - for initial_epoch_accounts_hash in parameters.clone() { - test_bank_serialize_style( - SerdeStyle::Newer, - reserialize_accounts_hash, - update_accounts_hash, - incremental_snapshot_persistence, - epoch_accounts_hash, - initial_epoch_accounts_hash, - ) - } + for initial_epoch_accounts_hash in [false, true] { + test_bank_serialize_style( + SerdeStyle::Newer, + reserialize_accounts_hash, + update_accounts_hash, + incremental_snapshot_persistence, + initial_epoch_accounts_hash, + ) } } } diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index bec5b98a3d..4fc671be8b 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2276,7 +2276,6 @@ pub fn package_and_archive_full_snapshot( accounts_package.slot, &bank.get_accounts_hash(), None, - None, // todo: this needs to be passed through ); let snapshot_package = SnapshotPackage::new(accounts_package, bank.get_accounts_hash()); @@ -2330,7 +2329,6 @@ pub fn package_and_archive_incremental_snapshot( accounts_package.slot, &bank.get_accounts_hash(), None, - None, // todo: this needs to be passed through ); let snapshot_package = SnapshotPackage::new(accounts_package, bank.get_accounts_hash());