From d23c04bb68b0ea92566ade3e9af9db227d9a22ef Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Tue, 19 Apr 2022 13:59:41 -0500 Subject: [PATCH] max_slot_in_storages_exclusive -> INclusive (#24450) --- runtime/src/accounts_db.rs | 4 +- runtime/src/expected_rent_collection.rs | 64 +++++++++++-------------- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 794fcfbb40..008ad458e4 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -5223,7 +5223,7 @@ impl AccountsDb { *slot, config.rent_collector, &stats, - max_slot + 1, // this wants an 'exclusive' number + max_slot, find_unskipped_slot, self.filler_account_suffix.as_ref(), ); @@ -5729,7 +5729,7 @@ impl AccountsDb { slot, config.rent_collector, stats, - storage.range().end, + storage.range().end.saturating_sub(1), // 'end' is exclusive, convert to inclusive find_unskipped_slot, filler_account_suffix, ); diff --git a/runtime/src/expected_rent_collection.rs b/runtime/src/expected_rent_collection.rs index ff6d1cc4a5..8f14b4cc78 100644 --- a/runtime/src/expected_rent_collection.rs +++ b/runtime/src/expected_rent_collection.rs @@ -392,7 +392,7 @@ impl ExpectedRentCollection { storage_slot: Slot, rent_collector: &RentCollector, stats: &HashStats, - max_slot_in_storages_exclusive: Slot, + max_slot_in_storages_inclusive: Slot, find_unskipped_slot: impl Fn(Slot) -> Option, filler_account_suffix: Option<&Pubkey>, ) -> Option { @@ -403,7 +403,7 @@ impl ExpectedRentCollection { loaded_account, storage_slot, rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, filler_account_suffix, ); @@ -439,7 +439,7 @@ impl ExpectedRentCollection { Some(recalc_hash) } - /// figure out whether the account stored at 'storage_slot' would have normally been rewritten at a slot that has already occurred: after 'storage_slot' but before 'max_slot_in_storages_exclusive' + /// figure out whether the account stored at 'storage_slot' would have normally been rewritten at a slot that has already occurred: after 'storage_slot' but <= 'max_slot_in_storages_inclusive' /// returns Some(...) if the account would have normally been rewritten /// returns None if the account was updated wrt rent already or if it is known that there must exist a future rewrite of this account (for example, non-zero rent is due) fn new( @@ -447,7 +447,7 @@ impl ExpectedRentCollection { loaded_account: &impl ReadableAccount, storage_slot: Slot, rent_collector: &RentCollector, - max_slot_in_storages_exclusive: Slot, + max_slot_in_storages_inclusive: Slot, find_unskipped_slot: impl Fn(Slot) -> Option, filler_account_suffix: Option<&Pubkey>, ) -> Option { @@ -455,9 +455,6 @@ impl ExpectedRentCollection { .epoch_schedule .get_slots_in_epoch(rent_collector.epoch); - assert!(max_slot_in_storages_exclusive > 0); - let max_slot_in_storages_inclusive = max_slot_in_storages_exclusive.saturating_sub(1); - let partition_from_pubkey = crate::bank::Bank::partition_from_pubkey(pubkey, slots_per_epoch); let (epoch_of_max_storage_slot, partition_index_from_max_slot) = rent_collector @@ -563,7 +560,7 @@ pub mod tests { let pubkey = Pubkey::new(&[5; 32]); let owner = solana_sdk::pubkey::new_rand(); let mut account = AccountSharedData::new(1, 0, &owner); - let max_slot_in_storages_exclusive = 1; + let max_slot_in_storages_inclusive = 0; let epoch_schedule = EpochSchedule::default(); let first_normal_slot = epoch_schedule.first_normal_slot; let storage_slot = first_normal_slot; @@ -587,7 +584,7 @@ pub mod tests { &account, storage_slot, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -603,10 +600,8 @@ pub mod tests { // several epochs ahead of now // first slot of new epoch is max slot EXclusive // so last slot of prior epoch is max slot INclusive - let max_slot_in_storages_exclusive = slots_per_epoch * 3 + first_normal_slot; - // -1 is because get_epoch takes slot and `max_slot_in_storages_exclusive` is EXclusive - rent_collector.epoch = epoch_schedule.get_epoch(max_slot_in_storages_exclusive - 1); - //let first_slot_in_max_epoch = max_slot_in_storages_exclusive - slots_per_epoch; + let max_slot_in_storages_inclusive = slots_per_epoch * 3 + first_normal_slot - 1; + rent_collector.epoch = epoch_schedule.get_epoch(max_slot_in_storages_inclusive); let partition_from_pubkey = 8470; // function of 432k slots and 'pubkey' above let first_slot_in_max_epoch = 1388256; let expected_rent_collection_slot_max_epoch = @@ -616,7 +611,7 @@ pub mod tests { &account, storage_slot, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -640,7 +635,7 @@ pub mod tests { &account, expected_rent_collection_slot_max_epoch, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -667,7 +662,7 @@ pub mod tests { &account, expected_rent_collection_slot_max_epoch + if greater { 1 } else { 0 }, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -684,7 +679,7 @@ pub mod tests { ); } - // test rewrite would have occurred in previous epoch from max_slot_in_storages_exclusive's epoch + // test rewrite would have occurred in previous epoch from max_slot_in_storages_inclusive's epoch // the change is in 'rent_epoch' returned in 'expected' for previous_epoch in [false, true] { let expected = ExpectedRentCollection::new( @@ -692,7 +687,7 @@ pub mod tests { &account, expected_rent_collection_slot_max_epoch, &rent_collector, - max_slot_in_storages_exclusive + if previous_epoch { slots_per_epoch } else { 0 }, + max_slot_in_storages_inclusive + if previous_epoch { slots_per_epoch } else { 0 }, find_unskipped_slot, None, ); @@ -725,7 +720,7 @@ pub mod tests { &account, expected_rent_collection_slot_max_epoch, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -746,7 +741,7 @@ pub mod tests { } account.set_rent_epoch(original_rent_epoch); - let storage_slot = max_slot_in_storages_exclusive - slots_per_epoch - 1; + let storage_slot = max_slot_in_storages_inclusive - slots_per_epoch; // check partition from pubkey code for end_partition_index in [0, 1, 2, 100, slots_per_epoch - 2, slots_per_epoch - 1] { // generate a pubkey range @@ -764,7 +759,7 @@ pub mod tests { &account, storage_slot, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -778,17 +773,17 @@ pub mod tests { expected_rent_collection_slot_max_epoch: first_slot_in_max_epoch + end_partition_index, rent_epoch: rent_collector.epoch, }), - "range: {:?}, pubkey: {:?}, end_partition_index: {}, max_slot_in_storages_exclusive: {}", + "range: {:?}, pubkey: {:?}, end_partition_index: {}, max_slot_in_storages_inclusive: {}", range, pubkey, end_partition_index, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, ); } } - // check max_slot_in_storages_exclusive related code - // so sweep through max_slot_in_storages_exclusive values within an epoch + // check max_slot_in_storages_inclusive related code + // so sweep through max_slot_in_storages_inclusive values within an epoch let first_slot_in_max_epoch = first_normal_slot + slots_per_epoch; rent_collector.epoch = epoch_schedule.get_epoch(first_slot_in_max_epoch); // an epoch in the past so we always collect rent @@ -807,13 +802,13 @@ pub mod tests { // partition_index=0 means first slot of second normal epoch // second normal epoch because we want to deal with accounts stored in the first normal epoch // + 1 because of exclusive - let max_slot_in_storages_exclusive = first_slot_in_max_epoch + partition_index + 1; + let max_slot_in_storages_inclusive = first_slot_in_max_epoch + partition_index; let expected = ExpectedRentCollection::new( &pubkey, &account, storage_slot, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -838,9 +833,9 @@ pub mod tests { expected_rent_collection_slot_max_epoch, rent_epoch: expected_rent_epoch, }), - "partition_index: {}, max_slot_in_storages_exclusive: {}, storage_slot: {}, first_normal_slot: {}", + "partition_index: {}, max_slot_in_storages_inclusive: {}, storage_slot: {}, first_normal_slot: {}", partition_index, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, storage_slot, first_normal_slot, ); @@ -855,7 +850,7 @@ pub mod tests { &account, storage_slot, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -887,7 +882,7 @@ pub mod tests { &account, storage_slot, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -1009,13 +1004,12 @@ pub mod tests { epoch_schedule.get_epoch_and_slot_index(first_slot_in_max_epoch) ); account.set_rent_epoch(1); - let max_slot_in_storages_exclusive = max_slot_in_storages_inclusive + 1; let expected = ExpectedRentCollection::new( &pubkey, &account, storage_slot, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, ); @@ -1048,7 +1042,7 @@ pub mod tests { &account, storage_slot, &rent_collector, - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, // treat this pubkey like a filler account so we get a 'LeaveAloneNoRent' result Some(&pubkey), @@ -1079,7 +1073,7 @@ pub mod tests { storage_slot, &rent_collector, &HashStats::default(), - max_slot_in_storages_exclusive, + max_slot_in_storages_inclusive, find_unskipped_slot, None, );