preserves rent_epoch for rent exempt accounts (#26479)

https://github.com/solana-labs/solana/pull/22292
prevents rent paying account creation going forward. As a result
rent_epoch field for rent exempt accounts is redundant, and advancing
this field will incur expensive account rewrites and cause discrepancy
between accounts-db and cached vote/stake accounts.

This commit adds a feature which upon activation preserves rent_epoch
field for rent exempt accounts so that the field is frozen and is no
longer advanced.
This commit is contained in:
behzad nouri 2022-07-08 20:04:08 +00:00 committed by GitHub
parent f3bba9723e
commit c99d9f00a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 120 additions and 36 deletions

View File

@ -265,6 +265,8 @@ impl Accounts {
let mut accounts = Vec::with_capacity(account_keys.len());
let mut account_deps = Vec::with_capacity(account_keys.len());
let mut rent_debits = RentDebits::default();
let preserve_rent_epoch_for_rent_exempt_accounts = feature_set
.is_active(&feature_set::preserve_rent_epoch_for_rent_exempt_accounts::id());
for (i, key) in account_keys.iter().enumerate() {
let account = if !message.is_non_loader_key(i) {
// Fill in an empty account for the program slots.
@ -292,6 +294,7 @@ impl Accounts {
key,
&mut account,
self.accounts_db.filler_account_suffix.as_ref(),
preserve_rent_epoch_for_rent_exempt_accounts,
)
.rent_amount;
(account, rent_due)
@ -1182,15 +1185,16 @@ impl Accounts {
/// Store the accounts into the DB
// allow(clippy) needed for various gating flags
#[allow(clippy::too_many_arguments)]
pub fn store_cached<'a>(
pub(crate) fn store_cached(
&self,
slot: Slot,
txs: &'a [SanitizedTransaction],
res: &'a [TransactionExecutionResult],
loaded: &'a mut [TransactionLoadResult],
txs: &[SanitizedTransaction],
res: &[TransactionExecutionResult],
loaded: &mut [TransactionLoadResult],
rent_collector: &RentCollector,
durable_nonce: &DurableNonce,
lamports_per_signature: u64,
preserve_rent_epoch_for_rent_exempt_accounts: bool,
) {
let (accounts_to_store, txn_signatures) = self.collect_accounts_to_store(
txs,
@ -1199,6 +1203,7 @@ impl Accounts {
rent_collector,
durable_nonce,
lamports_per_signature,
preserve_rent_epoch_for_rent_exempt_accounts,
);
self.accounts_db
.store_cached((slot, &accounts_to_store[..]), Some(&txn_signatures));
@ -1225,6 +1230,7 @@ impl Accounts {
rent_collector: &RentCollector,
durable_nonce: &DurableNonce,
lamports_per_signature: u64,
preserve_rent_epoch_for_rent_exempt_accounts: bool,
) -> (
Vec<(&'a Pubkey, &'a AccountSharedData)>,
Vec<Option<&'a Signature>>,
@ -1280,7 +1286,11 @@ impl Accounts {
if execution_status.is_ok() || is_nonce_account || is_fee_payer {
if account.rent_epoch() == INITIAL_RENT_EPOCH {
let rent = rent_collector
.collect_from_created_account(address, account)
.collect_from_created_account(
address,
account,
preserve_rent_epoch_for_rent_exempt_accounts,
)
.rent_amount;
loaded_transaction.rent += rent;
loaded_transaction.rent_debits.insert(
@ -2995,6 +3005,7 @@ mod tests {
&rent_collector,
&DurableNonce::default(),
0,
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(collected_accounts.len(), 2);
assert!(collected_accounts
@ -3478,6 +3489,7 @@ mod tests {
&rent_collector,
&durable_nonce,
0,
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(collected_accounts.len(), 2);
assert_eq!(
@ -3592,6 +3604,7 @@ mod tests {
&rent_collector,
&durable_nonce,
0,
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(collected_accounts.len(), 1);
let collected_nonce_account = collected_accounts

View File

@ -4929,6 +4929,7 @@ impl Bank {
&self.rent_collector,
&durable_nonce,
lamports_per_signature,
self.preserve_rent_epoch_for_rent_exempt_accounts(),
);
let rent_debits = self.collect_rent(&execution_results, loaded_txs);
@ -5336,6 +5337,7 @@ impl Bank {
pubkey,
account,
self.rc.accounts.accounts_db.filler_account_suffix.as_ref(),
self.preserve_rent_epoch_for_rent_exempt_accounts(),
));
time_collecting_rent_us += measure.as_us();
@ -7274,6 +7276,11 @@ impl Bank {
.is_active(&feature_set::send_to_tpu_vote_port::id())
}
fn preserve_rent_epoch_for_rent_exempt_accounts(&self) -> bool {
self.feature_set
.is_active(&feature_set::preserve_rent_epoch_for_rent_exempt_accounts::id())
}
pub fn read_cost_tracker(&self) -> LockResult<RwLockReadGuard<CostTracker>> {
self.cost_tracker.read()
}
@ -8202,6 +8209,7 @@ pub(crate) mod tests {
&keypairs[4].pubkey(),
&mut account_copy,
None,
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(expected_rent.rent_amount, too_few_lamports);
assert_eq!(account_copy.lamports(), 0);
@ -9720,21 +9728,19 @@ pub(crate) mod tests {
bank.collect_rent_in_partition((0, 0, 1), true, &RentMetrics::default());
{
let rewrites_skipped = bank.rewrites_skipped_this_slot.read().unwrap();
// `rewrites_skipped.len()` is the number of non-rent paying accounts in the slot. This
// is always at least the number of features in the Bank, due to
// `activate_all_features`. These accounts will stop being written to the append vec
// when we start skipping rewrites.
// `rewrites_skipped.len()` is the number of non-rent paying accounts in the slot.
// 'collect_rent_in_partition' fills 'rewrites_skipped_this_slot' with rewrites that
// were skipped during rent collection but should still be considered in the slot's
// bank hash. If the slot is also written in the append vec, then the bank hash calc
// code ignores the contents of this list. This assert is confirming that the expected #
// of accounts were included in 'rewrites_skipped' by the call to
// 'collect_rent_in_partition(..., true)' above.
let num_features = bank.feature_set.inactive.len() + bank.feature_set.active.len();
assert!(rewrites_skipped.len() >= num_features);
// should have skipped 'rent_exempt_pubkey'
assert!(rewrites_skipped.contains_key(&rent_exempt_pubkey));
// should NOT have skipped 'rent_exempt_pubkey'
assert_eq!(rewrites_skipped.len(), 1);
// should not have skipped 'rent_exempt_pubkey'
// Once preserve_rent_epoch_for_rent_exempt_accounts is activated,
// rewrite-skip is irrelevant to rent-exempt accounts.
assert!(!rewrites_skipped.contains_key(&rent_exempt_pubkey));
// should NOT have skipped 'rent_due_pubkey'
assert!(!rewrites_skipped.contains_key(&rent_due_pubkey));
}
@ -9754,9 +9760,11 @@ pub(crate) mod tests {
bank.get_account(&rent_exempt_pubkey).unwrap().lamports(),
large_lamports
);
// Once preserve_rent_epoch_for_rent_exempt_accounts is activated,
// rent_epoch of rent-exempt accounts will no longer advance.
assert_eq!(
bank.get_account(&rent_exempt_pubkey).unwrap().rent_epoch(),
current_epoch
0
);
assert_eq!(
bank.slots_by_pubkey(&rent_due_pubkey, &ancestors),
@ -19213,6 +19221,7 @@ pub(crate) mod tests {
&keypair.pubkey(),
&mut account,
None,
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(info.account_data_len_reclaimed, data_size as u64);
}

View File

@ -302,7 +302,12 @@ impl ExpectedRentCollection {
pubkey: &Pubkey,
rewrites_skipped_this_slot: &Rewrites,
) -> Option<Epoch> {
let next_epoch = match rent_collector.calculate_rent_result(pubkey, account, None) {
let next_epoch = match rent_collector.calculate_rent_result(
pubkey, account, None, // filler_account_suffix
// Skipping rewrites is not compatible with the below feature.
// We will not skip rewrites until the feature is activated.
false, // preserve_rent_epoch_for_rent_exempt_accounts
) {
RentResult::LeaveAloneNoRent => return None,
RentResult::CollectRent {
new_rent_epoch,
@ -532,8 +537,14 @@ impl ExpectedRentCollection {
// ask the rent collector what rent should be collected.
// Rent collector knows the current epoch.
let rent_result =
rent_collector.calculate_rent_result(pubkey, loaded_account, filler_account_suffix);
let rent_result = rent_collector.calculate_rent_result(
pubkey,
loaded_account,
filler_account_suffix,
// Skipping rewrites is not compatible with the below feature.
// We will not skip rewrites until the feature is activated.
false, // preserve_rent_epoch_for_rent_exempt_accounts
);
let current_rent_epoch = loaded_account.rent_epoch();
let new_rent_epoch = match rent_result {
RentResult::CollectRent {

View File

@ -67,7 +67,11 @@ impl RentCollector {
}
/// true if it is easy to determine this account should consider having rent collected from it
pub fn should_collect_rent(&self, address: &Pubkey, account: &impl ReadableAccount) -> bool {
pub(crate) fn should_collect_rent(
&self,
address: &Pubkey,
account: &impl ReadableAccount,
) -> bool {
!(account.executable() // executable accounts must be rent-exempt balance
|| *address == incinerator::id())
}
@ -121,8 +125,14 @@ impl RentCollector {
address: &Pubkey,
account: &mut AccountSharedData,
filler_account_suffix: Option<&Pubkey>,
preserve_rent_epoch_for_rent_exempt_accounts: bool,
) -> CollectedInfo {
match self.calculate_rent_result(address, account, filler_account_suffix) {
match self.calculate_rent_result(
address,
account,
filler_account_suffix,
preserve_rent_epoch_for_rent_exempt_accounts,
) {
RentResult::LeaveAloneNoRent => CollectedInfo::default(),
RentResult::CollectRent {
new_rent_epoch,
@ -154,6 +164,7 @@ impl RentCollector {
address: &Pubkey,
account: &impl ReadableAccount,
filler_account_suffix: Option<&Pubkey>,
preserve_rent_epoch_for_rent_exempt_accounts: bool,
) -> RentResult {
if self.can_skip_rent_collection(address, account, filler_account_suffix) {
return RentResult::LeaveAloneNoRent;
@ -161,10 +172,16 @@ impl RentCollector {
match self.get_rent_due(account) {
// Rent isn't collected for the next epoch.
// Make sure to check exempt status again later in current epoch.
RentDue::Exempt => RentResult::CollectRent {
new_rent_epoch: self.epoch,
rent_due: 0,
},
RentDue::Exempt => {
if preserve_rent_epoch_for_rent_exempt_accounts {
RentResult::LeaveAloneNoRent
} else {
RentResult::CollectRent {
new_rent_epoch: self.epoch,
rent_due: 0,
}
}
}
// Maybe collect rent later, leave account alone.
RentDue::Paying(0) => RentResult::LeaveAloneNoRent,
// Rent is collected for next epoch.
@ -180,10 +197,16 @@ impl RentCollector {
&self,
address: &Pubkey,
account: &mut AccountSharedData,
preserve_rent_epoch_for_rent_exempt_accounts: bool,
) -> CollectedInfo {
// initialize rent_epoch as created at this epoch
account.set_rent_epoch(self.epoch);
self.collect_from_existing_account(address, account, None)
self.collect_from_existing_account(
address,
account,
None, // filler_account_suffix
preserve_rent_epoch_for_rent_exempt_accounts,
)
}
/// Performs easy checks to see if rent collection can be skipped
@ -204,11 +227,11 @@ impl RentCollector {
/// Information computed during rent collection
#[derive(Debug, Default, Copy, Clone, Eq, PartialEq)]
pub struct CollectedInfo {
pub(crate) struct CollectedInfo {
/// Amount of rent collected from account
pub rent_amount: u64,
pub(crate) rent_amount: u64,
/// Size of data reclaimed from account (happens when account's lamports go to zero)
pub account_data_len_reclaimed: u64,
pub(crate) account_data_len_reclaimed: u64,
}
impl std::ops::Add for CollectedInfo {
@ -258,8 +281,11 @@ mod tests {
let rent_collector = default_rent_collector_clone_with_epoch(new_epoch);
// collect rent on a newly-created account
let collected = rent_collector
.collect_from_created_account(&solana_sdk::pubkey::new_rand(), &mut created_account);
let collected = rent_collector.collect_from_created_account(
&solana_sdk::pubkey::new_rand(),
&mut created_account,
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert!(created_account.lamports() < old_lamports);
assert_eq!(
created_account.lamports() + collected.rent_amount,
@ -272,7 +298,8 @@ mod tests {
let collected = rent_collector.collect_from_existing_account(
&solana_sdk::pubkey::new_rand(),
&mut existing_account,
None,
None, // filler_account_suffix
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert!(existing_account.lamports() < old_lamports);
assert_eq!(
@ -302,7 +329,12 @@ mod tests {
let rent_collector = default_rent_collector_clone_with_epoch(epoch);
// first mark account as being collected while being rent-exempt
let collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, None);
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
None, // filler_account_suffix
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(account.lamports(), huge_lamports);
assert_eq!(collected, CollectedInfo::default());
@ -310,7 +342,12 @@ mod tests {
account.set_lamports(tiny_lamports);
// ... and trigger another rent collection on the same epoch and check that rent is working
let collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, None);
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
None, // filler_account_suffix
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(account.lamports(), tiny_lamports - collected.rent_amount);
assert_ne!(collected, CollectedInfo::default());
}
@ -329,7 +366,12 @@ mod tests {
let epoch = 3;
let rent_collector = default_rent_collector_clone_with_epoch(epoch);
let collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, None);
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
None, // filler_account_suffix
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(account.lamports(), 0);
assert_eq!(collected.rent_amount, 1);
}
@ -349,8 +391,12 @@ mod tests {
});
let rent_collector = default_rent_collector_clone_with_epoch(account_rent_epoch + 1);
let collected =
rent_collector.collect_from_existing_account(&Pubkey::new_unique(), &mut account, None);
let collected = rent_collector.collect_from_existing_account(
&Pubkey::new_unique(),
&mut account,
None, // filler_account_suffix
true, // preserve_rent_epoch_for_rent_exempt_accounts
);
assert_eq!(collected.rent_amount, account_lamports);
assert_eq!(

View File

@ -448,6 +448,10 @@ pub mod cap_accounts_data_size_per_block {
solana_sdk::declare_id!("qywiJyZmqTKspFg2LeuUHqcA5nNvBgobqb9UprywS9N");
}
pub mod preserve_rent_epoch_for_rent_exempt_accounts {
solana_sdk::declare_id!("HH3MUYReL2BvqqA3oEcAa7txju5GY6G4nxJ51zvsEjEZ");
}
lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -554,6 +558,7 @@ lazy_static! {
(nonce_must_be_advanceable::id(), "durable nonces must be advanceable"),
(vote_authorize_with_seed::id(), "An instruction you can use to change a vote accounts authority when the current authority is a derived key #25860"),
(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"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()