perf: emulated equality assertion

[ivokub]

Description

This PR changes the implementation of emulated AssertIsEqual check.

Previously, the approach for checking modular equality of two elements a and b was to compute the difference diff=b-a in constraints, then find quotient k such that diff = k*p. However, we couldn't just check the equality directly as the multiplication result in k*p may have different overflows on the limbs than for diff.

To overcome that, we computed the difference of diff and k*p limb lengths, and then performed limb-by-limb carrying of the excess bits so that we would be then able to directly assert limb equality.

In any case, it is quite wasteful as we had to compute the binary decomposition for every limb (as we cannot easily carry otherwise currently). Fortunately, in many cases this wasn't a big performance impact as we would usually call AssertIsEqual quite infrequently in circuit.

However, in EC group arithmetic we are relying quite a lot on out-circuit computation (tower divisions, tower inverses etc.), so it start to make an impact.

Now, this PR is a continuation of the previous modular multiplication optimization #749. That PR uses polynomial representation of the non-native elements (by taking the limbs as coefficients of the univariate polynomial) to assert a(X) b(X) = r(X) + k(X) p(X) + (2^b X-X) c(X) on some random X=tau (and where c(X) is a polynomial representing the carries). We notice that for equality assertion it would be sufficient if we show (b(X) - a(X)) * 1(X) = 0(X) + k(X) * p(X) + (2^b X-X) c(X) for some random X=tau. We further notice that actually 1(X) == 1 and 0(X) == 0, so we can fully skip evaluating those polynomials. We're left with needing to evaluate only b(X) - a(X) == diff(X) and k(X). But we also can show that k is bounded (as degree of diff and p is fixed), allowing us to perform less range checking (usually only for a single limb).

With this new approach, we could remove a lot of boilerplate for previous equality assertion (right shifting, compacting of limbs, carrying over the excess etc.)

Additionally, this PR implements an optimization for IsZero -- previously we ran IsZero for every limb and then multiplied the per-limb result. However, when we consider that limbs are small and the sum doesn't overflow the native field, we could only add all the limbs and then compute only a single IsZero.

NB! This PR also introduces the deferred mul-check issue - we do not get anymore exact traces of the locations where the equality assertion failed. For multiplications only this was acceptable as the only cases where the check should fail was when we were mutating the emulated elements, but it definitely makes debugging more difficult.

As such, I'm not sure if we should wait a bit until we can figure out a bit user-friendly approach for debugging. I'm thinking about manually collecting circuit stack trace at mulcheck record moment (within the mulcheck instance) and then rewinding at defer time. Dunno however how feasible this is.

NB! This is a breaking change -- I also removed the AssertLimbsEquality method. There were no uses outside of emulated package and I think it could be used incorrectly.

Additional performance ideas: We can also save computation in deferred multiplication check. We cache the evaluations of the polynomials inferred from the emulated elements' limbs so that when during later mulcheck we encounter already evaluated value, we can reuse instead of evaluating again. But, this would also work for cases when the new emulated elements are sums/differences of existing emulated elements.

This would however require building dependency tree such that to minimize the number of computations and I'm not really sure it would be worth it. When considering the added benefit that maybe in that case we can relax the immutability of emulated elements, then we should do it. It is not urgent though currently, can postpone into new issue/PR

Type of change

• Performance optimization
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How has this been tested?

Does not break existing tests.

How has this been benchmarked?

Performance impact (compared to master on BN254).

test	master SCS	this pr SCS	master R1CS	this PR R1CS
secp256k1 ECDSA	682599	656397	174047	169264
PLONK verifier BW6	50750544	48119556	13336783	13063332
Groth16 verifier BW6	20246012	20020890	5276307	5251865
pairing BW6	14331870	14126844	3749549	3725569
pairing BN254	4270031	4158092	1080492	1060336

Checklist:

• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• I did not modify files generated from templates
• golangci-lint does not output errors locally
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[gbotrel]

overall the PR looks OK, but I really need to spend the time to re-review the emulated package as a whole -- I can't evaluate how this PR impacts the rest of the emulated package (not talking about the impl, just the algorithmic part).

[ivokub]

overall the PR looks OK, but I really need to spend the time to re-review the emulated package as a whole -- I can't evaluate how this PR impacts the rest of the emulated package (not talking about the impl, just the algorithmic part).

Completely makes sense. I think the emulation package in general needs another set of fresh eyes to look at. Imo it has become quite complicated.

Should I wait for additional review before merging this PR?