-
Notifications
You must be signed in to change notification settings - Fork 112
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
[saffron] add commitment type shared between user and provider #3005
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3005 +/- ##
==========================================
- Coverage 76.80% 76.80% -0.01%
==========================================
Files 261 261
Lines 61882 61888 +6
==========================================
Hits 47530 47530
- Misses 14352 14358 +6 ☔ View full report in Codecov by Sentry. |
@@ -11,6 +11,7 @@ SRS_ARG="" | |||
if [ $# -eq 2 ]; then | |||
SRS_ARG="--srs-filepath $2" | |||
fi | |||
COMMITMENT_FILE="${INPUT_FILE%.*}_commitment.bin" |
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.
Same comment that here.
@@ -11,6 +11,7 @@ SRS_ARG="" | |||
if [ $# -eq 2 ]; then | |||
SRS_ARG="--srs-filepath $2" | |||
fi | |||
COMMITMENT_FILE="${INPUT_FILE%.*}_commitment.bin" | |||
ENCODED_FILE="${INPUT_FILE%.*}.bin" | |||
DECODED_FILE="${INPUT_FILE%.*}-decoded${INPUT_FILE##*.}" |
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.
Same comment than here.
COMMITMENT=$(cargo run --release --bin saffron compute-commitment -i "$INPUT_FILE" $SRS_ARG | tee /dev/stderr | tail -n 1) | ||
|
||
|
||
COMMITMENT=$(cargo run --release --bin saffron compute-commitment -i "$INPUT_FILE" -o "$COMMITMENT_FILE" $SRS_ARG | tee /dev/stderr | tail -n 1) |
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.
Why tee /dev/stderr
?
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.
This is so that we can capture the final output of the command in the COMMITMENT
variable as well as display the logs that it generates in the console while it is running
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.
The changes are mostly structural. I would like seeing addressed https://github.com/o1-labs/proof-systems/pull/3005/files#r1948063167 and https://github.com/o1-labs/proof-systems/pull/3005/files#r1948063225 and https://github.com/o1-labs/proof-systems/pull/3005/files#r1948063496 in a follow-up PR.
Summary
The user and storage provider need to communicate about a common commitment type. When working on #3004 I realized that the user will also need to persist these commitments in order to apply updates to them when updating their data. It makes #3004 cleaner if I do this first
Changes
Commitment
type (5b99888) and use it in the blob (3940789)-o [output-file]
arg to thecompute-commitment
CLI command to write theCommitment
value to disk 9bb1b56commitment
module 927ff5b