Skip to content

Commit 485574f

Browse files
committed
blockifier: add validation for contract class manager config
1 parent ef2714e commit 485574f

File tree

4 files changed

+60
-6
lines changed

4 files changed

+60
-6
lines changed

crates/blockifier/src/blockifier/config.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ impl SerializeConfig for WorkerPoolConfig {
139139

140140
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize, Validate)]
141141
pub struct ContractClassManagerConfig {
142+
#[validate(nested)]
142143
pub cairo_native_run_config: CairoNativeRunConfig,
143144
pub contract_cache_size: usize,
144145
#[validate(nested)]
@@ -241,7 +242,8 @@ impl NativeClassesWhitelist {
241242
}
242243
}
243244

244-
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
245+
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize, Validate)]
246+
#[validate(schema(function = "validate_run_cairo_native"))]
245247
pub struct CairoNativeRunConfig {
246248
pub run_cairo_native: bool,
247249
pub wait_on_native_compilation: bool,
@@ -302,3 +304,16 @@ impl SerializeConfig for CairoNativeRunConfig {
302304
])
303305
}
304306
}
307+
308+
fn validate_run_cairo_native(
309+
cairo_native_run_config: &CairoNativeRunConfig,
310+
) -> Result<(), validator::ValidationError> {
311+
if cairo_native_run_config.wait_on_native_compilation
312+
&& !cairo_native_run_config.run_cairo_native
313+
{
314+
return Err(validator::ValidationError::new(
315+
"`wait_on_native_compilation` requires `run_cairo_native` to be enabled",
316+
));
317+
}
318+
Ok(())
319+
}

crates/blockifier/src/blockifier/config_test.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
use rstest::rstest;
22
use serde_json;
33
use starknet_api::class_hash;
4+
use validator::Validate;
45

5-
use crate::blockifier::config::NativeClassesWhitelist;
6+
use crate::blockifier::config::{CairoNativeRunConfig, NativeClassesWhitelist};
67

78
#[rstest]
89
#[case::all(NativeClassesWhitelist::All)]
@@ -14,3 +15,19 @@ fn test_native_classes_whitelist_serializes_and_back(#[case] value: NativeClasse
1415
let deserialized: NativeClassesWhitelist = serde_json::from_str(&serialized).unwrap();
1516
assert_eq!(deserialized, value);
1617
}
18+
19+
#[rstest]
20+
#[case(true, true, true)]
21+
#[case(true, false, true)]
22+
#[case(false, true, false)]
23+
#[case(false, false, true)]
24+
fn test_validate_run_cairo_native(
25+
#[case] run_cairo_native: bool,
26+
#[case] wait_on_native_compilation: bool,
27+
#[case] should_be_valid: bool,
28+
) {
29+
let config =
30+
CairoNativeRunConfig { run_cairo_native, wait_on_native_compilation, ..Default::default() };
31+
let result = config.validate();
32+
assert_eq!(result.is_ok(), should_be_valid, "unexpected validation result {result:?}");
33+
}

crates/blockifier/src/state/native_class_manager.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use starknet_api::class_cache::GlobalContractCache;
1212
use starknet_api::core::{ClassHash, CompiledClassHash};
1313
use starknet_api::state::SierraContractClass;
1414
use thiserror::Error;
15+
use validator::Validate;
1516

1617
use crate::blockifier::config::{CairoNativeRunConfig, ContractClassManagerConfig};
1718
use crate::execution::contract_class::{CompiledClassV1, RunnableCompiledClass};
@@ -64,6 +65,7 @@ impl NativeClassManager {
6465
/// 2. `config.run_cairo_native` is `false`.
6566
/// 3. `config.wait_on_native_compilation` is `true`.
6667
pub fn start(config: ContractClassManagerConfig) -> NativeClassManager {
68+
config.validate().expect("Invalid contract class manager config");
6769
// TODO(Avi, 15/12/2024): Add the size of the channel to the config.
6870
let class_cache = RawClassCache::new(config.contract_cache_size);
6971
let compiled_class_hash_v2_cache = GlobalContractCache::new(config.contract_cache_size);
@@ -122,8 +124,6 @@ impl NativeClassManager {
122124
if let CompiledClasses::V1(..) = cached_class {
123125
// When `wait_on_native_compilation` is active, all V1 classes should have been compiled
124126
// to native synchronously. A V1 cache entry indicates a pipeline bug.
125-
// TODO(Yoni): make sure `wait_on_native_compilation` cannot be set to true while
126-
// `run_cairo_native` is false.
127127
assert!(
128128
!self.wait_on_native_compilation(),
129129
"Manager did not wait on native compilation."
@@ -154,7 +154,6 @@ impl NativeClassManager {
154154
CompiledClasses::V1(compiled_class_v1, sierra_contract_class) => {
155155
// TODO(Yoni): instead of these two flag, use an enum.
156156
if self.wait_on_native_compilation() {
157-
assert!(self.run_cairo_native(), "Native compilation is disabled.");
158157
let compiler = self.compiler.as_ref().expect("Compiler not available.");
159158
// After this point, the Native class should be cached and available through
160159
// `get_runnable` access.

crates/blockifier/src/state/native_class_manager_test.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ use rstest::rstest;
1111
use starknet_api::class_cache::GlobalContractCache;
1212
use starknet_api::core::ClassHash;
1313

14-
use crate::blockifier::config::{CairoNativeRunConfig, NativeClassesWhitelist};
14+
use crate::blockifier::config::{
15+
CairoNativeRunConfig,
16+
ContractClassManagerConfig,
17+
NativeClassesWhitelist,
18+
};
1519
use crate::execution::contract_class::{CompiledClassV1, RunnableCompiledClass};
1620
use crate::state::global_cache::{
1721
CachedCairoNative,
@@ -29,6 +33,25 @@ use crate::test_utils::contracts::FeatureContractTrait;
2933

3034
const TEST_CHANNEL_SIZE: usize = 10;
3135

36+
#[rstest]
37+
#[should_panic(expected = "Invalid contract class manager config:")]
38+
#[case::invalid_contract_cache_size(ContractClassManagerConfig { contract_cache_size: 0, ..Default::default() })]
39+
#[should_panic(expected = "Invalid contract class manager config:")]
40+
#[case::invalid_cairo_native_run_config(ContractClassManagerConfig {
41+
cairo_native_run_config: CairoNativeRunConfig {
42+
run_cairo_native: false,
43+
wait_on_native_compilation: true,
44+
..Default::default()
45+
},
46+
..Default::default()
47+
})]
48+
fn test_start_invalid_contract_class_manager_config_panics(
49+
#[case] config: ContractClassManagerConfig,
50+
) {
51+
// This should panic after failing validation.
52+
let _manager = NativeClassManager::start(config.clone());
53+
}
54+
3255
fn get_casm(test_contract: FeatureContract) -> CompiledClassV1 {
3356
match test_contract.get_runnable_class() {
3457
RunnableCompiledClass::V1(casm) => casm,

0 commit comments

Comments
 (0)