From 595d6464ec023f1f6255be074815f5374ae6d4f5 Mon Sep 17 00:00:00 2001 From: Joe C Date: Tue, 27 Aug 2024 06:03:30 -0700 Subject: [PATCH] Runtime: add `RentCollectorWithMetrics` wrapper type for SVM (#2726) --- Cargo.lock | 1 + programs/sbf/Cargo.lock | 8 ++++ runtime/Cargo.toml | 1 + runtime/src/bank.rs | 5 ++ runtime/src/lib.rs | 1 + runtime/src/rent_collector.rs | 90 +++++++++++++++++++++++++++++++++++ 6 files changed, 106 insertions(+) create mode 100644 runtime/src/rent_collector.rs diff --git a/Cargo.lock b/Cargo.lock index 79484babb78665..8a32f9e599712e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7378,6 +7378,7 @@ dependencies = [ "solana-sdk", "solana-stake-program", "solana-svm", + "solana-svm-rent-collector", "solana-svm-transaction", "solana-system-program", "solana-timings", diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 9fc1e187c9a7ef..b3c0ee479b921d 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5719,6 +5719,7 @@ dependencies = [ "solana-sdk", "solana-stake-program", "solana-svm", + "solana-svm-rent-collector", "solana-svm-transaction", "solana-system-program", "solana-timings", @@ -6455,6 +6456,13 @@ dependencies = [ "thiserror", ] +[[package]] +name = "solana-svm-rent-collector" +version = "2.1.0" +dependencies = [ + "solana-sdk", +] + [[package]] name = "solana-svm-transaction" version = "2.1.0" diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 44d291725b7419..3e3573f0eeb9d8 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -68,6 +68,7 @@ solana-runtime-transaction = { workspace = true } solana-sdk = { workspace = true } solana-stake-program = { workspace = true } solana-svm = { 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/runtime/src/bank.rs b/runtime/src/bank.rs index 1a2887e3f9c3c9..de2cb370abfe8e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -43,6 +43,7 @@ use { bank_forks::BankForks, epoch_stakes::{split_epoch_stakes, EpochStakes, NodeVoteAccounts, VersionedEpochStakes}, installed_scheduler_pool::{BankWithScheduler, InstalledSchedulerRwLock}, + rent_collector::RentCollectorWithMetrics, runtime_config::RuntimeConfig, serde_snapshot::BankIncrementalSnapshotPersistence, snapshot_hash::SnapshotHash, @@ -3476,6 +3477,10 @@ 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 = + RentCollectorWithMetrics::new(self.rent_collector.clone()); let processing_environment = TransactionProcessingEnvironment { blockhash, epoch_total_stake: self.epoch_total_stake(self.epoch()), diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 8e9f4b4c82d931..c3772735b9ef42 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -17,6 +17,7 @@ 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 serde_snapshot; pub mod snapshot_archive_info; diff --git a/runtime/src/rent_collector.rs b/runtime/src/rent_collector.rs new file mode 100644 index 00000000000000..683bd91cd6cb30 --- /dev/null +++ b/runtime/src/rent_collector.rs @@ -0,0 +1,90 @@ +//! 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_sdk::{ + account::AccountSharedData, + clock::Epoch, + pubkey::Pubkey, + rent::{Rent, RentDue}, + rent_collector::{CollectedInfo, RentCollector}, + transaction::{Result, TransactionError}, + transaction_context::IndexOfAccount, + }, + solana_svm_rent_collector::{rent_state::RentState, svm_rent_collector::SVMRentCollector}, +}; + +/// 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 collect_rent(&self, address: &Pubkey, account: &mut AccountSharedData) -> CollectedInfo { + self.0.collect_rent(address, account) + } + + 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::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); + } + _ => {} + } +}