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

fix account resize test by requesting max tx data size #28826

Merged
merged 3 commits into from
Nov 16, 2022
Merged

fix account resize test by requesting max tx data size #28826

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2383a37
fix account resize test by requesting max tx data size
tao-stones Nov 16, 2022
b94a842
define data size limit in increamental of 1024
tao-stones Nov 16, 2022
77dc305
review: remove log setup for a test; use u32::MAX;
tao-stones Nov 16, 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
28 changes: 23 additions & 5 deletions runtime/src/bank.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19479,6 +19479,7 @@ pub(crate) mod tests {
new_balance: u64,
mock_program_id: Pubkey,
recent_blockhash: Hash,
tx_data_size_limit: u32,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this fn is only used in tests, and those tests are not meant to exercise the ComputeBudget's load-account-data-size cap, I think it'd be nicer to not take this parameter here, and instead just call ComputeBudgetInstruction::set_accounts_data_size_limit() with u32::MAX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially added the exact tx data size limit as a way to demonstrate the usage of set_accounts_data_size_limit(). But agree this may pollute this test. Replaced it with u32::MAX

) -> Transaction {
let account_metas = vec![
AccountMeta::new(funder.pubkey(), false),
Expand All @@ -19490,7 +19491,10 @@ pub(crate) mod tests {
account_metas,
);
Transaction::new_signed_with_payer(
&[instruction],
&[
instruction,
ComputeBudgetInstruction::set_accounts_data_size_limit(tx_data_size_limit),
],
Some(&payer.pubkey()),
&[payer],
recent_blockhash,
Expand Down Expand Up @@ -19550,6 +19554,7 @@ pub(crate) mod tests {
rent_exempt_minimum_small - 1,
mock_program_id,
recent_blockhash,
u32::MAX,
);
let expected_err = {
let account_index = tx
Expand All @@ -19575,6 +19580,7 @@ pub(crate) mod tests {
rent_exempt_minimum_large,
mock_program_id,
recent_blockhash,
u32::MAX,
);
let result = bank.process_transaction(&tx);
assert!(result.is_ok());
Expand All @@ -19592,6 +19598,7 @@ pub(crate) mod tests {
rent_exempt_minimum_small - 1,
mock_program_id,
recent_blockhash,
u32::MAX,
);
let expected_err = {
let account_index = tx
Expand All @@ -19617,6 +19624,7 @@ pub(crate) mod tests {
rent_exempt_minimum_small,
mock_program_id,
recent_blockhash,
u32::MAX,
);
let result = bank.process_transaction(&tx);
assert!(result.is_ok());
Expand All @@ -19634,6 +19642,7 @@ pub(crate) mod tests {
rent_exempt_minimum_large - 1,
mock_program_id,
recent_blockhash,
u32::MAX,
);
let expected_err = {
let account_index = tx
Expand All @@ -19659,6 +19668,7 @@ pub(crate) mod tests {
rent_exempt_minimum_large,
mock_program_id,
recent_blockhash,
u32::MAX,
);
let result = bank.process_transaction(&tx);
assert!(result.is_ok());
Expand Down Expand Up @@ -19780,6 +19790,7 @@ pub(crate) mod tests {
/// Ensure that accounts data size is updated correctly on resize transactions
#[test]
fn test_accounts_data_size_and_resize_transactions() {
solana_logger::setup();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

let GenesisConfigInfo {
genesis_config,
mint_keypair,
Expand All @@ -19792,6 +19803,12 @@ pub(crate) mod tests {
&mock_program_id,
mock_realloc_process_instruction,
);

// get builtin program account data size, will beused to request max data size for
// transaction.
let mock_account_data_size = bank.get_account_with_fixed_root(&mock_program_id).unwrap().data().len();
let compute_budget_account_data_size = bank.get_account_with_fixed_root(&solana_sdk::compute_budget::id()).unwrap().data().len();

let recent_blockhash = bank.last_blockhash();

let funding_keypair = Keypair::new();
Expand All @@ -19808,7 +19825,7 @@ pub(crate) mod tests {
let account_balance = LAMPORTS_PER_SOL;
let account_size = rng.gen_range(
1,
(MAX_PERMITTED_DATA_LENGTH / 4) as usize - MAX_PERMITTED_DATA_INCREASE,
MAX_PERMITTED_DATA_LENGTH as usize - MAX_PERMITTED_DATA_INCREASE,
);
let account_data =
AccountSharedData::new(account_balance, account_size, &mock_program_id);
Expand All @@ -19824,6 +19841,7 @@ pub(crate) mod tests {
account_balance,
mock_program_id,
recent_blockhash,
(account_size + mock_account_data_size + compute_budget_account_data_size) as u32,
);
let result = bank.process_transaction(&transaction);
assert!(result.is_ok());
Expand All @@ -19838,9 +19856,8 @@ pub(crate) mod tests {
{
let account_pubkey = Pubkey::new_unique();
let account_balance = LAMPORTS_PER_SOL;
let account_size = rng
.gen_range(MAX_PERMITTED_DATA_LENGTH / 8, MAX_PERMITTED_DATA_LENGTH / 4)
as usize;
let account_size =
rng.gen_range(MAX_PERMITTED_DATA_LENGTH / 2, MAX_PERMITTED_DATA_LENGTH) as usize;
let account_data =
AccountSharedData::new(account_balance, account_size, &mock_program_id);
bank.store_account(&account_pubkey, &account_data);
Expand All @@ -19855,6 +19872,7 @@ pub(crate) mod tests {
account_balance,
mock_program_id,
recent_blockhash,
(account_size + mock_account_data_size + compute_budget_account_data_size) as u32,
);
let result = bank.process_transaction(&transaction);
assert!(result.is_ok());
Expand Down