Synthetic PoRep

[cryptonemo]

No description provided.

[Kubuxu]

IDK in what code paths it is used, but I could see it being useful as a step before running the prover.
In this, the randomness should be available to verify just the 180 selected challenges.

[DrPeterVanNostrand]

In production this code path should not be hit; it's here just in case the prover runs the verification method prior to randomness being set. The synthetic prover verifies all synthetic proofs prior to writing them to disk, so the selected 180 proofs should always be valid prior to this method's invocation. I could add proof re-verification in prove_layers when the selected synthetic proofs are read from disk?

[vmx]

I've only reviewed some of the synthetic proofs serialization as I was interested in it for other reasons.

[vmx]

I found this code a bit hard to follow. Here's my version:

        if SubTreeArity::to_usize() == 0 {
            let base_path = path.into_iter().map(PathElement::from).collect::<Vec<_>>();
            MerkleProof {
                data: ProofData::Single(SingleProof {
                    leaf,
                    root,
                    path: base_path.into(),
                }),
            }
        } else if TopTreeArity::to_usize() == 0 {
            let sub_elem = path.pop().expect("path should not be empty");
            let base_path = path.into_iter().map(PathElement::from).collect::<Vec<_>>();
            MerkleProof {
                data: ProofData::Sub(SubProof {
                    leaf,
                    root,
                    base_proof: base_path.into(),
                    sub_proof: vec![PathElement::from(sub_elem)].into(),
                }),
            }
        } else {
            let sub_elem = path.pop().expect("path should not be empty");
            let top_elem = path.pop().expect("path should not be empty");
            let base_path = path.into_iter().map(PathElement::from).collect::<Vec<_>>();
            MerkleProof {
                data: ProofData::Top(TopProof {
                    leaf,
                    root,
                    base_proof: base_path.into(),
                    sub_proof: vec![PathElement::from(sub_elem)].into(),
                    top_proof: vec![PathElement::from(top_elem)].into(),
                }),
            }
        }
    }

It contains more repitition than yours , but I find it easier to follow. @DrPeterVanNostrand What do thou think?

[DrPeterVanNostrand]

Sure, this can be changed.

[vmx]

Casting a boolean into a usize is pretty C-like. I'd prefer having it in a more type-safe way. What about:

let layer_parents = if layer_index == 0 {
    num_drg_parents
} else {
    num_drg_parents + num_exp_parents
}

[DrPeterVanNostrand]

Yup, sounds good.

[vmx]

I would call it root_r_last for extra clarity (also in the code).

[jennijuju]

any reason to wait on landing the PR, as the test has ✅ ?

[cryptonemo]

any reason to wait on landing the PR, as the test has ✅ ?

We'll have it merged sooner than later, but as it is now, all of synthetic-porep is in feature branches across all of the repos. We didn't plan to cut a release until the SN work was complete (it's complete, but we're waiting on a release; @vmx has a meeting on that today and may have a solid update there). After that, we're going to start thinking about our next release scheduling

[jennijuju]

any reason to wait on landing the PR, as the test has ✅ ?

We'll have it merged sooner than later, but as it is now, all of synthetic-porep is in feature branches across all of the repos. We didn't plan to cut a release until the SN work was complete (it's complete, but we're waiting on a release; @vmx has a meeting on that today and may have a solid update there). After that, we're going to start thinking about our next release scheduling

Thanks! why don’t we decouple the releases for synthetic PoRep and SN changes? From lotus perspective, they will be introduced in two separate releases, one in a mandatory consensus breaking release, the other in an optional feature release. Would be great if we can minimize the changes in proof libraries for the mandatory release (or external dependencies in general!). (@rjan90 could you please follow up here after teams sync with SN and align with proof team on code landing/freeze expectations?)

[vmx]

I haven't had a fully in-depth review, but @DrPeterVanNostrand and @cryptonemo worked on it together, no open questions are left => approval.