Skip to content

Commit

Permalink
restricts rent-paying accounts lifeline extension
Browse files Browse the repository at this point in the history 
solana-labs#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.

Link to discord discussion:
https://discord.com/channels/428295358100013066/943609352068145162/995202300001927219

This commit restricts rent-paying accounts lifeline extension by
preventing increasing lamports on the account if the account stays below
the rent-exempt minimum.
  • Loading branch information
@behzadnouri
behzadnouri committed Jul 14, 2022
1 parent 4dea32e commit 7a1e96e
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 31 deletions.
168 changes: 137 additions & 31 deletions runtime/src/account_rent_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -24,35 +26,53 @@ 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,
prevent_crediting_rent_paying_accounts: 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 && {
!prevent_crediting_rent_paying_accounts || 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);
}
_ => {}
Expand All @@ -65,6 +85,7 @@ pub(crate) fn check_rent_state(
transaction_context: &TransactionContext,
index: usize,
include_account_index_in_err: bool,
prevent_crediting_rent_paying_accounts: 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";
Expand All @@ -79,6 +100,7 @@ pub(crate) fn check_rent_state(
.expect(expect_msg)
.borrow(),
include_account_index_in_err.then(|| index),
prevent_crediting_rent_paying_accounts,
)?;
}
Ok(())
Expand All @@ -90,10 +112,12 @@ pub(crate) fn check_rent_state_with_account(
address: &Pubkey,
account_state: &AccountSharedData,
account_index: Option<usize>,
prevent_crediting_rent_paying_accounts: 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, prevent_crediting_rent_paying_accounts)
{
debug!(
"Account {} not rent exempt, state {:?}",
Expand Down Expand Up @@ -152,7 +176,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),
Expand All @@ -162,21 +189,100 @@ mod tests {

#[test]
fn test_transition_allowed_from() {
check_transition_allowed_from(/*prevent_crediting_rent_paying_accounts:*/ false);
check_transition_allowed_from(/*prevent_crediting_rent_paying_accounts:*/ true);
}

fn check_transition_allowed_from(prevent_crediting_rent_paying_accounts: 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,
prevent_crediting_rent_paying_accounts
));
assert!(post_rent_state.transition_allowed_from(
&RentState::RentExempt,
prevent_crediting_rent_paying_accounts
));
assert!(post_rent_state.transition_allowed_from(
&RentState::RentPaying {
data_size: 0,
lamports: 1,
},
prevent_crediting_rent_paying_accounts
));

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,
prevent_crediting_rent_paying_accounts
));
assert!(post_rent_state.transition_allowed_from(
&RentState::RentExempt,
prevent_crediting_rent_paying_accounts
));
assert!(post_rent_state.transition_allowed_from(
&RentState::RentPaying {
data_size: 0,
lamports: 1,
},
prevent_crediting_rent_paying_accounts
));
let post_rent_state = RentState::RentPaying {
data_size: 2,
lamports: 5,
};
assert!(!post_rent_state.transition_allowed_from(
&RentState::Uninitialized,
prevent_crediting_rent_paying_accounts
));
assert!(!post_rent_state.transition_allowed_from(
&RentState::RentExempt,
prevent_crediting_rent_paying_accounts
));
assert!(!post_rent_state.transition_allowed_from(
&RentState::RentPaying {
data_size: 3,
lamports: 5
},
prevent_crediting_rent_paying_accounts
));
assert!(!post_rent_state.transition_allowed_from(
&RentState::RentPaying {
data_size: 1,
lamports: 5
},
prevent_crediting_rent_paying_accounts
));
// Transition is always allowed if there is no account data resize or
// change in account's lamports.
assert!(post_rent_state.transition_allowed_from(
&RentState::RentPaying {
data_size: 2,
lamports: 5
},
prevent_crediting_rent_paying_accounts
));
// Transition is always allowed if there is no account data resize and
// account's lamports is reduced because that will shorten rent paying
// account lifeline.
assert!(post_rent_state.transition_allowed_from(
&RentState::RentPaying {
data_size: 2,
lamports: 7
},
prevent_crediting_rent_paying_accounts
));
// Once the feature is activated, transition is not allowed if the
// account is credited with more lamports.
assert_eq!(
post_rent_state.transition_allowed_from(
&RentState::RentPaying {
data_size: 2,
lamports: 3
},
prevent_crediting_rent_paying_accounts
),
!prevent_crediting_rent_paying_accounts
);
}
}
1 change: 1 addition & 0 deletions runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::prevent_crediting_rent_paying_accounts::id()),
)
}

Expand Down
4 changes: 4 additions & 0 deletions runtime/src/bank/transaction_account_state_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 prevent_crediting_rent_paying_accounts = self
.feature_set
.is_active(&feature_set::prevent_crediting_rent_paying_accounts::id());
for (i, (pre_state_info, post_state_info)) in
pre_state_infos.iter().zip(post_state_infos).enumerate()
{
Expand All @@ -67,6 +70,7 @@ impl Bank {
transaction_context,
i,
include_account_index_in_err,
prevent_crediting_rent_paying_accounts,
)?;
}
Ok(())
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,10 @@ pub mod enable_bpf_loader_extend_program_data_ix {
solana_sdk::declare_id!("8Zs9W7D9MpSEtUWSQdGniZk2cNmV22y6FLJwCx53asme");
}

pub mod prevent_crediting_rent_paying_accounts {
solana_sdk::declare_id!("812kqX67odAp5NFwM8D2N24cku7WTm9CHUTFUXaDkWPn");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -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"),
(prevent_crediting_rent_paying_accounts::id(), "prevent crediting rent paying accounts #26606"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down

0 comments on commit 7a1e96e

Please sign in to comment.