Skip to content

Conversation

@einat-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

einat-starkware commented Dec 18, 2025

@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from d579010 to ed9ff7d Compare December 18, 2025 11:58
@einat-starkware einat-starkware force-pushed the einat/proof_manager/read_write_proof branch from 72c429f to 58f0c28 Compare December 18, 2025 11:58
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from ed9ff7d to 09769a0 Compare December 18, 2025 12:30
@einat-starkware einat-starkware force-pushed the einat/proof_manager/read_write_proof branch from 58f0c28 to c0ca05c Compare December 18, 2025 12:30
Copy link
Collaborator

@noaov1 noaov1 left a comment

Choose a reason for hiding this comment

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

@noaov1 reviewed 1 file and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @einat-starkware).


crates/apollo_proof_manager/src/proof_storage.rs line 130 at r1 (raw file):

    }

    fn write_proof_atomically(&self, facts_hash: Felt, proof: Proof) -> FsProofStorageResult<()> {

Why is it needed? 🤔

Code quote:

    fn write_proof_atomically(&self, facts_hash: Felt, proof: Proof) -> FsProofStorageResult<()> {

crates/apollo_proof_manager/src/proof_storage.rs line 151 at r1 (raw file):

    fn get_proof(&self, facts_hash: Felt) -> Result<Option<Proof>, Self::Error> {
        match self.read_proof_from_file(facts_hash) {

When do you return Ok(None)?

Code quote:

        match self.read_proof_from_file(facts_hash) {

@einat-starkware einat-starkware changed the base branch from einat/proof_manager/read_write_proof to graphite-base/10901 December 21, 2025 08:17
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from 09769a0 to b416cd8 Compare December 21, 2025 09:35
@einat-starkware einat-starkware changed the base branch from graphite-base/10901 to einat/proof_manager/read_write_proof December 21, 2025 09:35
Copy link
Contributor Author

@einat-starkware einat-starkware left a comment

Choose a reason for hiding this comment

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

@einat-starkware made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @noaov1).


crates/apollo_proof_manager/src/proof_storage.rs line 130 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why is it needed? 🤔

I followed the same logic as the class storage - if it crashes in the middle of writing the proof, it won't return an empty file or a partial write, but will instead be as if the write never happened


crates/apollo_proof_manager/src/proof_storage.rs line 151 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

When do you return Ok(None)?

If it doesn't contain the proof - i added in a check :)

Copy link
Collaborator

@noaov1 noaov1 left a comment

Choose a reason for hiding this comment

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

:lgtm:

@noaov1 reviewed 1 file and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @AvivYossef-starkware).

@einat-starkware einat-starkware force-pushed the einat/proof_manager/read_write_proof branch from 4be5b48 to 94630c2 Compare December 21, 2025 11:00
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from b416cd8 to 78db681 Compare December 21, 2025 11:00
@einat-starkware einat-starkware force-pushed the einat/proof_manager/read_write_proof branch from 94630c2 to bb3378c Compare December 21, 2025 14:47
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from 78db681 to 55123ae Compare December 21, 2025 14:47
Copy link
Collaborator

@noaov1 noaov1 left a comment

Choose a reason for hiding this comment

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

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

@einat-starkware einat-starkware force-pushed the einat/proof_manager/read_write_proof branch from bb3378c to 4aa1eff Compare December 22, 2025 08:25
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from 55123ae to 3da6770 Compare December 22, 2025 08:25
@graphite-app graphite-app bot changed the base branch from einat/proof_manager/read_write_proof to graphite-base/10901 December 22, 2025 08:53
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from 3da6770 to 868660e Compare December 22, 2025 09:27
@einat-starkware einat-starkware changed the base branch from graphite-base/10901 to einat/proof_manager/read_write_proof December 22, 2025 09:27
@einat-starkware einat-starkware force-pushed the einat/proof_manager/read_write_proof branch from c5a5a4c to 50d5465 Compare December 22, 2025 14:50
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from 868660e to 3041a88 Compare December 22, 2025 14:50
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from 3041a88 to f91b41c Compare December 25, 2025 10:28
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from f91b41c to fa7fc76 Compare December 28, 2025 11:08
@einat-starkware einat-starkware force-pushed the einat/proof_manager/read_write_proof branch from 2b6f1f1 to ec840fa Compare December 28, 2025 11:08
@einat-starkware einat-starkware changed the base branch from einat/proof_manager/read_write_proof to main December 28, 2025 13:12
@einat-starkware einat-starkware force-pushed the einat/proof_manager/impl_proof_storage branch from fa7fc76 to 71f2592 Compare December 28, 2025 13:12
@graphite-app
Copy link

graphite-app bot commented Dec 28, 2025

Merge activity

  • Dec 28, 1:13 PM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@einat-starkware einat-starkware added this pull request to the merge queue Dec 28, 2025
Merged via the queue into main with commit d631bfc Dec 28, 2025
13 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants