Audit fixes 1

go/acir/black_box_func/and.go

@@ -38,7 +38,7 @@ func (a *And[T, E]) UnmarshalReader(r io.Reader) error {
 
 func (a *And[T, E]) Equals(other BlackBoxFunction[E]) bool {
 	value, ok := other.(*And[T, E])
-	return ok && a.Lhs.Equals(&value.Lhs) && a.Rhs.Equals(&value.Rhs) && a.Output.Equals(&value.Output)
+	return ok && a.Lhs.Equals(&value.Lhs) && a.Rhs.Equals(&value.Rhs) && a.Output.Equals(&value.Output) && a.nBits == value.nBits
 }
 
 func (a *And[T, E]) Define(api frontend.Builder[E], witnesses map[shr.Witness]frontend.Variable) error {
@@ -77,10 +77,10 @@ func (a *And[T, E]) FillWitnessTree(tree *btree.BTree, index uint32) bool {
 		return false
 	}
 
-	if a.Lhs.FunctionInputKind == 1 {
+	if a.Lhs.IsWitness() {
 		tree.ReplaceOrInsert(*a.Lhs.Witness + shr.Witness(index))
 	}
-	if a.Rhs.FunctionInputKind == 1 {
+	if a.Rhs.IsWitness() {
 		tree.ReplaceOrInsert(*a.Rhs.Witness + shr.Witness(index))
 	}
 	tree.ReplaceOrInsert(a.Output + shr.Witness(index))

go/acir/black_box_func/and_test.go

@@ -39,6 +39,7 @@ func TestAndUnmarshalReader(t *testing.T) {
 				Witness:           &expectedWitnessRhs,
 			},
 			Output: shr.Witness(3456),
+			nBits:  5678,
 		},
 	}
 

go/acir/black_box_func/ecdsa_secp256k1.go

@@ -3,6 +3,7 @@ package blackboxfunc
 import (
 	"encoding/binary"
 	"io"
+	"math/big"
 	shr "sunspot/go/acir/shared"
 
 	"github.com/consensys/gnark/constraint"
@@ -23,7 +24,7 @@ type ECDSASECP256K1[T shr.ACIRField, E constraint.Element] struct {
 	Output        shr.Witness
 }
 
-func (a *ECDSASECP256K1[T, Equals]) UnmarshalReader(r io.Reader) error {
+func (a *ECDSASECP256K1[T, E]) UnmarshalReader(r io.Reader) error {
 	for i := 0; i < 32; i++ {
 		if err := a.PublicKeyX[i].UnmarshalReader(r); err != nil {
 			return err
@@ -59,7 +60,10 @@ func (a *ECDSASECP256K1[T, Equals]) UnmarshalReader(r io.Reader) error {
 }
 
 func (a *ECDSASECP256K1[T, E]) Equals(other BlackBoxFunction[E]) bool {
-	value := other.(*ECDSASECP256K1[T, E])
+	value, ok := other.(*ECDSASECP256K1[T, E])
+	if !ok {
+		return false
+	}
 	if len(a.PublicKeyX) != len(value.PublicKeyX) ||
 		len(a.PublicKeyY) != len(value.PublicKeyY) ||
 		len(a.Signature) != len(value.Signature) ||
@@ -134,7 +138,18 @@ func (a *ECDSASECP256K1[T, E]) Define(api frontend.Builder[E], witnesses map[shr
 	if err != nil {
 		return err
 	}
-	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(witnesses[a.Output], Q.IsValid(api, sw_emulated.GetSecp256k1Params(), msg, &sig))))
+
+	validSig := Q.IsValid(api, sw_emulated.GetSecp256k1Params(), msg, &sig)
+
+	// Noir's verify_signature rejects signatures with s > n/2 (low-s form) to
+	// prevent malleability; gnark's IsValid does not, so enforce it here.
+	halfOrder := new(big.Int).Rsh(emulated.Secp256k1Fr{}.Modulus(), 1)
+	sBits := scalarField.ToBits(&sig.S)
+	isLowS := isLessOrEqualConstant(api, sBits, halfOrder)
+
+	result := api.Mul(validSig, isLowS)
+
+	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(witnesses[a.Output], result)))
 	return nil
 }
 
@@ -172,7 +187,6 @@ func (a *ECDSASECP256K1[T, E]) FillWitnessTree(tree *btree.BTree, index uint32)
 // ACIR has signature variables as big endian bytes
 // but gnark wants them as 4 * 64 but limbs.
 // See https://pkg.go.dev/github.com/consensys/gnark/std/math/emulated@v0.14.0#hdr-Element_representation
-
 func BytesTo64BitLimbs[T shr.ACIRField](
 	api frontend.API,
 	vars []FunctionInput[T],

go/acir/black_box_func/ecdsa_secp256r1.go

@@ -3,6 +3,7 @@ package blackboxfunc
 import (
 	"encoding/binary"
 	"io"
+	"math/big"
 	shr "sunspot/go/acir/shared"
 
 	"github.com/google/btree"
@@ -134,7 +135,16 @@ func (a *ECDSASECP256R1[T, E]) Define(api frontend.Builder[E], witnesses map[shr
 	if err != nil {
 		return err
 	}
-	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(witnesses[a.Output], Q.IsValid(api, sw_emulated.GetP256Params(), msg, &sig))))
+
+	// Noir's verify_signature rejects signatures with s > n/2 (low-s form) to
+	// prevent malleability; gnark's IsValid does not, so enforce it here.
+	validSig := Q.IsValid(api, sw_emulated.GetP256Params(), msg, &sig)
+	halfOrder := new(big.Int).Rsh(emulated.P256Fr{}.Modulus(), 1)
+	sBits := scalarField.ToBits(&sig.S)
+	isLowS := isLessOrEqualConstant(api, sBits, halfOrder)
+	result := api.Mul(validSig, isLowS)
+
+	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(witnesses[a.Output], result)))
 	return nil
 }
 

go/acir/black_box_func/embedded_curve_add.go

@@ -5,8 +5,6 @@ import (
 	"io"
 	shr "sunspot/go/acir/shared"
 
-	grumpkin "sunspot/go/sw-grumpkin"
-
 	"github.com/consensys/gnark/constraint"
 	"github.com/consensys/gnark/frontend"
 	"github.com/google/btree"
@@ -65,44 +63,27 @@ func (a *EmbeddedCurveAdd[T, E]) Equals(other BlackBoxFunction[E]) bool {
 }
 
 func (a *EmbeddedCurveAdd[T, E]) Define(api frontend.Builder[E], witnesses map[shr.Witness]frontend.Variable) error {
-	// Initialise points and pairs
-	point1X, err := a.Input1[0].ToVariable(witnesses)
+	pred, err := a.predicate.ToVariable(witnesses)
 	if err != nil {
 		return err
 	}
 
-	point1Y, err := a.Input1[1].ToVariable(witnesses)
-	if err != nil {
-		return err
-	}
-	point2X, err := a.Input2[0].ToVariable(witnesses)
+	x, err := EmbeddedPointFromInputs(api, witnesses, pred, a.Input1)
 	if err != nil {
 		return err
 	}
-	point2Y, err := a.Input2[1].ToVariable(witnesses)
+	y, err := EmbeddedPointFromInputs(api, witnesses, pred, a.Input2)
 	if err != nil {
 		return err
 	}
 
-	x := grumpkin.G1Affine{
-		X: point1X,
-		Y: point1Y,
-	}
-
-	y := grumpkin.G1Affine{
-		X: point2X,
-		Y: point2Y,
-	}
+	output := maskedEmbeddedPoint(api, pred,
+		witnesses[a.Outputs[0]], witnesses[a.Outputs[1]], witnesses[a.Outputs[2]])
 
-	// Assert that the addition is correct
-	pred, err := a.predicate.ToVariable(witnesses)
-	if err != nil {
-		return err
-	}
 	constrained_output := x.AddUnified(api, y)
 	// Assert that the addition is correct, ignoring if the predicate is zero
-	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(constrained_output.X, witnesses[a.Outputs[0]])))
-	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(constrained_output.Y, witnesses[a.Outputs[1]])))
+	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(constrained_output.X, output.X)))
+	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(constrained_output.Y, output.Y)))
 
 	return nil
 }

