diff --git a/Cargo.lock b/Cargo.lock index 9663dbab7b5..a82f7d56600 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9753,16 +9753,6 @@ dependencies = [ "solana-sdk-ids", ] -[[package]] -name = "solana-rent-debits" -version = "2.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f6f9113c6003492e74438d1288e30cffa8ccfdc2ef7b49b9e816d8034da18cd" -dependencies = [ - "solana-pubkey", - "solana-reward-info", -] - [[package]] name = "solana-reward-info" version = "2.2.1" @@ -9838,7 +9828,6 @@ dependencies = [ "solana-quic-definitions", "solana-rayon-threadlimit", "solana-rent", - "solana-rent-debits", "solana-rpc", "solana-rpc-client-api", "solana-runtime", @@ -10137,7 +10126,6 @@ dependencies = [ "solana-rayon-threadlimit", "solana-rent", "solana-rent-collector", - "solana-rent-debits", "solana-reward-info", "solana-runtime", "solana-runtime-transaction", @@ -10776,7 +10764,6 @@ dependencies = [ "solana-pubkey", "solana-rent", "solana-rent-collector", - "solana-rent-debits", "solana-sbpf", "solana-sdk-ids", "solana-secp256k1-program", diff --git a/Cargo.toml b/Cargo.toml index 8e2d7d4b920..9b3695b66d4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -486,7 +486,6 @@ solana-rayon-threadlimit = { path = "rayon-threadlimit", version = "=3.0.0" } solana-remote-wallet = { path = "remote-wallet", version = "=3.0.0", default-features = false } solana-rent = "2.2.1" solana-rent-collector = "2.2.1" -solana-rent-debits = "2.2.1" solana-reward-info = "2.2.1" solana-rpc = { path = "rpc", version = "=3.0.0" } solana-rpc-client = { path = "rpc-client", version = "=3.0.0", default-features = false } diff --git a/feature-set/src/lib.rs b/feature-set/src/lib.rs index fa23dd4c4c8..6d30154e916 100644 --- a/feature-set/src/lib.rs +++ b/feature-set/src/lib.rs @@ -151,7 +151,6 @@ impl FeatureSet { .is_active(&fix_alt_bn128_multiplication_input_length::id()), loosen_cpi_size_restriction: self.is_active(&loosen_cpi_size_restriction::id()), increase_tx_account_lock_limit: self.is_active(&increase_tx_account_lock_limit::id()), - disable_rent_fees_collection: self.is_active(&disable_rent_fees_collection::id()), enable_extend_program_checked: self.is_active(&enable_extend_program_checked::id()), formalize_loaded_transaction_data_size: self .is_active(&formalize_loaded_transaction_data_size::id()), diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 73f0e568b04..93830624b09 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -3441,9 +3441,9 @@ pub mod tests { assert_eq!(bank.get_balance(&keypair1.pubkey()), 3); } - #[test_case(true, true; "rent_collected")] - #[test_case(false, true; "rent_not_collected")] - #[test_case(true, false; "rent_not-collected_part_rent_disabled")] + #[test_case(true, true; "fee_payer_in_rent_partition")] + #[test_case(false, true; "fee_payer_not_in_rent_partition")] + #[test_case(true, false; "fee_payer_in_rent_partition-partitioned_rent_disabled")] fn test_transaction_result_does_not_affect_bankhash( fee_payer_in_rent_partition: bool, should_run_partitioned_rent_collection: bool, diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index d7c2330efde..cd1c33f0b51 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -7561,16 +7561,6 @@ dependencies = [ "solana-sdk-ids", ] -[[package]] -name = "solana-rent-debits" -version = "2.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f6f9113c6003492e74438d1288e30cffa8ccfdc2ef7b49b9e816d8034da18cd" -dependencies = [ - "solana-pubkey", - "solana-reward-info", -] - [[package]] name = "solana-reward-info" version = "2.2.1" @@ -7851,7 +7841,6 @@ dependencies = [ "solana-rayon-threadlimit", "solana-rent", "solana-rent-collector", - "solana-rent-debits", "solana-reward-info", "solana-runtime-transaction", "solana-sdk-ids", @@ -9104,7 +9093,6 @@ dependencies = [ "solana-pubkey", "solana-rent", "solana-rent-collector", - "solana-rent-debits", "solana-sdk-ids", "solana-slot-hashes", "solana-svm-callback", diff --git a/rpc/Cargo.toml b/rpc/Cargo.toml index 93748850d8a..5b3e918062c 100644 --- a/rpc/Cargo.toml +++ b/rpc/Cargo.toml @@ -114,7 +114,6 @@ solana-nonce-account = { workspace = true } solana-program-option = { workspace = true } solana-program-runtime = { workspace = true } solana-rent = { workspace = true } -solana-rent-debits = { workspace = true } solana-rpc = { path = ".", features = ["dev-context-only-utils"] } solana-runtime = { workspace = true, features = ["dev-context-only-utils"] } solana-runtime-transaction = { workspace = true, features = [ diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 0899d7e503d..cfe418105d0 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -164,7 +164,6 @@ impl TransactionStatusService { return_data, executed_units, fee_details, - rent_debits, .. } = committed_tx; @@ -175,18 +174,7 @@ impl TransactionStatusService { let pre_token_balances = Some(pre_token_balances); let post_token_balances = Some(post_token_balances); - let rewards = Some( - rent_debits - .into_unordered_rewards_iter() - .map(|(pubkey, reward_info)| Reward { - pubkey: pubkey.to_string(), - lamports: reward_info.lamports, - post_balance: reward_info.post_balance, - reward_type: Some(reward_info.reward_type), - commission: reward_info.commission, - }) - .collect(), - ); + let rewards = Some(vec![]); let loaded_addresses = transaction.get_loaded_addresses(); let mut transaction_status_meta = TransactionStatusMeta { status, @@ -346,7 +334,6 @@ pub(crate) mod tests { solana_nonce::{self as nonce, state::DurableNonce}, solana_nonce_account as nonce_account, solana_pubkey::Pubkey, - solana_rent_debits::RentDebits, solana_runtime::bank::{Bank, TransactionBalancesSet}, solana_signature::Signature, solana_signer::Signer, @@ -443,7 +430,6 @@ pub(crate) mod tests { .unwrap(); let expected_transaction = transaction.clone(); - let pubkey = Pubkey::new_unique(); let mut nonce_account = nonce_account::create_account(1).into_inner(); let durable_nonce = DurableNonce::from_blockhash(&Hash::new_from_array([42u8; 32])); @@ -454,9 +440,6 @@ pub(crate) mod tests { )) .unwrap(); - let mut rent_debits = RentDebits::default(); - rent_debits.insert(&pubkey, 123, 456); - let commit_result = Ok(CommittedTransaction { status: Ok(()), log_messages: None, @@ -464,7 +447,6 @@ pub(crate) mod tests { return_data: None, executed_units: 0, fee_details: FeeDetails::default(), - rent_debits, loaded_account_stats: TransactionLoadedAccountsStats::default(), }); @@ -591,7 +573,6 @@ pub(crate) mod tests { return_data: None, executed_units: 0, fee_details: FeeDetails::default(), - rent_debits: RentDebits::default(), loaded_account_stats: TransactionLoadedAccountsStats::default(), }); diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index b7d39f182b1..4823fcaa060 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -138,7 +138,6 @@ solana-pubkey = { workspace = true, features = ["rand"] } solana-rayon-threadlimit = { workspace = true } solana-rent = { workspace = true } solana-rent-collector = { workspace = true, features = ["serde"] } -solana-rent-debits = { workspace = true } solana-reward-info = { workspace = true } solana-runtime-transaction = { workspace = true } solana-sdk-ids = { workspace = true } @@ -192,7 +191,6 @@ rand_chacha = { workspace = true } solana-accounts-db = { workspace = true, features = ["dev-context-only-utils"] } solana-builtins = { workspace = true, features = ["dev-context-only-utils"] } solana-logger = { workspace = true } -solana-rent-debits = { workspace = true, features = ["dev-context-only-utils"] } # See order-crates-for-publishing.py for using this unusual `path = "."` solana-runtime = { path = ".", features = ["dev-context-only-utils"] } solana-runtime-transaction = { workspace = true, features = [ diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs index 33ec1332e08..00aaf7fa3d0 100644 --- a/runtime/benches/accounts.rs +++ b/runtime/benches/accounts.rs @@ -40,8 +40,7 @@ fn bench_accounts_create(bencher: &mut Bencher) { #[bench] fn bench_accounts_squash(bencher: &mut Bencher) { - let (mut genesis_config, _) = create_genesis_config(100_000); - genesis_config.rent.burn_percent = 100; // Avoid triggering an assert in Bank::distribute_rent_to_validators() + let (genesis_config, _) = create_genesis_config(100_000); let mut prev_bank = Arc::new(Bank::new_with_paths_for_benches( &genesis_config, vec![PathBuf::from("bench_a1")], diff --git a/runtime/src/account_saver.rs b/runtime/src/account_saver.rs index ca364ae9aa7..e921a7c8286 100644 --- a/runtime/src/account_saver.rs +++ b/runtime/src/account_saver.rs @@ -189,7 +189,6 @@ mod tests { }, solana_nonce_account as nonce_account, solana_program_runtime::execution_budget::SVMTransactionExecutionBudget, - solana_rent_debits::RentDebits, solana_sdk_ids::native_loader, solana_signer::{signers::Signers, Signer}, solana_svm::{ @@ -280,8 +279,6 @@ mod tests { fee_details: FeeDetails::default(), rollback_accounts: RollbackAccounts::default(), compute_budget: SVMTransactionExecutionBudget::default(), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size: 0, }; @@ -291,8 +288,6 @@ mod tests { fee_details: FeeDetails::default(), rollback_accounts: RollbackAccounts::default(), compute_budget: SVMTransactionExecutionBudget::default(), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size: 0, }; @@ -354,8 +349,6 @@ mod tests { fee_payer_account: from_account_pre.clone(), }, compute_budget: SVMTransactionExecutionBudget::default(), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size: 0, }; @@ -449,8 +442,6 @@ mod tests { fee_payer_account: from_account_pre.clone(), }, compute_budget: SVMTransactionExecutionBudget::default(), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size: 0, }; @@ -557,8 +548,6 @@ mod tests { nonce: nonce.clone(), }, compute_budget: SVMTransactionExecutionBudget::default(), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size: 0, }; diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 2b0c4f0843e..becd3e3b539 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -86,7 +86,7 @@ use { IncrementalAccountsHash, MerkleOrLatticeAccountsHash, }, accounts_index::{IndexKey, ScanConfig, ScanResult}, - accounts_partition::{self, Partition, PartitionIndex}, + accounts_partition::{self, Partition}, accounts_update_notifier_interface::AccountsUpdateNotifier, ancestors::{Ancestors, AncestorsForSerialization}, blockhash_queue::BlockhashQueue, @@ -126,8 +126,7 @@ use { invoke_context::BuiltinFunctionWithContext, loaded_programs::ProgramCacheEntry, }, solana_pubkey::Pubkey, - solana_rent_collector::{CollectedInfo, RentCollector}, - solana_rent_debits::RentDebits, + solana_rent_collector::RentCollector, solana_reward_info::RewardInfo, solana_runtime_transaction::{ runtime_transaction::RuntimeTransaction, transaction_with_meta::TransactionWithMeta, @@ -139,7 +138,7 @@ use { solana_slot_history::{Check, SlotHistory}, solana_stake_interface::state::Delegation, solana_svm::{ - account_loader::{collect_rent_from_account, LoadedTransaction}, + account_loader::{update_rent_exempt_status_for_account, LoadedTransaction}, account_overrides::AccountOverrides, program_loader::load_program_with_pubkey, transaction_balances::BalanceCollector, @@ -446,7 +445,6 @@ pub struct BankFieldsToDeserialize { pub(crate) collector_id: Pubkey, pub(crate) collector_fees: u64, pub(crate) fee_rate_governor: FeeRateGovernor, - pub(crate) collected_rent: u64, pub(crate) rent_collector: RentCollector, pub(crate) epoch_schedule: EpochSchedule, pub(crate) inflation: Inflation, @@ -493,7 +491,6 @@ pub struct BankFieldsToSerialize { pub collector_id: Pubkey, pub collector_fees: u64, pub fee_rate_governor: FeeRateGovernor, - pub collected_rent: u64, pub rent_collector: RentCollector, pub epoch_schedule: EpochSchedule, pub inflation: Inflation, @@ -545,7 +542,6 @@ impl PartialEq for Bank { collector_id, collector_fees, fee_rate_governor, - collected_rent, rent_collector, epoch_schedule, inflation, @@ -610,7 +606,6 @@ impl PartialEq for Bank { && collector_id == &other.collector_id && collector_fees.load(Relaxed) == other.collector_fees.load(Relaxed) && fee_rate_governor == &other.fee_rate_governor - && collected_rent.load(Relaxed) == other.collected_rent.load(Relaxed) && rent_collector == &other.rent_collector && epoch_schedule == &other.epoch_schedule && *inflation.read().unwrap() == *other.inflation.read().unwrap() @@ -655,7 +650,6 @@ impl BankFieldsToSerialize { collector_id: Pubkey::default(), collector_fees: u64::default(), fee_rate_governor: FeeRateGovernor::default(), - collected_rent: u64::default(), rent_collector: RentCollector::default(), epoch_schedule: EpochSchedule::default(), inflation: Inflation::default(), @@ -827,9 +821,6 @@ pub struct Bank { /// Track cluster signature throughput and adjust fee rate pub(crate) fee_rate_governor: FeeRateGovernor, - /// Rent that has been collected - collected_rent: AtomicU64, - /// latest rent collector, knows the epoch rent_collector: RentCollector, @@ -1107,7 +1098,6 @@ impl Bank { collector_id: Pubkey::default(), collector_fees: AtomicU64::default(), fee_rate_governor: FeeRateGovernor::default(), - collected_rent: AtomicU64::default(), rent_collector: RentCollector::default(), epoch_schedule: EpochSchedule::default(), inflation: Arc::>::default(), @@ -1330,7 +1320,6 @@ impl Bank { genesis_creation_time: parent.genesis_creation_time, slots_per_year: parent.slots_per_year, epoch_schedule, - collected_rent: AtomicU64::new(0), rent_collector: Self::get_rent_collector_from(&parent.rent_collector, epoch), max_tick_height: slot .checked_add(1) @@ -1840,7 +1829,6 @@ impl Bank { collector_id: fields.collector_id, collector_fees: AtomicU64::new(fields.collector_fees), fee_rate_governor: fields.fee_rate_governor, - collected_rent: AtomicU64::new(fields.collected_rent), // clone()-ing is needed to consider a gated behavior in rent_collector rent_collector: Self::get_rent_collector_from(&fields.rent_collector, fields.epoch), epoch_schedule: fields.epoch_schedule, @@ -2040,7 +2028,6 @@ impl Bank { collector_id: self.collector_id, collector_fees: self.collector_fees.load(Relaxed), fee_rate_governor: self.fee_rate_governor.clone(), - collected_rent: self.collected_rent.load(Relaxed), rent_collector: self.rent_collector.clone(), epoch_schedule: self.epoch_schedule.clone(), inflation: *self.inflation.read().unwrap(), @@ -2654,9 +2641,8 @@ impl Bank { let mut hash = self.hash.write().unwrap(); if *hash == Hash::default() { // finish up any deferred changes to account state - self.collect_rent_eagerly(); + self.run_partitioned_rent_exempt_status_updates(); self.distribute_transaction_fee_details(); - self.distribute_rent_fees(); self.update_slot_history(); self.run_incinerator(); @@ -3796,8 +3782,6 @@ impl Bank { .store_cached(to_store, transactions.as_deref()); }); - self.collect_rent(&processing_results); - // Cached vote and stake accounts are synchronized with accounts-db // after each transaction. let ((), update_stakes_cache_us) = @@ -3867,19 +3851,11 @@ impl Bank { ProcessedTransaction::Executed(executed_tx) => { let execution_details = executed_tx.execution_details; let LoadedTransaction { - rent_debits, accounts: loaded_accounts, fee_details, .. } = executed_tx.loaded_transaction; - // Rent is only collected for successfully executed transactions - let rent_debits = if execution_details.was_successful() { - rent_debits - } else { - RentDebits::default() - }; - Ok(CommittedTransaction { status: execution_details.status, log_messages: execution_details.log_messages, @@ -3887,7 +3863,6 @@ impl Bank { return_data: execution_details.return_data, executed_units, fee_details, - rent_debits, loaded_account_stats: TransactionLoadedAccountsStats { loaded_accounts_count: loaded_accounts.len(), loaded_accounts_data_size, @@ -3900,7 +3875,6 @@ impl Bank { inner_instructions: None, return_data: None, executed_units, - rent_debits: RentDebits::default(), fee_details: fees_only_tx.fee_details, loaded_account_stats: TransactionLoadedAccountsStats { loaded_accounts_count: fees_only_tx.rollback_accounts.count(), @@ -3912,17 +3886,6 @@ impl Bank { .collect() } - fn collect_rent(&self, processing_results: &[TransactionProcessingResult]) { - let collected_rent = processing_results - .iter() - .filter_map(|processing_result| processing_result.processed_transaction()) - .filter_map(|processed_tx| processed_tx.executed_transaction()) - .filter(|executed_tx| executed_tx.was_successful()) - .map(|executed_tx| executed_tx.loaded_transaction.rent) - .sum(); - self.collected_rent.fetch_add(collected_rent, Relaxed); - } - fn run_incinerator(&self) { if let Some((account, _)) = self.get_account_modified_since_parent_with_fixed_root(&incinerator::id()) @@ -4060,7 +4023,7 @@ impl Bank { accounts_written_this_slot } - fn collect_rent_eagerly(&self) { + fn run_partitioned_rent_exempt_status_updates(&self) { if self.lazy_rent_collection.load(Relaxed) { return; } @@ -4111,17 +4074,17 @@ impl Bank { if parallel { let thread_pool = &self.rc.accounts.accounts_db.thread_pool; thread_pool.install(|| { - ranges.into_par_iter().for_each(|range| { - self.collect_rent_in_range(range.0, range.1, &rent_metrics) + ranges.into_par_iter().for_each(|(_, subrange_full)| { + self.update_rent_exempt_status_in_range(subrange_full, &rent_metrics) }); }); } } if !parallel { // collect serially - partitions - .into_iter() - .for_each(|partition| self.collect_rent_in_partition(partition, &rent_metrics)); + partitions.into_iter().for_each(|partition| { + self.update_rent_exempt_status_in_partition(partition, &rent_metrics) + }); } measure.stop(); datapoint_info!( @@ -4170,14 +4133,7 @@ impl Bank { .is_active(&feature_set::skip_rent_rewrites::id()) } - /// true if rent fees should be collected (i.e. disable_rent_fees_collection is NOT enabled) - fn should_collect_rent(&self) -> bool { - !self - .feature_set - .is_active(&feature_set::disable_rent_fees_collection::id()) - } - - /// Collect rent from `accounts` + /// Update rent exempt status for `accounts` /// /// This fn is called inside a parallel loop from `collect_rent_in_partition()`. Avoid adding /// any code that causes contention on shared memory/data (i.e. do not update atomic metrics). @@ -4186,14 +4142,10 @@ impl Bank { /// reduce at the end of its parallel loop. If possible, place data/computation that cause /// contention/take locks in the return struct and process them in /// `collect_rent_from_partition()` after reducing the parallel loop. - fn collect_rent_from_accounts( + fn update_rent_exempt_status_for_accounts( &self, mut accounts: Vec<(Pubkey, AccountSharedData, Slot)>, - rent_paying_pubkeys: Option<&HashSet>, - partition_index: PartitionIndex, ) -> CollectRentFromAccountsInfo { - let mut rent_debits = RentDebits::default(); - let mut total_rent_collected_info = CollectedInfo::default(); let mut accounts_to_store = Vec::<(&Pubkey, &AccountSharedData)>::with_capacity(accounts.len()); let mut time_collecting_rent_us = 0; @@ -4207,18 +4159,15 @@ impl Bank { let mut skipped_rewrites = Vec::default(); for (pubkey, account, _loaded_slot) in accounts.iter_mut() { let rent_epoch_pre = account.rent_epoch(); - let (rent_collected_info, collect_rent_us) = measure_us!(collect_rent_from_account( - &self.feature_set.runtime_features(), + let ((), collect_rent_us) = measure_us!(update_rent_exempt_status_for_account( &self.rent_collector, - pubkey, account )); time_collecting_rent_us += collect_rent_us; let rent_epoch_post = account.rent_epoch(); // did the account change in any way due to rent collection? - let rent_epoch_changed = rent_epoch_post != rent_epoch_pre; - let account_changed = rent_collected_info.rent_amount != 0 || rent_epoch_changed; + let account_changed = rent_epoch_post != rent_epoch_pre; // always store the account, regardless if it changed or not let always_store_accounts = @@ -4230,45 +4179,15 @@ impl Bank { // ensures we verify the whole on-chain state (= all accounts) // via the bank delta hash slowly once per an epoch. if account_changed || always_store_accounts { - if rent_collected_info.rent_amount > 0 { - if let Some(rent_paying_pubkeys) = rent_paying_pubkeys { - if !rent_paying_pubkeys.contains(pubkey) { - let partition_from_pubkey = accounts_partition::partition_from_pubkey( - pubkey, - self.epoch_schedule.slots_per_epoch, - ); - // Submit datapoint instead of assert while we verify this is correct - datapoint_warn!( - "bank-unexpected_rent_paying_pubkey", - ("slot", self.slot(), i64), - ("pubkey", pubkey.to_string(), String), - ("partition_index", partition_index, i64), - ("partition_from_pubkey", partition_from_pubkey, i64) - ); - warn!( - "Collecting rent from unexpected pubkey: {}, slot: {}, parent_slot: {:?}, \ - partition_index: {}, partition_from_pubkey: {}", - pubkey, - self.slot(), - self.parent().map(|bank| bank.slot()), - partition_index, - partition_from_pubkey, - ); - } - } - } else { - debug_assert_eq!(rent_collected_info.rent_amount, 0); - if rent_epoch_changed { - datapoint_info!( - "bank-rent_collection_updated_only_rent_epoch", - ("slot", self.slot(), i64), - ("pubkey", pubkey.to_string(), String), - ("rent_epoch_pre", rent_epoch_pre, i64), - ("rent_epoch_post", rent_epoch_post, i64), - ); - } + if account_changed { + datapoint_info!( + "bank-rent_collection_updated_only_rent_epoch", + ("slot", self.slot(), i64), + ("pubkey", pubkey.to_string(), String), + ("rent_epoch_pre", rent_epoch_pre, i64), + ("rent_epoch_post", rent_epoch_post, i64), + ); } - total_rent_collected_info += rent_collected_info; accounts_to_store.push((pubkey, account)); } else if !account_changed && !can_skip_rewrites @@ -4281,7 +4200,6 @@ impl Bank { let hash = AccountsDb::hash_account(account, pubkey); skipped_rewrites.push((*pubkey, hash)); } - rent_debits.insert(pubkey, rent_collected_info.rent_amount, account.lamports()); } if !accounts_to_store.is_empty() { @@ -4294,48 +4212,24 @@ impl Bank { CollectRentFromAccountsInfo { skipped_rewrites, - rent_collected_info: total_rent_collected_info, - rent_rewards: rent_debits.into_unordered_rewards_iter().collect(), time_collecting_rent_us, time_storing_accounts_us, num_accounts: accounts.len(), } } - /// convert 'partition' to a pubkey range and 'collect_rent_in_range' - fn collect_rent_in_partition(&self, partition: Partition, metrics: &RentMetrics) { + /// convert 'partition' to a pubkey range and 'update_rent_exempt_status_in_range' + fn update_rent_exempt_status_in_partition(&self, partition: Partition, metrics: &RentMetrics) { let subrange_full = accounts_partition::pubkey_range_from_partition(partition); - self.collect_rent_in_range(partition, subrange_full, metrics) + self.update_rent_exempt_status_in_range(subrange_full, metrics) } - /// get all pubkeys that we expect to be rent-paying or None, if this was not initialized at load time (that should only exist in test cases) - fn get_rent_paying_pubkeys(&self, partition: &Partition) -> Option> { - self.rc - .accounts - .accounts_db - .accounts_index - .rent_paying_accounts_by_partition - .get() - .and_then(|rent_paying_accounts| { - rent_paying_accounts.is_initialized().then(|| { - accounts_partition::get_partition_end_indexes(partition) - .into_iter() - .flat_map(|end_index| { - rent_paying_accounts.get_pubkeys_in_partition_index(end_index) - }) - .cloned() - .collect::>() - }) - }) - } - - /// load accounts with pubkeys in 'subrange_full' - /// collect rent and update 'account.rent_epoch' as necessary - /// store accounts, whether rent was collected or not (depending on whether we skipping rewrites is enabled) + /// load accounts with pubkeys in 'subrange_full', update + /// 'account.rent_epoch' as necessary, and store accounts, whether rent was + /// collected or not (depending on whether we skipping rewrites is enabled) /// update bank's rewrites set for all rewrites that were skipped - fn collect_rent_in_range( + fn update_rent_exempt_status_in_range( &self, - partition: Partition, subrange_full: RangeInclusive, metrics: &RentMetrics, ) { @@ -4348,9 +4242,6 @@ impl Bank { hold_range.stop(); metrics.hold_range_us.fetch_add(hold_range.as_us(), Relaxed); - let rent_paying_pubkeys_ = self.get_rent_paying_pubkeys(&partition); - let rent_paying_pubkeys = rent_paying_pubkeys_.as_ref(); - // divide the range into num_threads smaller ranges and process in parallel // Note that 'pubkey_range_from_partition' cannot easily be re-used here to break the range smaller. // It has special handling of 0..0 and partition_count changes affect all ranges unevenly. @@ -4360,7 +4251,7 @@ impl Bank { let end_prefix_inclusive = accounts_partition::prefix_from_pubkey(subrange_full.end()); let range = end_prefix_inclusive - start_prefix; let increment = range / num_threads; - let mut results = (0..num_threads) + let results = (0..num_threads) .into_par_iter() .map(|chunk| { let offset = |chunk| start_prefix + chunk * increment; @@ -4385,7 +4276,7 @@ impl Bank { .load_to_collect_rent_eagerly(&self.ancestors, subrange) }); CollectRentInPartitionInfo::new( - self.collect_rent_from_accounts(accounts, rent_paying_pubkeys, partition.1), + self.update_rent_exempt_status_for_accounts(accounts), Duration::from_nanos(measure_load_accounts.as_ns()), ) }) @@ -4406,16 +4297,6 @@ impl Bank { .accounts .hold_range_in_memory(&subrange_full, false, thread_pool); - self.collected_rent - .fetch_add(results.rent_collected, Relaxed); - self.update_accounts_data_size_delta_off_chain( - -(results.accounts_data_size_reclaimed as i64), - ); - self.rewards - .write() - .unwrap() - .append(&mut results.rent_rewards); - metrics .load_us .fetch_add(results.time_loading_accounts_us, Relaxed); @@ -7297,8 +7178,6 @@ enum ApplyFeatureActivationsCaller { #[derive(Debug, Default)] struct CollectRentFromAccountsInfo { skipped_rewrites: Vec<(Pubkey, AccountHash)>, - rent_collected_info: CollectedInfo, - rent_rewards: Vec<(Pubkey, RewardInfo)>, time_collecting_rent_us: u64, time_storing_accounts_us: u64, num_accounts: usize, @@ -7309,9 +7188,6 @@ struct CollectRentFromAccountsInfo { #[derive(Debug, Default)] struct CollectRentInPartitionInfo { skipped_rewrites: Vec<(Pubkey, AccountHash)>, - rent_collected: u64, - accounts_data_size_reclaimed: u64, - rent_rewards: Vec<(Pubkey, RewardInfo)>, time_loading_accounts_us: u64, time_collecting_rent_us: u64, time_storing_accounts_us: u64, @@ -7325,9 +7201,6 @@ impl CollectRentInPartitionInfo { fn new(info: CollectRentFromAccountsInfo, time_loading_accounts: Duration) -> Self { Self { skipped_rewrites: info.skipped_rewrites, - rent_collected: info.rent_collected_info.rent_amount, - accounts_data_size_reclaimed: info.rent_collected_info.account_data_len_reclaimed, - rent_rewards: info.rent_rewards, time_loading_accounts_us: time_loading_accounts.as_micros() as u64, time_collecting_rent_us: info.time_collecting_rent_us, time_storing_accounts_us: info.time_storing_accounts_us, @@ -7343,11 +7216,6 @@ impl CollectRentInPartitionInfo { fn reduce(lhs: Self, rhs: Self) -> Self { Self { skipped_rewrites: [lhs.skipped_rewrites, rhs.skipped_rewrites].concat(), - rent_collected: lhs.rent_collected.saturating_add(rhs.rent_collected), - accounts_data_size_reclaimed: lhs - .accounts_data_size_reclaimed - .saturating_add(rhs.accounts_data_size_reclaimed), - rent_rewards: [lhs.rent_rewards, rhs.rent_rewards].concat(), time_loading_accounts_us: lhs .time_loading_accounts_us .saturating_add(rhs.time_loading_accounts_us), diff --git a/runtime/src/bank/fee_distribution.rs b/runtime/src/bank/fee_distribution.rs index be7bb6e44a6..39274665be0 100644 --- a/runtime/src/bank/fee_distribution.rs +++ b/runtime/src/bank/fee_distribution.rs @@ -1,7 +1,7 @@ use { super::Bank, crate::bank::CollectorFeeDetails, - log::{debug, warn}, + log::debug, solana_account::{ReadableAccount, WritableAccount}, solana_fee::FeeFeatures, solana_fee_structure::FeeBudgetLimits, @@ -10,7 +10,6 @@ use { solana_runtime_transaction::transaction_with_meta::TransactionWithMeta, solana_svm_rent_collector::svm_rent_collector::SVMRentCollector, solana_system_interface::program as system_program, - solana_vote::vote_account::VoteAccountsHashMap, std::{result::Result, sync::atomic::Ordering::Relaxed}, thiserror::Error, }; @@ -48,10 +47,6 @@ impl Bank { // earning transaction fees are fairly distributed by stake. And missing the opportunity // (not producing a block as a leader) earns nothing. So, being online is incentivized as a // form of transaction fees as well. - // - // On the other hand, rent fees are distributed under slightly different philosophy, while - // still being stake-weighted. - // Ref: distribute_rent_to_validators pub(super) fn distribute_transaction_fee_details(&self) { let fee_details = self.collector_fee_details.read().unwrap(); if fee_details.total() == 0 { @@ -175,177 +170,17 @@ impl Bank { self.store_account(pubkey, &account); Ok(account.lamports()) } - - // Distribute collected rent fees for this slot to staked validators (excluding stakers) - // according to stake. - // - // The nature of rent fee is the cost of doing business, every validator has to hold (or have - // access to) the same list of accounts, so we pay according to stake, which is a rough proxy for - // value to the network. - // - // Currently, rent distribution doesn't consider given validator's uptime at all (this might - // change). That's because rent should be rewarded for the storage resource utilization cost. - // It's treated differently from transaction fees, which is for the computing resource - // utilization cost. - // - // We can't use collector_id (which is rotated according to stake-weighted leader schedule) - // as an approximation to the ideal rent distribution to simplify and avoid this per-slot - // computation for the distribution (time: N log N, space: N acct. stores; N = # of - // validators). - // The reason is that rent fee doesn't need to be incentivized for throughput unlike transaction - // fees - // - // Ref: distribute_transaction_fee_details - fn distribute_rent_to_validators( - &self, - vote_accounts: &VoteAccountsHashMap, - rent_to_be_distributed: u64, - ) { - let mut total_staked = 0; - - // Collect the stake associated with each validator. - // Note that a validator may be present in this vector multiple times if it happens to have - // more than one staked vote account somehow - let mut validator_stakes = vote_accounts - .iter() - .filter_map(|(_vote_pubkey, (staked, account))| { - if *staked == 0 { - None - } else { - total_staked += *staked; - Some((*account.node_pubkey(), *staked)) - } - }) - .collect::>(); - - #[cfg(test)] - if validator_stakes.is_empty() { - // some tests bank.freezes() with bad staking state - self.capitalization - .fetch_sub(rent_to_be_distributed, Relaxed); - return; - } - #[cfg(not(test))] - assert!(!validator_stakes.is_empty()); - - // Sort first by stake and then by validator identity pubkey for determinism. - // If two items are still equal, their relative order does not matter since - // both refer to the same validator. - validator_stakes.sort_unstable_by(|(pubkey1, staked1), (pubkey2, staked2)| { - (staked1, pubkey1).cmp(&(staked2, pubkey2)).reverse() - }); - - let mut rent_distributed_in_initial_round = 0; - let validator_rent_shares = validator_stakes - .into_iter() - .map(|(pubkey, staked)| { - let rent_share = (((staked as u128) * (rent_to_be_distributed as u128)) - / (total_staked as u128)) - .try_into() - .unwrap(); - rent_distributed_in_initial_round += rent_share; - (pubkey, rent_share) - }) - .collect::>(); - - // Leftover lamports after fraction calculation, will be paid to validators starting from highest stake - // holder - let mut leftover_lamports = rent_to_be_distributed - rent_distributed_in_initial_round; - - let mut rent_to_burn: u64 = 0; - let mut rewards = vec![]; - validator_rent_shares - .into_iter() - .for_each(|(pubkey, rent_share)| { - let rent_to_be_paid = if leftover_lamports > 0 { - leftover_lamports -= 1; - rent_share + 1 - } else { - rent_share - }; - if rent_to_be_paid > 0 { - match self.deposit_fees(&pubkey, rent_to_be_paid) { - Ok(post_balance) => { - rewards.push(( - pubkey, - RewardInfo { - reward_type: RewardType::Rent, - lamports: rent_to_be_paid as i64, - post_balance, - commission: None, - }, - )); - } - Err(err) => { - debug!( - "Burned {} lamport rent fee instead of sending to {} due to {}", - rent_to_be_paid, pubkey, err - ); - - // overflow adding lamports or resulting account is invalid - // so burn lamports and track lamports burned per slot - rent_to_burn = rent_to_burn.saturating_add(rent_to_be_paid); - } - } - } - }); - self.rewards.write().unwrap().append(&mut rewards); - - if rent_to_burn > 0 { - self.capitalization.fetch_sub(rent_to_burn, Relaxed); - datapoint_warn!( - "bank-burned_rent", - ("slot", self.slot(), i64), - ("num_lamports", rent_to_burn, i64) - ); - } - - assert_eq!(leftover_lamports, 0); - } - - pub(super) fn distribute_rent_fees(&self) { - let total_rent_collected = self.collected_rent.load(Relaxed); - - if !self.should_collect_rent() { - if total_rent_collected != 0 { - warn!("Rent fees collection is disabled, yet total rent collected was non zero! Total rent collected: {total_rent_collected}"); - } - return; - } - - let (burned_portion, rent_to_be_distributed) = self - .rent_collector - .rent - .calculate_burn(total_rent_collected); - - debug!( - "distributed rent: {} (rounded from: {}, burned: {})", - rent_to_be_distributed, total_rent_collected, burned_portion - ); - self.capitalization.fetch_sub(burned_portion, Relaxed); - - if rent_to_be_distributed == 0 { - return; - } - - self.distribute_rent_to_validators(&self.vote_accounts(), rent_to_be_distributed); - } } #[cfg(test)] pub mod tests { use { super::*, - crate::genesis_utils::{ - create_genesis_config, create_genesis_config_with_leader, - create_genesis_config_with_vote_accounts, ValidatorVoteKeypairs, - }, + crate::genesis_utils::{create_genesis_config, create_genesis_config_with_leader}, solana_account::AccountSharedData, - solana_native_token::sol_to_lamports, solana_pubkey as pubkey, solana_rent::Rent, solana_signer::Signer, - solana_svm_rent_collector::rent_state::RentState, std::sync::RwLock, }; @@ -513,202 +348,6 @@ pub mod tests { ); } - #[test] - fn test_distribute_rent_to_validators_rent_paying() { - solana_logger::setup(); - - const RENT_PER_VALIDATOR: u64 = 55; - const TOTAL_RENT: u64 = RENT_PER_VALIDATOR * 4; - - let empty_validator = ValidatorVoteKeypairs::new_rand(); - let rent_paying_validator = ValidatorVoteKeypairs::new_rand(); - let becomes_rent_exempt_validator = ValidatorVoteKeypairs::new_rand(); - let rent_exempt_validator = ValidatorVoteKeypairs::new_rand(); - let keypairs = vec![ - &empty_validator, - &rent_paying_validator, - &becomes_rent_exempt_validator, - &rent_exempt_validator, - ]; - let genesis_config_info = create_genesis_config_with_vote_accounts( - sol_to_lamports(1000.), - &keypairs, - vec![sol_to_lamports(1000.); 4], - ); - let mut genesis_config = genesis_config_info.genesis_config; - genesis_config.rent = Rent::default(); // Ensure rent is non-zero, as genesis_utils sets Rent::free by default - - let bank = Bank::new_for_tests(&genesis_config); - let rent_exempt_minimum = bank.rent_collector().get_rent().minimum_balance(0); - - // Make one validator have an empty identity account - let mut empty_validator_account = bank - .get_account_with_fixed_root(&empty_validator.node_keypair.pubkey()) - .unwrap(); - empty_validator_account.set_lamports(0); - bank.store_account( - &empty_validator.node_keypair.pubkey(), - &empty_validator_account, - ); - - // Make one validator almost rent-exempt, less RENT_PER_VALIDATOR - let mut becomes_rent_exempt_validator_account = bank - .get_account_with_fixed_root(&becomes_rent_exempt_validator.node_keypair.pubkey()) - .unwrap(); - becomes_rent_exempt_validator_account - .set_lamports(rent_exempt_minimum - RENT_PER_VALIDATOR); - bank.store_account( - &becomes_rent_exempt_validator.node_keypair.pubkey(), - &becomes_rent_exempt_validator_account, - ); - - // Make one validator rent-exempt - let mut rent_exempt_validator_account = bank - .get_account_with_fixed_root(&rent_exempt_validator.node_keypair.pubkey()) - .unwrap(); - rent_exempt_validator_account.set_lamports(rent_exempt_minimum); - bank.store_account( - &rent_exempt_validator.node_keypair.pubkey(), - &rent_exempt_validator_account, - ); - - let get_rent_state = |bank: &Bank, address: &Pubkey| -> RentState { - let account = bank - .get_account_with_fixed_root(address) - .unwrap_or_default(); - bank.rent_collector().get_account_rent_state(&account) - }; - - // Assert starting RentStates - assert_eq!( - get_rent_state(&bank, &empty_validator.node_keypair.pubkey()), - RentState::Uninitialized - ); - assert_eq!( - get_rent_state(&bank, &rent_paying_validator.node_keypair.pubkey()), - RentState::RentPaying { - lamports: 42, - data_size: 0, - } - ); - assert_eq!( - get_rent_state(&bank, &becomes_rent_exempt_validator.node_keypair.pubkey()), - RentState::RentPaying { - lamports: rent_exempt_minimum - RENT_PER_VALIDATOR, - data_size: 0, - } - ); - assert_eq!( - get_rent_state(&bank, &rent_exempt_validator.node_keypair.pubkey()), - RentState::RentExempt - ); - - let old_empty_validator_lamports = bank.get_balance(&empty_validator.node_keypair.pubkey()); - let old_rent_paying_validator_lamports = - bank.get_balance(&rent_paying_validator.node_keypair.pubkey()); - let old_becomes_rent_exempt_validator_lamports = - bank.get_balance(&becomes_rent_exempt_validator.node_keypair.pubkey()); - let old_rent_exempt_validator_lamports = - bank.get_balance(&rent_exempt_validator.node_keypair.pubkey()); - - bank.distribute_rent_to_validators(&bank.vote_accounts(), TOTAL_RENT); - - let new_empty_validator_lamports = bank.get_balance(&empty_validator.node_keypair.pubkey()); - let new_rent_paying_validator_lamports = - bank.get_balance(&rent_paying_validator.node_keypair.pubkey()); - let new_becomes_rent_exempt_validator_lamports = - bank.get_balance(&becomes_rent_exempt_validator.node_keypair.pubkey()); - let new_rent_exempt_validator_lamports = - bank.get_balance(&rent_exempt_validator.node_keypair.pubkey()); - - // Assert ending balances; rent should be withheld if test is active and ending RentState - // is RentPaying, ie. empty_validator and rent_paying_validator - assert_eq!(old_empty_validator_lamports, new_empty_validator_lamports); - - assert_eq!( - old_rent_paying_validator_lamports, - new_rent_paying_validator_lamports - ); - - assert_eq!( - old_becomes_rent_exempt_validator_lamports + RENT_PER_VALIDATOR, - new_becomes_rent_exempt_validator_lamports - ); - - assert_eq!( - old_rent_exempt_validator_lamports + RENT_PER_VALIDATOR, - new_rent_exempt_validator_lamports - ); - - // Assert ending RentStates - assert_eq!( - RentState::Uninitialized, - get_rent_state(&bank, &empty_validator.node_keypair.pubkey()), - ); - assert_eq!( - RentState::RentPaying { - lamports: old_rent_paying_validator_lamports, - data_size: 0, - }, - get_rent_state(&bank, &rent_paying_validator.node_keypair.pubkey()), - ); - assert_eq!( - RentState::RentExempt, - get_rent_state(&bank, &becomes_rent_exempt_validator.node_keypair.pubkey()), - ); - assert_eq!( - RentState::RentExempt, - get_rent_state(&bank, &rent_exempt_validator.node_keypair.pubkey()), - ); - } - - #[test] - fn test_distribute_rent_to_validators_invalid_owner() { - struct TestCase { - use_invalid_owner: bool, - } - - impl TestCase { - fn new(use_invalid_owner: bool) -> Self { - Self { use_invalid_owner } - } - } - - for test_case in [TestCase::new(false), TestCase::new(true)] { - let genesis_config_info = - create_genesis_config_with_leader(0, &Pubkey::new_unique(), 100); - let mut genesis_config = genesis_config_info.genesis_config; - genesis_config.rent = Rent::default(); // Ensure rent is non-zero, as genesis_utils sets Rent::free by default - - let bank = Bank::new_for_tests(&genesis_config); - - let initial_balance = 1_000_000; - let account_owner = if test_case.use_invalid_owner { - Pubkey::new_unique() - } else { - system_program::id() - }; - let account = AccountSharedData::new(initial_balance, 0, &account_owner); - bank.store_account(bank.collector_id(), &account); - - let initial_capitalization = bank.capitalization(); - let rent_fees = 100; - bank.distribute_rent_to_validators(&bank.vote_accounts(), rent_fees); - let new_capitalization = bank.capitalization(); - let new_balance = bank.get_balance(bank.collector_id()); - - if test_case.use_invalid_owner { - assert_eq!(initial_balance, new_balance); - assert_eq!(initial_capitalization - rent_fees, new_capitalization); - assert_eq!(bank.rewards.read().unwrap().len(), 0); - } else { - assert_eq!(initial_balance + rent_fees, new_balance); - assert_eq!(initial_capitalization, new_capitalization); - assert_eq!(bank.rewards.read().unwrap().len(), 1); - } - } - } - #[test] fn test_distribute_transaction_fee_details_normal() { let genesis = create_genesis_config(0); diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index f8e44058f0c..0bbbf289e6e 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -43,7 +43,7 @@ use { accounts_index::{ AccountIndex, AccountSecondaryIndexes, IndexKey, ScanConfig, ScanError, ITER_BATCH_SIZE, }, - accounts_partition::{self, PartitionIndex, RentPayingAccountsByPartition}, + accounts_partition, ancestors::Ancestors, }, solana_client_traits::SyncClient, @@ -111,12 +111,10 @@ use { MAX_PERMITTED_DATA_LENGTH, }, solana_system_transaction as system_transaction, solana_sysvar as sysvar, - solana_time_utils::years_as_slots, solana_timings::ExecuteTimings, solana_transaction::{ sanitized::SanitizedTransaction, Transaction, TransactionVerificationMode, }, - solana_transaction_context::TransactionAccount, solana_transaction_error::{TransactionError, TransactionResult as Result}, solana_vote_interface::state::TowerSync, solana_vote_program::{ @@ -463,257 +461,6 @@ fn test_bank_capitalization() { ); } -fn rent_with_exemption_threshold(exemption_threshold: f64) -> Rent { - Rent { - lamports_per_byte_year: 1, - exemption_threshold, - burn_percent: 10, - } -} - -#[test] -/// 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(); - - let (mut genesis_config, _mint_keypair) = create_genesis_config_no_tx_fee(10); - - 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, bank_forks_1) = Bank::new_with_bank_forks_for_tests(&genesis_config); - let bank = new_bank_from_parent_with_bank_forks( - bank_forks_1.as_ref(), - root_bank, - &Pubkey::default(), - slot, - ); - - let (root_bank_2, bank_forks_2) = Bank::new_with_bank_forks_for_tests(&genesis_config); - let bank_with_success_txs = new_bank_from_parent_with_bank_forks( - bank_forks_2.as_ref(), - root_bank_2, - &Pubkey::default(), - slot, - ); - - assert_eq!(bank.last_blockhash(), genesis_config.hash()); - - let min_balance = genesis_config.rent.minimum_balance(0); - let plenty_of_lamports = min_balance + 1; - 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, 0, &Pubkey::default()), - AccountSharedData::new(plenty_of_lamports, 0, &Pubkey::default()), - AccountSharedData::new(plenty_of_lamports, 0, &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); - 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()), - ); - 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( - bank: &Bank, - keypairs: &[Keypair], - mock_program_id: Pubkey, - generic_rent_due_for_system_account: u64, -) { - let mut account_pairs: Vec = Vec::with_capacity(keypairs.len() - 1); - account_pairs.push(( - keypairs[0].pubkey(), - AccountSharedData::new( - generic_rent_due_for_system_account + 2, - 0, - &Pubkey::default(), - ), - )); - account_pairs.push(( - keypairs[1].pubkey(), - AccountSharedData::new( - generic_rent_due_for_system_account + 2, - 0, - &Pubkey::default(), - ), - )); - account_pairs.push(( - keypairs[2].pubkey(), - AccountSharedData::new( - generic_rent_due_for_system_account + 2, - 0, - &Pubkey::default(), - ), - )); - account_pairs.push(( - keypairs[3].pubkey(), - AccountSharedData::new( - generic_rent_due_for_system_account + 2, - 0, - &Pubkey::default(), - ), - )); - account_pairs.push(( - keypairs[4].pubkey(), - AccountSharedData::new(10, 0, &Pubkey::default()), - )); - account_pairs.push(( - keypairs[5].pubkey(), - AccountSharedData::new(10, 0, &Pubkey::default()), - )); - account_pairs.push(( - keypairs[6].pubkey(), - AccountSharedData::new( - (2 * generic_rent_due_for_system_account) + 24, - 0, - &Pubkey::default(), - ), - )); - - account_pairs.push(( - keypairs[8].pubkey(), - AccountSharedData::new( - generic_rent_due_for_system_account + 2 + 929, - 0, - &Pubkey::default(), - ), - )); - account_pairs.push(( - keypairs[9].pubkey(), - AccountSharedData::new(10, 0, &Pubkey::default()), - )); - - // Feeding to MockProgram to test read only rent behaviour - account_pairs.push(( - keypairs[10].pubkey(), - AccountSharedData::new( - generic_rent_due_for_system_account + 3, - 0, - &Pubkey::default(), - ), - )); - account_pairs.push(( - keypairs[11].pubkey(), - AccountSharedData::new(generic_rent_due_for_system_account + 3, 0, &mock_program_id), - )); - account_pairs.push(( - keypairs[12].pubkey(), - AccountSharedData::new(generic_rent_due_for_system_account + 3, 0, &mock_program_id), - )); - account_pairs.push(( - keypairs[13].pubkey(), - AccountSharedData::new(14, 22, &mock_program_id), - )); - - for account_pair in account_pairs.iter() { - bank.store_account(&account_pair.0, &account_pair.1); - } -} - -fn create_child_bank_for_rent_test( - root_bank: Arc, - genesis_config: &GenesisConfig, - bank_forks: &RwLock, - mock_builtin: Option<(Pubkey, BuiltinFunctionWithContext)>, -) -> Arc { - let mut bank = Bank::new_from_parent( - root_bank, - &Pubkey::default(), - years_as_slots( - 2.0, - &genesis_config.poh_config.target_tick_duration, - genesis_config.ticks_per_slot, - ) as u64, - ); - bank.rent_collector.slots_per_year = 421_812.0; - if let Some((program_id, builtin_function)) = mock_builtin { - bank.add_mockup_builtin(program_id, builtin_function); - } - bank_forks - .write() - .unwrap() - .insert(bank) - .clone_without_scheduler() -} - /// if asserter returns true, check the capitalization /// Checking the capitalization requires that the bank be a root and the slot be flushed. /// All tests are getting converted to use the write cache, so over time, each caller will be visited to throttle this input. @@ -818,488 +565,6 @@ fn test_store_account_and_update_capitalization_unchanged() { assert_eq!(account, bank.get_account(&pubkey).unwrap()); } -#[test] -#[ignore] -fn test_rent_distribution() { - solana_logger::setup(); - - let bootstrap_validator_pubkey = solana_pubkey::new_rand(); - let bootstrap_validator_stake_lamports = 30; - let mut genesis_config = create_genesis_config_with_leader( - 10, - &bootstrap_validator_pubkey, - bootstrap_validator_stake_lamports, - ) - .genesis_config; - // While we are preventing new accounts left in a rent-paying state, not quite ready to rip - // out all the rent assessment tests. Just deactivate the feature for now. - genesis_config - .accounts - .remove(&feature_set::require_rent_exempt_accounts::id()) - .unwrap(); - - genesis_config.epoch_schedule = EpochSchedule::custom( - MINIMUM_SLOTS_PER_EPOCH, - genesis_config.epoch_schedule.leader_schedule_slot_offset, - false, - ); - - genesis_config.rent = rent_with_exemption_threshold(2.0); - - let rent = Rent::free(); - - let validator_1_pubkey = solana_pubkey::new_rand(); - let validator_1_stake_lamports = 20; - let validator_1_staking_keypair = Keypair::new(); - let validator_1_voting_keypair = Keypair::new(); - - let validator_1_vote_account = vote_state::create_account( - &validator_1_voting_keypair.pubkey(), - &validator_1_pubkey, - 0, - validator_1_stake_lamports, - ); - - let validator_1_stake_account = stake_state::create_account( - &validator_1_staking_keypair.pubkey(), - &validator_1_voting_keypair.pubkey(), - &validator_1_vote_account, - &rent, - validator_1_stake_lamports, - ); - - genesis_config.accounts.insert( - validator_1_pubkey, - Account::new(42, 0, &system_program::id()), - ); - genesis_config.accounts.insert( - validator_1_staking_keypair.pubkey(), - Account::from(validator_1_stake_account), - ); - genesis_config.accounts.insert( - validator_1_voting_keypair.pubkey(), - Account::from(validator_1_vote_account), - ); - - let validator_2_pubkey = solana_pubkey::new_rand(); - let validator_2_stake_lamports = 20; - let validator_2_staking_keypair = Keypair::new(); - let validator_2_voting_keypair = Keypair::new(); - - let validator_2_vote_account = vote_state::create_account( - &validator_2_voting_keypair.pubkey(), - &validator_2_pubkey, - 0, - validator_2_stake_lamports, - ); - - let validator_2_stake_account = stake_state::create_account( - &validator_2_staking_keypair.pubkey(), - &validator_2_voting_keypair.pubkey(), - &validator_2_vote_account, - &rent, - validator_2_stake_lamports, - ); - - genesis_config.accounts.insert( - validator_2_pubkey, - Account::new(42, 0, &system_program::id()), - ); - genesis_config.accounts.insert( - validator_2_staking_keypair.pubkey(), - Account::from(validator_2_stake_account), - ); - genesis_config.accounts.insert( - validator_2_voting_keypair.pubkey(), - Account::from(validator_2_vote_account), - ); - - let validator_3_pubkey = solana_pubkey::new_rand(); - let validator_3_stake_lamports = 30; - let validator_3_staking_keypair = Keypair::new(); - let validator_3_voting_keypair = Keypair::new(); - - let validator_3_vote_account = vote_state::create_account( - &validator_3_voting_keypair.pubkey(), - &validator_3_pubkey, - 0, - validator_3_stake_lamports, - ); - - let validator_3_stake_account = stake_state::create_account( - &validator_3_staking_keypair.pubkey(), - &validator_3_voting_keypair.pubkey(), - &validator_3_vote_account, - &rent, - validator_3_stake_lamports, - ); - - genesis_config.accounts.insert( - validator_3_pubkey, - Account::new(42, 0, &system_program::id()), - ); - genesis_config.accounts.insert( - validator_3_staking_keypair.pubkey(), - Account::from(validator_3_stake_account), - ); - genesis_config.accounts.insert( - validator_3_voting_keypair.pubkey(), - Account::from(validator_3_vote_account), - ); - - genesis_config.rent = rent_with_exemption_threshold(10.0); - - let mut bank = Bank::new_for_tests(&genesis_config); - // Enable rent collection - bank.rent_collector.epoch = 5; - bank.rent_collector.slots_per_year = 192.0; - let (bank, _bank_forks) = bank.wrap_with_bank_forks_for_tests(); - - let payer = Keypair::new(); - let payer_account = AccountSharedData::new(400, 0, &system_program::id()); - bank.store_account_and_update_capitalization(&payer.pubkey(), &payer_account); - - let payee = Keypair::new(); - let payee_account = AccountSharedData::new(70, 1, &system_program::id()); - bank.store_account_and_update_capitalization(&payee.pubkey(), &payee_account); - - let bootstrap_validator_initial_balance = bank.get_balance(&bootstrap_validator_pubkey); - - let tx = system_transaction::transfer(&payer, &payee.pubkey(), 180, genesis_config.hash()); - - let result = bank.process_transaction(&tx); - assert_eq!(result, Ok(())); - - let mut total_rent_deducted = 0; - - // 400 - 128(Rent) - 180(Transfer) - assert_eq!(bank.get_balance(&payer.pubkey()), 92); - total_rent_deducted += 128; - - // 70 - 70(Rent) + 180(Transfer) - 21(Rent) - assert_eq!(bank.get_balance(&payee.pubkey()), 159); - total_rent_deducted += 70 + 21; - - let previous_capitalization = bank.capitalization.load(Relaxed); - - bank.freeze(); - - assert_eq!(bank.collected_rent.load(Relaxed), total_rent_deducted); - - let burned_portion = - total_rent_deducted * u64::from(bank.rent_collector.rent.burn_percent) / 100; - let rent_to_be_distributed = total_rent_deducted - burned_portion; - - let bootstrap_validator_portion = - ((bootstrap_validator_stake_lamports * rent_to_be_distributed) as f64 / 100.0) as u64 + 1; // Leftover lamport - assert_eq!( - bank.get_balance(&bootstrap_validator_pubkey), - bootstrap_validator_portion + bootstrap_validator_initial_balance - ); - - // Since, validator 1 and validator 2 has equal smallest stake, it comes down to comparison - // between their pubkey. - let tweak_1 = u64::from(validator_1_pubkey > validator_2_pubkey); - let validator_1_portion = - ((validator_1_stake_lamports * rent_to_be_distributed) as f64 / 100.0) as u64 + tweak_1; - assert_eq!( - bank.get_balance(&validator_1_pubkey), - validator_1_portion + 42 - tweak_1, - ); - - // Since, validator 1 and validator 2 has equal smallest stake, it comes down to comparison - // between their pubkey. - let tweak_2 = u64::from(validator_2_pubkey > validator_1_pubkey); - let validator_2_portion = - ((validator_2_stake_lamports * rent_to_be_distributed) as f64 / 100.0) as u64 + tweak_2; - assert_eq!( - bank.get_balance(&validator_2_pubkey), - validator_2_portion + 42 - tweak_2, - ); - - let validator_3_portion = - ((validator_3_stake_lamports * rent_to_be_distributed) as f64 / 100.0) as u64 + 1; - assert_eq!( - bank.get_balance(&validator_3_pubkey), - validator_3_portion + 42 - ); - - let current_capitalization = bank.capitalization.load(Relaxed); - - // only slot history is newly created - let sysvar_and_builtin_program_delta = - min_rent_exempt_balance_for_sysvars(&bank, &[sysvar::slot_history::id()]); - assert_eq!( - previous_capitalization - (current_capitalization - sysvar_and_builtin_program_delta), - burned_portion - ); - - assert!(bank.calculate_and_verify_capitalization(true)); - - assert_eq!( - rent_to_be_distributed, - bank.rewards - .read() - .unwrap() - .iter() - .map(|(address, reward)| { - if reward.lamports > 0 { - assert_eq!(reward.reward_type, RewardType::Rent); - if *address == validator_2_pubkey { - assert_eq!(reward.post_balance, validator_2_portion + 42 - tweak_2); - } else if *address == validator_3_pubkey { - assert_eq!(reward.post_balance, validator_3_portion + 42); - } - reward.lamports as u64 - } else { - 0 - } - }) - .sum::() - ); -} - -#[test] -fn test_rent_exempt_executable_account() { - let (mut genesis_config, mint_keypair) = create_genesis_config(100_000); - genesis_config.rent = rent_with_exemption_threshold(1000.0); - - let (root_bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); - let bank = - create_child_bank_for_rent_test(root_bank, &genesis_config, bank_forks.as_ref(), None); - - let account_pubkey = solana_pubkey::new_rand(); - let account_balance = 1; - let mut account = AccountSharedData::new(account_balance, 0, &solana_pubkey::new_rand()); - account.set_executable(true); - account.set_owner(bpf_loader_upgradeable::id()); - bank.store_account(&account_pubkey, &account); - - let transfer_lamports = 1; - let tx = system_transaction::transfer( - &mint_keypair, - &account_pubkey, - transfer_lamports, - genesis_config.hash(), - ); - assert_matches!( - bank.process_transaction(&tx), - Err(TransactionError::InstructionError(0, _)) - ); - assert_eq!(bank.get_balance(&account_pubkey), account_balance); -} - -#[test] -#[ignore] -#[allow(clippy::cognitive_complexity)] -fn test_rent_complex() { - solana_logger::setup(); - let mock_program_id = Pubkey::from([2u8; 32]); - - #[derive(Serialize, Deserialize)] - enum MockInstruction { - Deduction, - } - - declare_process_instruction!(MockBuiltin, 1, |invoke_context| { - let transaction_context = &invoke_context.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let instruction_data = instruction_context.get_instruction_data(); - if let Ok(instruction) = bincode::deserialize(instruction_data) { - match instruction { - MockInstruction::Deduction => { - instruction_context - .try_borrow_instruction_account(transaction_context, 1)? - .checked_add_lamports(1)?; - instruction_context - .try_borrow_instruction_account(transaction_context, 2)? - .checked_sub_lamports(1)?; - Ok(()) - } - } - } else { - Err(InstructionError::InvalidInstructionData) - } - }); - - let (mut genesis_config, _mint_keypair) = create_genesis_config(10); - let mut keypairs: Vec = Vec::with_capacity(14); - for _i in 0..14 { - keypairs.push(Keypair::new()); - } - - genesis_config.rent = rent_with_exemption_threshold(1000.0); - - let (root_bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); - // until we completely transition to the eager rent collection, - // we must ensure lazy rent collection doesn't get broken! - root_bank.restore_old_behavior_for_fragile_tests(); - let bank = create_child_bank_for_rent_test( - root_bank, - &genesis_config, - bank_forks.as_ref(), - Some((mock_program_id, MockBuiltin::vm)), - ); - - assert_eq!(bank.last_blockhash(), genesis_config.hash()); - - let slots_elapsed: u64 = (0..=bank.epoch) - .map(|epoch| { - bank.rent_collector - .epoch_schedule - .get_slots_in_epoch(epoch + 1) - }) - .sum(); - let generic_rent_due_for_system_account = bank - .rent_collector - .rent - .due( - bank.get_minimum_balance_for_rent_exemption(0) - 1, - 0, - slots_elapsed as f64 / bank.rent_collector.slots_per_year, - ) - .lamports(); - - store_accounts_for_rent_test( - &bank, - &keypairs, - mock_program_id, - generic_rent_due_for_system_account, - ); - - let magic_rent_number = 131; // yuck, derive this value programmatically one day - - 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(), - ); - let t3 = system_transaction::transfer( - &keypairs[4], - &keypairs[5].pubkey(), - 1, - genesis_config.hash(), - ); - let t4 = system_transaction::transfer( - &keypairs[6], - &keypairs[7].pubkey(), - generic_rent_due_for_system_account + 1, - genesis_config.hash(), - ); - let t5 = system_transaction::transfer( - &keypairs[8], - &keypairs[9].pubkey(), - 929, - genesis_config.hash(), - ); - - let account_metas = vec![ - AccountMeta::new(keypairs[10].pubkey(), true), - AccountMeta::new(keypairs[11].pubkey(), true), - AccountMeta::new(keypairs[12].pubkey(), true), - AccountMeta::new_readonly(keypairs[13].pubkey(), false), - ]; - let deduct_instruction = - Instruction::new_with_bincode(mock_program_id, &MockInstruction::Deduction, account_metas); - let t6 = Transaction::new_signed_with_payer( - &[deduct_instruction], - Some(&keypairs[10].pubkey()), - &[&keypairs[10], &keypairs[11], &keypairs[12]], - genesis_config.hash(), - ); - - let txs = vec![t6, t5, t1, t2, t3, t4]; - let res = bank.process_transactions(txs.iter()); - - assert_eq!(res.len(), 6); - assert_eq!(res[0], Ok(())); - assert_eq!(res[1], Ok(())); - assert_eq!(res[2], Ok(())); - assert_eq!(res[3], Ok(())); - assert_eq!(res[4], Err(TransactionError::AccountNotFound)); - assert_eq!(res[5], Ok(())); - - bank.freeze(); - - let mut rent_collected = 0; - - // 48992 - generic_rent_due_for_system_account(Rent) - 1(transfer) - assert_eq!(bank.get_balance(&keypairs[0].pubkey()), 1); - rent_collected += generic_rent_due_for_system_account; - - // 48992 - generic_rent_due_for_system_account(Rent) + 1(transfer) - assert_eq!(bank.get_balance(&keypairs[1].pubkey()), 3); - rent_collected += generic_rent_due_for_system_account; - - // 48992 - generic_rent_due_for_system_account(Rent) - 1(transfer) - assert_eq!(bank.get_balance(&keypairs[2].pubkey()), 1); - rent_collected += generic_rent_due_for_system_account; - - // 48992 - generic_rent_due_for_system_account(Rent) + 1(transfer) - assert_eq!(bank.get_balance(&keypairs[3].pubkey()), 3); - rent_collected += generic_rent_due_for_system_account; - - // No rent deducted - assert_eq!(bank.get_balance(&keypairs[4].pubkey()), 10); - assert_eq!(bank.get_balance(&keypairs[5].pubkey()), 10); - - // 98004 - generic_rent_due_for_system_account(Rent) - 48991(transfer) - assert_eq!(bank.get_balance(&keypairs[6].pubkey()), 23); - rent_collected += generic_rent_due_for_system_account; - - // 0 + 48990(transfer) - magic_rent_number(Rent) - assert_eq!( - bank.get_balance(&keypairs[7].pubkey()), - generic_rent_due_for_system_account + 1 - magic_rent_number - ); - - // Epoch should be updated - // Rent deducted on store side - let account8 = bank.get_account(&keypairs[7].pubkey()).unwrap(); - // Epoch should be set correctly. - assert_eq!(account8.rent_epoch(), bank.epoch + 1); - rent_collected += magic_rent_number; - - // 49921 - generic_rent_due_for_system_account(Rent) - 929(Transfer) - assert_eq!(bank.get_balance(&keypairs[8].pubkey()), 2); - rent_collected += generic_rent_due_for_system_account; - - let account10 = bank.get_account(&keypairs[9].pubkey()).unwrap(); - // Account was overwritten at load time, since it didn't have sufficient balance to pay rent - // Then, at store time we deducted `magic_rent_number` rent for the current epoch, once it has balance - assert_eq!(account10.rent_epoch(), bank.epoch + 1); - // account data is blank now - assert_eq!(account10.data().len(), 0); - // 10 - 10(Rent) + 929(Transfer) - magic_rent_number(Rent) - assert_eq!(account10.lamports(), 929 - magic_rent_number); - rent_collected += magic_rent_number + 10; - - // 48993 - generic_rent_due_for_system_account(Rent) - assert_eq!(bank.get_balance(&keypairs[10].pubkey()), 3); - rent_collected += generic_rent_due_for_system_account; - - // 48993 - generic_rent_due_for_system_account(Rent) + 1(Addition by program) - assert_eq!(bank.get_balance(&keypairs[11].pubkey()), 4); - rent_collected += generic_rent_due_for_system_account; - - // 48993 - generic_rent_due_for_system_account(Rent) - 1(Deduction by program) - assert_eq!(bank.get_balance(&keypairs[12].pubkey()), 2); - rent_collected += generic_rent_due_for_system_account; - - // No rent for read-only account - assert_eq!(bank.get_balance(&keypairs[13].pubkey()), 14); - - // Bank's collected rent should be sum of rent collected from all accounts - assert_eq!(bank.collected_rent.load(Relaxed), rent_collected); -} - #[test] fn test_rent_eager_across_epoch_without_gap() { let (mut bank, _bank_forks) = create_simple_test_arc_bank(1); @@ -1654,9 +919,8 @@ impl Bank { } } -#[test_case(true; "enable rent fees collection")] -#[test_case(false; "disable rent fees collection")] -fn test_rent_eager_collect_rent_in_partition(should_collect_rent: bool) { +#[test] +fn test_rent_eager_collect_rent_in_partition() { solana_logger::setup(); let (mut genesis_config, _mint_keypair) = create_genesis_config(1_000_000); activate_all_features(&mut genesis_config); @@ -1668,30 +932,18 @@ fn test_rent_eager_collect_rent_in_partition(should_collect_rent: bool) { .accounts .remove(&feature_set::disable_partitioned_rent_collection::id()) .unwrap(); - if should_collect_rent { - genesis_config - .accounts - .remove(&feature_set::disable_rent_fees_collection::id()) - .unwrap(); - } let zero_lamport_pubkey = solana_pubkey::new_rand(); let rent_due_pubkey = solana_pubkey::new_rand(); let rent_exempt_pubkey = solana_pubkey::new_rand(); let mut bank = Arc::new(Bank::new_for_tests(&genesis_config)); - - assert_eq!(should_collect_rent, bank.should_collect_rent()); + let genesis_slot = 0; let zero_lamports = 0; let little_lamports = 1234; let large_lamports = 123_456_789; // genesis_config.epoch_schedule.slots_per_epoch == 432_000 and is unsuitable for this test let some_slot = MINIMUM_SLOTS_PER_EPOCH; // chosen to cause epoch to be +1 - let rent_collected = if bank.should_collect_rent() { - 1 /* this is a function of 'some_slot' */ - } else { - 0 - }; bank.store_account( &zero_lamport_pubkey, @@ -1706,19 +958,6 @@ fn test_rent_eager_collect_rent_in_partition(should_collect_rent: bool) { &AccountSharedData::new(large_lamports, 0, &Pubkey::default()), ); - let genesis_slot = 0; - - let previous_epoch = bank.epoch(); - bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::default(), some_slot)); - let current_epoch = bank.epoch(); - assert_eq!(previous_epoch + 1, current_epoch); - - assert_eq!(bank.collected_rent.load(Relaxed), 0); - assert_eq!( - bank.get_account(&rent_due_pubkey).unwrap().lamports(), - little_lamports - ); - assert_eq!(bank.get_account(&rent_due_pubkey).unwrap().rent_epoch(), 0); assert_eq!(bank.slots_by_pubkey(&rent_due_pubkey), vec![genesis_slot]); assert_eq!( bank.slots_by_pubkey(&rent_exempt_pubkey), @@ -1729,21 +968,16 @@ fn test_rent_eager_collect_rent_in_partition(should_collect_rent: bool) { vec![genesis_slot] ); - assert_eq!(bank.collected_rent.load(Relaxed), 0); - bank.collect_rent_in_partition((0, 0, 1), &RentMetrics::default()); // all range + let previous_epoch = bank.epoch(); + bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::default(), some_slot)); + bank.freeze(); + let current_epoch = bank.epoch(); + assert_eq!(previous_epoch + 1, current_epoch); - assert_eq!(bank.collected_rent.load(Relaxed), rent_collected); + // Rent epoch is not updated for rent paying accounts assert_eq!( - bank.get_account(&rent_due_pubkey).unwrap().lamports(), - little_lamports - rent_collected - ); - assert!( - bank.get_account(&rent_due_pubkey).unwrap().rent_epoch() == current_epoch + 1 - || !bank.should_collect_rent() - ); - assert_eq!( - bank.get_account(&rent_exempt_pubkey).unwrap().lamports(), - large_lamports + bank.get_account(&rent_due_pubkey).unwrap().rent_epoch(), + previous_epoch ); assert_eq!( bank.get_account(&rent_exempt_pubkey).unwrap().rent_epoch(), @@ -1816,15 +1050,11 @@ fn test_collect_rent_from_accounts() { account3.set_rent_epoch(0); // stake accounts in genesis have a rent epoch of 0 // loaded from previous slot, so we skip rent collection on it - let _result = later_bank.collect_rent_from_accounts( - vec![ - (address1, account1, later_slot - 1), - (address2, account2, later_slot - 1), - (address3, account3, later_slot - 1), - ], - None, - PartitionIndex::default(), - ); + let _result = later_bank.update_rent_exempt_status_for_accounts(vec![ + (address1, account1, later_slot - 1), + (address2, account2, later_slot - 1), + (address3, account3, later_slot - 1), + ]); let deltas = later_bank .rc @@ -1901,8 +1131,8 @@ fn test_rent_eager_collect_rent_zero_lamport_deterministic() { assert_eq!(hash1_with_zero, hash1_without_zero); assert_ne!(hash1_with_zero, Hash::default()); - bank2_with_zero.collect_rent_in_partition((0, 0, 1), &RentMetrics::default()); // all - bank2_without_zero.collect_rent_in_partition((0, 0, 1), &RentMetrics::default()); // all + bank2_with_zero.update_rent_exempt_status_in_partition((0, 0, 1), &RentMetrics::default()); // all + bank2_without_zero.update_rent_exempt_status_in_partition((0, 0, 1), &RentMetrics::default()); // all bank2_with_zero.freeze(); let hash2_with_zero = bank2_with_zero.hash(); @@ -3304,7 +2534,6 @@ fn test_load_and_execute_commit_transactions_fees_only() { return_data: None, executed_units: 0, fee_details: FeeDetails::new(5000, 0), - rent_debits: RentDebits::default(), loaded_account_stats: TransactionLoadedAccountsStats { loaded_accounts_count: 2, loaded_accounts_data_size: nonce_size as u32, @@ -6562,26 +5791,26 @@ fn test_bank_hash_consistency() { if bank.slot == 0 { assert_eq!( bank.hash().to_string(), - "5b72TRrdMhGED3boghe55CyX8hmnpYt7RTMrwrHTrNpP", + "CTg8Vq5RjXhfp332YC9DHQjAfFueLPimszv9i6xBFgPW", ); } if bank.slot == 32 { assert_eq!( bank.hash().to_string(), - "2k9XFkra1XyobQb4Z73xSEgFsUdp87cmftYfWEpXQoah" + "CK1siD9yP37R4ErCECKg1rofsEAk9fdGpsfpMQnSvHBL" ); } if bank.slot == 64 { assert_eq!( bank.hash().to_string(), - "GpePwzXm6nomkj9CKPU4qwvrFnBcjYRrs3hSoRgHN5CP" + "5h8yw8oU78G4JeVB28U9ZjZpV5fCgm9gA8LfVJF8YD8W" ); } if bank.slot == 128 { assert_eq!( bank.hash().to_string(), - "8GjxSMXwRe7AyFZoF9R7XVcTC5wYoVKvdawU8ysRcBid" + "87cnbyVPkbfpQkjuQ5sCKXNYhvUbpzHNac6GJv1BnqDM" ); break; } @@ -9667,69 +8896,6 @@ fn test_tx_return_data() { } } -#[test] -fn test_load_and_execute_commit_transactions_rent_debits() { - let (mut genesis_config, mint_keypair) = create_genesis_config(sol_to_lamports(1.)); - genesis_config.rent = Rent::default(); - let (bank, _bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); - let bank = Bank::new_from_parent( - bank, - &Pubkey::new_unique(), - genesis_config.epoch_schedule.get_first_slot_in_epoch(1), - ); - let amount = genesis_config.rent.minimum_balance(0); - - // Make sure that rent debits are tracked for successful transactions - { - let alice = Keypair::new(); - test_utils::deposit(&bank, &alice.pubkey(), amount - 1).unwrap(); - let tx = system_transaction::transfer( - &mint_keypair, - &alice.pubkey(), - amount, - genesis_config.hash(), - ); - - let batch = bank.prepare_batch_for_tests(vec![tx]); - let commit_result = bank - .load_execute_and_commit_transactions( - &batch, - MAX_PROCESSING_AGE, - ExecutionRecordingConfig::new_single_setting(false), - &mut ExecuteTimings::default(), - None, - ) - .0 - .remove(0); - assert!(commit_result.is_ok()); - assert!(commit_result.was_executed_successfully()); - assert!(!commit_result.ok().unwrap().rent_debits.is_empty()); - } - - // Make sure that rent debits are ignored for failed transactions - { - let bob = Keypair::new(); - test_utils::deposit(&bank, &bob.pubkey(), amount - 1).unwrap(); - let tx = - system_transaction::transfer(&mint_keypair, &bob.pubkey(), 1, genesis_config.hash()); - - let batch = bank.prepare_batch_for_tests(vec![tx]); - let commit_result = bank - .load_execute_and_commit_transactions( - &batch, - MAX_PROCESSING_AGE, - ExecutionRecordingConfig::new_single_setting(false), - &mut ExecuteTimings::default(), - None, - ) - .0 - .remove(0); - assert!(commit_result.is_ok()); - assert!(!commit_result.was_executed_successfully()); - assert!(commit_result.ok().unwrap().rent_debits.is_empty()); - } -} - #[test] fn test_get_largest_accounts() { let GenesisConfigInfo { genesis_config, .. } = @@ -10038,21 +9204,6 @@ fn do_test_clean_dropped_unrooted_banks(freeze_bank1: FreezeBank1) { ); } -#[test] -fn test_rent_debits() { - let mut rent_debits = RentDebits::default(); - - // No entry for 0 rewards - rent_debits.insert(&Pubkey::new_unique(), 0, 0); - assert_eq!(rent_debits.len(), 0); - - // Some that actually work - rent_debits.insert(&Pubkey::new_unique(), 1, 0); - assert_eq!(rent_debits.len(), 1); - rent_debits.insert(&Pubkey::new_unique(), i64::MAX as u64, 0); - assert_eq!(rent_debits.len(), 2); -} - #[test] fn test_compute_budget_program_noop() { solana_logger::setup(); @@ -11876,63 +11027,6 @@ fn test_accounts_data_size_and_resize_transactions() { } } -#[test] -fn test_get_rent_paying_pubkeys() { - let lamports = 1; - let bank = create_simple_test_bank(lamports); - - let n = 432_000; - assert!(bank.get_rent_paying_pubkeys(&(0, 1, n)).is_none()); - assert!(bank.get_rent_paying_pubkeys(&(0, 2, n)).is_none()); - assert!(bank.get_rent_paying_pubkeys(&(0, 0, n)).is_none()); - - let pk1 = Pubkey::from([2; 32]); - let pk2 = Pubkey::from([3; 32]); - let index1 = accounts_partition::partition_from_pubkey(&pk1, n); - let index2 = accounts_partition::partition_from_pubkey(&pk2, n); - assert!(index1 > 0, "{}", index1); - assert!(index2 > index1, "{index2}, {index1}"); - - let epoch_schedule = EpochSchedule::custom(n, 0, false); - - let mut rent_paying_accounts_by_partition = RentPayingAccountsByPartition::new(&epoch_schedule); - rent_paying_accounts_by_partition.add_account(&pk1); - rent_paying_accounts_by_partition.add_account(&pk2); - - bank.rc - .accounts - .accounts_db - .accounts_index - .rent_paying_accounts_by_partition - .set(rent_paying_accounts_by_partition) - .unwrap(); - - assert_eq!( - bank.get_rent_paying_pubkeys(&(0, 1, n)), - Some(HashSet::default()) - ); - assert_eq!( - bank.get_rent_paying_pubkeys(&(0, 2, n)), - Some(HashSet::default()) - ); - assert_eq!( - bank.get_rent_paying_pubkeys(&(index1.saturating_sub(1), index1, n)), - Some(HashSet::from([pk1])) - ); - assert_eq!( - bank.get_rent_paying_pubkeys(&(index2.saturating_sub(1), index2, n)), - Some(HashSet::from([pk2])) - ); - assert_eq!( - bank.get_rent_paying_pubkeys(&(index1.saturating_sub(1), index2, n)), - Some(HashSet::from([pk2, pk1])) - ); - assert_eq!( - bank.get_rent_paying_pubkeys(&(0, 0, n)), - Some(HashSet::default()) - ); -} - /// Ensure that accounts rent epoch is updated correctly by rent collection #[test_case(true; "enable partitioned rent fees collection")] #[test_case(false; "disable partitioned rent fees collection")] @@ -11966,7 +11060,7 @@ fn test_partitioned_rent_collection(should_run_partitioned_rent_collection: bool // Run partitioned rent collection. If enabled, partitioned rent collection // will update the rent epoch for any rent exempt accounts whose rent epoch // is not already set to RENT_EXEMPT_RENT_EPOCH. - bank.collect_rent_eagerly(); + bank.run_partitioned_rent_exempt_status_updates(); let updated_account = bank.get_account(&account_pubkey).unwrap(); if should_run_partitioned_rent_collection { assert_eq!(updated_account.rent_epoch(), RENT_EXEMPT_RENT_EPOCH); @@ -11975,62 +11069,6 @@ fn test_partitioned_rent_collection(should_run_partitioned_rent_collection: bool } } -/// Ensure that accounts data size is updated correctly by rent collection -#[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) { - let GenesisConfigInfo { - mut genesis_config, .. - } = genesis_utils::create_genesis_config(100 * LAMPORTS_PER_SOL); - genesis_config.rent = Rent::default(); - if should_collect_rent { - genesis_config - .accounts - .remove(&agave_feature_set::disable_rent_fees_collection::id()); - genesis_config - .accounts - .remove(&agave_feature_set::disable_partitioned_rent_collection::id()); - } - - let bank = Arc::new(Bank::new_for_tests(&genesis_config)); - - let slot = bank.slot() + bank.slot_count_per_normal_epoch(); - let bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::default(), slot)); - - // make another bank so that any reclaimed accounts from the previous bank do not impact - // this test - let slot = bank.slot() + bank.slot_count_per_normal_epoch(); - let bank = Arc::new(Bank::new_from_parent(bank, &Pubkey::default(), slot)); - - // 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); - assert_eq!(info.account_data_len_reclaimed, data_size as u64); - } - - // Collect rent for real - assert_eq!(should_collect_rent, bank.should_collect_rent()); - 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!(!should_collect_rent || 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!(!should_collect_rent || reclaimed_data_size == data_size); -} - #[test] fn test_accounts_data_size_with_default_bank() { let bank = Bank::default_for_tests(); @@ -13038,7 +12076,7 @@ fn test_rebuild_skipped_rewrites() { // This fn is called within freeze(), but freeze() *consumes* Self::skipped_rewrites! // For testing, we want to know what's in the skipped rewrites, so we perform // rent collection manually. - bank.collect_rent_eagerly(); + bank.run_partitioned_rent_exempt_status_updates(); let actual_skipped_rewrites = bank.skipped_rewrites.lock().unwrap().clone(); // Ensure skipped rewrites now includes the account we stored above assert!(actual_skipped_rewrites.contains_key(&pubkey)); diff --git a/runtime/src/rent_collector.rs b/runtime/src/rent_collector.rs index 31da6303137..7392d49df43 100644 --- a/runtime/src/rent_collector.rs +++ b/runtime/src/rent_collector.rs @@ -14,7 +14,7 @@ use { solana_clock::Epoch, solana_pubkey::Pubkey, solana_rent::{Rent, RentDue}, - solana_rent_collector::{CollectedInfo, RentCollector}, + solana_rent_collector::RentCollector, solana_svm_rent_collector::{rent_state::RentState, svm_rent_collector::SVMRentCollector}, solana_transaction_context::IndexOfAccount, solana_transaction_error::{TransactionError, TransactionResult as Result}, @@ -35,10 +35,6 @@ impl RentCollectorWithMetrics { } impl SVMRentCollector for RentCollectorWithMetrics { - fn collect_rent(&self, address: &Pubkey, account: &mut AccountSharedData) -> CollectedInfo { - self.0.collect_rent(address, account) - } - fn get_rent(&self) -> &Rent { self.0.get_rent() } diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index 3ac4399dd8c..3fbb4befaea 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -152,7 +152,7 @@ struct DeserializableVersionedBank { collector_fees: u64, _fee_calculator: FeeCalculator, fee_rate_governor: FeeRateGovernor, - collected_rent: u64, + _collected_rent: u64, rent_collector: RentCollector, epoch_schedule: EpochSchedule, inflation: Inflation, @@ -189,7 +189,6 @@ impl From for BankFieldsToDeserialize { collector_id: dvb.collector_id, collector_fees: dvb.collector_fees, fee_rate_governor: dvb.fee_rate_governor, - collected_rent: dvb.collected_rent, rent_collector: dvb.rent_collector, epoch_schedule: dvb.epoch_schedule, inflation: dvb.inflation, @@ -270,7 +269,7 @@ impl From for SerializableVersionedBank { collector_fees: rhs.collector_fees, fee_calculator: FeeCalculator::default(), fee_rate_governor: rhs.fee_rate_governor, - collected_rent: rhs.collected_rent, + collected_rent: u64::default(), rent_collector: rhs.rent_collector, epoch_schedule: rhs.epoch_schedule, inflation: rhs.inflation, diff --git a/svm-feature-set/src/lib.rs b/svm-feature-set/src/lib.rs index 1fa0109ccb1..4f2cc899a98 100644 --- a/svm-feature-set/src/lib.rs +++ b/svm-feature-set/src/lib.rs @@ -34,7 +34,6 @@ pub struct SVMFeatureSet { pub fix_alt_bn128_multiplication_input_length: bool, pub loosen_cpi_size_restriction: bool, pub increase_tx_account_lock_limit: bool, - pub disable_rent_fees_collection: bool, pub enable_extend_program_checked: bool, pub formalize_loaded_transaction_data_size: bool, pub disable_zk_elgamal_proof_program: bool, @@ -78,7 +77,6 @@ impl SVMFeatureSet { fix_alt_bn128_multiplication_input_length: true, loosen_cpi_size_restriction: true, increase_tx_account_lock_limit: true, - disable_rent_fees_collection: true, enable_extend_program_checked: true, formalize_loaded_transaction_data_size: true, disable_zk_elgamal_proof_program: true, diff --git a/svm-rent-collector/src/svm_rent_collector.rs b/svm-rent-collector/src/svm_rent_collector.rs index bba32248bac..867bf9d69dc 100644 --- a/svm-rent-collector/src/svm_rent_collector.rs +++ b/svm-rent-collector/src/svm_rent_collector.rs @@ -6,7 +6,6 @@ use { solana_clock::Epoch, solana_pubkey::Pubkey, solana_rent::{Rent, RentDue}, - solana_rent_collector::CollectedInfo, solana_transaction_context::{IndexOfAccount, TransactionContext}, solana_transaction_error::{TransactionError, TransactionResult}, }; @@ -74,9 +73,6 @@ pub trait SVMRentCollector { } } - /// Collect rent from an account. - fn collect_rent(&self, address: &Pubkey, account: &mut AccountSharedData) -> CollectedInfo; - /// Determine the rent state of an account. /// /// This method has a default implementation that treats accounts with zero diff --git a/svm-rent-collector/src/svm_rent_collector/rent_collector.rs b/svm-rent-collector/src/svm_rent_collector/rent_collector.rs index f308ef11fd6..c4beb67a49c 100644 --- a/svm-rent-collector/src/svm_rent_collector/rent_collector.rs +++ b/svm-rent-collector/src/svm_rent_collector/rent_collector.rs @@ -3,18 +3,12 @@ use { crate::svm_rent_collector::SVMRentCollector, - solana_account::AccountSharedData, solana_clock::Epoch, - solana_pubkey::Pubkey, solana_rent::{Rent, RentDue}, - solana_rent_collector::{CollectedInfo, RentCollector}, + solana_rent_collector::RentCollector, }; impl SVMRentCollector for RentCollector { - fn collect_rent(&self, address: &Pubkey, account: &mut AccountSharedData) -> CollectedInfo { - self.collect_from_existing_account(address, account) - } - fn get_rent(&self) -> &Rent { &self.rent } @@ -29,7 +23,7 @@ mod tests { use { super::*, crate::rent_state::RentState, - solana_account::ReadableAccount, + solana_account::{AccountSharedData, ReadableAccount}, solana_clock::Epoch, solana_epoch_schedule::EpochSchedule, solana_pubkey::Pubkey, diff --git a/svm/Cargo.toml b/svm/Cargo.toml index 1980c3be9dc..32fcab7448f 100644 --- a/svm/Cargo.toml +++ b/svm/Cargo.toml @@ -65,7 +65,6 @@ solana-program-runtime = { workspace = true, features = ["metrics"] } solana-pubkey = { workspace = true } solana-rent = { workspace = true } solana-rent-collector = { workspace = true } -solana-rent-debits = { workspace = true } solana-sdk-ids = { workspace = true } solana-slot-hashes = { workspace = true } solana-svm-callback = { workspace = true } diff --git a/svm/examples/Cargo.lock b/svm/examples/Cargo.lock index ed799bb2c49..ed16157eb46 100644 --- a/svm/examples/Cargo.lock +++ b/svm/examples/Cargo.lock @@ -7373,16 +7373,6 @@ dependencies = [ "solana-sdk-ids", ] -[[package]] -name = "solana-rent-debits" -version = "2.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4f6f9113c6003492e74438d1288e30cffa8ccfdc2ef7b49b9e816d8034da18cd" -dependencies = [ - "solana-pubkey", - "solana-reward-info", -] - [[package]] name = "solana-reward-info" version = "2.2.1" @@ -7663,7 +7653,6 @@ dependencies = [ "solana-rayon-threadlimit", "solana-rent", "solana-rent-collector", - "solana-rent-debits", "solana-reward-info", "solana-runtime-transaction", "solana-sdk-ids", @@ -8168,7 +8157,6 @@ dependencies = [ "solana-pubkey", "solana-rent", "solana-rent-collector", - "solana-rent-debits", "solana-sdk-ids", "solana-slot-hashes", "solana-svm-callback", diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 8608a8d4dad..804f384f0d0 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -22,8 +22,7 @@ use { }, solana_pubkey::Pubkey, solana_rent::RentDue, - solana_rent_collector::{CollectedInfo, RENT_EXEMPT_RENT_EPOCH}, - solana_rent_debits::RentDebits, + solana_rent_collector::RENT_EXEMPT_RENT_EPOCH, solana_sdk_ids::{ bpf_loader_upgradeable, native_loader, sysvar::{self, slot_history}, @@ -47,7 +46,6 @@ pub(crate) const TRANSACTION_ACCOUNT_BASE_SIZE: usize = 64; const ADDRESS_LOOKUP_TABLE_BASE_SIZE: usize = 8248; // for the load instructions -pub(crate) type TransactionRent = u64; pub(crate) type TransactionProgramIndices = Vec>; pub type TransactionCheckResult = Result; type TransactionValidationResult = Result; @@ -127,7 +125,6 @@ impl Default for ValidatedTransactionDetails { pub(crate) struct LoadedTransactionAccount { pub(crate) account: AccountSharedData, pub(crate) loaded_size: usize, - pub(crate) rent_collected: u64, } #[derive(PartialEq, Eq, Debug, Clone)] @@ -142,8 +139,6 @@ pub struct LoadedTransaction { pub fee_details: FeeDetails, pub rollback_accounts: RollbackAccounts, pub(crate) compute_budget: SVMTransactionExecutionBudget, - pub rent: TransactionRent, - pub rent_debits: RentDebits, pub loaded_accounts_data_size: u32, } @@ -223,7 +218,6 @@ impl<'a, CB: TransactionProcessingCallback> AccountLoader<'a, CB> { account.map(|account| LoadedTransactionAccount { loaded_size: base_account_size.saturating_add(account.data().len()), account, - rent_collected: 0, }) } @@ -355,33 +349,24 @@ impl solana_svm_callback::InvokeContextCallba { } -/// Collect rent from an account if rent is still enabled and regardless of -/// whether rent is enabled, set the rent epoch to u64::MAX if the account is -/// rent exempt. -pub fn collect_rent_from_account( - feature_set: &SVMFeatureSet, +/// Set the rent epoch to u64::MAX if the account is rent exempt. +pub fn update_rent_exempt_status_for_account( rent_collector: &dyn SVMRentCollector, - address: &Pubkey, account: &mut AccountSharedData, -) -> CollectedInfo { - if !feature_set.disable_rent_fees_collection { - rent_collector.collect_rent(address, account) - } else { - // When rent fee collection is disabled, we won't collect rent for any account. If there - // 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 account.rent_epoch() != RENT_EXEMPT_RENT_EPOCH - && rent_collector.get_rent_due( - account.lamports(), - account.data().len(), - account.rent_epoch(), - ) == RentDue::Exempt - { - account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); - } - - CollectedInfo::default() +) { + // Now that rent fee collection is disabled, we won't collect rent for any + // account. If there 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 account.rent_epoch() != RENT_EXEMPT_RENT_EPOCH + && rent_collector.get_rent_due( + account.lamports(), + account.data().len(), + account.rent_epoch(), + ) == RentDue::Exempt + { + account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); } } @@ -465,8 +450,6 @@ pub(crate) fn load_transaction( accounts: loaded_tx_accounts.accounts, program_indices: loaded_tx_accounts.program_indices, fee_details: tx_details.fee_details, - rent: loaded_tx_accounts.rent, - rent_debits: loaded_tx_accounts.rent_debits, rollback_accounts: tx_details.rollback_accounts, compute_budget: tx_details.compute_budget, loaded_accounts_data_size: loaded_tx_accounts.loaded_accounts_data_size, @@ -485,8 +468,6 @@ pub(crate) fn load_transaction( struct LoadedTransactionAccounts { pub(crate) accounts: Vec, pub(crate) program_indices: TransactionProgramIndices, - pub(crate) rent: TransactionRent, - pub(crate) rent_debits: RentDebits, pub(crate) loaded_accounts_data_size: u32, } @@ -561,8 +542,6 @@ fn load_transaction_accounts_simd186( let mut loaded_transaction_accounts = LoadedTransactionAccounts { accounts: Vec::with_capacity(account_keys.len()), program_indices: Vec::with_capacity(message.num_instructions()), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size: 0, }; @@ -576,11 +555,10 @@ fn load_transaction_accounts_simd186( )?; let mut collect_loaded_account = - |account_loader: &mut AccountLoader, key, loaded_account| -> Result<()> { + |account_loader: &mut AccountLoader, key: &Pubkey, loaded_account| -> Result<()> { let LoadedTransactionAccount { account, loaded_size, - rent_collected, } = loaded_account; loaded_transaction_accounts.increase_calculated_data_size( @@ -589,14 +567,6 @@ fn load_transaction_accounts_simd186( error_metrics, )?; - loaded_transaction_accounts.rent = loaded_transaction_accounts - .rent - .saturating_add(rent_collected); - - loaded_transaction_accounts - .rent_debits - .insert(key, rent_collected, account.lamports()); - // This has been annotated branch-by-branch because collapsing the logic is infeasible. // Its purpose is to ensure programdata accounts are counted once and *only* once per // transaction. By checking account_keys, we never double-count a programdata account @@ -696,18 +666,15 @@ fn load_transaction_accounts_old( error_metrics: &mut TransactionErrorMetrics, rent_collector: &dyn SVMRentCollector, ) -> Result { - let mut tx_rent: TransactionRent = 0; let account_keys = message.account_keys(); let mut accounts = Vec::with_capacity(account_keys.len()); let mut validated_loaders = AHashSet::with_capacity(PROGRAM_OWNERS.len()); - let mut rent_debits = RentDebits::default(); let mut accumulated_accounts_data_size: Saturating = Saturating(0); - let mut collect_loaded_account = |key, loaded_account| -> Result<()> { + let mut collect_loaded_account = |key: &Pubkey, loaded_account| -> Result<()> { let LoadedTransactionAccount { account, loaded_size, - rent_collected, } = loaded_account; accumulate_and_check_loaded_account_data_size( @@ -717,9 +684,6 @@ fn load_transaction_accounts_old( error_metrics, )?; - tx_rent += rent_collected; - rent_debits.insert(key, rent_collected, account.lamports()); - accounts.push((*key, account)); Ok(()) }; @@ -800,8 +764,6 @@ fn load_transaction_accounts_old( Ok(LoadedTransactionAccounts { accounts, program_indices, - rent: tx_rent, - rent_debits, loaded_accounts_data_size: accumulated_accounts_data_size.0, }) } @@ -820,23 +782,13 @@ fn load_transaction_account( LoadedTransactionAccount { loaded_size: 0, account: construct_instructions_account(message), - rent_collected: 0, } } else if let Some(mut loaded_account) = account_loader.load_transaction_account(account_key, is_writable) { - loaded_account.rent_collected = if is_writable { - collect_rent_from_account( - account_loader.feature_set, - rent_collector, - account_key, - &mut loaded_account.account, - ) - .rent_amount - } else { - 0 - }; - + if is_writable { + update_rent_exempt_status_for_account(rent_collector, &mut loaded_account.account); + } loaded_account } else { let mut default_account = AccountSharedData::default(); @@ -847,7 +799,6 @@ fn load_transaction_account( LoadedTransactionAccount { loaded_size: default_account.data().len(), account: default_account, - rent_collected: 0, } }; @@ -934,7 +885,6 @@ mod tests { solana_pubkey::Pubkey, solana_rent::Rent, solana_rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, - solana_rent_debits::RentDebits, solana_sdk_ids::{ bpf_loader, bpf_loader_upgradeable, native_loader, system_program, sysvar, }, @@ -1013,9 +963,8 @@ mod tests { accounts: &[TransactionAccount], rent_collector: &RentCollector, error_metrics: &mut TransactionErrorMetrics, - mut feature_set: SVMFeatureSet, + feature_set: SVMFeatureSet, ) -> TransactionLoadResult { - feature_set.disable_rent_fees_collection = false; let sanitized_tx = SanitizedTransaction::from_transaction_for_tests(tx); let fee_payer_account = accounts[0].1.clone(); let mut accounts_map = HashMap::new(); @@ -1636,7 +1585,6 @@ mod tests { .accounts_map .insert(fee_payer_address, fee_payer_account.clone()); let mut account_loader = (&mock_bank).into(); - let fee_payer_rent_debit = 42; let mut error_metrics = TransactionErrorMetrics::default(); @@ -1651,25 +1599,16 @@ mod tests { LoadedTransactionAccount { loaded_size: fee_payer_account.data().len(), account: fee_payer_account.clone(), - rent_collected: fee_payer_rent_debit, }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, &RentCollector::default(), ); - - let expected_rent_debits = { - let mut rent_debits = RentDebits::default(); - rent_debits.insert(&fee_payer_address, fee_payer_rent_debit, fee_payer_balance); - rent_debits - }; assert_eq!( result.unwrap(), LoadedTransactionAccounts { accounts: vec![(fee_payer_address, fee_payer_account)], program_indices: vec![], - rent: fee_payer_rent_debit, - rent_debits: expected_rent_debits, loaded_accounts_data_size: 0, } ); @@ -1724,7 +1663,6 @@ mod tests { LoadedTransactionAccount { account: fee_payer_account.clone(), loaded_size: base_account_size, - ..LoadedTransactionAccount::default() }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, @@ -1749,8 +1687,6 @@ mod tests { ) ], program_indices: vec![vec![]], - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size, } ); @@ -1900,7 +1836,6 @@ mod tests { LoadedTransactionAccount { account: fee_payer_account.clone(), loaded_size: base_account_size, - ..LoadedTransactionAccount::default() }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, @@ -1920,8 +1855,6 @@ mod tests { ), ], program_indices: vec![vec![1]], - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size, } ); @@ -2092,7 +2025,6 @@ mod tests { LoadedTransactionAccount { account: fee_payer_account.clone(), loaded_size: base_account_size, - ..LoadedTransactionAccount::default() }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, @@ -2112,8 +2044,6 @@ mod tests { ), ], program_indices: vec![vec![1]], - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size, } ); @@ -2191,7 +2121,6 @@ mod tests { LoadedTransactionAccount { account: fee_payer_account.clone(), loaded_size: base_account_size, - ..LoadedTransactionAccount::default() }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, @@ -2214,8 +2143,6 @@ mod tests { (key3.pubkey(), account_data), ], program_indices: vec![vec![1], vec![1]], - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size, } ); @@ -2354,7 +2281,6 @@ mod tests { loaded_fee_payer_account: LoadedTransactionAccount { account: fee_payer_account, loaded_size: base_account_size, - ..LoadedTransactionAccount::default() }, ..ValidatedTransactionDetails::default() }); @@ -2393,8 +2319,6 @@ mod tests { fee_details: FeeDetails::default(), rollback_accounts: RollbackAccounts::default(), compute_budget: SVMTransactionExecutionBudget::default(), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size, } ); @@ -2458,76 +2382,39 @@ mod tests { } #[test] - fn test_collect_rent_from_account() { - let feature_set = SVMFeatureSet::all_enabled(); + fn test_update_rent_exempt_status_for_account() { let rent_collector = RentCollector { epoch: 1, ..RentCollector::default() }; - let address = Pubkey::new_unique(); let min_exempt_balance = rent_collector.rent.minimum_balance(0); let mut account = AccountSharedData::from(Account { lamports: min_exempt_balance, ..Account::default() }); - assert_eq!( - collect_rent_from_account(&feature_set, &rent_collector, &address, &mut account), - CollectedInfo::default() - ); + update_rent_exempt_status_for_account(&rent_collector, &mut account); assert_eq!(account.rent_epoch(), RENT_EXEMPT_RENT_EPOCH); } #[test] - fn test_collect_rent_from_account_rent_paying() { - let feature_set = SVMFeatureSet::all_enabled(); + fn test_update_rent_exempt_status_for_rent_paying_account() { let rent_collector = RentCollector { epoch: 1, ..RentCollector::default() }; - let address = Pubkey::new_unique(); let mut account = AccountSharedData::from(Account { lamports: 1, ..Account::default() }); - assert_eq!( - collect_rent_from_account(&feature_set, &rent_collector, &address, &mut account), - CollectedInfo::default() - ); + update_rent_exempt_status_for_account(&rent_collector, &mut account); assert_eq!(account.rent_epoch(), 0); assert_eq!(account.lamports(), 1); } - #[test] - fn test_collect_rent_from_account_rent_enabled() { - let mut feature_set = SVMFeatureSet::all_enabled(); - feature_set.disable_rent_fees_collection = false; - let rent_collector = RentCollector { - epoch: 1, - ..RentCollector::default() - }; - - let address = Pubkey::new_unique(); - let mut account = AccountSharedData::from(Account { - lamports: 1, - data: vec![0], - ..Account::default() - }); - - assert_eq!( - collect_rent_from_account(&feature_set, &rent_collector, &address, &mut account), - CollectedInfo { - rent_amount: 1, - account_data_len_reclaimed: 1 - } - ); - assert_eq!(account.rent_epoch(), 0); - assert_eq!(account.lamports(), 0); - } - // Ensure `TransactionProcessingCallback::inspect_account()` is called when // loading accounts for transaction processing. #[test] @@ -2718,7 +2605,6 @@ mod tests { LoadedTransactionAccount { account: fee_payer_account.clone(), loaded_size: fee_payer_size as usize, - rent_collected: 0, }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut TransactionErrorMetrics::default(), @@ -3160,7 +3046,6 @@ mod tests { LoadedTransactionAccount { loaded_size: TRANSACTION_ACCOUNT_BASE_SIZE + fee_payer_account.data().len(), account: fee_payer_account, - rent_collected: 0, }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut TransactionErrorMetrics::default(), diff --git a/svm/src/rollback_accounts.rs b/svm/src/rollback_accounts.rs index e25f5074994..850d0beb1c9 100644 --- a/svm/src/rollback_accounts.rs +++ b/svm/src/rollback_accounts.rs @@ -35,18 +35,8 @@ impl RollbackAccounts { nonce: Option, fee_payer_address: Pubkey, mut fee_payer_account: AccountSharedData, - fee_payer_rent_debit: u64, fee_payer_loaded_rent_epoch: Epoch, ) -> Self { - // When the fee payer account is rolled back due to transaction failure, - // rent should not be charged so credit the previously debited rent - // amount. - fee_payer_account.set_lamports( - fee_payer_account - .lamports() - .saturating_add(fee_payer_rent_debit), - ); - if let Some(nonce) = nonce { if &fee_payer_address == nonce.address() { // `nonce` contains an AccountSharedData which has already been advanced to the current DurableNonce @@ -120,10 +110,9 @@ mod tests { let fee_payer_account = AccountSharedData::new(100, 0, &Pubkey::default()); let fee_payer_rent_epoch = fee_payer_account.rent_epoch(); - const TEST_RENT_DEBIT: u64 = 1; - let rent_collected_fee_payer_account = { + let rent_epoch_updated_fee_payer_account = { let mut account = fee_payer_account.clone(); - account.set_lamports(fee_payer_account.lamports() - TEST_RENT_DEBIT); + account.set_lamports(fee_payer_account.lamports()); account.set_rent_epoch(fee_payer_rent_epoch + 1); account }; @@ -131,8 +120,7 @@ mod tests { let rollback_accounts = RollbackAccounts::new( None, fee_payer_address, - rent_collected_fee_payer_account, - TEST_RENT_DEBIT, + rent_epoch_updated_fee_payer_account, fee_payer_rent_epoch, ); @@ -161,19 +149,17 @@ mod tests { ) .unwrap(); - const TEST_RENT_DEBIT: u64 = 1; - let rent_collected_nonce_account = { + let rent_epoch_updated_fee_payer_account = { let mut account = nonce_account.clone(); - account.set_lamports(nonce_account.lamports() - TEST_RENT_DEBIT); + account.set_lamports(nonce_account.lamports()); account }; - let nonce = NonceInfo::new(nonce_address, rent_collected_nonce_account.clone()); + let nonce = NonceInfo::new(nonce_address, rent_epoch_updated_fee_payer_account.clone()); let rollback_accounts = RollbackAccounts::new( Some(nonce), nonce_address, - rent_collected_nonce_account, - TEST_RENT_DEBIT, + rent_epoch_updated_fee_payer_account, u64::MAX, // ignored ); @@ -205,10 +191,9 @@ mod tests { let fee_payer_address = Pubkey::new_unique(); let fee_payer_account = AccountSharedData::new(44, 0, &Pubkey::default()); - const TEST_RENT_DEBIT: u64 = 1; - let rent_collected_fee_payer_account = { + let rent_epoch_updated_fee_payer_account = { let mut account = fee_payer_account.clone(); - account.set_lamports(fee_payer_account.lamports() - TEST_RENT_DEBIT); + account.set_lamports(fee_payer_account.lamports()); account }; @@ -216,8 +201,7 @@ mod tests { let rollback_accounts = RollbackAccounts::new( Some(nonce), fee_payer_address, - rent_collected_fee_payer_account.clone(), - TEST_RENT_DEBIT, + rent_epoch_updated_fee_payer_account.clone(), u64::MAX, // ignored ); diff --git a/svm/src/transaction_commit_result.rs b/svm/src/transaction_commit_result.rs index 671d2052e74..cf7dee43340 100644 --- a/svm/src/transaction_commit_result.rs +++ b/svm/src/transaction_commit_result.rs @@ -1,8 +1,7 @@ use { crate::transaction_execution_result::TransactionLoadedAccountsStats, solana_fee_structure::FeeDetails, solana_message::inner_instruction::InnerInstructionsList, - solana_rent_debits::RentDebits, solana_transaction_context::TransactionReturnData, - solana_transaction_error::TransactionResult, + solana_transaction_context::TransactionReturnData, solana_transaction_error::TransactionResult, }; pub type TransactionCommitResult = TransactionResult; @@ -16,7 +15,6 @@ pub struct CommittedTransaction { pub return_data: Option, pub executed_units: u64, pub fee_details: FeeDetails, - pub rent_debits: RentDebits, pub loaded_account_stats: TransactionLoadedAccountsStats, } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 7d00785a6fa..fe560b70008 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -3,8 +3,8 @@ use qualifier_attr::{field_qualifiers, qualifiers}; use { crate::{ account_loader::{ - collect_rent_from_account, load_transaction, validate_fee_payer, AccountLoader, - CheckedTransactionDetails, LoadedTransaction, TransactionCheckResult, + load_transaction, update_rent_exempt_status_for_account, validate_fee_payer, + AccountLoader, CheckedTransactionDetails, LoadedTransaction, TransactionCheckResult, TransactionLoadResult, ValidatedTransactionDetails, }, account_overrides::AccountOverrides, @@ -582,13 +582,7 @@ impl TransactionBatchProcessor { }; let fee_payer_loaded_rent_epoch = loaded_fee_payer.account.rent_epoch(); - loaded_fee_payer.rent_collected = collect_rent_from_account( - account_loader.feature_set, - rent_collector, - fee_payer_address, - &mut loaded_fee_payer.account, - ) - .rent_amount; + update_rent_exempt_status_for_account(rent_collector, &mut loaded_fee_payer.account); let fee_payer_index = 0; validate_fee_payer( @@ -606,7 +600,6 @@ impl TransactionBatchProcessor { nonce, *fee_payer_address, loaded_fee_payer.account.clone(), - loaded_fee_payer.rent_collected, fee_payer_loaded_rent_epoch, ); @@ -1125,7 +1118,6 @@ mod tests { }, solana_rent::Rent, solana_rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, - solana_rent_debits::RentDebits, solana_sdk_ids::{bpf_loader, system_program, sysvar}, solana_signature::Signature, solana_svm_callback::{AccountState, InvokeContextCallback}, @@ -1372,8 +1364,6 @@ mod tests { fee_details: FeeDetails::default(), rollback_accounts: RollbackAccounts::default(), compute_budget: SVMTransactionExecutionBudget::default(), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size: 32, }; @@ -1469,8 +1459,6 @@ mod tests { fee_details: FeeDetails::default(), rollback_accounts: RollbackAccounts::default(), compute_budget: SVMTransactionExecutionBudget::default(), - rent: 0, - rent_debits: RentDebits::default(), loaded_accounts_data_size: 0, }; @@ -2082,7 +2070,6 @@ mod tests { ); let fee_payer_rent_epoch = current_epoch; - let fee_payer_rent_debit = 0; let fee_payer_account = AccountSharedData::new_rent_epoch( starting_balance, 0, @@ -2138,7 +2125,6 @@ mod tests { None, // nonce *fee_payer_address, post_validation_fee_payer_account.clone(), - fee_payer_rent_debit, fee_payer_rent_epoch ), compute_budget: compute_budget_and_limits.budget, @@ -2148,7 +2134,6 @@ mod tests { loaded_fee_payer_account: LoadedTransactionAccount { loaded_size: base_account_size + fee_payer_account.data().len(), account: post_validation_fee_payer_account, - rent_collected: fee_payer_rent_debit, }, }) ); @@ -2172,14 +2157,6 @@ mod tests { let transaction_fee = lamports_per_signature; let starting_balance = min_balance - 1; let fee_payer_account = AccountSharedData::new(starting_balance, 0, &Pubkey::default()); - let fee_payer_rent_debit = rent_collector - .get_rent_due( - fee_payer_account.lamports(), - fee_payer_account.data().len(), - fee_payer_account.rent_epoch(), - ) - .lamports(); - assert!(fee_payer_rent_debit > 0); let mut mock_accounts = HashMap::new(); mock_accounts.insert(*fee_payer_address, fee_payer_account.clone()); @@ -2189,7 +2166,6 @@ mod tests { }; mock_bank.feature_set.formalize_loaded_transaction_data_size = formalize_loaded_transaction_data_size; - mock_bank.feature_set.disable_rent_fees_collection = false; let mut account_loader = (&mock_bank).into(); let mut error_counters = TransactionErrorMetrics::default(); @@ -2208,8 +2184,7 @@ mod tests { let post_validation_fee_payer_account = { let mut account = fee_payer_account.clone(); - account.set_rent_epoch(1); - account.set_lamports(starting_balance - transaction_fee - fee_payer_rent_debit); + account.set_lamports(starting_balance - transaction_fee); account }; @@ -2226,7 +2201,6 @@ mod tests { None, // nonce *fee_payer_address, post_validation_fee_payer_account.clone(), - fee_payer_rent_debit, 0, // rent epoch ), compute_budget: compute_budget_and_limits.budget, @@ -2236,7 +2210,6 @@ mod tests { loaded_fee_payer_account: LoadedTransactionAccount { loaded_size: base_account_size + fee_payer_account.data().len(), account: post_validation_fee_payer_account, - rent_collected: fee_payer_rent_debit, } }) ); @@ -2509,7 +2482,6 @@ mod tests { Some(future_nonce), *fee_payer_address, post_validation_fee_payer_account.clone(), - 0, // fee_payer_rent_debit 0, // fee_payer_rent_epoch ), compute_budget: compute_budget_and_limits.budget, @@ -2519,7 +2491,6 @@ mod tests { loaded_fee_payer_account: LoadedTransactionAccount { loaded_size: base_account_size + fee_payer_account.data().len(), account: post_validation_fee_payer_account, - rent_collected: 0, } }) );