remove activated feature set_exempt_rent_epoch_max (#35082)

* remove activated feature set_exempt_rent_epoch_max

* fix test_rent_eager_collect_rent_in_partition test

* update hash values for test_bank_hash_consistency

* clean up commas
This commit is contained in:
Jeff Washington (jwash) 2024-02-07 09:20:31 -06:00 committed by GitHub
parent 99247d150d
commit 2aa8b82990
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 387 additions and 587 deletions

View File

@ -111,13 +111,10 @@ impl RentCollector {
&self,
address: &Pubkey,
account: &mut AccountSharedData,
set_exempt_rent_epoch_max: bool,
) -> CollectedInfo {
match self.calculate_rent_result(address, account) {
RentResult::Exempt => {
if set_exempt_rent_epoch_max {
account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
CollectedInfo::default()
}
RentResult::NoRentCollectionNow => CollectedInfo::default(),
@ -219,17 +216,15 @@ mod tests {
&self,
address: &Pubkey,
account: &mut AccountSharedData,
set_exempt_rent_epoch_max: bool,
) -> CollectedInfo {
// initialize rent_epoch as created at this epoch
account.set_rent_epoch(self.epoch);
self.collect_from_existing_account(address, account, set_exempt_rent_epoch_max)
self.collect_from_existing_account(address, account)
}
}
#[test]
fn test_calculate_rent_result() {
for set_exempt_rent_epoch_max in [false, true] {
let mut rent_collector = RentCollector::default();
let mut account = AccountSharedData::default();
@ -240,11 +235,8 @@ mod tests {
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
set_exempt_rent_epoch_max
),
rent_collector
.collect_from_existing_account(&Pubkey::default(), &mut account_clone),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
@ -258,15 +250,10 @@ mod tests {
{
let mut account_clone = account.clone();
let mut account_expected = account.clone();
if set_exempt_rent_epoch_max {
account_expected.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
set_exempt_rent_epoch_max
),
rent_collector
.collect_from_existing_account(&Pubkey::default(), &mut account_clone),
CollectedInfo::default()
);
assert_eq!(account_clone, account_expected);
@ -280,15 +267,10 @@ mod tests {
{
let mut account_clone = account.clone();
let mut account_expected = account.clone();
if set_exempt_rent_epoch_max {
account_expected.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
assert_eq!(
rent_collector.collect_from_existing_account(
&incinerator::id(),
&mut account_clone,
set_exempt_rent_epoch_max
),
rent_collector
.collect_from_existing_account(&incinerator::id(), &mut account_clone),
CollectedInfo::default()
);
assert_eq!(account_clone, account_expected);
@ -312,11 +294,8 @@ mod tests {
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
set_exempt_rent_epoch_max
),
rent_collector
.collect_from_existing_account(&Pubkey::default(), &mut account_clone),
CollectedInfo {
rent_amount: rent_due_expected,
account_data_len_reclaimed: 0
@ -332,22 +311,14 @@ mod tests {
// enough lamports to make us exempt
account.set_lamports(1_000_000);
let result = rent_collector.calculate_rent_result(&Pubkey::default(), &account);
assert!(
matches!(result, RentResult::Exempt),
"{result:?}, set_exempt_rent_epoch_max: {set_exempt_rent_epoch_max}",
);
assert!(matches!(result, RentResult::Exempt), "{result:?}",);
{
let mut account_clone = account.clone();
let mut account_expected = account.clone();
if set_exempt_rent_epoch_max {
account_expected.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
set_exempt_rent_epoch_max
),
rent_collector
.collect_from_existing_account(&Pubkey::default(), &mut account_clone),
CollectedInfo::default()
);
assert_eq!(account_clone, account_expected);
@ -364,21 +335,16 @@ mod tests {
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
set_exempt_rent_epoch_max
),
rent_collector
.collect_from_existing_account(&Pubkey::default(), &mut account_clone),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
}
}
}
#[test]
fn test_collect_from_account_created_and_existing() {
for set_exempt_rent_epoch_max in [false, true] {
let old_lamports = 1000;
let old_epoch = 1;
let new_epoch = 2;
@ -396,11 +362,8 @@ mod tests {
let rent_collector = default_rent_collector_clone_with_epoch(new_epoch);
// collect rent on a newly-created account
let collected = rent_collector.collect_from_created_account(
&solana_sdk::pubkey::new_rand(),
&mut created_account,
set_exempt_rent_epoch_max,
);
let collected = rent_collector
.collect_from_created_account(&solana_sdk::pubkey::new_rand(), &mut created_account);
assert!(created_account.lamports() < old_lamports);
assert_eq!(
created_account.lamports() + collected.rent_amount,
@ -410,11 +373,8 @@ mod tests {
assert_eq!(collected.account_data_len_reclaimed, 0);
// collect rent on a already-existing account
let collected = rent_collector.collect_from_existing_account(
&solana_sdk::pubkey::new_rand(),
&mut existing_account,
set_exempt_rent_epoch_max,
);
let collected = rent_collector
.collect_from_existing_account(&solana_sdk::pubkey::new_rand(), &mut existing_account);
assert!(existing_account.lamports() < old_lamports);
assert_eq!(
existing_account.lamports() + collected.rent_amount,
@ -427,11 +387,9 @@ mod tests {
assert!(created_account.lamports() > existing_account.lamports());
assert_eq!(created_account.rent_epoch(), existing_account.rent_epoch());
}
}
#[test]
fn test_rent_exempt_temporal_escape() {
for set_exempt_rent_epoch_max in [false, true] {
for pass in 0..2 {
let mut account = AccountSharedData::default();
let epoch = 3;
@ -447,11 +405,7 @@ mod tests {
if pass == 0 {
account.set_lamports(huge_lamports);
// first mark account as being collected while being rent-exempt
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
set_exempt_rent_epoch_max,
);
let collected = rent_collector.collect_from_existing_account(&pubkey, &mut account);
assert_eq!(account.lamports(), huge_lamports);
assert_eq!(collected, CollectedInfo::default());
continue;
@ -463,20 +417,14 @@ mod tests {
account.set_lamports(tiny_lamports);
// ... and trigger another rent collection on the same epoch and check that rent is working
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
set_exempt_rent_epoch_max,
);
let collected = rent_collector.collect_from_existing_account(&pubkey, &mut account);
assert_eq!(account.lamports(), tiny_lamports - collected.rent_amount);
assert_ne!(collected, CollectedInfo::default());
}
}
}
#[test]
fn test_rent_exempt_sysvar() {
for set_exempt_rent_epoch_max in [false, true] {
let tiny_lamports = 1;
let mut account = AccountSharedData::default();
account.set_owner(sysvar::id());
@ -489,20 +437,14 @@ mod tests {
let epoch = 3;
let rent_collector = default_rent_collector_clone_with_epoch(epoch);
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
set_exempt_rent_epoch_max,
);
let collected = rent_collector.collect_from_existing_account(&pubkey, &mut account);
assert_eq!(account.lamports(), 0);
assert_eq!(collected.rent_amount, 1);
}
}
/// Ensure that when an account is "rent collected" away, its data len is returned.
#[test]
fn test_collect_cleans_up_account() {
for set_exempt_rent_epoch_max in [false, true] {
solana_logger::setup();
let account_lamports = 1; // must be *below* rent amount
let account_data_len = 567;
@ -515,11 +457,8 @@ mod tests {
});
let rent_collector = default_rent_collector_clone_with_epoch(account_rent_epoch + 1);
let collected = rent_collector.collect_from_existing_account(
&Pubkey::new_unique(),
&mut account,
set_exempt_rent_epoch_max,
);
let collected =
rent_collector.collect_from_existing_account(&Pubkey::new_unique(), &mut account);
assert_eq!(collected.rent_amount, account_lamports);
assert_eq!(
@ -529,4 +468,3 @@ mod tests {
assert_eq!(account, AccountSharedData::default());
}
}
}

View File

@ -5240,15 +5240,12 @@ impl Bank {
.accounts
.accounts_db
.test_skip_rewrites_but_include_in_bank_hash;
let set_exempt_rent_epoch_max: bool = self
.feature_set
.is_active(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id());
let mut skipped_rewrites = Vec::default();
for (pubkey, account, _loaded_slot) in accounts.iter_mut() {
let rent_collected_info = if self.should_collect_rent() {
let (rent_collected_info, measure) = measure!(self
.rent_collector
.collect_from_existing_account(pubkey, account, set_exempt_rent_epoch_max,));
.collect_from_existing_account(pubkey, account));
time_collecting_rent_us += measure.as_us();
rent_collected_info
} else {
@ -5256,9 +5253,8 @@ impl Bank {
// are any rent paying accounts, their `rent_epoch` won't change either. However, if the
// account itself is rent-exempted but its `rent_epoch` is not u64::MAX, we will set its
// `rent_epoch` to u64::MAX. In such case, the behavior stays the same as before.
if set_exempt_rent_epoch_max
&& (account.rent_epoch() != RENT_EXEMPT_RENT_EPOCH
&& self.rent_collector.get_rent_due(account) == RentDue::Exempt)
if account.rent_epoch() != RENT_EXEMPT_RENT_EPOCH
&& self.rent_collector.get_rent_due(account) == RentDue::Exempt
{
account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}

View File

@ -454,7 +454,6 @@ fn rent_with_exemption_threshold(exemption_threshold: f64) -> Rent {
/// one thing being tested here is that a failed tx (due to rent collection using up all lamports) followed by rent collection
/// results in the same state as if just rent collection ran (and emptied the accounts that have too few lamports)
fn test_credit_debit_rent_no_side_effect_on_hash() {
for set_exempt_rent_epoch_max in [false, true] {
solana_logger::setup();
let (mut genesis_config, _mint_keypair) = create_genesis_config_no_tx_fee(10);
@ -501,11 +500,9 @@ fn test_credit_debit_rent_no_side_effect_on_hash() {
{
// make sure rent and epoch change are such that we collect all lamports in accounts 4 & 5
let mut account_copy = accounts[4].clone();
let expected_rent = bank.rent_collector().collect_from_existing_account(
&keypairs[4].pubkey(),
&mut account_copy,
set_exempt_rent_epoch_max,
);
let expected_rent = bank
.rent_collector()
.collect_from_existing_account(&keypairs[4].pubkey(), &mut account_copy);
assert_eq!(expected_rent.rent_amount, too_few_lamports);
assert_eq!(account_copy.lamports(), 0);
}
@ -572,7 +569,6 @@ fn test_credit_debit_rent_no_side_effect_on_hash() {
assert_eq!(bank_with_success_txs_hash, bank_hash);
}
}
fn store_accounts_for_rent_test(
bank: &Bank,
@ -1657,7 +1653,7 @@ fn test_rent_eager_collect_rent_in_partition(should_collect_rent: bool) {
solana_logger::setup();
let (mut genesis_config, _mint_keypair) = create_genesis_config(1_000_000);
for feature_id in FeatureSet::default().inactive {
if feature_id != solana_sdk::feature_set::set_exempt_rent_epoch_max::id()
if feature_id != solana_sdk::feature_set::skip_rent_rewrites::id()
&& (!should_collect_rent
|| feature_id != solana_sdk::feature_set::disable_rent_fees_collection::id())
{
@ -1739,11 +1735,9 @@ fn test_rent_eager_collect_rent_in_partition(should_collect_rent: bool) {
bank.get_account(&rent_exempt_pubkey).unwrap().lamports(),
large_lamports
);
// Once preserve_rent_epoch_for_rent_exempt_accounts is activated,
// rent_epoch of rent-exempt accounts will no longer advance.
assert_eq!(
bank.get_account(&rent_exempt_pubkey).unwrap().rent_epoch(),
0
RENT_EXEMPT_RENT_EPOCH
);
assert_eq!(
bank.slots_by_pubkey(&rent_due_pubkey, &ancestors),
@ -6479,58 +6473,27 @@ fn test_fuzz_instructions() {
info!("results: {:?}", results);
}
#[test_case(true; "set_rent_epoch_max")]
#[test_case(false; "disable_set_rent_epoch_max")]
fn test_bank_hash_consistency(set_rent_epoch_max: bool) {
#[test]
fn test_bank_hash_consistency() {
solana_logger::setup();
let account = AccountSharedData::new(1_000_000_000_000, 0, &system_program::id());
if !set_rent_epoch_max {
assert_eq!(account.rent_epoch(), 0);
}
let mut genesis_config = GenesisConfig::new(&[(Pubkey::from([42; 32]), account)], &[]);
genesis_config.creation_time = 0;
genesis_config.cluster_type = ClusterType::MainnetBeta;
genesis_config.rent.burn_percent = 100;
if set_rent_epoch_max {
activate_feature(
&mut genesis_config,
solana_sdk::feature_set::set_exempt_rent_epoch_max::id(),
);
}
let mut bank = Arc::new(Bank::new_for_tests(&genesis_config));
// Check a few slots, cross an epoch boundary
assert_eq!(bank.get_slots_in_epoch(0), 32);
loop {
goto_end_of_slot(bank.clone());
if !set_rent_epoch_max {
if bank.slot == 0 {
assert_eq!(
bank.hash().to_string(),
"trdzvRDTAXAqo1i2GX4JfK9ReixV1NYNG7DRaVq43Do",
);
}
if bank.slot == 32 {
assert_eq!(
bank.hash().to_string(),
"2rdj8QEnDnBSyMv81rCmncss4UERACyXXB3pEvkep8eS",
);
}
if bank.slot == 64 {
assert_eq!(
bank.hash().to_string(),
"7g3ofXVQB3reFt9ki8zLA8S4w1GdmEWsWuWrwkPN3SSv"
);
}
if bank.slot == 128 {
assert_eq!(
bank.hash().to_string(),
"4uX1AZFbqwjwWBACWbAW3V8rjbWH4N3ZRTbNysSLAzj2"
);
break;
}
} else {
if bank.slot == 0 {
assert_eq!(
bank.hash().to_string(),
@ -6556,7 +6519,6 @@ fn test_bank_hash_consistency(set_rent_epoch_max: bool) {
);
break;
}
}
bank = Arc::new(new_from_parent(bank));
}
}
@ -11635,7 +11597,6 @@ fn test_get_rent_paying_pubkeys() {
#[test_case(true; "enable rent fees collection")]
#[test_case(false; "disable rent fees collection")]
fn test_accounts_data_size_and_rent_collection(should_collect_rent: bool) {
for set_exempt_rent_epoch_max in [false, true] {
let GenesisConfigInfo {
mut genesis_config, ..
} = genesis_utils::create_genesis_config(100 * LAMPORTS_PER_SOL);
@ -11664,11 +11625,9 @@ fn test_accounts_data_size_and_rent_collection(should_collect_rent: bool) {
// Ensure if we collect rent from the account that it will be reclaimed
{
let info = bank.rent_collector.collect_from_existing_account(
&keypair.pubkey(),
&mut account,
set_exempt_rent_epoch_max,
);
let info = bank
.rent_collector
.collect_from_existing_account(&keypair.pubkey(), &mut account);
assert_eq!(info.account_data_len_reclaimed, data_size as u64);
}
@ -11686,7 +11645,6 @@ fn test_accounts_data_size_and_rent_collection(should_collect_rent: bool) {
// Ensure the account is reclaimed by rent collection
assert!(!should_collect_rent || reclaimed_data_size == data_size);
}
}
#[test]
fn test_accounts_data_size_with_default_bank() {
@ -11895,87 +11853,6 @@ fn test_feature_hashes_per_tick() {
assert_eq!(bank.hashes_per_tick, Some(UPDATED_HASHES_PER_TICK6));
}
#[test_case(true)]
#[test_case(false)]
fn test_stake_account_consistency_with_rent_epoch_max_feature(
rent_epoch_max_enabled_initially: bool,
) {
// this test can be removed once set_exempt_rent_epoch_max gets activated
solana_logger::setup();
let (mut genesis_config, _mint_keypair) = create_genesis_config(100 * LAMPORTS_PER_SOL);
genesis_config.rent = Rent::default();
let mut bank = Bank::new_for_tests(&genesis_config);
let expected_initial_rent_epoch = if rent_epoch_max_enabled_initially {
bank.activate_feature(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id());
RENT_EXEMPT_RENT_EPOCH
} else {
Epoch::default()
};
let mut pubkey_bytes_early = [0u8; 32];
pubkey_bytes_early[31] = 2;
let stake_id1 = Pubkey::from(pubkey_bytes_early);
let vote_id = solana_sdk::pubkey::new_rand();
let stake_account1 = crate::stakes::tests::create_stake_account(12300000, &vote_id, &stake_id1);
// set up accounts
bank.store_account_and_update_capitalization(&stake_id1, &stake_account1);
// create banks at a few slots
assert_eq!(
bank.load_slow(&bank.ancestors, &stake_id1)
.unwrap()
.0
.rent_epoch(),
0 // manually created, so default is 0
);
let slot = 1;
let slots_per_epoch = bank.epoch_schedule().get_slots_in_epoch(0);
let mut bank = Bank::new_from_parent(Arc::new(bank), &Pubkey::default(), slot);
if !rent_epoch_max_enabled_initially {
bank.activate_feature(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id());
}
let bank = Arc::new(bank);
let slot = slots_per_epoch - 1;
assert_eq!(
bank.load_slow(&bank.ancestors, &stake_id1)
.unwrap()
.0
.rent_epoch(),
// rent has been collected, so if rent epoch is max is activated, this will be max by now
expected_initial_rent_epoch
);
let mut bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::default(), slot));
let last_slot_in_epoch = bank.epoch_schedule().get_last_slot_in_epoch(1);
let slot = last_slot_in_epoch - 2;
assert_eq!(
bank.load_slow(&bank.ancestors, &stake_id1)
.unwrap()
.0
.rent_epoch(),
expected_initial_rent_epoch
);
bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::default(), slot));
assert_eq!(
bank.load_slow(&bank.ancestors, &stake_id1)
.unwrap()
.0
.rent_epoch(),
expected_initial_rent_epoch
);
let slot = last_slot_in_epoch - 1;
bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::default(), slot));
assert_eq!(
bank.load_slow(&bank.ancestors, &stake_id1)
.unwrap()
.0
.rent_epoch(),
RENT_EXEMPT_RENT_EPOCH
);
}
#[test]
fn test_calculate_fee_with_congestion_multiplier() {
let lamports_scale: u64 = 5;

View File

@ -132,9 +132,6 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
let mut rent_debits = RentDebits::default();
let rent_collector = callbacks.get_rent_collector();
let set_exempt_rent_epoch_max =
feature_set.is_active(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id());
let requested_loaded_accounts_data_size_limit =
get_requested_loaded_accounts_data_size_limit(tx)?;
let mut accumulated_accounts_data_size: usize = 0;
@ -179,11 +176,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
.is_active(&feature_set::disable_rent_fees_collection::id())
{
let rent_due = rent_collector
.collect_from_existing_account(
key,
&mut account,
set_exempt_rent_epoch_max,
)
.collect_from_existing_account(key, &mut account)
.rent_amount;
(account.data().len(), account, rent_due)
@ -192,10 +185,8 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
// are any rent paying accounts, their `rent_epoch` won't change either. However, if the
// account itself is rent-exempted but its `rent_epoch` is not u64::MAX, we will set its
// `rent_epoch` to u64::MAX. In such case, the behavior stays the same as before.
if set_exempt_rent_epoch_max
&& (account.rent_epoch() != RENT_EXEMPT_RENT_EPOCH
&& rent_collector.get_rent_due(&account)
== RentDue::Exempt)
if account.rent_epoch() != RENT_EXEMPT_RENT_EPOCH
&& rent_collector.get_rent_due(&account) == RentDue::Exempt
{
account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
@ -208,12 +199,10 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
.unwrap_or_else(|| {
account_found = false;
let mut default_account = AccountSharedData::default();
if set_exempt_rent_epoch_max {
// All new accounts must be rent-exempt (enforced in Bank::execute_loaded_transaction).
// Currently, rent collection sets rent_epoch to u64::MAX, but initializing the account
// with this field already set would allow us to skip rent collection for these accounts.
default_account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
(default_account.data().len(), default_account, 0)
})
};