Skip to content

starknet_committer,starknet_os: relocate commitment infos to starknet_committer#14447

Merged
itamar-starkware merged 1 commit into
mainfrom
starknet_committer_relocate_commitment_infos
Jun 14, 2026
Merged

starknet_committer,starknet_os: relocate commitment infos to starknet_committer#14447
itamar-starkware merged 1 commit into
mainfrom
starknet_committer_relocate_commitment_infos

Conversation

@itamar-starkware

Copy link
Copy Markdown
Contributor

No description provided.

itamar-starkware commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@itamar-starkware itamar-starkware marked this pull request as ready for review June 10, 2026 12:32
@itamar-starkware

Copy link
Copy Markdown
Contributor Author

Why this PR exists

Second in the stack (#14446#14447#14448). The goal is to persist StateCommitmentInfos in apollo_storage (#14448) and, later, have the committer produce it.

CommitmentInfo / StateCommitmentInfos currently live in starknet_os. But the components that need to store/produce them — apollo_storage, and later the committer — sit below the OS. If the structs stayed in starknet_os, those lower layers would have to depend on starknet_os: a backwards dependency.

So this PR relocates the two structs down to starknet_committer (right next to StarknetForestProofs), starknet_os re-exports them so all OS consumers are unaffected, and adds the serde / Clone / PartialEq derives the storage layer needs. Because the struct now lives in another crate, Rust's orphan rule forbids keeping the inherent StateCommitmentInfos::new builder in starknet_os, so it becomes a free function build_state_commitment_infos (kept in starknet_os, marked temporary — to be removed once the committer builds the infos from its own storage).

This does not change the dependency graph:

  • starknet_os already depends on starknet_committer (commitment_infos.rs imports fetch_previous_and_new_patricia_paths, RootHashes, get_leaves, … from it), so the move is "downhill" — no new or reversed edge.
  • The structs are defined over low-level types only (HashOutput, SubTreeHeight, ContractAddress, Felt), all of which starknet_committer already depends on — so starknet_committer gains no dependency on starknet_os.
  • Downstream, this is exactly what avoids a bad edge: the committer can build/return these types and apollo_storage can store them, both via their existing starknet_committer dependency — neither acquires a starknet_os dependency.

Unblocks #14448.

@itamar-starkware itamar-starkware requested a review from yoavGrs June 10, 2026 14:00

@yoavGrs yoavGrs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yoavGrs reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on itamar-starkware).


crates/starknet_os/src/commitment_infos.rs line 39 at r1 (raw file):

/// Creates the commitment infos for the OS from previous and new state roots and the
/// keys that were read during execution.
// TODO: Temporary — to be deleted once the committer builds `StateCommitmentInfos` from its own

a. Do you plan to change the OS test to use the new committer's API?
b. Assign yourself to the TODO?

Code quote:

TODO: Temporary 

@itamar-starkware

Copy link
Copy Markdown
Contributor Author

Thanks @yoavGrs! Addressed both:

a. Yes — once the committer exposes the new StateCommitmentInfos API (your upcoming PR), the OS flow test (test_manager.rs) and the prover (storage_proofs.rs) will switch to it and build_state_commitment_infos gets deleted. We'll add tests against the new committer API at that point.

b. Done — assigned the TODO to me: // TODO(ItamarS): Temporary — … (in the updated commit).

@itamar-starkware itamar-starkware force-pushed the starknet_committer_relocate_commitment_infos branch from 9f39c98 to 8df0e9b Compare June 10, 2026 15:29
@itamar-starkware itamar-starkware force-pushed the starknet_patricia_subtree_height_serde branch from e2df863 to 107091d Compare June 11, 2026 07:09
@itamar-starkware itamar-starkware force-pushed the starknet_committer_relocate_commitment_infos branch from 8df0e9b to 2923883 Compare June 11, 2026 07:09
@itamar-starkware itamar-starkware force-pushed the starknet_committer_relocate_commitment_infos branch from 2923883 to c64de43 Compare June 11, 2026 10:02
@itamar-starkware itamar-starkware force-pushed the starknet_patricia_subtree_height_serde branch from 107091d to 737058f Compare June 11, 2026 10:02

@yoavGrs yoavGrs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yoavGrs reviewed 4 files and all commit messages, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on itamar-starkware).

@itamar-starkware itamar-starkware changed the base branch from starknet_patricia_subtree_height_serde to main June 11, 2026 12:17
@itamar-starkware itamar-starkware force-pushed the starknet_committer_relocate_commitment_infos branch 4 times, most recently from 247fbd5 to c728652 Compare June 13, 2026 23:12
@itamar-starkware itamar-starkware changed the base branch from main to graphite-base/14447 June 14, 2026 11:01
@itamar-starkware itamar-starkware force-pushed the starknet_committer_relocate_commitment_infos branch from c728652 to e6f6485 Compare June 14, 2026 11:01
@itamar-starkware itamar-starkware changed the base branch from graphite-base/14447 to starknet_os_commitment_infos_free_function June 14, 2026 11:01
@graphite-app

graphite-app Bot commented Jun 14, 2026

Copy link
Copy Markdown

Merge activity

  • Jun 14, 11:02 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.
  • Jun 14, 12:57 PM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@graphite-app graphite-app Bot changed the base branch from starknet_os_commitment_infos_free_function to main June 14, 2026 11:02
@itamar-starkware itamar-starkware changed the base branch from main to graphite-base/14447 June 14, 2026 11:55
@itamar-starkware itamar-starkware changed the base branch from graphite-base/14447 to main June 14, 2026 11:56
@itamar-starkware itamar-starkware changed the base branch from main to graphite-base/14447 June 14, 2026 11:57
@itamar-starkware itamar-starkware changed the base branch from graphite-base/14447 to starknet_os_commitment_infos_free_function June 14, 2026 11:59

yoavGrs commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Add apollo_storage to the tests in .github/workflows/apollo_storage_os_input_ci.yml

@yoavGrs yoavGrs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add apollo_storage to the tests in .github/workflows/apollo_storage_os_input_ci.yml

@itamar-starkware

Copy link
Copy Markdown
Contributor Author

It's already inside this yaml

@yoavGrs yoavGrs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yoavGrs reviewed 9 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on itamar-starkware).

@sobhe-Starkware sobhe-Starkware changed the base branch from starknet_os_commitment_infos_free_function to main June 14, 2026 12:56
@itamar-starkware itamar-starkware force-pushed the starknet_committer_relocate_commitment_infos branch from e6f6485 to 8b03628 Compare June 14, 2026 12:56
@itamar-starkware itamar-starkware added this pull request to the merge queue Jun 14, 2026
Merged via the queue into main with commit 62c30ea Jun 14, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants