apollo_starknet_os_program: apply v0.14.3 OS-core audit review comments

crates/apollo_starknet_os_program/src/cairo/starkware/starknet/core/os/execution/syscall_impls.cairo

@@ -1065,8 +1065,10 @@ func execute_sha512_process_block{
     let state: Sha512State* = &sha512_ptr.in_state;
     assert [state] = [request.state_ptr];
 
-    // Relocate response.state_ptr (a temporary segment) to actual_out_state in the sha512
-    // segment, and copy the state so finalize_sha512 can read it.
+    // Relocate `response.state_ptr` (allocated by the syscall hint) to the next `out_state`
+    // slot in the sha512 segment (`actual_out_state`) and assert that the two pointers are equal.
+    // Also copy [state_ptr] into [actual_out_state], since finalize_sha512 reads from
+    // [actual_out_state] and the relocation happens in the opposite direction.
     %{ RelocateSha512Segment %}
 
     assert [response] = Sha512ProcessBlockResponse(state_ptr=actual_out_state);

crates/apollo_starknet_os_program/src/cairo/starkware/starknet/core/os/naive_blake.cairo

@@ -3,6 +3,9 @@ from starkware.cairo.common.cairo_blake2s.blake2s import blake_with_opcode
 
 // Gets a felt that represents a 256-bit unsigned integer stored as an array of eight 32-bit unsigned integers
 // represented in little-endian notation. Returns the felt representation of the integer modulo prime.
+// Assumption: each element of `u32s` is a valid 32-bit value (in [0, 2**32)). This is not checked
+// here; the caller must guarantee it (in practice, the Blake opcode range-checks the limbs it
+// consumes).
 func felt_from_le_u32s(u32s: felt*) -> felt {
     let value = u32s[7] * 2 ** 224 + u32s[6] * 2 ** 192 + u32s[5] * 2 ** 160 + u32s[4] * 2 ** 128 +
         u32s[3] * 2 ** 96 + u32s[2] * 2 ** 64 + u32s[1] * 2 ** 32 + u32s[0];
@@ -59,6 +62,15 @@ func create_initial_state_for_blake2s() -> (initial_state: felt*) {
 }
 
 // Encodes one felt252 into eight u32s represented in little-endian order.
+//
+// The hint-provided limbs are only constrained here by the assertion below, i.e. that
+// `felt_from_le_u32s(unpacked_u32s) == packed_value` (which holds modulo PRIME). This function does
+// NOT guarantee that:
+//   (1) each limb is a valid 32-bit value (in [0, 2**32)) - this is enforced later by the Blake
+//       opcode that consumes the limbs; or
+//   (2) the 256-bit value the limbs represent is the canonical representative in [0, PRIME-1] - a
+//       non-canonical encoding of `packed_value + k*PRIME` would also satisfy the assertion.
+// Callers that require either property must enforce it themselves.
 func naive_encode_felt252_to_u32s(packed_value: felt, unpacked_u32s: felt*) {
     %{ NaiveUnpackFelt252ToU32s %}
     // TODO(Noa): Assert that the limbs represent a number in the range [0, PRIME-1].
@@ -71,6 +83,9 @@ func naive_encode_felt252_to_u32s(packed_value: felt, unpacked_u32s: felt*) {
 
 // Encodes one `Felt` into an eight 32-bit word, then hashes the resulting byte stream
 // with Blake2s-256 and returns the 256-bit digest to a 252-bit field element `Felt`.
+// Note: the encoded limbs are not range-checked by the encoding step (see
+// `naive_encode_felt252_to_u32s`); their u32 range is enforced by the Blake opcode that consumes
+// them, and the encoding is not guaranteed to be the canonical (< PRIME) representation of `item`.
 func calc_blake_hash_single{range_check_ptr: felt}(item: felt) -> (hash: felt) {
     alloc_locals;
     let (local encoded_data: felt*) = alloc();
@@ -83,6 +98,9 @@ func calc_blake_hash_single{range_check_ptr: felt}(item: felt) -> (hash: felt) {
 
 // Encodes `n_felts` felt252 values (starting at `data`) into 8 u32 LE limbs each,
 // then hashes the resulting byte stream with Blake2s-256.
+// Note: the encoded limbs are not range-checked by the encoding step (see
+// `naive_encode_felt252_to_u32s`); their u32 range is enforced by the Blake opcode that consumes
+// them, and the encoding is not guaranteed to be the canonical (< PRIME) representation of `data`.
 func calc_naive_blake_hash{range_check_ptr: felt}(n_felts: felt, data: felt*) -> felt {
     alloc_locals;
     let (local encoded_data: felt*) = alloc();

crates/apollo_starknet_os_program/src/cairo/starkware/starknet/core/os/os_config/os_config.cairo

@@ -24,8 +24,6 @@ struct StarknetOsConfig {
 
 // Calculates the hash of StarkNet OS config. The public keys hash is not included if there are no
 // public keys (i.e., for envs where the state diff is not encrypted).
-// The top-level hash uses Blake; the public_keys_hash field is itself a Blake digest (see
-// get_public_keys_hash) and is absorbed here as a single felt.
 func get_starknet_os_config_hash{range_check_ptr}(starknet_os_config: StarknetOsConfig*) -> (
     starknet_os_config_hash: felt
 ) {
@@ -39,8 +37,8 @@ func get_starknet_os_config_hash{range_check_ptr}(starknet_os_config: StarknetOs
         // remove the following `if`.
         if (starknet_os_config.public_keys_hash != DEFAULT_PUBLIC_KEYS_HASH) {
             hash_update_single(item=starknet_os_config.public_keys_hash);
-            tempvar hash_state = hash_state;
         } else {
+            // Align the stack.
             tempvar hash_state = hash_state;
         }
     }

crates/apollo_starknet_os_program/src/cairo/starkware/starknet/core/os/state/state.cairo

@@ -147,8 +147,10 @@ func squash_state_changes_and_maybe_allocate_aliases{range_check_ptr}(
 
     // Squash the storage updates of the alias contract.
     // There should be no access to this storage before, so it is enough to squash it separately
-    // instead of running `squash_state_changes` again. This assumption is enforced by the Patricia
-    // update code.
+    // instead of running `squash_state_changes` again.
+    // This holds because the alias contract has no code of its own, and no syscall lets a contract
+    // read or write another contract's storage; hence nothing can access the alias contract's
+    // storage. This assumption is enforced by the Patricia update code.
     local squashed_aliases_storage_start: DictAccess*;
     local prev_aliases_state_entry: StateEntry*;
     %{ GuessAliasesContractStoragePtr %}