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

Removes cap_accounts_data_allocations_per_transaction featurization #33754

Merged
Merged
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: 0 additions & 1 deletion program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,6 @@ macro_rules! with_mock_invoke_context {
compute_budget.max_invoke_stack_height,
compute_budget.max_instruction_trace_length,
);
$transaction_context.enable_cap_accounts_data_allocations_per_transaction();
let mut sysvar_cache = SysvarCache::default();
sysvar_cache.fill_missing_entries(|pubkey, callback| {
for index in 0..$transaction_context.get_number_of_accounts() {
Expand Down
23 changes: 7 additions & 16 deletions programs/bpf_loader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,16 @@ use {
clock::Slot,
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
feature_set::{
bpf_account_data_direct_mapping, cap_accounts_data_allocations_per_transaction,
delay_visibility_of_program_deployment, enable_bpf_loader_extend_program_ix,
enable_bpf_loader_set_authority_checked_ix, enable_program_redeployment_cooldown,
limit_max_instruction_trace_length, native_programs_consume_cu,
remove_bpf_loader_incorrect_program_id,
bpf_account_data_direct_mapping, delay_visibility_of_program_deployment,
enable_bpf_loader_extend_program_ix, enable_bpf_loader_set_authority_checked_ix,
enable_program_redeployment_cooldown, limit_max_instruction_trace_length,
native_programs_consume_cu, remove_bpf_loader_incorrect_program_id,
},
instruction::{AccountMeta, InstructionError},
loader_instruction::LoaderInstruction,
loader_upgradeable_instruction::UpgradeableLoaderInstruction,
native_loader,
program_error::{
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED, MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED,
},
program_error::MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED,
program_utils::limited_deserialize,
pubkey::Pubkey,
saturating_add_assign,
Expand Down Expand Up @@ -1595,17 +1592,11 @@ fn execute<'a, 'b: 'a>(
}
match result {
ProgramResult::Ok(status) if status != SUCCESS => {
let error: InstructionError = if (status == MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED
let error: InstructionError = if status == MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED
&& !invoke_context
.feature_set
.is_active(&cap_accounts_data_allocations_per_transaction::id()))
|| (status == MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED
Comment on lines 1596 to -1602
Copy link
Contributor

Choose a reason for hiding this comment

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

ideally don't combine feature gate logic with other logic to make these removal diffs cleaner

&& !invoke_context
.feature_set
.is_active(&limit_max_instruction_trace_length::id()))
.is_active(&limit_max_instruction_trace_length::id())
{
// Until the cap_accounts_data_allocations_per_transaction feature is
// enabled, map the `MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED` error to `InvalidError`.
// Until the limit_max_instruction_trace_length feature is
// enabled, map the `MAX_INSTRUCTION_TRACE_LENGTH_EXCEEDED` error to `InvalidError`.
InstructionError::InvalidError
Expand Down
6 changes: 0 additions & 6 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4864,12 +4864,6 @@ impl Bank {
std::usize::MAX
},
);
if self
.feature_set
.is_active(&feature_set::cap_accounts_data_allocations_per_transaction::id())
{
transaction_context.enable_cap_accounts_data_allocations_per_transaction();
}
#[cfg(debug_assertions)]
transaction_context.set_signature(tx.signature());

Expand Down
3 changes: 1 addition & 2 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12171,8 +12171,7 @@ fn test_cap_accounts_data_allocations_per_transaction() {
/ MAX_PERMITTED_DATA_LENGTH as usize;

let (genesis_config, mint_keypair) = create_genesis_config(1_000_000 * LAMPORTS_PER_SOL);
let mut bank = Bank::new_for_tests(&genesis_config);
bank.activate_feature(&feature_set::cap_accounts_data_allocations_per_transaction::id());
let bank = Bank::new_for_tests(&genesis_config);

let mut instructions = Vec::new();
let mut keypairs = vec![mint_keypair.insecure_clone()];
Expand Down
26 changes: 6 additions & 20 deletions sdk/src/transaction_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ pub struct TransactionContext {
accounts_resize_delta: RefCell<i64>,
#[cfg(not(target_os = "solana"))]
rent: Rent,
#[cfg(not(target_os = "solana"))]
is_cap_accounts_data_allocations_per_transaction_enabled: bool,
/// Useful for debugging to filter by or to look it up on the explorer
#[cfg(all(not(target_os = "solana"), debug_assertions))]
signature: Signature,
Expand Down Expand Up @@ -174,7 +172,6 @@ impl TransactionContext {
return_data: TransactionReturnData::default(),
accounts_resize_delta: RefCell::new(0),
rent,
is_cap_accounts_data_allocations_per_transaction_enabled: false,
#[cfg(all(not(target_os = "solana"), debug_assertions))]
signature: Signature::default(),
}
Expand Down Expand Up @@ -440,12 +437,6 @@ impl TransactionContext {
.map_err(|_| InstructionError::GenericError)
.map(|value_ref| *value_ref)
}

/// Enables enforcing a maximum accounts data allocation size per transaction
#[cfg(not(target_os = "solana"))]
pub fn enable_cap_accounts_data_allocations_per_transaction(&mut self) {
self.is_cap_accounts_data_allocations_per_transaction_enabled = true;
}
}

/// Return data at the end of a transaction
Expand Down Expand Up @@ -1114,20 +1105,15 @@ impl<'a> BorrowedAccount<'a> {
if new_length > MAX_PERMITTED_DATA_LENGTH as usize {
return Err(InstructionError::InvalidRealloc);
}
// The resize can not exceed the per-transaction maximum
let length_delta = (new_length as i64).saturating_sub(old_length as i64);
if self
.transaction_context
.is_cap_accounts_data_allocations_per_transaction_enabled
.accounts_resize_delta()?
.saturating_add(length_delta)
> MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION
{
// The resize can not exceed the per-transaction maximum
let length_delta = (new_length as i64).saturating_sub(old_length as i64);
if self
.transaction_context
.accounts_resize_delta()?
.saturating_add(length_delta)
> MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION
{
return Err(InstructionError::MaxAccountsDataAllocationsExceeded);
}
return Err(InstructionError::MaxAccountsDataAllocationsExceeded);
}
Ok(())
}
Expand Down