Add a transcript protocol function that checks for identity points.

[hdevalence]

No description provided.

[cathieyun]

This LGTM, but I know @oleganza had some comments about this approach so I don't want to merge it in just yet

[oleganza]

For completeness sake we need to allow zero V because the proof should be sound even with zero blinding factors, and with zero secret value.

[hdevalence]

On a properly-generated Pedersen commitment, the blinding factor will be zero with negligible probability, so a zero blinding factor almost certainly indicates an RNG failure and I don't think there's a good reason to accept them.

It might be the case that some higher-level protocol uses inputs that aren't secret, as you mentioned (private communication), and so someone might choose a zero blinding factor. I'm not sure this is a good design compared to forming the commitment properly and giving the opening (v, v_blinding); if the blinding factor is set to zero, the committer still has to supply v (since there's not an easy way to recover v from vB, even for low-entropy v). So this "optimization" doesn't seem to really simplify the structure, the committer still has to supply the opening, it's just that the opening is smaller.

That said, I'm not totally set on forcing the V commitments to be nonzero, I just don't see a good reason to allow it, and there are (in my opinion) good reasons not to allow it (see next comment).

[hdevalence]

I could see two options for resolving this:

1. Remove the V != 0 check to allow the zero-blinding-factors case, and leave other checks in place;
2. Leave all of the checks in place, and later, if V != 0 turns out to block something, we can relax it in the future (since relaxing a check is easier to do later than adding one).

WDYT?

[oleganza]

Let's do (1) and merge this PR?

[oleganza]

T1 and T2 commit to “leftover” cross-terms and separated from the rest of the statement using non-zero challenge powers. There should not be a valid reason for them to not be allowed to be zero. If there is such reason, it must be specifically explored and called out. Maybe with some choice of prior challenges and zero blinding factors, some cross terms could actually end up being zeroes.

[oleganza]

(I mean, maybe not in this RP protocol, but in R1CS, for instance)

[hdevalence]

There should not be a valid reason for them to not be allowed to be zero. If there is such reason, it must be specifically explored and called out.

I disagree somewhat with this reasoning.

My reasoning for not allowing them to be zero is: allowing prover-supplied points to be the identity point is a risk, because it allows a prover to kill arbitrary terms in the verification equation (by setting those points to the identity, so that their coefficients disappear). It seems to be the case for Bulletproofs that this doesn't cause problems, but there are certainly other protocols where it does. Checking that the prover-supplied points aren't the identity closes off this line of attack upfront, even if, for this protocol, that line may not lead anywhere.

So I view this as a defense-in-depth mechanism. Legitimate proofs won't have zero terms (or will have them with negligible probability), so I don't think that this check costs anything to the protocol, but it does add a hedge against a class of risk to the protocol.

[hdevalence]

As a follow-up, here's a concrete example of the kind of thing I'm worried about: https://twitter.com/claudiorlandi/status/1090705045333659648

The description of the protocol implicitly assumed that some data was not the identity and nobody noticed, leaving a gap. Preemptively checking that points are not the identity (unless there's a reason to allow them) seems to me like a low-cost, sensible precaution.

[oleganza]

I could see two options for resolving this:

1. Remove the V != 0 check to allow the zero-blinding-factors case, and leave other checks in place;
2. Leave all of the checks in place, and later, if V != 0 turns out to block something, we can relax it in the future (since relaxing a check is easier to do later than adding one).

Relaxing the check does not always work compatibly in the scope of a larger system, e.g. in a blockchain protocol, where the set of invalid inputs cannot shrink w/o breaking consensus with older nodes.

I'll try to think out loud about the available options:

Option 1: implicit zero checks with explicit opt-out

If we have implicit check for !=0 for normal points, then we can have an explicit API for "opt out".

For instance, we can consider a separate wrapper type like MaybeIdentity<P: Identity>.

Pro

This might be safer for all sorts of protocols that casually require a generator point, and need to ensure that such point is indeed a generator.

Con

Only one problem with this: for aforementioned blockchain applications, the user has to explicitly decide upfront which points (e.g. Vs) must be "nullable" and decorate them as such. And not to forget about this issue before deployment.

Non-issue

Cross-term commitments are safe to be null, but can only be zero when the proof is completely degenerate (nothing is blinded at all), which is probably not an issue worth worrying about.

Option 2: explicit zero-checks (opt-in)

The point that must be a generator of a group (== "not identity") has a special type RistrettoGenerator. The APIs that need a generator require that type instead of a RistrettoPoint. Conversion method .into_generator() returns an Option<RistrettoGenerator> because it may be an identity.

Pro

Explicit strong subtype of all points. BulletproofsGens, PedersenGens can produce this type by default. Schnorr proof API takes that type making the requirement explicit. Code that does not care remains the same.

Con

Dangerously easy to write a protocol that does not require a Generator subtype. Since in most protocols generators are static, people do not have a habit to check for identity and treat all points as group generators by default.

Option 3: explicit choice

Transcript has explicit choice of methods (naming TBD):

transcript.commit_nullable_point("V", V);
transcript.commit_nonnullable_point("G", G);

Pro

The API forces the user to consider whether the point is supposed to be nullable or not, so either choice is not made lightly. Hopefully, avoiding any compatibility or safety issues down the road. The naming should be adjusted to make "nullable" sound scarier, and "nonnullable" more like a safer choice, but still being explicit.

Con

The transcript API is more awkward than we'd all like. Maybe can be improved with a variant from option 1.

[hdevalence]

Hmm, how about option 4: use validate_and_commit_point for the points which need validation? The problem is to decide which those are; I think the function I made is a good design to actually do it.

[oleganza]

Hmm, how about option 4: use validate_and_commit_point for the points which need validation? The problem is to decide which those are; I think the function I made is a good design to actually do it.

Yeah, this sounds like a nicer variant of option 3.

How about you use commit_point for V, use validate_and_commit_point for all the others? Then I think it'd be totally reasonable.

[oleganza]

Wait, for the rangeproof API it probably doesn't make sense to not validate V. For R1CS it does, though.

[hdevalence]

I think a zero V is weird but OK; I will re-run through all the uses, check whether it could legitimately be zero, and leave comments, then update this PR.

[hdevalence]

Updated this PR with the following changes:

1. Removed the validation check for Vs to allow zero-value, zero-blinding Vs (still not sure that this is useful, but I don't think it's harmful);
2. Updated the transcript calls to use the renamed functions in Merlin 1.1.