-
Notifications
You must be signed in to change notification settings - Fork 65
apollo_proof_manager: create proof manager #10943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
noaov1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@noaov1 reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @einat-starkware).
crates/apollo_proof_manager/src/proof_manager.rs line 17 at r1 (raw file):
pub struct ProofManager { pub proof_storage: FsProofStorage, pub proof_manager_config: ProofManagerConfig,
Is it needed after creating the struct?
Code quote:
pub proof_manager_config: ProofManagerConfig,fc523ad to
d1c8766
Compare
0801697 to
c33cbdd
Compare
d1c8766 to
53af9a7
Compare
53af9a7 to
cd36ce2
Compare
c33cbdd to
4d076f5
Compare
einat-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@einat-starkware made 1 comment.
Reviewable status: 1 of 5 files reviewed, 1 unresolved discussion (waiting on @noaov1).
crates/apollo_proof_manager/src/proof_manager.rs line 17 at r1 (raw file):
Previously, noaov1 (Noa Oved) wrote…
Is it needed after creating the struct?
No, at least not currently, removed now :)
4d076f5 to
13ec6f4
Compare
cd36ce2 to
0b0cc5d
Compare
3d69625 to
ba460f3
Compare
2242925 to
5401815
Compare
einat-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@einat-starkware made 1 comment.
Reviewable status: 3 of 5 files reviewed, 1 unresolved discussion (waiting on @noaov1).
crates/apollo_proof_manager/src/proof_manager.rs line 62 at r3 (raw file):
Previously, noaov1 (Noa Oved) wrote…
Why is it needed?
Removed (I was following the pattern from other components but it is not necessary).
5401815 to
7b3250f
Compare
ba460f3 to
45412de
Compare
Itay-Tsabary-Starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 3 of 5 files reviewed, 2 unresolved discussions (waiting on @einat-starkware and @noaov1).
crates/apollo_proof_manager/src/proof_manager.rs line 32 at r5 (raw file):
)]) } }
Please move this to a new crate: apollo_proof_manager_config 🙏
Code quote:
/// Configuration for the proof manager.
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Validate)]
pub struct ProofManagerConfig {
pub persistent_root: PathBuf,
}
impl Default for ProofManagerConfig {
fn default() -> Self {
Self { persistent_root: "/data/proofs".into() }
}
}
impl SerializeConfig for ProofManagerConfig {
fn dump(&self) -> BTreeMap<ParamPath, SerializedParam> {
BTreeMap::from([ser_param(
"persistent_root",
&self.persistent_root,
"Persistent root for proof storage.",
ParamPrivacyInput::Public,
)])
}
}
Itay-Tsabary-Starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 3 of 5 files reviewed, 3 unresolved discussions (waiting on @einat-starkware and @noaov1).
crates/apollo_proof_manager/src/lib.rs line 5 at r5 (raw file):
pub use proof_manager::{ProofManager, ProofManagerConfig}; pub use proof_storage::ProofStorage;
Why re-export?
Code quote:
pub use proof_manager::{ProofManager, ProofManagerConfig};
pub use proof_storage::ProofStorage;d961693 to
0e117cc
Compare
einat-starkware
left a comment
There was a problem hiding this 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: 1 of 9 files reviewed, 3 unresolved discussions (waiting on @Itay-Tsabary-Starkware and @noaov1).
crates/apollo_proof_manager/src/lib.rs line 5 at r5 (raw file):
Previously, Itay-Tsabary-Starkware wrote…
Why re-export?
a mistake, removed now
crates/apollo_proof_manager/src/proof_manager.rs line 32 at r5 (raw file):
Previously, Itay-Tsabary-Starkware wrote…
Please move this to a new crate:
apollo_proof_manager_config🙏
Moved to new crate
0e117cc to
bd7d1ae
Compare
bd7d1ae to
b17fef6
Compare
noaov1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@noaov1 reviewed 8 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Itay-Tsabary-Starkware).
Itay-Tsabary-Starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Itay-Tsabary-Starkware made 1 comment and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @einat-starkware).
crates/apollo_proof_manager_config/Cargo.toml line 7 at r7 (raw file):
repository.workspace = true version.workspace = true
Please add a description field 🙏
Itay-Tsabary-Starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @einat-starkware).
crates/apollo_proof_manager_config/src/config.rs line 13 at r7 (raw file):
pub struct ProofManagerConfig { pub persistent_root: PathBuf, }
Please add new-lines after struct / impl blocks 🙏
b17fef6 to
54bd1d9
Compare
einat-starkware
left a comment
There was a problem hiding this 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: 7 of 9 files reviewed, 2 unresolved discussions (waiting on @Itay-Tsabary-Starkware and @noaov1).
crates/apollo_proof_manager_config/Cargo.toml line 7 at r7 (raw file):
Previously, Itay-Tsabary-Starkware wrote…
Please add a description field 🙏
Done.
crates/apollo_proof_manager_config/src/config.rs line 13 at r7 (raw file):
Previously, Itay-Tsabary-Starkware wrote…
Please add new-lines after struct / impl blocks 🙏
Done.
Itay-Tsabary-Starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Itay-Tsabary-Starkware reviewed 9 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @einat-starkware).

No description provided.