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.

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

Track more accounts data size changes #26467

Merged
Merged
Changes from all commits
Commits
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
173 changes: 134 additions & 39 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3497,6 +3497,7 @@ impl Bank {
);
self.store_account(pubkey, account);
self.capitalization.fetch_add(account.lamports(), Relaxed);
self.accounts_data_size_initial += account.data().len() as u64;
}
// updating sysvars (the fees sysvar in this case) now depends on feature activations in
// genesis_config.accounts above
Expand All @@ -3509,6 +3510,7 @@ impl Bank {
pubkey
);
self.store_account(pubkey, account);
self.accounts_data_size_initial += account.data().len() as u64;
}

// highest staked node is the first collector
Expand Down Expand Up @@ -3548,12 +3550,14 @@ impl Bank {
}

fn burn_and_purge_account(&self, program_id: &Pubkey, mut account: AccountSharedData) {
let old_data_size = account.data().len();
self.capitalization.fetch_sub(account.lamports(), Relaxed);
// Both resetting account balance to 0 and zeroing the account data
// is needed to really purge from AccountsDb and flush the Stakes cache
account.set_lamports(0);
account.data_as_mut_slice().fill(0);
self.store_account(program_id, &account);
self.calculate_and_update_accounts_data_size_delta_off_chain(old_data_size, 0);
}

// NOTE: must hold idempotent for the same set of arguments
Expand Down Expand Up @@ -4730,6 +4734,16 @@ impl Bank {
.unwrap();
}

/// Calculate the data size delta and update the off-chain accounts data size delta
fn calculate_and_update_accounts_data_size_delta_off_chain(
&self,
old_data_size: usize,
new_data_size: usize,
) {
let data_size_delta = calculate_data_size_delta(old_data_size, new_data_size);
self.update_accounts_data_size_delta_off_chain(data_size_delta);
}

/// Set the initial accounts data size
/// NOTE: This fn is *ONLY FOR TESTS*
pub fn set_accounts_data_size_initial_for_tests(&mut self, amount: u64) {
Expand Down Expand Up @@ -6267,39 +6281,46 @@ impl Bank {
pubkey: &Pubkey,
new_account: &AccountSharedData,
) {
if let Some(old_account) = self.get_account_with_fixed_root(pubkey) {
match new_account.lamports().cmp(&old_account.lamports()) {
std::cmp::Ordering::Greater => {
let increased = new_account.lamports() - old_account.lamports();
trace!(
"store_account_and_update_capitalization: increased: {} {}",
pubkey,
increased
);
self.capitalization.fetch_add(increased, Relaxed);
}
std::cmp::Ordering::Less => {
let decreased = old_account.lamports() - new_account.lamports();
trace!(
"store_account_and_update_capitalization: decreased: {} {}",
pubkey,
decreased
);
self.capitalization.fetch_sub(decreased, Relaxed);
let old_account_data_size =
if let Some(old_account) = self.get_account_with_fixed_root(pubkey) {
match new_account.lamports().cmp(&old_account.lamports()) {
std::cmp::Ordering::Greater => {
let increased = new_account.lamports() - old_account.lamports();
trace!(
"store_account_and_update_capitalization: increased: {} {}",
pubkey,
increased
);
self.capitalization.fetch_add(increased, Relaxed);
}
std::cmp::Ordering::Less => {
let decreased = old_account.lamports() - new_account.lamports();
trace!(
"store_account_and_update_capitalization: decreased: {} {}",
pubkey,
decreased
);
self.capitalization.fetch_sub(decreased, Relaxed);
}
std::cmp::Ordering::Equal => {}
}
std::cmp::Ordering::Equal => {}
}
} else {
trace!(
"store_account_and_update_capitalization: created: {} {}",
pubkey,
new_account.lamports()
);
self.capitalization
.fetch_add(new_account.lamports(), Relaxed);
}
old_account.data().len()
} else {
trace!(
"store_account_and_update_capitalization: created: {} {}",
pubkey,
new_account.lamports()
);
self.capitalization
.fetch_add(new_account.lamports(), Relaxed);
0
};

self.store_account(pubkey, new_account);
self.calculate_and_update_accounts_data_size_delta_off_chain(
old_account_data_size,
new_account.data().len(),
);
}

fn withdraw(&self, pubkey: &Pubkey, lamports: u64) -> Result<()> {
Expand Down Expand Up @@ -7486,6 +7507,11 @@ impl Bank {
self.store_account(new_address, &AccountSharedData::default());

self.remove_executor(old_address);

self.calculate_and_update_accounts_data_size_delta_off_chain(
old_account.data().len(),
new_account.data().len(),
);
}
}
}
Expand All @@ -7510,23 +7536,30 @@ impl Bank {
// As a workaround for
// https://github.com/solana-labs/solana-program-library/issues/374, ensure that the
// spl-token 2 native mint account is owned by the spl-token 2 program.
let old_account_data_size;
let store = if let Some(existing_native_mint_account) =
self.get_account_with_fixed_root(&inline_spl_token::native_mint::id())
{
old_account_data_size = existing_native_mint_account.data().len();
if existing_native_mint_account.owner() == &solana_sdk::system_program::id() {
native_mint_account.set_lamports(existing_native_mint_account.lamports());
true
} else {
false
}
} else {
old_account_data_size = 0;
self.capitalization
.fetch_add(native_mint_account.lamports(), Relaxed);
true
};

if store {
self.store_account(&inline_spl_token::native_mint::id(), &native_mint_account);
self.calculate_and_update_accounts_data_size_delta_off_chain(
old_account_data_size,
native_mint_account.data().len(),
);
}
}
}
Expand Down Expand Up @@ -7605,6 +7638,17 @@ impl Bank {
}
}

/// Compute how much an account has changed size. This function is useful when the data size delta
/// needs to be computed and passed to an `update_accounts_data_size_delta` function.
fn calculate_data_size_delta(old_data_size: usize, new_data_size: usize) -> i64 {
assert!(old_data_size <= i64::MAX as usize);
assert!(new_data_size <= i64::MAX as usize);
let old_data_size = old_data_size as i64;
let new_data_size = new_data_size as i64;

new_data_size.saturating_sub(old_data_size)
}

/// Since `apply_feature_activations()` has different behavior depending on its caller, enumerate
/// those callers explicitly.
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -8754,19 +8798,14 @@ pub(crate) mod tests {
genesis_config.rent = rent_with_exemption_threshold(1000.0);

let root_bank = Arc::new(Bank::new_for_tests(&genesis_config));
let mut bank = create_child_bank_for_rent_test(&root_bank, &genesis_config);
let bank = create_child_bank_for_rent_test(&root_bank, &genesis_config);

let account_pubkey = solana_sdk::pubkey::new_rand();
let account_balance = 1;
let data_size = 12345_u64; // use non-zero data size to also test accounts_data_size
let mut account = AccountSharedData::new(
account_balance,
data_size as usize,
&solana_sdk::pubkey::new_rand(),
);
let mut account =
AccountSharedData::new(account_balance, 0, &solana_sdk::pubkey::new_rand());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still rent exempt? Not sure how test is relying on that, but test name implies that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add this test originally, but did later modify it to include a non-zero data size. Since then, I have added more tests for the accounts data size and it's no longer required here, so I wanted to return this test back to its original behavior. Since store_account() is called, I wanted to remove as much special-case code around it as possible. Let me know if you'd like me to change something in here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's fine. just a little confusing and making sure we understand.

account.set_executable(true);
bank.store_account(&account_pubkey, &account);
bank.accounts_data_size_initial = data_size;

let transfer_lamports = 1;
let tx = system_transaction::transfer(
Expand All @@ -8781,7 +8820,6 @@ pub(crate) mod tests {
Err(TransactionError::InvalidWritableAccount)
);
assert_eq!(bank.get_balance(&account_pubkey), account_balance);
assert_eq!(bank.load_accounts_data_size(), data_size);
}

#[test]
Expand Down Expand Up @@ -19238,4 +19276,61 @@ pub(crate) mod tests {
// also be reclaimed by rent collection.
assert!(reclaimed_data_size >= data_size);
}

#[test]
fn test_accounts_data_size_with_default_bank() {
let bank = Bank::default_for_tests();
assert_eq!(
bank.load_accounts_data_size() as usize,
bank.get_total_accounts_stats().unwrap().data_len
);
}

#[test]
fn test_accounts_data_size_from_genesis() {
let GenesisConfigInfo {
mut genesis_config,
mint_keypair,
..
} = genesis_utils::create_genesis_config_with_leader(
1_000_000 * LAMPORTS_PER_SOL,
&Pubkey::new_unique(),
100 * LAMPORTS_PER_SOL,
);
genesis_config.rent = Rent::default();
genesis_config.ticks_per_slot = 3;

let mut bank = Arc::new(Bank::new_for_tests(&genesis_config));
assert_eq!(
bank.load_accounts_data_size() as usize,
bank.get_total_accounts_stats().unwrap().data_len
);

// Create accounts over a number of banks and ensure the accounts data size remains correct
for _ in 0..10 {
bank = Arc::new(Bank::new_from_parent(
&bank,
&Pubkey::default(),
bank.slot() + 1,
));

// Store an account into the bank that is rent-exempt and has data
let data_size = rand::thread_rng().gen_range(3333, 4444);
let transaction = system_transaction::create_account(
&mint_keypair,
&Keypair::new(),
bank.last_blockhash(),
genesis_config.rent.minimum_balance(data_size),
data_size as u64,
&solana_sdk::system_program::id(),
);
bank.process_transaction(&transaction).unwrap();
bank.fill_bank_with_ticks_for_tests();

assert_eq!(
bank.load_accounts_data_size() as usize,
bank.get_total_accounts_stats().unwrap().data_len,
);
}
}
}