From 4690474b4c9a9a980081fcf88ed997619f62c54d Mon Sep 17 00:00:00 2001 From: Tao Zhu <82401714+taozhu-chicago@users.noreply.github.com> Date: Tue, 15 Nov 2022 07:59:04 -0600 Subject: [PATCH] Revert "Cap accounts data a transaction can load by its requested limit (#27840)" This reverts commit 81dc2e56ace6705a97ab64e5f360222f0160a3de. --- .../developing/programming-model/runtime.md | 20 +- program-runtime/src/compute_budget.rs | 131 ------- programs/bpf-loader-tests/tests/common.rs | 4 +- .../tests/extend_program_ix.rs | 40 +- programs/sbf/tests/programs.rs | 169 ++++---- runtime/src/accounts.rs | 360 +----------------- runtime/src/bank.rs | 229 ++++------- runtime/src/cost_model.rs | 7 +- runtime/src/transaction_error_metrics.rs | 20 - runtime/src/transaction_priority_details.rs | 5 +- sdk/src/compute_budget.rs | 7 - sdk/src/feature_set.rs | 5 - sdk/src/transaction/error.rs | 10 - storage-proto/proto/transaction_by_addr.proto | 2 - storage-proto/src/convert.rs | 8 - 15 files changed, 169 insertions(+), 848 deletions(-) diff --git a/docs/src/developing/programming-model/runtime.md b/docs/src/developing/programming-model/runtime.md index 89547cbd8a09ae..e626bd8195f97f 100644 --- a/docs/src/developing/programming-model/runtime.md +++ b/docs/src/developing/programming-model/runtime.md @@ -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: @@ -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 diff --git a/program-runtime/src/compute_budget.rs b/program-runtime/src/compute_budget.rs index 249227656cbfa1..30ebed1de534df 100644 --- a/program-runtime/src/compute_budget.rs +++ b/program-runtime/src/compute_budget.rs @@ -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 { @@ -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 @@ -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, @@ -192,14 +162,11 @@ impl ComputeBudget { instructions: impl Iterator, default_units_per_instruction: bool, support_request_units_deprecated: bool, - cap_transaction_accounts_data_size: bool, - loaded_accounts_data_limit_type: LoadedAccountsDataLimitType, ) -> Result { 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) { @@ -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 { @@ -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()) @@ -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); @@ -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() - ); - } } diff --git a/programs/bpf-loader-tests/tests/common.rs b/programs/bpf-loader-tests/tests/common.rs index d2cfb7994bcd8d..e4e6d2eb254f5e 100644 --- a/programs/bpf-loader-tests/tests/common.rs +++ b/programs/bpf-loader-tests/tests/common.rs @@ -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, @@ -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, diff --git a/programs/bpf-loader-tests/tests/extend_program_ix.rs b/programs/bpf-loader-tests/tests/extend_program_ix.rs index 7ce3961d08dbdf..afac48a24e0d35 100644 --- a/programs/bpf-loader-tests/tests/extend_program_ix.rs +++ b/programs/bpf-loader-tests/tests/extend_program_ix.rs @@ -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}, @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 22fc4c8a5df2a3..6c6f6a941dcace 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -11,9 +11,7 @@ use { }, solana_ledger::token_balances::collect_token_balances, solana_program_runtime::{ - compute_budget::{self, ComputeBudget}, - invoke_context::InvokeContext, - timings::ExecuteTimings, + compute_budget::ComputeBudget, invoke_context::InvokeContext, timings::ExecuteTimings, }, solana_runtime::{ bank::{ @@ -2665,17 +2663,13 @@ fn test_program_sbf_realloc() { .send_and_confirm_message( signer, Message::new( - &[ - realloc_extend_and_fill( - &program_id, - &pubkey, - MAX_PERMITTED_DATA_INCREASE, - 1, - &mut bump, - ), - // Request max transaction accounts data size to allow large instruction - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[realloc_extend_and_fill( + &program_id, + &pubkey, + MAX_PERMITTED_DATA_INCREASE, + 1, + &mut bump, + )], Some(&mint_pubkey), ), ) @@ -2693,16 +2687,12 @@ fn test_program_sbf_realloc() { .send_and_confirm_message( signer, Message::new( - &[ - realloc_extend( - &program_id, - &pubkey, - MAX_PERMITTED_DATA_INCREASE, - &mut bump - ), - // Request max transaction accounts data size to allow large instruction - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[realloc_extend( + &program_id, + &pubkey, + MAX_PERMITTED_DATA_INCREASE, + &mut bump + )], Some(&mint_pubkey), ) ) @@ -2716,10 +2706,7 @@ fn test_program_sbf_realloc() { .send_and_confirm_message( signer, Message::new( - &[ - realloc(&program_id, &pubkey, 0, &mut bump), - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[realloc(&program_id, &pubkey, 0, &mut bump)], Some(&mint_pubkey), ), ) @@ -2923,18 +2910,14 @@ fn test_program_sbf_realloc_invoke() { .send_and_confirm_message( signer, Message::new( - &[ - Instruction::new_with_bytes( - realloc_invoke_program_id, - &[INVOKE_REALLOC_MAX_PLUS_ONE], - vec![ - AccountMeta::new(pubkey, false), - AccountMeta::new_readonly(realloc_program_id, false), - ], - ), - // Request max transaction accounts data size to allow large instruction - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[Instruction::new_with_bytes( + realloc_invoke_program_id, + &[INVOKE_REALLOC_MAX_PLUS_ONE], + vec![ + AccountMeta::new(pubkey, false), + AccountMeta::new_readonly(realloc_program_id, false), + ], + )], Some(&mint_pubkey), ) ) @@ -2949,18 +2932,14 @@ fn test_program_sbf_realloc_invoke() { .send_and_confirm_message( signer, Message::new( - &[ - Instruction::new_with_bytes( - realloc_invoke_program_id, - &[INVOKE_REALLOC_MAX_TWICE], - vec![ - AccountMeta::new(pubkey, false), - AccountMeta::new_readonly(realloc_program_id, false), - ], - ), - // Request max transaction accounts data size to allow large instruction - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[Instruction::new_with_bytes( + realloc_invoke_program_id, + &[INVOKE_REALLOC_MAX_TWICE], + vec![ + AccountMeta::new(pubkey, false), + AccountMeta::new_readonly(realloc_program_id, false), + ], + )], Some(&mint_pubkey), ) ) @@ -3183,18 +3162,14 @@ fn test_program_sbf_realloc_invoke() { .send_and_confirm_message( signer, Message::new( - &[ - Instruction::new_with_bytes( - realloc_invoke_program_id, - &[INVOKE_REALLOC_MAX_INVOKE_MAX], - vec![ - AccountMeta::new(invoke_pubkey, false), - AccountMeta::new_readonly(realloc_program_id, false), - ], - ), - // Request max transaction accounts data size to allow large instruction - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[Instruction::new_with_bytes( + realloc_invoke_program_id, + &[INVOKE_REALLOC_MAX_INVOKE_MAX], + vec![ + AccountMeta::new(invoke_pubkey, false), + AccountMeta::new_readonly(realloc_program_id, false), + ], + )], Some(&mint_pubkey), ) ) @@ -3256,19 +3231,15 @@ fn test_program_sbf_realloc_invoke() { .send_and_confirm_message( signer, Message::new( - &[ - Instruction::new_with_bytes( - realloc_invoke_program_id, - &[INVOKE_INVOKE_MAX_TWICE], - vec![ - AccountMeta::new(invoke_pubkey, false), - AccountMeta::new_readonly(realloc_invoke_program_id, false), - AccountMeta::new_readonly(realloc_program_id, false), - ], - ), - // Request max transaction accounts data size to allow large instruction - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[Instruction::new_with_bytes( + realloc_invoke_program_id, + &[INVOKE_INVOKE_MAX_TWICE], + vec![ + AccountMeta::new(invoke_pubkey, false), + AccountMeta::new_readonly(realloc_invoke_program_id, false), + AccountMeta::new_readonly(realloc_program_id, false), + ], + )], Some(&mint_pubkey), ) ) @@ -3296,18 +3267,14 @@ fn test_program_sbf_realloc_invoke() { .send_and_confirm_message( signer, Message::new( - &[ - Instruction::new_with_bytes( - realloc_invoke_program_id, - &[INVOKE_REALLOC_EXTEND_MAX, 1, i as u8, (i / 255) as u8], - vec![ - AccountMeta::new(pubkey, false), - AccountMeta::new_readonly(realloc_program_id, false), - ], - ), - // Request max transaction accounts data size to allow large instruction - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[Instruction::new_with_bytes( + realloc_invoke_program_id, + &[INVOKE_REALLOC_EXTEND_MAX, 1, i as u8, (i / 255) as u8], + vec![ + AccountMeta::new(pubkey, false), + AccountMeta::new_readonly(realloc_program_id, false), + ], + )], Some(&mint_pubkey), ), ) @@ -3325,18 +3292,14 @@ fn test_program_sbf_realloc_invoke() { .send_and_confirm_message( signer, Message::new( - &[ - Instruction::new_with_bytes( - realloc_invoke_program_id, - &[INVOKE_REALLOC_EXTEND_MAX, 2, 1, 1], - vec![ - AccountMeta::new(pubkey, false), - AccountMeta::new_readonly(realloc_program_id, false), - ], - ), - // Request max transaction accounts data size to allow large instruction - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[Instruction::new_with_bytes( + realloc_invoke_program_id, + &[INVOKE_REALLOC_EXTEND_MAX, 2, 1, 1], + vec![ + AccountMeta::new(pubkey, false), + AccountMeta::new_readonly(realloc_program_id, false), + ], + )], Some(&mint_pubkey), ) ) @@ -3501,8 +3464,6 @@ fn test_program_fees() { &fee_structure, true, false, - false, - compute_budget::LoadedAccountsDataLimitType::V0, ); bank_client .send_and_confirm_message(&[&mint_keypair], message) @@ -3525,8 +3486,6 @@ fn test_program_fees() { &fee_structure, true, false, - false, - compute_budget::LoadedAccountsDataLimitType::V0, ); assert!(expected_normal_fee < expected_prioritized_fee); diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 2a725129ab6fcb..612e3b30e97d03 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -25,15 +25,14 @@ use { dashmap::DashMap, log::*, solana_address_lookup_table_program::{error::AddressLookupError, state::AddressLookupTable}, - solana_program_runtime::compute_budget::ComputeBudget, solana_sdk::{ account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, account_utils::StateMut, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::{BankId, Slot}, feature_set::{ - self, cap_transaction_accounts_data_size, remove_deprecated_request_unit_ix, - use_default_units_in_fee_calculation, FeatureSet, + self, remove_deprecated_request_unit_ix, use_default_units_in_fee_calculation, + FeatureSet, }, fee::FeeStructure, genesis_config::ClusterType, @@ -57,7 +56,6 @@ use { std::{ cmp::Reverse, collections::{hash_map, BinaryHeap, HashMap, HashSet}, - num::NonZeroUsize, ops::RangeBounds, path::PathBuf, sync::{ @@ -262,15 +260,6 @@ impl Accounts { let account_keys = message.account_keys(); let mut account_deps = Vec::with_capacity(account_keys.len()); let mut rent_debits = RentDebits::default(); - let requested_loaded_accounts_data_size_limit = - if feature_set.is_active(&feature_set::cap_transaction_accounts_data_size::id()) { - let requested_loaded_accounts_data_size = - Self::get_requested_loaded_accounts_data_size_limit(tx, feature_set)?; - Some(requested_loaded_accounts_data_size) - } else { - None - }; - let mut accumulated_accounts_data_size: usize = 0; let set_exempt_rent_epoch_max = feature_set.is_active(&solana_sdk::feature_set::set_exempt_rent_epoch_max::id()); @@ -278,20 +267,16 @@ impl Accounts { .iter() .enumerate() .map(|(i, key)| { - let (account, loaded_programdata_account_size) = if !message.is_non_loader_key(i) { + let account = if !message.is_non_loader_key(i) { // Fill in an empty account for the program slots. - (AccountSharedData::default(), 0) + AccountSharedData::default() } else { #[allow(clippy::collapsible_else_if)] if solana_sdk::sysvar::instructions::check_id(key) { - ( - Self::construct_instructions_account( - message, - feature_set.is_active( - &feature_set::instructions_sysvar_owned_by_sysvar::id(), - ), - ), - 0, + Self::construct_instructions_account( + message, + feature_set + .is_active(&feature_set::instructions_sysvar_owned_by_sysvar::id()), ) } else { let (mut account, rent) = if let Some(account_override) = @@ -346,7 +331,6 @@ impl Accounts { validated_fee_payer = true; } - let mut loaded_programdata_account_size: usize = 0; if bpf_loader_upgradeable::check_id(account.owner()) { if message.is_writable(i) && !message.is_upgradeable_loader_present() { error_counters.invalid_writable_account += 1; @@ -363,8 +347,6 @@ impl Accounts { .accounts_db .load_with_fixed_root(ancestors, &programdata_address) { - loaded_programdata_account_size = - programdata_account.data().len(); account_deps .push((programdata_address, programdata_account)); } else { @@ -384,18 +366,9 @@ impl Accounts { tx_rent += rent; rent_debits.insert(key, rent, account.lamports()); - (account, loaded_programdata_account_size) + account } }; - Self::accumulate_and_check_loaded_account_data_size( - &mut accumulated_accounts_data_size, - account - .data() - .len() - .saturating_add(loaded_programdata_account_size), - requested_loaded_accounts_data_size_limit, - error_counters, - )?; Ok((*key, account)) }) @@ -418,8 +391,6 @@ impl Accounts { &mut accounts, instruction.program_id_index as IndexOfAccount, error_counters, - &mut accumulated_accounts_data_size, - requested_loaded_accounts_data_size_limit, ) }) .collect::>>>()?; @@ -436,22 +407,6 @@ impl Accounts { } } - fn get_requested_loaded_accounts_data_size_limit( - tx: &SanitizedTransaction, - feature_set: &FeatureSet, - ) -> Result { - let mut compute_budget = ComputeBudget::default(); - let _prioritization_fee_details = compute_budget.process_instructions( - tx.message().program_instructions_iter(), - feature_set.is_active(&use_default_units_in_fee_calculation::id()), - !feature_set.is_active(&remove_deprecated_request_unit_ix::id()), - feature_set.is_active(&cap_transaction_accounts_data_size::id()), - Bank::get_loaded_accounts_data_limit_type(feature_set), - )?; - NonZeroUsize::new(compute_budget.accounts_data_size_limit as usize) - .ok_or(TransactionError::InvalidLoadedAccountsDataSizeLimit) - } - fn validate_fee_payer( payer_address: &Pubkey, payer_account: &mut AccountSharedData, @@ -504,22 +459,15 @@ impl Accounts { accounts: &mut Vec, mut program_account_index: IndexOfAccount, error_counters: &mut TransactionErrorMetrics, - accumulated_accounts_data_size: &mut usize, - requested_loaded_accounts_data_size_limit: Option, ) -> Result> { let mut account_indices = Vec::new(); - let (mut program_id, already_loaded_as_non_loader) = - match accounts.get(program_account_index as usize) { - Some(program_account) => ( - program_account.0, - // program account is already loaded if it's not empty in `accounts` - program_account.1 != AccountSharedData::default(), - ), - None => { - error_counters.account_not_found += 1; - return Err(TransactionError::ProgramAccountNotFound); - } - }; + let mut program_id = match accounts.get(program_account_index as usize) { + Some(program_account) => program_account.0, + None => { + error_counters.account_not_found += 1; + return Err(TransactionError::ProgramAccountNotFound); + } + }; let mut depth = 0; while !native_loader::check_id(&program_id) { if depth >= 5 { @@ -527,7 +475,6 @@ impl Accounts { return Err(TransactionError::CallChainTooDeep); } depth += 1; - let mut loaded_account_total_size: usize = 0; program_account_index = match self .accounts_db @@ -535,13 +482,6 @@ impl Accounts { { Some((program_account, _)) => { let account_index = accounts.len() as IndexOfAccount; - // do not double count account size for program account on top of call chain - // that has already been loaded during load_transaction as non-loader account. - // Other accounts data size in the call chain are counted. - if !(depth == 1 && already_loaded_as_non_loader) { - loaded_account_total_size = - loaded_account_total_size.saturating_add(program_account.data().len()); - } accounts.push((program_id, program_account)); account_index } @@ -559,7 +499,6 @@ impl Accounts { // Add loader to chain let program_owner = *program.owner(); account_indices.insert(0, program_account_index); - if bpf_loader_upgradeable::check_id(&program_owner) { // The upgradeable loader requires the derived ProgramData account if let Ok(UpgradeableLoaderState::Program { @@ -572,10 +511,6 @@ impl Accounts { { Some((programdata_account, _)) => { let account_index = accounts.len() as IndexOfAccount; - if !(depth == 1 && already_loaded_as_non_loader) { - loaded_account_total_size = loaded_account_total_size - .saturating_add(programdata_account.data().len()); - } accounts.push((programdata_address, programdata_account)); account_index } @@ -590,43 +525,12 @@ impl Accounts { return Err(TransactionError::InvalidProgramForExecution); } } - Self::accumulate_and_check_loaded_account_data_size( - accumulated_accounts_data_size, - loaded_account_total_size, - requested_loaded_accounts_data_size_limit, - error_counters, - )?; program_id = program_owner; } Ok(account_indices) } - /// Accumulate loaded account data size into `accumulated_accounts_data_size`. - /// Returns TransactionErr::MaxLoadedAccountsDataSizeExceeded if - /// `requested_loaded_accounts_data_size_limit` is specified and - /// `accumulated_accounts_data_size` exceeds it. - fn accumulate_and_check_loaded_account_data_size( - accumulated_loaded_accounts_data_size: &mut usize, - account_data_size: usize, - requested_loaded_accounts_data_size_limit: Option, - error_counters: &mut TransactionErrorMetrics, - ) -> Result<()> { - if let Some(requested_loaded_accounts_data_size) = requested_loaded_accounts_data_size_limit - { - *accumulated_loaded_accounts_data_size = - accumulated_loaded_accounts_data_size.saturating_add(account_data_size); - if *accumulated_loaded_accounts_data_size > requested_loaded_accounts_data_size.get() { - error_counters.max_loaded_accounts_data_size_exceeded += 1; - Err(TransactionError::MaxLoadedAccountsDataSizeExceeded) - } else { - Ok(()) - } - } else { - Ok(()) - } - } - #[allow(clippy::too_many_arguments)] pub fn load_accounts( &self, @@ -657,8 +561,6 @@ impl Accounts { fee_structure, feature_set.is_active(&use_default_units_in_fee_calculation::id()), !feature_set.is_active(&remove_deprecated_request_unit_ix::id()), - feature_set.is_active(&cap_transaction_accounts_data_size::id()), - Bank::get_loaded_accounts_data_limit_type(feature_set), ) } else { return (Err(TransactionError::BlockhashNotFound), None); @@ -1461,7 +1363,7 @@ mod tests { }, assert_matches::assert_matches, solana_address_lookup_table_program::state::LookupTableMeta, - solana_program_runtime::{compute_budget, executor_cache::TransactionExecutorCache}, + solana_program_runtime::executor_cache::TransactionExecutorCache, solana_sdk::{ account::{AccountSharedData, WritableAccount}, epoch_schedule::EpochSchedule, @@ -1737,8 +1639,6 @@ mod tests { &FeeStructure::default(), true, false, - true, - compute_budget::LoadedAccountsDataLimitType::V0, ); assert_eq!(fee, lamports_per_signature); @@ -2564,8 +2464,6 @@ mod tests { &mut vec![(keypair.pubkey(), account)], 0, &mut error_counters, - &mut 0, - None, ), Err(TransactionError::ProgramAccountNotFound) ); @@ -3995,228 +3893,4 @@ mod tests { )); } } - - #[test] - fn test_accumulate_and_check_loaded_account_data_size() { - let mut error_counter = TransactionErrorMetrics::default(); - - // assert check is OK if data limit is not enabled - { - let mut accumulated_data_size: usize = 0; - let data_size = usize::MAX; - let requested_data_size_limit = None; - - assert!(Accounts::accumulate_and_check_loaded_account_data_size( - &mut accumulated_data_size, - data_size, - requested_data_size_limit, - &mut error_counter - ) - .is_ok()); - } - - // assert accounts are accumulated and check properly - { - let mut accumulated_data_size: usize = 0; - let data_size: usize = 123; - let requested_data_size_limit = NonZeroUsize::new(data_size); - - // OK - assert!(Accounts::accumulate_and_check_loaded_account_data_size( - &mut accumulated_data_size, - data_size, - requested_data_size_limit, - &mut error_counter - ) - .is_ok()); - assert_eq!(data_size, accumulated_data_size); - - // exceeds - let another_byte: usize = 1; - assert_eq!( - Accounts::accumulate_and_check_loaded_account_data_size( - &mut accumulated_data_size, - another_byte, - requested_data_size_limit, - &mut error_counter - ), - Err(TransactionError::MaxLoadedAccountsDataSizeExceeded) - ); - } - } - - #[test] - fn test_load_executable_accounts() { - solana_logger::setup(); - let accounts = Accounts::new_with_config_for_tests( - Vec::new(), - &ClusterType::Development, - AccountSecondaryIndexes::default(), - AccountShrinkThreshold::default(), - ); - let mut error_counters = TransactionErrorMetrics::default(); - let ancestors = vec![(0, 0)].into_iter().collect(); - - let space: usize = 9; - let keypair = Keypair::new(); - let mut account = AccountSharedData::new(1, space, &native_loader::id()); - account.set_executable(true); - accounts.store_for_tests(0, &keypair.pubkey(), &account); - - let mut accumulated_accounts_data_size: usize = 0; - let mut expect_accumulated_accounts_data_size: usize; - - // test: program account has been loaded as non-loader, load_executable_accounts - // will not double count its data size - { - let mut loaded_accounts = vec![(keypair.pubkey(), account)]; - accumulated_accounts_data_size += space; - expect_accumulated_accounts_data_size = accumulated_accounts_data_size; - assert!(accounts - .load_executable_accounts( - &ancestors, - &mut loaded_accounts, - 0, - &mut error_counters, - &mut accumulated_accounts_data_size, - NonZeroUsize::new(expect_accumulated_accounts_data_size), - ) - .is_ok()); - assert_eq!( - expect_accumulated_accounts_data_size, - accumulated_accounts_data_size - ); - } - - // test: program account has not been loaded cause it's loader, load_executable_accounts - // will accumulate its data size - { - let mut loaded_accounts = vec![(keypair.pubkey(), AccountSharedData::default())]; - expect_accumulated_accounts_data_size = accumulated_accounts_data_size + space; - assert!(accounts - .load_executable_accounts( - &ancestors, - &mut loaded_accounts, - 0, - &mut error_counters, - &mut accumulated_accounts_data_size, - NonZeroUsize::new(expect_accumulated_accounts_data_size), - ) - .is_ok()); - assert_eq!( - expect_accumulated_accounts_data_size, - accumulated_accounts_data_size - ); - } - - // test: try to load one more account will accumulate additional `space` bytes, therefore - // exceed limit of `expect_accumulated_accounts_data_size` set above. - { - let mut loaded_accounts = vec![(keypair.pubkey(), AccountSharedData::default())]; - assert_eq!( - accounts.load_executable_accounts( - &ancestors, - &mut loaded_accounts, - 0, - &mut error_counters, - &mut accumulated_accounts_data_size, - NonZeroUsize::new(expect_accumulated_accounts_data_size), - ), - Err(TransactionError::MaxLoadedAccountsDataSizeExceeded) - ); - } - } - - #[test] - fn test_load_executable_accounts_with_upgradeable_program() { - solana_logger::setup(); - let accounts = Accounts::new_with_config_for_tests( - Vec::new(), - &ClusterType::Development, - AccountSecondaryIndexes::default(), - AccountShrinkThreshold::default(), - ); - let mut error_counters = TransactionErrorMetrics::default(); - let ancestors = vec![(0, 0)].into_iter().collect(); - - // call chain: native_loader -> key0 -> upgradeable bpf loader -> program_key (that has programdata_key) - let programdata_key = Pubkey::new(&[3u8; 32]); - let program_data = UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: None, - }; - let mut program_data_account = - AccountSharedData::new_data(1, &program_data, &Pubkey::default()).unwrap(); - let program_data_account_size = program_data_account.data().len(); - program_data_account.set_executable(true); - program_data_account.set_rent_epoch(0); - accounts.store_for_tests(0, &programdata_key, &program_data_account); - - let program_key = Pubkey::new(&[2u8; 32]); - let program = UpgradeableLoaderState::Program { - programdata_address: programdata_key, - }; - let mut program_account = - AccountSharedData::new_data(1, &program, &bpf_loader_upgradeable::id()).unwrap(); - let program_account_size = program_account.data().len(); - program_account.set_executable(true); - program_account.set_rent_epoch(0); - accounts.store_for_tests(0, &program_key, &program_account); - - let key0 = Pubkey::new(&[1u8; 32]); - let mut bpf_loader_account = AccountSharedData::new(1, 0, &key0); - bpf_loader_account.set_executable(true); - accounts.store_for_tests(0, &bpf_loader_upgradeable::id(), &bpf_loader_account); - - let space: usize = 9; - let mut account = AccountSharedData::new(1, space, &native_loader::id()); - account.set_executable(true); - accounts.store_for_tests(0, &key0, &account); - - // test: program_key account has been loaded as non-loader, load_executable_accounts - // will not double count its data size, but still accumulate data size from - // callchain - { - let expect_accumulated_accounts_data_size: usize = space; - let mut accumulated_accounts_data_size: usize = 0; - let mut loaded_accounts = vec![(program_key, program_account)]; - assert!(accounts - .load_executable_accounts( - &ancestors, - &mut loaded_accounts, - 0, - &mut error_counters, - &mut accumulated_accounts_data_size, - NonZeroUsize::new(expect_accumulated_accounts_data_size), - ) - .is_ok()); - assert_eq!( - expect_accumulated_accounts_data_size, - accumulated_accounts_data_size - ); - } - - // test: program account has not been loaded cause it's loader, load_executable_accounts - // will accumulate entire callchain data size. - { - let mut loaded_accounts = vec![(program_key, AccountSharedData::default())]; - let mut accumulated_accounts_data_size: usize = 0; - let expect_accumulated_accounts_data_size = - program_account_size + program_data_account_size + space; - assert!(accounts - .load_executable_accounts( - &ancestors, - &mut loaded_accounts, - 0, - &mut error_counters, - &mut accumulated_accounts_data_size, - NonZeroUsize::new(expect_accumulated_accounts_data_size), - ) - .is_ok()); - assert_eq!( - expect_accumulated_accounts_data_size, - accumulated_accounts_data_size - ); - } - } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 6c9743cbc7af02..d011e3ccda9dcc 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -119,9 +119,8 @@ use { epoch_schedule::EpochSchedule, feature, feature_set::{ - self, cap_transaction_accounts_data_size, disable_fee_calculator, - enable_early_verification_of_account_modifications, remove_deprecated_request_unit_ix, - use_default_units_in_fee_calculation, FeatureSet, + self, disable_fee_calculator, enable_early_verification_of_account_modifications, + remove_deprecated_request_unit_ix, use_default_units_in_fee_calculation, FeatureSet, }, fee::FeeStructure, fee_calculator::{FeeCalculator, FeeRateGovernor}, @@ -279,7 +278,7 @@ impl RentDebits { } pub type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "3qia1Zm8X66bzFaBuC8ahz3hADRRATyUPRV36ZzrSois")] +#[frozen_abi(digest = "A7T7XohiSoo8FGoCPTsaXAYYugXTkoYnBjQAdBgYHH85")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. @@ -3439,9 +3438,6 @@ impl Bank { !self .feature_set .is_active(&remove_deprecated_request_unit_ix::id()), - self.feature_set - .is_active(&cap_transaction_accounts_data_size::id()), - Self::get_loaded_accounts_data_limit_type(&self.feature_set), )) } @@ -3486,9 +3482,6 @@ impl Bank { !self .feature_set .is_active(&remove_deprecated_request_unit_ix::id()), - self.feature_set - .is_active(&cap_transaction_accounts_data_size::id()), - Self::get_loaded_accounts_data_limit_type(&self.feature_set), ) } @@ -4395,9 +4388,6 @@ impl Bank { !self .feature_set .is_active(&remove_deprecated_request_unit_ix::id()), - self.feature_set - .is_active(&cap_transaction_accounts_data_size::id()), - Self::get_loaded_accounts_data_limit_type(&self.feature_set), ); compute_budget_process_transaction_time.stop(); saturating_add_assign!( @@ -4690,8 +4680,6 @@ impl Bank { fee_structure: &FeeStructure, use_default_units_per_instruction: bool, support_request_units_deprecated: bool, - cap_transaction_accounts_data_size: bool, - loaded_accounts_data_limit_type: compute_budget::LoadedAccountsDataLimitType, ) -> u64 { // Fee based on compute units and signatures const BASE_CONGESTION: f64 = 5_000.0; @@ -4708,8 +4696,6 @@ impl Bank { message.program_instructions_iter(), use_default_units_per_instruction, support_request_units_deprecated, - cap_transaction_accounts_data_size, - loaded_accounts_data_limit_type, ) .unwrap_or_default(); let prioritization_fee = prioritization_fee_details.get_fee(); @@ -4778,9 +4764,6 @@ impl Bank { !self .feature_set .is_active(&remove_deprecated_request_unit_ix::id()), - self.feature_set - .is_active(&cap_transaction_accounts_data_size::id()), - Self::get_loaded_accounts_data_limit_type(&self.feature_set), ); // In case of instruction error, even though no accounts @@ -7756,17 +7739,6 @@ impl Bank { &mut error_counters, ) } - - /// if the `default` and/or `max` value for ComputeBudget:::Accounts_data_size_limit changes, - /// the change needs to be gated by feature gate with corresponding new enum value. - /// should use this function to get correct loaded_accounts_data_limit_type based on - /// feature_set. - pub fn get_loaded_accounts_data_limit_type( - _feature_set: &FeatureSet, - ) -> compute_budget::LoadedAccountsDataLimitType { - compute_budget::LoadedAccountsDataLimitType::V0 - // In the future, use feature_set to determine correct LoadedAccountsDataLimitType here. - } } /// Compute how much an account has changed size. This function is useful when the data size delta @@ -10956,8 +10928,6 @@ pub(crate) mod tests { &FeeStructure::default(), true, false, - true, - compute_budget::LoadedAccountsDataLimitType::V0, ); let (expected_fee_collected, expected_fee_burned) = @@ -11139,8 +11109,6 @@ pub(crate) mod tests { &FeeStructure::default(), true, false, - true, - compute_budget::LoadedAccountsDataLimitType::V0, ); assert_eq!( bank.get_balance(&mint_keypair.pubkey()), @@ -11159,8 +11127,6 @@ pub(crate) mod tests { &FeeStructure::default(), true, false, - true, - compute_budget::LoadedAccountsDataLimitType::V0, ); assert_eq!( bank.get_balance(&mint_keypair.pubkey()), @@ -11277,8 +11243,6 @@ pub(crate) mod tests { &FeeStructure::default(), true, false, - true, - compute_budget::LoadedAccountsDataLimitType::V0, ) * 2 ) .0 @@ -18454,42 +18418,34 @@ pub(crate) mod tests { // Default: no fee. let message = SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(); - for cap_transaction_accounts_data_size in &[true, false] { - assert_eq!( - Bank::calculate_fee( - &message, - 0, - &FeeStructure { - lamports_per_signature: 0, - ..FeeStructure::default() - }, - true, - false, - *cap_transaction_accounts_data_size, - compute_budget::LoadedAccountsDataLimitType::V0, - ), - 0 - ); - } + assert_eq!( + Bank::calculate_fee( + &message, + 0, + &FeeStructure { + lamports_per_signature: 0, + ..FeeStructure::default() + }, + true, + false, + ), + 0 + ); // One signature, a fee. - for cap_transaction_accounts_data_size in &[true, false] { - assert_eq!( - Bank::calculate_fee( - &message, - 1, - &FeeStructure { - lamports_per_signature: 1, - ..FeeStructure::default() - }, - true, - false, - *cap_transaction_accounts_data_size, - compute_budget::LoadedAccountsDataLimitType::V0, - ), - 1 - ); - } + assert_eq!( + Bank::calculate_fee( + &message, + 1, + &FeeStructure { + lamports_per_signature: 1, + ..FeeStructure::default() + }, + true, + false, + ), + 1 + ); // Two signatures, double the fee. let key0 = Pubkey::new_unique(); @@ -18497,23 +18453,19 @@ pub(crate) mod tests { let ix0 = system_instruction::transfer(&key0, &key1, 1); let ix1 = system_instruction::transfer(&key1, &key0, 1); let message = SanitizedMessage::try_from(Message::new(&[ix0, ix1], Some(&key0))).unwrap(); - for cap_transaction_accounts_data_size in &[true, false] { - assert_eq!( - Bank::calculate_fee( - &message, - 2, - &FeeStructure { - lamports_per_signature: 2, - ..FeeStructure::default() - }, - true, - false, - *cap_transaction_accounts_data_size, - compute_budget::LoadedAccountsDataLimitType::V0, - ), - 4 - ); - } + assert_eq!( + Bank::calculate_fee( + &message, + 2, + &FeeStructure { + lamports_per_signature: 2, + ..FeeStructure::default() + }, + true, + false, + ), + 4 + ); } #[test] @@ -18529,20 +18481,10 @@ pub(crate) mod tests { let message = SanitizedMessage::try_from(Message::new(&[], Some(&Pubkey::new_unique()))).unwrap(); - for cap_transaction_accounts_data_size in &[true, false] { - assert_eq!( - Bank::calculate_fee( - &message, - 1, - &fee_structure, - true, - false, - *cap_transaction_accounts_data_size, - compute_budget::LoadedAccountsDataLimitType::V0 - ), - max_fee + lamports_per_signature - ); - } + assert_eq!( + Bank::calculate_fee(&message, 1, &fee_structure, true, false), + max_fee + lamports_per_signature + ); // Three signatures, two instructions, no unit request @@ -18551,20 +18493,10 @@ pub(crate) mod tests { let message = SanitizedMessage::try_from(Message::new(&[ix0, ix1], Some(&Pubkey::new_unique()))) .unwrap(); - for cap_transaction_accounts_data_size in &[true, false] { - assert_eq!( - Bank::calculate_fee( - &message, - 1, - &fee_structure, - true, - false, - *cap_transaction_accounts_data_size, - compute_budget::LoadedAccountsDataLimitType::V0 - ), - max_fee + 3 * lamports_per_signature - ); - } + assert_eq!( + Bank::calculate_fee(&message, 1, &fee_structure, true, false), + max_fee + 3 * lamports_per_signature + ); // Explicit fee schedule @@ -18595,21 +18527,11 @@ pub(crate) mod tests { Some(&Pubkey::new_unique()), )) .unwrap(); - for cap_transaction_accounts_data_size in &[true, false] { - let fee = Bank::calculate_fee( - &message, - 1, - &fee_structure, - true, - false, - *cap_transaction_accounts_data_size, - compute_budget::LoadedAccountsDataLimitType::V0, - ); - assert_eq!( - fee, - lamports_per_signature + prioritization_fee_details.get_fee() - ); - } + let fee = Bank::calculate_fee(&message, 1, &fee_structure, true, false); + assert_eq!( + fee, + lamports_per_signature + prioritization_fee_details.get_fee() + ); } } @@ -18643,20 +18565,10 @@ pub(crate) mod tests { Some(&key0), )) .unwrap(); - for cap_transaction_accounts_data_size in &[true, false] { - assert_eq!( - Bank::calculate_fee( - &message, - 1, - &fee_structure, - true, - false, - *cap_transaction_accounts_data_size, - compute_budget::LoadedAccountsDataLimitType::V0 - ), - 2 - ); - } + assert_eq!( + Bank::calculate_fee(&message, 1, &fee_structure, true, false), + 2 + ); secp_instruction1.data = vec![0]; secp_instruction2.data = vec![10]; @@ -18665,20 +18577,10 @@ pub(crate) mod tests { Some(&key0), )) .unwrap(); - for cap_transaction_accounts_data_size in &[true, false] { - assert_eq!( - Bank::calculate_fee( - &message, - 1, - &fee_structure, - true, - false, - *cap_transaction_accounts_data_size, - compute_budget::LoadedAccountsDataLimitType::V0 - ), - 11 - ); - } + assert_eq!( + Bank::calculate_fee(&message, 1, &fee_structure, true, false), + 11 + ); } #[test] @@ -19670,10 +19572,7 @@ pub(crate) mod tests { account_metas, ); Transaction::new_signed_with_payer( - &[ - instruction, - ComputeBudgetInstruction::set_accounts_data_size_limit(u32::MAX), - ], + &[instruction], Some(&payer.pubkey()), &[payer], recent_blockhash, diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index 0d181ae5916dac..48fbf981174202 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -6,7 +6,7 @@ //! use { - crate::{bank::Bank, block_cost_limits::*}, + crate::block_cost_limits::*, log::*, solana_program_runtime::compute_budget::{ ComputeBudget, DEFAULT_INSTRUCTION_COMPUTE_UNIT_LIMIT, @@ -14,8 +14,7 @@ use { solana_sdk::{ compute_budget, feature_set::{ - cap_transaction_accounts_data_size, remove_deprecated_request_unit_ix, - use_default_units_in_fee_calculation, FeatureSet, + remove_deprecated_request_unit_ix, use_default_units_in_fee_calculation, FeatureSet, }, instruction::CompiledInstruction, program_utils::limited_deserialize, @@ -152,8 +151,6 @@ impl CostModel { transaction.message().program_instructions_iter(), feature_set.is_active(&use_default_units_in_fee_calculation::id()), !feature_set.is_active(&remove_deprecated_request_unit_ix::id()), - feature_set.is_active(&cap_transaction_accounts_data_size::id()), - Bank::get_loaded_accounts_data_limit_type(feature_set), ); // if tx contained user-space instructions and a more accurate estimate available correct it diff --git a/runtime/src/transaction_error_metrics.rs b/runtime/src/transaction_error_metrics.rs index 438112cdd5cfd7..baa25a07364839 100644 --- a/runtime/src/transaction_error_metrics.rs +++ b/runtime/src/transaction_error_metrics.rs @@ -23,8 +23,6 @@ pub struct TransactionErrorMetrics { pub would_exceed_max_account_cost_limit: usize, pub would_exceed_max_vote_cost_limit: usize, pub would_exceed_account_data_block_limit: usize, - pub max_loaded_accounts_data_size_exceeded: usize, - pub invalid_loaded_accounts_data_size_limit: usize, } impl TransactionErrorMetrics { @@ -78,14 +76,6 @@ impl TransactionErrorMetrics { self.would_exceed_account_data_block_limit, other.would_exceed_account_data_block_limit ); - saturating_add_assign!( - self.max_loaded_accounts_data_size_exceeded, - other.max_loaded_accounts_data_size_exceeded - ); - saturating_add_assign!( - self.invalid_loaded_accounts_data_size_limit, - other.invalid_loaded_accounts_data_size_limit - ); } pub fn report(&self, id: u32, slot: Slot) { @@ -162,16 +152,6 @@ impl TransactionErrorMetrics { self.would_exceed_account_data_block_limit as i64, i64 ), - ( - "max_loaded_accounts_data_size_exceeded", - self.max_loaded_accounts_data_size_exceeded as i64, - i64 - ), - ( - "invalid_loaded_accounts_data_size_limit", - self.invalid_loaded_accounts_data_size_limit as i64, - i64 - ), ); } } diff --git a/runtime/src/transaction_priority_details.rs b/runtime/src/transaction_priority_details.rs index 6a67bf7bab2ca5..2337dc24cedaed 100644 --- a/runtime/src/transaction_priority_details.rs +++ b/runtime/src/transaction_priority_details.rs @@ -1,5 +1,5 @@ use { - solana_program_runtime::compute_budget::{self, ComputeBudget}, + solana_program_runtime::compute_budget::ComputeBudget, solana_sdk::{ instruction::CompiledInstruction, pubkey::Pubkey, @@ -25,9 +25,6 @@ pub trait GetTransactionPriorityDetails { instructions, true, // use default units per instruction false, // stop supporting prioritization by request_units_deprecated instruction - false, //transaction priority doesn't care about accounts data size limit, - compute_budget::LoadedAccountsDataLimitType::V0, // transaction priority doesn't - // care about accounts data size limit. ) .ok()?; Some(TransactionPriorityDetails { diff --git a/sdk/src/compute_budget.rs b/sdk/src/compute_budget.rs index ec81b10f2025cd..b51325aa5c0364 100644 --- a/sdk/src/compute_budget.rs +++ b/sdk/src/compute_budget.rs @@ -41,8 +41,6 @@ pub enum ComputeBudgetInstruction { /// Set a compute unit price in "micro-lamports" to pay a higher transaction /// fee for higher transaction prioritization. SetComputeUnitPrice(u64), - /// Set a specific transaction-wide account data size limit, in bytes, is allowed to allocate. - SetAccountsDataSizeLimit(u32), } impl ComputeBudgetInstruction { @@ -61,11 +59,6 @@ impl ComputeBudgetInstruction { Instruction::new_with_borsh(id(), &Self::SetComputeUnitPrice(micro_lamports), vec![]) } - /// Create a `ComputeBudgetInstruction::SetAccountsDataSizeLimit` `Instruction` - pub fn set_accounts_data_size_limit(bytes: u32) -> Instruction { - Instruction::new_with_borsh(id(), &Self::SetAccountsDataSizeLimit(bytes), vec![]) - } - /// Serialize Instruction using borsh, this is only used in runtime::cost_model::tests but compilation /// can't be restricted as it's used across packages // #[cfg(test)] diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index d13fdb48bb1907..0ecb238d4c4ac7 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -550,10 +550,6 @@ pub mod enable_bpf_loader_set_authority_checked_ix { solana_sdk::declare_id!("5x3825XS7M2A3Ekbn5VGGkvFoAg5qrRWkTrY4bARP1GL"); } -pub mod cap_transaction_accounts_data_size { - solana_sdk::declare_id!("DdLwVYuvDz26JohmgSbA7mjpJFgX5zP2dkp8qsF2C33V"); -} - pub mod enable_alt_bn128_syscall { solana_sdk::declare_id!("A16q37opZdQMCbe5qJ6xpBB9usykfv8jZaMkxvZQi4GJ"); } @@ -714,7 +710,6 @@ lazy_static! { (limit_max_instruction_trace_length::id(), "limit max instruction trace length #27939"), (check_syscall_outputs_do_not_overlap::id(), "check syscall outputs do_not overlap #28600"), (enable_bpf_loader_set_authority_checked_ix::id(), "enable bpf upgradeable loader SetAuthorityChecked instruction #28424"), - (cap_transaction_accounts_data_size::id(), "cap transaction accounts data size up to its compute unit limits #27839"), (enable_alt_bn128_syscall::id(), "add alt_bn128 syscalls #27961"), (enable_program_redeployment_cooldown::id(), "enable program redeployment cooldown #29135"), (commission_updates_only_allowed_in_first_half_of_epoch::id(), "validator commission updates are only allowed in the first half of an epoch #29362"), diff --git a/sdk/src/transaction/error.rs b/sdk/src/transaction/error.rs index 9c902c7b2662f0..47d5856ce0da60 100644 --- a/sdk/src/transaction/error.rs +++ b/sdk/src/transaction/error.rs @@ -149,16 +149,6 @@ pub enum TransactionError { "Transaction results in an account ({account_index}) with insufficient funds for rent" )] InsufficientFundsForRent { account_index: u8 }, - - /// Transaction exceeded max loaded accounts data size capped by requested compute units - #[error( - "Transaction exceeded max loaded accounts data size capped by requested compute units" - )] - MaxLoadedAccountsDataSizeExceeded, - - /// LoadedAccountsDataSizeLimit set for transaction must be greater than 0. - #[error("LoadedAccountsDataSizeLimit set for transaction must be greater than 0.")] - InvalidLoadedAccountsDataSizeLimit, } impl From for TransactionError { diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index b29b628f10c28d..6a1869fc2c517e 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -57,8 +57,6 @@ enum TransactionErrorType { WOULD_EXCEED_ACCOUNT_DATA_TOTAL_LIMIT = 29; DUPLICATE_INSTRUCTION = 30; INSUFFICIENT_FUNDS_FOR_RENT = 31; - MAX_LOADED_ACCOUNTS_DATA_SIZE_EXCEEDED = 32; - INVALID_LOADED_ACCOUNTS_DATA_SIZE_LIMIT = 33; } message InstructionError { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 3aef05649e8d9c..a3807a45fe6838 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -793,8 +793,6 @@ impl TryFrom for TransactionError { 27 => TransactionError::InvalidRentPayingAccount, 28 => TransactionError::WouldExceedMaxVoteCostLimit, 29 => TransactionError::WouldExceedAccountDataTotalLimit, - 32 => TransactionError::MaxLoadedAccountsDataSizeExceeded, - 33 => TransactionError::InvalidLoadedAccountsDataSizeLimit, _ => return Err("Invalid TransactionError"), }) } @@ -898,12 +896,6 @@ impl From for tx_by_addr::TransactionError { TransactionError::InsufficientFundsForRent { .. } => { tx_by_addr::TransactionErrorType::InsufficientFundsForRent } - TransactionError::MaxLoadedAccountsDataSizeExceeded => { - tx_by_addr::TransactionErrorType::MaxLoadedAccountsDataSizeExceeded - } - TransactionError::InvalidLoadedAccountsDataSizeLimit => { - tx_by_addr::TransactionErrorType::InvalidLoadedAccountsDataSizeLimit - } } as i32, instruction_error: match transaction_error { TransactionError::InstructionError(index, ref instruction_error) => {