Set cap for new allocations per transaction (#27385)

This commit is contained in:
Brooks Prumo 2022-08-29 14:30:48 -04:00 committed by GitHub
parent 49411aaae0
commit 757e46c3c7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 131 additions and 25 deletions

View File

@ -1028,6 +1028,7 @@ pub fn with_mock_invoke_context<R, F: FnMut(&mut InvokeContext) -> R>(
ComputeBudget::default().max_invoke_depth.saturating_add(1),
1,
);
transaction_context.enable_cap_accounts_data_allocations_per_transaction();
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
invoke_context
.push(&preparation.instruction_accounts, &program_indices, &[])
@ -1059,6 +1060,7 @@ pub fn mock_process_instruction(
ComputeBudget::default().max_invoke_depth.saturating_add(1),
1,
);
transaction_context.enable_cap_accounts_data_allocations_per_transaction();
let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]);
if let Some(sysvar_cache) = sysvar_cache_override {
invoke_context.sysvar_cache = Cow::Borrowed(sysvar_cache);

View File

@ -41,7 +41,7 @@ use {
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
entrypoint::{HEAP_LENGTH, SUCCESS},
feature_set::{
cap_accounts_data_len, cap_bpf_program_instruction_accounts,
cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts,
disable_deploy_of_alloc_free_syscall, disable_deprecated_loader,
enable_bpf_loader_extend_program_data_ix,
error_on_syscall_bpf_function_hash_collisions, reject_callx_r10,
@ -49,7 +49,7 @@ use {
instruction::{AccountMeta, InstructionError},
loader_instruction::LoaderInstruction,
loader_upgradeable_instruction::UpgradeableLoaderInstruction,
program_error::MAX_ACCOUNTS_DATA_SIZE_EXCEEDED,
program_error::MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED,
program_utils::limited_deserialize,
pubkey::Pubkey,
saturating_add_assign,
@ -1339,13 +1339,14 @@ impl Executor for BpfExecutor {
}
match result {
Ok(status) if status != SUCCESS => {
let error: InstructionError = if status == MAX_ACCOUNTS_DATA_SIZE_EXCEEDED
let error: InstructionError = if status
== MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED
&& !invoke_context
.feature_set
.is_active(&cap_accounts_data_len::id())
.is_active(&cap_accounts_data_allocations_per_transaction::id())
{
// Until the cap_accounts_data_len feature is enabled, map the
// MAX_ACCOUNTS_DATA_SIZE_EXCEEDED error to InvalidError
// Until the cap_accounts_data_allocations_per_transaction feature is
// enabled, map the MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED error to InvalidError
InstructionError::InvalidError
} else {
status.into()

View File

@ -284,7 +284,7 @@ impl RentDebits {
}
pub type BankStatusCache = StatusCache<Result<()>>;
#[frozen_abi(digest = "HEJXoycXvGT2pwMuKcUKzzbeemnqbfrUC4jHZx1ncaWv")]
#[frozen_abi(digest = "7FSSacrCi7vf2QZFm3Ui9JqTii4U6h1XWYD3LKSuVwV8")]
pub type BankSlotDelta = SlotDelta<Result<()>>;
// Eager rent collection repeats in cyclic manner.
@ -4352,6 +4352,12 @@ impl Bank {
compute_budget.max_invoke_depth.saturating_add(1),
tx.message().instructions().len(),
);
if self
.feature_set
.is_active(&feature_set::cap_accounts_data_allocations_per_transaction::id())
{
transaction_context.enable_cap_accounts_data_allocations_per_transaction();
}
let pre_account_state_info =
self.get_transaction_account_state_info(&transaction_context, tx.message());
@ -8038,7 +8044,10 @@ pub(crate) mod tests {
instruction as stake_instruction,
state::{Authorized, Delegation, Lockup, Stake},
},
system_instruction::{self, SystemError, MAX_PERMITTED_DATA_LENGTH},
system_instruction::{
self, SystemError, MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION,
MAX_PERMITTED_DATA_LENGTH,
},
system_program,
timing::duration_as_s,
transaction::MAX_TX_ACCOUNT_LOCKS,
@ -19482,4 +19491,55 @@ pub(crate) mod tests {
);
}
}
/// Ensures that if a transaction exceeds the maximum allowed accounts data allocation size:
/// 1. The transaction fails
/// 2. The bank's accounts_data_size is unmodified
#[test]
fn test_cap_accounts_data_allocations_per_transaction() {
use solana_sdk::signature::KeypairInsecureClone;
const NUM_MAX_SIZE_ALLOCATIONS_PER_TRANSACTION: usize =
MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION as usize
/ 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::enable_early_verification_of_account_modifications::id(),
);
bank.activate_feature(&feature_set::cap_accounts_data_allocations_per_transaction::id());
let mut instructions = Vec::new();
let mut keypairs = vec![mint_keypair.clone()];
for _ in 0..=NUM_MAX_SIZE_ALLOCATIONS_PER_TRANSACTION {
let keypair = Keypair::new();
let instruction = system_instruction::create_account(
&mint_keypair.pubkey(),
&keypair.pubkey(),
bank.rent_collector()
.rent
.minimum_balance(MAX_PERMITTED_DATA_LENGTH as usize),
MAX_PERMITTED_DATA_LENGTH,
&solana_sdk::system_program::id(),
);
keypairs.push(keypair);
instructions.push(instruction);
}
let message = Message::new(&instructions, Some(&mint_keypair.pubkey()));
let signers: Vec<_> = keypairs.iter().collect();
let transaction = Transaction::new(&signers, message, bank.last_blockhash());
let accounts_data_size_before = bank.load_accounts_data_size();
let result = bank.process_transaction(&transaction);
let accounts_data_size_after = bank.load_accounts_data_size();
assert_eq!(accounts_data_size_before, accounts_data_size_after);
assert_eq!(
result,
Err(TransactionError::InstructionError(
NUM_MAX_SIZE_ALLOCATIONS_PER_TRANSACTION as u8,
solana_sdk::instruction::InstructionError::MaxAccountsDataAllocationsExceeded,
)),
);
}
}

View File

@ -249,9 +249,9 @@ pub enum InstructionError {
#[error("Provided owner is not allowed")]
IllegalOwner,
/// Account data allocation exceeded the maximum accounts data size limit
#[error("Account data allocation exceeded the maximum accounts data size limit")]
MaxAccountsDataSizeExceeded,
/// Accounts data allocations exceeded the maximum allowed per transaction
#[error("Accounts data allocations exceeded the maximum allowed per transaction")]
MaxAccountsDataAllocationsExceeded,
/// Max accounts exceeded
#[error("Max accounts exceeded")]

View File

@ -51,8 +51,8 @@ pub enum ProgramError {
UnsupportedSysvar,
#[error("Provided owner is not allowed")]
IllegalOwner,
#[error("Account data allocation exceeded the maximum accounts data size limit")]
MaxAccountsDataSizeExceeded,
#[error("Accounts data allocations exceeded the maximum allowed per transaction")]
MaxAccountsDataAllocationsExceeded,
#[error("Account data reallocation was invalid")]
InvalidRealloc,
}
@ -93,7 +93,9 @@ impl PrintProgramError for ProgramError {
Self::AccountNotRentExempt => msg!("Error: AccountNotRentExempt"),
Self::UnsupportedSysvar => msg!("Error: UnsupportedSysvar"),
Self::IllegalOwner => msg!("Error: IllegalOwner"),
Self::MaxAccountsDataSizeExceeded => msg!("Error: MaxAccountsDataSizeExceeded"),
Self::MaxAccountsDataAllocationsExceeded => {
msg!("Error: MaxAccountsDataAllocationsExceeded")
}
Self::InvalidRealloc => msg!("Error: InvalidRealloc"),
}
}
@ -125,7 +127,7 @@ pub const BORSH_IO_ERROR: u64 = to_builtin!(15);
pub const ACCOUNT_NOT_RENT_EXEMPT: u64 = to_builtin!(16);
pub const UNSUPPORTED_SYSVAR: u64 = to_builtin!(17);
pub const ILLEGAL_OWNER: u64 = to_builtin!(18);
pub const MAX_ACCOUNTS_DATA_SIZE_EXCEEDED: u64 = to_builtin!(19);
pub const MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED: u64 = to_builtin!(19);
pub const INVALID_ACCOUNT_DATA_REALLOC: u64 = to_builtin!(20);
// Warning: Any new program errors added here must also be:
// - Added to the below conversions
@ -153,7 +155,9 @@ impl From<ProgramError> for u64 {
ProgramError::AccountNotRentExempt => ACCOUNT_NOT_RENT_EXEMPT,
ProgramError::UnsupportedSysvar => UNSUPPORTED_SYSVAR,
ProgramError::IllegalOwner => ILLEGAL_OWNER,
ProgramError::MaxAccountsDataSizeExceeded => MAX_ACCOUNTS_DATA_SIZE_EXCEEDED,
ProgramError::MaxAccountsDataAllocationsExceeded => {
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED
}
ProgramError::InvalidRealloc => INVALID_ACCOUNT_DATA_REALLOC,
ProgramError::Custom(error) => {
if error == 0 {
@ -187,7 +191,7 @@ impl From<u64> for ProgramError {
ACCOUNT_NOT_RENT_EXEMPT => Self::AccountNotRentExempt,
UNSUPPORTED_SYSVAR => Self::UnsupportedSysvar,
ILLEGAL_OWNER => Self::IllegalOwner,
MAX_ACCOUNTS_DATA_SIZE_EXCEEDED => Self::MaxAccountsDataSizeExceeded,
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED => Self::MaxAccountsDataAllocationsExceeded,
INVALID_ACCOUNT_DATA_REALLOC => Self::InvalidRealloc,
_ => Self::Custom(error as u32),
}
@ -217,7 +221,9 @@ impl TryFrom<InstructionError> for ProgramError {
Self::Error::AccountNotRentExempt => Ok(Self::AccountNotRentExempt),
Self::Error::UnsupportedSysvar => Ok(Self::UnsupportedSysvar),
Self::Error::IllegalOwner => Ok(Self::IllegalOwner),
Self::Error::MaxAccountsDataSizeExceeded => Ok(Self::MaxAccountsDataSizeExceeded),
Self::Error::MaxAccountsDataAllocationsExceeded => {
Ok(Self::MaxAccountsDataAllocationsExceeded)
}
Self::Error::InvalidRealloc => Ok(Self::InvalidRealloc),
_ => Err(error),
}
@ -249,7 +255,7 @@ where
ACCOUNT_NOT_RENT_EXEMPT => Self::AccountNotRentExempt,
UNSUPPORTED_SYSVAR => Self::UnsupportedSysvar,
ILLEGAL_OWNER => Self::IllegalOwner,
MAX_ACCOUNTS_DATA_SIZE_EXCEEDED => Self::MaxAccountsDataSizeExceeded,
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED => Self::MaxAccountsDataAllocationsExceeded,
INVALID_ACCOUNT_DATA_REALLOC => Self::InvalidRealloc,
_ => {
// A valid custom error has no bits set in the upper 32

View File

@ -142,6 +142,13 @@ pub fn instruction_to_nonce_error(
/// Maximum permitted size of data: 10 MiB
pub const MAX_PERMITTED_DATA_LENGTH: u64 = 10 * 1024 * 1024;
/// Maximum permitted size of new allocations per transaction, in bytes
///
/// The value was chosen such at least one max sized account could be created,
/// plus some additional resize allocations.
pub const MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION: i64 =
MAX_PERMITTED_DATA_LENGTH as i64 * 2;
// SBF program entrypoint assumes that the max account data length
// will fit inside a u32. If this constant no longer fits in a u32,
// the entrypoint deserialization code in the SDK must be updated.

View File

@ -510,6 +510,10 @@ pub mod vote_state_update_root_fix {
solana_sdk::declare_id!("G74BkWBzmsByZ1kxHy44H3wjwp5hp7JbrGRuDpco22tY");
}
pub mod cap_accounts_data_allocations_per_transaction {
solana_sdk::declare_id!("9gxu85LYRAcZL38We8MYJ4A9AwgBBPtVBAqebMcT1241");
}
lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -632,6 +636,7 @@ lazy_static! {
(relax_authority_signer_check_for_lookup_table_creation::id(), "relax authority signer check for lookup table creation #27205"),
(stop_sibling_instruction_search_at_parent::id(), "stop the search in get_processed_sibling_instruction when the parent instruction is reached #27289"),
(vote_state_update_root_fix::id(), "fix root in vote state updates #27361"),
(cap_accounts_data_allocations_per_transaction::id(), "cap accounts data allocations per transaction #27375"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()

View File

@ -6,7 +6,9 @@ use {
instruction::InstructionError,
pubkey::Pubkey,
rent::Rent,
system_instruction::MAX_PERMITTED_DATA_LENGTH,
system_instruction::{
MAX_PERMITTED_ACCOUNTS_DATA_ALLOCATIONS_PER_TRANSACTION, MAX_PERMITTED_DATA_LENGTH,
},
},
std::{
cell::{RefCell, RefMut},
@ -52,6 +54,7 @@ pub struct TransactionContext {
return_data: TransactionReturnData,
accounts_resize_delta: RefCell<i64>,
rent: Option<Rent>,
is_cap_accounts_data_allocations_per_transaction_enabled: bool,
}
impl TransactionContext {
@ -78,6 +81,7 @@ impl TransactionContext {
return_data: TransactionReturnData::default(),
accounts_resize_delta: RefCell::new(0),
rent,
is_cap_accounts_data_allocations_per_transaction_enabled: false,
}
}
@ -312,6 +316,11 @@ impl TransactionContext {
.map_err(|_| InstructionError::GenericError)
.map(|value_ref| *value_ref)
}
/// Enables enforcing a maximum accounts data allocation size per transaction
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
@ -859,14 +868,30 @@ impl<'a> BorrowedAccount<'a> {
{
return Ok(());
}
let old_length = self.get_data().len();
// Only the owner can change the length of the data
if new_length != self.get_data().len() && !self.is_owned_by_current_program() {
if new_length != old_length && !self.is_owned_by_current_program() {
return Err(InstructionError::AccountDataSizeChanged);
}
// The new length can not exceed the maximum permitted length
if new_length > MAX_PERMITTED_DATA_LENGTH as usize {
return Err(InstructionError::InvalidRealloc);
}
if self
.transaction_context
.is_cap_accounts_data_allocations_per_transaction_enabled
{
// 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);
}
}
Ok(())
}

View File

@ -120,7 +120,7 @@ enum InstructionErrorType {
ARITHMETIC_OVERFLOW = 47;
UNSUPPORTED_SYSVAR = 48;
ILLEGAL_OWNER = 49;
MAX_ACCOUNTS_DATA_SIZE_EXCEEDED = 50;
MAX_ACCOUNTS_DATA_ALLOCATIONS_EXCEEDED = 50;
MAX_ACCOUNTS_EXCEEDED = 51;
}

View File

@ -710,7 +710,7 @@ impl TryFrom<tx_by_addr::TransactionError> for TransactionError {
47 => InstructionError::ArithmeticOverflow,
48 => InstructionError::UnsupportedSysvar,
49 => InstructionError::IllegalOwner,
50 => InstructionError::MaxAccountsDataSizeExceeded,
50 => InstructionError::MaxAccountsDataAllocationsExceeded,
51 => InstructionError::MaxAccountsExceeded,
_ => return Err("Invalid InstructionError"),
};
@ -1025,8 +1025,8 @@ impl From<TransactionError> for tx_by_addr::TransactionError {
InstructionError::IllegalOwner => {
tx_by_addr::InstructionErrorType::IllegalOwner
}
InstructionError::MaxAccountsDataSizeExceeded => {
tx_by_addr::InstructionErrorType::MaxAccountsDataSizeExceeded
InstructionError::MaxAccountsDataAllocationsExceeded => {
tx_by_addr::InstructionErrorType::MaxAccountsDataAllocationsExceeded
}
InstructionError::MaxAccountsExceeded => {
tx_by_addr::InstructionErrorType::MaxAccountsExceeded