starknet_os_flow_tests: drive commitment keys from compute_accessed_keys

[yoavGrs]

Replace the legacy initial_reads.keys() input to StateCommitmentInfos::new
with the same accessed-keys aggregate the batcher writes — validating
end-to-end that what the batcher persists is sufficient for the OS to
replay the block. ProofFacts block numbers are extracted from the block's
BlockifierTransactions (via create_tx_info -> Current -> proof_facts ->
ProofFactsVariant::Snos) and passed through to compute_accessed_keys.
Virtual-OS mode skips the alias-contract entries since the executor doesn't
run allocate_aliases_in_storage there. The resulting StateChangesKeys is
built via From.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

[reviewable-StarkWare]

This change is 

[yoavGrs]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• starknet_os_flow_tests: drive commitment keys from compute_accessed_keys #14106 👈 (View in Graphite)
• apollo_storage,apollo_batcher,apollo_reverts: write accessed keys instead of tx execution infos #14105
• blockifier: take CommitmentStateDiff in AccessedKeys::new and predicted_alias_storage_entries #14156
• apollo_batcher: record ProofFacts block_number per tx #14104
• blockifier: add compute_accessed_keys and AccessedKeys #14103
• blockifier: add predicted_alias_storage_entries #14102
• blockifier: add CallInfo::get_visited_contract_addresses #14101
• apollo_integration_tests: add shared proof-flow infrastructure (#13760) #14100
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[cursor]

PR Summary

Low Risk
Changes are limited to shared proof-facts parsing, committer conversion helpers, and OS flow-test wiring; production batcher behavior is a small refactor to the shared helper.

Overview
OS flow tests now build StateCommitmentInfos from AccessedKeys (execution infos, SNOS proof-facts block numbers, CommitmentStateDiff, versioned constants) instead of initial_reads.keys(), matching what the batcher persists via write_block_accessed_keys. In virtual OS mode, versioned constants clone with enable_stateful_compression disabled so alias-contract keys are not expected when the executor skips alias allocation.

Shared SNOS parsing: snos_block_number_from_proof_facts in starknet_api centralizes extracting the proof base block number; the batcher’s proof_facts_block_number and flow-test collect_proof_facts_block_numbers call it.

Committer path: state is committed from CommitmentStateDiff via new commitment_state_diff_to_committer_state_diff (parallel to existing StateMaps conversion).

^{Reviewed by Cursor Bugbot for commit 24b6455. Bugbot is set up for automated code reviews on this repo. Configure here.}

[ArielElp]

@ArielElp reviewed 1 file and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on yoavGrs).

---

crates/starknet_os_flow_tests/src/test_manager.rs line 875 at r1 (raw file):

                &execution_outputs,
            );
            let initial_reads = get_extended_initial_reads(&final_state);

Should this be removed now? Are they used elsewhere?

Code quote:

let initial_reads = get_extended_initial_reads(&final_state);

---

crates/starknet_os_flow_tests/src/test_manager.rs line 906 at r1 (raw file):

                !self.virtual_os,
            );
            let commitment_keys = StateChangesKeys::from(accessed_keys);

commitment_keys is a weird name IMO, maybe add a type hint on accessed_keys and use .into()

---

crates/starknet_os_flow_tests/src/test_manager.rs line 1027 at r1 (raw file):

/// and non-account transactions yield no entry. Mirrors the production extraction performed in the
/// batcher's block_builder.
fn collect_proof_facts_block_numbers(block_txs: &[BlockifierTransaction]) -> Vec<BlockNumber> {

Any hope to unify this logic with the batcher's or is there no common ground between blockifier transactions and consensus transactions?

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on ArielElp and yoavGrs).

---

crates/starknet_os_flow_tests/src/test_manager.rs line 1027 at r1 (raw file):

Previously, ArielElp wrote…

Any hope to unify this logic with the batcher's or is there no common ground between blockifier transactions and consensus transactions?

the match statement looks identical, seems like it could be shared, no?

---

crates/starknet_os_flow_tests/src/test_manager.rs line 904 at r2 (raw file):

                &state_diff,
                *ALIAS_CONTRACT_ADDRESS,
                !self.virtual_os,

ah.
if that's the case, I still stand with my VC-as-input request for production - use helper functions to get the behavior you need here, like

fn compute_accessed_keys(.., vc: &VersionedConstants) -> AccessedKeys {
    compute_accessed_keys_helper(.., vc.alias_contract_address(), vc.enable_compression)
}

Code quote:

!self.virtual_os

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on ArielElp and yoavGrs).

---

