From b04765f8b582a3156bf009307708e3c9701cea8d Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 24 Jan 2024 17:35:56 -0800 Subject: [PATCH] Code cleanup in account_rent_state (#34941) --- runtime/src/accounts/account_rent_state.rs | 109 +++++++++--------- runtime/src/accounts/mod.rs | 7 +- .../bank/transaction_account_state_info.rs | 7 +- 3 files changed, 59 insertions(+), 64 deletions(-) diff --git a/runtime/src/accounts/account_rent_state.rs b/runtime/src/accounts/account_rent_state.rs index 0949e21acfd7d5..3fc71ac6a27686 100644 --- a/runtime/src/accounts/account_rent_state.rs +++ b/runtime/src/accounts/account_rent_state.rs @@ -56,66 +56,67 @@ impl RentState { } } } -} -pub(super) 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); + fn submit_rent_state_metrics(pre_rent_state: &Self, post_rent_state: &Self) { + 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); + } + _ => {} } - _ => {} } -} -pub(crate) fn check_rent_state( - pre_rent_state: Option<&RentState>, - post_rent_state: Option<&RentState>, - transaction_context: &TransactionContext, - index: IndexOfAccount, -) -> Result<()> { - 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 - .get_account_at_index(index) - .expect(expect_msg) - .borrow(), - index, - )?; + pub(crate) fn check_rent_state( + pre_rent_state: Option<&Self>, + post_rent_state: Option<&Self>, + transaction_context: &TransactionContext, + index: IndexOfAccount, + ) -> Result<()> { + 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 + .get_account_at_index(index) + .expect(expect_msg) + .borrow(), + index, + )?; + } + Ok(()) } - Ok(()) -} -pub(super) fn check_rent_state_with_account( - 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) - && !post_rent_state.transition_allowed_from(pre_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(()) + pub(super) fn check_rent_state_with_account( + pre_rent_state: &Self, + post_rent_state: &Self, + address: &Pubkey, + account_state: &AccountSharedData, + account_index: IndexOfAccount, + ) -> Result<()> { + Self::submit_rent_state_metrics(pre_rent_state, post_rent_state); + if !solana_sdk::incinerator::check_id(address) + && !post_rent_state.transition_allowed_from(pre_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(()) + } } } diff --git a/runtime/src/accounts/mod.rs b/runtime/src/accounts/mod.rs index 28b1be02283448..4bf09d94ffb7a1 100644 --- a/runtime/src/accounts/mod.rs +++ b/runtime/src/accounts/mod.rs @@ -1,10 +1,7 @@ pub mod account_rent_state; use { - crate::{ - accounts::account_rent_state::{check_rent_state_with_account, RentState}, - bank::RewardInterval, - }, + crate::{accounts::account_rent_state::RentState, bank::RewardInterval}, itertools::Itertools, log::warn, solana_accounts_db::{ @@ -476,7 +473,7 @@ pub fn validate_fee_payer( .map_err(|_| TransactionError::InsufficientFundsForFee)?; let payer_post_rent_state = RentState::from_account(payer_account, &rent_collector.rent); - check_rent_state_with_account( + RentState::check_rent_state_with_account( &payer_pre_rent_state, &payer_post_rent_state, payer_address, diff --git a/runtime/src/bank/transaction_account_state_info.rs b/runtime/src/bank/transaction_account_state_info.rs index 4e5f58d85fffc8..c09127a6f32bb3 100644 --- a/runtime/src/bank/transaction_account_state_info.rs +++ b/runtime/src/bank/transaction_account_state_info.rs @@ -1,8 +1,5 @@ use { - crate::{ - accounts::account_rent_state::{check_rent_state, RentState}, - bank::Bank, - }, + crate::{accounts::account_rent_state::RentState, bank::Bank}, solana_sdk::{ account::ReadableAccount, message::SanitizedMessage, @@ -63,7 +60,7 @@ impl Bank { for (i, (pre_state_info, post_state_info)) in pre_state_infos.iter().zip(post_state_infos).enumerate() { - check_rent_state( + RentState::check_rent_state( pre_state_info.rent_state.as_ref(), post_state_info.rent_state.as_ref(), transaction_context,