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

feat: new zigzag commitments #864

Closed
wants to merge 72 commits into from

Conversation

@dignifiedquire
Copy link
Member

commented Sep 6, 2019

This PR aims to implement the new zigzag commitment construction as described in (1) and (2).

Roadmap

  • Merge layered_drgporep and zigzag_dgrporep
  • implement naive replication
  • implement more efficient replication
  • implement vanilla proving
  • implement vanilla verification
    • check 1
    • check 2
    • check 3
  • implement compound & circuit setup
  • implement circuit verification
    • check 1
    • check 2
    • check 3
    • generate_inputs
  • resolve open TODOs
  • update PoSt
  • integrate into filecoin-proofs
  • remove expander parents in layer 0
  • Make parent generation spec compliant
  • Decide on Blake2s vs generic hasher for column hashes
  • Implement generic hasher for column hashes (too horrible)
  • cleanup code
  • fix tests
  • switch column hash to pedersen
  • update rust-fil-sector-builder filecoin-project/rust-fil-sector-builder#54
  • update rust-fil-proofs-ffi: filecoin-project/rust-fil-proofs-ffi#22

Fixes #827.

@dignifiedquire dignifiedquire self-assigned this Sep 6, 2019
Copy link
Collaborator

left a comment

I made many comments, but did not have time to get through everything. Some of my comments may be inconsistent, but they should be mostly on point. Let's sync to discuss because I think that will be easier than trying to tease out the exact steps forward needed in this way.

filecoin-proofs/src/parameters.rs Show resolved Hide resolved
storage-proofs/src/challenge_derivation.rs Outdated Show resolved Hide resolved
storage-proofs/src/zigzag_graph.rs Outdated Show resolved Hide resolved
self.size() as u32 - 1
} else {
0
},

This comment has been minimized.

Copy link
@porcuquine

porcuquine Sep 9, 2019

Collaborator

Please at least add a comment explaining this calculation. (The padding node is first/last depending on direction.)

storage-proofs/src/zigzag_drgporep.rs Outdated Show resolved Hide resolved
storage-proofs/src/zigzag_drgporep.rs Outdated Show resolved Hide resolved
storage-proofs/src/zigzag_drgporep.rs Outdated Show resolved Hide resolved
storage-proofs/src/zigzag_drgporep.rs Outdated Show resolved Hide resolved
storage-proofs/src/zigzag_drgporep.rs Outdated Show resolved Hide resolved
storage-proofs/src/zigzag_drgporep.rs Outdated Show resolved Hide resolved
@porcuquine

This comment has been minimized.

Copy link
Collaborator

commented Sep 9, 2019

NOTE: Some of my comments above may be mismatched to how you organized this — so if that seems to be the case let's just discuss. I think we can work through this pretty quickly that way.

@dignifiedquire dignifiedquire force-pushed the feat/zigzag-comm branch from 3bf3665 to 391e3f6 Sep 12, 2019
return false;
}

self.verify()

This comment has been minimized.

Copy link
@porcuquine

porcuquine Sep 12, 2019

Collaborator

Should this call self.validate() instead? As written, this skips the node/path check. (Maybe that's okay, not clear about global usage — but even if okay, probably safer for all possible uses to include that check.)

MerkleProof::new_from_proof(&t_aux.tree_d.gen_proof(challenge));

// ZigZag replica column openings
let rpc = {

This comment has been minimized.

Copy link
@porcuquine

porcuquine Sep 12, 2019

Collaborator

Can we please use a different variable name? This one has too many associates to not be distracting/confusing.

.full_column(&graph_0, challenge)?
.into_proof_all(&t_aux.tree_c);

// Only odd-layer labels in the renumbered column C_\bar{X}

This comment has been minimized.

Copy link
@porcuquine

porcuquine Sep 12, 2019

Collaborator

Comment is out of date.

@dignifiedquire dignifiedquire force-pushed the feat/zigzag-comm branch 2 times, most recently from 4e7a440 to 52aba1c Sep 16, 2019
dignifiedquire added a commit to filecoin-project/rust-fil-sector-builder that referenced this pull request Sep 17, 2019
- remove comm_r_star
- persistent p_aux as private input to post generation

Ref filecoin-project/rust-fil-proofs#864
@dignifiedquire dignifiedquire force-pushed the feat/zigzag-comm branch from 1f67198 to 4212d48 Sep 19, 2019
dignifiedquire added a commit to filecoin-project/rust-fil-sector-builder that referenced this pull request Sep 19, 2019
- remove comm_r_star
- persistent p_aux as private input to post generation

Ref filecoin-project/rust-fil-proofs#864
…xpaned-parents' into feat/zigzag-comm
let cs = os
.par_iter()
.zip(es.par_iter())
.flat_map(|(o_i, e_i)| hash2(o_i, e_i).as_ref().to_vec())

This comment has been minimized.

Copy link
@schomatis

schomatis Sep 23, 2019

Contributor

Probably not following the new ZigZag paper correctly, but isn't the even column index inverted for C_i?

@dignifiedquire

This comment has been minimized.

Copy link
Member Author

commented Oct 6, 2019

we will not be using zigzag anymore, closing in favor of #895

dignifiedquire added a commit to filecoin-project/rust-fil-sector-builder that referenced this pull request Oct 7, 2019
- remove comm_r_star
- persistent p_aux as private input to post generation

Ref filecoin-project/rust-fil-proofs#864
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.