From 2a2be69511ef01dd771d31f6ca9be5562e5df8d3 Mon Sep 17 00:00:00 2001 From: Arni Hod Date: Wed, 26 Jun 2024 11:43:46 +0300 Subject: [PATCH] feat: post compilation size limit validation --- config/mempool_default_config.json | 10 +++ crates/gateway/src/compilation.rs | 71 ++++++++++++++----- crates/gateway/src/compilation_test.rs | 23 ++++++ crates/gateway/src/config.rs | 37 ++++++++-- crates/gateway/src/errors.rs | 13 ++++ crates/gateway/src/gateway_test.rs | 11 +-- .../stateful_transaction_validator_test.rs | 2 +- 7 files changed, 136 insertions(+), 31 deletions(-) diff --git a/config/mempool_default_config.json b/config/mempool_default_config.json index 42fa1ecad..08dc95809 100644 --- a/config/mempool_default_config.json +++ b/config/mempool_default_config.json @@ -9,6 +9,16 @@ "privacy": "Public", "value": true }, + "gateway_config.compiler_config.max_casm_bytecode_size": { + "description": "Limitation of contract bytecode size.", + "privacy": "Public", + "value": 81920 + }, + "gateway_config.compiler_config.max_raw_casm_class_size": { + "description": "Limitation of contract class object size.", + "privacy": "Public", + "value": 4089446 + }, "gateway_config.network_config.ip": { "description": "The gateway server ip.", "privacy": "Public", diff --git a/crates/gateway/src/compilation.rs b/crates/gateway/src/compilation.rs index e1c4ea213..cfdc321ce 100644 --- a/crates/gateway/src/compilation.rs +++ b/crates/gateway/src/compilation.rs @@ -21,9 +21,9 @@ use crate::utils::is_subsequence; mod compilation_test; // TODO(Arni): Pass the compiler with dependancy injection. +// TODO(Define a function for `compile_contract_class` - which ignores the `config` parameter). #[derive(Clone)] pub struct GatewayCompiler { - #[allow(dead_code)] pub config: GatewayCompilerConfig, } @@ -42,7 +42,7 @@ impl GatewayCompiler { let casm_contract_class = self.compile(cairo_lang_contract_class)?; validate_compiled_class_hash(&casm_contract_class, &tx.compiled_class_hash)?; - validate_casm_class(&casm_contract_class)?; + self.validate_casm(&casm_contract_class)?; Ok(ClassInfo::new( &ContractClass::V1(ContractClassV1::try_from(casm_contract_class)?), @@ -63,24 +63,63 @@ impl GatewayCompiler { Ok(casm_contract_class) } -} -// TODO(Arni): Add test. -fn validate_casm_class(contract_class: &CasmContractClass) -> Result<(), GatewayError> { - let CasmContractEntryPoints { external, l1_handler, constructor } = - &contract_class.entry_points_by_type; - let entry_points_iterator = external.iter().chain(l1_handler.iter()).chain(constructor.iter()); - - for entry_point in entry_points_iterator { - let builtins = &entry_point.builtins; - if !is_subsequence(builtins, supported_builtins()) { - return Err(GatewayError::UnsupportedBuiltins { - builtins: builtins.clone(), - supported_builtins: supported_builtins().to_vec(), + fn validate_casm(&self, casm_contract_class: &CasmContractClass) -> Result<(), GatewayError> { + self.validate_casm_class(casm_contract_class)?; + self.validate_casm_class_size(casm_contract_class)?; + Ok(()) + } + + // TODO(Arni): Add test. + /// Validates that the Casm class is structured correctly. Specifically, this function ensures + /// that the builtins used by this class are supported and ordered correctly, as expected by + /// the OS. + fn validate_casm_class(&self, contract_class: &CasmContractClass) -> Result<(), GatewayError> { + let CasmContractEntryPoints { external, l1_handler, constructor } = + &contract_class.entry_points_by_type; + let entry_points_iterator = + external.iter().chain(l1_handler.iter()).chain(constructor.iter()); + + for entry_point in entry_points_iterator { + let builtins = &entry_point.builtins; + if !is_subsequence(builtins, supported_builtins()) { + return Err(GatewayError::UnsupportedBuiltins { + builtins: builtins.clone(), + supported_builtins: supported_builtins().to_vec(), + }); + } + } + Ok(()) + } + + // TODO(Arni): consider validating the size of other members of the Casm class. Cosider removing + // the validation of the raw class size. The validation should be linked to the way the class is + // saved in Papyrus etc. + /// Validates that the Casm class is within size limit. Specifically, this function validates + /// the size of the bytecode and the serialized class. + fn validate_casm_class_size( + &self, + casm_contract_class: &CasmContractClass, + ) -> Result<(), GatewayError> { + let bytecode_size = casm_contract_class.bytecode.len(); + if bytecode_size > self.config.max_casm_bytecode_size { + return Err(GatewayError::CasmBytecodeSizeTooLarge { + bytecode_size, + max_bytecode_size: self.config.max_casm_bytecode_size, + }); + } + let contract_class_object_size = serde_json::to_string(&casm_contract_class) + .expect("Unexpected error serializing Casm contract class.") + .len(); + if contract_class_object_size > self.config.max_raw_casm_class_size { + return Err(GatewayError::CasmContractClassObjectSizeTooLarge { + contract_class_object_size, + max_contract_class_object_size: self.config.max_raw_casm_class_size, }); } + + Ok(()) } - Ok(()) } // TODO(Arni): Add to a config. diff --git a/crates/gateway/src/compilation_test.rs b/crates/gateway/src/compilation_test.rs index c7a364e8c..6f7f3576e 100644 --- a/crates/gateway/src/compilation_test.rs +++ b/crates/gateway/src/compilation_test.rs @@ -8,6 +8,7 @@ use starknet_api::rpc_transaction::{RPCDeclareTransaction, RPCTransaction}; use starknet_sierra_compile::errors::CompilationUtilError; use crate::compilation::GatewayCompiler; +use crate::config::GatewayCompilerConfig; use crate::errors::GatewayError; #[fixture] @@ -43,6 +44,28 @@ fn test_compile_contract_class_compiled_class_hash_mismatch( ); } +// TODO(Arni): Redesign this test once the compiler is passed with dependancy injection. +#[rstest] +fn test_compile_contract_class_bytecode_size_validation(declare_tx: RPCDeclareTransaction) { + let gateway_compiler = GatewayCompiler { + config: GatewayCompilerConfig { max_casm_bytecode_size: 1, ..Default::default() }, + }; + + let result = gateway_compiler.process_declare_tx(&declare_tx); + assert_matches!(result.unwrap_err(), GatewayError::CasmBytecodeSizeTooLarge { .. }) +} + +// TODO(Arni): Redesign this test once the compiler is passed with dependancy injection. +#[rstest] +fn test_compile_contract_class_raw_class_size_validation(declare_tx: RPCDeclareTransaction) { + let gateway_compiler = GatewayCompiler { + config: GatewayCompilerConfig { max_raw_casm_class_size: 1, ..Default::default() }, + }; + + let result = gateway_compiler.process_declare_tx(&declare_tx); + assert_matches!(result.unwrap_err(), GatewayError::CasmContractClassObjectSizeTooLarge { .. }) +} + #[rstest] fn test_compile_contract_class_bad_sierra( gateway_compiler: GatewayCompiler, diff --git a/crates/gateway/src/config.rs b/crates/gateway/src/config.rs index 49970fa7a..e46f28f01 100644 --- a/crates/gateway/src/config.rs +++ b/crates/gateway/src/config.rs @@ -11,6 +11,9 @@ use validator::Validate; use crate::compiler_version::VersionId; +const MAX_BYTECODE_SIZE: usize = 81920; +const MAX_RAW_CLASS_SIZE: usize = 4089446; + #[derive(Clone, Debug, Default, Serialize, Deserialize, Validate, PartialEq)] pub struct GatewayConfig { pub network_config: GatewayNetworkConfig, @@ -88,8 +91,8 @@ impl Default for StatelessTransactionValidatorConfig { validate_non_zero_l2_gas_fee: false, max_calldata_length: 4000, max_signature_length: 4000, - max_bytecode_size: 81920, - max_raw_class_size: 4089446, + max_bytecode_size: MAX_BYTECODE_SIZE, + max_raw_class_size: MAX_RAW_CLASS_SIZE, min_sierra_version: VersionId { major: 1, minor: 1, patch: 0 }, max_sierra_version: VersionId { major: 1, minor: 5, patch: usize::MAX }, } @@ -295,12 +298,36 @@ impl StatefulTransactionValidatorConfig { } } } +#[derive(Clone, Copy, Debug, Serialize, Deserialize, Validate, PartialEq)] +pub struct GatewayCompilerConfig { + pub max_casm_bytecode_size: usize, + pub max_raw_casm_class_size: usize, +} -#[derive(Clone, Copy, Debug, Default, Serialize, Deserialize, Validate, PartialEq)] -pub struct GatewayCompilerConfig {} +impl Default for GatewayCompilerConfig { + fn default() -> Self { + Self { + max_casm_bytecode_size: MAX_BYTECODE_SIZE, + max_raw_casm_class_size: MAX_RAW_CLASS_SIZE, + } + } +} impl SerializeConfig for GatewayCompilerConfig { fn dump(&self) -> BTreeMap { - BTreeMap::new() + BTreeMap::from_iter([ + ser_param( + "max_casm_bytecode_size", + &self.max_casm_bytecode_size, + "Limitation of contract bytecode size.", + ParamPrivacyInput::Public, + ), + ser_param( + "max_raw_casm_class_size", + &self.max_raw_casm_class_size, + "Limitation of contract class object size.", + ParamPrivacyInput::Public, + ), + ]) } } diff --git a/crates/gateway/src/errors.rs b/crates/gateway/src/errors.rs index 0ea24fba0..3a83e97e3 100644 --- a/crates/gateway/src/errors.rs +++ b/crates/gateway/src/errors.rs @@ -19,6 +19,19 @@ use crate::compiler_version::{VersionId, VersionIdError}; /// Errors directed towards the end-user, as a result of gateway requests. #[derive(Debug, Error)] pub enum GatewayError { + #[error( + "Cannot declare Casm contract class with bytecode size of {bytecode_size}; max allowed \ + size: {max_bytecode_size}." + )] + CasmBytecodeSizeTooLarge { bytecode_size: usize, max_bytecode_size: usize }, + #[error( + "Cannot declare Casm contract class with size of {contract_class_object_size}; max \ + allowed size: {max_contract_class_object_size}." + )] + CasmContractClassObjectSizeTooLarge { + contract_class_object_size: usize, + max_contract_class_object_size: usize, + }, #[error(transparent)] CompilationError(#[from] CompilationUtilError), #[error( diff --git a/crates/gateway/src/gateway_test.rs b/crates/gateway/src/gateway_test.rs index d88d55cbb..4ebbca672 100644 --- a/crates/gateway/src/gateway_test.rs +++ b/crates/gateway/src/gateway_test.rs @@ -30,19 +30,12 @@ pub fn app_state( ) -> AppState { AppState { stateless_tx_validator: StatelessTransactionValidator { - config: StatelessTransactionValidatorConfig { - validate_non_zero_l1_gas_fee: true, - max_calldata_length: 10, - max_signature_length: 2, - max_bytecode_size: 10000, - max_raw_class_size: 1000000, - ..Default::default() - }, + config: StatelessTransactionValidatorConfig::default(), }, stateful_tx_validator: Arc::new(StatefulTransactionValidator { config: StatefulTransactionValidatorConfig::create_for_testing(), }), - gateway_compiler: GatewayCompiler { config: GatewayCompilerConfig {} }, + gateway_compiler: GatewayCompiler { config: GatewayCompilerConfig::default() }, state_reader_factory: Arc::new(state_reader_factory), mempool_client, } diff --git a/crates/gateway/src/stateful_transaction_validator_test.rs b/crates/gateway/src/stateful_transaction_validator_test.rs index 4dc509fbf..0e90f3ef2 100644 --- a/crates/gateway/src/stateful_transaction_validator_test.rs +++ b/crates/gateway/src/stateful_transaction_validator_test.rs @@ -75,7 +75,7 @@ fn test_stateful_tx_validator( ) { let optional_class_info = match &external_tx { RPCTransaction::Declare(declare_tx) => Some( - GatewayCompiler { config: GatewayCompilerConfig {} } + GatewayCompiler { config: GatewayCompilerConfig::default() } .process_declare_tx(declare_tx) .unwrap(), ),