Skip to content
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

Store commitments in blob #2967

Merged
merged 2 commits into from
Jan 27, 2025
Merged

Conversation

martyall
Copy link
Contributor

@martyall martyall commented Jan 25, 2025

Summary

This PR changes the way we (de)serialize blobs to use macros rather than doing it manually -- there should be no net effect. I also started storing the commitments in the blob metadata to save us from having to recompute them for reads.

Finally there is a small change to the "sanity check" to make sure the user and storage provider agree on the data. This is not strictly part of the protocol, but rather it helps with the testing.

Changes

  • use the serde_as package to derive the serialization instances: previously defined serialization instances by hand using arkworks CanonicalSerialize trait. I found other places in the codebase that used this simpler approach.
  • store the commitments in the FieldBlob struct: otherwise we will need to recompute them for each read request. This means FieldBlob is now parameterized by the group instead of the scalar field.
  • stop returning the "randomized sum of commitments" from store request -- they are not used until later anyway. We can just return a hash for now for the "sanity check". @marcbeunardeau88 Now I understand this is what you were saying here

@martyall martyall mentioned this pull request Jan 25, 2025
Copy link

codecov bot commented Jan 25, 2025

Codecov Report

Attention: Patch coverage is 60.60606% with 13 lines in your changes missing coverage. Please review.

Project coverage is 76.82%. Comparing base (e0ac06d) to head (8128e3d).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
saffron/src/main.rs 0.00% 12 Missing ⚠️
saffron/src/commitment.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2967   +/-   ##
=======================================
  Coverage   76.81%   76.82%           
=======================================
  Files         260      260           
  Lines       61371    61307   -64     
=======================================
- Hits        47145    47098   -47     
+ Misses      14226    14209   -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@martyall martyall force-pushed the martin/saffron-store-commitments-in-blob branch from 6e28e8e to 8128e3d Compare January 27, 2025 18:26
@martyall martyall marked this pull request as ready for review January 27, 2025 18:39
@@ -26,18 +25,6 @@ pub fn commit_to_field_elems<G: CommitmentCurve>(
.collect()
}

#[instrument(skip_all)]
pub fn commit_to_blob<G: CommitmentCurve>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not deleted -- this and the tests below just got moved to the blob module

Copy link
Contributor

@Fizzixnerd Fizzixnerd left a comment

Choose a reason for hiding this comment

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

looks good

@martyall martyall merged commit 071617f into master Jan 27, 2025
13 checks passed
@martyall martyall deleted the martin/saffron-store-commitments-in-blob branch January 27, 2025 22:06
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.

2 participants