From d7201a8d1aa6acecc64c791c0699f6fb99139fc6 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Wed, 6 Jul 2022 20:01:16 +0000 Subject: [PATCH] names fields in RentResullt::CollectRent enum variant (#26449) Avoiding ambiguous raw tuple: CollectRent((Epoch, u64)) Using named fields instead: CollectRent { new_rent_epoch: Epoch, rent_due: u64, }, --- runtime/src/accounts.rs | 4 +- runtime/src/accounts_db.rs | 7 +- runtime/src/bank.rs | 4 +- runtime/src/expected_rent_collection.rs | 36 +++++---- runtime/src/rent_collector.rs | 101 +++++++++++------------- 5 files changed, 76 insertions(+), 76 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 3ecca73ea0..2ed469df9c 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1703,9 +1703,9 @@ mod tests { let mut error_counters = TransactionErrorMetrics::default(); let rent_collector = RentCollector::new( 0, - &EpochSchedule::default(), + EpochSchedule::default(), 500_000.0, - &Rent { + Rent { lamports_per_byte_year: 42, ..Rent::default() }, diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index d525d26390..e55a977e97 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -6231,7 +6231,8 @@ impl AccountsDb { ) } - pub fn update_accounts_hash_test(&self, slot: Slot, ancestors: &Ancestors) -> (Hash, u64) { + #[cfg(test)] + fn update_accounts_hash_test(&self, slot: Slot, ancestors: &Ancestors) -> (Hash, u64) { self.update_accounts_hash_with_index_option( true, true, @@ -8151,9 +8152,9 @@ impl AccountsDb { let schedule = genesis_config.epoch_schedule; let rent_collector = RentCollector::new( schedule.get_epoch(max_slot), - &schedule, + schedule, genesis_config.slots_per_year(), - &genesis_config.rent, + genesis_config.rent, ); let accounts_data_len = AtomicU64::new(0); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 7e22e3bb77..2f2f304c17 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3536,9 +3536,9 @@ impl Bank { self.rent_collector = RentCollector::new( self.epoch, - self.epoch_schedule(), + *self.epoch_schedule(), self.slots_per_year, - &genesis_config.rent, + genesis_config.rent, ); // Add additional builtin programs specified in the genesis config diff --git a/runtime/src/expected_rent_collection.rs b/runtime/src/expected_rent_collection.rs index 537caf8c81..92ad974511 100644 --- a/runtime/src/expected_rent_collection.rs +++ b/runtime/src/expected_rent_collection.rs @@ -266,7 +266,7 @@ impl SlotInfoInEpoch { impl ExpectedRentCollection { /// 'account' is being loaded from 'storage_slot' in 'bank_slot' /// adjusts 'account.rent_epoch' if we skipped the last rewrite on this account - pub fn maybe_update_rent_epoch_on_load( + pub(crate) fn maybe_update_rent_epoch_on_load( account: &mut AccountSharedData, storage_slot: &SlotInfoInEpoch, bank_slot: &SlotInfoInEpoch, @@ -302,14 +302,17 @@ impl ExpectedRentCollection { pubkey: &Pubkey, rewrites_skipped_this_slot: &Rewrites, ) -> Option { - if let RentResult::CollectRent((next_epoch, rent_due)) = - rent_collector.calculate_rent_result(pubkey, account, None) + let next_epoch = match rent_collector.calculate_rent_result(pubkey, account, None) { + RentResult::LeaveAloneNoRent => return None, + RentResult::CollectRent { + new_rent_epoch, + rent_due: 0, + } => new_rent_epoch, + // Rent is due on this account in this epoch, + // so we did not skip a rewrite. + RentResult::CollectRent { .. } => return None, + }; { - if rent_due != 0 { - // rent is due on this account in this epoch, so we did not skip a rewrite - return None; - } - // grab epoch infno for bank slot and storage slot let bank_info = bank_slot.get_epoch_info(epoch_schedule); let (current_epoch, partition_from_current_slot) = @@ -533,7 +536,10 @@ impl ExpectedRentCollection { rent_collector.calculate_rent_result(pubkey, loaded_account, filler_account_suffix); let current_rent_epoch = loaded_account.rent_epoch(); let new_rent_epoch = match rent_result { - RentResult::CollectRent((next_epoch, rent_due)) => { + RentResult::CollectRent { + new_rent_epoch: next_epoch, + rent_due, + } => { if next_epoch > current_rent_epoch && rent_due != 0 { // this is an account that would have had rent collected since this storage slot, so just use the hash we have since there must be a newer version of this account already in a newer slot // It would be a waste of time to recalcluate a hash. @@ -595,9 +601,9 @@ pub mod tests { let genesis_config = GenesisConfig::default(); let mut rent_collector = RentCollector::new( epoch, - &epoch_schedule, + epoch_schedule, genesis_config.slots_per_year(), - &genesis_config.rent, + genesis_config.rent, ); rent_collector.rent.lamports_per_byte_year = 0; // temporarily disable rent let find_unskipped_slot = Some; @@ -976,9 +982,9 @@ pub mod tests { let genesis_config = GenesisConfig::default(); let mut rent_collector = RentCollector::new( epoch, - &epoch_schedule, + epoch_schedule, genesis_config.slots_per_year(), - &genesis_config.rent, + genesis_config.rent, ); rent_collector.rent.lamports_per_byte_year = 0; // temporarily disable rent @@ -1169,9 +1175,9 @@ pub mod tests { let genesis_config = GenesisConfig::default(); let mut rent_collector = RentCollector::new( epoch, - &epoch_schedule, + epoch_schedule, genesis_config.slots_per_year(), - &genesis_config.rent, + genesis_config.rent, ); rent_collector.rent.lamports_per_byte_year = 0; // temporarily disable rent diff --git a/runtime/src/rent_collector.rs b/runtime/src/rent_collector.rs index 046535c160..6ae978a9a5 100644 --- a/runtime/src/rent_collector.rs +++ b/runtime/src/rent_collector.rs @@ -34,44 +34,33 @@ impl Default for RentCollector { /// when rent is collected for this account, this is the action to apply to the account #[derive(Debug)] -pub enum RentResult { +pub(crate) enum RentResult { /// maybe collect rent later, leave account alone LeaveAloneNoRent, /// collect rent - /// value is (new rent epoch, lamports of rent_due) - CollectRent((Epoch, u64)), + CollectRent { + new_rent_epoch: Epoch, + rent_due: u64, // lamports + }, } impl RentCollector { - pub fn new( + pub(crate) fn new( epoch: Epoch, - epoch_schedule: &EpochSchedule, + epoch_schedule: EpochSchedule, slots_per_year: f64, - rent: &Rent, + rent: Rent, ) -> Self { Self { epoch, - epoch_schedule: *epoch_schedule, + epoch_schedule, slots_per_year, - rent: *rent, + rent, } } - pub fn clone_with_epoch(&self, epoch: Epoch) -> Self { - self.clone_with_epoch_and_rate(epoch, self.rent.lamports_per_byte_year) - } - - pub fn clone_with_epoch_and_rate(&self, epoch: Epoch, lamports_per_byte_year: u64) -> Self { - let rent = if lamports_per_byte_year != self.rent.lamports_per_byte_year { - Rent { - lamports_per_byte_year, - ..self.rent - } - } else { - self.rent - }; + pub(crate) fn clone_with_epoch(&self, epoch: Epoch) -> Self { Self { - rent, epoch, ..self.clone() } @@ -85,7 +74,7 @@ impl RentCollector { /// given an account that 'should_collect_rent' /// returns (amount rent due, is_exempt_from_rent) - pub fn get_rent_due(&self, account: &impl ReadableAccount) -> RentDue { + pub(crate) fn get_rent_due(&self, account: &impl ReadableAccount) -> RentDue { if self .rent .is_exempt(account.lamports(), account.data().len()) @@ -127,7 +116,7 @@ impl RentCollector { // This is NOT thread safe at some level. If we try to collect from the same account in // parallel, we may collect twice. #[must_use = "add to Bank::collected_rent"] - pub fn collect_from_existing_account( + pub(crate) fn collect_from_existing_account( &self, address: &Pubkey, account: &mut AccountSharedData, @@ -135,28 +124,32 @@ impl RentCollector { ) -> CollectedInfo { match self.calculate_rent_result(address, account, filler_account_suffix) { RentResult::LeaveAloneNoRent => CollectedInfo::default(), - RentResult::CollectRent((next_epoch, rent_due)) => { - account.set_rent_epoch(next_epoch); - - let begin_lamports = account.lamports(); - account.saturating_sub_lamports(rent_due); - let end_lamports = account.lamports(); - let mut account_data_len_reclaimed = 0; - if end_lamports == 0 { - account_data_len_reclaimed = account.data().len() as u64; - *account = AccountSharedData::default(); + RentResult::CollectRent { + new_rent_epoch, + rent_due, + } => match account.lamports().checked_sub(rent_due) { + None | Some(0) => { + let account = std::mem::take(account); + CollectedInfo { + rent_amount: account.lamports(), + account_data_len_reclaimed: account.data().len() as u64, + } } - CollectedInfo { - rent_amount: begin_lamports - end_lamports, - account_data_len_reclaimed, + Some(lamports) => { + account.set_lamports(lamports); + account.set_rent_epoch(new_rent_epoch); + CollectedInfo { + rent_amount: rent_due, + account_data_len_reclaimed: 0u64, + } } - } + }, } } /// determine what should happen to collect rent from this account #[must_use] - pub fn calculate_rent_result( + pub(crate) fn calculate_rent_result( &self, address: &Pubkey, account: &impl ReadableAccount, @@ -165,25 +158,25 @@ impl RentCollector { if self.can_skip_rent_collection(address, account, filler_account_suffix) { return RentResult::LeaveAloneNoRent; } - - let rent_due = self.get_rent_due(account); - if let RentDue::Paying(0) = rent_due { - // maybe collect rent later, leave account alone - return RentResult::LeaveAloneNoRent; + match self.get_rent_due(account) { + // Rent isn't collected for the next epoch. + // Make sure to check exempt status again later in current epoch. + RentDue::Exempt => RentResult::CollectRent { + new_rent_epoch: self.epoch, + rent_due: 0, + }, + // Maybe collect rent later, leave account alone. + RentDue::Paying(0) => RentResult::LeaveAloneNoRent, + // Rent is collected for next epoch. + RentDue::Paying(rent_due) => RentResult::CollectRent { + new_rent_epoch: self.epoch + 1, + rent_due, + }, } - - let new_rent_epoch = match rent_due { - // Rent isn't collected for the next epoch - // Make sure to check exempt status again later in current epoch - RentDue::Exempt => self.epoch, - // Rent is collected for next epoch - RentDue::Paying(_) => self.epoch + 1, - }; - RentResult::CollectRent((new_rent_epoch, rent_due.lamports())) } #[must_use = "add to Bank::collected_rent"] - pub fn collect_from_created_account( + pub(crate) fn collect_from_created_account( &self, address: &Pubkey, account: &mut AccountSharedData,