From e4327394b41ebb8f93682774470821e4f89e20e2 Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Wed, 21 Aug 2024 14:31:30 -0700 Subject: [PATCH] Runtime: add `RentCollectorWithMetrics` wrapper type for SVM --- Cargo.lock | 1 + runtime/Cargo.toml | 1 + runtime/src/bank.rs | 5 ++ runtime/src/lib.rs | 1 + runtime/src/rent_collector.rs | 90 +++++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+) create mode 100644 runtime/src/rent_collector.rs diff --git a/Cargo.lock b/Cargo.lock index d244a91d6ac653..1d32c0b6e168fa 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/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 f54e5f46793c64..cc94ed58a84d73 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, @@ -3470,6 +3471,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); + } + _ => {} + } +}