-
Notifications
You must be signed in to change notification settings - Fork 56
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
Implement persistent commitments #543
Changes from 4 commits
09af891
076b70d
2fce9ed
68d26bd
4d4b44d
4cd3914
c1f3fb8
af42e1b
5fe6b6a
d87c21d
a4cb3e4
b79a72d
55be212
d66321e
b0a0fde
e1aa3cc
52b4d2b
1d9deae
e3c7ca9
46bf080
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
use anyhow::Result; | ||
|
||
use lurk::{field::LurkField, ptr::Ptr, store::Store, z_ptr::ZExprPtr, z_store::ZStore}; | ||
use serde::{Deserialize, Serialize}; | ||
|
||
/// Holds data for commitments. | ||
/// | ||
/// WARNING: CONTAINS PRIVATE DATA | ||
#[derive(Serialize, Deserialize)] | ||
pub struct Commitment<F: LurkField> { | ||
pub hidden: ZExprPtr<F>, | ||
pub zstore: ZStore<F>, | ||
} | ||
|
||
impl<'a, F: LurkField + Serialize + Deserialize<'a>> Commitment<F> { | ||
arthurpaulino marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#[allow(dead_code)] | ||
pub fn new(secret: F, payload: Ptr<F>, store: &mut Store<F>) -> Result<Self> { | ||
let hidden = store.hide(secret, payload); | ||
let mut zstore = Some(ZStore::<F>::default()); | ||
let hidden = store.get_z_expr(&hidden, &mut zstore)?.0; | ||
Ok(Self { | ||
hidden, | ||
zstore: zstore.unwrap(), | ||
}) | ||
} | ||
|
||
#[cfg(not(target_arch = "wasm32"))] | ||
huitseeker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
pub fn persist(&self, hash: &str) -> Result<()> { | ||
use super::{field_data::FieldData, paths::commitment_path}; | ||
use std::{fs::File, io::BufWriter}; | ||
|
||
let fd = &FieldData::wrap::<F, Commitment<F>>(self)?; | ||
bincode::serialize_into(BufWriter::new(&File::create(commitment_path(hash))?), fd)?; | ||
Ok(()) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
use anyhow::Result; | ||
use serde::{Deserialize, Serialize}; | ||
|
||
use lurk::field::{LanguageField, LurkField}; | ||
|
||
/// A wrapper for data whose deserialization depends on a certain LurkField | ||
#[derive(Serialize, Deserialize)] | ||
pub struct FieldData { | ||
pub field: LanguageField, | ||
arthurpaulino marked this conversation as resolved.
Show resolved
Hide resolved
|
||
data: Vec<u8>, | ||
huitseeker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
#[allow(dead_code)] | ||
impl FieldData { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, but with this version you are moving the conversion to/from bytes at the wrapping time. I suspect you could peg what you want on a struct Labeled<T: Serialize + DeserializeOwned> {
label: Language field,
val: T,
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this case I do intend to deserialize in two steps. We first read from FS to know the field and then we read from the vector to get the data with the correct field elements There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. You can definitely have those two steps, but in sequence within the same function, with the above structure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't get the idea. I don't want to deserialize the vector of bytes if there is some inconsistency in the field. I want to error earlier. In other words, the vector is desirable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the record, I did try something like you propose in my first attempt but then I got stuck because Rust doesn't have dependent types. I wanted There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm. As partially unrelated topic, I don't think any strategy that persists important Lurk data using an ad-hoc/bincode serialization of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How can we encode that information, then? Should we create another enum that we try to assure its stability? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you want to have a completely general, 'dynamic' serialization, then it's going to require more design. But is that really what is needed here? I think you just need to know what Therefore, as long as you are looking values up by their hash/digest, then it's fine to completely segregate them by field. So, given that this work is still using a filesystem-based commitment store, you could have a directory structure like:
If the current Also, since (as above), we also cannot have collisions between fields (assuming indexes are always hashes, and the chosen field/hash combinations still preserve our security assumptions) you could even get the 'dynamic' behavior by searching for a given commitment in all available field directories. To prevent having to search in multiple places, you could use symlinks (for example) to provide a single index (perhaps hierarchically structured to avoid too-large directories, etc.) Obviously, with a more powerful database management system than 'the file system', a different approach could be taken. But I think the above (especially the simplest version, which is probably all we need initially) should be fine. The point is: I think you may be trying to solve the wrong problem. Certainly if the goal is a quick PR that decrees a format through code then that is the case. While we may want to eventually have a format that allows dynamically mixing field types, that will need to be worked out as a careful extension of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That works for commitments, but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't proofs also always with respect to a known prover/verifier, which is itself necessarily parameterized on curve+field? Also, why the heck are proofs (apparently) being stored with an id that is the timestamp rather than using content-addressing as previously? Question: Are you trying to support verifying any Lurk proof, or do we assume that a given REPL session will only verify proofs of statements in the current field? Either way, if you content-address the proofs, you should be able to use the symlink approach as above. (NOTE: in that model, you will need to check the actual location of resolved symlinks to get that meta-data — but I still think that's not what should be needed here.) Opinion: Proof IDs should be the content address of the claim — just as they are in A real outsourced-but-provable computation service could do the same. |
||
#[inline] | ||
pub fn wrap<F: LurkField, T: Serialize>(t: &T) -> Result<Self> { | ||
Ok(Self { | ||
field: F::FIELD, | ||
data: bincode::serialize(t)?, | ||
}) | ||
} | ||
|
||
#[inline] | ||
pub fn extract<'a, T: Deserialize<'a>>(&'a self) -> Result<T> { | ||
Ok(bincode::deserialize(&self.data)?) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
mod commitment; | ||
mod field_data; | ||
mod lurk_proof; | ||
mod paths; | ||
mod repl; | ||
|
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.
Does that warning differ qualitatively from any other instance of a store?
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.
Yes, this one contains secrets
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.
That I would expect. The idea that other stores don't is more surprising.
In practical terms, consider the code base you're editing here will be reviewed by a professional cryptography auditor. How does the comment you're offering here help them contextualize how files for
Commitment
should be considered, as opposed to other store files?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.
Will address this tomorrow