From 5ebcc1eb63e54bfdc24b2d7efa3c6dcd4c487807 Mon Sep 17 00:00:00 2001 From: Yael Doweck Date: Mon, 12 Feb 2024 14:25:09 +0200 Subject: [PATCH] refactor: move the event size limit constants into versioned_constants --- .../resources/versioned_constants.json | 7 ++- .../deprecated_syscalls_test.rs | 26 +++++----- .../src/execution/deprecated_syscalls/mod.rs | 6 ++- .../blockifier/src/execution/syscalls/mod.rs | 35 +++++++------- .../src/execution/syscalls/syscalls_test.rs | 26 +++++----- .../src/transaction/transactions_test.rs | 47 ++++++++++++------- crates/blockifier/src/versioned_constants.rs | 23 +++++---- 7 files changed, 101 insertions(+), 69 deletions(-) diff --git a/crates/blockifier/resources/versioned_constants.json b/crates/blockifier/resources/versioned_constants.json index 2226642b68..1228b825ed 100644 --- a/crates/blockifier/resources/versioned_constants.json +++ b/crates/blockifier/resources/versioned_constants.json @@ -1,14 +1,19 @@ { + "event_size_limit": { + "max_data_length": 40, + "max_keys_length": 40, + "max_n_emitted_events": 1000 + }, "gateway": { "max_calldata_length": 4000, "max_contract_bytecode_size": 61440 }, + "invoke_tx_max_n_steps": 4000000, "l2_resource_gas_costs": { "milligas_per_data_felt": 128, "event_key_factor": 2, "milligas_per_code_byte": 875 }, - "invoke_tx_max_n_steps": 4000000, "max_recursion_depth": 50, "os_constants": { "nop_entry_point_offset": -1, diff --git a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs index 66885dcf3d..1bdb7ed646 100644 --- a/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs +++ b/crates/blockifier/src/execution/deprecated_syscalls/deprecated_syscalls_test.rs @@ -27,9 +27,6 @@ use crate::execution::entry_point::{CallEntryPoint, CallType}; use crate::execution::errors::EntryPointExecutionError; use crate::execution::execution_utils::felt_to_stark_felt; use crate::execution::syscalls::hint_processor::EmitEventError; -use crate::execution::syscalls::{ - SYSCALL_MAX_EVENT_DATA, SYSCALL_MAX_EVENT_KEYS, SYSCALL_MAX_N_EMITTED_EVENTS, -}; use crate::state::state_api::StateReader; use crate::test_utils::cached_state::{ deprecated_create_deploy_test_state, deprecated_create_test_state, @@ -46,6 +43,7 @@ use crate::transaction::constants::QUERY_VERSION_BASE_BIT; use crate::transaction::objects::{ CommonAccountFields, DeprecatedTransactionInfo, TransactionInfo, }; +use crate::versioned_constants::VersionedConstants; use crate::{check_entry_point_execution_error_for_custom_hint, retdata}; #[test] @@ -509,6 +507,7 @@ fn test_tx_info(#[case] only_query: bool) { #[test] fn test_emit_event() { + let versioned_constants = VersionedConstants::create_for_testing(); // Positive flow. let keys = vec![stark_felt!(2019_u16), stark_felt!(2020_u16)]; let data = vec![stark_felt!(2021_u16), stark_felt!(2022_u16), stark_felt!(2023_u16)]; @@ -528,31 +527,34 @@ fn test_emit_event() { ); // Negative flow, the data length exceeds the limit. - let data_too_long = vec![stark_felt!(2_u16); SYSCALL_MAX_EVENT_DATA + 1]; + let max_event_data_length = versioned_constants.event_size_limit.max_data_length; + let data_too_long = vec![stark_felt!(2_u16); max_event_data_length + 1]; let error = emit_events(&n_emitted_events, &keys, &data_too_long).unwrap_err(); let expected_error = EmitEventError::ExceedsMaxDataLength { - data_length: SYSCALL_MAX_EVENT_DATA + 1, - max_data_length: SYSCALL_MAX_EVENT_DATA, + data_length: max_event_data_length + 1, + max_data_length: max_event_data_length, }; assert!(error.to_string().contains(format!("{}", expected_error).as_str())); // Negative flow, the keys length exceeds the limit. - let keys_too_long = vec![stark_felt!(1_u16); SYSCALL_MAX_EVENT_KEYS + 1]; + let max_event_keys_length = versioned_constants.event_size_limit.max_keys_length; + let keys_too_long = vec![stark_felt!(1_u16); max_event_keys_length + 1]; let error = emit_events(&n_emitted_events, &keys_too_long, &data).unwrap_err(); let expected_error = EmitEventError::ExceedsMaxKeysLength { - keys_length: SYSCALL_MAX_EVENT_KEYS + 1, - max_keys_length: SYSCALL_MAX_EVENT_KEYS, + keys_length: max_event_keys_length + 1, + max_keys_length: max_event_keys_length, }; assert!(error.to_string().contains(format!("{}", expected_error).as_str())); // Negative flow, the number of events exceeds the limit. + let max_n_emitted_events = versioned_constants.event_size_limit.max_n_emitted_events; let n_emitted_events_too_big = vec![stark_felt!( - u16::try_from(SYSCALL_MAX_N_EMITTED_EVENTS + 1).expect("Failed to convert usize to u16.") + u16::try_from(max_n_emitted_events + 1).expect("Failed to convert usize to u16.") )]; let error = emit_events(&n_emitted_events_too_big, &keys, &data).unwrap_err(); let expected_error = EmitEventError::ExceedsMaxNumberOfEmittedEvents { - n_emitted_events: SYSCALL_MAX_N_EMITTED_EVENTS + 1, - max_n_emitted_events: SYSCALL_MAX_N_EMITTED_EVENTS, + n_emitted_events: max_n_emitted_events + 1, + max_n_emitted_events, }; assert!(error.to_string().contains(format!("{}", expected_error).as_str())); } diff --git a/crates/blockifier/src/execution/deprecated_syscalls/mod.rs b/crates/blockifier/src/execution/deprecated_syscalls/mod.rs index 31d6ca373f..35645593f0 100644 --- a/crates/blockifier/src/execution/deprecated_syscalls/mod.rs +++ b/crates/blockifier/src/execution/deprecated_syscalls/mod.rs @@ -373,7 +373,11 @@ pub fn emit_event( syscall_handler: &mut DeprecatedSyscallHintProcessor<'_>, ) -> DeprecatedSyscallResult { let execution_context = &mut syscall_handler.context; - exceeds_event_size_limit(execution_context.n_emitted_events + 1, &request.content)?; + exceeds_event_size_limit( + &execution_context.tx_context.block_context.versioned_constants, + execution_context.n_emitted_events + 1, + &request.content, + )?; let ordered_event = OrderedEvent { order: execution_context.n_emitted_events, event: request.content }; syscall_handler.events.push(ordered_event); diff --git a/crates/blockifier/src/execution/syscalls/mod.rs b/crates/blockifier/src/execution/syscalls/mod.rs index cca71cb6a8..a2b5aeee24 100644 --- a/crates/blockifier/src/execution/syscalls/mod.rs +++ b/crates/blockifier/src/execution/syscalls/mod.rs @@ -29,6 +29,7 @@ use crate::execution::execution_utils::{ }; use crate::execution::syscalls::hint_processor::{INVALID_INPUT_LENGTH_ERROR, OUT_OF_GAS_ERROR}; use crate::transaction::transaction_utils::update_remaining_gas; +use crate::versioned_constants::VersionedConstants; pub mod hint_processor; mod secp; @@ -42,10 +43,6 @@ pub type WriteResponseResult = SyscallResult<()>; type SyscallSelector = DeprecatedSyscallSelector; -pub const SYSCALL_MAX_EVENT_KEYS: usize = 40; -pub const SYSCALL_MAX_EVENT_DATA: usize = 40; -pub const SYSCALL_MAX_N_EMITTED_EVENTS: usize = 1000; - pub trait SyscallRequest: Sized { fn read(_vm: &VirtualMachine, _ptr: &mut Relocatable) -> SyscallResult; } @@ -293,28 +290,26 @@ impl SyscallRequest for EmitEventRequest { type EmitEventResponse = EmptyResponse; pub fn exceeds_event_size_limit( + versioned_constants: &VersionedConstants, n_emitted_events: usize, event: &EventContent, ) -> Result<(), EmitEventError> { - if n_emitted_events > SYSCALL_MAX_N_EMITTED_EVENTS { + let max_n_emitted_events = versioned_constants.event_size_limit.max_n_emitted_events; + if n_emitted_events > max_n_emitted_events { return Err(EmitEventError::ExceedsMaxNumberOfEmittedEvents { n_emitted_events, - max_n_emitted_events: SYSCALL_MAX_N_EMITTED_EVENTS, + max_n_emitted_events, }); } - let key_length = event.keys.len(); - if key_length > SYSCALL_MAX_EVENT_KEYS { - return Err(EmitEventError::ExceedsMaxKeysLength { - keys_length: key_length, - max_keys_length: SYSCALL_MAX_EVENT_KEYS, - }); + let max_keys_length = versioned_constants.event_size_limit.max_keys_length; + let keys_length = event.keys.len(); + if keys_length > versioned_constants.event_size_limit.max_keys_length { + return Err(EmitEventError::ExceedsMaxKeysLength { keys_length, max_keys_length }); } + let max_data_length = versioned_constants.event_size_limit.max_data_length; let data_length = event.data.0.len(); - if data_length > SYSCALL_MAX_EVENT_DATA { - return Err(EmitEventError::ExceedsMaxDataLength { - data_length, - max_data_length: SYSCALL_MAX_EVENT_DATA, - }); + if data_length > max_data_length { + return Err(EmitEventError::ExceedsMaxDataLength { data_length, max_data_length }); } Ok(()) @@ -327,7 +322,11 @@ pub fn emit_event( _remaining_gas: &mut u64, ) -> SyscallResult { let execution_context = &mut syscall_handler.context; - exceeds_event_size_limit(execution_context.n_emitted_events + 1, &request.content)?; + exceeds_event_size_limit( + &execution_context.tx_context.block_context.versioned_constants, + execution_context.n_emitted_events + 1, + &request.content, + )?; let ordered_event = OrderedEvent { order: execution_context.n_emitted_events, event: request.content }; syscall_handler.events.push(ordered_event); diff --git a/crates/blockifier/src/execution/syscalls/syscalls_test.rs b/crates/blockifier/src/execution/syscalls/syscalls_test.rs index e8e71b7f0b..622e2946da 100644 --- a/crates/blockifier/src/execution/syscalls/syscalls_test.rs +++ b/crates/blockifier/src/execution/syscalls/syscalls_test.rs @@ -36,9 +36,6 @@ use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt}; use crate::execution::syscalls::hint_processor::{ EmitEventError, BLOCK_NUMBER_OUT_OF_RANGE_ERROR, L1_GAS, L2_GAS, OUT_OF_GAS_ERROR, }; -use crate::execution::syscalls::{ - SYSCALL_MAX_EVENT_DATA, SYSCALL_MAX_EVENT_KEYS, SYSCALL_MAX_N_EMITTED_EVENTS, -}; use crate::state::state_api::{State, StateReader}; use crate::test_utils::cached_state::{create_deploy_test_state, create_test_state}; use crate::test_utils::contracts::FeatureContract; @@ -53,6 +50,7 @@ use crate::transaction::constants::QUERY_VERSION_BASE_BIT; use crate::transaction::objects::{ CommonAccountFields, CurrentTransactionInfo, DeprecatedTransactionInfo, TransactionInfo, }; +use crate::versioned_constants::VersionedConstants; use crate::{check_entry_point_execution_error_for_custom_hint, retdata}; pub const REQUIRED_GAS_STORAGE_READ_WRITE_TEST: u64 = 34650; @@ -116,6 +114,7 @@ fn test_call_contract() { #[test] fn test_emit_event() { + let versioned_constants = VersionedConstants::create_for_testing(); // Positive flow. let keys = vec![stark_felt!(2019_u16), stark_felt!(2020_u16)]; let data = vec![stark_felt!(2021_u16), stark_felt!(2022_u16), stark_felt!(2023_u16)]; @@ -136,31 +135,34 @@ fn test_emit_event() { ); // Negative flow, the data length exceeds the limit. - let data_too_long = vec![stark_felt!(2_u16); SYSCALL_MAX_EVENT_DATA + 1]; + let max_event_data_length = versioned_constants.event_size_limit.max_data_length; + let data_too_long = vec![stark_felt!(2_u16); max_event_data_length + 1]; let error = emit_events(&n_emitted_events, &keys, &data_too_long).unwrap_err(); let expected_error = EmitEventError::ExceedsMaxDataLength { - data_length: SYSCALL_MAX_EVENT_DATA + 1, - max_data_length: SYSCALL_MAX_EVENT_DATA, + data_length: max_event_data_length + 1, + max_data_length: max_event_data_length, }; assert!(error.to_string().contains(format!("{}", expected_error).as_str())); // Negative flow, the keys length exceeds the limit. - let keys_too_long = vec![stark_felt!(1_u16); SYSCALL_MAX_EVENT_KEYS + 1]; + let max_event_keys_length = versioned_constants.event_size_limit.max_keys_length; + let keys_too_long = vec![stark_felt!(1_u16); max_event_keys_length + 1]; let error = emit_events(&n_emitted_events, &keys_too_long, &data).unwrap_err(); let expected_error = EmitEventError::ExceedsMaxKeysLength { - keys_length: SYSCALL_MAX_EVENT_KEYS + 1, - max_keys_length: SYSCALL_MAX_EVENT_KEYS, + keys_length: max_event_keys_length + 1, + max_keys_length: max_event_keys_length, }; assert!(error.to_string().contains(format!("{}", expected_error).as_str())); // Negative flow, the number of events exceeds the limit. + let max_n_emitted_events = versioned_constants.event_size_limit.max_n_emitted_events; let n_emitted_events_too_big = vec![stark_felt!( - u16::try_from(SYSCALL_MAX_N_EMITTED_EVENTS + 1).expect("Failed to convert usize to u16.") + u16::try_from(max_n_emitted_events + 1).expect("Failed to convert usize to u16.") )]; let error = emit_events(&n_emitted_events_too_big, &keys, &data).unwrap_err(); let expected_error = EmitEventError::ExceedsMaxNumberOfEmittedEvents { - n_emitted_events: SYSCALL_MAX_N_EMITTED_EVENTS + 1, - max_n_emitted_events: SYSCALL_MAX_N_EMITTED_EVENTS, + n_emitted_events: max_n_emitted_events + 1, + max_n_emitted_events, }; assert!(error.to_string().contains(format!("{}", expected_error).as_str())); } diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 19eb593f63..39a74fbc6d 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -7,6 +7,7 @@ use cairo_vm::vm::runners::builtin_runner::{HASH_BUILTIN_NAME, RANGE_CHECK_BUILT use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use itertools::concat; use num_traits::Pow; +use once_cell::sync::Lazy; use pretty_assertions::assert_eq; use rstest::{fixture, rstest}; use starknet_api::core::{ChainId, ClassHash, ContractAddress, EthAddress, Nonce, PatriciaKey}; @@ -35,9 +36,6 @@ use crate::execution::entry_point::{CallEntryPoint, CallType}; use crate::execution::errors::EntryPointExecutionError; use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt}; use crate::execution::syscalls::hint_processor::EmitEventError; -use crate::execution::syscalls::{ - SYSCALL_MAX_EVENT_DATA, SYSCALL_MAX_EVENT_KEYS, SYSCALL_MAX_N_EMITTED_EVENTS, -}; use crate::fee::fee_utils::calculate_tx_fee; use crate::fee::gas_usage::{ calculate_tx_gas_usage_vector, estimate_minimal_gas_vector, @@ -86,14 +84,17 @@ use crate::{ deploy_account_tx_args, invoke_tx_args, retdata, }; +static VERSIONED_CONSTANTS: Lazy = + Lazy::new(VersionedConstants::create_for_testing); + #[fixture] fn tx_initial_gas() -> u64 { - VersionedConstants::create_for_testing().tx_initial_gas() + VERSIONED_CONSTANTS.tx_initial_gas() } #[fixture] fn versioned_constants_for_account_testing() -> VersionedConstants { - VersionedConstants::create_for_account_testing() + VERSIONED_CONSTANTS.clone() } struct ExpectedResultTestInvokeTx { @@ -1834,37 +1835,49 @@ fn test_execute_tx_with_invalid_transaction_version() { ); } +fn max_n_emitted_events() -> usize { + VERSIONED_CONSTANTS.event_size_limit.max_n_emitted_events +} + +fn max_event_keys() -> usize { + VERSIONED_CONSTANTS.event_size_limit.max_keys_length +} + +fn max_event_data() -> usize { + VERSIONED_CONSTANTS.event_size_limit.max_data_length +} + #[test_case( - vec![stark_felt!(1_u16); SYSCALL_MAX_EVENT_KEYS], - vec![stark_felt!(2_u16); SYSCALL_MAX_EVENT_DATA], - SYSCALL_MAX_N_EMITTED_EVENTS, + vec![stark_felt!(1_u16); max_event_keys()], + vec![stark_felt!(2_u16); max_event_data()], + max_n_emitted_events(), None; "Positive flow")] #[test_case( vec![stark_felt!(1_u16)], vec![stark_felt!(2_u16)], - SYSCALL_MAX_N_EMITTED_EVENTS + 1, + max_n_emitted_events() + 1, Some(EmitEventError::ExceedsMaxNumberOfEmittedEvents { - n_emitted_events: SYSCALL_MAX_N_EMITTED_EVENTS + 1, - max_n_emitted_events: SYSCALL_MAX_N_EMITTED_EVENTS, + n_emitted_events: max_n_emitted_events() + 1, + max_n_emitted_events: max_n_emitted_events(), }); "exceeds max number of events")] #[test_case( - vec![stark_felt!(3_u16); SYSCALL_MAX_EVENT_KEYS + 1], + vec![stark_felt!(3_u16); max_event_keys() + 1], vec![stark_felt!(4_u16)], 1, Some(EmitEventError::ExceedsMaxKeysLength{ - keys_length: SYSCALL_MAX_EVENT_KEYS + 1, - max_keys_length: SYSCALL_MAX_EVENT_KEYS, + keys_length: max_event_keys() + 1, + max_keys_length: max_event_keys(), }); "exceeds max number of keys")] #[test_case( vec![stark_felt!(5_u16)], - vec![stark_felt!(6_u16); SYSCALL_MAX_EVENT_DATA + 1], + vec![stark_felt!(6_u16); max_event_data() + 1], 1, Some(EmitEventError::ExceedsMaxDataLength{ - data_length: SYSCALL_MAX_EVENT_DATA + 1, - max_data_length: SYSCALL_MAX_EVENT_DATA, + data_length: max_event_data() + 1, + max_data_length: max_event_data(), }); "exceeds data length")] fn test_emit_event_exceeds_limit( diff --git a/crates/blockifier/src/versioned_constants.rs b/crates/blockifier/src/versioned_constants.rs index 7abbe4320c..1b64d9f6cd 100644 --- a/crates/blockifier/src/versioned_constants.rs +++ b/crates/blockifier/src/versioned_constants.rs @@ -35,9 +35,16 @@ static DEFAULT_CONSTANTS: Lazy = Lazy::new(|| { #[derive(Clone, Debug, Default, Deserialize)] pub struct VersionedConstants { // Limits. + pub event_size_limit: EventSizeLimit, pub invoke_tx_max_n_steps: u32, - pub validate_max_n_steps: u32, + pub l2_resource_gas_costs: L2ResourceGasCosts, pub max_recursion_depth: usize, + pub validate_max_n_steps: u32, + + // Cairo OS constants. + // Note: if loaded from a json file, there are some assumptions made on its structure. + // See the struct's docstring for more details. + os_constants: Arc, // Resources. os_resources: Arc, @@ -46,13 +53,6 @@ pub struct VersionedConstants { // TODO: Consider making this a struct, this will require change the way we access these // values. vm_resource_fee_cost: Arc>, - - // Cairo OS constants. - // Note: if loaded from a json file, there are some assumptions made on its structure. - // See the struct's docstring for more details. - os_constants: Arc, - - pub l2_resource_gas_costs: L2ResourceGasCosts, } impl VersionedConstants { @@ -149,6 +149,13 @@ pub struct L2ResourceGasCosts { pub milligas_per_code_byte: u128, } +#[derive(Clone, Debug, Default, Deserialize)] +pub struct EventSizeLimit { + pub max_data_length: usize, + pub max_keys_length: usize, + pub max_n_emitted_events: usize, +} + #[derive(Clone, Debug, Default, Deserialize)] // Serde trick for adding validations via a customr deserializer, without forgoing the derive. // See: https://github.com/serde-rs/serde/issues/1220.