diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 140d05c0c..53329dbc8 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -7983,7 +7983,6 @@ pub(crate) mod tests { genesis_sysvar_and_builtin_program_lamports, GenesisConfigInfo, ValidatorVoteKeypairs, }, - rent_collector::TEST_SET_EXEMPT_RENT_EPOCH_MAX, rent_paying_accounts_by_partition::RentPayingAccountsByPartition, status_cache::MAX_CACHE_ENTRIES, }, @@ -8401,113 +8400,116 @@ pub(crate) mod tests { /// 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() { - solana_logger::setup(); + for set_exempt_rent_epoch_max in [false, true] { + solana_logger::setup(); - let (mut genesis_config, _mint_keypair) = create_genesis_config(10); + let (mut genesis_config, _mint_keypair) = create_genesis_config(10); - genesis_config.rent = rent_with_exemption_threshold(21.0); + genesis_config.rent = rent_with_exemption_threshold(21.0); - let slot = years_as_slots( - 2.0, - &genesis_config.poh_config.target_tick_duration, - genesis_config.ticks_per_slot, - ) as u64; - let root_bank = Arc::new(Bank::new_for_tests(&genesis_config)); - let bank = Bank::new_from_parent(&root_bank, &Pubkey::default(), slot); + let slot = years_as_slots( + 2.0, + &genesis_config.poh_config.target_tick_duration, + genesis_config.ticks_per_slot, + ) as u64; + let root_bank = Arc::new(Bank::new_for_tests(&genesis_config)); + let bank = Bank::new_from_parent(&root_bank, &Pubkey::default(), slot); - let root_bank_2 = Arc::new(Bank::new_for_tests(&genesis_config)); - let bank_with_success_txs = Bank::new_from_parent(&root_bank_2, &Pubkey::default(), slot); + let root_bank_2 = Arc::new(Bank::new_for_tests(&genesis_config)); + let bank_with_success_txs = + Bank::new_from_parent(&root_bank_2, &Pubkey::default(), slot); - assert_eq!(bank.last_blockhash(), genesis_config.hash()); + assert_eq!(bank.last_blockhash(), genesis_config.hash()); - let plenty_of_lamports = 264; - let too_few_lamports = 10; - // Initialize credit-debit and credit only accounts - let accounts = [ - AccountSharedData::new(plenty_of_lamports, 0, &Pubkey::default()), - AccountSharedData::new(plenty_of_lamports, 1, &Pubkey::default()), - AccountSharedData::new(plenty_of_lamports, 0, &Pubkey::default()), - AccountSharedData::new(plenty_of_lamports, 1, &Pubkey::default()), - // Transaction between these two accounts will fail - AccountSharedData::new(too_few_lamports, 0, &Pubkey::default()), - AccountSharedData::new(too_few_lamports, 1, &Pubkey::default()), - ]; + let plenty_of_lamports = 264; + let too_few_lamports = 10; + // Initialize credit-debit and credit only accounts + let accounts = [ + AccountSharedData::new(plenty_of_lamports, 0, &Pubkey::default()), + AccountSharedData::new(plenty_of_lamports, 1, &Pubkey::default()), + AccountSharedData::new(plenty_of_lamports, 0, &Pubkey::default()), + AccountSharedData::new(plenty_of_lamports, 1, &Pubkey::default()), + // Transaction between these two accounts will fail + AccountSharedData::new(too_few_lamports, 0, &Pubkey::default()), + AccountSharedData::new(too_few_lamports, 1, &Pubkey::default()), + ]; - let keypairs = accounts.iter().map(|_| Keypair::new()).collect::>(); - { - // 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, - None, - TEST_SET_EXEMPT_RENT_EPOCH_MAX, + let keypairs = accounts.iter().map(|_| Keypair::new()).collect::>(); + { + // 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, + None, + set_exempt_rent_epoch_max, + ); + assert_eq!(expected_rent.rent_amount, too_few_lamports); + assert_eq!(account_copy.lamports(), 0); + } + + for i in 0..accounts.len() { + let account = &accounts[i]; + bank.store_account(&keypairs[i].pubkey(), account); + bank_with_success_txs.store_account(&keypairs[i].pubkey(), account); + } + + // Make builtin instruction loader rent exempt + let system_program_id = system_program::id(); + let mut system_program_account = bank.get_account(&system_program_id).unwrap(); + system_program_account.set_lamports( + bank.get_minimum_balance_for_rent_exemption(system_program_account.data().len()), ); - assert_eq!(expected_rent.rent_amount, too_few_lamports); - assert_eq!(account_copy.lamports(), 0); + bank.store_account(&system_program_id, &system_program_account); + bank_with_success_txs.store_account(&system_program_id, &system_program_account); + + let t1 = system_transaction::transfer( + &keypairs[0], + &keypairs[1].pubkey(), + 1, + genesis_config.hash(), + ); + let t2 = system_transaction::transfer( + &keypairs[2], + &keypairs[3].pubkey(), + 1, + genesis_config.hash(), + ); + // the idea is this transaction will result in both accounts being drained of all lamports due to rent collection + let t3 = system_transaction::transfer( + &keypairs[4], + &keypairs[5].pubkey(), + 1, + genesis_config.hash(), + ); + + let txs = vec![t1.clone(), t2.clone(), t3]; + let res = bank.process_transactions(txs.iter()); + + assert_eq!(res.len(), 3); + assert_eq!(res[0], Ok(())); + assert_eq!(res[1], Ok(())); + assert_eq!(res[2], Err(TransactionError::AccountNotFound)); + + bank.freeze(); + + let rwlockguard_bank_hash = bank.hash.read().unwrap(); + let bank_hash = rwlockguard_bank_hash.as_ref(); + + let txs = vec![t2, t1]; + let res = bank_with_success_txs.process_transactions(txs.iter()); + + assert_eq!(res.len(), 2); + assert_eq!(res[0], Ok(())); + assert_eq!(res[1], Ok(())); + + bank_with_success_txs.freeze(); + + let rwlockguard_bank_with_success_txs_hash = bank_with_success_txs.hash.read().unwrap(); + let bank_with_success_txs_hash = rwlockguard_bank_with_success_txs_hash.as_ref(); + + assert_eq!(bank_with_success_txs_hash, bank_hash); } - - for i in 0..accounts.len() { - let account = &accounts[i]; - bank.store_account(&keypairs[i].pubkey(), account); - bank_with_success_txs.store_account(&keypairs[i].pubkey(), account); - } - - // Make builtin instruction loader rent exempt - let system_program_id = system_program::id(); - let mut system_program_account = bank.get_account(&system_program_id).unwrap(); - system_program_account.set_lamports( - bank.get_minimum_balance_for_rent_exemption(system_program_account.data().len()), - ); - bank.store_account(&system_program_id, &system_program_account); - bank_with_success_txs.store_account(&system_program_id, &system_program_account); - - let t1 = system_transaction::transfer( - &keypairs[0], - &keypairs[1].pubkey(), - 1, - genesis_config.hash(), - ); - let t2 = system_transaction::transfer( - &keypairs[2], - &keypairs[3].pubkey(), - 1, - genesis_config.hash(), - ); - // the idea is this transaction will result in both accounts being drained of all lamports due to rent collection - let t3 = system_transaction::transfer( - &keypairs[4], - &keypairs[5].pubkey(), - 1, - genesis_config.hash(), - ); - - let txs = vec![t1.clone(), t2.clone(), t3]; - let res = bank.process_transactions(txs.iter()); - - assert_eq!(res.len(), 3); - assert_eq!(res[0], Ok(())); - assert_eq!(res[1], Ok(())); - assert_eq!(res[2], Err(TransactionError::AccountNotFound)); - - bank.freeze(); - - let rwlockguard_bank_hash = bank.hash.read().unwrap(); - let bank_hash = rwlockguard_bank_hash.as_ref(); - - let txs = vec![t2, t1]; - let res = bank_with_success_txs.process_transactions(txs.iter()); - - assert_eq!(res.len(), 2); - assert_eq!(res[0], Ok(())); - assert_eq!(res[1], Ok(())); - - bank_with_success_txs.freeze(); - - let rwlockguard_bank_with_success_txs_hash = bank_with_success_txs.hash.read().unwrap(); - let bank_with_success_txs_hash = rwlockguard_bank_with_success_txs_hash.as_ref(); - - assert_eq!(bank_with_success_txs_hash, bank_hash); } fn store_accounts_for_rent_test( @@ -20012,55 +20014,59 @@ pub(crate) mod tests { /// Ensure that accounts data size is updated correctly by rent collection #[test] fn test_accounts_data_size_and_rent_collection() { - let GenesisConfigInfo { - mut genesis_config, .. - } = genesis_utils::create_genesis_config(100 * LAMPORTS_PER_SOL); - genesis_config.rent = Rent::default(); - activate_all_features(&mut genesis_config); - let bank = Arc::new(Bank::new_for_tests(&genesis_config)); - let bank = Arc::new(Bank::new_from_parent( - &bank, - &Pubkey::default(), - bank.slot() + bank.slot_count_per_normal_epoch(), - )); + for set_exempt_rent_epoch_max in [false, true] { + let GenesisConfigInfo { + mut genesis_config, .. + } = genesis_utils::create_genesis_config(100 * LAMPORTS_PER_SOL); + genesis_config.rent = Rent::default(); + activate_all_features(&mut genesis_config); + let bank = Arc::new(Bank::new_for_tests(&genesis_config)); + let bank = Arc::new(Bank::new_from_parent( + &bank, + &Pubkey::default(), + bank.slot() + bank.slot_count_per_normal_epoch(), + )); - // make another bank so that any reclaimed accounts from the previous bank do not impact - // this test - let bank = Arc::new(Bank::new_from_parent( - &bank, - &Pubkey::default(), - bank.slot() + bank.slot_count_per_normal_epoch(), - )); + // make another bank so that any reclaimed accounts from the previous bank do not impact + // this test + let bank = Arc::new(Bank::new_from_parent( + &bank, + &Pubkey::default(), + bank.slot() + bank.slot_count_per_normal_epoch(), + )); - // Store an account into the bank that is rent-paying and has data - let data_size = 123; - let mut account = AccountSharedData::new(1, data_size, &Pubkey::default()); - let keypair = Keypair::new(); - bank.store_account(&keypair.pubkey(), &account); + // Store an account into the bank that is rent-paying and has data + let data_size = 123; + let mut account = AccountSharedData::new(1, data_size, &Pubkey::default()); + let keypair = Keypair::new(); + bank.store_account(&keypair.pubkey(), &account); - // 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, - None, - TEST_SET_EXEMPT_RENT_EPOCH_MAX, - ); - assert_eq!(info.account_data_len_reclaimed, data_size as u64); + // 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, + None, + set_exempt_rent_epoch_max, + ); + assert_eq!(info.account_data_len_reclaimed, data_size as u64); + } + + // Collect rent for real + let accounts_data_size_delta_before_collecting_rent = + bank.load_accounts_data_size_delta(); + bank.collect_rent_eagerly(); + let accounts_data_size_delta_after_collecting_rent = + bank.load_accounts_data_size_delta(); + + let accounts_data_size_delta_delta = accounts_data_size_delta_after_collecting_rent + - accounts_data_size_delta_before_collecting_rent; + assert!(accounts_data_size_delta_delta < 0); + let reclaimed_data_size = accounts_data_size_delta_delta.saturating_neg() as usize; + + // Ensure the account is reclaimed by rent collection + assert_eq!(reclaimed_data_size, data_size,); } - - // Collect rent for real - let accounts_data_size_delta_before_collecting_rent = bank.load_accounts_data_size_delta(); - bank.collect_rent_eagerly(); - let accounts_data_size_delta_after_collecting_rent = bank.load_accounts_data_size_delta(); - - let accounts_data_size_delta_delta = accounts_data_size_delta_after_collecting_rent - - accounts_data_size_delta_before_collecting_rent; - assert!(accounts_data_size_delta_delta < 0); - let reclaimed_data_size = accounts_data_size_delta_delta.saturating_neg() as usize; - - // Ensure the account is reclaimed by rent collection - assert_eq!(reclaimed_data_size, data_size,); } #[test] diff --git a/runtime/src/rent_collector.rs b/runtime/src/rent_collector.rs index 3794805a4..bf6a9518c 100644 --- a/runtime/src/rent_collector.rs +++ b/runtime/src/rent_collector.rs @@ -34,8 +34,6 @@ impl Default for RentCollector { /// This enables us to get rid of the field completely. pub const RENT_EXEMPT_RENT_EPOCH: Epoch = Epoch::MAX; -pub const TEST_SET_EXEMPT_RENT_EPOCH_MAX: bool = false; - /// when rent is collected for this account, this is the action to apply to the account #[derive(Debug)] enum RentResult { @@ -231,6 +229,7 @@ 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); @@ -238,7 +237,7 @@ mod tests { address, account, /*filler_account_suffix:*/ None, - TEST_SET_EXEMPT_RENT_EPOCH_MAX, + set_exempt_rent_epoch_max, ) } } @@ -435,51 +434,56 @@ mod tests { #[test] fn test_collect_from_account_created_and_existing() { - let old_lamports = 1000; - let old_epoch = 1; - let new_epoch = 2; + for set_exempt_rent_epoch_max in [false, true] { + let old_lamports = 1000; + let old_epoch = 1; + let new_epoch = 2; - let (mut created_account, mut existing_account) = { - let account = AccountSharedData::from(Account { - lamports: old_lamports, - rent_epoch: old_epoch, - ..Account::default() - }); + let (mut created_account, mut existing_account) = { + let account = AccountSharedData::from(Account { + lamports: old_lamports, + rent_epoch: old_epoch, + ..Account::default() + }); - (account.clone(), account) - }; + (account.clone(), account) + }; - let rent_collector = default_rent_collector_clone_with_epoch(new_epoch); + 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); - assert!(created_account.lamports() < old_lamports); - assert_eq!( - created_account.lamports() + collected.rent_amount, - old_lamports - ); - assert_ne!(created_account.rent_epoch(), old_epoch); - assert_eq!(collected.account_data_len_reclaimed, 0); + // 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, + ); + assert!(created_account.lamports() < old_lamports); + assert_eq!( + created_account.lamports() + collected.rent_amount, + old_lamports + ); + assert_ne!(created_account.rent_epoch(), old_epoch); + 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, - None, // filler_account_suffix - TEST_SET_EXEMPT_RENT_EPOCH_MAX, - ); - assert!(existing_account.lamports() < old_lamports); - assert_eq!( - existing_account.lamports() + collected.rent_amount, - old_lamports - ); - assert_ne!(existing_account.rent_epoch(), old_epoch); - 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, + None, // filler_account_suffix + set_exempt_rent_epoch_max, + ); + assert!(existing_account.lamports() < old_lamports); + assert_eq!( + existing_account.lamports() + collected.rent_amount, + old_lamports + ); + assert_ne!(existing_account.rent_epoch(), old_epoch); + assert_eq!(collected.account_data_len_reclaimed, 0); - // newly created account should be collected for less rent; thus more remaining balance - assert!(created_account.lamports() > existing_account.lamports()); - assert_eq!(created_account.rent_epoch(), existing_account.rent_epoch()); + // newly created account should be collected for less rent; thus more remaining balance + assert!(created_account.lamports() > existing_account.lamports()); + assert_eq!(created_account.rent_epoch(), existing_account.rent_epoch()); + } } #[test] @@ -496,12 +500,15 @@ mod tests { // create a tested rent collector let rent_collector = default_rent_collector_clone_with_epoch(epoch); + // this test fails with set_exempt_rent_epoch_max = true atm + let set_exempt_rent_epoch_max = false; + // first mark account as being collected while being rent-exempt let collected = rent_collector.collect_from_existing_account( &pubkey, &mut account, None, // filler_account_suffix - TEST_SET_EXEMPT_RENT_EPOCH_MAX, + set_exempt_rent_epoch_max, ); assert_eq!(account.lamports(), huge_lamports); assert_eq!(collected, CollectedInfo::default()); @@ -514,7 +521,7 @@ mod tests { &pubkey, &mut account, None, // filler_account_suffix - TEST_SET_EXEMPT_RENT_EPOCH_MAX, + set_exempt_rent_epoch_max, ); assert_eq!(account.lamports(), tiny_lamports - collected.rent_amount); assert_ne!(collected, CollectedInfo::default()); @@ -522,55 +529,59 @@ mod tests { #[test] fn test_rent_exempt_sysvar() { - let tiny_lamports = 1; - let mut account = AccountSharedData::default(); - account.set_owner(sysvar::id()); - account.set_lamports(tiny_lamports); + for set_exempt_rent_epoch_max in [false, true] { + let tiny_lamports = 1; + let mut account = AccountSharedData::default(); + account.set_owner(sysvar::id()); + account.set_lamports(tiny_lamports); - let pubkey = solana_sdk::pubkey::new_rand(); + let pubkey = solana_sdk::pubkey::new_rand(); - assert_eq!(account.rent_epoch(), 0); + assert_eq!(account.rent_epoch(), 0); - let epoch = 3; - let rent_collector = default_rent_collector_clone_with_epoch(epoch); + let epoch = 3; + let rent_collector = default_rent_collector_clone_with_epoch(epoch); - let collected = rent_collector.collect_from_existing_account( - &pubkey, - &mut account, - None, // filler_account_suffix - TEST_SET_EXEMPT_RENT_EPOCH_MAX, - ); - assert_eq!(account.lamports(), 0); - assert_eq!(collected.rent_amount, 1); + let collected = rent_collector.collect_from_existing_account( + &pubkey, + &mut account, + None, // filler_account_suffix + set_exempt_rent_epoch_max, + ); + 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() { - solana_logger::setup(); - let account_lamports = 1; // must be *below* rent amount - let account_data_len = 567; - let account_rent_epoch = 11; - let mut account = AccountSharedData::from(Account { - lamports: account_lamports, // <-- must be below rent-exempt amount - data: vec![u8::default(); account_data_len], - rent_epoch: account_rent_epoch, - ..Account::default() - }); - let rent_collector = default_rent_collector_clone_with_epoch(account_rent_epoch + 1); + 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; + let account_rent_epoch = 11; + let mut account = AccountSharedData::from(Account { + lamports: account_lamports, // <-- must be below rent-exempt amount + data: vec![u8::default(); account_data_len], + rent_epoch: account_rent_epoch, + ..Account::default() + }); + 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, - None, // filler_account_suffix - TEST_SET_EXEMPT_RENT_EPOCH_MAX, - ); + let collected = rent_collector.collect_from_existing_account( + &Pubkey::new_unique(), + &mut account, + None, // filler_account_suffix + set_exempt_rent_epoch_max, + ); - assert_eq!(collected.rent_amount, account_lamports); - assert_eq!( - collected.account_data_len_reclaimed, - account_data_len as u64 - ); - assert_eq!(account, AccountSharedData::default()); + assert_eq!(collected.rent_amount, account_lamports); + assert_eq!( + collected.account_data_len_reclaimed, + account_data_len as u64 + ); + assert_eq!(account, AccountSharedData::default()); + } } }