Merge pull request #512 from Consensys/fix/sis-issue-when-logTwoBound…

…-larger-than-8 fix: sis limb-decomposition works with log-two_bound > 8
Consensys · Jun 24, 2024 · 1ef1276 · 1ef1276
2 parents e557ad0 + 027312b
commit 1ef1276
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 51 deletions.
diff --git a/ecc/bls12-377/fr/sis/sis.go b/ecc/bls12-377/fr/sis/sis.go
@@ -337,7 +337,7 @@ func LimbDecomposeBytes(buf []byte, m fr.Vector, logTwoBound int) {
 // big-endian form into an array of limbs representing the same field elements
 // in little-endian form. Namely, if our field is represented with 64 bits and we
 // have the following field element 0x0123456789abcdef (0 being the most significant
-// character and and f being the least significant one) and our log norm bound is
+// character and and f being the least significant one) and our norm bound is
 // 16 (so 1 hex character = 1 limb). The function assigns the values of m to [f, e,
 // d, c, b, a, ..., 3, 2, 1, 0]. m should be preallocated and zeroized. mValues is
 // an optional bitSet. If provided, it must be empty. The function will set bit "i"
@@ -374,7 +374,7 @@ func limbDecomposeBytes(buf []byte, m fr.Vector, logTwoBound, degree int, mValue
 			// and set the bits from LSB to MSB.
 			at := fieldStart + fr.Bytes*8 - bitInField - 1
 
-			m[mPos][0] |= uint64(bitAt(at) << j)
+			m[mPos][0] |= uint64(bitAt(at)) << j
 			bitInField++
 
 			// Check if mPos is zero and mark as non-zero in the bitset if not

diff --git a/ecc/bls12-377/fr/sis/sis_test.go b/ecc/bls12-377/fr/sis/sis_test.go
@@ -172,62 +172,97 @@ func TestLimbDecomposition(t *testing.T) {
 		t.Skip("skipping this test in 32bit.")
 	}
 
-	sis, _ := NewRSis(0, 4, 4, 3)
-
-	testcases := []fr.Vector{
-		{fr.One()},
-		{fr.NewElement(2)},
-		{fr.NewElement(1 << 32), fr.NewElement(2), fr.NewElement(1)},
+	testcases := []struct {
+		logTwoDegree, logTwoBound int
+		vec                       fr.Vector
+	}{
+		{
+			logTwoDegree: 4,
+			logTwoBound:  4,
+			vec:          fr.Vector{fr.One()},
+		},
+		{
+			logTwoDegree: 4,
+			logTwoBound:  4,
+			vec:          fr.Vector{fr.NewElement(2)},
+		},
+		{
+			logTwoDegree: 4,
+			logTwoBound:  4,
+			vec:          fr.Vector{fr.NewElement(1 << 32), fr.NewElement(2), fr.NewElement(1)},
+		},
+		{
+			logTwoDegree: 4,
+			logTwoBound:  16,
+			vec:          fr.Vector{fr.One()},
+		},
+		{
+			logTwoDegree: 4,
+			logTwoBound:  16,
+			vec:          fr.Vector{fr.NewElement(2)},
+		},
+		{
+			logTwoDegree: 4,
+			logTwoBound:  16,
+			vec:          fr.Vector{fr.NewElement(1 << 32), fr.NewElement(2), fr.NewElement(1)},
+		},
 	}
 
-	for _, testcase := range testcases {
+	for i, testcase := range testcases {
 
-		// clean the sis hasher
-		sis.bufMValues.ClearAll()
-		for i := 0; i < len(sis.bufM); i++ {
-			sis.bufM[i].SetZero()
-		}
-		for i := 0; i < len(sis.bufRes); i++ {
-			sis.bufRes[i].SetZero()
-		}
+		t.Run(fmt.Sprintf("testcase-%v", i), func(t *testing.T) {
 
-		buf := bytes.Buffer{}
-		for _, x := range testcase {
-			xBytes := x.Bytes()
-			buf.Write(xBytes[:])
-		}
-		limbDecomposeBytes(buf.Bytes(), sis.bufM, sis.LogTwoBound, sis.Degree, sis.bufMValues)
-
-		// Just to test, this does not return panic
-		dummyBuffer := make(fr.Vector, 192)
-		LimbDecomposeBytes(buf.Bytes(), dummyBuffer, sis.LogTwoBound)
-
-		// b is a field element representing the max norm bound
-		// used for limb splitting the input field elements.
-		b := fr.NewElement(1 << sis.LogTwoBound)
-		numLimbsPerField := fr.Bytes * 8 / sis.LogTwoBound
-
-		// Compute r (corresponds to the Montgommery constant)
-		var r fr.Element
-		r.SetString("6014086494747379908336260804527802945383293308637734276299549080986809532403")
-
-		// Attempt to recompose the entry #i in the test-case
-		for i := range testcase {
-			// allegedly corresponds to the limbs of the entry i
-			subRes := sis.bufM[i*numLimbsPerField : (i+1)*numLimbsPerField]
-
-			// performs a Horner evaluation of subres by b
-			var y fr.Element
-			for j := numLimbsPerField - 1; j >= 0; j-- {
-				y.Mul(&y, &b)
-				y.Add(&y, &subRes[j])
+			t.Logf("testcase %v", testcase)
+
+			sis, _ := NewRSis(0, testcase.logTwoDegree, testcase.logTwoBound, 3)
+
+			// clean the sis hasher
+			sis.bufMValues.ClearAll()
+			for i := 0; i < len(sis.bufM); i++ {
+				sis.bufM[i].SetZero()
+			}
+			for i := 0; i < len(sis.bufRes); i++ {
+				sis.bufRes[i].SetZero()
 			}
 
-			fmt.Printf("subres: %v\n", subRes)
+			buf := bytes.Buffer{}
+			for _, x := range testcase.vec {
+				xBytes := x.Bytes()
+				buf.Write(xBytes[:])
+			}
+
+			limbDecomposeBytes(buf.Bytes(), sis.bufM, sis.LogTwoBound, sis.Degree, sis.bufMValues)
+
+			// Just to test, this does not return panic
+			dummyBuffer := make(fr.Vector, 192)
+			LimbDecomposeBytes(buf.Bytes(), dummyBuffer, sis.LogTwoBound)
+
+			// b is a field element representing the max norm bound
+			// used for limb splitting the input field elements.
+			b := fr.NewElement(1 << sis.LogTwoBound)
+			numLimbsPerField := fr.Bytes * 8 / sis.LogTwoBound
+
+			// Compute r (corresponds to the Montgommery constant)
+			var r fr.Element
+			r.SetString("6014086494747379908336260804527802945383293308637734276299549080986809532403")
+
+			// Attempt to recompose the entry #i in the test-case
+			for i := range testcase.vec {
+				// allegedly corresponds to the limbs of the entry i
+				subRes := sis.bufM[i*numLimbsPerField : (i+1)*numLimbsPerField]
+
+				// performs a Horner evaluation of subres by b
+				var y fr.Element
+				for j := numLimbsPerField - 1; j >= 0; j-- {
+					y.Mul(&y, &b)
+					y.Add(&y, &subRes[j])
+				}
+
+				y.Mul(&y, &r)
+				require.Equal(t, testcase.vec[i].String(), y.String(), "the subRes was %v", subRes)
+			}
+		})
 
-			y.Mul(&y, &r)
-			require.Equal(t, testcase[i].String(), y.String(), "the subRes was %v", subRes)
-		}
 	}
 }