crates/starknet_os_flow_tests/src/test_manager.rs line 877 at r3 (raw file):

            let initial_reads = get_extended_initial_reads(&final_state);
            let state_diff = final_state.to_state_diff().unwrap().state_maps;
            // Drive the OS commitment-keys input from the same accessed-keys aggregate the batcher

typo?

Code quote:

Drive

[yoavGrs]

@yoavGrs made 5 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on ArielElp and dorimedini-starkware).

---

crates/starknet_os_flow_tests/src/test_manager.rs line 875 at r1 (raw file):

Previously, ArielElp wrote…

Should this be removed now? Are they used elsewhere?

Not in this PR.
It is used for reading the values of the initial reads.

---

crates/starknet_os_flow_tests/src/test_manager.rs line 906 at r1 (raw file):

Previously, ArielElp wrote…

commitment_keys is a weird name IMO, maybe add a type hint on accessed_keys and use .into()

Done.

---

crates/starknet_os_flow_tests/src/test_manager.rs line 1027 at r1 (raw file):

Previously, dorimedini-starkware wrote…

the match statement looks identical, seems like it could be shared, no?

Share some code.

---

crates/starknet_os_flow_tests/src/test_manager.rs line 904 at r2 (raw file):

Previously, dorimedini-starkware wrote…

ah.
if that's the case, I still stand with my VC-as-input request for production - use helper functions to get the behavior you need here, like

fn compute_accessed_keys(.., vc: &VersionedConstants) -> AccessedKeys {
    compute_accessed_keys_helper(.., vc.alias_contract_address(), vc.enable_compression)
}

Now it's with VC.

---

crates/starknet_os_flow_tests/src/test_manager.rs line 877 at r3 (raw file):

Previously, dorimedini-starkware wrote…

typo?

Rephasing the comment.

[github-actions]

Artifacts upload workflows:

• Started at 2026-05-24T10:21:48Z
• Started at 2026-05-24T10:41:07Z
• Started at 2026-05-24T14:11:04Z
• Started at 2026-05-24T14:23:08Z
• Started at 2026-05-24T14:36:32Z
• Started at 2026-05-25T07:13:13Z

[dorimedini-starkware]

@dorimedini-starkware reviewed 3 files and all commit messages, made 2 comments, and resolved 3 discussions.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on ArielElp and yoavGrs).

---

crates/starknet_api/src/transaction/fields.rs line 827 at r4 (raw file):

        Ok(ProofFactsVariant::Empty) | Err(_) => None,
    }
}

shouldn't this be a method of ProofFacts? (non blocking)

Code quote:

/// If `proof facts` is parsed as valid SNOS data, returns the base block number used to verify the
/// proof. Otherwise returns `None`.
pub fn snos_block_number_from_proof_facts(proof_facts: &ProofFacts) -> Option<BlockNumber> {
    match ProofFactsVariant::try_from(proof_facts) {
        Ok(ProofFactsVariant::Snos(snos)) => Some(snos.block_number),
        Ok(ProofFactsVariant::Empty) | Err(_) => None,
    }
}

---

crates/starknet_os_flow_tests/src/test_manager.rs line 886 at r4 (raw file):

            let accessed_keys_versioned_constants: Cow<'_, VersionedConstants> = if self.virtual_os
            {
                let mut vc = base_block_context.versioned_constants().clone();

this is the block context used to derive all block contexts for the blocks running in the OS, right?

Code quote:

base_block_context

[yoavGrs]

@yoavGrs made 2 comments.
Reviewable status: 2 of 4 files reviewed, 4 unresolved discussions (waiting on ArielElp and dorimedini-starkware).

---

crates/starknet_api/src/transaction/fields.rs line 827 at r4 (raw file):

Previously, dorimedini-starkware wrote…

shouldn't this be a method of ProofFacts? (non blocking)

Claude did it that way, but I asked him to change it. ProofFacts doesn't know what a block number is.

---

crates/starknet_os_flow_tests/src/test_manager.rs line 886 at r4 (raw file):

Previously, dorimedini-starkware wrote…

this is the block context used to derive all block contexts for the blocks running in the OS, right?

Right, so maybe it's better to use the current block context.

[dorimedini-starkware]

@dorimedini-starkware reviewed 2 files and all commit messages, and resolved 2 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on ArielElp).

[yoavGrs]

@yoavGrs made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on ArielElp).

---

crates/starknet_os_flow_tests/src/test_manager.rs line 875 at r1 (raw file):

Previously, yoavGrs wrote…

Not in this PR.
It is used for reading the values of the initial reads.

I can filter out keys that are not in the accessed_keys. Does it have a benefit?