From ccb09866b59917bb01918299b7c4d2adfa313781 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Tue, 2 Apr 2024 13:25:43 -0500 Subject: [PATCH] stop requiring data allocation to check for rent (#543) --- accounts-db/src/accounts_db.rs | 41 +++++++++++++++++++++++----------- runtime/src/bank.rs | 16 ++++++++++--- sdk/src/rent_collector.rs | 27 +++++++++++++--------- svm/src/account_loader.rs | 6 ++++- 4 files changed, 62 insertions(+), 28 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 53b8d1346b..3e9eaa1445 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -8608,20 +8608,25 @@ impl AccountsDb { } /// return Some(lamports_to_top_off) if 'account' would collect rent - fn stats_for_rent_payers( + fn stats_for_rent_payers( pubkey: &Pubkey, - account: &T, + lamports: u64, + account_data_len: usize, + account_rent_epoch: Epoch, + executable: bool, rent_collector: &RentCollector, ) -> Option { - if account.lamports() == 0 { + if lamports == 0 { return None; } - (rent_collector.should_collect_rent(pubkey, account) - && !rent_collector.get_rent_due(account).is_exempt()) + (rent_collector.should_collect_rent(pubkey, executable) + && !rent_collector + .get_rent_due(lamports, account_data_len, account_rent_epoch) + .is_exempt()) .then(|| { - let min_balance = rent_collector.rent.minimum_balance(account.data().len()); + let min_balance = rent_collector.rent.minimum_balance(account_data_len); // return lamports required to top off this account to make it rent exempt - min_balance.saturating_sub(account.lamports()) + min_balance.saturating_sub(lamports) }) } @@ -8661,9 +8666,14 @@ impl AccountsDb { accounts_data_len += stored_account.data().len() as u64; } - if let Some(amount_to_top_off_rent_this_account) = - Self::stats_for_rent_payers(pubkey, &stored_account, rent_collector) - { + if let Some(amount_to_top_off_rent_this_account) = Self::stats_for_rent_payers( + pubkey, + stored_account.lamports(), + stored_account.data().len(), + stored_account.rent_epoch(), + stored_account.executable(), + rent_collector, + ) { amount_to_top_off_rent += amount_to_top_off_rent_this_account; num_accounts_rent_paying += 1; // remember this rent-paying account pubkey @@ -9090,9 +9100,14 @@ impl AccountsDb { ); let loaded_account = accessor.check_and_get_loaded_account(); accounts_data_len_from_duplicates += loaded_account.data().len(); - if let Some(lamports_to_top_off) = - Self::stats_for_rent_payers(pubkey, &loaded_account, rent_collector) - { + if let Some(lamports_to_top_off) = Self::stats_for_rent_payers( + pubkey, + loaded_account.lamports(), + loaded_account.data().len(), + loaded_account.rent_epoch(), + loaded_account.executable(), + rent_collector, + ) { removed_rent_paying += 1; removed_top_off += lamports_to_top_off; } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1b8c670195..97816cb9ed 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5165,7 +5165,11 @@ impl Bank { // 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 - && self.rent_collector.get_rent_due(account) == RentDue::Exempt + && self.rent_collector.get_rent_due( + account.lamports(), + account.data().len(), + account.rent_epoch(), + ) == RentDue::Exempt { account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); } @@ -7803,8 +7807,14 @@ impl TotalAccountsStats { self.executable_data_len += data_len; } - if !rent_collector.should_collect_rent(address, account) - || rent_collector.get_rent_due(account).is_exempt() + if !rent_collector.should_collect_rent(address, account.executable()) + || rent_collector + .get_rent_due( + account.lamports(), + account.data().len(), + account.rent_epoch(), + ) + .is_exempt() { self.num_rent_exempt_accounts += 1; } else { diff --git a/sdk/src/rent_collector.rs b/sdk/src/rent_collector.rs index 1de6ce1995..1dd8c4a029 100644 --- a/sdk/src/rent_collector.rs +++ b/sdk/src/rent_collector.rs @@ -73,21 +73,22 @@ impl RentCollector { } /// true if it is easy to determine this account should consider having rent collected from it - pub fn should_collect_rent(&self, address: &Pubkey, account: &impl ReadableAccount) -> bool { - !(account.executable() // executable accounts must be rent-exempt balance + pub fn should_collect_rent(&self, address: &Pubkey, executable: bool) -> bool { + !(executable // executable accounts must be rent-exempt balance || *address == incinerator::id()) } /// 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 { - if self - .rent - .is_exempt(account.lamports(), account.data().len()) - { + pub fn get_rent_due( + &self, + lamports: u64, + data_len: usize, + account_rent_epoch: Epoch, + ) -> RentDue { + if self.rent.is_exempt(lamports, data_len) { RentDue::Exempt } else { - let account_rent_epoch = account.rent_epoch(); let slots_elapsed: u64 = (account_rent_epoch..=self.epoch) .map(|epoch| { self.epoch_schedule @@ -103,7 +104,7 @@ impl RentCollector { }; // we know this account is not exempt - let due = self.rent.due_amount(account.data().len(), years_elapsed); + let due = self.rent.due_amount(data_len, years_elapsed); RentDue::Paying(due) } } @@ -158,11 +159,15 @@ impl RentCollector { // Maybe collect rent later, leave account alone for now. return RentResult::NoRentCollectionNow; } - if !self.should_collect_rent(address, account) { + if !self.should_collect_rent(address, account.executable()) { // easy to determine this account should not consider having rent collected from it return RentResult::Exempt; } - match self.get_rent_due(account) { + match self.get_rent_due( + account.lamports(), + account.data().len(), + account.rent_epoch(), + ) { // account will not have rent collected ever RentDue::Exempt => RentResult::Exempt, // potentially rent paying account diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 87c18b9717..dd47da3c60 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -250,7 +250,11 @@ fn load_transaction_accounts( // 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) == RentDue::Exempt + && rent_collector.get_rent_due( + account.lamports(), + account.data().len(), + account.rent_epoch(), + ) == RentDue::Exempt { account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); }