Skip to content

apollo_committer,starknet_committer: use blockifier's AccessedKeys#14252

Merged
ArielElp merged 1 commit into
mainfrom
ariel/switch_to_blockifier_accessed_keys
Jun 7, 2026
Merged

apollo_committer,starknet_committer: use blockifier's AccessedKeys#14252
ArielElp merged 1 commit into
mainfrom
ariel/switch_to_blockifier_accessed_keys

Conversation

@ArielElp

Copy link
Copy Markdown
Contributor

No description provided.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@ArielElp ArielElp changed the base branch from ariel/add_request_handler to graphite-base/14252 May 28, 2026 15:29
@ArielElp ArielElp marked this pull request as ready for review May 31, 2026 10:43
@cursor

cursor Bot commented May 31, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes the OS/committer witness input shape across apollo_committer, starknet_committer, and starknet_os; behavior should be equivalent but any mismatch in key sets would affect Patricia digests and proofs.

Overview
Unifies trie witness / Patricia path collection on blockifier’s AccessedKeys instead of ad‑hoc class_hashes, contract lists, and per-contract storage maps (or StateChangesKeys in the OS).

starknet_committer now depends on blockifier and builds LeavesRequest via From<&AccessedKeys>, with sorting moved to LeavesRequest::sorted(). fetch_previous_and_new_patricia_paths takes a single &AccessedKeys. apollo_committer’s read_paths_and_commit_block uses that path directly for digest + witnesses.

blockifier adds From<StateChangesKeys> for AccessedKeys. starknet_os StateCommitmentInfos::new and the transaction prover pass AccessedKeys (or .into() from state-change keys) and drop the local StateChangesKeys → Patricia wrapper. Flow tests pass &accessed_keys without converting to StateChangesKeys first.

Reviewed by Cursor Bugbot for commit 0d7ef4c. Bugbot is set up for automated code reviews on this repo. Configure here.

@ArielElp ArielElp force-pushed the graphite-base/14252 branch from 74ed1a0 to b46768a Compare May 31, 2026 13:47
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from 53fa580 to 5fc1b5d Compare May 31, 2026 13:47
@ArielElp ArielElp changed the base branch from graphite-base/14252 to ariel/add_request_handler May 31, 2026 13:48
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from 5fc1b5d to c805f67 Compare May 31, 2026 14:41
@ArielElp ArielElp force-pushed the ariel/add_request_handler branch from b46768a to 715e282 Compare May 31, 2026 14:41
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from c805f67 to 97e9ad4 Compare May 31, 2026 17:03
@ArielElp ArielElp force-pushed the ariel/add_request_handler branch 2 times, most recently from d6c7ffb to df29941 Compare June 1, 2026 08:24
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from 97e9ad4 to a4466a7 Compare June 1, 2026 08:24
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from a4466a7 to a5ac682 Compare June 1, 2026 08:41
@ArielElp ArielElp requested a review from yoavGrs June 1, 2026 12:39
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from a5ac682 to 320dff3 Compare June 2, 2026 11:57

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 50bbdd5. Configure here.

Comment thread crates/blockifier/src/state/accessed_keys.rs

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

@ArielElp ArielElp force-pushed the graphite-base/14252 branch from 5e5b48a to 972670a Compare June 2, 2026 16:02
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from 50bbdd5 to 1741bec Compare June 2, 2026 16:02
@ArielElp ArielElp changed the base branch from graphite-base/14252 to ariel/test_revert_flow_with_witnesses June 2, 2026 16:03
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from 1741bec to 263e820 Compare June 2, 2026 17:21
@ArielElp ArielElp force-pushed the ariel/test_revert_flow_with_witnesses branch from 972670a to 00aaf1a Compare June 2, 2026 17:21

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

@ArielElp ArielElp force-pushed the ariel/test_revert_flow_with_witnesses branch from 00aaf1a to 74fccf6 Compare June 3, 2026 11:07
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch 2 times, most recently from 2534a7d to c698d25 Compare June 3, 2026 13:14
@ArielElp ArielElp force-pushed the ariel/test_revert_flow_with_witnesses branch from 74fccf6 to 41082ca Compare June 3, 2026 13:14
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from c698d25 to b9f4b32 Compare June 3, 2026 15:17
@ArielElp ArielElp force-pushed the ariel/test_revert_flow_with_witnesses branch from 41082ca to 651ab0e Compare June 3, 2026 15:17

@ArielElp ArielElp left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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


crates/apollo_committer/src/committer.rs line 35 at r2 (raw file):

Previously, dorimedini-starkware wrote…

looks like this should be gated

Not needed actually


crates/starknet_committer/Cargo.toml line 18 at r2 (raw file):

Previously, dorimedini-starkware wrote…

right? not needed if no os inputs?

Done.


crates/starknet_os/src/commitment_infos.rs line 188 at r2 (raw file):

Previously, dorimedini-starkware wrote…

can't this function just be deleted now?

Agreed, note this induces using os_input in the starknet_committer dep of starknet_os, in order for fetch_previous_and_new_patricia_paths to be available there (now that blockifier and AccessedKeys are optional)

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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


crates/starknet_os/src/commitment_infos.rs line 188 at r2 (raw file):

Previously, ArielElp wrote…

Agreed, note this induces using os_input in the starknet_committer dep of starknet_os, in order for fetch_previous_and_new_patricia_paths to be available there (now that blockifier and AccessedKeys are optional)

ah... so, revert. not worth the risk of activating the feature in production code.
you can consider adding an os_input feature to starknet_os, which - if active - activates the starknet_committer's feature


crates/starknet_os/Cargo.toml line 65 at r5 (raw file):

starknet-types-core.workspace = true
starknet_api.workspace = true
starknet_committer = { workspace = true, features = ["os_input"] }

not in production deps!

Code quote:

starknet_committer = { workspace = true, features = ["os_input"] }

@ArielElp ArielElp changed the base branch from ariel/test_revert_flow_with_witnesses to graphite-base/14252 June 3, 2026 15:52
@ArielElp ArielElp force-pushed the graphite-base/14252 branch from 651ab0e to c26c606 Compare June 3, 2026 15:52
@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from b9f4b32 to ad400eb Compare June 3, 2026 15:52
@ArielElp ArielElp changed the base branch from graphite-base/14252 to main June 3, 2026 15:52

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from ad400eb to 1ef5a1c Compare June 4, 2026 08:15

@ArielElp ArielElp left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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


crates/starknet_os/Cargo.toml line 65 at r5 (raw file):

Previously, dorimedini-starkware wrote…

not in production deps!

Done.


crates/starknet_os/src/commitment_infos.rs line 188 at r2 (raw file):

Previously, dorimedini-starkware wrote…

ah... so, revert. not worth the risk of activating the feature in production code.
you can consider adding an os_input feature to starknet_os, which - if active - activates the starknet_committer's feature

Then I think we must revert making blockifier optional in starknet_committer:

  1. starknet_os needs to call fetch_previous_and_new_patricia_paths, either with the above (useless) wrapper or directly
  2. this function only exists in starknet_committer under os_input, because of the blockifier type

So I think that either we accept os_input in starknet_committer or keep blockifier non optional

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

@ArielElp ArielElp force-pushed the ariel/switch_to_blockifier_accessed_keys branch from 1ef5a1c to 0d7ef4c Compare June 7, 2026 07:57

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

@ArielElp ArielElp added this pull request to the merge queue Jun 7, 2026
Merged via the queue into main with commit 7e41fc2 Jun 7, 2026
47 of 52 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