Skip to content

Commit

Permalink
Revert "Cap accounts data a transaction can load by its requested lim…
Browse files Browse the repository at this point in the history
…it (#27840)"

This reverts commit 81dc2e5.
  • Loading branch information
tao-stones committed Jan 13, 2023
1 parent 064f163 commit 4690474
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 848 deletions.
20 changes: 2 additions & 18 deletions docs/src/developing/programming-model/runtime.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,8 @@ to.
As the transaction is processed compute units are consumed by its
instruction's programs performing operations such as executing SBF instructions,
calling syscalls, etc... When the transaction consumes its entire budget, or
exceeds a bound such as attempting a call stack that is too deep, or loaded
account data exceeds limit, the runtime halts the transaction processing and
returns an error.
exceeds a bound such as attempting a call stack that is too deep, the runtime
halts the transaction processing and returns an error.

The following operations incur a compute cost:

Expand Down Expand Up @@ -151,21 +150,6 @@ let instruction = ComputeBudgetInstruction::set_compute_unit_limit(300_000);
let instruction = ComputeBudgetInstruction::set_compute_unit_price(1);
```

### Accounts data size limit

A transaction should request the maximum bytes of accounts data it is
allowed to load by including a `SetAccountsDataSizeLimit` instruction, requested
limit is capped by `get_max_loaded_accounts_data_limit()`. If no
`SetAccountsDataSizeLimit` is provided, the transaction is defaulted to
have limit of `get_default_loaded_accounts_data_limit()`.

The `ComputeBudgetInstruction::set_accounts_data_size_limit` function can be used
to create this instruction:

```rust
let instruction = ComputeBudgetInstruction::set_accounts_data_size_limit(100_000);
```

## New Features

As Solana evolves, new features or patches may be introduced that changes the
Expand Down
131 changes: 0 additions & 131 deletions program-runtime/src/compute_budget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,6 @@ pub const DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT: u32 = 200_000;
pub const MAX_COMPUTE_UNIT_LIMIT: u32 = 1_400_000;
const MAX_HEAP_FRAME_BYTES: u32 = 256 * 1024;

/// To change `default` and/or `max` values for `accounts_data_size_limit` in the future,
/// add new enum type here, link to feature gate, and implement the enum in
/// `get_default_loaded_accounts_data_limit()` and/or `get_max_loaded_accounts_data_limit()`.
#[derive(Debug)]
pub enum LoadedAccountsDataLimitType {
V0,
// add future versions here
}

/// Get default value of `ComputeBudget::accounts_data_size_limit` if not set specifically. It
/// sets to 10MB initially, may be changed with feature gate.
const DEFAULT_LOADED_ACCOUNTS_DATA_LIMIT: u32 = 10 * 1024 * 1024;
pub fn get_default_loaded_accounts_data_limit(limit_type: &LoadedAccountsDataLimitType) -> u32 {
match limit_type {
LoadedAccountsDataLimitType::V0 => DEFAULT_LOADED_ACCOUNTS_DATA_LIMIT,
}
}
/// Get max value of `ComputeBudget::accounts_data_size_limit`, it caps value user
/// sets via `ComputeBudgetInstruction::set_compute_unit_limit`. It is set to 100MB
/// initially, can be changed with feature gate.
const MAX_LOADED_ACCOUNTS_DATA_LIMIT: u32 = 100 * 1024 * 1024;
pub fn get_max_loaded_accounts_data_limit(limit_type: &LoadedAccountsDataLimitType) -> u32 {
match limit_type {
LoadedAccountsDataLimitType::V0 => MAX_LOADED_ACCOUNTS_DATA_LIMIT,
}
}

#[cfg(RUSTC_WITH_SPECIALIZATION)]
impl ::solana_frozen_abi::abi_example::AbiExample for ComputeBudget {
fn example() -> Self {
Expand All @@ -55,8 +28,6 @@ pub struct ComputeBudget {
/// allowed to consume. Compute units are consumed by program execution,
/// resources they use, etc...
pub compute_unit_limit: u64,
/// Maximum accounts data size, in bytes, that a transaction is allowed to load
pub accounts_data_size_limit: u64,
/// Number of compute units consumed by a log_u64 call
pub log_64_units: u64,
/// Number of compute units consumed by a create_program_address call
Expand Down Expand Up @@ -148,7 +119,6 @@ impl ComputeBudget {
pub fn new(compute_unit_limit: u64) -> Self {
ComputeBudget {
compute_unit_limit,
accounts_data_size_limit: DEFAULT_LOADED_ACCOUNTS_DATA_LIMIT as u64,
log_64_units: 100,
create_program_address_units: 1500,
invoke_units: 1000,
Expand Down Expand Up @@ -192,14 +162,11 @@ impl ComputeBudget {
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
default_units_per_instruction: bool,
support_request_units_deprecated: bool,
cap_transaction_accounts_data_size: bool,
loaded_accounts_data_limit_type: LoadedAccountsDataLimitType,
) -> Result<PrioritizationFeeDetails, TransactionError> {
let mut num_non_compute_budget_instructions: usize = 0;
let mut updated_compute_unit_limit = None;
let mut requested_heap_size = None;
let mut prioritization_fee = None;
let mut updated_accounts_data_size_limit = None;

for (i, (program_id, instruction)) in instructions.enumerate() {
if compute_budget::check_id(program_id) {
Expand Down Expand Up @@ -243,14 +210,6 @@ impl ComputeBudget {
prioritization_fee =
Some(PrioritizationFeeType::ComputeUnitPrice(micro_lamports));
}
Ok(ComputeBudgetInstruction::SetAccountsDataSizeLimit(bytes))
if cap_transaction_accounts_data_size =>
{
if updated_accounts_data_size_limit.is_some() {
return Err(duplicate_instruction_error);
}
updated_accounts_data_size_limit = Some(bytes);
}
_ => return Err(invalid_instruction_data_error),
}
} else {
Expand Down Expand Up @@ -286,14 +245,6 @@ impl ComputeBudget {
.unwrap_or(MAX_COMPUTE_UNIT_LIMIT)
.min(MAX_COMPUTE_UNIT_LIMIT) as u64;

self.accounts_data_size_limit = updated_accounts_data_size_limit
.unwrap_or_else(|| {
get_default_loaded_accounts_data_limit(&loaded_accounts_data_limit_type)
})
.min(get_max_loaded_accounts_data_limit(
&loaded_accounts_data_limit_type,
)) as u64;

Ok(prioritization_fee
.map(|fee_type| PrioritizationFeeDetails::new(fee_type, self.compute_unit_limit))
.unwrap_or_default())
Expand Down Expand Up @@ -328,8 +279,6 @@ mod tests {
tx.message().program_instructions_iter(),
true,
false, /*not support request_units_deprecated*/
true, /*supports cap transaction accounts data size feature*/
LoadedAccountsDataLimitType::V0,
);
assert_eq!($expected_result, result);
assert_eq!(compute_budget, $expected_budget);
Expand Down Expand Up @@ -595,84 +544,4 @@ mod tests {
ComputeBudget::default()
);
}

#[test]
fn test_process_accounts_data_size_limit_instruction() {
test!(
&[],
Ok(PrioritizationFeeDetails::default()),
ComputeBudget {
compute_unit_limit: 0,
..ComputeBudget::default()
}
);
test!(
&[
ComputeBudgetInstruction::set_accounts_data_size_limit(1),
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
],
Ok(PrioritizationFeeDetails::default()),
ComputeBudget {
compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64,
accounts_data_size_limit: 1,
..ComputeBudget::default()
}
);
test!(
&[
ComputeBudgetInstruction::set_accounts_data_size_limit(
get_max_loaded_accounts_data_limit(&LoadedAccountsDataLimitType::V0) + 1
),
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
],
Ok(PrioritizationFeeDetails::default()),
ComputeBudget {
compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64,
accounts_data_size_limit: get_max_loaded_accounts_data_limit(
&LoadedAccountsDataLimitType::V0
) as u64,
..ComputeBudget::default()
}
);
test!(
&[
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
ComputeBudgetInstruction::set_accounts_data_size_limit(
get_max_loaded_accounts_data_limit(&LoadedAccountsDataLimitType::V0)
),
],
Ok(PrioritizationFeeDetails::default()),
ComputeBudget {
compute_unit_limit: DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64,
accounts_data_size_limit: get_max_loaded_accounts_data_limit(
&LoadedAccountsDataLimitType::V0
) as u64,
..ComputeBudget::default()
}
);
test!(
&[
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
ComputeBudgetInstruction::set_accounts_data_size_limit(1),
],
Ok(PrioritizationFeeDetails::default()),
ComputeBudget {
compute_unit_limit: 3 * DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT as u64,
accounts_data_size_limit: 1,
..ComputeBudget::default()
}
);

test!(
&[
Instruction::new_with_bincode(Pubkey::new_unique(), &0_u8, vec![]),
ComputeBudgetInstruction::set_accounts_data_size_limit(1),
ComputeBudgetInstruction::set_accounts_data_size_limit(1),
],
Err(TransactionError::DuplicateInstruction(2)),
ComputeBudget::default()
);
}
}
4 changes: 2 additions & 2 deletions programs/bpf-loader-tests/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub async fn setup_test_context() -> ProgramTestContext {

pub async fn assert_ix_error(
context: &mut ProgramTestContext,
ixs: &[Instruction],
ix: Instruction,
additional_payer_keypair: Option<&Keypair>,
expected_err: InstructionError,
assertion_failed_msg: &str,
Expand All @@ -36,7 +36,7 @@ pub async fn assert_ix_error(
}

let transaction = Transaction::new_signed_with_payer(
ixs,
&[ix],
Some(&fee_payer.pubkey()),
&signers,
recent_blockhash,
Expand Down
40 changes: 17 additions & 23 deletions programs/bpf-loader-tests/tests/extend_program_ix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use {
solana_sdk::{
account::{AccountSharedData, ReadableAccount, WritableAccount},
bpf_loader_upgradeable::{extend_program, id, UpgradeableLoaderState},
compute_budget::ComputeBudgetInstruction,
instruction::InstructionError,
pubkey::Pubkey,
signature::{Keypair, Signer},
Expand Down Expand Up @@ -104,7 +103,7 @@ async fn test_extend_program_not_upgradeable() {
let payer_address = context.payer.pubkey();
assert_ix_error(
&mut context,
&[extend_program(&program_address, Some(&payer_address), 42)],
extend_program(&program_address, Some(&payer_address), 42),
None,
InstructionError::Immutable,
"should fail because the program data account isn't upgradeable",
Expand Down Expand Up @@ -143,7 +142,7 @@ async fn test_extend_program_by_zero_bytes() {
let payer_address = context.payer.pubkey();
assert_ix_error(
&mut context,
&[extend_program(&program_address, Some(&payer_address), 0)],
extend_program(&program_address, Some(&payer_address), 0),
None,
InstructionError::InvalidInstructionData,
"should fail because the program data account must be extended by more than 0 bytes",
Expand Down Expand Up @@ -182,12 +181,7 @@ async fn test_extend_program_past_max_size() {
let payer_address = context.payer.pubkey();
assert_ix_error(
&mut context,
&[
extend_program(&program_address, Some(&payer_address), 1),
// To request large transaction accounts data size to allow max sized test
// instruction being loaded and processed.
ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX),
],
extend_program(&program_address, Some(&payer_address), 1),
None,
InstructionError::InvalidRealloc,
"should fail because the program data account cannot be extended past the max data size",
Expand Down Expand Up @@ -244,11 +238,11 @@ async fn test_extend_program_with_invalid_payer() {

assert_ix_error(
&mut context,
&[extend_program(
extend_program(
&program_address,
Some(&payer_with_insufficient_funds.pubkey()),
1024,
)],
),
Some(&payer_with_insufficient_funds),
InstructionError::from(SystemError::ResultWithNegativeLamports),
"should fail because the payer has insufficient funds to cover program data account rent",
Expand All @@ -257,11 +251,11 @@ async fn test_extend_program_with_invalid_payer() {

assert_ix_error(
&mut context,
&[extend_program(
extend_program(
&program_address,
Some(&payer_with_invalid_owner.pubkey()),
1,
)],
),
Some(&payer_with_invalid_owner),
InstructionError::ExternalAccountLamportSpend,
"should fail because the payer is not a system account",
Expand All @@ -286,7 +280,7 @@ async fn test_extend_program_with_invalid_payer() {

assert_ix_error(
&mut context,
&[ix],
ix,
None,
InstructionError::PrivilegeEscalation,
"should fail because the payer did not sign",
Expand Down Expand Up @@ -326,7 +320,7 @@ async fn test_extend_program_without_payer() {

assert_ix_error(
&mut context,
&[extend_program(&program_address, None, 1024)],
extend_program(&program_address, None, 1024),
None,
InstructionError::NotEnoughAccountKeys,
"should fail because program data has insufficient funds to cover rent",
Expand Down Expand Up @@ -412,7 +406,7 @@ async fn test_extend_program_with_invalid_system_program() {

assert_ix_error(
&mut context,
&[ix],
ix,
None,
InstructionError::MissingAccount,
"should fail because the system program is missing",
Expand Down Expand Up @@ -465,7 +459,7 @@ async fn test_extend_program_with_mismatch_program_data() {

assert_ix_error(
&mut context,
&[ix],
ix,
None,
InstructionError::InvalidArgument,
"should fail because the program data account doesn't match the program",
Expand Down Expand Up @@ -516,7 +510,7 @@ async fn test_extend_program_with_readonly_program_data() {

assert_ix_error(
&mut context,
&[ix],
ix,
None,
InstructionError::InvalidArgument,
"should fail because the program data account is not writable",
Expand Down Expand Up @@ -554,7 +548,7 @@ async fn test_extend_program_with_invalid_program_data_state() {

assert_ix_error(
&mut context,
&[extend_program(&program_address, Some(&payer_address), 1024)],
extend_program(&program_address, Some(&payer_address), 1024),
None,
InstructionError::InvalidAccountData,
"should fail because the program data account state isn't valid",
Expand Down Expand Up @@ -595,7 +589,7 @@ async fn test_extend_program_with_invalid_program_data_owner() {

assert_ix_error(
&mut context,
&[extend_program(&program_address, Some(&payer_address), 1024)],
extend_program(&program_address, Some(&payer_address), 1024),
None,
InstructionError::InvalidAccountOwner,
"should fail because the program data account owner isn't valid",
Expand Down Expand Up @@ -646,7 +640,7 @@ async fn test_extend_program_with_readonly_program() {

assert_ix_error(
&mut context,
&[ix],
ix,
None,
InstructionError::InvalidArgument,
"should fail because the program account is not writable",
Expand Down Expand Up @@ -686,7 +680,7 @@ async fn test_extend_program_with_invalid_program_owner() {

assert_ix_error(
&mut context,
&[extend_program(&program_address, Some(&payer_address), 1024)],
extend_program(&program_address, Some(&payer_address), 1024),
None,
InstructionError::InvalidAccountOwner,
"should fail because the program account owner isn't valid",
Expand Down Expand Up @@ -726,7 +720,7 @@ async fn test_extend_program_with_invalid_program_state() {

assert_ix_error(
&mut context,
&[extend_program(&program_address, Some(&payer_address), 1024)],
extend_program(&program_address, Some(&payer_address), 1024),
None,
InstructionError::InvalidAccountData,
"should fail because the program account state isn't valid",
Expand Down
Loading

0 comments on commit 4690474

Please sign in to comment.