feat: handle invalid signature failures in ECRecover precompile

[ivokub]

Description

In zkEVM, the signature values for the ECRecover precompile can be invalid in some cases. The failing cases are:

• r^3 + ax+b is not a quadratic residue, preventing computing the y-coordinate of commitment R
• the resulting public key is 0

We need to be able to check such cases to ensure that we can prove reverts. This PR adds a path to also check invalid inputs when isFailure input is given.

Type of change

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Breaking change due to changing the signature of ECRecover function.

How has this been tested?

• TestECRecoverQNR - tests that check for QNR failure succeeds
• TestECRecoverQNRWoFailure - tests that QNR input cannot be provided when isFailure argument is false
• TestECRecoverInfinity - tests that check for zero public key succeeds
• TestECRecoverInfinityWoFailure - tests that zero PK input cannot be provided when isFailure is false
• TestInvalidFailureTag - tests that valid signature cannot be proven when isFailure is true

How has this been benchmarked?

ECRecover master 459082, now 465156

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

[ivokub]

Implemented for now, but the increase in the number of constraints is huge. I recon this is due to needing to use WithCompleteArithmetic when computing the public key in-circuit. I'm trying to figure out maybe we can still use incomplete arithmetic (by adding-subtracting G for example) for scalar multiplication.

[ivokub]

Implemented for now, but the increase in the number of constraints is huge. I recon this is due to needing to use WithCompleteArithmetic when computing the public key in-circuit. I'm trying to figure out maybe we can still use incomplete arithmetic (by adding-subtracting G for example) for scalar multiplication.

This approach works. Now can avoid complete arithmetic in JointScalarMul, but I still need to do one complete addition. It is now 477275 constraints (vs master 459082)

[ivokub]

Optimization pointed out by @yelhousni - instead of using AddUnified we can just select between G and P. Saves 10k constraints. Now it is 466703 constraints.

[ivokub]

And, before we can merge this one, we need to merge Consensys/gnark-crypto#497 and update go.mod here to point to gnark-crypto master.

[yelhousni]

great work 🔥