Skip to content

Commit

Permalink
Removes filler accounts (#34115)
Browse files Browse the repository at this point in the history
  • Loading branch information
@brooksprumo
brooksprumo authored Nov 20, 2023
1 parent 2024638 commit e02f25d
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 432 deletions.
224 changes: 2 additions & 222 deletions accounts-db/src/accounts_db.rs
View file

Large diffs are not rendered by default.

26 changes: 4 additions & 22 deletions accounts-db/src/accounts_hash.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,6 @@ impl CumulativeOffsets {

#[derive(Debug)]
pub struct AccountsHasher<'a> {
pub filler_account_suffix: Option<Pubkey>,
pub zero_lamport_accounts: ZeroLamportAccounts,
/// The directory where temporary cache files are put
pub dir_for_temp_cache_files: PathBuf,
Expand All @@ -495,11 +494,6 @@ struct ItemLocation<'a> {
}

impl<'a> AccountsHasher<'a> {
/// true if it is possible that there are filler accounts present
pub fn filler_accounts_enabled(&self) -> bool {
self.filler_account_suffix.is_some()
}

pub fn calculate_hash(hashes: Vec<Vec<Hash>>) -> (Hash, usize) {
let cumulative_offsets = CumulativeOffsets::from_raw(&hashes);

Expand Down Expand Up @@ -1151,7 +1145,6 @@ impl<'a> AccountsHasher<'a> {
};

let mut overall_sum = 0;
let filler_accounts_enabled = self.filler_accounts_enabled();

while let Some(pointer) = working_set.pop() {
let key = &sorted_data_by_pubkey[pointer.slot_group_index][pointer.offset].pubkey;
Expand All @@ -1166,13 +1159,10 @@ impl<'a> AccountsHasher<'a> {

// add lamports and get hash
if item.lamports != 0 {
// do not include filler accounts in the hash
if !(filler_accounts_enabled && self.is_filler_account(&item.pubkey)) {
overall_sum = Self::checked_cast_for_capitalization(
item.lamports as u128 + overall_sum as u128,
);
hashes.write(&item.hash.0);
}
overall_sum = Self::checked_cast_for_capitalization(
item.lamports as u128 + overall_sum as u128,
);
hashes.write(&item.hash.0);
} else {
// if lamports == 0, check if they should be included
if self.zero_lamport_accounts == ZeroLamportAccounts::Included {
Expand All @@ -1196,13 +1186,6 @@ impl<'a> AccountsHasher<'a> {
(hashes, overall_sum)
}

fn is_filler_account(&self, pubkey: &Pubkey) -> bool {
crate::accounts_db::AccountsDb::is_filler_account_helper(
pubkey,
self.filler_account_suffix.as_ref(),
)
}

/// input:
/// vec: group of slot data, ordered by Slot (low to high)
/// vec: [..] - items found in that slot range Sorted by: Pubkey, higher Slot, higher Write version (if pubkey =)
Expand Down Expand Up @@ -1343,7 +1326,6 @@ mod tests {
impl<'a> AccountsHasher<'a> {
fn new(dir_for_temp_cache_files: PathBuf) -> Self {
Self {
filler_account_suffix: None,
zero_lamport_accounts: ZeroLamportAccounts::Excluded,
dir_for_temp_cache_files,
active_stats: &ACTIVE_STATS,
Expand Down
132 changes: 38 additions & 94 deletions accounts-db/src/rent_collector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,9 @@ impl RentCollector {
&self,
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) {
match self.calculate_rent_result(address, account) {
RentResult::Exempt => {
if set_exempt_rent_epoch_max {
account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH);
Expand Down Expand Up @@ -151,19 +150,13 @@ impl RentCollector {
&self,
address: &Pubkey,
account: &impl ReadableAccount,
filler_account_suffix: Option<&Pubkey>,
) -> RentResult {
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;
}
if !self.should_collect_rent(address, account)
|| crate::accounts_db::AccountsDb::is_filler_account_helper(
address,
filler_account_suffix,
)
{
if !self.should_collect_rent(address, account) {
// easy to determine this account should not consider having rent collected from it
return RentResult::Exempt;
}
Expand Down Expand Up @@ -230,12 +223,7 @@ mod tests {
) -> CollectedInfo {
// 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,
set_exempt_rent_epoch_max,
)
self.collect_from_existing_account(address, account, set_exempt_rent_epoch_max)
}
}

Expand All @@ -246,7 +234,7 @@ mod tests {

let mut account = AccountSharedData::default();
assert_matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
rent_collector.calculate_rent_result(&Pubkey::default(), &account),
RentResult::NoRentCollectionNow
);
{
Expand All @@ -255,7 +243,6 @@ mod tests {
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None,
set_exempt_rent_epoch_max
),
CollectedInfo::default()
Expand All @@ -265,7 +252,7 @@ mod tests {

account.set_executable(true);
assert_matches!(
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
rent_collector.calculate_rent_result(&Pubkey::default(), &account),
RentResult::Exempt
);
{
Expand All @@ -278,7 +265,6 @@ mod tests {
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None,
set_exempt_rent_epoch_max
),
CollectedInfo::default()
Expand All @@ -288,7 +274,7 @@ mod tests {

account.set_executable(false);
assert_matches!(
rent_collector.calculate_rent_result(&incinerator::id(), &account, None),
rent_collector.calculate_rent_result(&incinerator::id(), &account),
RentResult::Exempt
);
{
Expand All @@ -301,57 +287,51 @@ mod tests {
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,
// try a few combinations of rent collector rent epoch and collecting rent
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),
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)
);

{
let mut account_clone = account.clone();
assert_eq!(
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
set_exempt_rent_epoch_max
),
"{:?}",
rent_collector.calculate_rent_result(&Pubkey::default(), &account, None,)
CollectedInfo {
rent_amount: rent_due_expected,
account_data_len_reclaimed: 0
}
);

{
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);
}
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);
let result = rent_collector.calculate_rent_result(&Pubkey::default(), &account, None);
let result = rent_collector.calculate_rent_result(&Pubkey::default(), &account);
assert!(
matches!(result, RentResult::Exempt),
"{result:?}, set_exempt_rent_epoch_max: {set_exempt_rent_epoch_max}",
Expand All @@ -366,7 +346,6 @@ mod tests {
rent_collector.collect_from_existing_account(
&Pubkey::default(),
&mut account_clone,
None,
set_exempt_rent_epoch_max
),
CollectedInfo::default()
Expand All @@ -379,7 +358,7 @@ mod tests {
// 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),
rent_collector.calculate_rent_result(&Pubkey::default(), &account),
RentResult::NoRentCollectionNow
);
{
Expand All @@ -388,42 +367,12 @@ mod tests {
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();
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);
}
}
}

Expand Down Expand Up @@ -464,7 +413,6 @@ mod tests {
let collected = rent_collector.collect_from_existing_account(
&solana_sdk::pubkey::new_rand(),
&mut existing_account,
None, // filler_account_suffix
set_exempt_rent_epoch_max,
);
assert!(existing_account.lamports() < old_lamports);
Expand Down Expand Up @@ -502,7 +450,6 @@ mod tests {
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
None, // filler_account_suffix
set_exempt_rent_epoch_max,
);
assert_eq!(account.lamports(), huge_lamports);
Expand All @@ -519,7 +466,6 @@ mod tests {
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
None, // filler_account_suffix
set_exempt_rent_epoch_max,
);
assert_eq!(account.lamports(), tiny_lamports - collected.rent_amount);
Expand All @@ -546,7 +492,6 @@ mod tests {
let collected = rent_collector.collect_from_existing_account(
&pubkey,
&mut account,
None, // filler_account_suffix
set_exempt_rent_epoch_max,
);
assert_eq!(account.lamports(), 0);
Expand All @@ -573,7 +518,6 @@ mod tests {
let collected = rent_collector.collect_from_existing_account(
&Pubkey::new_unique(),
&mut account,
None, // filler_account_suffix
set_exempt_rent_epoch_max,
);

Expand Down
8 changes: 1 addition & 7 deletions core/src/validator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,13 +748,7 @@ impl Validator {

let (snapshot_package_sender, snapshot_packager_service) =
if config.snapshot_config.should_generate_snapshots() {
// filler accounts make snapshots invalid for use
// so, do not publish that we have snapshots
let enable_gossip_push = config
.accounts_db_config
.as_ref()
.map(|config| config.filler_accounts_config.count == 0)
.unwrap_or(true);
let enable_gossip_push = true;
let (snapshot_package_sender, snapshot_package_receiver) =
crossbeam_channel::unbounded();
let snapshot_packager_service = SnapshotPackagerService::new(
Expand Down
8 changes: 1 addition & 7 deletions ledger-tool/src/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use {
crate::LEDGER_TOOL_DIRECTORY,
clap::{value_t, values_t_or_exit, ArgMatches},
solana_accounts_db::{
accounts_db::{AccountsDb, AccountsDbConfig, FillerAccountsConfig},
accounts_db::{AccountsDb, AccountsDbConfig},
accounts_index::{AccountsIndexConfig, IndexLimitMb},
partitioned_rewards::TestPartitionedEpochRewards,
},
Expand Down Expand Up @@ -53,11 +53,6 @@ pub fn get_accounts_db_config(
..AccountsIndexConfig::default()
};

let filler_accounts_config = FillerAccountsConfig {
count: value_t!(arg_matches, "accounts_filler_count", usize).unwrap_or(0),
size: value_t!(arg_matches, "accounts_filler_size", usize).unwrap_or(0),
};

let accounts_hash_cache_path = arg_matches
.value_of("accounts_hash_cache_path")
.map(Into::into)
Expand All @@ -77,7 +72,6 @@ pub fn get_accounts_db_config(
index: Some(accounts_index_config),
base_working_path: Some(ledger_tool_ledger_path),
accounts_hash_cache_path: Some(accounts_hash_cache_path),
filler_accounts_config,
ancient_append_vec_offset: value_t!(arg_matches, "accounts_db_ancient_append_vecs", i64)
.ok(),
exhaustively_verify_refcounts: arg_matches.is_present("accounts_db_verify_refcounts"),
Expand Down
Loading

0 comments on commit e02f25d

Please sign in to comment.