From 0835ad95c9a1d0886fdf4bbb57369ed8560d73a1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 13:30:05 +0000 Subject: [PATCH 1/2] blockifier: avoid redundant memory reads in read_felt_array read_felt_array's empty-span check already reads both the start and end pointer cells via get_maybe; the non-empty path then re-read the same two memory cells via get_relocatable. Reuse the values already fetched instead of doing the memory lookup twice, since this function runs on every array-typed syscall argument (calldata, event keys/data, etc.) across a block. --- .../src/execution/syscalls/hint_processor.rs | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/blockifier/src/execution/syscalls/hint_processor.rs b/crates/blockifier/src/execution/syscalls/hint_processor.rs index 9da8f4fa687..6aef957c514 100644 --- a/crates/blockifier/src/execution/syscalls/hint_processor.rs +++ b/crates/blockifier/src/execution/syscalls/hint_processor.rs @@ -864,21 +864,37 @@ where // If the start and end pointers are the same, the array is empty. // This check is necessary to handle the case where both pointers are zero, and thus are not // relocatable values. - let array_start = vm.get_maybe(&*ptr); - if array_start.is_some() && array_start == vm.get_maybe(&(*ptr + 1_usize)?) { + let array_data_start = vm.get_maybe(&*ptr); + let array_data_end = vm.get_maybe(&(*ptr + 1_usize)?); + if array_data_start.is_some() && array_data_start == array_data_end { *ptr = (*ptr + 2)?; return Ok(vec![]); } - let array_data_start_ptr = vm.get_relocatable(*ptr)?; + // Reuse the values already read above instead of re-reading the same memory cells via + // `vm.get_relocatable`. + let array_data_start_ptr = expect_relocatable(array_data_start, *ptr)?; *ptr = (*ptr + 1)?; - let array_data_end_ptr = vm.get_relocatable(*ptr)?; + let array_data_end_ptr = expect_relocatable(array_data_end, *ptr)?; *ptr = (*ptr + 1)?; let array_size = (array_data_end_ptr - array_data_start_ptr)?; Ok(felt_range_from_ptr(vm, array_data_start_ptr, array_size)?) } +/// Mirrors `VirtualMachine::get_relocatable`'s error semantics for a value already read via +/// `VirtualMachine::get_maybe`. +fn expect_relocatable( + value: Option, + address: Relocatable, +) -> Result { + match value { + Some(MaybeRelocatable::RelocatableValue(relocatable)) => Ok(relocatable), + Some(MaybeRelocatable::Int(_)) => Err(MemoryError::ExpectedRelocatable(Box::new(address))), + None => Err(MemoryError::UnknownMemoryCell(Box::new(address))), + } +} + pub fn write_segment( vm: &mut VirtualMachine, ptr: &mut Relocatable, From ecdf1be7f0c02fecc62c49dddd74f9e34ad96c81 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 13:43:41 +0000 Subject: [PATCH 2/2] blockifier: polish read_felt_array refactor per review Rename the helper to relocatable_from_memory_value (expect_* implies panicking in Rust), move it below its public caller, and rephrase the comment to state why the second read is skipped. Add tests pinning the exact error variant and address for the missing-cell and wrong-type cases, since preserving get_relocatable's error semantics is the main correctness risk of this refactor. --- .../src/execution/syscalls/hint_processor.rs | 34 +++++++------- .../syscalls/syscall_tests/null_empty_span.rs | 47 +++++++++++++++++-- 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/crates/blockifier/src/execution/syscalls/hint_processor.rs b/crates/blockifier/src/execution/syscalls/hint_processor.rs index 6aef957c514..2d2066e90a8 100644 --- a/crates/blockifier/src/execution/syscalls/hint_processor.rs +++ b/crates/blockifier/src/execution/syscalls/hint_processor.rs @@ -871,30 +871,17 @@ where return Ok(vec![]); } - // Reuse the values already read above instead of re-reading the same memory cells via - // `vm.get_relocatable`. - let array_data_start_ptr = expect_relocatable(array_data_start, *ptr)?; + // This function runs once per array-typed syscall argument, so the endpoints are converted from + // the values read above rather than looked up in memory a second time. + let array_data_start_ptr = relocatable_from_memory_value(array_data_start, *ptr)?; *ptr = (*ptr + 1)?; - let array_data_end_ptr = expect_relocatable(array_data_end, *ptr)?; + let array_data_end_ptr = relocatable_from_memory_value(array_data_end, *ptr)?; *ptr = (*ptr + 1)?; let array_size = (array_data_end_ptr - array_data_start_ptr)?; Ok(felt_range_from_ptr(vm, array_data_start_ptr, array_size)?) } -/// Mirrors `VirtualMachine::get_relocatable`'s error semantics for a value already read via -/// `VirtualMachine::get_maybe`. -fn expect_relocatable( - value: Option, - address: Relocatable, -) -> Result { - match value { - Some(MaybeRelocatable::RelocatableValue(relocatable)) => Ok(relocatable), - Some(MaybeRelocatable::Int(_)) => Err(MemoryError::ExpectedRelocatable(Box::new(address))), - None => Err(MemoryError::UnknownMemoryCell(Box::new(address))), - } -} - pub fn write_segment( vm: &mut VirtualMachine, ptr: &mut Relocatable, @@ -906,3 +893,16 @@ pub fn write_segment( Ok(()) } + +/// Converts a value read from `address` via `VirtualMachine::get_maybe` into a `Relocatable`, +/// reproducing the errors `VirtualMachine::get_relocatable` would have returned for that address. +fn relocatable_from_memory_value( + memory_value: Option, + address: Relocatable, +) -> Result { + match memory_value { + Some(MaybeRelocatable::RelocatableValue(relocatable)) => Ok(relocatable), + Some(MaybeRelocatable::Int(_)) => Err(MemoryError::ExpectedRelocatable(Box::new(address))), + None => Err(MemoryError::UnknownMemoryCell(Box::new(address))), + } +} diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/null_empty_span.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/null_empty_span.rs index f728933efd1..161c9b2ca83 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/null_empty_span.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/null_empty_span.rs @@ -1,4 +1,6 @@ +use assert_matches::assert_matches; use cairo_vm::types::relocatable::{MaybeRelocatable, Relocatable}; +use cairo_vm::vm::errors::memory_errors::MemoryError; use cairo_vm::vm::vm_core::VirtualMachine; use starknet_types_core::felt::Felt; @@ -69,11 +71,48 @@ fn read_felt_array_accepts_real_empty_span() { fn read_felt_array_rejects_mixed_null_and_pointer_span() { let mut vm = VirtualMachine::new(false, false); let data_ptr = vm.add_memory_segment(); - let span_ptr = vm.add_memory_segment(); - vm.load_data(span_ptr, &[Felt::ZERO.into(), data_ptr.into()]).unwrap(); - let mut span_ptr = span_ptr; + let (vm, mut span_ptr) = vm_with_span_in(vm, Felt::ZERO.into(), data_ptr.into()); + let expected_address = span_ptr; + + let error = read_felt_array::(&vm, &mut span_ptr).unwrap_err(); + + assert_matches!( + error, + SyscallExecutorBaseError::Memory(MemoryError::ExpectedRelocatable(address)) + if *address == expected_address + ); +} + +#[test] +fn read_felt_array_rejects_span_with_felt_end_pointer() { + // The reported address must be the end pointer's cell, not the span's base. + let mut vm = VirtualMachine::new(false, false); + let data_ptr = vm.add_memory_segment(); + let (vm, mut span_ptr) = vm_with_span_in(vm, data_ptr.into(), Felt::ZERO.into()); + let expected_address = (span_ptr + 1_usize).unwrap(); + + let error = read_felt_array::(&vm, &mut span_ptr).unwrap_err(); + + assert_matches!( + error, + SyscallExecutorBaseError::Memory(MemoryError::ExpectedRelocatable(address)) + if *address == expected_address + ); +} + +#[test] +fn read_felt_array_rejects_unwritten_span() { + let mut vm = VirtualMachine::new(false, false); + let mut span_ptr = vm.add_memory_segment(); + let expected_address = span_ptr; + + let error = read_felt_array::(&vm, &mut span_ptr).unwrap_err(); - assert!(read_felt_array::(&vm, &mut span_ptr).is_err()); + assert_matches!( + error, + SyscallExecutorBaseError::Memory(MemoryError::UnknownMemoryCell(address)) + if *address == expected_address + ); } #[test]