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

Add default value for loaded-accounts-data-size #1355

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions compute-budget/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ solana-sdk = { workspace = true }
rustc_version = { workspace = true }

[features]
dev-context-only-utils = []
frozen-abi = [
"dep:solana-frozen-abi",
"solana-sdk/frozen-abi",
Expand Down
113 changes: 81 additions & 32 deletions compute-budget/src/compute_budget_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ pub const MIN_HEAP_FRAME_BYTES: u32 = HEAP_LENGTH as u32;

/// The total accounts data a transaction can load is limited to 64MiB to not break
/// anyone in Mainnet-beta today. It can be set by set_loaded_accounts_data_size_limit instruction
pub const MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES: u32 = 64 * 1024 * 1024;
const MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES: u32 = 64 * 1024 * 1024;
/// The default accounts data size a transaction can load if sender didn't set specific
/// value via set_loaded_accounts_data_size_limit
const DEFAULT_LOADED_ACCOUNTS_DATA_SIZE_BYTES: u32 = 2 * 1024 * 1024;

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct ComputeBudgetLimits {
Expand All @@ -31,13 +34,35 @@ pub struct ComputeBudgetLimits {
pub loaded_accounts_bytes: u32,
}

// Default is only usable from dev context (ie. tests, benches etc)
#[cfg(any(test, feature = "dev-context-only-utils"))]
impl Default for ComputeBudgetLimits {
fn default() -> Self {
Self::new_with(false)
}
}

impl ComputeBudgetLimits {
// default constructor with feature gate
pub fn new_with(use_default_loaded_accounts_data_size: bool) -> Self {
ComputeBudgetLimits {
updated_heap_bytes: MIN_HEAP_FRAME_BYTES,
compute_unit_limit: MAX_COMPUTE_UNIT_LIMIT,
compute_unit_price: 0,
loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES,
loaded_accounts_bytes: Self::get_default_loaded_accounts_data_size_bytes(
use_default_loaded_accounts_data_size,
),
}
}

// behavior before/after feature gate `default_loaded_accounts_data_size_limit`
pub fn get_default_loaded_accounts_data_size_bytes(
use_default_loaded_accounts_data_size: bool,
) -> u32 {
if use_default_loaded_accounts_data_size {
DEFAULT_LOADED_ACCOUNTS_DATA_SIZE_BYTES
} else {
MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES
}
}
}
Expand Down Expand Up @@ -68,6 +93,7 @@ impl From<ComputeBudgetLimits> for FeeBudgetLimits {
/// are retrieved and returned,
pub fn process_compute_budget_instructions<'a>(
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
use_default_loaded_accounts_data_size: bool,
) -> Result<ComputeBudgetLimits, TransactionError> {
let mut num_non_compute_budget_instructions: u32 = 0;
let mut updated_compute_unit_limit = None;
Expand Down Expand Up @@ -136,7 +162,11 @@ pub fn process_compute_budget_instructions<'a>(
let compute_unit_price = updated_compute_unit_price.unwrap_or(0);

let loaded_accounts_bytes = updated_loaded_accounts_data_size_limit
.unwrap_or(MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES)
.unwrap_or(
ComputeBudgetLimits::get_default_loaded_accounts_data_size_bytes(
use_default_loaded_accounts_data_size,
),
)
.min(MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES);

Ok(ComputeBudgetLimits {
Expand Down Expand Up @@ -168,17 +198,22 @@ mod tests {
};

macro_rules! test {
( $instructions: expr, $expected_result: expr) => {
( $instructions: expr, $expected_result: expr, $use_default_loaded_accounts_data_size: expr) => {
let payer_keypair = Keypair::new();
let tx = SanitizedTransaction::from_transaction_for_tests(Transaction::new(
&[&payer_keypair],
Message::new($instructions, Some(&payer_keypair.pubkey())),
Hash::default(),
));
let result =
process_compute_budget_instructions(tx.message().program_instructions_iter());
let result = process_compute_budget_instructions(
tx.message().program_instructions_iter(),
$use_default_loaded_accounts_data_size,
);
assert_eq!($expected_result, result);
};
( $instructions: expr, $expected_result: expr) => {
test!($instructions, $expected_result, true);
};
}

#[test]
Expand Down Expand Up @@ -438,20 +473,26 @@ mod tests {

// Assert when set_loaded_accounts_data_size_limit is not presented
// budget is set to default data size
let expected_result = Ok(ComputeBudgetLimits {
compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT,
loaded_accounts_bytes: MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES,
..ComputeBudgetLimits::default()
});

test!(
&[Instruction::new_with_bincode(
Pubkey::new_unique(),
&0_u8,
vec![]
),],
expected_result
);
for use_default_loaded_accounts_data_size in [true, false] {
let expected_result = Ok(ComputeBudgetLimits {
compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT,
loaded_accounts_bytes:
ComputeBudgetLimits::get_default_loaded_accounts_data_size_bytes(
use_default_loaded_accounts_data_size,
),
..ComputeBudgetLimits::default()
});

test!(
&[Instruction::new_with_bincode(
Pubkey::new_unique(),
&0_u8,
vec![]
),],
expected_result,
use_default_loaded_accounts_data_size
);
}

// Assert when set_loaded_accounts_data_size_limit presents more than once,
// return DuplicateInstruction
Expand Down Expand Up @@ -483,18 +524,26 @@ mod tests {
Hash::default(),
));

let result =
process_compute_budget_instructions(transaction.message().program_instructions_iter());
for use_default_loaded_accounts_data_size in [true, false] {
let result = process_compute_budget_instructions(
transaction.message().program_instructions_iter(),
use_default_loaded_accounts_data_size,
);

// assert process_instructions will be successful with default,
// and the default compute_unit_limit is 2 times default: one for bpf ix, one for
// builtin ix.
assert_eq!(
result,
Ok(ComputeBudgetLimits {
compute_unit_limit: 2 * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT,
..ComputeBudgetLimits::default()
})
);
// assert process_instructions will be successful with default,
// and the default compute_unit_limit is 2 times default: one for bpf ix, one for
// builtin ix.
assert_eq!(
result,
Ok(ComputeBudgetLimits {
compute_unit_limit: 2 * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT,
loaded_accounts_bytes:
ComputeBudgetLimits::get_default_loaded_accounts_data_size_bytes(
use_default_loaded_accounts_data_size
),
..ComputeBudgetLimits::default()
})
);
}
}
}
8 changes: 6 additions & 2 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,8 +741,12 @@ impl Consumer {
error_counters: &mut TransactionErrorMetrics,
) -> Result<(), TransactionError> {
let fee_payer = message.fee_payer();
let budget_limits =
process_compute_budget_instructions(message.program_instructions_iter())?.into();
let budget_limits = process_compute_budget_instructions(
message.program_instructions_iter(),
bank.feature_set
.is_active(&feature_set::default_loaded_accounts_data_size_limit::id()),
)?
.into();
let fee = bank.fee_structure().calculate_fee(
message,
bank.get_lamports_per_signature(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ mod tests {
let transaction = Transaction::new_unsigned(Message::new(
&[
ComputeBudgetInstruction::set_compute_unit_price(priority),
ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(64 * 1024 * 1024),
system_instruction::transfer(&from_account, write_to_account, 2),
],
Some(&from_account),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use {
solana_sdk::{
self,
clock::{FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE},
feature_set,
fee::FeeBudgetLimits,
saturating_add_assign,
transaction::SanitizedTransaction,
Expand Down Expand Up @@ -508,9 +509,13 @@ impl SchedulerController {
.is_ok()
})
.filter_map(|(packet, tx)| {
process_compute_budget_instructions(tx.message().program_instructions_iter())
.map(|compute_budget| (packet, tx, compute_budget.into()))
.ok()
process_compute_budget_instructions(
tx.message().program_instructions_iter(),
bank.feature_set
.is_active(&feature_set::default_loaded_accounts_data_size_limit::id()),
)
.map(|compute_budget| (packet, tx, compute_budget.into()))
.ok()
})
.for_each(|(packet, tx, fee_budget_limits)| {
arc_packets.push(packet);
Expand Down
10 changes: 6 additions & 4 deletions cost-model/src/cost_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,10 @@ impl CostModel {

// if failed to process compute_budget instructions, the transaction will not be executed
// by `bank`, therefore it should be considered as no execution cost by cost model.
match process_compute_budget_instructions(transaction.message().program_instructions_iter())
{
match process_compute_budget_instructions(
transaction.message().program_instructions_iter(),
feature_set.is_active(&feature_set::default_loaded_accounts_data_size_limit::id()),
) {
Ok(compute_budget_limits) => {
// if tx contained user-space instructions and a more accurate estimate available correct it,
// where "user-space instructions" must be specifically checked by
Expand Down Expand Up @@ -279,6 +281,7 @@ impl CostModel {
mod tests {
use {
super::*,
solana_compute_budget::compute_budget_processor::ComputeBudgetLimits,
solana_sdk::{
compute_budget::{self, ComputeBudgetInstruction},
fee::ACCOUNT_DATA_COST_PAGE_SIZE,
Expand Down Expand Up @@ -615,8 +618,7 @@ mod tests {
.unwrap();
const DEFAULT_PAGE_COST: u64 = 8;
let expected_loaded_accounts_data_size_cost =
solana_compute_budget::compute_budget_processor::MAX_LOADED_ACCOUNTS_DATA_SIZE_BYTES
as u64
ComputeBudgetLimits::get_default_loaded_accounts_data_size_bytes(true) as u64
/ ACCOUNT_DATA_COST_PAGE_SIZE
* DEFAULT_PAGE_COST;

Expand Down
15 changes: 11 additions & 4 deletions cost-model/src/transaction_cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,10 @@ mod tests {
use {
super::*,
crate::cost_model::CostModel,
solana_compute_budget::compute_budget_processor::ComputeBudgetLimits,
solana_sdk::{
feature_set::FeatureSet,
fee::ACCOUNT_DATA_COST_PAGE_SIZE,
hash::Hash,
message::SimpleAddressLoader,
reserved_account_keys::ReservedAccountKeys,
Expand Down Expand Up @@ -246,16 +248,21 @@ mod tests {
)
.unwrap();

// expected vote tx cost: 2 write locks, 1 sig, 1 vote ix, 8cu of loaded accounts size,
// expected vote tx cost: 2 write locks, 1 sig, 1 vote ix, 8cu of ix data bytes,
let expected_vote_cost = SIMPLE_VOTE_USAGE_COST;
// expected non-vote tx cost would include default loaded accounts size cost (16384) additionally
let expected_none_vote_cost = 20535;
// expected non-vote tx cost would include default loaded accounts size cost additionally
const DEFAULT_PAGE_COST: u64 = 8;
let expected_loaded_accounts_data_size_cost =
ComputeBudgetLimits::get_default_loaded_accounts_data_size_bytes(true) as u64
/ ACCOUNT_DATA_COST_PAGE_SIZE
* DEFAULT_PAGE_COST;
let min_none_vote_cost = SIMPLE_VOTE_USAGE_COST + expected_loaded_accounts_data_size_cost;

let vote_cost = CostModel::calculate_cost(&vote_transaction, &FeatureSet::all_enabled());
let none_vote_cost =
CostModel::calculate_cost(&none_vote_transaction, &FeatureSet::all_enabled());

assert_eq!(expected_vote_cost, vote_cost.sum());
assert_eq!(expected_none_vote_cost, none_vote_cost.sum());
assert!(min_none_vote_cost < none_vote_cost.sum());
}
}
6 changes: 5 additions & 1 deletion programs/bpf-loader-tests/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use {
account::AccountSharedData,
account_utils::StateMut,
bpf_loader_upgradeable::{id, UpgradeableLoaderState},
compute_budget::ComputeBudgetInstruction,
instruction::{Instruction, InstructionError},
pubkey::Pubkey,
signature::{Keypair, Signer},
Expand Down Expand Up @@ -35,7 +36,10 @@ pub async fn assert_ix_error(
}

let transaction = Transaction::new_signed_with_payer(
&[ix],
&[
ix,
ComputeBudgetInstruction::set_loaded_accounts_data_size_limit(64 * 1024 * 1024),
],
Some(&fee_payer.pubkey()),
&signers,
recent_blockhash,
Expand Down
23 changes: 16 additions & 7 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use {
},
solana_compute_budget::{
compute_budget::ComputeBudget,
compute_budget_processor::process_compute_budget_instructions,
compute_budget_processor::{process_compute_budget_instructions, ComputeBudgetLimits},
},
solana_ledger::token_balances::collect_token_balances,
solana_program_runtime::{invoke_context::mock_process_instruction, timings::ExecuteTimings},
Expand Down Expand Up @@ -3848,6 +3848,9 @@ fn test_program_fees() {
let fee_structure =
FeeStructure::new(0.000005, 0.0, vec![(200, 0.0000005), (1400000, 0.000005)]);
bank.set_fee_structure(&fee_structure);
let use_default_loaded_accounts_data_size = bank
.feature_set
.is_active(&feature_set::default_loaded_accounts_data_size_limit::id());
let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests();
let mut bank_client = BankClient::new_shared(bank);
let authority_keypair = Keypair::new();
Expand All @@ -3874,9 +3877,12 @@ fn test_program_fees() {
let expected_normal_fee = fee_structure.calculate_fee(
&sanitized_message,
congestion_multiplier,
&process_compute_budget_instructions(sanitized_message.program_instructions_iter())
.unwrap_or_default()
.into(),
&process_compute_budget_instructions(
sanitized_message.program_instructions_iter(),
use_default_loaded_accounts_data_size,
)
.unwrap_or_else(|_| ComputeBudgetLimits::new_with(use_default_loaded_accounts_data_size))
.into(),
false,
true,
);
Expand All @@ -3902,9 +3908,12 @@ fn test_program_fees() {
let expected_prioritized_fee = fee_structure.calculate_fee(
&sanitized_message,
congestion_multiplier,
&process_compute_budget_instructions(sanitized_message.program_instructions_iter())
.unwrap_or_default()
.into(),
&process_compute_budget_instructions(
sanitized_message.program_instructions_iter(),
use_default_loaded_accounts_data_size,
)
.unwrap_or_else(|_| ComputeBudgetLimits::new_with(use_default_loaded_accounts_data_size))
.into(),
false,
true,
);
Expand Down
Loading