From 1d657e7bfe9f396bbaea0e9836d39388a997ac75 Mon Sep 17 00:00:00 2001 From: jeff washington Date: Tue, 15 Nov 2022 11:27:13 -0600 Subject: [PATCH] feature: set rent_epoch to Epoch::MAX --- runtime/src/accounts.rs | 109 +++++++++++++++++++++++++++++----- runtime/src/bank.rs | 10 +++- runtime/src/rent_collector.rs | 55 ++++++++++++++--- sdk/src/feature_set.rs | 5 ++ 4 files changed, 152 insertions(+), 27 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index fbb49137198530..1ea432e2bb56e8 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -283,6 +283,8 @@ impl Accounts { }; let mut accumulated_accounts_data_size: usize = 0; + let set_exempt_rent_epoch_max = + feature_set.is_active(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id()); let mut accounts = account_keys .iter() .enumerate() @@ -317,6 +319,7 @@ impl Accounts { key, &mut account, self.accounts_db.filler_account_suffix.as_ref(), + set_exempt_rent_epoch_max, ) .rent_amount; (account, rent_due) @@ -1605,11 +1608,20 @@ mod tests { ) } + fn all_features_except(exclude: Option<&[Pubkey]>) -> FeatureSet { + let mut features = FeatureSet::all_enabled(); + if let Some(exclude) = exclude { + features.active.retain(|k, _v| !exclude.contains(k)); + } + features + } + fn load_accounts_with_fee( tx: Transaction, ka: &[TransactionAccount], lamports_per_signature: u64, error_counters: &mut TransactionErrorMetrics, + exclude_features: Option<&[Pubkey]>, ) -> Vec { load_accounts_with_fee_and_rent( tx, @@ -1617,7 +1629,7 @@ mod tests { lamports_per_signature, &RentCollector::default(), error_counters, - &FeatureSet::all_enabled(), + &all_features_except(exclude_features), &FeeStructure::default(), ) } @@ -1627,7 +1639,16 @@ mod tests { ka: &[TransactionAccount], error_counters: &mut TransactionErrorMetrics, ) -> Vec { - load_accounts_with_fee(tx, ka, 0, error_counters) + load_accounts_with_excluded_features(tx, ka, error_counters, None) + } + + fn load_accounts_with_excluded_features( + tx: Transaction, + ka: &[TransactionAccount], + error_counters: &mut TransactionErrorMetrics, + exclude_features: Option<&[Pubkey]>, + ) -> Vec { + load_accounts_with_fee(tx, ka, 0, error_counters, exclude_features) } #[test] @@ -1777,8 +1798,13 @@ mod tests { ); assert_eq!(fee, lamports_per_signature); - let loaded_accounts = - load_accounts_with_fee(tx, &accounts, lamports_per_signature, &mut error_counters); + let loaded_accounts = load_accounts_with_fee( + tx, + &accounts, + lamports_per_signature, + &mut error_counters, + None, + ); assert_eq!(error_counters.insufficient_funds, 1); assert_eq!(loaded_accounts.len(), 1); @@ -1858,7 +1884,9 @@ mod tests { lamports_per_signature, &rent_collector, &mut error_counters, - &FeatureSet::all_enabled(), + &all_features_except(Some(&[ + solana_sdk::feature_set::set_exempt_rent_epoch_max::id(), + ])), &FeeStructure::default(), ); assert_eq!(loaded_accounts.len(), 1); @@ -1925,7 +1953,12 @@ mod tests { instructions, ); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.account_not_found, 0); assert_eq!(loaded_accounts.len(), 1); @@ -2113,7 +2146,12 @@ mod tests { instructions, ); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.account_not_found, 0); assert_eq!(loaded_accounts.len(), 1); @@ -2344,7 +2382,12 @@ mod tests { instructions, ); let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); @@ -2357,7 +2400,12 @@ mod tests { message.account_keys = vec![key0, key1, key2]; // revert key change message.header.num_readonly_unsigned_accounts = 2; // mark both executables as readonly let tx = Transaction::new(&[&keypair], message, Hash::default()); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); @@ -2431,7 +2479,12 @@ mod tests { instructions, ); let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); @@ -2443,7 +2496,12 @@ mod tests { // Solution 1: include bpf_loader_upgradeable account message.account_keys = vec![key0, key1, bpf_loader_upgradeable::id()]; let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); @@ -2458,7 +2516,12 @@ mod tests { message.account_keys = vec![key0, key1, key2]; // revert key change message.header.num_readonly_unsigned_accounts = 2; // mark both executables as readonly let tx = Transaction::new(&[&keypair], message, Hash::default()); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); @@ -2516,7 +2579,12 @@ mod tests { instructions, ); let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); @@ -2536,8 +2604,12 @@ mod tests { ]; message.account_keys = vec![key0, key1, bpf_loader_upgradeable::id()]; let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = - load_accounts(tx, &accounts_with_upgradeable_loader, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts_with_upgradeable_loader, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); @@ -2552,7 +2624,12 @@ mod tests { message.account_keys = vec![key0, key1, key2]; // revert key change message.header.num_readonly_unsigned_accounts = 2; // extend readonly set to include programdata let tx = Transaction::new(&[&keypair], message, Hash::default()); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4cf59fe5d2271e..e8ae1510ba71ac 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5383,6 +5383,8 @@ impl Bank { pubkey, account, self.rc.accounts.accounts_db.filler_account_suffix.as_ref(), + self.feature_set + .is_active(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id()), )); time_collecting_rent_us += measure.as_us(); @@ -8056,6 +8058,7 @@ 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, }, @@ -8513,6 +8516,7 @@ pub(crate) mod tests { &keypairs[4].pubkey(), &mut account_copy, None, + TEST_SET_EXEMPT_RENT_EPOCH_MAX, ); assert_eq!(expected_rent.rent_amount, too_few_lamports); assert_eq!(account_copy.lamports(), 0); @@ -9960,8 +9964,9 @@ pub(crate) mod tests { let zero_lamport_pubkey = solana_sdk::pubkey::new_rand(); let rent_due_pubkey = solana_sdk::pubkey::new_rand(); let rent_exempt_pubkey = solana_sdk::pubkey::new_rand(); - - let mut bank = Arc::new(Bank::new_for_tests(&genesis_config)); + let mut bank = Bank::new_for_tests(&genesis_config); + bank.deactivate_feature(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id()); + let mut bank = Arc::new(bank); let zero_lamports = 0; let little_lamports = 1234; let large_lamports = 123_456_789; @@ -20034,6 +20039,7 @@ pub(crate) mod tests { &keypair.pubkey(), &mut account, None, + TEST_SET_EXEMPT_RENT_EPOCH_MAX, ); assert_eq!(info.account_data_len_reclaimed, data_size as u64); } diff --git a/runtime/src/rent_collector.rs b/runtime/src/rent_collector.rs index c44fbff1d9c52e..a6273b580fe2fd 100644 --- a/runtime/src/rent_collector.rs +++ b/runtime/src/rent_collector.rs @@ -32,15 +32,22 @@ impl Default for RentCollector { } } +/// When rent is collected from an exempt account, rent_epoch is set to this +/// value. The idea is to have a fixed, consistent value for rent_epoch for all accounts that do not collect rent. +/// This enables us to get rid of the field completely. +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 { - /// maybe collect rent later, leave account alone - LeaveAloneNoRent, + /// this account will never have rent collected from it + Exempt, /// collect rent CollectRent { new_rent_epoch: Epoch, - rent_due: u64, // lamports + rent_due: u64, // lamports, could be 0 }, } @@ -125,9 +132,20 @@ impl RentCollector { address: &Pubkey, account: &mut AccountSharedData, filler_account_suffix: Option<&Pubkey>, + set_exempt_rent_epoch_max: bool, ) -> CollectedInfo { - match self.calculate_rent_result(address, account, filler_account_suffix) { - RentResult::LeaveAloneNoRent => CollectedInfo::default(), + match self.calculate_rent_result( + address, + account, + filler_account_suffix, + set_exempt_rent_epoch_max, + ) { + RentResult::Exempt => { + if set_exempt_rent_epoch_max { + account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); + } + CollectedInfo::default() + } RentResult::CollectRent { new_rent_epoch, rent_due, @@ -158,16 +176,27 @@ impl RentCollector { address: &Pubkey, account: &impl ReadableAccount, filler_account_suffix: Option<&Pubkey>, + set_exempt_rent_epoch_max: bool, ) -> RentResult { if self.can_skip_rent_collection(address, account, filler_account_suffix) { - return RentResult::LeaveAloneNoRent; + return RentResult::Exempt; } match self.get_rent_due(account) { // Rent isn't collected for the next epoch. // Make sure to check exempt status again later in current epoch. - RentDue::Exempt => RentResult::LeaveAloneNoRent, + RentDue::Exempt => RentResult::Exempt, // Maybe collect rent later, leave account alone. - RentDue::Paying(0) => RentResult::LeaveAloneNoRent, + RentDue::Paying(0) => { + if set_exempt_rent_epoch_max { + RentResult::CollectRent { + new_rent_epoch: account.rent_epoch(), + rent_due: 0, + } + } else { + RentResult::Exempt + } + } + // Rent is collected for next epoch. RentDue::Paying(rent_due) => RentResult::CollectRent { new_rent_epoch: self.epoch + 1, @@ -239,7 +268,10 @@ mod tests { // initialize rent_epoch as created at this epoch account.set_rent_epoch(self.epoch); self.collect_from_existing_account( - address, account, /*filler_account_suffix:*/ None, + address, + account, + /*filler_account_suffix:*/ None, + TEST_SET_EXEMPT_RENT_EPOCH_MAX, ) } } @@ -278,6 +310,7 @@ mod tests { &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!( @@ -311,6 +344,7 @@ mod tests { &pubkey, &mut account, None, // filler_account_suffix + TEST_SET_EXEMPT_RENT_EPOCH_MAX, ); assert_eq!(account.lamports(), huge_lamports); assert_eq!(collected, CollectedInfo::default()); @@ -323,6 +357,7 @@ mod tests { &pubkey, &mut account, None, // filler_account_suffix + TEST_SET_EXEMPT_RENT_EPOCH_MAX, ); assert_eq!(account.lamports(), tiny_lamports - collected.rent_amount); assert_ne!(collected, CollectedInfo::default()); @@ -346,6 +381,7 @@ mod tests { &pubkey, &mut account, None, // filler_account_suffix + TEST_SET_EXEMPT_RENT_EPOCH_MAX, ); assert_eq!(account.lamports(), 0); assert_eq!(collected.rent_amount, 1); @@ -370,6 +406,7 @@ mod tests { &Pubkey::new_unique(), &mut account, None, // filler_account_suffix + TEST_SET_EXEMPT_RENT_EPOCH_MAX, ); assert_eq!(collected.rent_amount, account_lamports); diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index e33edc997590a4..a05f360b14ec59 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -498,6 +498,10 @@ pub mod account_hash_ignore_slot { solana_sdk::declare_id!("SVn36yVApPLYsa8koK3qUcy14zXDnqkNYWyUh1f4oK1"); } +pub mod set_exempt_rent_epoch_max { + solana_sdk::declare_id!("5wAGiy15X1Jb2hkHnPDCM8oB9V42VNA9ftNVFK84dEgv"); +} + pub mod relax_authority_signer_check_for_lookup_table_creation { solana_sdk::declare_id!("FKAcEvNgSY79RpqsPNUV5gDyumopH4cEHqUxyfm8b8Ap"); } @@ -653,6 +657,7 @@ lazy_static! { (enable_bpf_loader_extend_program_ix::id(), "enable bpf upgradeable loader ExtendProgram instruction #25234"), (enable_early_verification_of_account_modifications::id(), "enable early verification of account modifications #25899"), (account_hash_ignore_slot::id(), "ignore slot when calculating an account hash #28420"), + (set_exempt_rent_epoch_max::id(), "set rent epoch to Epoch::MAX for rent-exempt accounts #28683"), (on_load_preserve_rent_epoch_for_rent_exempt_accounts::id(), "on bank load account, do not try to fix up rent_epoch #28541"), (prevent_crediting_accounts_that_end_rent_paying::id(), "prevent crediting rent paying accounts #26606"), (cap_bpf_program_instruction_accounts::id(), "enforce max number of accounts per bpf program instruction #26628"),