-
Notifications
You must be signed in to change notification settings - Fork 32
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
Ft/os upgrade #453
Ft/os upgrade #453
Changes from 18 commits
494b307
e4a4665
b817540
085e43c
3fc423f
a9e55ec
fbe56b8
e9de05e
ec3fdf0
213adde
8184adb
d7a8a7a
a394e9e
96d4c29
ee1cbad
feda12c
bd1ee68
8b1aed8
106b2a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,7 @@ const DEFAULT_COMPILED_OS: &[u8] = include_bytes!("../../../../build/os_latest.j | |
#[case::inconsistent_cairo0_class_hash_1(204936)] | ||
#[case::no_possible_convertion_1(155007)] | ||
#[case::no_possible_convertion_2(155029)] | ||
#[case::reference_pie_with_full_output_enabled(173404)] | ||
// #[case::reference_pie_with_full_output_enabled(173404)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dont remove this test: Fix: Find a new v0.13.3 PIE (we need them form Starkware) and modify this test to check that Block and output. |
||
#[case::inconsistent_cairo0_class_hash_2(159674)] | ||
#[case::inconsistent_cairo0_class_hash_3(164180)] | ||
#[case::key_not_in_proof_0(155087)] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,8 +19,8 @@ pub const fn default_layout() -> LayoutName { | |
LayoutName::all_cairo | ||
} | ||
|
||
// https://github.com/starkware-libs/blockifier/blob/8da582b285bfbc7d4c21178609bbd43f80a69240/crates/native_blockifier/src/py_block_executor.rs#L44 | ||
const MAX_STEPS_PER_TX: u32 = 4_000_000; | ||
// The following values were taken from general_config.yml from cairo-lang | ||
const VALIDATE_MAX_N_STEPS_OVERRIDE: u32 = 1000000; | ||
|
||
const DEFAULT_CONFIG_PATH: &str = "../../cairo-lang/src/starkware/starknet/definitions/general_config.yml"; | ||
pub const STORED_BLOCK_HASH_BUFFER: u64 = 10; | ||
|
@@ -32,9 +32,9 @@ pub const COMPILED_CLASS_HASH_COMMITMENT_TREE_HEIGHT: usize = 251; | |
pub const CONTRACT_STATES_COMMITMENT_TREE_HEIGHT: usize = 251; | ||
pub const DEFAULT_INNER_TREE_HEIGHT: u64 = 64; | ||
// TODO: update with relevant address | ||
pub const DEFAULT_FEE_TOKEN_ADDR: &str = "482bc27fc5627bf974a72b65c43aa8a0464a70aab91ad8379b56a4f17a84c3"; | ||
pub const DEFAULT_DEPRECATED_FEE_TOKEN_ADDR: &str = "482bc27fc5627bf974a72b65c43aa8a0464a70aab91ad8379b56a4f17a84c3"; | ||
pub const SEQUENCER_ADDR_0_13_2: &str = "0x795488c127693ffb36733cc054f9e2be39241a794a4877dc8fc1dbe52750488"; | ||
pub const DEFAULT_FEE_TOKEN_ADDR: &str = "7ce4aa542d72a82662cda96b147da9b041ecf8c61f67ef657f3bbb852fc698f"; | ||
pub const DEFAULT_DEPRECATED_FEE_TOKEN_ADDR: &str = "5195ba458d98a8d5a390afa87e199566e473d1124c07a3c57bf19813255ac41"; | ||
pub const SEQUENCER_ADDR_0_13_3: &str = "0x31c641e041f8d25997985b0efe68d0c5ce89d418ca9a127ae043aebed6851c5"; | ||
pub const CONTRACT_ADDRESS_BITS: usize = 251; | ||
pub const CONTRACT_CLASS_LEAF_VERSION: &[u8] = "CONTRACT_CLASS_LEAF_V0".as_bytes(); | ||
|
||
|
@@ -67,8 +67,7 @@ const fn default_use_kzg_da() -> bool { | |
pub struct StarknetGeneralConfig { | ||
pub starknet_os_config: StarknetOsConfig, | ||
pub gas_price_bounds: GasPriceBounds, | ||
pub invoke_tx_max_n_steps: u32, | ||
pub validate_max_n_steps: u32, | ||
pub validate_max_n_steps_override: u32, | ||
pub default_eth_price_in_fri: u128, | ||
pub sequencer_address: ContractAddress, | ||
pub enforce_l1_handler_fee: bool, | ||
|
@@ -92,10 +91,9 @@ impl Default for StarknetGeneralConfig { | |
min_wei_l1_data_gas_price: 100000, | ||
min_wei_l1_gas_price: 10000000000, | ||
}, | ||
invoke_tx_max_n_steps: MAX_STEPS_PER_TX, | ||
validate_max_n_steps: MAX_STEPS_PER_TX, | ||
validate_max_n_steps_override: VALIDATE_MAX_N_STEPS_OVERRIDE, | ||
default_eth_price_in_fri: 1_000_000_000_000_000_000_000, | ||
sequencer_address: contract_address!(SEQUENCER_ADDR_0_13_2), | ||
sequencer_address: contract_address!(SEQUENCER_ADDR_0_13_3), | ||
enforce_l1_handler_fee: true, | ||
use_kzg_da: false, | ||
} | ||
|
@@ -114,8 +112,7 @@ impl StarknetGeneralConfig { | |
|
||
pub fn empty_block_context(&self) -> BlockContext { | ||
let mut versioned_constants = VersionedConstants::default(); | ||
versioned_constants.invoke_tx_max_n_steps = self.invoke_tx_max_n_steps; | ||
versioned_constants.validate_max_n_steps = self.validate_max_n_steps; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dont you need to change it for: |
||
|
||
versioned_constants.max_recursion_depth = 50; | ||
|
||
let block_info = BlockInfo { | ||
|
@@ -171,15 +168,14 @@ mod tests { | |
|
||
#[test] | ||
fn parse_starknet_config() { | ||
let expected_seq_addr = contract_address!(SEQUENCER_ADDR_0_13_2); | ||
let expected_seq_addr = contract_address!(SEQUENCER_ADDR_0_13_3); | ||
|
||
let conf = StarknetGeneralConfig::from_default_file().expect("Failed to load default config file"); | ||
|
||
assert!(conf.enforce_l1_handler_fee); | ||
|
||
assert_eq!(1000000, conf.invoke_tx_max_n_steps); | ||
assert_eq!(1000000000000000000000, conf.default_eth_price_in_fri); | ||
assert_eq!(1000000, conf.validate_max_n_steps); | ||
assert_eq!(1000000, conf.validate_max_n_steps_override); | ||
|
||
assert_eq!(expected_seq_addr, conf.sequencer_address); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ use indoc::indoc; | |
use num_integer::div_ceil; | ||
|
||
use crate::hints::vars; | ||
use crate::utils::get_variable_from_root_exec_scope; | ||
use crate::utils::{get_constant, get_variable_from_root_exec_scope}; | ||
|
||
const MAX_PAGE_SIZE: usize = 3800; | ||
|
||
|
@@ -105,21 +105,31 @@ pub fn set_tree_structure( | |
Ok(()) | ||
} | ||
|
||
pub const SET_STATE_UPDATES_START: &str = indoc! {r#"if ids.use_kzg_da: | ||
ids.state_updates_start = segments.add() | ||
else: | ||
# Assign a temporary segment, to be relocated into the output segment. | ||
ids.state_updates_start = segments.add_temp_segment()"#}; | ||
pub const SET_STATE_UPDATES_START: &str = indoc! {r#"# `use_kzg_da` is used in a hint in `process_data_availability`. | ||
use_kzg_da = ids.use_kzg_da | ||
if use_kzg_da or ids.compress_state_updates: | ||
ids.state_updates_start = segments.add() | ||
else: | ||
# Assign a temporary segment, to be relocated into the output segment. | ||
ids.state_updates_start = segments.add_temp_segment()"#}; | ||
|
||
pub fn set_state_updates_start( | ||
vm: &mut VirtualMachine, | ||
_exec_scopes: &mut ExecutionScopes, | ||
exec_scopes: &mut ExecutionScopes, | ||
ids_data: &HashMap<String, HintReference>, | ||
ap_tracking: &ApTracking, | ||
_constants: &HashMap<String, Felt252>, | ||
) -> Result<(), HintError> { | ||
let use_kzg_da_felt = get_integer_from_var_name(vars::ids::USE_KZG_DA, vm, ids_data, ap_tracking)?; | ||
|
||
// Set `use_kzg_da` in globals since it will be used in `process_data_availability` | ||
exec_scopes.insert_value(vars::scopes::USE_KZG_DA, use_kzg_da_felt); | ||
|
||
// Recompute `compress_state_updates` until this issue is fixed | ||
// https://github.com/lambdaclass/cairo-vm/issues/1897 | ||
let full_output = get_integer_from_var_name(vars::ids::FULL_OUTPUT, vm, ids_data, ap_tracking)?; | ||
let compress_state_updates = Felt252::ONE - full_output; | ||
|
||
let use_kzg_da = if use_kzg_da_felt == Felt252::ONE { | ||
Ok(true) | ||
} else if use_kzg_da_felt == Felt252::ZERO { | ||
|
@@ -128,7 +138,15 @@ pub fn set_state_updates_start( | |
Err(HintError::CustomHint("ids.use_kzg_da is not a boolean".to_string().into_boxed_str())) | ||
}?; | ||
|
||
if use_kzg_da { | ||
let use_compress_state_updates = if compress_state_updates == Felt252::ONE { | ||
Ok(true) | ||
} else if compress_state_updates == Felt252::ZERO { | ||
Ok(false) | ||
} else { | ||
Err(HintError::CustomHint("ids.compress_state_updates is not a boolean".to_string().into_boxed_str())) | ||
}?; | ||
|
||
if use_kzg_da || use_compress_state_updates { | ||
insert_value_from_var_name(vars::ids::STATE_UPDATES_START, vm.add_memory_segment(), vm, ids_data, ap_tracking)?; | ||
} else { | ||
// Assign a temporary segment, to be relocated into the output segment. | ||
|
@@ -144,6 +162,58 @@ pub fn set_state_updates_start( | |
Ok(()) | ||
} | ||
|
||
pub const SET_COMPRESSED_START: &str = indoc! {r#"if use_kzg_da: | ||
ids.compressed_start = segments.add() | ||
else: | ||
# Assign a temporary segment, to be relocated into the output segment. | ||
ids.compressed_start = segments.add_temp_segment()"#}; | ||
|
||
pub fn set_compressed_start( | ||
vm: &mut VirtualMachine, | ||
exec_scopes: &mut ExecutionScopes, | ||
ids_data: &HashMap<String, HintReference>, | ||
ap_tracking: &ApTracking, | ||
_constants: &HashMap<String, Felt252>, | ||
) -> Result<(), HintError> { | ||
let use_kzg_da_felt = exec_scopes.get::<Felt252>(vars::scopes::USE_KZG_DA)?; | ||
|
||
let use_kzg_da = if use_kzg_da_felt == Felt252::ONE { | ||
Ok(true) | ||
} else if use_kzg_da_felt == Felt252::ZERO { | ||
Ok(false) | ||
} else { | ||
Err(HintError::CustomHint("ids.use_kzg_da is not a boolean".to_string().into_boxed_str())) | ||
}?; | ||
Comment on lines
+171
to
+186
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add unit test for this hint |
||
|
||
if use_kzg_da { | ||
insert_value_from_var_name(vars::ids::COMPRESSED_START, vm.add_memory_segment(), vm, ids_data, ap_tracking)?; | ||
} else { | ||
// Assign a temporary segment, to be relocated into the output segment. | ||
insert_value_from_var_name(vars::ids::COMPRESSED_START, vm.add_temporary_segment(), vm, ids_data, ap_tracking)?; | ||
} | ||
|
||
Ok(()) | ||
} | ||
|
||
pub const SET_N_UPDATES_SMALL: &str = | ||
indoc! {r#"ids.is_n_updates_small = ids.n_actual_updates < ids.N_UPDATES_SMALL_PACKING_BOUND"#}; | ||
|
||
pub fn set_n_updates_small( | ||
vm: &mut VirtualMachine, | ||
_exec_scopes: &mut ExecutionScopes, | ||
ids_data: &HashMap<String, HintReference>, | ||
ap_tracking: &ApTracking, | ||
constants: &HashMap<String, Felt252>, | ||
) -> Result<(), HintError> { | ||
let n_actual_updates = get_integer_from_var_name(vars::ids::N_ACTUAL_UPDATES, vm, ids_data, ap_tracking)?; | ||
let n_updates_small_packing_bound = get_constant(vars::ids::N_UPDATES_SMALL_PACKING_BOUND, constants)?; | ||
|
||
let is_n_updates_small = | ||
if n_actual_updates < *n_updates_small_packing_bound { Felt252::ONE } else { Felt252::ZERO }; | ||
|
||
insert_value_from_var_name(vars::ids::IS_N_UPDATES_SMALL, is_n_updates_small, vm, ids_data, ap_tracking) | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use cairo_vm::hint_processor::builtin_hint_processor::hint_utils::insert_value_from_var_name; | ||
|
@@ -215,4 +285,103 @@ mod tests { | |
let gps_fact_topology = attributes.get("gps_fact_topology").unwrap(); | ||
assert_eq!(gps_fact_topology, &vec![1 + n_expected_pages, n_expected_pages, 0, 2]); | ||
} | ||
|
||
use rstest::rstest; | ||
|
||
#[rstest] | ||
// small updates | ||
#[case(10, 1)] | ||
#[case(255, 1)] | ||
// big updates | ||
#[case(256, 0)] | ||
#[case(1024, 0)] | ||
|
||
fn test_set_n_updates_small_parameterized(#[case] actual_updates: u64, #[case] expected_is_n_updates_small: u64) { | ||
let mut vm = VirtualMachine::new(false); | ||
vm.add_memory_segment(); | ||
vm.add_memory_segment(); | ||
vm.set_fp(2); | ||
let ap_tracking = ApTracking::new(); | ||
let constants = | ||
HashMap::from([(vars::ids::N_UPDATES_SMALL_PACKING_BOUND.to_string(), Felt252::from(1u128 << 8))]); | ||
let ids_data = HashMap::from([ | ||
(vars::ids::N_ACTUAL_UPDATES.to_string(), HintReference::new_simple(-2)), | ||
(vars::ids::IS_N_UPDATES_SMALL.to_string(), HintReference::new_simple(-1)), | ||
]); | ||
vm.insert_value(Relocatable::from((1, 0)), Felt252::from(actual_updates)).unwrap(); | ||
let mut exec_scopes: ExecutionScopes = Default::default(); | ||
|
||
set_n_updates_small(&mut vm, &mut exec_scopes, &ids_data, &ap_tracking, &constants).unwrap(); | ||
let is_n_updates_small = | ||
get_integer_from_var_name(vars::ids::IS_N_UPDATES_SMALL, &vm, &ids_data, &ap_tracking).unwrap(); | ||
assert_eq!(Felt252::from(expected_is_n_updates_small), is_n_updates_small); | ||
} | ||
|
||
#[rstest] | ||
#[case(0, 0)] | ||
#[case(0, 1)] | ||
#[case(1, 0)] | ||
#[case(0, 1)] | ||
fn test_set_state_updates_start(#[case] use_kzg_da: u64, #[case] full_output: u64) { | ||
let mut vm = VirtualMachine::new(false); | ||
vm.add_memory_segment(); | ||
vm.add_memory_segment(); | ||
vm.set_fp(3); | ||
let ap_tracking = ApTracking::new(); | ||
let constants = | ||
HashMap::from([(vars::ids::N_UPDATES_SMALL_PACKING_BOUND.to_string(), Felt252::from(1u128 << 8))]); | ||
|
||
let ids_data = HashMap::from([ | ||
(vars::ids::USE_KZG_DA.to_string(), HintReference::new_simple(-3)), | ||
(vars::ids::FULL_OUTPUT.to_string(), HintReference::new_simple(-2)), | ||
(vars::ids::STATE_UPDATES_START.to_string(), HintReference::new_simple(-1)), | ||
]); | ||
|
||
insert_value_from_var_name(vars::ids::USE_KZG_DA, Felt252::from(use_kzg_da), &mut vm, &ids_data, &ap_tracking) | ||
.unwrap(); | ||
|
||
insert_value_from_var_name( | ||
vars::ids::FULL_OUTPUT, | ||
Felt252::from(full_output), | ||
&mut vm, | ||
&ids_data, | ||
&ap_tracking, | ||
) | ||
.unwrap(); | ||
|
||
let mut exec_scopes: ExecutionScopes = Default::default(); | ||
|
||
set_state_updates_start(&mut vm, &mut exec_scopes, &ids_data, &ap_tracking, &constants).unwrap(); | ||
|
||
// Temp segment will be used only when full_output = 1 and use_kzg_da = 0 | ||
if (use_kzg_da, full_output) == (0, 1) { | ||
assert_eq!(vm.segments.num_temp_segments(), 1); | ||
} else { | ||
assert_eq!(vm.segments.num_temp_segments(), 0); | ||
} | ||
} | ||
|
||
#[rstest] | ||
#[case(0)] | ||
#[case(1)] | ||
fn test_set_compressed_start(#[case] use_kzg_da: u64) { | ||
let mut vm = VirtualMachine::new(false); | ||
vm.add_memory_segment(); | ||
vm.add_memory_segment(); | ||
vm.set_fp(1); | ||
let ap_tracking = ApTracking::new(); | ||
let constants = HashMap::new(); | ||
let mut exec_scopes: ExecutionScopes = Default::default(); | ||
let ids_data = HashMap::from([(vars::ids::COMPRESSED_START.to_string(), HintReference::new_simple(-1))]); | ||
|
||
exec_scopes.insert_value(vars::scopes::USE_KZG_DA, Felt252::from(use_kzg_da)); | ||
|
||
set_compressed_start(&mut vm, &mut exec_scopes, &ids_data, &ap_tracking, &constants).unwrap(); | ||
|
||
if use_kzg_da == 0 { | ||
assert_eq!(vm.segments.num_temp_segments(), 1); | ||
} else { | ||
assert_eq!(vm.segments.num_temp_segments(), 0); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this for:
StarknetVersion::V0_13_2_1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was not available in the Blockifier version that we're using. I can double check this but updating Blockifier to a commit where it's defined lead to some extra changes that are meant for os v0.13.4 and are not needed