Fix BLS12 G2PreparedGadget's AllocVar with a divisive twist

[weikengchen]

Description

When working in Aleo's new Marlin implementation, we need gadgets for SonicPC, in which one needs to prepare a G2 element on BLS12-377.

It appears that the following two allocated variables are different:

• G2PreparedVar, via AllocVar on a G2Prepared
• G2PreparedVar, via prepare_g2 on a G2Var

This seems to be because the AllocVar on G2Prepared is specific to a BLS12 pairing where G2 has a multiplicative twist (i.e., TwistType::M). And the same conversion does not work for a divisive twist.

The case for the multiplicative twist:

• It interprets the ell_coeff as (a, b, c) and allocates (a/c, b/c) for the gadget case.
• This works correctly:
  • in the native world, the ell function is to compute mul_014 over (a, bx, cy)
  • in the constraint world, the ell function is to compute mul_014 over (a/c, b/c x, y)

The case for the divisive twist:

• It interprets the ell_coeff as (a, b, c) and wrongly allocates (a/c, b/c), which indeed should be (b/a, c/a).
• This works incorrectly:
  • in the native world, the ell function is to compute mul_034 over (ay, bx, c)
  • in the constraint world, the ell function now computes mul_034 over (y, a/c x, 1) but it should be (y, b/a x, c/a)

There are multiple ways to fix: let the native follow the constraint, let the constraint follow the native, or let the AllocVar take care of the conversion. This PR does the last one, by making AllocVar implementation of G2PreparedVar aware of such difference.

We did not discover this bug before because we commonly use BLS12-381 (type-M), but not BLS12-377 (type-D).

This is related to https://github.com/AleoHQ/snarkVM/pull/317

---

One question: do we want to keep the tests in this PR? They require additional dev dependencies since ark-test-curves does not have BLS12-377 or G2 of BLS12-381.

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (master)
• Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
• Wrote unit tests
• Updated relevant documentation in the code
• Added a relevant changelog entry to the Pending section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer

[daira]

We did not discover this bug before because we commonly use BLS12-381 (type-M), but not BLS12-371 (type-D).

Do you mean BLS12-377 (type D) here?

[weikengchen]

We did not discover this bug before because we commonly use BLS12-381 (type-M), but not BLS12-371 (type-D).

Do you mean BLS12-377 (type D) here?

Yes. Thanks for catching this.

[Pratyush]

Thanks for the fix! I'm surprised this doesn't result in a cyclic dependency, as ark-bls12-377 itself depends on r1cs-std when the r1cs feature is on. It might be worthwhile to move the test to ark-curve-constraint-tests, so that it's exercised directly for bls12-377 always?

LGTM otherwise.

[weikengchen]

Good idea. Then my plan is as follows: drop the test from this PR and make a new PR in ark-curve-constraint-tests that adds this test.

[Pratyush]

Sounds good!

[weikengchen]

Changes have been made. A separate PR will be created in arkworks-rs/curves