feature: set rent_epoch to Epoch::MAX (#28690)

* check android builds

* feature: set rent_epoch to Epoch::MAX

* tweaks

* Update runtime/src/rent_collector.rs

Co-authored-by: Brooks Prumo <brooks@prumo.org>

* simplify changes to tests

* back out some test changes

* calculate_rent_result passes through Exempt

* move calc outside loop

* if rent epoch is already max, use 'NoRentCollectionNow'

Co-authored-by: Brooks Prumo <brooks@prumo.org>
This commit is contained in:
Jeff Washington (jwash) 2023-01-02 09:51:08 -06:00 committed by GitHub
parent 3d72012a31
commit cf1aa4beea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 227 additions and 153 deletions

View File

@ -45,7 +45,7 @@ extern uint64_t entrypoint(const uint8_t *input) {
sol_assert(accounts[ARGUMENT_INDEX].data_len == 100);
sol_assert(accounts[ARGUMENT_INDEX].is_signer);
sol_assert(accounts[ARGUMENT_INDEX].is_writable);
sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == 0);
sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == UINT64_MAX);
sol_assert(!accounts[ARGUMENT_INDEX].executable);
for (int i = 0; i < accounts[ARGUMENT_INDEX].data_len; i++) {
sol_assert(accounts[ARGUMENT_INDEX].data[i] == i);
@ -57,7 +57,7 @@ extern uint64_t entrypoint(const uint8_t *input) {
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].data_len == 10);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].is_signer);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].is_writable);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == 0);
sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == UINT64_MAX);
sol_assert(!accounts[INVOKED_ARGUMENT_INDEX].executable);
sol_assert(
@ -66,7 +66,7 @@ extern uint64_t entrypoint(const uint8_t *input) {
&sbf_loader_id));
sol_assert(!accounts[INVOKED_PROGRAM_INDEX].is_signer);
sol_assert(!accounts[INVOKED_PROGRAM_INDEX].is_writable);
sol_assert(accounts[INVOKED_PROGRAM_INDEX].rent_epoch == 0);
sol_assert(accounts[INVOKED_PROGRAM_INDEX].rent_epoch == UINT64_MAX);
sol_assert(accounts[INVOKED_PROGRAM_INDEX].executable);
sol_assert(SolPubkey_same(accounts[INVOKED_PROGRAM_INDEX].key,

View File

@ -48,7 +48,7 @@ fn process_instruction(
assert_eq!(accounts[ARGUMENT_INDEX].data_len(), 100);
assert!(accounts[ARGUMENT_INDEX].is_signer);
assert!(accounts[ARGUMENT_INDEX].is_writable);
assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, 0);
assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, u64::MAX);
assert!(!accounts[ARGUMENT_INDEX].executable);
{
let data = accounts[ARGUMENT_INDEX].try_borrow_data()?;
@ -65,14 +65,14 @@ fn process_instruction(
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].data_len(), 10);
assert!(accounts[INVOKED_ARGUMENT_INDEX].is_signer);
assert!(accounts[INVOKED_ARGUMENT_INDEX].is_writable);
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch, 0);
assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch, u64::MAX);
assert!(!accounts[INVOKED_ARGUMENT_INDEX].executable);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].key, program_id);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].owner, &bpf_loader::id());
assert!(!accounts[INVOKED_PROGRAM_INDEX].is_signer);
assert!(!accounts[INVOKED_PROGRAM_INDEX].is_writable);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].rent_epoch, 0);
assert_eq!(accounts[INVOKED_PROGRAM_INDEX].rent_epoch, u64::MAX);
assert!(accounts[INVOKED_PROGRAM_INDEX].executable);
assert_eq!(

View File

@ -275,6 +275,8 @@ impl Accounts {
};
let mut accumulated_accounts_data_size: usize = 0;
let set_exempt_rent_epoch_max =
feature_set.is_active(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id());
let mut accounts = account_keys
.iter()
.enumerate()
@ -309,6 +311,7 @@ impl Accounts {
key,
&mut account,
self.accounts_db.filler_account_suffix.as_ref(),
set_exempt_rent_epoch_max,
)
.rent_amount;
(account, rent_due)

View File

@ -5338,12 +5338,16 @@ impl Bank {
let mut time_hashing_skipped_rewrites_us = 0;
let mut time_storing_accounts_us = 0;
let can_skip_rewrites = self.rc.accounts.accounts_db.skip_rewrites;
let set_exempt_rent_epoch_max: bool = self
.feature_set
.is_active(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id());
for (pubkey, account, _loaded_slot) in accounts.iter_mut() {
let (rent_collected_info, measure) =
measure!(self.rent_collector.collect_from_existing_account(
pubkey,
account,
self.rc.accounts.accounts_db.filler_account_suffix.as_ref(),
set_exempt_rent_epoch_max,
));
time_collecting_rent_us += measure.as_us();
@ -7974,11 +7978,12 @@ pub(crate) mod tests {
ancestors::Ancestors,
bank_client::BankClient,
genesis_utils::{
self, activate_all_features, bootstrap_validator_stake_lamports,
self, activate_all_features, activate_feature, bootstrap_validator_stake_lamports,
create_genesis_config_with_leader, create_genesis_config_with_vote_accounts,
genesis_sysvar_and_builtin_program_lamports, GenesisConfigInfo,
ValidatorVoteKeypairs,
},
rent_collector::TEST_SET_EXEMPT_RENT_EPOCH_MAX,
rent_paying_accounts_by_partition::RentPayingAccountsByPartition,
status_cache::MAX_CACHE_ENTRIES,
},
@ -8436,6 +8441,7 @@ pub(crate) mod tests {
&keypairs[4].pubkey(),
&mut account_copy,
None,
TEST_SET_EXEMPT_RENT_EPOCH_MAX,
);
assert_eq!(expected_rent.rent_amount, too_few_lamports);
assert_eq!(account_copy.lamports(), 0);
@ -9900,12 +9906,15 @@ pub(crate) mod tests {
solana_logger::setup();
let (mut genesis_config, _mint_keypair) = create_genesis_config(1_000_000);
activate_all_features(&mut genesis_config);
for feature_id in FeatureSet::default().inactive {
if feature_id != solana_sdk::feature_set::set_exempt_rent_epoch_max::id() {
activate_feature(&mut genesis_config, feature_id);
}
}
let zero_lamport_pubkey = solana_sdk::pubkey::new_rand();
let rent_due_pubkey = solana_sdk::pubkey::new_rand();
let rent_exempt_pubkey = solana_sdk::pubkey::new_rand();
let mut bank = Arc::new(Bank::new_for_tests(&genesis_config));
let zero_lamports = 0;
let little_lamports = 1234;
@ -11942,7 +11951,9 @@ pub(crate) mod tests {
let mut expected_next_slot = expected_previous_slot + 1;
// First, initialize the clock sysvar
activate_all_features(&mut genesis_config);
for feature_id in FeatureSet::default().inactive {
activate_feature(&mut genesis_config, feature_id);
}
let bank1 = Arc::new(Bank::new_for_tests_with_config(
&genesis_config,
BankTestConfig::default(),
@ -20033,6 +20044,7 @@ pub(crate) mod tests {
&keypair.pubkey(),
&mut account,
None,
TEST_SET_EXEMPT_RENT_EPOCH_MAX,
);
assert_eq!(info.account_data_len_reclaimed, data_size as u64);
}

View File

@ -29,6 +29,13 @@ impl Default for RentCollector {
}
}
/// When rent is collected from an exempt account, rent_epoch is set to this
/// value. The idea is to have a fixed, consistent value for rent_epoch for all accounts that do not collect rent.
/// This enables us to get rid of the field completely.
pub const RENT_EXEMPT_RENT_EPOCH: Epoch = Epoch::MAX;
pub const TEST_SET_EXEMPT_RENT_EPOCH_MAX: bool = false;
/// when rent is collected for this account, this is the action to apply to the account
#[derive(Debug)]
enum RentResult {
@ -111,9 +118,16 @@ impl RentCollector {
address: &Pubkey,
account: &mut AccountSharedData,
filler_account_suffix: Option<&Pubkey>,
set_exempt_rent_epoch_max: bool,
) -> CollectedInfo {
match self.calculate_rent_result(address, account, filler_account_suffix) {
RentResult::Exempt | RentResult::NoRentCollectionNow => CollectedInfo::default(),
RentResult::Exempt => {
if set_exempt_rent_epoch_max {
account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
CollectedInfo::default()
}
RentResult::NoRentCollectionNow => CollectedInfo::default(),
RentResult::CollectRent {
new_rent_epoch,
rent_due,
@ -145,8 +159,8 @@ impl RentCollector {
account: &impl ReadableAccount,
filler_account_suffix: Option<&Pubkey>,
) -> RentResult {
if account.rent_epoch() > self.epoch {
// potentially rent paying account
if account.rent_epoch() == RENT_EXEMPT_RENT_EPOCH || account.rent_epoch() > self.epoch {
// potentially rent paying account (or known and already marked exempt)
// Maybe collect rent later, leave account alone for now.
return RentResult::NoRentCollectionNow;
}
@ -221,166 +235,201 @@ mod tests {
// initialize rent_epoch as created at this epoch
account.set_rent_epoch(self.epoch);
self.collect_from_existing_account(
address, account, /*filler_account_suffix:*/ None,
address,
account,
/*filler_account_suffix:*/ None,
TEST_SET_EXEMPT_RENT_EPOCH_MAX,
)
}
}
#[test]
fn test_calculate_rent_result() {
let mut rent_collector = RentCollector::default();
for set_exempt_rent_epoch_max in [false, true] {
let mut rent_collector = RentCollector::default();
let mut account = AccountSharedData::default();
assert!(matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
RentResult::NoRentCollectionNow,
));
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None
),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
}
account.set_executable(true);
assert!(matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
RentResult::Exempt
));
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None
),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
}
account.set_executable(false);
assert!(matches!(
rent_collector.calculate_rent_result(&incinerator::id(), &account, None),
RentResult::Exempt
));
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&incinerator::id(),
&mut account_clone,
None
),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
}
// try a few combinations of rent collector rent epoch and collecting rent with and without filler accounts specified (but we aren't a filler)
let filler_account = solana_sdk::pubkey::new_rand();
for filler_accounts in [None, Some(&filler_account)] {
for (rent_epoch, rent_due_expected) in [(2, 2), (3, 5)] {
rent_collector.epoch = rent_epoch;
account.set_lamports(10);
account.set_rent_epoch(1);
let new_rent_epoch_expected = rent_collector.epoch + 1;
assert!(
matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, filler_accounts),
RentResult::CollectRent{ new_rent_epoch, rent_due} if new_rent_epoch == new_rent_epoch_expected && rent_due == rent_due_expected,
let mut account = AccountSharedData::default();
assert!(matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None,),
RentResult::NoRentCollectionNow,
));
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None,
set_exempt_rent_epoch_max
),
"{:?}",
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None)
CollectedInfo::default()
);
assert_eq!(account_clone, account);
}
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
filler_accounts
account.set_executable(true);
assert!(matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None,),
RentResult::Exempt
));
{
let mut account_clone = account.clone();
let mut account_expected = account.clone();
if set_exempt_rent_epoch_max {
account_expected.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None,
set_exempt_rent_epoch_max
),
CollectedInfo::default()
);
assert_eq!(account_clone, account_expected);
}
account.set_executable(false);
assert!(matches!(
rent_collector.calculate_rent_result(&incinerator::id(), &account, None,),
RentResult::Exempt
));
{
let mut account_clone = account.clone();
let mut account_expected = account.clone();
if set_exempt_rent_epoch_max {
account_expected.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
assert_eq!(
rent_collector.collect_from_existing_account(
&incinerator::id(),
&mut account_clone,
None,
set_exempt_rent_epoch_max
),
CollectedInfo::default()
);
assert_eq!(account_clone, account_expected);
}
// try a few combinations of rent collector rent epoch and collecting rent with and without filler accounts specified (but we aren't a filler)
let filler_account = solana_sdk::pubkey::new_rand();
for filler_accounts in [None, Some(&filler_account)] {
for (rent_epoch, rent_due_expected) in [(2, 2), (3, 5)] {
rent_collector.epoch = rent_epoch;
account.set_lamports(10);
account.set_rent_epoch(1);
let new_rent_epoch_expected = rent_collector.epoch + 1;
assert!(
matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, filler_accounts),
RentResult::CollectRent{ new_rent_epoch, rent_due} if new_rent_epoch == new_rent_epoch_expected && rent_due == rent_due_expected,
),
CollectedInfo {
rent_amount: rent_due_expected,
account_data_len_reclaimed: 0
}
"{:?}",
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None,)
);
let mut account_expected = account.clone();
account_expected.set_lamports(account.lamports() - rent_due_expected);
account_expected.set_rent_epoch(new_rent_epoch_expected);
assert_eq!(account_clone, account_expected);
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
filler_accounts,
set_exempt_rent_epoch_max
),
CollectedInfo {
rent_amount: rent_due_expected,
account_data_len_reclaimed: 0
}
);
let mut account_expected = account.clone();
account_expected.set_lamports(account.lamports() - rent_due_expected);
account_expected.set_rent_epoch(new_rent_epoch_expected);
assert_eq!(account_clone, account_expected);
}
}
}
}
// enough lamports to make us exempt
account.set_lamports(1_000_000);
assert!(matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
RentResult::Exempt,
));
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None
),
CollectedInfo::default()
// enough lamports to make us exempt
account.set_lamports(1_000_000);
let result = rent_collector.calculate_rent_result(&Pubkey::default(), &account, None);
assert!(
matches!(result, RentResult::Exempt),
"{:?}, set_exempt_rent_epoch_max: {}",
result,
set_exempt_rent_epoch_max,
);
assert_eq!(account_clone, account);
}
{
let mut account_clone = account.clone();
let mut account_expected = account.clone();
if set_exempt_rent_epoch_max {
account_expected.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None,
set_exempt_rent_epoch_max
),
CollectedInfo::default()
);
assert_eq!(account_clone, account_expected);
}
// enough lamports to make us exempt
// but, our rent_epoch is set in the future, so we can't know if we are exempt yet or not.
// We don't calculate rent amount vs data if the rent_epoch is already in the future.
account.set_rent_epoch(1_000_000);
assert!(matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
RentResult::NoRentCollectionNow,
));
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None
),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
}
// enough lamports to make us exempt
// but, our rent_epoch is set in the future, so we can't know if we are exempt yet or not.
// We don't calculate rent amount vs data if the rent_epoch is already in the future.
account.set_rent_epoch(1_000_000);
assert!(matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None,),
RentResult::NoRentCollectionNow,
));
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None,
set_exempt_rent_epoch_max
),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
}
// filler accounts are exempt
account.set_rent_epoch(1);
account.set_lamports(10);
assert!(matches!(
rent_collector.calculate_rent_result(&filler_account, &account, Some(&filler_account)),
RentResult::Exempt,
));
{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
// filler accounts are exempt
account.set_rent_epoch(1);
account.set_lamports(10);
assert!(matches!(
rent_collector.calculate_rent_result(
&filler_account,
&mut account_clone,
Some(&filler_account)
&account,
Some(&filler_account),
),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
RentResult::Exempt,
));
{
let mut account_clone = account.clone();
let mut account_expected = account.clone();
if set_exempt_rent_epoch_max {
account_expected.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
}
assert_eq!(
rent_collector.collect_from_existing_account(
&filler_account,
&mut account_clone,
Some(&filler_account),
set_exempt_rent_epoch_max
),
CollectedInfo::default()
);
assert_eq!(account_clone, account_expected);
}
}
}
@ -418,6 +467,7 @@ mod tests {
&solana_sdk::pubkey::new_rand(),
&mut existing_account,
None, // filler_account_suffix
TEST_SET_EXEMPT_RENT_EPOCH_MAX,
);
assert!(existing_account.lamports() < old_lamports);
assert_eq!(
@ -451,6 +501,7 @@ mod tests {
&pubkey,
&mut account,
None, // filler_account_suffix
TEST_SET_EXEMPT_RENT_EPOCH_MAX,
);
assert_eq!(account.lamports(), huge_lamports);
assert_eq!(collected, CollectedInfo::default());
@ -463,6 +514,7 @@ mod tests {
&pubkey,
&mut account,
None, // filler_account_suffix
TEST_SET_EXEMPT_RENT_EPOCH_MAX,
);
assert_eq!(account.lamports(), tiny_lamports - collected.rent_amount);
assert_ne!(collected, CollectedInfo::default());
@ -486,6 +538,7 @@ mod tests {
&pubkey,
&mut account,
None, // filler_account_suffix
TEST_SET_EXEMPT_RENT_EPOCH_MAX,
);
assert_eq!(account.lamports(), 0);
assert_eq!(collected.rent_amount, 1);
@ -510,6 +563,7 @@ mod tests {
&Pubkey::new_unique(),
&mut account,
None, // filler_account_suffix
TEST_SET_EXEMPT_RENT_EPOCH_MAX,
);
assert_eq!(collected.rent_amount, account_lamports);

View File

@ -498,6 +498,10 @@ pub mod account_hash_ignore_slot {
solana_sdk::declare_id!("SVn36yVApPLYsa8koK3qUcy14zXDnqkNYWyUh1f4oK1");
}
pub mod set_exempt_rent_epoch_max {
solana_sdk::declare_id!("5wAGiy15X1Jb2hkHnPDCM8oB9V42VNA9ftNVFK84dEgv");
}
pub mod relax_authority_signer_check_for_lookup_table_creation {
solana_sdk::declare_id!("FKAcEvNgSY79RpqsPNUV5gDyumopH4cEHqUxyfm8b8Ap");
}
@ -678,6 +682,7 @@ lazy_static! {
(enable_early_verification_of_account_modifications::id(), "enable early verification of account modifications #25899"),
(disable_rehash_for_rent_epoch::id(), "on accounts hash calculation, do not try to rehash accounts #28934"),
(account_hash_ignore_slot::id(), "ignore slot when calculating an account hash #28420"),
(set_exempt_rent_epoch_max::id(), "set rent epoch to Epoch::MAX for rent-exempt accounts #28683"),
(on_load_preserve_rent_epoch_for_rent_exempt_accounts::id(), "on bank load account, do not try to fix up rent_epoch #28541"),
(prevent_crediting_accounts_that_end_rent_paying::id(), "prevent crediting rent paying accounts #26606"),
(cap_bpf_program_instruction_accounts::id(), "enforce max number of accounts per bpf program instruction #26628"),