Feat: emulated subgroup check

[yelhousni]

Adds the methods:

• AssertIsOnCurve to sw_emulated,
• AssertIsOnG1 and AssertIsOnG2 to sw_bn254 and sw_bl12381 with hints.

This PR needs Consensys/gnark-crypto#376.

TODO:

• use these methods in the EVM precompiles (std/evmprecompiles). This is to confirm with @OlivierBBB, as the arithmetization would probably do the AssertIsOnCurve part.

[OlivierBBB]

The current plan is for the zk-evm arithmetization to perform preliminary checks (is on curve for C1 and C2) but not subgroup checks (for G2) in order to reduce the number of junk calls to this circuit.

[yelhousni]

The current plan is for the zk-evm arithmetization to perform preliminary checks (is on curve for C1 and C2) but not subgroup checks (for G2) in order to reduce the number of junk calls to this circuit.

Yes! this is captured in the last commit 0e0958c.

[yelhousni]

Follows https://github.com/ConsenSys/zkevm-spec/issues/50 and fixes #634

[ivokub]

Suggested edit:

diff --git a/std/algebra/emulated/sw_bn254/hints.go b/std/algebra/emulated/sw_bn254/hints.go
index b1674afd..9f240981 100644
--- a/std/algebra/emulated/sw_bn254/hints.go
+++ b/std/algebra/emulated/sw_bn254/hints.go
@@ -9,7 +9,11 @@ import (
 )
 
 func init() {
-	solver.RegisterHint(subgroupG2Hint)
+	solver.RegisterHint(GetHints()...)
+}
+
+func GetHints() []solver.Hint {
+	return []solver.Hint{subgroupG2Hint}
 }
 
 func subgroupG2Hint(nativeMod *big.Int, nativeInputs, nativeOutputs []*big.Int) error {
diff --git a/std/hints.go b/std/hints.go
index 2d044830..5d54fb10 100644
--- a/std/hints.go
+++ b/std/hints.go
@@ -4,6 +4,8 @@ import (
 	"sync"
 
 	"github.com/consensys/gnark/constraint/solver"
+	"github.com/consensys/gnark/std/algebra/emulated/sw_bls12381"
+	"github.com/consensys/gnark/std/algebra/emulated/sw_bn254"
 	"github.com/consensys/gnark/std/algebra/native/sw_bls12377"
 	"github.com/consensys/gnark/std/algebra/native/sw_bls24315"
 	"github.com/consensys/gnark/std/evmprecompiles"
@@ -37,4 +39,6 @@ func registerHints() {
 	solver.RegisterHint(emulated.GetHints()...)
 	solver.RegisterHint(rangecheck.CountHint, rangecheck.DecomposeHint)
 	solver.RegisterHint(evmprecompiles.GetHints()...)
+	solver.RegisterHint(sw_bn254.GetHints()...)
+	solver.RegisterHint(sw_bls12381.GetHints()...)
 }

[ivokub]

Suggested edit:

diff --git a/std/algebra/emulated/sw_bn254/hints.go b/std/algebra/emulated/sw_bn254/hints.go
index b1674afd..9f240981 100644
--- a/std/algebra/emulated/sw_bn254/hints.go
+++ b/std/algebra/emulated/sw_bn254/hints.go
@@ -9,7 +9,11 @@ import (
 )
 
 func init() {
-	solver.RegisterHint(subgroupG2Hint)
+	solver.RegisterHint(GetHints()...)
+}
+
+func GetHints() []solver.Hint {
+	return []solver.Hint{subgroupG2Hint}
 }
 
 func subgroupG2Hint(nativeMod *big.Int, nativeInputs, nativeOutputs []*big.Int) error {
diff --git a/std/hints.go b/std/hints.go
index 2d044830..5d54fb10 100644
--- a/std/hints.go
+++ b/std/hints.go
@@ -4,6 +4,8 @@ import (
 	"sync"
 
 	"github.com/consensys/gnark/constraint/solver"
+	"github.com/consensys/gnark/std/algebra/emulated/sw_bls12381"
+	"github.com/consensys/gnark/std/algebra/emulated/sw_bn254"
 	"github.com/consensys/gnark/std/algebra/native/sw_bls12377"
 	"github.com/consensys/gnark/std/algebra/native/sw_bls24315"
 	"github.com/consensys/gnark/std/evmprecompiles"
@@ -37,4 +39,6 @@ func registerHints() {
 	solver.RegisterHint(emulated.GetHints()...)
 	solver.RegisterHint(rangecheck.CountHint, rangecheck.DecomposeHint)
 	solver.RegisterHint(evmprecompiles.GetHints()...)
+	solver.RegisterHint(sw_bn254.GetHints()...)
+	solver.RegisterHint(sw_bls12381.GetHints()...)
 }

This is to ensure that if the compiler and prover are in different processes then in the prover process we do std.RegisterHints() once to get all hints defined in std/ packages.

[ivokub]

Suggested edit:

diff --git a/std/algebra/emulated/sw_bls12381/doc_test.go b/std/algebra/emulated/sw_bls12381/doc_test.go
index ae87b729..a1ce0f5c 100644
--- a/std/algebra/emulated/sw_bls12381/doc_test.go
+++ b/std/algebra/emulated/sw_bls12381/doc_test.go
@@ -23,6 +23,10 @@ func (c *PairCircuit) Define(api frontend.API) error {
 	if err != nil {
 		return fmt.Errorf("new pairing: %w", err)
 	}
+	// Pair method does not check that the points are in the proper groups.
+	pairing.AssertIsOnG1(&c.InG1)
+	pairing.AssertIsOnG2(&c.InG2)
+	// Compute the pairing
 	res, err := pairing.Pair([]*sw_bls12381.G1Affine{&c.InG1}, []*sw_bls12381.G2Affine{&c.InG2})
 	if err != nil {
 		return fmt.Errorf("pair: %w", err)
diff --git a/std/algebra/emulated/sw_bn254/doc_test.go b/std/algebra/emulated/sw_bn254/doc_test.go
index db095a02..7d8ef6a6 100644
--- a/std/algebra/emulated/sw_bn254/doc_test.go
+++ b/std/algebra/emulated/sw_bn254/doc_test.go
@@ -23,6 +23,10 @@ func (c *PairCircuit) Define(api frontend.API) error {
 	if err != nil {
 		return fmt.Errorf("new pairing: %w", err)
 	}
+	// Pair method does not check that the points are in the proper groups.
+	pairing.AssertIsOnG1(&c.InG1)
+	pairing.AssertIsOnG2(&c.InG2)
+	// Compute the pairing
 	res, err := pairing.Pair([]*sw_bn254.G1Affine{&c.InG1}, []*sw_bn254.G2Affine{&c.InG2})
 	if err != nil {
 		return fmt.Errorf("pair: %w", err)

[ivokub]

I'm not sure the G1 group checks are sufficient. Imo if the prover is malicious and replaces the hints with identity functions then the in-circuit assertions hold but the points may still be outside the group?

It is also possible I have misunderstood. Otherwise I think is perfect, but I would only wrap AssertIsOnCurve of sw_emulated instead of reimplementing in this package, but I can do it myself.

[yelhousni]

I'm not sure the G1 group checks are sufficient. Imo if the prover is malicious and replaces the hints with identity functions then the in-circuit assertions hold but the points may still be outside the group?

I assumed that the hint is a private function and a gnark user cannot change it, but I see your point.

It is also possible I have misunderstood. Otherwise I think is perfect, but I would only wrap AssertIsOnCurve of sw_emulated instead of reimplementing in this package, but I can do it myself.

Go for the suggested edit and I'll think about the subgroup membership hints.

[ivokub]

I'm not sure the G1 group checks are sufficient. Imo if the prover is malicious and replaces the hints with identity functions then the in-circuit assertions hold but the points may still be outside the group?

I assumed that the hint is a private function and a gnark user cannot change it, but I see your point.

We now even have made it easier with a solver option for overriding hints: https://github.com/ConsenSys/gnark/blob/develop/constraint/solver/options.go#L39-L44

[yelhousni]

@ivokub Ok I did the BN254 G2 membership directly in-circuit (without hints). I optimised quite a bit the (fixed) scalar mul. A full pairing with both G1 and G2 membership:

• first attempt (unoptimized b6cc505): 1578534
• optimized b5caf13: 1498532
• with hint (but insecure): 1384729

Will do BLS12-381 later when you review the structure of the code?

[ivokub]

I think it looks good! Imo considering that the subgroup check is optional the performance of the check is great.

[yelhousni]

I think it looks good! Imo considering that the subgroup check is optional the performance of the check is great.

Did the BLS12-381 too but did not optimize the fixed scalar Mul in G1. We don't use the AssertIsOnG1 anyway in EVM nor in Pair method anyway.

Edit: An addition chain using the public method AddUnified from sw_emulated for both add/double in the chain costs more constraints (3.6M vs 2.7M) than calling ScalarMul with the fixed scalar (which uses private double and add methods).

[ivokub]

Looks good! Only fixed one typo.

Edit: and merge develop. No should be good to merge!