diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 800bd1dc4ea1d3..74c0219a170d37 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -8,10 +8,9 @@ /programs/loader-v4/ @anza-xyz/svm /runtime-transaction/ @anza-xyz/tx-metadata /svm-conformance/ @anza-xyz/svm -/svm-rent-collector/ @anza-xyz/svm /svm-transaction/ @anza-xyz/svm /svm/ @anza-xyz/svm /svm/examples/Cargo.lock /svm-callback/ @anza-xyz/svm /transaction-context/ @anza-xyz/svm -/transaction-view/ @anza-xyz/tx-metadata \ No newline at end of file +/transaction-view/ @anza-xyz/tx-metadata diff --git a/Cargo.lock b/Cargo.lock index 55758deed44989..45ddf00ebe075f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10282,7 +10282,6 @@ dependencies = [ "solana-stake-program", "solana-svm", "solana-svm-callback", - "solana-svm-rent-collector", "solana-svm-transaction", "solana-system-interface", "solana-system-program", @@ -10919,7 +10918,6 @@ dependencies = [ "solana-svm-callback", "solana-svm-conformance", "solana-svm-feature-set", - "solana-svm-rent-collector", "solana-svm-transaction", "solana-system-interface", "solana-system-program", @@ -10959,21 +10957,6 @@ dependencies = [ name = "solana-svm-feature-set" version = "3.0.0" -[[package]] -name = "solana-svm-rent-collector" -version = "3.0.0" -dependencies = [ - "solana-account", - "solana-clock", - "solana-epoch-schedule", - "solana-pubkey", - "solana-rent", - "solana-rent-collector", - "solana-sdk-ids", - "solana-transaction-context", - "solana-transaction-error", -] - [[package]] name = "solana-svm-transaction" version = "3.0.0" diff --git a/Cargo.toml b/Cargo.toml index fb20aab1672f53..be85ab298a016f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -106,7 +106,6 @@ members = [ "svm-callback", "svm-conformance", "svm-feature-set", - "svm-rent-collector", "svm-transaction", "syscalls", "syscalls/gen-syscall-list", @@ -527,7 +526,6 @@ solana-svm = { path = "svm", version = "=3.0.0" } solana-svm-callback = { path = "svm-callback", version = "=3.0.0" } solana-svm-conformance = { path = "svm-conformance", version = "=3.0.0" } solana-svm-feature-set = { path = "svm-feature-set", version = "=3.0.0" } -solana-svm-rent-collector = { path = "svm-rent-collector", version = "=3.0.0" } solana-svm-transaction = { path = "svm-transaction", version = "=3.0.0" } solana-system-interface = "1.0" solana-system-program = { path = "programs/system", version = "=3.0.0" } diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 7fc4974592b511..4b477812466270 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -473,7 +473,7 @@ impl Consumer { &mut fee_payer_account, 0, error_counters, - bank.rent_collector(), + &bank.rent_collector().rent, fee, ) } diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 43287d92f11c2a..4623febf6d3704 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -7924,7 +7924,6 @@ dependencies = [ "solana-stake-program", "solana-svm", "solana-svm-callback", - "solana-svm-rent-collector", "solana-svm-transaction", "solana-system-interface", "solana-system-program", @@ -9168,7 +9167,6 @@ dependencies = [ "solana-slot-hashes", "solana-svm-callback", "solana-svm-feature-set", - "solana-svm-rent-collector", "solana-svm-transaction", "solana-system-interface", "solana-sysvar-id", @@ -9193,20 +9191,6 @@ dependencies = [ name = "solana-svm-feature-set" version = "3.0.0" -[[package]] -name = "solana-svm-rent-collector" -version = "3.0.0" -dependencies = [ - "solana-account", - "solana-clock", - "solana-pubkey", - "solana-rent", - "solana-rent-collector", - "solana-sdk-ids", - "solana-transaction-context", - "solana-transaction-error", -] - [[package]] name = "solana-svm-transaction" version = "3.0.0" diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 279ba10649bba2..27fbeca8205ffe 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -154,7 +154,6 @@ solana-stake-interface = { workspace = true } solana-stake-program = { workspace = true } solana-svm = { workspace = true } solana-svm-callback = { workspace = true } -solana-svm-rent-collector = { workspace = true } solana-svm-transaction = { workspace = true } solana-system-interface = { workspace = true } solana-system-program = { workspace = true, optional = true } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 5f8d73acaabd47..2c74006dc571ce 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -44,7 +44,6 @@ use { epoch_stakes::{NodeVoteAccounts, VersionedEpochStakes}, inflation_rewards::points::InflationPointCalculationEvent, installed_scheduler_pool::{BankWithScheduler, InstalledSchedulerRwLock}, - rent_collector::RentCollectorWithMetrics, runtime_config::RuntimeConfig, snapshot_hash::SnapshotHash, stake_account::StakeAccount, @@ -3266,14 +3265,12 @@ impl Bank { let (blockhash, blockhash_lamports_per_signature) = self.last_blockhash_and_lamports_per_signature(); - let rent_collector_with_metrics = - RentCollectorWithMetrics::new(self.rent_collector.clone()); let processing_environment = TransactionProcessingEnvironment { blockhash, blockhash_lamports_per_signature, epoch_total_stake: self.get_current_epoch_total_stake(), feature_set: self.feature_set.runtime_features(), - rent_collector: Some(&rent_collector_with_metrics), + rent: self.rent_collector.rent.clone(), }; let sanitized_output = self diff --git a/runtime/src/bank/fee_distribution.rs b/runtime/src/bank/fee_distribution.rs index 37b4d2d0535e0d..89f2975f53549c 100644 --- a/runtime/src/bank/fee_distribution.rs +++ b/runtime/src/bank/fee_distribution.rs @@ -8,7 +8,7 @@ use { solana_pubkey::Pubkey, solana_reward_info::{RewardInfo, RewardType}, solana_runtime_transaction::transaction_with_meta::TransactionWithMeta, - solana_svm_rent_collector::svm_rent_collector::SVMRentCollector, + solana_svm::rent_calculator::{get_account_rent_state, transition_allowed}, solana_system_interface::program as system_program, std::{result::Result, sync::atomic::Ordering::Relaxed}, thiserror::Error, @@ -153,16 +153,17 @@ impl Bank { return Err(DepositFeeError::InvalidAccountOwner); } - let recipient_pre_rent_state = self.rent_collector().get_account_rent_state(&account); + let recipient_pre_rent_state = + get_account_rent_state(&self.rent_collector().rent, &account); let distribution = account.checked_add_lamports(fees); if distribution.is_err() { return Err(DepositFeeError::LamportOverflow); } - 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); + let recipient_post_rent_state = + get_account_rent_state(&self.rent_collector().rent, &account); + let rent_state_transition_allowed = + transition_allowed(&recipient_pre_rent_state, &recipient_post_rent_state); if !rent_state_transition_allowed { return Err(DepositFeeError::InvalidRentPayingAccount); } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index b203365dfb9c96..138d9b38560207 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -18,7 +18,6 @@ pub mod loader_utils; pub mod non_circulating_supply; pub mod prioritization_fee; pub mod prioritization_fee_cache; -pub mod rent_collector; pub mod root_bank_cache; pub mod runtime_config; pub mod serde_snapshot; diff --git a/runtime/src/rent_collector.rs b/runtime/src/rent_collector.rs deleted file mode 100644 index 7392d49df430e7..00000000000000 --- a/runtime/src/rent_collector.rs +++ /dev/null @@ -1,84 +0,0 @@ -//! Bank's wrapper around `RentCollector` to allow for overriding of some -//! `SVMRentCollector` trait methods, which are otherwise implemented on -//! `RentCollector` directly. -//! -//! Agave requires submission of logs and metrics during account rent state -//! assessment, which is not included in the `RentCollector` implementation -//! of the `SVMRentCollector` trait. This wrapper allows all `SVMRentCollector` -//! methods to be passed through to the underlying `RentCollector`, except for -//! those which require additional logging and metrics. - -use { - log::*, - solana_account::AccountSharedData, - solana_clock::Epoch, - solana_pubkey::Pubkey, - solana_rent::{Rent, RentDue}, - 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}, -}; - -/// Wrapper around `RentCollector` to allow for overriding of some -/// `SVMRentCollector` trait methods, which are otherwise implemented on -/// `RentCollector` directly. -/// -/// Overrides inject logging and metrics submission into the rent state -/// assessment process. -pub struct RentCollectorWithMetrics(RentCollector); - -impl RentCollectorWithMetrics { - pub fn new(rent_collector: RentCollector) -> Self { - Self(rent_collector) - } -} - -impl SVMRentCollector for RentCollectorWithMetrics { - fn get_rent(&self) -> &Rent { - self.0.get_rent() - } - - fn get_rent_due(&self, lamports: u64, data_len: usize, account_rent_epoch: Epoch) -> RentDue { - self.0.get_rent_due(lamports, data_len, account_rent_epoch) - } - - // Overriden to inject logging and metrics. - fn check_rent_state_with_account( - &self, - pre_rent_state: &RentState, - post_rent_state: &RentState, - address: &Pubkey, - account_state: &AccountSharedData, - account_index: IndexOfAccount, - ) -> Result<()> { - submit_rent_state_metrics(pre_rent_state, post_rent_state); - if !solana_sdk_ids::incinerator::check_id(address) - && !self.transition_allowed(pre_rent_state, post_rent_state) - { - debug!( - "Account {} not rent exempt, state {:?}", - address, account_state, - ); - let account_index = account_index as u8; - Err(TransactionError::InsufficientFundsForRent { account_index }) - } else { - Ok(()) - } - } -} - -fn submit_rent_state_metrics(pre_rent_state: &RentState, post_rent_state: &RentState) { - match (pre_rent_state, post_rent_state) { - (&RentState::Uninitialized, &RentState::RentPaying { .. }) => { - inc_new_counter_info!("rent_paying_err-new_account", 1); - } - (&RentState::RentPaying { .. }, &RentState::RentPaying { .. }) => { - inc_new_counter_info!("rent_paying_ok-legacy", 1); - } - (_, &RentState::RentPaying { .. }) => { - inc_new_counter_info!("rent_paying_err-other", 1); - } - _ => {} - } -} diff --git a/scripts/patch-crates.sh b/scripts/patch-crates.sh index 88c9ad73d18172..f07e87dbf4325a 100644 --- a/scripts/patch-crates.sh +++ b/scripts/patch-crates.sh @@ -61,7 +61,7 @@ update_solana_dependencies() { solana-storage-bigtable solana-storage-proto solana-streamer - solana-svm-rent-collector + solana-svm-rent-calculator solana-svm-transaction solana-test-validator solana-thin-client diff --git a/svm-rent-collector/Cargo.toml b/svm-rent-calculator/Cargo.toml similarity index 62% rename from svm-rent-collector/Cargo.toml rename to svm-rent-calculator/Cargo.toml index 7dec6d3a7d6e7d..1695328298ae47 100644 --- a/svm-rent-collector/Cargo.toml +++ b/svm-rent-calculator/Cargo.toml @@ -1,7 +1,7 @@ [package] -name = "solana-svm-rent-collector" -description = "Solana SVM Rent Collector" -documentation = "https://docs.rs/solana-svm-rent-collector" +name = "solana-svm-rent-calculator" +description = "Solana SVM Rent Calculator" +documentation = "https://docs.rs/solana-svm-rent-calculator" version = { workspace = true } authors = { workspace = true } repository = { workspace = true } @@ -11,13 +11,8 @@ edition = { workspace = true } [dependencies] solana-account = { workspace = true } -solana-clock = { workspace = true } solana-pubkey = { workspace = true } solana-rent = { workspace = true } -solana-rent-collector = { workspace = true } solana-sdk-ids = { workspace = true } solana-transaction-context = { workspace = true } solana-transaction-error = { workspace = true } - -[dev-dependencies] -solana-epoch-schedule = { workspace = true } diff --git a/svm-rent-calculator/src/lib.rs b/svm-rent-calculator/src/lib.rs new file mode 100644 index 00000000000000..94277bb5bbd6f9 --- /dev/null +++ b/svm-rent-calculator/src/lib.rs @@ -0,0 +1,6 @@ +//! Solana SVM Rent Calculator. +//! +//! Rent management for SVM. + +pub mod rent_state; +pub mod svm_rent_calculator; diff --git a/svm-rent-collector/src/rent_state.rs b/svm-rent-calculator/src/rent_state.rs similarity index 100% rename from svm-rent-collector/src/rent_state.rs rename to svm-rent-calculator/src/rent_state.rs diff --git a/svm-rent-calculator/src/svm_rent_calculator.rs b/svm-rent-calculator/src/svm_rent_calculator.rs new file mode 100644 index 00000000000000..2e6596f0c8c358 --- /dev/null +++ b/svm-rent-calculator/src/svm_rent_calculator.rs @@ -0,0 +1,107 @@ +//! Helpers for rent calculation within the Solana VM. + +use { + crate::rent_state::RentState, + solana_account::{AccountSharedData, ReadableAccount}, + solana_pubkey::Pubkey, + solana_rent::Rent, + solana_transaction_context::{IndexOfAccount, TransactionContext}, + solana_transaction_error::{TransactionError, TransactionResult}, +}; + +/// Check rent state transition for an account in a transaction. +/// +/// This method has a default implementation that calls into +/// `check_rent_state_with_account`. +pub fn check_rent_state( + pre_rent_state: Option<&RentState>, + post_rent_state: Option<&RentState>, + transaction_context: &TransactionContext, + index: IndexOfAccount, +) -> TransactionResult<()> { + if let Some((pre_rent_state, post_rent_state)) = pre_rent_state.zip(post_rent_state) { + let expect_msg = "account must exist at TransactionContext index if rent-states are Some"; + check_rent_state_with_account( + pre_rent_state, + post_rent_state, + transaction_context + .get_key_of_account_at_index(index) + .expect(expect_msg), + &transaction_context + .accounts() + .try_borrow(index) + .expect(expect_msg), + index, + )?; + } + Ok(()) +} + +/// Check rent state transition for an account directly. +/// +/// This method has a default implementation that checks whether the +/// transition is allowed and returns an error if it is not. It also +/// verifies that the account is not the incinerator. +pub fn check_rent_state_with_account( + pre_rent_state: &RentState, + post_rent_state: &RentState, + address: &Pubkey, + _account_state: &AccountSharedData, + account_index: IndexOfAccount, +) -> TransactionResult<()> { + if !solana_sdk_ids::incinerator::check_id(address) + && !transition_allowed(pre_rent_state, post_rent_state) + { + let account_index = account_index as u8; + Err(TransactionError::InsufficientFundsForRent { account_index }) + } else { + Ok(()) + } +} + +/// Determine the rent state of an account. +/// +/// This method has a default implementation that treats accounts with zero +/// lamports as uninitialized and uses the implemented `get_rent` to +/// determine whether an account is rent-exempt. +pub fn get_account_rent_state(rent: &Rent, account: &AccountSharedData) -> RentState { + if account.lamports() == 0 { + RentState::Uninitialized + } else if rent.is_exempt(account.lamports(), account.data().len()) { + RentState::RentExempt + } else { + RentState::RentPaying { + data_size: account.data().len(), + lamports: account.lamports(), + } + } +} + +/// Check whether a transition from the pre_rent_state to the +/// post_rent_state is valid. +/// +/// This method has a default implementation that allows transitions from +/// any state to `RentState::Uninitialized` or `RentState::RentExempt`. +/// Pre-state `RentState::RentPaying` can only transition to +/// `RentState::RentPaying` if the data size remains the same and the +/// account is not credited. +pub fn transition_allowed(pre_rent_state: &RentState, post_rent_state: &RentState) -> bool { + match post_rent_state { + RentState::Uninitialized | RentState::RentExempt => true, + RentState::RentPaying { + data_size: post_data_size, + lamports: post_lamports, + } => { + match pre_rent_state { + RentState::Uninitialized | RentState::RentExempt => false, + RentState::RentPaying { + data_size: pre_data_size, + lamports: pre_lamports, + } => { + // Cannot remain RentPaying if resized or credited. + post_data_size == pre_data_size && post_lamports <= pre_lamports + } + } + } + } +} diff --git a/svm-rent-collector/src/lib.rs b/svm-rent-collector/src/lib.rs deleted file mode 100644 index 5038410e9e9f6f..00000000000000 --- a/svm-rent-collector/src/lib.rs +++ /dev/null @@ -1,6 +0,0 @@ -//! Solana SVM Rent Collector. -//! -//! Rent management for SVM. - -pub mod rent_state; -pub mod svm_rent_collector; diff --git a/svm-rent-collector/src/svm_rent_collector.rs b/svm-rent-collector/src/svm_rent_collector.rs deleted file mode 100644 index 867bf9d69dc759..00000000000000 --- a/svm-rent-collector/src/svm_rent_collector.rs +++ /dev/null @@ -1,131 +0,0 @@ -//! Plugin trait for rent collection within the Solana SVM. - -use { - crate::rent_state::RentState, - solana_account::{AccountSharedData, ReadableAccount}, - solana_clock::Epoch, - solana_pubkey::Pubkey, - solana_rent::{Rent, RentDue}, - solana_transaction_context::{IndexOfAccount, TransactionContext}, - solana_transaction_error::{TransactionError, TransactionResult}, -}; - -mod rent_collector; - -/// Rent collector trait. Represents an entity that can evaluate the rent state -/// of an account, determine rent due, and collect rent. -/// -/// Implementors are responsible for evaluating rent due and collecting rent -/// from accounts, if required. Methods for evaluating account rent state have -/// default implementations, which can be overridden for customized rent -/// management. -pub trait SVMRentCollector { - /// Check rent state transition for an account in a transaction. - /// - /// This method has a default implementation that calls into - /// `check_rent_state_with_account`. - fn check_rent_state( - &self, - pre_rent_state: Option<&RentState>, - post_rent_state: Option<&RentState>, - transaction_context: &TransactionContext, - index: IndexOfAccount, - ) -> TransactionResult<()> { - if let Some((pre_rent_state, post_rent_state)) = pre_rent_state.zip(post_rent_state) { - let expect_msg = - "account must exist at TransactionContext index if rent-states are Some"; - self.check_rent_state_with_account( - pre_rent_state, - post_rent_state, - transaction_context - .get_key_of_account_at_index(index) - .expect(expect_msg), - &transaction_context - .accounts() - .try_borrow(index) - .expect(expect_msg), - index, - )?; - } - Ok(()) - } - - /// Check rent state transition for an account directly. - /// - /// This method has a default implementation that checks whether the - /// transition is allowed and returns an error if it is not. It also - /// verifies that the account is not the incinerator. - fn check_rent_state_with_account( - &self, - pre_rent_state: &RentState, - post_rent_state: &RentState, - address: &Pubkey, - _account_state: &AccountSharedData, - account_index: IndexOfAccount, - ) -> TransactionResult<()> { - if !solana_sdk_ids::incinerator::check_id(address) - && !self.transition_allowed(pre_rent_state, post_rent_state) - { - let account_index = account_index as u8; - Err(TransactionError::InsufficientFundsForRent { account_index }) - } else { - Ok(()) - } - } - - /// Determine the rent state of an account. - /// - /// This method has a default implementation that treats accounts with zero - /// lamports as uninitialized and uses the implemented `get_rent` to - /// determine whether an account is rent-exempt. - fn get_account_rent_state(&self, account: &AccountSharedData) -> RentState { - if account.lamports() == 0 { - RentState::Uninitialized - } else if self - .get_rent() - .is_exempt(account.lamports(), account.data().len()) - { - RentState::RentExempt - } else { - RentState::RentPaying { - data_size: account.data().len(), - lamports: account.lamports(), - } - } - } - - /// Get the rent collector's rent instance. - fn get_rent(&self) -> &Rent; - - /// Get the rent due for an account. - fn get_rent_due(&self, lamports: u64, data_len: usize, account_rent_epoch: Epoch) -> RentDue; - - /// Check whether a transition from the pre_rent_state to the - /// post_rent_state is valid. - /// - /// This method has a default implementation that allows transitions from - /// any state to `RentState::Uninitialized` or `RentState::RentExempt`. - /// Pre-state `RentState::RentPaying` can only transition to - /// `RentState::RentPaying` if the data size remains the same and the - /// account is not credited. - fn transition_allowed(&self, pre_rent_state: &RentState, post_rent_state: &RentState) -> bool { - match post_rent_state { - RentState::Uninitialized | RentState::RentExempt => true, - RentState::RentPaying { - data_size: post_data_size, - lamports: post_lamports, - } => { - match pre_rent_state { - RentState::Uninitialized | RentState::RentExempt => false, - RentState::RentPaying { - data_size: pre_data_size, - lamports: pre_lamports, - } => { - // Cannot remain RentPaying if resized or credited. - post_data_size == pre_data_size && post_lamports <= pre_lamports - } - } - } - } - } -} diff --git a/svm-rent-collector/src/svm_rent_collector/rent_collector.rs b/svm-rent-collector/src/svm_rent_collector/rent_collector.rs deleted file mode 100644 index c4beb67a49cc6e..00000000000000 --- a/svm-rent-collector/src/svm_rent_collector/rent_collector.rs +++ /dev/null @@ -1,245 +0,0 @@ -//! Implementation of `SVMRentCollector` for `RentCollector` from the Solana -//! SDK. - -use { - crate::svm_rent_collector::SVMRentCollector, - solana_clock::Epoch, - solana_rent::{Rent, RentDue}, - solana_rent_collector::RentCollector, -}; - -impl SVMRentCollector for RentCollector { - fn get_rent(&self) -> &Rent { - &self.rent - } - - fn get_rent_due(&self, lamports: u64, data_len: usize, account_rent_epoch: Epoch) -> RentDue { - self.get_rent_due(lamports, data_len, account_rent_epoch) - } -} - -#[cfg(test)] -mod tests { - use { - super::*, - crate::rent_state::RentState, - solana_account::{AccountSharedData, ReadableAccount}, - solana_clock::Epoch, - solana_epoch_schedule::EpochSchedule, - solana_pubkey::Pubkey, - solana_transaction_context::{IndexOfAccount, TransactionContext}, - solana_transaction_error::TransactionError, - }; - - #[test] - fn test_get_account_rent_state() { - let program_id = Pubkey::new_unique(); - let uninitialized_account = AccountSharedData::new(0, 0, &Pubkey::default()); - - let account_data_size = 100; - - let rent_collector = RentCollector::new( - Epoch::default(), - EpochSchedule::default(), - 0.0, - Rent::free(), - ); - - let rent_exempt_account = AccountSharedData::new(1, account_data_size, &program_id); // if rent is free, all accounts with non-zero lamports and non-empty data are rent-exempt - - assert_eq!( - rent_collector.get_account_rent_state(&uninitialized_account), - RentState::Uninitialized - ); - assert_eq!( - rent_collector.get_account_rent_state(&rent_exempt_account), - RentState::RentExempt - ); - - let rent = Rent::default(); - let rent_minimum_balance = rent.minimum_balance(account_data_size); - let rent_paying_account = AccountSharedData::new( - rent_minimum_balance.saturating_sub(1), - account_data_size, - &program_id, - ); - let rent_exempt_account = AccountSharedData::new( - rent.minimum_balance(account_data_size), - account_data_size, - &program_id, - ); - let rent_collector = - RentCollector::new(Epoch::default(), EpochSchedule::default(), 0.0, rent); - - assert_eq!( - rent_collector.get_account_rent_state(&uninitialized_account), - RentState::Uninitialized - ); - assert_eq!( - rent_collector.get_account_rent_state(&rent_paying_account), - RentState::RentPaying { - data_size: account_data_size, - lamports: rent_paying_account.lamports(), - } - ); - assert_eq!( - rent_collector.get_account_rent_state(&rent_exempt_account), - RentState::RentExempt - ); - } - - #[test] - fn test_transition_allowed() { - let rent_collector = RentCollector::default(); - - let post_rent_state = RentState::Uninitialized; - assert!(rent_collector.transition_allowed(&RentState::Uninitialized, &post_rent_state)); - assert!(rent_collector.transition_allowed(&RentState::RentExempt, &post_rent_state)); - assert!(rent_collector.transition_allowed( - &RentState::RentPaying { - data_size: 0, - lamports: 1, - }, - &post_rent_state - )); - - let post_rent_state = RentState::RentExempt; - assert!(rent_collector.transition_allowed(&RentState::Uninitialized, &post_rent_state)); - assert!(rent_collector.transition_allowed(&RentState::RentExempt, &post_rent_state)); - assert!(rent_collector.transition_allowed( - &RentState::RentPaying { - data_size: 0, - lamports: 1, - }, - &post_rent_state - )); - - let post_rent_state = RentState::RentPaying { - data_size: 2, - lamports: 5, - }; - - // These transitions are not allowed. - assert!(!rent_collector.transition_allowed(&RentState::Uninitialized, &post_rent_state)); - assert!(!rent_collector.transition_allowed(&RentState::RentExempt, &post_rent_state)); - - // Transition is not allowed if data size changes. - assert!(!rent_collector.transition_allowed( - &RentState::RentPaying { - data_size: 3, - lamports: 5, - }, - &post_rent_state - )); - assert!(!rent_collector.transition_allowed( - &RentState::RentPaying { - data_size: 1, - lamports: 5, - }, - &post_rent_state - )); - - // Transition is always allowed if there is no account data resize or - // change in account's lamports. - assert!(rent_collector.transition_allowed( - &RentState::RentPaying { - data_size: 2, - lamports: 5, - }, - &post_rent_state - )); - // Transition is always allowed if there is no account data resize and - // account's lamports is reduced. - assert!(rent_collector.transition_allowed( - &RentState::RentPaying { - data_size: 2, - lamports: 7, - }, - &post_rent_state - )); - // Transition is not allowed if the account is credited with more - // lamports and remains rent-paying. - assert!(!rent_collector.transition_allowed( - &RentState::RentPaying { - data_size: 2, - lamports: 3, - }, - &post_rent_state - )); - } - - #[test] - fn test_check_rent_state_with_account() { - let rent_collector = RentCollector::default(); - - let pre_rent_state = RentState::RentPaying { - data_size: 2, - lamports: 3, - }; - - let post_rent_state = RentState::RentPaying { - data_size: 2, - lamports: 5, - }; - let account_index = 2 as IndexOfAccount; - let key = Pubkey::new_unique(); - let result = rent_collector.check_rent_state_with_account( - &pre_rent_state, - &post_rent_state, - &key, - &AccountSharedData::default(), - account_index, - ); - assert_eq!( - result.err(), - Some(TransactionError::InsufficientFundsForRent { - account_index: account_index as u8 - }) - ); - - let result = rent_collector.check_rent_state_with_account( - &pre_rent_state, - &post_rent_state, - &solana_sdk_ids::incinerator::id(), - &AccountSharedData::default(), - account_index, - ); - assert!(result.is_ok()); - } - - #[test] - fn test_check_rent_state() { - let rent_collector = RentCollector::default(); - - let context = TransactionContext::new( - vec![(Pubkey::new_unique(), AccountSharedData::default())], - Rent::default(), - 20, - 20, - ); - - let pre_rent_state = RentState::RentPaying { - data_size: 2, - lamports: 3, - }; - - let post_rent_state = RentState::RentPaying { - data_size: 2, - lamports: 5, - }; - - let result = rent_collector.check_rent_state( - Some(&pre_rent_state), - Some(&post_rent_state), - &context, - 0, - ); - assert_eq!( - result.err(), - Some(TransactionError::InsufficientFundsForRent { account_index: 0 }) - ); - - let result = rent_collector.check_rent_state(None, Some(&post_rent_state), &context, 0); - assert!(result.is_ok()); - } -} diff --git a/svm/Cargo.toml b/svm/Cargo.toml index 0c38d3f1969bfd..5cf40c13604599 100644 --- a/svm/Cargo.toml +++ b/svm/Cargo.toml @@ -72,7 +72,6 @@ solana-sdk-ids = { workspace = true } solana-slot-hashes = { workspace = true } solana-svm-callback = { workspace = true } solana-svm-feature-set = { workspace = true } -solana-svm-rent-collector = { workspace = true } solana-svm-transaction = { workspace = true } solana-system-interface = { workspace = true } solana-sysvar-id = { workspace = true } diff --git a/svm/examples/Cargo.lock b/svm/examples/Cargo.lock index 9a1eea51c51e35..0f7de40b9496f4 100644 --- a/svm/examples/Cargo.lock +++ b/svm/examples/Cargo.lock @@ -2982,6 +2982,7 @@ dependencies = [ "solana-perf", "solana-program-runtime", "solana-pubkey", + "solana-rent", "solana-rpc-client-api", "solana-sdk-ids", "solana-signature", @@ -7723,7 +7724,6 @@ dependencies = [ "solana-stake-program", "solana-svm", "solana-svm-callback", - "solana-svm-rent-collector", "solana-svm-transaction", "solana-system-interface", "solana-system-transaction", @@ -8218,7 +8218,6 @@ dependencies = [ "solana-slot-hashes", "solana-svm-callback", "solana-svm-feature-set", - "solana-svm-rent-collector", "solana-svm-transaction", "solana-system-interface", "solana-sysvar-id", @@ -8260,7 +8259,7 @@ dependencies = [ "solana-program-pack", "solana-program-runtime", "solana-pubkey", - "solana-rent-collector", + "solana-rent", "solana-sdk-ids", "solana-signer", "solana-svm", @@ -8280,20 +8279,6 @@ dependencies = [ name = "solana-svm-feature-set" version = "3.0.0" -[[package]] -name = "solana-svm-rent-collector" -version = "3.0.0" -dependencies = [ - "solana-account", - "solana-clock", - "solana-pubkey", - "solana-rent", - "solana-rent-collector", - "solana-sdk-ids", - "solana-transaction-context", - "solana-transaction-error", -] - [[package]] name = "solana-svm-transaction" version = "3.0.0" diff --git a/svm/examples/Cargo.toml b/svm/examples/Cargo.toml index 2e50375a270f8f..ac6530d96d270d 100644 --- a/svm/examples/Cargo.toml +++ b/svm/examples/Cargo.toml @@ -49,7 +49,7 @@ solana-perf = { path = "../../perf" } solana-program-pack = "2.2.1" solana-program-runtime = { path = "../../program-runtime" } solana-pubkey = "2.3.0" -solana-rent-collector = "2.2.1" +solana-rent = "2.2.1" solana-rpc-client-api = { path = "../../rpc-client-api" } solana-sdk-ids = "2.2.1" solana-signature = "2.2.1" diff --git a/svm/examples/json-rpc/server/Cargo.toml b/svm/examples/json-rpc/server/Cargo.toml index 34a5719ec0ec96..15d63429546e41 100644 --- a/svm/examples/json-rpc/server/Cargo.toml +++ b/svm/examples/json-rpc/server/Cargo.toml @@ -39,6 +39,7 @@ solana-nonce = { workspace = true } solana-perf = { workspace = true } solana-program-runtime = { workspace = true } solana-pubkey = { workspace = true } +solana-rent = { workspace = true } solana-rpc-client-api = { workspace = true } solana-sdk-ids = { workspace = true } solana-signature = { workspace = true } diff --git a/svm/examples/json-rpc/server/src/rpc_process.rs b/svm/examples/json-rpc/server/src/rpc_process.rs index d75d91bf79fdf0..c104bcede95643 100644 --- a/svm/examples/json-rpc/server/src/rpc_process.rs +++ b/svm/examples/json-rpc/server/src/rpc_process.rs @@ -32,6 +32,7 @@ use { loaded_programs::ProgramCacheEntry, }, solana_pubkey::Pubkey, + solana_rent::Rent, solana_rpc_client_api::{ config::*, response::{Response as RpcResponse, *}, @@ -555,7 +556,7 @@ impl JsonRpcRequestProcessor { blockhash_lamports_per_signature: lamports_per_signature, epoch_total_stake: 0, feature_set: bank.feature_set.runtime_features(), - rent_collector: None, + rent: Rent::default(), }; let sanitized_output = self diff --git a/svm/examples/paytube/Cargo.toml b/svm/examples/paytube/Cargo.toml index 2139bf46fdf4b5..b7def3734e8469 100644 --- a/svm/examples/paytube/Cargo.toml +++ b/svm/examples/paytube/Cargo.toml @@ -27,7 +27,7 @@ solana-logger = { workspace = true } solana-program-pack = { workspace = true } solana-program-runtime = { workspace = true, features = ["dev-context-only-utils"] } solana-pubkey = { workspace = true } -solana-rent-collector = { workspace = true } +solana-rent = { workspace = true } solana-sdk-ids = { workspace = true } solana-signer = { workspace = true } solana-svm = { workspace = true } diff --git a/svm/examples/paytube/src/lib.rs b/svm/examples/paytube/src/lib.rs index c831eceac66987..5caffc6efcb83e 100644 --- a/svm/examples/paytube/src/lib.rs +++ b/svm/examples/paytube/src/lib.rs @@ -71,7 +71,7 @@ use { solana_hash::Hash, solana_keypair::Keypair, solana_program_runtime::execution_budget::SVMTransactionExecutionBudget, - solana_rent_collector::RentCollector, + solana_rent::Rent, solana_svm::transaction_processor::{ TransactionProcessingConfig, TransactionProcessingEnvironment, }, @@ -118,7 +118,7 @@ impl PayTubeChannel { let compute_budget = SVMTransactionExecutionBudget::default(); let feature_set = SVMFeatureSet::all_enabled(); let fee_structure = FeeStructure::default(); - let rent_collector = RentCollector::default(); + let rent = Rent::default(); // PayTube loader/callback implementation. // @@ -151,7 +151,7 @@ impl PayTubeChannel { blockhash_lamports_per_signature: fee_structure.lamports_per_signature, epoch_total_stake: 0, feature_set, - rent_collector: Some(&rent_collector), + rent, }; // The PayTube transaction processing config for Solana SVM. diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index abd9ba4697080d..69344124c04d37 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -2,8 +2,11 @@ use qualifier_attr::{field_qualifiers, qualifiers}; use { crate::{ - account_overrides::AccountOverrides, nonce_info::NonceInfo, - rollback_accounts::RollbackAccounts, transaction_error_metrics::TransactionErrorMetrics, + account_overrides::AccountOverrides, + nonce_info::NonceInfo, + rent_calculator::{check_rent_state_with_account, get_account_rent_state}, + rollback_accounts::RollbackAccounts, + transaction_error_metrics::TransactionErrorMetrics, transaction_execution_result::ExecutedTransaction, }, ahash::{AHashMap, AHashSet}, @@ -21,7 +24,7 @@ use { SVMTransactionExecutionAndFeeBudgetLimits, SVMTransactionExecutionBudget, }, solana_pubkey::Pubkey, - solana_rent::RentDue, + solana_rent::Rent, solana_rent_collector::RENT_EXEMPT_RENT_EPOCH, solana_sdk_ids::{ bpf_loader_upgradeable, native_loader, @@ -29,7 +32,6 @@ use { }, solana_svm_callback::{AccountState, TransactionProcessingCallback}, solana_svm_feature_set::SVMFeatureSet, - solana_svm_rent_collector::svm_rent_collector::SVMRentCollector, solana_svm_transaction::svm_message::SVMMessage, solana_transaction_context::{IndexOfAccount, TransactionAccount}, solana_transaction_error::{TransactionError, TransactionResult as Result}, @@ -339,21 +341,14 @@ impl solana_svm_callback::InvokeContextCallba /// TODO: This function is used to update the rent epoch of an account. Once we /// completely switched to lthash, where rent_epoch is ignored in accounts /// hashing, we can remove this function. -pub fn update_rent_exempt_status_for_account( - rent_collector: &dyn SVMRentCollector, - account: &mut AccountSharedData, -) { +pub fn update_rent_exempt_status_for_account(rent: &Rent, account: &mut AccountSharedData) { // 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 + && rent.is_exempt(account.lamports(), account.data().len()) { account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); } @@ -369,7 +364,7 @@ pub fn validate_fee_payer( payer_account: &mut AccountSharedData, payer_index: IndexOfAccount, error_metrics: &mut TransactionErrorMetrics, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, fee: u64, ) -> Result<()> { if payer_account.lamports() == 0 { @@ -385,9 +380,7 @@ 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 - .get_rent() - .minimum_balance(NonceState::size()) + rent.minimum_balance(NonceState::size()) } }; @@ -400,13 +393,13 @@ pub fn validate_fee_payer( TransactionError::InsufficientFundsForFee })?; - let payer_pre_rent_state = rent_collector.get_account_rent_state(payer_account); + let payer_pre_rent_state = get_account_rent_state(rent, payer_account); payer_account .checked_sub_lamports(fee) .map_err(|_| TransactionError::InsufficientFundsForFee)?; - let payer_post_rent_state = rent_collector.get_account_rent_state(payer_account); - rent_collector.check_rent_state_with_account( + let payer_post_rent_state = get_account_rent_state(rent, payer_account); + check_rent_state_with_account( &payer_pre_rent_state, &payer_post_rent_state, payer_address, @@ -420,7 +413,7 @@ pub(crate) fn load_transaction( message: &impl SVMMessage, validation_result: TransactionValidationResult, error_metrics: &mut TransactionErrorMetrics, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, ) -> TransactionLoadResult { match validation_result { Err(e) => TransactionLoadResult::NotLoaded(e), @@ -431,7 +424,7 @@ pub(crate) fn load_transaction( tx_details.loaded_fee_payer_account, tx_details.loaded_accounts_bytes_limit, error_metrics, - rent_collector, + rent, ); match load_result { @@ -491,7 +484,7 @@ fn load_transaction_accounts( loaded_fee_payer_account: LoadedTransactionAccount, loaded_accounts_bytes_limit: NonZeroU32, error_metrics: &mut TransactionErrorMetrics, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, ) -> Result { if account_loader .feature_set @@ -503,7 +496,7 @@ fn load_transaction_accounts( loaded_fee_payer_account, loaded_accounts_bytes_limit, error_metrics, - rent_collector, + rent, ) } else { load_transaction_accounts_old( @@ -512,7 +505,7 @@ fn load_transaction_accounts( loaded_fee_payer_account, loaded_accounts_bytes_limit, error_metrics, - rent_collector, + rent, ) } } @@ -523,7 +516,7 @@ fn load_transaction_accounts_simd186( loaded_fee_payer_account: LoadedTransactionAccount, loaded_accounts_bytes_limit: NonZeroU32, error_metrics: &mut TransactionErrorMetrics, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, ) -> Result { let account_keys = message.account_keys(); let mut additional_loaded_accounts: AHashSet = AHashSet::new(); @@ -608,13 +601,8 @@ fn load_transaction_accounts_simd186( // Attempt to load and collect remaining non-fee payer accounts. for (account_index, account_key) in account_keys.iter().enumerate().skip(1) { - let loaded_account = load_transaction_account( - account_loader, - message, - account_key, - account_index, - rent_collector, - ); + let loaded_account = + load_transaction_account(account_loader, message, account_key, account_index, rent); collect_loaded_account(account_loader, account_key, loaded_account)?; } @@ -653,7 +641,7 @@ fn load_transaction_accounts_old( loaded_fee_payer_account: LoadedTransactionAccount, loaded_accounts_bytes_limit: NonZeroU32, error_metrics: &mut TransactionErrorMetrics, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, ) -> Result { let account_keys = message.account_keys(); let mut accounts = Vec::with_capacity(account_keys.len()); @@ -683,13 +671,8 @@ fn load_transaction_accounts_old( // Attempt to load and collect remaining non-fee payer accounts for (account_index, account_key) in account_keys.iter().enumerate().skip(1) { - let loaded_account = load_transaction_account( - account_loader, - message, - account_key, - account_index, - rent_collector, - ); + let loaded_account = + load_transaction_account(account_loader, message, account_key, account_index, rent); collect_loaded_account(account_key, loaded_account)?; } @@ -762,7 +745,7 @@ fn load_transaction_account( message: &impl SVMMessage, account_key: &Pubkey, account_index: usize, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, ) -> LoadedTransactionAccount { let is_writable = message.is_writable(account_index); let loaded_account = if solana_sdk_ids::sysvar::instructions::check_id(account_key) { @@ -776,7 +759,7 @@ fn load_transaction_account( account_loader.load_transaction_account(account_key, is_writable) { if is_writable { - update_rent_exempt_status_for_account(rent_collector, &mut loaded_account.account); + update_rent_exempt_status_for_account(rent, &mut loaded_account.account); } loaded_account } else { @@ -853,7 +836,6 @@ mod tests { agave_reserved_account_keys::ReservedAccountKeys, rand0_7::prelude::*, solana_account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, - solana_epoch_schedule::EpochSchedule, solana_hash::Hash, solana_instruction::{AccountMeta, Instruction}, solana_keypair::Keypair, @@ -870,7 +852,7 @@ mod tests { }, solana_pubkey::Pubkey, solana_rent::Rent, - solana_rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, + solana_rent_collector::RENT_EXEMPT_RENT_EPOCH, solana_sdk_ids::{ bpf_loader, bpf_loader_upgradeable, native_loader, system_program, sysvar, }, @@ -947,7 +929,7 @@ mod tests { fn load_accounts_with_features_and_rent( tx: Transaction, accounts: &[TransactionAccount], - rent_collector: &RentCollector, + rent: &Rent, error_metrics: &mut TransactionErrorMetrics, feature_set: SVMFeatureSet, ) -> TransactionLoadResult { @@ -974,7 +956,7 @@ mod tests { ..ValidatedTransactionDetails::default() }), error_metrics, - rent_collector, + rent, ) } @@ -1016,7 +998,7 @@ mod tests { let load_results = load_accounts_with_features_and_rent( tx, &accounts, - &RentCollector::default(), + &Rent::default(), &mut error_metrics, feature_set, ); @@ -1064,7 +1046,7 @@ mod tests { let loaded_accounts = load_accounts_with_features_and_rent( tx, &accounts, - &RentCollector::default(), + &Rent::default(), &mut error_metrics, feature_set, ); @@ -1124,7 +1106,7 @@ mod tests { let load_results = load_accounts_with_features_and_rent( tx, &accounts, - &RentCollector::default(), + &Rent::default(), &mut error_metrics, feature_set, ); @@ -1182,7 +1164,7 @@ mod tests { let load_results = load_accounts_with_features_and_rent( tx, &accounts, - &RentCollector::default(), + &Rent::default(), &mut error_metrics, feature_set, ); @@ -1242,7 +1224,7 @@ mod tests { let loaded_accounts = load_accounts_with_features_and_rent( tx, &accounts, - &RentCollector::default(), + &Rent::default(), &mut error_metrics, feature_set, ); @@ -1289,7 +1271,7 @@ mod tests { &tx, Ok(ValidatedTransactionDetails::default()), &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ) } @@ -1398,10 +1380,7 @@ mod tests { expected_result: Result<()>, payer_post_balance: u64, } - fn validate_fee_payer_account( - test_parameter: ValidateFeePayerTestParameter, - rent_collector: &RentCollector, - ) { + fn validate_fee_payer_account(test_parameter: ValidateFeePayerTestParameter, rent: &Rent) { let payer_account_keys = Keypair::new(); let mut account = if test_parameter.is_nonce { AccountSharedData::new_data( @@ -1418,7 +1397,7 @@ mod tests { &mut account, 0, &mut TransactionErrorMetrics::default(), - rent_collector, + rent, test_parameter.fee, ); @@ -1428,16 +1407,11 @@ mod tests { #[test] fn test_validate_fee_payer() { - let rent_collector = RentCollector::new( - 0, - EpochSchedule::default(), - 500_000.0, - Rent { - lamports_per_byte_year: 1, - ..Rent::default() - }, - ); - let min_balance = rent_collector.rent.minimum_balance(NonceState::size()); + let rent = Rent { + lamports_per_byte_year: 1, + ..Rent::default() + }; + let min_balance = rent.minimum_balance(NonceState::size()); let fee = 5_000; // If payer account has sufficient balance, expect successful fee deduction, @@ -1452,7 +1426,7 @@ mod tests { expected_result: Ok(()), payer_post_balance: min_balance, }, - &rent_collector, + &rent, ); } } @@ -1469,7 +1443,7 @@ mod tests { expected_result: Err(TransactionError::AccountNotFound), payer_post_balance: 0, }, - &rent_collector, + &rent, ); } } @@ -1486,7 +1460,7 @@ mod tests { expected_result: Err(TransactionError::InsufficientFundsForFee), payer_post_balance: min_balance + fee - 1, }, - &rent_collector, + &rent, ); } } @@ -1502,22 +1476,17 @@ mod tests { expected_result: Ok(()), payer_post_balance: 0, }, - &rent_collector, + &rent, ); } } #[test] fn test_validate_nonce_fee_payer_with_checked_arithmetic() { - let rent_collector = RentCollector::new( - 0, - EpochSchedule::default(), - 500_000.0, - Rent { - lamports_per_byte_year: 1, - ..Rent::default() - }, - ); + let rent = Rent { + lamports_per_byte_year: 1, + ..Rent::default() + }; // nonce payer account has balance of u64::MAX, so does fee; due to nonce account // requires additional min_balance, expect InsufficientFundsForFee error if feature gate is @@ -1530,7 +1499,7 @@ mod tests { expected_result: Err(TransactionError::InsufficientFundsForFee), payer_post_balance: u64::MAX, }, - &rent_collector, + &rent, ); } @@ -1588,7 +1557,7 @@ mod tests { }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); assert_eq!( result.unwrap(), @@ -1652,7 +1621,7 @@ mod tests { }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); if formalize_loaded_transaction_data_size { @@ -1715,7 +1684,7 @@ mod tests { LoadedTransactionAccount::default(), MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1757,7 +1726,7 @@ mod tests { LoadedTransactionAccount::default(), MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); assert_eq!( @@ -1825,7 +1794,7 @@ mod tests { }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); let loaded_accounts_data_size = base_account_size as u32 * 2; @@ -1886,7 +1855,7 @@ mod tests { LoadedTransactionAccount::default(), MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); @@ -1938,7 +1907,7 @@ mod tests { LoadedTransactionAccount::default(), MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); assert_eq!( @@ -2014,7 +1983,7 @@ mod tests { }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); let loaded_accounts_data_size = base_account_size as u32 * 2; @@ -2110,7 +2079,7 @@ mod tests { }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); let loaded_accounts_data_size = base_account_size as u32 * 2; @@ -2169,7 +2138,7 @@ mod tests { &sanitized_tx.clone(), Ok(ValidatedTransactionDetails::default()), &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); let TransactionLoadResult::Loaded(loaded_transaction) = load_result else { @@ -2180,21 +2149,17 @@ mod tests { compute_unit_limit: u64::from(DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT), ..SVMTransactionExecutionBudget::default() }; - let rent_collector = RentCollector::default(); + let rent = Rent::default(); let transaction_context = TransactionContext::new( loaded_transaction.accounts, - rent_collector.get_rent().clone(), + rent.clone(), compute_budget.max_instruction_stack_depth, compute_budget.max_instruction_trace_length, ); assert_eq!( - TransactionAccountStateInfo::new( - &transaction_context, - sanitized_tx.message(), - &rent_collector, - ) - .len(), + TransactionAccountStateInfo::new(&transaction_context, sanitized_tx.message(), &rent,) + .len(), num_accounts, ); } @@ -2276,7 +2241,7 @@ mod tests { &sanitized_transaction, validation_result, &mut error_metrics, - &RentCollector::default(), + &Rent::default(), ); let loaded_accounts_data_size = base_account_size as u32 * 2; @@ -2314,7 +2279,7 @@ mod tests { fn test_load_accounts_error() { let mock_bank = TestCallbacks::default(); let mut account_loader = (&mock_bank).into(); - let rent_collector = RentCollector::default(); + let rent = Rent::default(); let message = Message { account_keys: vec![Pubkey::new_from_array([0; 32])], @@ -2340,7 +2305,7 @@ mod tests { &sanitized_transaction, validation_result.clone(), &mut TransactionErrorMetrics::default(), - &rent_collector, + &rent, ); assert!(matches!( @@ -2358,7 +2323,7 @@ mod tests { &sanitized_transaction, validation_result, &mut TransactionErrorMetrics::default(), - &rent_collector, + &rent, ); assert!(matches!( @@ -2369,34 +2334,28 @@ mod tests { #[test] fn test_update_rent_exempt_status_for_account() { - let rent_collector = RentCollector { - epoch: 1, - ..RentCollector::default() - }; + let rent = Rent::default(); - let min_exempt_balance = rent_collector.rent.minimum_balance(0); + let min_exempt_balance = rent.minimum_balance(0); let mut account = AccountSharedData::from(Account { lamports: min_exempt_balance, ..Account::default() }); - update_rent_exempt_status_for_account(&rent_collector, &mut account); + update_rent_exempt_status_for_account(&rent, &mut account); assert_eq!(account.rent_epoch(), RENT_EXEMPT_RENT_EPOCH); } #[test] fn test_update_rent_exempt_status_for_rent_paying_account() { - let rent_collector = RentCollector { - epoch: 1, - ..RentCollector::default() - }; + let rent = Rent::default(); let mut account = AccountSharedData::from(Account { lamports: 1, ..Account::default() }); - update_rent_exempt_status_for_account(&rent_collector, &mut account); + update_rent_exempt_status_for_account(&rent, &mut account); assert_eq!(account.rent_epoch(), 0); assert_eq!(account.lamports(), 1); } @@ -2469,7 +2428,7 @@ mod tests { &sanitized_transaction, validation_result, &mut TransactionErrorMetrics::default(), - &RentCollector::default(), + &Rent::default(), ); // ensure the loaded accounts are inspected @@ -2594,7 +2553,7 @@ mod tests { }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut TransactionErrorMetrics::default(), - &RentCollector::default(), + &Rent::default(), ) .unwrap(); @@ -3026,7 +2985,7 @@ mod tests { }, MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES, &mut TransactionErrorMetrics::default(), - &RentCollector::default(), + &Rent::default(), ) .unwrap(); diff --git a/svm/src/lib.rs b/svm/src/lib.rs index d6bda1ec522e32..f6f018938ebc80 100644 --- a/svm/src/lib.rs +++ b/svm/src/lib.rs @@ -6,6 +6,7 @@ pub mod account_overrides; pub mod message_processor; pub mod nonce_info; pub mod program_loader; +pub mod rent_calculator; pub mod rollback_accounts; pub mod transaction_account_state_info; pub mod transaction_balances; diff --git a/svm/src/rent_calculator.rs b/svm/src/rent_calculator.rs new file mode 100644 index 00000000000000..3b64ff75a26912 --- /dev/null +++ b/svm/src/rent_calculator.rs @@ -0,0 +1,122 @@ +//! Solana SVM Rent Calculator. +//! +//! Rent management for SVM. + +use { + solana_account::{AccountSharedData, ReadableAccount}, + solana_pubkey::Pubkey, + solana_rent::Rent, + solana_transaction_context::{IndexOfAccount, TransactionContext}, + solana_transaction_error::{TransactionError, TransactionResult}, +}; + +/// Rent state of a Solana account. +#[derive(Debug, PartialEq, Eq)] +pub enum RentState { + /// account.lamports == 0 + Uninitialized, + /// 0 < account.lamports < rent-exempt-minimum + RentPaying { + lamports: u64, // account.lamports() + data_size: usize, // account.data().len() + }, + /// account.lamports >= rent-exempt-minimum + RentExempt, +} + +/// Check rent state transition for an account in a transaction. +/// +/// This method has a default implementation that calls into +/// `check_rent_state_with_account`. +pub fn check_rent_state( + pre_rent_state: Option<&RentState>, + post_rent_state: Option<&RentState>, + transaction_context: &TransactionContext, + index: IndexOfAccount, +) -> TransactionResult<()> { + if let Some((pre_rent_state, post_rent_state)) = pre_rent_state.zip(post_rent_state) { + let expect_msg = "account must exist at TransactionContext index if rent-states are Some"; + check_rent_state_with_account( + pre_rent_state, + post_rent_state, + transaction_context + .get_key_of_account_at_index(index) + .expect(expect_msg), + &transaction_context + .accounts() + .try_borrow(index) + .expect(expect_msg), + index, + )?; + } + Ok(()) +} + +/// Check rent state transition for an account directly. +/// +/// This method has a default implementation that checks whether the +/// transition is allowed and returns an error if it is not. It also +/// verifies that the account is not the incinerator. +pub fn check_rent_state_with_account( + pre_rent_state: &RentState, + post_rent_state: &RentState, + address: &Pubkey, + _account_state: &AccountSharedData, + account_index: IndexOfAccount, +) -> TransactionResult<()> { + if !solana_sdk_ids::incinerator::check_id(address) + && !transition_allowed(pre_rent_state, post_rent_state) + { + let account_index = account_index as u8; + Err(TransactionError::InsufficientFundsForRent { account_index }) + } else { + Ok(()) + } +} + +/// Determine the rent state of an account. +/// +/// This method has a default implementation that treats accounts with zero +/// lamports as uninitialized and uses the implemented `get_rent` to +/// determine whether an account is rent-exempt. +pub fn get_account_rent_state(rent: &Rent, account: &AccountSharedData) -> RentState { + if account.lamports() == 0 { + RentState::Uninitialized + } else if rent.is_exempt(account.lamports(), account.data().len()) { + RentState::RentExempt + } else { + RentState::RentPaying { + data_size: account.data().len(), + lamports: account.lamports(), + } + } +} + +/// Check whether a transition from the pre_rent_state to the +/// post_rent_state is valid. +/// +/// This method has a default implementation that allows transitions from +/// any state to `RentState::Uninitialized` or `RentState::RentExempt`. +/// Pre-state `RentState::RentPaying` can only transition to +/// `RentState::RentPaying` if the data size remains the same and the +/// account is not credited. +pub fn transition_allowed(pre_rent_state: &RentState, post_rent_state: &RentState) -> bool { + match post_rent_state { + RentState::Uninitialized | RentState::RentExempt => true, + RentState::RentPaying { + data_size: post_data_size, + lamports: post_lamports, + } => { + match pre_rent_state { + RentState::Uninitialized | RentState::RentExempt => false, + RentState::RentPaying { + data_size: pre_data_size, + lamports: pre_lamports, + } => { + // Cannot remain RentPaying if resized or credited. + post_data_size == pre_data_size && post_lamports <= pre_lamports + } + } + } + } +} diff --git a/svm/src/transaction_account_state_info.rs b/svm/src/transaction_account_state_info.rs index 80d647019f12a0..03caffb8f7410f 100644 --- a/svm/src/transaction_account_state_info.rs +++ b/svm/src/transaction_account_state_info.rs @@ -1,7 +1,8 @@ use { + crate::rent_calculator::{check_rent_state, get_account_rent_state, RentState}, solana_account::ReadableAccount, + solana_rent::Rent, solana_sdk_ids::native_loader, - solana_svm_rent_collector::{rent_state::RentState, svm_rent_collector::SVMRentCollector}, solana_svm_transaction::svm_message::SVMMessage, solana_transaction_context::{IndexOfAccount, TransactionContext}, solana_transaction_error::TransactionResult as Result, @@ -16,7 +17,7 @@ impl TransactionAccountStateInfo { pub(crate) fn new( transaction_context: &TransactionContext, message: &impl SVMMessage, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, ) -> Vec { (0..message.account_keys().len()) .map(|i| { @@ -29,7 +30,7 @@ impl TransactionAccountStateInfo { // balances; however they will never be loaded as writable debug_assert!(!native_loader::check_id(account.owner())); - Some(rent_collector.get_account_rent_state(&account)) + Some(get_account_rent_state(rent, &account)) } else { None }; @@ -50,12 +51,11 @@ impl TransactionAccountStateInfo { pre_state_infos: &[Self], post_state_infos: &[Self], transaction_context: &TransactionContext, - rent_collector: &dyn SVMRentCollector, ) -> Result<()> { for (i, (pre_state_info, post_state_info)) in pre_state_infos.iter().zip(post_state_infos).enumerate() { - rent_collector.check_rent_state( + check_rent_state( pre_state_info.rent_state.as_ref(), post_state_info.rent_state.as_ref(), transaction_context, @@ -79,7 +79,6 @@ mod test { SanitizedMessage, }, solana_rent::Rent, - solana_rent_collector::RentCollector, solana_signer::Signer, solana_transaction_context::TransactionContext, solana_transaction_error::TransactionError, @@ -87,7 +86,7 @@ mod test { #[test] fn test_new() { - let rent_collector = RentCollector::default(); + let rent = Rent::default(); let key1 = Keypair::new(); let key2 = Keypair::new(); let key3 = Keypair::new(); @@ -122,14 +121,8 @@ mod test { (key3.pubkey(), AccountSharedData::default()), ]; - let context = TransactionContext::new( - transaction_accounts, - rent_collector.get_rent().clone(), - 20, - 20, - ); - let result = - TransactionAccountStateInfo::new(&context, &sanitized_message, &rent_collector); + let context = TransactionContext::new(transaction_accounts, rent.clone(), 20, 20); + let result = TransactionAccountStateInfo::new(&context, &sanitized_message, &rent); assert_eq!( result, vec![ @@ -147,7 +140,7 @@ mod test { #[test] #[should_panic(expected = "message and transaction context out of sync, fatal")] fn test_new_panic() { - let rent_collector = RentCollector::default(); + let rent = Rent::default(); let key1 = Keypair::new(); let key2 = Keypair::new(); let key3 = Keypair::new(); @@ -182,19 +175,12 @@ mod test { (key3.pubkey(), AccountSharedData::default()), ]; - let context = TransactionContext::new( - transaction_accounts, - rent_collector.get_rent().clone(), - 20, - 20, - ); - let _result = - TransactionAccountStateInfo::new(&context, &sanitized_message, &rent_collector); + let context = TransactionContext::new(transaction_accounts, rent.clone(), 20, 20); + let _result = TransactionAccountStateInfo::new(&context, &sanitized_message, &rent); } #[test] fn test_verify_changes() { - let rent_collector = RentCollector::default(); let key1 = Keypair::new(); let key2 = Keypair::new(); let pre_rent_state = vec![ @@ -220,7 +206,6 @@ mod test { &pre_rent_state, &post_rent_state, &context, - &rent_collector, ); assert!(result.is_ok()); @@ -244,7 +229,6 @@ mod test { &pre_rent_state, &post_rent_state, &context, - &rent_collector, ); assert_eq!( result.err(), diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 80a565e5b5eb9f..4a98648858bfbb 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -45,11 +45,10 @@ use { sysvar_cache::SysvarCache, }, solana_pubkey::Pubkey, - solana_rent_collector::RentCollector, + solana_rent::Rent, solana_sdk_ids::{native_loader, system_program}, solana_svm_callback::TransactionProcessingCallback, solana_svm_feature_set::SVMFeatureSet, - solana_svm_rent_collector::svm_rent_collector::SVMRentCollector, solana_svm_transaction::{svm_message::SVMMessage, svm_transaction::SVMTransaction}, solana_timings::{ExecuteTimingType, ExecuteTimings}, solana_transaction_context::{ExecutionRecord, TransactionContext}, @@ -122,7 +121,7 @@ pub struct TransactionProcessingConfig<'a> { /// Runtime environment for transaction batch processing. #[derive(Default)] -pub struct TransactionProcessingEnvironment<'a> { +pub struct TransactionProcessingEnvironment { /// The blockhash to use for the transaction batch. pub blockhash: Hash, /// Lamports per signature that corresponds to this blockhash. @@ -136,8 +135,8 @@ pub struct TransactionProcessingEnvironment<'a> { pub epoch_total_stake: u64, /// Runtime feature set to use for the transaction batch. pub feature_set: SVMFeatureSet, - /// Rent collector to use for the transaction batch. - pub rent_collector: Option<&'a dyn SVMRentCollector>, + /// Rent calculator to use for the transaction batch. + pub rent: Rent, } #[cfg_attr(feature = "frozen-abi", derive(AbiExample))] @@ -410,9 +409,7 @@ impl TransactionBatchProcessor { tx, tx_details, &environment.blockhash, - environment - .rent_collector - .unwrap_or(&RentCollector::default()), + &environment.rent, &mut error_metrics, ) })); @@ -424,9 +421,7 @@ impl TransactionBatchProcessor { tx, validate_result, &mut error_metrics, - environment - .rent_collector - .unwrap_or(&RentCollector::default()), + &environment.rent, )); load_us = load_us.saturating_add(single_load_us); @@ -517,7 +512,7 @@ impl TransactionBatchProcessor { message: &impl SVMMessage, checked_details: CheckedTransactionDetails, environment_blockhash: &Hash, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, error_counters: &mut TransactionErrorMetrics, ) -> TransactionResult { // If this is a nonce transaction, validate the nonce info. @@ -544,7 +539,7 @@ impl TransactionBatchProcessor { account_loader, message, checked_details, - rent_collector, + rent, error_counters, ) } @@ -556,7 +551,7 @@ impl TransactionBatchProcessor { account_loader: &mut AccountLoader, message: &impl SVMMessage, checked_details: CheckedTransactionDetails, - rent_collector: &dyn SVMRentCollector, + rent: &Rent, error_counters: &mut TransactionErrorMetrics, ) -> TransactionResult { let CheckedTransactionDetails { @@ -581,7 +576,7 @@ impl TransactionBatchProcessor { }; let fee_payer_loaded_rent_epoch = loaded_fee_payer.account.rent_epoch(); - update_rent_exempt_status_for_account(rent_collector, &mut loaded_fee_payer.account); + update_rent_exempt_status_for_account(rent, &mut loaded_fee_payer.account); let fee_payer_index = 0; validate_fee_payer( @@ -589,7 +584,7 @@ impl TransactionBatchProcessor { &mut loaded_fee_payer.account, fee_payer_index, error_counters, - rent_collector, + rent, compute_budget_and_limits.fee_details.total_fee(), )?; @@ -827,11 +822,6 @@ impl TransactionBatchProcessor { }) } - let default_rent_collector = RentCollector::default(); - let rent_collector = environment - .rent_collector - .unwrap_or(&default_rent_collector); - let lamports_before_tx = transaction_accounts_lamports_sum(&transaction_accounts).unwrap_or(0); @@ -839,7 +829,7 @@ impl TransactionBatchProcessor { let mut transaction_context = TransactionContext::new( transaction_accounts, - rent_collector.get_rent().clone(), + environment.rent.clone(), compute_budget.max_instruction_stack_depth, compute_budget.max_instruction_trace_length, ); @@ -850,7 +840,7 @@ impl TransactionBatchProcessor { ); let pre_account_state_info = - TransactionAccountStateInfo::new(&transaction_context, tx, rent_collector); + TransactionAccountStateInfo::new(&transaction_context, tx, &environment.rent); let log_collector = if config.recording_config.enable_log_recording { match config.log_messages_bytes_limit { @@ -898,12 +888,11 @@ impl TransactionBatchProcessor { let mut status = process_result .and_then(|info| { let post_account_state_info = - TransactionAccountStateInfo::new(&transaction_context, tx, rent_collector); + TransactionAccountStateInfo::new(&transaction_context, tx, &environment.rent); TransactionAccountStateInfo::verify_changes( &pre_account_state_info, &post_account_state_info, &transaction_context, - rent_collector, ) .map(|_| info) }) @@ -1110,7 +1099,7 @@ mod tests { loaded_programs::{BlockRelation, ProgramCacheEntryType}, }, solana_rent::Rent, - solana_rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, + solana_rent_collector::RENT_EXEMPT_RENT_EPOCH, solana_sdk_ids::{bpf_loader, system_program, sysvar}, solana_signature::Signature, solana_svm_callback::{AccountState, InvokeContextCallback}, @@ -2046,13 +2035,8 @@ mod tests { )); let fee_payer_address = message.fee_payer(); let current_epoch = 42; - let rent_collector = RentCollector { - epoch: current_epoch, - ..RentCollector::default() - }; - let min_balance = rent_collector - .rent - .minimum_balance(nonce::state::State::size()); + let rent = Rent::default(); + let min_balance = rent.minimum_balance(nonce::state::State::size()); let transaction_fee = lamports_per_signature; let priority_fee = 2_000_000u64; let starting_balance = transaction_fee + priority_fee; @@ -2094,7 +2078,7 @@ mod tests { &message, CheckedTransactionDetails::new(None, Ok(compute_budget_and_limits)), &Hash::default(), - &rent_collector, + &rent, &mut error_counters, ); @@ -2144,9 +2128,11 @@ mod tests { &Hash::new_unique(), )); let fee_payer_address = message.fee_payer(); - let mut rent_collector = RentCollector::default(); - rent_collector.rent.lamports_per_byte_year = 1_000_000; - let min_balance = rent_collector.rent.minimum_balance(0); + let rent = Rent { + lamports_per_byte_year: 1_000_000, + ..Default::default() + }; + let min_balance = rent.minimum_balance(0); let transaction_fee = lamports_per_signature; let starting_balance = min_balance - 1; let fee_payer_account = AccountSharedData::new(starting_balance, 0, &Pubkey::default()); @@ -2171,7 +2157,7 @@ mod tests { &message, CheckedTransactionDetails::new(None, Ok(compute_budget_and_limits)), &Hash::default(), - &rent_collector, + &rent, &mut error_counters, ); @@ -2225,7 +2211,7 @@ mod tests { Ok(SVMTransactionExecutionAndFeeBudgetLimits::default()), ), &Hash::default(), - &RentCollector::default(), + &Rent::default(), &mut error_counters, ); @@ -2264,7 +2250,7 @@ mod tests { )), ), &Hash::default(), - &RentCollector::default(), + &Rent::default(), &mut error_counters, ); @@ -2279,8 +2265,8 @@ mod tests { new_unchecked_sanitized_message(Message::new(&[], Some(&Pubkey::new_unique()))); let fee_payer_address = message.fee_payer(); let transaction_fee = lamports_per_signature; - let rent_collector = RentCollector::default(); - let min_balance = rent_collector.rent.minimum_balance(0); + let rent = Rent::default(); + let min_balance = rent.minimum_balance(0); let starting_balance = min_balance + transaction_fee - 1; let fee_payer_account = AccountSharedData::new(starting_balance, 0, &Pubkey::default()); let mut mock_accounts = HashMap::new(); @@ -2307,7 +2293,7 @@ mod tests { )), ), &Hash::default(), - &rent_collector, + &rent, &mut error_counters, ); @@ -2348,7 +2334,7 @@ mod tests { )), ), &Hash::default(), - &RentCollector::default(), + &Rent::default(), &mut error_counters, ); @@ -2375,7 +2361,7 @@ mod tests { &message, CheckedTransactionDetails::new(None, Err(DuplicateInstruction(1))), &Hash::default(), - &RentCollector::default(), + &Rent::default(), &mut error_counters, ); @@ -2387,7 +2373,7 @@ mod tests { #[test_case(true; "simd186_loaded_size")] fn test_validate_transaction_fee_payer_is_nonce(formalize_loaded_transaction_data_size: bool) { let lamports_per_signature = 5000; - let rent_collector = RentCollector::default(); + let rent = Rent::default(); let compute_unit_limit = 1000u64; let last_blockhash = Hash::new_unique(); let message = new_unchecked_sanitized_message(Message::new_with_blockhash( @@ -2451,7 +2437,7 @@ mod tests { &message, tx_details, &environment_blockhash, - &rent_collector, + &rent, &mut error_counters, ); @@ -2514,7 +2500,7 @@ mod tests { &message, CheckedTransactionDetails::new(None, Ok(compute_budget_and_limits)), &Hash::default(), - &rent_collector, + &rent, &mut error_counters, ); @@ -2560,7 +2546,7 @@ mod tests { )), ), &Hash::default(), - &RentCollector::default(), + &Rent::default(), &mut TransactionErrorMetrics::default(), ) .unwrap(); diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 1c2859874dbd1c..1ff6d21d90ffb4 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -70,7 +70,7 @@ pub struct SvmTestEnvironment<'a> { pub fork_graph: Arc>, pub batch_processor: TransactionBatchProcessor, pub processing_config: TransactionProcessingConfig<'a>, - pub processing_environment: TransactionProcessingEnvironment<'a>, + pub processing_environment: TransactionProcessingEnvironment, pub test_entry: SvmTestEntry, }