From 84ce1dbdbb55206b03688325070c6b69e25c5b49 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Wed, 13 Jul 2022 10:08:05 -0400 Subject: [PATCH] restricts rent-paying accounts TTL extension https://github.com/solana-labs/solana/pull/22292 prevents rent paying accounts creation going forward. However a rent paying account can linger on for ever if it is continually topped up but stays below the rent-exempt minimum. This can prevent eliminating accounts-rewrites and the problematic rent_epoch field in accounts. This commit restricts rent-paying accounts TTL extension by preventing increasing lamports on the account if the account stays below the rent-exempt minimum. --- runtime/src/account_rent_state.rs | 167 ++++++++++++++---- runtime/src/accounts.rs | 1 + .../bank/transaction_account_state_info.rs | 4 + sdk/src/feature_set.rs | 5 + 4 files changed, 146 insertions(+), 31 deletions(-) diff --git a/runtime/src/account_rent_state.rs b/runtime/src/account_rent_state.rs index 45c81b040cd36c..2c5191c933fb0f 100644 --- a/runtime/src/account_rent_state.rs +++ b/runtime/src/account_rent_state.rs @@ -14,8 +14,10 @@ pub(crate) enum RentState { /// account.lamports == 0 Uninitialized, /// 0 < account.lamports < rent-exempt-minimum - /// Parameter is the size of the account data - RentPaying(usize), + RentPaying { + lamports: u64, // account.lamports() + data_size: usize, // account.data().len() + }, /// account.lamports >= rent-exempt-minimum RentExempt, } @@ -24,35 +26,54 @@ impl RentState { pub(crate) fn from_account(account: &AccountSharedData, rent: &Rent) -> Self { if account.lamports() == 0 { Self::Uninitialized - } else if !rent.is_exempt(account.lamports(), account.data().len()) { - Self::RentPaying(account.data().len()) - } else { + } else if rent.is_exempt(account.lamports(), account.data().len()) { Self::RentExempt + } else { + Self::RentPaying { + data_size: account.data().len(), + lamports: account.lamports(), + } } } - pub(crate) fn transition_allowed_from(&self, pre_rent_state: &RentState) -> bool { - if let Self::RentPaying(post_data_size) = self { - if let Self::RentPaying(pre_data_size) = pre_rent_state { - post_data_size == pre_data_size // Cannot be RentPaying if resized - } else { - false // Only RentPaying can continue to be RentPaying + pub(crate) fn transition_allowed_from( + &self, + pre_rent_state: &RentState, + restrict_rent_paying_accounts_ttl_extension: bool, + ) -> bool { + match self { + Self::Uninitialized | Self::RentExempt => true, + Self::RentPaying { + data_size: post_data_size, + lamports: post_lamports, + } => { + match pre_rent_state { + Self::Uninitialized | Self::RentExempt => false, + Self::RentPaying { + data_size: pre_data_size, + lamports: pre_lamports, + } => { + // Cannot be RentPaying if resized + post_data_size == pre_data_size && { + !restrict_rent_paying_accounts_ttl_extension + || post_lamports <= pre_lamports + } + } + } } - } else { - true // Post not-RentPaying always ok } } } pub(crate) fn submit_rent_state_metrics(pre_rent_state: &RentState, post_rent_state: &RentState) { match (pre_rent_state, post_rent_state) { - (&RentState::Uninitialized, &RentState::RentPaying(_)) => { + (&RentState::Uninitialized, &RentState::RentPaying { .. }) => { inc_new_counter_info!("rent_paying_err-new_account", 1); } - (&RentState::RentPaying(_), &RentState::RentPaying(_)) => { + (&RentState::RentPaying { .. }, &RentState::RentPaying { .. }) => { inc_new_counter_info!("rent_paying_ok-legacy", 1); } - (_, &RentState::RentPaying(_)) => { + (_, &RentState::RentPaying { .. }) => { inc_new_counter_info!("rent_paying_err-other", 1); } _ => {} @@ -65,6 +86,7 @@ pub(crate) fn check_rent_state( transaction_context: &TransactionContext, index: usize, include_account_index_in_err: bool, + restrict_rent_paying_accounts_ttl_extension: bool, ) -> 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"; @@ -79,6 +101,7 @@ pub(crate) fn check_rent_state( .expect(expect_msg) .borrow(), include_account_index_in_err.then(|| index), + restrict_rent_paying_accounts_ttl_extension, )?; } Ok(()) @@ -90,10 +113,12 @@ pub(crate) fn check_rent_state_with_account( address: &Pubkey, account_state: &AccountSharedData, account_index: Option, + restrict_rent_paying_accounts_ttl_extension: bool, ) -> 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) + && !post_rent_state + .transition_allowed_from(pre_rent_state, restrict_rent_paying_accounts_ttl_extension) { debug!( "Account {} not rent exempt, state {:?}", @@ -152,7 +177,10 @@ mod tests { ); assert_eq!( RentState::from_account(&rent_paying_account, &rent), - RentState::RentPaying(account_data_size) + RentState::RentPaying { + data_size: account_data_size, + lamports: rent_paying_account.lamports(), + } ); assert_eq!( RentState::from_account(&rent_exempt_account, &rent), @@ -162,21 +190,98 @@ mod tests { #[test] fn test_transition_allowed_from() { + check_transition_allowed_from(/*restrict_rent_paying_accounts_ttl_extension:*/ false); + check_transition_allowed_from(/*restrict_rent_paying_accounts_ttl_extension:*/ true); + } + + fn check_transition_allowed_from(restrict_rent_paying_accounts_ttl_extension: bool) { let post_rent_state = RentState::Uninitialized; - assert!(post_rent_state.transition_allowed_from(&RentState::Uninitialized)); - assert!(post_rent_state.transition_allowed_from(&RentState::RentExempt)); - assert!(post_rent_state.transition_allowed_from(&RentState::RentPaying(0))); + assert!(post_rent_state.transition_allowed_from( + &RentState::Uninitialized, + restrict_rent_paying_accounts_ttl_extension + )); + assert!(post_rent_state.transition_allowed_from( + &RentState::RentExempt, + restrict_rent_paying_accounts_ttl_extension + )); + assert!(post_rent_state.transition_allowed_from( + &RentState::RentPaying { + data_size: 0, + lamports: 1, + }, + restrict_rent_paying_accounts_ttl_extension + )); let post_rent_state = RentState::RentExempt; - assert!(post_rent_state.transition_allowed_from(&RentState::Uninitialized)); - assert!(post_rent_state.transition_allowed_from(&RentState::RentExempt)); - assert!(post_rent_state.transition_allowed_from(&RentState::RentPaying(0))); - - let post_rent_state = RentState::RentPaying(2); - assert!(!post_rent_state.transition_allowed_from(&RentState::Uninitialized)); - assert!(!post_rent_state.transition_allowed_from(&RentState::RentExempt)); - assert!(!post_rent_state.transition_allowed_from(&RentState::RentPaying(3))); - assert!(!post_rent_state.transition_allowed_from(&RentState::RentPaying(1))); - assert!(post_rent_state.transition_allowed_from(&RentState::RentPaying(2))); + assert!(post_rent_state.transition_allowed_from( + &RentState::Uninitialized, + restrict_rent_paying_accounts_ttl_extension + )); + assert!(post_rent_state.transition_allowed_from( + &RentState::RentExempt, + restrict_rent_paying_accounts_ttl_extension + )); + assert!(post_rent_state.transition_allowed_from( + &RentState::RentPaying { + data_size: 0, + lamports: 1, + }, + restrict_rent_paying_accounts_ttl_extension + )); + let post_rent_state = RentState::RentPaying { + data_size: 2, + lamports: 5, + }; + assert!(!post_rent_state.transition_allowed_from( + &RentState::Uninitialized, + restrict_rent_paying_accounts_ttl_extension + )); + assert!(!post_rent_state.transition_allowed_from( + &RentState::RentExempt, + restrict_rent_paying_accounts_ttl_extension + )); + assert!(!post_rent_state.transition_allowed_from( + &RentState::RentPaying { + data_size: 3, + lamports: 5 + }, + restrict_rent_paying_accounts_ttl_extension + )); + assert!(!post_rent_state.transition_allowed_from( + &RentState::RentPaying { + data_size: 1, + lamports: 5 + }, + restrict_rent_paying_accounts_ttl_extension + )); + // No resize, same lamports is always allowed. + assert!(post_rent_state.transition_allowed_from( + &RentState::RentPaying { + data_size: 2, + lamports: 5 + }, + restrict_rent_paying_accounts_ttl_extension + )); + // No resize, fewer lamports is always allowed becasue that shortens + // rent paying account TTL. + assert!(post_rent_state.transition_allowed_from( + &RentState::RentPaying { + data_size: 2, + lamports: 7 + }, + restrict_rent_paying_accounts_ttl_extension + )); + // No resize, more lamports is not allowed once the feature is + // activated. + assert_eq!( + post_rent_state.transition_allowed_from( + &RentState::RentPaying { + data_size: 2, + lamports: 3 + }, + restrict_rent_paying_accounts_ttl_extension + ), + !restrict_rent_paying_accounts_ttl_extension + ); } } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 747c57728708a5..21ab39cee76d2e 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -441,6 +441,7 @@ impl Accounts { feature_set .is_active(&feature_set::include_account_index_in_rent_error::ID) .then(|| payer_index), + feature_set.is_active(&feature_set::restrict_rent_paying_accounts_ttl_extension::id()), ) } diff --git a/runtime/src/bank/transaction_account_state_info.rs b/runtime/src/bank/transaction_account_state_info.rs index 824f231236842d..ec15b8a54c6f62 100644 --- a/runtime/src/bank/transaction_account_state_info.rs +++ b/runtime/src/bank/transaction_account_state_info.rs @@ -58,6 +58,9 @@ impl Bank { let include_account_index_in_err = self .feature_set .is_active(&feature_set::include_account_index_in_rent_error::id()); + let restrict_rent_paying_accounts_ttl_extension = self + .feature_set + .is_active(&feature_set::restrict_rent_paying_accounts_ttl_extension::id()); for (i, (pre_state_info, post_state_info)) in pre_state_infos.iter().zip(post_state_infos).enumerate() { @@ -67,6 +70,7 @@ impl Bank { transaction_context, i, include_account_index_in_err, + restrict_rent_paying_accounts_ttl_extension, )?; } Ok(()) diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index cdbe902bcf00ce..e33467639747cd 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -456,6 +456,10 @@ pub mod enable_bpf_loader_extend_program_data_ix { solana_sdk::declare_id!("8Zs9W7D9MpSEtUWSQdGniZk2cNmV22y6FLJwCx53asme"); } +pub mod restrict_rent_paying_accounts_ttl_extension { + solana_sdk::declare_id!("812kqX67odAp5NFwM8D2N24cku7WTm9CHUTFUXaDkWPn"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -564,6 +568,7 @@ lazy_static! { (cap_accounts_data_size_per_block::id(), "cap the accounts data size per block #25517"), (preserve_rent_epoch_for_rent_exempt_accounts::id(), "preserve rent epoch for rent exempt accounts #26479"), (enable_bpf_loader_extend_program_data_ix::id(), "enable bpf upgradeable loader ExtendProgramData instruction #25234"), + (restrict_rent_paying_accounts_ttl_extension::id(), "restrict rent paying accounts TTL extension #"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()