go/acir/black_box_func/function_input_test.go

@@ -1,6 +1,7 @@
 package blackboxfunc
 
 import (
+	"math/big"
 	"os"
 	shr "sunspot/go/acir/shared"
 	"sunspot/go/bn254"
@@ -18,7 +19,7 @@ func TestFunctionInputUnmarshalReaderConstant(t *testing.T) {
 		t.Fatalf("Failed to unmarshal FunctionInput: %v", err)
 	}
 
-	expectedField := bn254.Zero()
+	expectedField := &bn254.BN254Field{Value: *big.NewInt(1234)}
 	expected := FunctionInput[*bn254.BN254Field]{
 		FunctionInputKind: ACIRFunctionInputKindConstant,
 		ConstantInput:     &expectedField,

go/acir/black_box_func/multi_scalar_mul.go

@@ -89,44 +89,33 @@ func (a *MultiScalarMul[T, E]) Define(api frontend.Builder[E], witnesses map[shr
 
 	scalars := make([]interface{}, len(a.Scalars)/2)
 
-	for i := 0; i < len(a.Points); i += 3 {
-		pointX, err := a.Points[i].ToVariable(witnesses)
-		if err != nil {
-			return err
-		}
+	pred, err := a.predicate.ToVariable(witnesses)
+	if err != nil {
+		return err
+	}
 
-		pointY, err := a.Points[i+1].ToVariable(witnesses)
+	for i := 0; i < len(a.Points); i += 3 {
+		point, err := EmbeddedPointFromInputs(api, witnesses, pred,
+			[3]FunctionInput[T]{a.Points[i], a.Points[i+1], a.Points[i+2]})
 		if err != nil {
 			return err
 		}
-
-		point := grumpkin.G1Affine{
-			X: pointX,
-			Y: pointY,
-		}
 		points[i/3] = &point
 	}
 
 	for i := 0; i < len(a.Scalars); i += 2 {
-		scalar, err := a.Scalars[i].ToVariable(witnesses)
+		scalar, err := ScalarFromLimbs(api, witnesses, a.Scalars[i], a.Scalars[i+1])
 		if err != nil {
 			return err
 		}
 		scalars[i/2] = scalar
 	}
 
-	output := grumpkin.G1Affine{
-		X: witnesses[a.Outputs[0]],
-		Y: witnesses[a.Outputs[1]],
-	}
+	output := maskedEmbeddedPoint(api, pred,
+		witnesses[a.Outputs[0]], witnesses[a.Outputs[1]], witnesses[a.Outputs[2]])
 
 	constrained_output := grumpkin.MultiScalarMul(api, points, scalars)
 
-	pred, err := a.predicate.ToVariable(witnesses)
-	if err != nil {
-		return err
-	}
-
 	// To assert the two points are the same (and ignore if predicate is zero), we have to split into
 	// its X and Y coordinates
 	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, api.Sub(constrained_output.X, output.X)))

go/acir/black_box_func/range.go

@@ -29,26 +29,17 @@ func (a *Range[T, E]) UnmarshalReader(r io.Reader) error {
 
 func (a Range[T, E]) Equals(other BlackBoxFunction[E]) bool {
 	value, ok := other.(*Range[T, E])
-	return ok && a.Input.Equals(&value.Input)
+	return ok && a.Input.Equals(&value.Input) && a.nBits == value.nBits
 }
 
 func (a Range[T, E]) Define(api frontend.Builder[E], witnesses map[shr.Witness]frontend.Variable) error {
-	if a.Input.FunctionInputKind == ACIRFunctionInputKindConstant {
-		return nil
-	}
-
-	witness := a.Input.Witness
-	if witness == nil {
-		return fmt.Errorf("witness is nil for Range function input")
-	}
-
-	w, ok := witnesses[*witness]
-	if !ok {
-		return fmt.Errorf("witness %v not found in witnesses map", *witness)
+	input, err := a.Input.ToVariable(witnesses)
+	if err != nil {
+		return fmt.Errorf("failed to resolve Range function input: %w", err)
 	}
 
 	rangechecker := rangecheck.New(api)
-	rangechecker.Check(w, int(a.nBits))
+	rangechecker.Check(input, int(a.nBits))
 	return nil
 }
 

go/acir/black_box_func/shared_ecdsa.go

@@ -0,0 +1,28 @@
+package blackboxfunc
+
+import (
+	"math/big"
+
+	"github.com/consensys/gnark/frontend"
+)
+
+// isLessOrEqualConstant returns 1 if the unsigned integer encoded by the
+// little-endian bits is ≤ c, otherwise 0. bitsLE must be boolean-constrained
+// and c must fit in len(bitsLE) bits.
+func isLessOrEqualConstant(api frontend.API, bitsLE []frontend.Variable, c *big.Int) frontend.Variable {
+	var isLess frontend.Variable = 0
+	var isEqual frontend.Variable = 1
+	for i := len(bitsLE) - 1; i >= 0; i-- {
+		sBit := bitsLE[i]
+		if c.Bit(i) == 1 {
+			// isEqual*(1-sBit) = isEqual - isEqual*sBit; reuse the product to
+			// keep this branch at a single multiplication constraint.
+			newIsEqual := api.Mul(isEqual, sBit)
+			isLess = api.Add(isLess, api.Sub(isEqual, newIsEqual))
+			isEqual = newIsEqual
+		} else {
+			isEqual = api.Mul(isEqual, api.Sub(1, sBit))
+		}
+	}
+	return api.Add(isLess, isEqual)
+}

go/acir/black_box_func/shared_embedded_curve.go

@@ -0,0 +1,74 @@
+package blackboxfunc
+
+import (
+	"math/big"
+	shr "sunspot/go/acir/shared"
+	grumpkin "sunspot/go/sw-grumpkin"
+
+	"github.com/consensys/gnark-crypto/ecc"
+	"github.com/consensys/gnark/constraint"
+	"github.com/consensys/gnark/frontend"
+)
+
+var twoTo128 = new(big.Int).Lsh(big.NewInt(1), 128)
+
+// grumpkinScalarModulusHighLimb is floor(p / 2^128) where p is the grumpkin
+// scalar modulus — the upper bound for a scalar's high 128-bit limb.
+var grumpkinScalarModulusHighLimb = new(big.Int).Rsh(ecc.GRUMPKIN.ScalarField(), 128)
+
+// ScalarFromLimbs recomposes a scalar from its (lo, hi) 128-bit limb
+// FunctionInputs into lo + hi * 2^128.
+func ScalarFromLimbs[T shr.ACIRField, E constraint.Element](
+	api frontend.Builder[E],
+	witnesses map[shr.Witness]frontend.Variable,
+	lo, hi FunctionInput[T],
+) (frontend.Variable, error) {
+	scalarLo, err := lo.ToVariable(witnesses)
+	if err != nil {
+		return nil, err
+	}
+	scalarHi, err := hi.ToVariable(witnesses)
+	if err != nil {
+		return nil, err
+	}
+	api.AssertIsLessOrEqual(scalarHi, grumpkinScalarModulusHighLimb)
+	return api.Add(scalarLo, api.Mul(scalarHi, twoTo128)), nil
+}
+
+// maskedEmbeddedPoint constrains isInf to a boolean (gated by pred) and returns
+// a grumpkin point whose coordinates are masked to (0, 0) when isInf is set.
+func maskedEmbeddedPoint[E constraint.Element](
+	api frontend.Builder[E],
+	pred, x, y, isInf frontend.Variable,
+) grumpkin.G1Affine {
+	api.AssertIsEqual(frontend.Variable(0), api.Mul(pred, isInf, api.Sub(frontend.Variable(1), isInf)))
+	notInf := api.Sub(frontend.Variable(1), isInf)
+	return grumpkin.G1Affine{
+		X: api.Mul(notInf, x),
+		Y: api.Mul(notInf, y),
+	}
+}
+
+// EmbeddedPointFromInputs resolves an (x, y, is_infinite) triple of
+// FunctionInputs into a grumpkin point whose coordinates are masked to (0, 0)
+// when is_infinite is set.
+func EmbeddedPointFromInputs[T shr.ACIRField, E constraint.Element](
+	api frontend.Builder[E],
+	witnesses map[shr.Witness]frontend.Variable,
+	pred frontend.Variable,
+	in [3]FunctionInput[T],
+) (grumpkin.G1Affine, error) {
+	x, err := in[0].ToVariable(witnesses)
+	if err != nil {
+		return grumpkin.G1Affine{}, err
+	}
+	y, err := in[1].ToVariable(witnesses)
+	if err != nil {
+		return grumpkin.G1Affine{}, err
+	}
+	isInf, err := in[2].ToVariable(witnesses)
+	if err != nil {
+		return grumpkin.G1Affine{}, err
+	}
+	return maskedEmbeddedPoint(api, pred, x, y, isInf), nil
+}

go/acir/black_box_func/xor.go

@@ -39,7 +39,7 @@ func (a *Xor[T, E]) UnmarshalReader(r io.Reader) error {
 func (a *Xor[T, E]) Equals(other BlackBoxFunction[E]) bool {
 	value, ok := other.(*Xor[T, E])
 
-	if !ok || !a.Lhs.Equals(&value.Lhs) || !a.Rhs.Equals(&value.Rhs) {
+	if !ok || !a.Lhs.Equals(&value.Lhs) || !a.Rhs.Equals(&value.Rhs) || a.nBits != value.nBits {
 		return false
 	}
 	return a.Output == value.Output