SVM: Wire up SVMRentCollector (anza-xyz#2753)

* SVM: account loader: wire up `SVMRentCollector` * SVM: transaction account state info: wire up `SVMRentCollector` * Runtime: fee distribution: wire up `SVMRentCollector` * Runtime: bank: wire up `SVMRentCollector` * SVM: drop everything unused
pgarg66 · Aug 30, 2024 · 5866bfb · 5866bfb
1 parent a72f981
commit 5866bfb
Show file tree

Hide file tree

Showing 10 changed files with 72 additions and 370 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock
diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
@@ -3478,9 +3478,7 @@ impl Bank {
         timings.saturating_add_in_place(ExecuteTimingType::CheckUs, check_us);
 
         let (blockhash, lamports_per_signature) = self.last_blockhash_and_lamports_per_signature();
-        // TODO: Pass into `TransactionProcessingEnvironment` in place of
-        // `rent_collector` when SVM supports the new `SVMRentCollector` trait.
-        let _rent_collector_with_metrics =
+        let rent_collector_with_metrics =
             RentCollectorWithMetrics::new(self.rent_collector.clone());
         let processing_environment = TransactionProcessingEnvironment {
             blockhash,
@@ -3489,7 +3487,7 @@ impl Bank {
             feature_set: Arc::clone(&self.feature_set),
             fee_structure: Some(&self.fee_structure),
             lamports_per_signature,
-            rent_collector: Some(&self.rent_collector),
+            rent_collector: Some(&rent_collector_with_metrics),
         };
 
         let sanitized_output = self

diff --git a/runtime/src/bank/fee_distribution.rs b/runtime/src/bank/fee_distribution.rs
@@ -12,7 +12,7 @@ use {
         system_program,
         transaction::SanitizedTransaction,
     },
-    solana_svm::account_rent_state::RentState,
+    solana_svm_rent_collector::svm_rent_collector::SVMRentCollector,
     solana_vote::vote_account::VoteAccountsHashMap,
     std::{result::Result, sync::atomic::Ordering::Relaxed},
     thiserror::Error,
@@ -148,16 +148,16 @@ impl Bank {
             return Err(DepositFeeError::InvalidAccountOwner);
         }
 
-        let rent = &self.rent_collector().rent;
-        let recipient_pre_rent_state = RentState::from_account(&account, rent);
+        let recipient_pre_rent_state = self.rent_collector().get_account_rent_state(&account);
         let distribution = account.checked_add_lamports(fees);
         if distribution.is_err() {
             return Err(DepositFeeError::LamportOverflow);
         }
 
-        let recipient_post_rent_state = RentState::from_account(&account, rent);
-        let rent_state_transition_allowed =
-            recipient_post_rent_state.transition_allowed_from(&recipient_pre_rent_state);
+        let recipient_post_rent_state = self.rent_collector().get_account_rent_state(&account);
+        let rent_state_transition_allowed = self
+            .rent_collector()
+            .transition_allowed(&recipient_pre_rent_state, &recipient_post_rent_state);
         if !rent_state_transition_allowed {
             return Err(DepositFeeError::InvalidRentPayingAccount);
         }
@@ -334,6 +334,7 @@ pub mod tests {
             account::AccountSharedData, native_token::sol_to_lamports, pubkey, rent::Rent,
             signature::Signer,
         },
+        solana_svm_rent_collector::rent_state::RentState,
         std::sync::RwLock,
     };
 
@@ -633,8 +634,7 @@ pub mod tests {
         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 = &bank.rent_collector().rent;
-        let rent_exempt_minimum = rent.minimum_balance(0);
+        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
@@ -671,7 +671,7 @@ pub mod tests {
             let account = bank
                 .get_account_with_fixed_root(address)
                 .unwrap_or_default();
-            RentState::from_account(&account, rent)
+            bank.rent_collector().get_account_rent_state(&account)
         };
 
         // Assert starting RentStates

diff --git a/svm/Cargo.toml b/svm/Cargo.toml
@@ -24,10 +24,10 @@ solana-frozen-abi-macro = { workspace = true, optional = true }
 solana-loader-v4-program = { workspace = true }
 solana-log-collector = { workspace = true }
 solana-measure = { workspace = true }
-solana-metrics = { workspace = true }
 solana-program-runtime = { workspace = true }
 solana-runtime-transaction = { workspace = true }
 solana-sdk = { workspace = true }
+solana-svm-rent-collector = { workspace = true }
 solana-svm-transaction = { workspace = true }
 solana-system-program = { workspace = true }
 solana-timings = { workspace = true }

diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs
@@ -1,7 +1,6 @@
 use {
     crate::{
         account_overrides::AccountOverrides,
-        account_rent_state::RentState,
         nonce_info::NonceInfo,
         rollback_accounts::RollbackAccounts,
         transaction_error_metrics::TransactionErrorMetrics,
@@ -18,7 +17,7 @@ use {
         nonce::State as NonceState,
         pubkey::Pubkey,
         rent::RentDue,
-        rent_collector::{CollectedInfo, RentCollector, RENT_EXEMPT_RENT_EPOCH},
+        rent_collector::{CollectedInfo, RENT_EXEMPT_RENT_EPOCH},
         rent_debits::RentDebits,
         saturating_add_assign,
         sysvar::{
@@ -28,6 +27,7 @@ use {
         transaction::{Result, TransactionError},
         transaction_context::{IndexOfAccount, TransactionAccount},
     },
+    solana_svm_rent_collector::svm_rent_collector::SVMRentCollector,
     solana_svm_transaction::svm_message::SVMMessage,
     solana_system_program::{get_system_account_kind, SystemAccountKind},
     std::num::NonZeroU32,
@@ -100,12 +100,12 @@ pub struct FeesOnlyTransaction {
 /// rent exempt.
 pub fn collect_rent_from_account(
     feature_set: &FeatureSet,
-    rent_collector: &RentCollector,
+    rent_collector: &dyn SVMRentCollector,
     address: &Pubkey,
     account: &mut AccountSharedData,
 ) -> CollectedInfo {
     if !feature_set.is_active(&feature_set::disable_rent_fees_collection::id()) {
-        rent_collector.collect_from_existing_account(address, account)
+        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
@@ -135,7 +135,7 @@ pub fn validate_fee_payer(
     payer_account: &mut AccountSharedData,
     payer_index: IndexOfAccount,
     error_metrics: &mut TransactionErrorMetrics,
-    rent_collector: &RentCollector,
+    rent_collector: &dyn SVMRentCollector,
     fee: u64,
 ) -> Result<()> {
     if payer_account.lamports() == 0 {
@@ -151,7 +151,9 @@ pub fn validate_fee_payer(
         SystemAccountKind::Nonce => {
             // Should we ever allow a fees charge to zero a nonce account's
             // balance. The state MUST be set to uninitialized in that case
-            rent_collector.rent.minimum_balance(NonceState::size())
+            rent_collector
+                .get_rent()
+                .minimum_balance(NonceState::size())
         }
     };
 
@@ -164,13 +166,13 @@ pub fn validate_fee_payer(
             TransactionError::InsufficientFundsForFee
         })?;
 
-    let payer_pre_rent_state = RentState::from_account(payer_account, &rent_collector.rent);
+    let payer_pre_rent_state = rent_collector.get_account_rent_state(payer_account);
     payer_account
         .checked_sub_lamports(fee)
         .map_err(|_| TransactionError::InsufficientFundsForFee)?;
 
-    let payer_post_rent_state = RentState::from_account(payer_account, &rent_collector.rent);
-    RentState::check_rent_state_with_account(
+    let payer_post_rent_state = rent_collector.get_account_rent_state(payer_account);
+    rent_collector.check_rent_state_with_account(
         &payer_pre_rent_state,
         &payer_post_rent_state,
         payer_address,
@@ -191,7 +193,7 @@ pub(crate) fn load_accounts<CB: TransactionProcessingCallback>(
     error_metrics: &mut TransactionErrorMetrics,
     account_overrides: Option<&AccountOverrides>,
     feature_set: &FeatureSet,
-    rent_collector: &RentCollector,
+    rent_collector: &dyn SVMRentCollector,
     loaded_programs: &ProgramCacheForTxBatch,
 ) -> Vec<TransactionLoadResult> {
     txs.iter()
@@ -218,7 +220,7 @@ fn load_transaction<CB: TransactionProcessingCallback>(
     error_metrics: &mut TransactionErrorMetrics,
     account_overrides: Option<&AccountOverrides>,
     feature_set: &FeatureSet,
-    rent_collector: &RentCollector,
+    rent_collector: &dyn SVMRentCollector,
     loaded_programs: &ProgramCacheForTxBatch,
 ) -> TransactionLoadResult {
     match validation_result {
@@ -274,7 +276,7 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
     error_metrics: &mut TransactionErrorMetrics,
     account_overrides: Option<&AccountOverrides>,
     feature_set: &FeatureSet,
-    rent_collector: &RentCollector,
+    rent_collector: &dyn SVMRentCollector,
     loaded_programs: &ProgramCacheForTxBatch,
 ) -> Result<LoadedTransactionAccounts> {
     let mut tx_rent: TransactionRent = 0;
@@ -409,7 +411,7 @@ fn load_transaction_account<CB: TransactionProcessingCallback>(
     instruction_accounts: &[&u8],
     account_overrides: Option<&AccountOverrides>,
     feature_set: &FeatureSet,
-    rent_collector: &RentCollector,
+    rent_collector: &dyn SVMRentCollector,
     loaded_programs: &ProgramCacheForTxBatch,
 ) -> Result<(LoadedTransactionAccount, bool)> {
     let mut account_found = true;
@@ -1848,18 +1850,19 @@ mod tests {
         let compute_budget = ComputeBudget::new(u64::from(
             compute_budget_limits::DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT,
         ));
+        let rent_collector = RentCollector::default();
         let transaction_context = TransactionContext::new(
             loaded_transaction.accounts,
-            Rent::default(),
+            rent_collector.get_rent().clone(),
             compute_budget.max_instruction_stack_depth,
             compute_budget.max_instruction_trace_length,
         );
 
         assert_eq!(
             TransactionAccountStateInfo::new(
-                &Rent::default(),
                 &transaction_context,
-                sanitized_tx.message()
+                sanitized_tx.message(),
+                &rent_collector,
             )
             .len(),
             num_accounts,