Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change RentResult enum values #29139

Merged
merged 2 commits into from
Dec 9, 2022
Merged

change RentResult enum values #29139

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1269ba6
test_shrink_candidate_slots uses write cache (#29145)
jeffwashington Dec 8, 2022
cf5b77d
add tests for collect_from_existing_account
jeffwashington Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 182 additions & 26 deletions runtime/src/rent_collector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@ impl Default for RentCollector {
/// when rent is collected for this account, this is the action to apply to the account
#[derive(Debug)]
enum RentResult {
/// maybe collect rent later, leave account alone
LeaveAloneNoRent,
/// this account will never have rent collected from it
Exempt,
/// maybe we collect rent later, but not now
NoRentCollectionNow,
/// collect rent
CollectRent {
new_rent_epoch: Epoch,
rent_due: u64, // lamports
rent_due: u64, // lamports, could be 0
},
}

Expand Down Expand Up @@ -127,7 +129,7 @@ impl RentCollector {
filler_account_suffix: Option<&Pubkey>,
) -> CollectedInfo {
match self.calculate_rent_result(address, account, filler_account_suffix) {
RentResult::LeaveAloneNoRent => CollectedInfo::default(),
RentResult::Exempt | RentResult::NoRentCollectionNow => CollectedInfo::default(),
RentResult::CollectRent {
new_rent_epoch,
rent_due,
Expand Down Expand Up @@ -159,37 +161,33 @@ impl RentCollector {
account: &impl ReadableAccount,
filler_account_suffix: Option<&Pubkey>,
) -> RentResult {
if self.can_skip_rent_collection(address, account, filler_account_suffix) {
return RentResult::LeaveAloneNoRent;
if account.rent_epoch() > self.epoch {
// potentially rent paying account
// Maybe collect rent later, leave account alone for now.
return RentResult::NoRentCollectionNow;
}
if !self.should_collect_rent(address, account)
|| crate::accounts_db::AccountsDb::is_filler_account_helper(
address,
filler_account_suffix,
)
brooksprumo marked this conversation as resolved.
Show resolved Hide resolved
{
// easy to determine this account should not consider having rent collected from it
return RentResult::Exempt;
brooksprumo marked this conversation as resolved.
Show resolved Hide resolved
}
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::LeaveAloneNoRent,
// Maybe collect rent later, leave account alone.
RentDue::Paying(0) => RentResult::LeaveAloneNoRent,
// account will not have rent collected ever
RentDue::Exempt => RentResult::Exempt,
// potentially rent paying account
// Maybe collect rent later, leave account alone for now.
RentDue::Paying(0) => RentResult::NoRentCollectionNow,
// Rent is collected for next epoch.
RentDue::Paying(rent_due) => RentResult::CollectRent {
new_rent_epoch: self.epoch + 1,
rent_due,
},
}
}

/// Performs easy checks to see if rent collection can be skipped
fn can_skip_rent_collection(
&self,
address: &Pubkey,
account: &impl ReadableAccount,
filler_account_suffix: Option<&Pubkey>,
) -> bool {
!self.should_collect_rent(address, account)
|| account.rent_epoch() > self.epoch
|| crate::accounts_db::AccountsDb::is_filler_account_helper(
address,
filler_account_suffix,
)
}
}

/// Information computed during rent collection
Expand Down Expand Up @@ -244,6 +242,164 @@ mod tests {
}
}

#[test]
fn test_calculate_rent_result() {
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,
),
"{:?}",
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None)
);

{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
filler_accounts
),
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()
);
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
),
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_account,
&mut account_clone,
Some(&filler_account)
),
CollectedInfo::default()
);
assert_eq!(account_clone, account);
}
}

#[test]
fn test_collect_from_account_created_and_existing() {
let old_lamports = 1000;
Expand Down