diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 05aedbc30..67ffa7a84 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -31,7 +31,7 @@ jobs: - name: "Install cairo-lang" run: | # We need to pin the version of sympy, 1.13.0 is incompatible with cairo-lang - pip install cairo-lang==0.13.2 "sympy<1.13.0" + pip install cairo-lang==0.13.3 "sympy<1.13.0" - name: "Prepare test environment" run: | mkdir -p build @@ -53,7 +53,7 @@ jobs: - name: "Install cairo-lang" run: | # We need to pin the version of sympy, 1.13.0 is incompatible with cairo-lang - pip install cairo-lang==0.13.2 "sympy<1.13.0" + pip install cairo-lang==0.13.3 "sympy<1.13.0" - name: "Prepare test environment" run: | bash ./setup-scripts/reset-tests.sh @@ -78,21 +78,10 @@ jobs: - name: "Install cairo-lang" run: | # We need to pin the version of sympy, 1.13.0 is incompatible with cairo-lang - pip install cairo-lang==0.13.2 "sympy<1.13.0" + pip install cairo-lang==0.13.3 "sympy<1.13.0" - name: "Prepare test environment" run: | mkdir -p build cairo-compile cairo-lang/src/starkware/starknet/core/os/os.cairo --output build/os_latest.json --cairo_path cairo-lang/src - run: cargo install cargo-udeps --locked - run: cargo udeps --all-targets - - orphans: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - run: rustup show - - run: rustup component add rustfmt - - uses: Swatinem/rust-cache@v2 - - run: git submodule update --init - - run: ./scripts/check-orphans.sh - diff --git a/.github/workflows/prove_blocks.yml b/.github/workflows/prove_blocks.yml index 12af39ece..d04f685ce 100644 --- a/.github/workflows/prove_blocks.yml +++ b/.github/workflows/prove_blocks.yml @@ -43,7 +43,7 @@ jobs: - name: Setup the tests run: | source venv/bin/activate - pip install cairo-lang==0.13.2 "sympy<1.13.0" + pip install cairo-lang==0.13.3 "sympy<1.13.0" bash setup-scripts/setup-tests.sh - name: Prove Blocks @@ -57,3 +57,13 @@ jobs: PATHFINDER_RPC_URL: ${{ secrets.PATHFINDER_RPC_URL }} run: | cargo test --release --package prove_block --test hash_tests -- test_recompute_class_hash test_class_proof_verification_ok test_class_proof_verification_non_inclusion --show-output --ignored + + - name: Hint tool Orphans + env: + PATHFINDER_RPC_URL: ${{ secrets.PATHFINDER_RPC_URL }} + run: | + RESULT=$(cargo run --release -p hint_tool -- --subset orphaned --in-file build/os_latest.json | grep -oP '\d+$') + if [ "$RESULT" -gt 1 ]; then + echo "Error: Only breakpoint hint is allowed to be orphaned." + exit 1 + fi diff --git a/cairo-lang b/cairo-lang index a86e92bfd..8e11b8cc6 160000 --- a/cairo-lang +++ b/cairo-lang @@ -1 +1 @@ -Subproject commit a86e92bfde9c171c0856d7b46580c66e004922f3 +Subproject commit 8e11b8cc65ae1d0959328b1b4a40b92df8b58595 diff --git a/crates/bin/prove_block/reference-pies/173404.zip b/crates/bin/prove_block/reference-pies/173404.zip deleted file mode 100644 index 4d4d8dfc4..000000000 Binary files a/crates/bin/prove_block/reference-pies/173404.zip and /dev/null differ diff --git a/crates/bin/prove_block/reference-pies/341097.zip b/crates/bin/prove_block/reference-pies/341097.zip new file mode 100644 index 000000000..b3968991a Binary files /dev/null and b/crates/bin/prove_block/reference-pies/341097.zip differ diff --git a/crates/bin/prove_block/reference-pies/341101.zip b/crates/bin/prove_block/reference-pies/341101.zip new file mode 100644 index 000000000..39a038a52 Binary files /dev/null and b/crates/bin/prove_block/reference-pies/341101.zip differ diff --git a/crates/bin/prove_block/reference-pies/355710.zip b/crates/bin/prove_block/reference-pies/355710.zip new file mode 100644 index 000000000..3bf3a0050 Binary files /dev/null and b/crates/bin/prove_block/reference-pies/355710.zip differ diff --git a/crates/bin/prove_block/reference-pies/355783.zip b/crates/bin/prove_block/reference-pies/355783.zip new file mode 100644 index 000000000..698ac7ff2 Binary files /dev/null and b/crates/bin/prove_block/reference-pies/355783.zip differ diff --git a/crates/bin/prove_block/reference-pies/355802.zip b/crates/bin/prove_block/reference-pies/355802.zip new file mode 100644 index 000000000..998f738b3 Binary files /dev/null and b/crates/bin/prove_block/reference-pies/355802.zip differ diff --git a/crates/bin/prove_block/src/lib.rs b/crates/bin/prove_block/src/lib.rs index f0bd0f0bc..417ef8a35 100644 --- a/crates/bin/prove_block/src/lib.rs +++ b/crates/bin/prove_block/src/lib.rs @@ -149,7 +149,7 @@ pub async fn prove_block( // This is a workaorund to catch the case where the block number is less than the buffer and still preserve the check // The OS will also handle the case where the block number is less than the buffer. let older_block_number = - if block_number <= STORED_BLOCK_HASH_BUFFER { 1 } else { block_number - STORED_BLOCK_HASH_BUFFER }; + if block_number <= STORED_BLOCK_HASH_BUFFER { 0 } else { block_number - STORED_BLOCK_HASH_BUFFER }; let older_block = match rpc_client.starknet_rpc().get_block_with_tx_hashes(BlockId::Number(older_block_number)).await? { @@ -309,8 +309,15 @@ pub async fn prove_block( let updated_root = block_hash_storage_proof.class_commitment.unwrap_or(Felt::ZERO); let previous_root = previous_block_hash_storage_proof.class_commitment.unwrap_or(Felt::ZERO); - let previous_contract_trie_root = previous_block_hash_storage_proof.contract_proof[0].hash::(); - let current_contract_trie_root = block_hash_storage_proof.contract_proof[0].hash::(); + // On devnet and until block 10, the storage_root_idx might be None and that means that contract_proof is empty + let previous_contract_trie_root = match previous_block_hash_storage_proof.contract_proof.first() { + Some(proof) => proof.hash::(), + None => Felt252::ZERO, + }; + let current_contract_trie_root = match block_hash_storage_proof.contract_proof.first() { + Some(proof) => proof.hash::(), + None => Felt252::ZERO, + }; let previous_contract_proofs: Vec<_> = previous_storage_proofs.values().map(|proof| proof.contract_proof.clone()).collect(); diff --git a/crates/bin/prove_block/src/reexecute.rs b/crates/bin/prove_block/src/reexecute.rs index ba33e3ce1..15c491955 100644 --- a/crates/bin/prove_block/src/reexecute.rs +++ b/crates/bin/prove_block/src/reexecute.rs @@ -10,7 +10,7 @@ use blockifier::transaction::objects::TransactionExecutionInfo; use blockifier::transaction::transaction_execution::Transaction; use blockifier::transaction::transactions::ExecutableTransaction; use cairo_vm::Felt252; -use rpc_client::pathfinder::proofs::{PathfinderProof, TrieNode}; +use rpc_client::pathfinder::proofs::{ContractData, PathfinderProof, TrieNode}; use rpc_client::RpcClient; use starknet::core::types::{BlockId, StarknetError}; use starknet::providers::{Provider as _, ProviderError}; @@ -163,8 +163,11 @@ pub(crate) fn format_commitment_facts( impl PerContractStorage for ProverPerContractStorage { async fn compute_commitment(&mut self) -> Result { // TODO: error code - let contract_data = - self.storage_proof.contract_data.as_ref().expect("storage proof should have a contract_data field"); + let contract_data = match self.storage_proof.contract_data.as_ref() { + None => &ContractData::default(), + Some(data) => data, + }; + let updated_root = contract_data.root; let commitment_facts = format_commitment_facts::(&contract_data.storage_proofs); diff --git a/crates/bin/prove_block/src/rpc_utils.rs b/crates/bin/prove_block/src/rpc_utils.rs index 861c6e246..0fe3ad9a2 100644 --- a/crates/bin/prove_block/src/rpc_utils.rs +++ b/crates/bin/prove_block/src/rpc_utils.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use blockifier::transaction::objects::TransactionExecutionInfo; use cairo_vm::Felt252; @@ -12,7 +12,7 @@ use starknet::core::types::BlockWithTxs; use starknet_api::core::{ContractAddress, PatriciaKey}; use starknet_api::state::StorageKey; use starknet_api::{contract_address, felt, patricia_key}; -use starknet_os::config::DEFAULT_STORAGE_TREE_HEIGHT; +use starknet_os::config::{DEFAULT_STORAGE_TREE_HEIGHT, STORED_BLOCK_HASH_BUFFER}; use starknet_os::starkware_utils::commitment_tree::base_types::Height; use starknet_types_core::felt::Felt; @@ -111,6 +111,36 @@ fn verify_storage_proof(contract_data: &ContractData, keys: &[Felt]) -> Vec>, +) { + // A list of the contracts that accessed to the storage from 0x1 using `get_block_hash_syscall` + let special_addresses: Vec = vec![ + contract_address!("0x01246c3031c5d0d1cf60a9370aac03a4717538f659e4a2bfb0f692e970e0c4b5"), + contract_address!("0x00656ca4889a405ec5222e4b0997e5a043902a98cb1f85a039f76f50c000479d"), + contract_address!("0x022207b425a6c0239bbf5d58fbf0272fbb059ee4bb89f48255321d6e7c1606ef"), + // Ekubo:core contract address. Source code is not available but `key_not_in_preimage` error is triggered every time it's called + contract_address!("0x5dd3d2f4429af886cd1a3b08289dbcea99a294197e9eb43b0e0325b4b"), + ]; + + if special_addresses.iter().any(|address| keys.contains_key(address)) { + let extra_storage_reads = 200 * STORED_BLOCK_HASH_BUFFER; + if old_block_number >= Felt252::from(extra_storage_reads) { + for i in 1..=extra_storage_reads { + keys.entry(contract_address!("0x1")) + .or_default() + .insert((old_block_number - i).try_into().expect("Felt to StorageKey conversion failed")); + } + } + } +} + pub(crate) async fn get_storage_proofs( client: &RpcClient, block_number: u64, @@ -121,6 +151,8 @@ pub(crate) async fn get_storage_proofs( let mut keys = get_all_accessed_keys(tx_execution_infos); // We need to fetch the storage proof for the block hash contract keys.entry(contract_address!("0x1")).or_default().insert(old_block_number.try_into().unwrap()); + // Include extra keys for contracts that trigger get_block_hash_syscall + insert_extra_storage_reads_keys(old_block_number, &mut keys); keys }; @@ -132,6 +164,7 @@ pub(crate) async fn get_storage_proofs( let contract_address_felt = *contract_address.key(); let storage_proof = get_storage_proof_for_contract(client, contract_address, storage_keys.into_iter(), block_number).await?; + storage_proofs.insert(contract_address_felt, storage_proof); } @@ -215,6 +248,7 @@ pub(crate) fn get_starknet_version(block_with_txs: &BlockWithTxs) -> blockifier: "0.13.1.1" => blockifier::versioned_constants::StarknetVersion::V0_13_1_1, "0.13.2" => blockifier::versioned_constants::StarknetVersion::V0_13_2, "0.13.2.1" => blockifier::versioned_constants::StarknetVersion::Latest, + "0.13.3" => blockifier::versioned_constants::StarknetVersion::Latest, other => { unimplemented!("Unsupported Starknet version: {}", other) } diff --git a/crates/bin/prove_block/tests/prove_block.rs b/crates/bin/prove_block/tests/prove_block.rs index ed98b62b3..f599b9621 100644 --- a/crates/bin/prove_block/tests/prove_block.rs +++ b/crates/bin/prove_block/tests/prove_block.rs @@ -49,7 +49,6 @@ 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::inconsistent_cairo0_class_hash_2(159674)] #[case::inconsistent_cairo0_class_hash_3(164180)] #[case::key_not_in_proof_0(155087)] @@ -72,6 +71,23 @@ const DEFAULT_COMPILED_OS: &[u8] = include_bytes!("../../../../build/os_latest.j #[case::memory_invalid_signature(216914)] #[case::diff_assert_values(218624)] #[case::could_nt_compute_operand_op1(204337)] +// The following ten tests were added due key not found in preimage (verify_game contract function related) +#[case::key_not_found_in_preimage_0(237025)] +#[case::key_not_found_in_preimage_1(237030)] +#[case::key_not_found_in_preimage_2(237037)] +#[case::key_not_found_in_preimage_3(237042)] +#[case::key_not_found_in_preimage_4(237044)] +#[case::key_not_found_in_preimage_5(237053)] +#[case::key_not_found_in_preimage_6(237083)] +#[case::key_not_found_in_preimage_7(237086)] +#[case::key_not_found_in_preimage_8(235385)] +#[case::key_not_found_in_preimage_9(235620)] +// Reference pies for v0.13.3 +#[case::reference_pie_with_full_output_enabled_00(341097)] +#[case::reference_pie_with_full_output_enabled_01(341101)] +#[case::reference_pie_with_full_output_enabled_02(355710)] +#[case::reference_pie_with_full_output_enabled_03(355783)] +#[case::reference_pie_with_full_output_enabled_04(355802)] #[ignore = "Requires a running Pathfinder node"] #[tokio::test(flavor = "multi_thread")] async fn test_prove_selected_blocks(#[case] block_number: u64) { @@ -98,7 +114,11 @@ async fn test_prove_selected_blocks(#[case] block_number: u64) { fn get_reference_pie_bytes(block_number: u64) -> Option> { match block_number { - 173404 => Some(include_bytes!("../reference-pies/173404.zip").to_vec()), + 341097 => Some(include_bytes!("../reference-pies/341097.zip").to_vec()), + 341101 => Some(include_bytes!("../reference-pies/341101.zip").to_vec()), + 355710 => Some(include_bytes!("../reference-pies/355710.zip").to_vec()), + 355783 => Some(include_bytes!("../reference-pies/355783.zip").to_vec()), + 355802 => Some(include_bytes!("../reference-pies/355802.zip").to_vec()), _ => None, } } diff --git a/crates/rpc-client/src/pathfinder/proofs.rs b/crates/rpc-client/src/pathfinder/proofs.rs index 500156f11..f0315a40e 100644 --- a/crates/rpc-client/src/pathfinder/proofs.rs +++ b/crates/rpc-client/src/pathfinder/proofs.rs @@ -38,7 +38,7 @@ impl TrieNode { } } -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Deserialize, Default)] pub struct ContractData { /// Root of the Contract state tree pub root: Felt, @@ -75,7 +75,7 @@ impl ContractData { #[derive(Debug, Clone, Deserialize)] pub struct PathfinderProof { - pub state_commitment: Felt, + pub state_commitment: Option, pub class_commitment: Option, pub contract_proof: Vec, pub contract_data: Option, diff --git a/crates/starknet-os/src/config.rs b/crates/starknet-os/src/config.rs index 0511ecdba..e967038ad 100644 --- a/crates/starknet-os/src/config.rs +++ b/crates/starknet-os/src/config.rs @@ -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; + 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); } diff --git a/crates/starknet-os/src/execution/syscall_handler.rs b/crates/starknet-os/src/execution/syscall_handler.rs index 3965804d2..458675a83 100644 --- a/crates/starknet-os/src/execution/syscall_handler.rs +++ b/crates/starknet-os/src/execution/syscall_handler.rs @@ -247,7 +247,6 @@ pub struct DeployHandler; pub struct DeployResponse { pub contract_address: Felt252, pub constructor_retdata: ReadOnlySegment, - pub need_retdata_hack: bool, } impl SyscallHandler for DeployHandler @@ -292,59 +291,13 @@ where "No more deployed contracts available to replay".to_string().into_boxed_str(), ))?; - let need_retdata_hack = if let Some(os_input) = execution_helper.os_input.as_ref() { - let class_hash = os_input - .contract_address_to_class_hash - .get(&contract_address) - .expect("No class_hash for contract_address"); - let num_constructors = - if let Some(compiled_class_hash) = os_input.class_hash_to_compiled_class_hash.get(class_hash) { - let casm = os_input.compiled_classes.get(compiled_class_hash).expect("No CASM"); - let num_constructors = casm - .get_cairo_lang_contract_class() - .expect("couldn't get cairo lang class") - .entry_points_by_type - .constructor - .len(); - num_constructors - } else { - let deprecated_cc = os_input.deprecated_compiled_classes.get(class_hash).expect("no deprecated CC"); - let num_constructors = deprecated_cc - .get_starknet_api_contract_class() - .expect("couldn't get starknet api class") - .entry_points_by_type - .get(&starknet_api::deprecated_contract_class::EntryPointType::Constructor) - .expect("should have constructor list") - .len(); - num_constructors - }; - - // we need the hack if there are no constructor entry points - num_constructors == 0 - } else { - false - }; - - Ok(DeployResponse { contract_address, constructor_retdata, need_retdata_hack }) + Ok(DeployResponse { contract_address, constructor_retdata }) } fn write_response(response: Self::Response, vm: &mut VirtualMachine, ptr: &mut Relocatable) -> WriteResponseResult { write_felt(vm, ptr, response.contract_address)?; + write_segment(vm, ptr, response.constructor_retdata)?; - // Bugfix: - // When retdata size is 0, the OS will return retdata as a Int(0) instead of a relocatable - // as a result of `cast(0, felt*)`. To work around this, we can write the same here so that - // later the OS will assert this value is the same as retdata; that is, both need to be the - // same value which is Int(0). - // - // retdata is cast here: https://github.com/starkware-libs/cairo-lang/blob/a86e92bfde9c171c0856d7b46580c66e004922f3/src/starkware/starknet/core/os/execution/execute_entry_point.cairo#L170 - if response.need_retdata_hack { - log::warn!("Writing Felt::ZERO instead of pointer since retdata size is 0"); - write_felt(vm, ptr, Felt252::ZERO)?; // casted pointer - write_felt(vm, ptr, Felt252::ZERO)?; // size - } else { - write_segment(vm, ptr, response.constructor_retdata)?; - } Ok(()) } } diff --git a/crates/starknet-os/src/hints/execution.rs b/crates/starknet-os/src/hints/execution.rs index c04d0ec50..364ffbdc6 100644 --- a/crates/starknet-os/src/hints/execution.rs +++ b/crates/starknet-os/src/hints/execution.rs @@ -1122,23 +1122,10 @@ pub fn check_new_deploy_response( ) -> Result<(), HintError> { let response_ptr = get_ptr_from_var_name(vars::ids::RESPONSE, vm, ids_data, ap_tracking)?; - // Bugfix: - // If constructor_retdata_start is a Int(0) instead of a Relocatable, we need to do nothing. This - // can happen sometimes when a constructor is called but not defined in the class'es entry - // points. Immediately following this, `add_relocation_rule()` will be called with src=0, dest=0 - // and it will also no-op. - // - // If "retdata" is an Int instead of a Relocatable, then the vm will error when we try to - // request it as such. In this case, there is no retdata anyway, so there is no need to assert - // that the contents are equal. - let retdata_start_key: Relocatable = - (response_ptr + new_syscalls::DeployResponse::constructor_retdata_start_offset())?; - let maybe_retdata_start = vm - .get_maybe(&retdata_start_key) - .ok_or(HintError::VariableNotInScopeError("retdata".to_string().into_boxed_str()))?; - let zero = MaybeRelocatable::Int(Felt252::ZERO); - if maybe_retdata_start == zero { - log::warn!("retdata_start is 0, not a relocatable, ignoring add_relocation_rule()"); + let retdata_size = + felt_to_usize(get_integer_from_var_name(vars::ids::RETDATA_SIZE, vm, ids_data, ap_tracking)?.as_ref())?; + if retdata_size == 0 { + log::warn!("Ignoring empty retdata"); return Ok(()); } @@ -1149,8 +1136,6 @@ pub fn check_new_deploy_response( let response_retdata_size = (constructor_retdata_end - constructor_retdata_start)?; let retdata = get_ptr_from_var_name(vars::ids::RETDATA, vm, ids_data, ap_tracking)?; - let retdata_size = - felt_to_usize(get_integer_from_var_name(vars::ids::RETDATA_SIZE, vm, ids_data, ap_tracking)?.as_ref())?; assert_memory_ranges_equal(vm, constructor_retdata_start, response_retdata_size, retdata, retdata_size)?; @@ -1228,8 +1213,8 @@ pub fn check_response_return_value( ap_tracking: &ApTracking, _constants: &HashMap, ) -> Result<(), HintError> { - let retdata = get_ptr_from_var_name(vars::ids::RETDATA, vm, ids_data, ap_tracking)?; let retdata_size = get_integer_from_var_name(vars::ids::RETDATA_SIZE, vm, ids_data, ap_tracking)?; + let retdata = get_ptr_from_var_name(vars::ids::RETDATA, vm, ids_data, ap_tracking)?; let response = get_ptr_from_var_name(vars::ids::RESPONSE, vm, ids_data, ap_tracking)?; let response_retdata_start = @@ -1289,19 +1274,6 @@ pub fn add_relocation_rule( ap_tracking: &ApTracking, _constants: &HashMap, ) -> Result<(), HintError> { - // Bugfix: - // add_relocation_rule() can be called sometimes with Int(0) instead of Relocatables. This is - // a result of `cast(0, felt*)` being treated as Int(0). We can work around this here by - // ensuring that both src and dest end up being Int(0), and in that case we no-op here. - let src_ptr_maybe = get_maybe_relocatable_from_var_name(vars::ids::SRC_PTR, vm, ids_data, ap_tracking)?; - let dest_ptr_maybe = get_maybe_relocatable_from_var_name(vars::ids::DEST_PTR, vm, ids_data, ap_tracking)?; - - let zero = MaybeRelocatable::Int(Felt252::ZERO); - if src_ptr_maybe == zero && dest_ptr_maybe == zero { - log::warn!("add_relocation_rule with Int(0) => Int(0), doing nothing"); - return Ok(()); - } - let src_ptr = get_ptr_from_var_name(vars::ids::SRC_PTR, vm, ids_data, ap_tracking)?; assert_eq!(src_ptr.offset, 0, "src_ptr offset should be 0"); @@ -1317,12 +1289,12 @@ pub fn add_relocation_rule( let dest_ptr = if let Ok(infos) = get_ptr_from_var_name(vars::ids::INFOS, vm, ids_data, ap_tracking) { let infos_0_end = vm.get_relocatable((infos + 1)?)?; log::warn!("rewriting dest_ptr for segment_arena workaround"); - (infos_0_end + 1u32)? + (infos_0_end + 1u32)?.into() } else { - get_ptr_from_var_name(vars::ids::DEST_PTR, vm, ids_data, ap_tracking)? + get_maybe_relocatable_from_var_name(vars::ids::DEST_PTR, vm, ids_data, ap_tracking)? }; - vm.add_relocation_rule(src_ptr, dest_ptr)?; + vm.add_relocation_rule_maybe_relocatable(src_ptr, dest_ptr)?; Ok(()) } diff --git a/crates/starknet-os/src/hints/mod.rs b/crates/starknet-os/src/hints/mod.rs index 282f0fa0b..3c5cc982d 100644 --- a/crates/starknet-os/src/hints/mod.rs +++ b/crates/starknet-os/src/hints/mod.rs @@ -176,7 +176,9 @@ fn hints() -> HashMap where hints.insert(os::SET_AP_TO_PREV_BLOCK_HASH.into(), os::set_ap_to_prev_block_hash); hints.insert(kzg::STORE_DA_SEGMENT.into(), kzg::store_da_segment::); hints.insert(output::SET_STATE_UPDATES_START.into(), output::set_state_updates_start); + hints.insert(output::SET_COMPRESSED_START.into(), output::set_compressed_start); hints.insert(output::SET_TREE_STRUCTURE.into(), output::set_tree_structure); + hints.insert(output::SET_N_UPDATES_SMALL.into(), output::set_n_updates_small); hints.insert(patricia::ASSERT_CASE_IS_RIGHT.into(), patricia::assert_case_is_right); hints.insert(patricia::BUILD_DESCENT_MAP.into(), patricia::build_descent_map); hints.insert(patricia::HEIGHT_IS_ZERO_OR_LEN_NODE_PREIMAGE_IS_TWO.into(), patricia::height_is_zero_or_len_node_preimage_is_two); diff --git a/crates/starknet-os/src/hints/output.rs b/crates/starknet-os/src/hints/output.rs index 916811d78..ac3634ca2 100644 --- a/crates/starknet-os/src/hints/output.rs +++ b/crates/starknet-os/src/hints/output.rs @@ -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, ap_tracking: &ApTracking, _constants: &HashMap, ) -> 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, + ap_tracking: &ApTracking, + _constants: &HashMap, +) -> Result<(), HintError> { + let use_kzg_da_felt = exec_scopes.get::(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())) + }?; + + 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, + ap_tracking: &ApTracking, + constants: &HashMap, +) -> 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); + } + } } diff --git a/crates/starknet-os/src/hints/vars.rs b/crates/starknet-os/src/hints/vars.rs index 98aee63f0..540f5fb4c 100644 --- a/crates/starknet-os/src/hints/vars.rs +++ b/crates/starknet-os/src/hints/vars.rs @@ -33,6 +33,7 @@ pub mod scopes { pub const TRANSACTIONS: &str = "transactions"; pub const TX: &str = "tx"; pub const VALUE: &str = "value"; + pub const USE_KZG_DA: &str = "use_kzg_da"; } pub mod ids { @@ -142,6 +143,7 @@ pub mod ids { pub const STATE_ENTRY: &str = "state_entry"; pub const STATE_UPDATES_START: &str = "state_updates_start"; pub const STATE_UPDATES_END: &str = "state_updates_end"; + pub const COMPRESSED_START: &str = "compressed_start"; pub const SYSCALL_PTR: &str = "syscall_ptr"; pub const TRANSACTION_HASH: &str = "transaction_hash"; pub const TX_EXECUTION_CONTEXT: &str = "tx_execution_context"; @@ -157,6 +159,12 @@ pub mod ids { pub const BATCH_SIZE: &str = "starkware.cairo.common.cairo_sha256.sha256_utils.BATCH_SIZE"; pub const SHA256_INPUT_CHUNK_SIZE_FELTS: &str = "starkware.cairo.common.cairo_sha256.sha256_utils.SHA256_INPUT_CHUNK_SIZE_FELTS"; + pub const COMPRESS_STATE_UPDATES: &str = "compress_state_updates"; + pub const IS_N_UPDATES_SMALL: &str = "is_n_updates_small"; + pub const N_ACTUAL_UPDATES: &str = "n_actual_updates"; + pub const N_UPDATES_SMALL_PACKING_BOUND: &str = + "starkware.starknet.core.os.state.output.N_UPDATES_SMALL_PACKING_BOUND"; + pub const FULL_OUTPUT: &str = "full_output"; pub const PREV_OFFSET: &str = "prev_offset"; pub const BUCKET_INDEX: &str = "bucket_index"; pub const DICT_PTR: &str = "dict_ptr"; diff --git a/crates/starknet-os/src/io/output.rs b/crates/starknet-os/src/io/output.rs index 5d32a62b3..a68e7f663 100644 --- a/crates/starknet-os/src/io/output.rs +++ b/crates/starknet-os/src/io/output.rs @@ -35,6 +35,14 @@ pub struct ContractChanges { pub storage_changes: HashMap, } +#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Default)] +pub struct OsStateDiff { + /// The list of contracts that were changed. + pub contract_changes: Vec, + /// The list of classes that were declared. A map from class hash to compiled class hash. + pub classes: HashMap, +} + #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] pub struct StarknetOsOutput { /// The root before. @@ -61,10 +69,8 @@ pub struct StarknetOsOutput { pub messages_to_l1: Vec, /// Messages from L1 to L2. pub messages_to_l2: Vec, - /// The list of contracts that were changed. - pub contracts: Vec, - /// The list of classes that were declared. A map from class hash to compiled class hash. - pub classes: HashMap, + /// The state diff. + pub state_diff: Option, } impl StarknetOsOutput { @@ -141,12 +147,31 @@ fn deserialize_contract_state_inner>( ) -> Result { let bound = Felt252::from(1u128 << 64).try_into().expect("2**64 should be considered non-zero. Did you change the value?"); + let n_updates_small_packing_bound = + Felt252::from(1u128 << 8).try_into().expect("2**8 should be considered non-zero. Did you change the value?"); + let flag_bound = + Felt252::from(1u128 << 1).try_into().expect("2**1 should be considered non-zero. Did you change the value?"); let addr = next_or_fail(output_iter, "contract change addr")?; + let nonce_n_changes_two_flags = next_or_fail(output_iter, "contract nonce_n_changes_two_flags")?; + + // Parse flags + let (nonce_n_changes_one_flag, was_class_updated) = nonce_n_changes_two_flags.div_rem(&flag_bound); + let (nonce_n_changes, is_n_updates_small) = nonce_n_changes_one_flag.div_rem(&flag_bound); - let value = next_or_fail(output_iter, "contract change value")?; - let (value, n_actual_updates) = value.div_rem(&bound); - let (was_class_updated, new_state_nonce) = value.div_rem(&bound); + // Parse n_changes + let n_updates_bound = if is_n_updates_small == Felt252::ONE { n_updates_small_packing_bound } else { bound }; + let (nonce, n_changes) = nonce_n_changes.div_rem(&n_updates_bound); + + // Parse nonces + let new_state_nonce = if !full_output.is_zero() { + // | old_nonce | new_nonce | + let (_old_nonce, new_nonce) = nonce.div_rem(&bound); + new_nonce + } else { + // | new_nonce | or Zero + nonce + }; #[allow(clippy::collapsible_else_if)] // Mirror the Cairo code as much as possible let new_state_class_hash = if !full_output.is_zero() { @@ -160,10 +185,9 @@ fn deserialize_contract_state_inner>( } }; - let n_actual_updates = n_actual_updates - .to_usize() - .expect("n_updates should be 64-bit by definition. Did you modify the parsing above?"); - let storage_changes = deserialize_da_changes(output_iter, n_actual_updates, full_output)?; + let n_changes = + n_changes.to_usize().expect("n_updates should be 8 or 64-bit by definition. Did you modify the parsing above?"); + let storage_changes = deserialize_da_changes(output_iter, n_changes, full_output)?; Ok(ContractChanges { addr, nonce: new_state_nonce, class_hash: new_state_class_hash, storage_changes }) } @@ -261,6 +285,25 @@ fn read_segment>( Ok(segment) } +fn deserialize_os_state_diff>( + output_iter: &mut I, + full_output: Felt252, +) -> Result, SnOsError> { + // If not full_output + if full_output == Felt252::ZERO { + // state_diff = decompress(compressed=output_iter) + // output_iter = itertools.chain(iter(state_diff), output_iter) + return Ok(None); + } + + // Contract changes + let contract_changes = deserialize_contract_state(output_iter, full_output)?; + // Class changes + let classes = deserialize_contract_class_da_changes(output_iter, full_output)?; + + Ok(Some(OsStateDiff { contract_changes, classes })) +} + // Reverse of serialize_os_output in os/output.cairo pub fn deserialize_os_output(output_iter: &mut I) -> Result where @@ -285,14 +328,7 @@ where let (messages_to_l1, messages_to_l2) = deserialize_messages(output_iter)?; - let (contract_changes, classes) = if use_kzg_da.is_zero() { - ( - deserialize_contract_state(output_iter, full_output)?, - deserialize_contract_class_da_changes(output_iter, full_output)?, - ) - } else { - (vec![], HashMap::default()) - }; + let state_diff = deserialize_os_state_diff(output_iter, full_output)?; Ok(StarknetOsOutput { initial_root: header[PREVIOUS_MERKLE_UPDATE_OFFSET], @@ -307,8 +343,7 @@ where full_output, messages_to_l1, messages_to_l2, - contracts: contract_changes, - classes, + state_diff, }) } @@ -344,26 +379,28 @@ mod tests { Felt252::from(27), ], messages_to_l2: vec![], - contracts: vec![ContractChanges { - addr: Felt252::ONE, - nonce: Felt252::from(100), - class_hash: None, - storage_changes: HashMap::from([ - ( - Felt252::from_hex_unchecked( - "0x723973208639b7839ce298f7ffea61e3f9533872defd7abdb91023db4658812", + state_diff: Some(OsStateDiff { + contract_changes: vec![ContractChanges { + addr: Felt252::ONE, + nonce: Felt252::from(100), + class_hash: None, + storage_changes: HashMap::from([ + ( + Felt252::from_hex_unchecked( + "0x723973208639b7839ce298f7ffea61e3f9533872defd7abdb91023db4658812", + ), + Felt252::from_hex_unchecked("0x1f67eee3d0800"), ), - Felt252::from_hex_unchecked("0x1f67eee3d0800"), - ), - ( - Felt252::from_hex_unchecked( - "0x27e66af6f5df3e043d32367d68ece7e13645cca1ca9f80dfdaff9013fddf0c5", + ( + Felt252::from_hex_unchecked( + "0x27e66af6f5df3e043d32367d68ece7e13645cca1ca9f80dfdaff9013fddf0c5", + ), + Felt252::from_hex_unchecked("0xddec034b926f800"), ), - Felt252::from_hex_unchecked("0xddec034b926f800"), - ), - ]), - }], - classes: Default::default(), + ]), + }], + classes: Default::default(), + }), }; let os_output_str = serde_json::to_string(&os_output).expect("OS output serialization failed"); diff --git a/crates/starknet-os/src/starkware_utils/commitment_tree/binary_fact_tree_node.rs b/crates/starknet-os/src/starkware_utils/commitment_tree/binary_fact_tree_node.rs index fd6a64d53..984eba04f 100644 --- a/crates/starknet-os/src/starkware_utils/commitment_tree/binary_fact_tree_node.rs +++ b/crates/starknet-os/src/starkware_utils/commitment_tree/binary_fact_tree_node.rs @@ -174,20 +174,7 @@ where ffc: &'a mut FactFetchingContext, indices: &'a [TreeIndex], facts: &'a mut Option, - ) -> impl std::future::Future, TreeError>> + Send { - async move { - if indices.is_empty() { - return Ok(HashMap::default()); - } - - if self.is_leaf() { - return self._get_leaf(ffc, indices).await; - } - - self._get_binary_node_leaves(ffc, indices, facts).await - } - .boxed() - } + ) -> impl std::future::Future, TreeError>> + Send; /// Returns the values of the leaves whose indices are given. fn _get_binary_node_leaves<'a>( diff --git a/crates/starknet-os/src/starkware_utils/commitment_tree/calculation.rs b/crates/starknet-os/src/starkware_utils/commitment_tree/calculation.rs index a967e45e6..113a8bfb2 100644 --- a/crates/starknet-os/src/starkware_utils/commitment_tree/calculation.rs +++ b/crates/starknet-os/src/starkware_utils/commitment_tree/calculation.rs @@ -50,15 +50,6 @@ pub trait Calculation { /// (using their hash as the key). fn calculate(&self, dependency_results: Vec>, fact_nodes: &mut NodeFactDict) -> T; - /// Same as calculate(), but return the facts. - - fn calculate_new_fact_nodes(&self, dependency_results: Vec>) -> (T, NodeFactDict) { - let mut fact_nodes = NodeFactDict::default(); - let result = self.calculate(dependency_results, &mut fact_nodes); - - (result, fact_nodes) - } - /// Produces the result of this calculation. /// /// Recursively calculates the result of the dependency calculations. @@ -74,17 +65,6 @@ pub trait Calculation { self.calculate(dependency_results, fact_nodes) } - - /// Produces the result of this calculation. Returns the result and a dict containing generated - /// facts. - /// - /// Recursively calculates the result of the dependency calculations. - fn full_calculate_new_fact_nodes(&self) -> (T, NodeFactDict) { - let mut fact_nodes = NodeFactDict::default(); - let result = self.full_calculate(&mut fact_nodes); - - (result, fact_nodes) - } } pub(crate) struct DependencyWrapper diff --git a/crates/starknet-os/src/starkware_utils/commitment_tree/patricia_tree/nodes.rs b/crates/starknet-os/src/starkware_utils/commitment_tree/patricia_tree/nodes.rs index 9addd00cc..ce6114bdc 100644 --- a/crates/starknet-os/src/starkware_utils/commitment_tree/patricia_tree/nodes.rs +++ b/crates/starknet-os/src/starkware_utils/commitment_tree/patricia_tree/nodes.rs @@ -11,51 +11,6 @@ use crate::storage::storage::{DbObject, Fact, HashFunctionType, Storage, HASH_BY const PATRICIA_NODE_PREFIX: &[u8] = "patricia_node".as_bytes(); -/// Represents the root of an empty (all leaves are 0) full binary tree. -pub struct EmptyNodeFact; - -impl EmptyNodeFact { - const PREIMAGE_LENGTH: usize = 0; -} - -impl InnerNodeFact for EmptyNodeFact -where - H: HashFunctionType, - S: Storage, -{ - fn to_tuple(&self) -> Vec { - vec![] - } -} - -impl Fact for EmptyNodeFact -where - H: HashFunctionType, - S: Storage, -{ - fn hash(&self) -> Hash { - Hash::empty() - } -} - -impl DbObject for EmptyNodeFact {} - -impl SerializationPrefix for EmptyNodeFact { - fn prefix() -> Vec { - PATRICIA_NODE_PREFIX.to_vec() - } -} - -impl Serializable for EmptyNodeFact { - fn serialize(&self) -> Result, SerializeError> { - Ok("".as_bytes().to_vec()) - } - - fn deserialize(_data: &[u8]) -> Result { - Ok(Self {}) - } -} - #[derive(thiserror::Error, Debug)] pub enum BinaryNodeError { #[allow(unused)] @@ -240,7 +195,6 @@ fn hash_edge(bottom: &[u8], path: NodePath, length: Length) } pub enum PatriciaNodeFact { - Empty(EmptyNodeFact), Binary(BinaryNodeFact), Edge(EdgeNodeFact), } @@ -254,7 +208,6 @@ impl SerializationPrefix for PatriciaNodeFact { impl Serializable for PatriciaNodeFact { fn serialize(&self) -> Result, SerializeError> { match self { - Self::Empty(empty) => empty.serialize(), Self::Binary(binary) => binary.serialize(), Self::Edge(edge) => edge.serialize(), } @@ -262,7 +215,6 @@ impl Serializable for PatriciaNodeFact { fn deserialize(data: &[u8]) -> Result { let node = match data.len() { - EmptyNodeFact::PREIMAGE_LENGTH => Self::Empty(EmptyNodeFact::deserialize(data)?), BinaryNodeFact::PREIMAGE_LENGTH => Self::Binary(BinaryNodeFact::deserialize(data)?), EdgeNodeFact::PREIMAGE_LENGTH => Self::Edge(EdgeNodeFact::deserialize(data)?), other => { @@ -280,7 +232,6 @@ where { fn hash(&self) -> Hash { match self { - Self::Empty(empty) => >::hash(empty), Self::Binary(binary) => >::hash(binary), Self::Edge(edge) => >::hash(edge), } @@ -296,7 +247,6 @@ where { fn to_tuple(&self) -> Vec { match self { - Self::Empty(empty) => >::to_tuple(empty), Self::Binary(binary) => >::to_tuple(binary), Self::Edge(edge) => >::to_tuple(edge), } diff --git a/crates/starknet-os/src/starkware_utils/commitment_tree/patricia_tree/virtual_patricia_node.rs b/crates/starknet-os/src/starkware_utils/commitment_tree/patricia_tree/virtual_patricia_node.rs index 990659bc7..cc66052d7 100644 --- a/crates/starknet-os/src/starkware_utils/commitment_tree/patricia_tree/virtual_patricia_node.rs +++ b/crates/starknet-os/src/starkware_utils/commitment_tree/patricia_tree/virtual_patricia_node.rs @@ -248,7 +248,6 @@ where Self::from_hash(binary_node_fact.left_node, children_height), Self::from_hash(binary_node_fact.right_node, children_height), )), - PatriciaNodeFact::Empty(_) => Err(TreeError::IsEmpty), } } .boxed() diff --git a/crates/starknet-os/src/starkware_utils/serializable.rs b/crates/starknet-os/src/starkware_utils/serializable.rs index 924a74f74..281d6bc0c 100644 --- a/crates/starknet-os/src/starkware_utils/serializable.rs +++ b/crates/starknet-os/src/starkware_utils/serializable.rs @@ -69,16 +69,6 @@ mod tests { impl SerializationPrefix for MySerializable {} - impl Serializable for MySerializable { - fn serialize(&self) -> Result, SerializeError> { - panic!("Not implemented, on purpose"); - } - - fn deserialize(_data: &[u8]) -> Result { - panic!("Not implemented, on purpose"); - } - } - #[test] fn test_class_name_prefix() { assert_eq!(MySerializable::class_name_prefix(), "my_serializable".as_bytes()); diff --git a/requirements.txt b/requirements.txt index 30f5e344d..54da5cefc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ sympy==1.11.1 -cairo-lang==0.13.2 +cairo-lang==0.13.3 ecdsa==0.18.0 bitarray==2.7.3 fastecdsa==2.3.0 \ No newline at end of file diff --git a/scripts/check-orphans.sh b/scripts/check-orphans.sh deleted file mode 100755 index 61e0783fa..000000000 --- a/scripts/check-orphans.sh +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/bash -cargo run --quiet --example hints orphans | jq -e 'if length == 0 then . else empty end' -if [ $? -ne '0' ]; then - echo -e "orphaned hints found:" - cargo run --quiet --example hints orphans | jq . - exit 1 -fi \ No newline at end of file diff --git a/setup-scripts/setup-tests.sh b/setup-scripts/setup-tests.sh index 86a728145..470adc8d2 100755 --- a/setup-scripts/setup-tests.sh +++ b/setup-scripts/setup-tests.sh @@ -1,6 +1,6 @@ #!/bin/bash -CAIRO_VER="0.13.2" +CAIRO_VER="0.13.3" if ! command -v cairo-compile >/dev/null; then echo "please start cairo($CAIRO_VER) dev environment" diff --git a/tests/integration/common/block_utils.rs b/tests/integration/common/block_utils.rs index cf838a532..b4cc0b688 100644 --- a/tests/integration/common/block_utils.rs +++ b/tests/integration/common/block_utils.rs @@ -213,7 +213,7 @@ where declared_class_hash_to_component_hashes, new_block_hash: Default::default(), prev_block_hash: Default::default(), - full_output: false, + full_output: !block_context.block_info().use_kzg_da, }); let execution_helper = ExecutionHelperWrapper::new( diff --git a/tests/integration/common/utils.rs b/tests/integration/common/utils.rs index 0cff9a521..8085859de 100644 --- a/tests/integration/common/utils.rs +++ b/tests/integration/common/utils.rs @@ -59,10 +59,12 @@ pub fn check_os_output_read_only_syscall(os_output: StarknetOsOutput, block_cont // TODO: finer-grained contract changes checks // Just check that the two contracts have been modified, these should be storage changes // related to the fees. - assert_eq!(os_output.contracts.len(), 2); + assert!(os_output.state_diff.is_some()); + let state_diff = os_output.state_diff.unwrap(); + assert_eq!(state_diff.contract_changes.len(), 4); assert_eq!(os_output.new_block_number.to_u64().unwrap(), block_context.block_info().block_number.0); - assert!(os_output.classes.is_empty()); + assert!(state_diff.classes.is_empty()); assert!(os_output.messages_to_l1.is_empty()); assert!(os_output.messages_to_l2.is_empty()); let use_kzg_da = os_output.use_kzg_da != Felt252::ZERO; diff --git a/tests/integration/deprecated_syscalls_tests.rs b/tests/integration/deprecated_syscalls_tests.rs index ccf370bd4..9940ce2bd 100644 --- a/tests/integration/deprecated_syscalls_tests.rs +++ b/tests/integration/deprecated_syscalls_tests.rs @@ -226,9 +226,12 @@ async fn test_syscall_get_tx_info_cairo0( .await .expect("OS run failed"); + assert!(os_output.state_diff.is_some()); + let state_diff = os_output.state_diff.unwrap(); + // This test causes storage changes in the test contract. Check them. let contract_changes_by_address: HashMap<_, _> = - os_output.contracts.iter().map(|change| (change.addr, change)).collect(); + state_diff.contract_changes.iter().map(|change| (change.addr, change)).collect(); let test_contract_changes = contract_changes_by_address .get(contract_address.0.key()) .expect("The test contract should appear as modified in the OS output"); @@ -391,14 +394,17 @@ async fn test_syscall_deploy_cairo0( .await .expect("OS run failed"); + assert!(os_output.state_diff.is_some()); + let state_diff = os_output.state_diff.unwrap(); + // Check that the new contract address appears in the OS output let contract_changes_by_address: HashMap<_, _> = - os_output.contracts.iter().map(|change| (change.addr, change)).collect(); + state_diff.contract_changes.iter().map(|change| (change.addr, change)).collect(); assert!(contract_changes_by_address.contains_key(expected_contract_address.key())); // Check other output fields assert_eq!(os_output.new_block_number.to_u64().unwrap(), block_context.block_info().block_number.0); - assert!(os_output.classes.is_empty()); + assert!(state_diff.classes.is_empty()); assert!(os_output.messages_to_l1.is_empty()); assert!(os_output.messages_to_l2.is_empty()); let use_kzg_da = os_output.use_kzg_da != Felt252::ZERO;