From c09537890c6f07b23633499ad19b8703d149ba61 Mon Sep 17 00:00:00 2001 From: garyschulte Date: Mon, 12 Aug 2024 18:33:08 -0700 Subject: [PATCH 1/2] use args rather than return params for bn254 unmarshaling Signed-off-by: garyschulte --- gnark/gnark-jni/gnark-eip-196.go | 64 ++++++++++++------- ...8PairingPrecompiledContractLegacyTest.java | 2 +- .../besu/nativelib/gnark/eip196_g1_add.csv | 6 +- .../besu/nativelib/gnark/eip196_g1_mul.csv | 6 +- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/gnark/gnark-jni/gnark-eip-196.go b/gnark/gnark-jni/gnark-eip-196.go index 68052a7d..30a9632e 100644 --- a/gnark/gnark-jni/gnark-eip-196.go +++ b/gnark/gnark-jni/gnark-eip-196.go @@ -21,6 +21,7 @@ var ErrMalformedPointEIP196 = errors.New("invalid point encoding") var ErrInvalidInputPairingLengthEIP196 = errors.New("invalid input parameters, invalid input length for pairing") var ErrPointNotInFieldEIP196 = errors.New("point not in field") var ErrPointOnCurveCheckFailedEIP196 = errors.New("point is not on curve") +var ErrPointInSubgroupCheckFailedEIP196 = errors.New("point is not in subgroup") const ( EIP196PreallocateForResult = 128 @@ -66,7 +67,9 @@ func eip196altbn128G1Add(javaInputBuf, javaOutputBuf, javaErrorBuf *C.char, cInp } // generate p0 g1 affine - p0, err := safeUnmarshalEIP196(input, 0) + var p0 bn254.G1Affine + + err := safeUnmarshalEIP196(&p0, input, 0) if err != nil { dryError(err, errorBuf, outputLen, errorLen) @@ -83,7 +86,8 @@ func eip196altbn128G1Add(javaInputBuf, javaOutputBuf, javaErrorBuf *C.char, cInp } } // generate p1 g1 affine - p1, err := safeUnmarshalEIP196(input, 64) + var p1 bn254.G1Affine + err = safeUnmarshalEIP196(&p1, input, 64) if err != nil { dryError(err, errorBuf, outputLen, errorLen) @@ -91,13 +95,8 @@ func eip196altbn128G1Add(javaInputBuf, javaOutputBuf, javaErrorBuf *C.char, cInp } var result *bn254.G1Affine - if p1 == nil { - // if p1 is nil, just return p0 - result = p0 - } else { - // Use the Add method to combine points - result = p0.Add(p0, p1) - } + // Use the Add method to combine points + result = p0.Add(&p0, &p1) // marshal the resulting point and encode directly to the output buffer ret := result.Marshal() @@ -131,7 +130,8 @@ func eip196altbn128G1Mul(javaInputBuf, javaOutputBuf, javaErrorBuf *C.char, cInp input := (*[EIP196PreallocateForG1 + EIP196PreallocateForScalar]byte)(unsafe.Pointer(javaInputBuf))[:inputLen:inputLen] // generate p0 g1 affine - p0, err := safeUnmarshalEIP196(input, 0) + var p0 bn254.G1Affine + err := safeUnmarshalEIP196(&p0, input, 0) if err != nil { dryError(err, errorBuf, outputLen, errorLen) @@ -156,7 +156,7 @@ func eip196altbn128G1Mul(javaInputBuf, javaOutputBuf, javaErrorBuf *C.char, cInp scalar.SetBytes(scalarBytes[:]) // multiply g1 point by scalar - result := p0.ScalarMultiplication(p0, scalar) + result := p0.ScalarMultiplication(&p0, scalar) // marshal the resulting point and encode directly to the output buffer ret := result.Marshal() @@ -201,7 +201,7 @@ func eip196altbn128Pairing(javaInputBuf, javaOutputBuf, javaErrorBuf *C.char, cI // g1 x and y are the first 64 bytes of each 192 byte pair var g1 bn254.G1Affine - err := g1.Unmarshal(input[i*192:i*192+64]) + err := safeUnmarshalEIP196(&g1, input[i*192:i*192+64], 0) if err != nil { dryError(err, errorBuf, outputLen, errorLen) @@ -210,7 +210,7 @@ func eip196altbn128Pairing(javaInputBuf, javaOutputBuf, javaErrorBuf *C.char, cI // g2 points are latter 128 bytes of each 192 byte pair var g2 bn254.G2Affine - err = g2.Unmarshal(input[i*192+64:(i+1)*192]) + err = safeUnmarshalG2EIP196(&g2, input[i*192+64:(i+1)*192]) if err != nil { dryError(err, errorBuf, outputLen, errorLen) @@ -251,14 +251,12 @@ func g1AffineEncode(g1Point []byte, output *C.char) (error) { return nil } -func safeUnmarshalEIP196(input []byte, offset int) (*bn254.G1Affine, error) { - var g1 bn254.G1Affine - +func safeUnmarshalEIP196(g1 *bn254.G1Affine, input []byte, offset int) (error) { var pointBytes []byte // If we effectively have _NO_ input, return empty if len(input)-offset <= 0 { - return nil, nil + return nil } else if len(input)-offset < 64 { // If we have some input, but it is incomplete, pad with zero pointBytes = make([]byte, 64) @@ -272,7 +270,7 @@ func safeUnmarshalEIP196(input []byte, offset int) (*bn254.G1Affine, error) { } if !checkInFieldEIP196(pointBytes[0:32]) { - return nil, ErrPointNotInFieldEIP196 + return ErrPointNotInFieldEIP196 } err := g1.X.SetBytesCanonical(pointBytes[0:32]) @@ -280,21 +278,43 @@ func safeUnmarshalEIP196(input []byte, offset int) (*bn254.G1Affine, error) { if (err == nil) { if !checkInFieldEIP196(pointBytes[32:64]) { - return nil, ErrPointNotInFieldEIP196 + return ErrPointNotInFieldEIP196 } err := g1.Y.SetBytesCanonical(pointBytes[32:64]) if (err == nil) { if (!g1.IsOnCurve()) { - return nil, ErrPointOnCurveCheckFailedEIP196 + return ErrPointInSubgroupCheckFailedEIP196 } - return &g1, nil + return nil } } - return nil, err + return err } +func safeUnmarshalG2EIP196(g2 *bn254.G2Affine, input []byte) (error) { + if len(input) < EIP196PreallocateForG2 { + return ErrInvalidInputPairingLengthEIP196 + } + + if !(checkInFieldEIP196(input[0:32]) && checkInFieldEIP196(input[32:64]) && + checkInFieldEIP196(input[64:96]) && checkInFieldEIP196(input[96:128])) { + return ErrPointNotInFieldEIP196 + } + + g2.X.A1.SetBytesCanonical(input[:32]) + g2.X.A0.SetBytesCanonical(input[32:64]) + g2.Y.A1.SetBytesCanonical(input[64:96]) + g2.Y.A0.SetBytesCanonical(input[96:128]) + if (!g2.IsInSubGroup()) { + return ErrPointOnCurveCheckFailedEIP196 + } + + return nil +} + + // checkInField checks that an element is in the field, not-in-field will normally // be caught during unmarshal, but here in case of no-op calls of a single parameter func checkInFieldEIP196(data []byte) bool { diff --git a/gnark/src/test/java/org/hyperledger/besu/nativelib/gnark/AltBN128PairingPrecompiledContractLegacyTest.java b/gnark/src/test/java/org/hyperledger/besu/nativelib/gnark/AltBN128PairingPrecompiledContractLegacyTest.java index 22e082db..48efa2c7 100644 --- a/gnark/src/test/java/org/hyperledger/besu/nativelib/gnark/AltBN128PairingPrecompiledContractLegacyTest.java +++ b/gnark/src/test/java/org/hyperledger/besu/nativelib/gnark/AltBN128PairingPrecompiledContractLegacyTest.java @@ -130,7 +130,7 @@ public void compute_invalidPointsOutsideSubgroupG2() { // assert there is an error assertThat(errorLength.getValue()).isNotEqualTo(0); String errorStr = new String(error, 0, errorLength.getValue()); - assertThat(errorStr).isEqualTo("invalid input parameters, invalid point: subgroup check failed"); + assertThat(errorStr).isEqualTo("invalid input parameters, point is not on curve"); // assert there is no output assertThat(outputLength.getValue()).isEqualTo(0); } diff --git a/gnark/src/test/resources/org/hyperledger/besu/nativelib/gnark/eip196_g1_add.csv b/gnark/src/test/resources/org/hyperledger/besu/nativelib/gnark/eip196_g1_add.csv index 435ec06f..552166e3 100644 --- a/gnark/src/test/resources/org/hyperledger/besu/nativelib/gnark/eip196_g1_add.csv +++ b/gnark/src/test/resources/org/hyperledger/besu/nativelib/gnark/eip196_g1_add.csv @@ -100,9 +100,9 @@ input,result,gas,notes 16eea4cc6a5c21b361c4d144f81f0c5774181b19b6341ebc3317b6d9c78b493407abc49d40a88a2336ea698f6bcc5b767f290fa5255f1ea9c449237dfa75259210264114425d52ebb998a32b103fe50d3c44b156f53dd16fec92c805c51b3bc117986b497207e3198ca7beb705537ba0d6c82cb8e6e1c507e74dece3fbb17fc2,2ef81763011baa4945a15da4c312cd2c280017994fcbea76048907c58acaf8f01a66e2bd9a4c4e1ca39a7fa9ba43f615c3720ae73c0d2aa6b2cb14e5a24f233d,150, 1ca0717a8dfb9c3940a731d7c52f1699f64fe05e76189a91dc622e8fafd99de62313a1df5b32b17c21e53e2d0a1ff3eeac6ab4359a9f86e51b1c236f414d87ea0e9729271df80f9967d618c33d9e8389bd4afb88e8b1e26e20b98868406da8ce1aca0647ae2e8573e39970f442aa3900175beeef2984af814fa51cf4ab59e07c,278782b7f77402d99c21f0bb831e899431703967252026abb6a6cfefd6d368600bd99290fca19622267066ee211842111904f2ced987023a34048236be757231,150, 1ca0717a8dfb9c3940a731d7c52f1699f64fe05e76189a91dc622e8fafd99de62313a1df5b32b17c21e53e2d0a1ff3eeac6ab4359a9f86e51b1c236f414d87ea00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,1ca0717a8dfb9c3940a731d7c52f1699f64fe05e76189a91dc622e8fafd99de62313a1df5b32b17c21e53e2d0a1ff3eeac6ab4359a9f86e51b1c236f414d87ea,150, -1234,,150,invalid input parameters, point is not on curve -0174fc233104c2ad4f56a8396b8c1b7d9c6ad10bffc70761c5e8f5280862f137029733a9f20a4cdbb7ae9c5dd1adf6ccc7fe3439d7dc71093af0656ae0ca0f290964773f12e2292f332306374f957d10,,150,invalid input parameters, point is not on curve -0174fc233104c2ad4f56a8396b8c1b7d9c6ad10bffc70761c5e8f5280862f137029733a9f20a4cdbb7ae9c5dd1adf6ccc7fe3439d7dc71093af0656ae0ca0f290964773f12e2292f332306374f957d10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,150,invalid input parameters, point is not on curve +1234,,150,invalid input parameters, point is not in subgroup +0174fc233104c2ad4f56a8396b8c1b7d9c6ad10bffc70761c5e8f5280862f137029733a9f20a4cdbb7ae9c5dd1adf6ccc7fe3439d7dc71093af0656ae0ca0f290964773f12e2292f332306374f957d10,,150,invalid input parameters, point is not in subgroup +0174fc233104c2ad4f56a8396b8c1b7d9c6ad10bffc70761c5e8f5280862f137029733a9f20a4cdbb7ae9c5dd1adf6ccc7fe3439d7dc71093af0656ae0ca0f290964773f12e2292f332306374f957d10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,150,invalid input parameters, point is not in subgroup 67376aad340c93eb0fc9bc8e040dc691bd00e426c6456b4d13079e7f1dbb3da847eb0fc271cd23da50c6ebc261928abb1af9bfcea998791e18af1817b06221e1fe708d2f4224275523fcd37460a310ce4b56f1694dfe36280410f0fb6efc5f47b85662e5b08d881242a72acbc2c8e2fa71ac593be977ad3e090c8158aace0247,,150,invalid input parameters, point not in field ,00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,150, ff000000000000000000000000000000000000000000000000000000000000ff,,150,invalid input parameters, point not in field diff --git a/gnark/src/test/resources/org/hyperledger/besu/nativelib/gnark/eip196_g1_mul.csv b/gnark/src/test/resources/org/hyperledger/besu/nativelib/gnark/eip196_g1_mul.csv index 7a3ffd54..5606c104 100644 --- a/gnark/src/test/resources/org/hyperledger/besu/nativelib/gnark/eip196_g1_mul.csv +++ b/gnark/src/test/resources/org/hyperledger/besu/nativelib/gnark/eip196_g1_mul.csv @@ -100,10 +100,10 @@ input,result,gas,notes 1c2cb2b4504b19c7e073679432e625f2706b7c4728cd9bd3ce36579f4de2f3902c8605f723ac2f73baa15eac674f62ab06c79809aa4a4be3391c4d41d5a6e62ca0a73a5b559cd0384ca11d444e34f40e04f2080483db9e54ae09e44ec19c26a1,1147da8fc5774b5ef94fda682437b04f9b6aea72987d1269496769baf7e67dc729fe144eab44c6da4c851123165cd6506ce3e10b7691b8bf11b757e2ae058fcb,6000, 0d6ad8e12b4f61e3e2a2252ce11428941f2a84b7f0a821cb8cc7699303bd4fec2247870562618fd8d6169072d9b33614d2acf800b3ba0ff68ef8d5fd4d6c250d3e70b3bed17894f958579644c83fa9d485121d580e2b061c697e68f950297768,0be6d75e2fe2887835d396dae11321ca7c53083abd6a0b270ee1c087593517aa2ffd1bad577de7cf2b19b82bfff0c66e2afbfb79a72cbe834290437f3caf2f21,6000, d2acf800b3ba0ff68ef8d5fd4d6c250d3e70b3bed17894f958579644c83fa9d485121d580e2b061c697e68f9502977680d6ad8e12b4f61e3e2a2252ce11428941f2a84b7f0a821cb8cc7699303bd4fec2247870562618fd8d6169072d9b33614,,6000,invalid input parameters, point not in field -02acf800b3ba0ff68ef8d5fd4d6c250d3e70b3bed17894f958579644c83fa9d405121d580e2b061c697e68f9502977680d6ad8e12b4f61e3e2a2252ce11428941f2a84b7f0a821cb8cc7699303bd4fec2247870562618fd8d6169072d9b33614,,6000,invalid input parameters, point is not on curve -1c2cb2b4504b19c7e073679432e625f2706b7c4728cd9bd3ce36579f4de2f3902c8605f723ac2f73baa15eac674f62ab,00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,6000,invalid input parameters, point is not on curve +02acf800b3ba0ff68ef8d5fd4d6c250d3e70b3bed17894f958579644c83fa9d405121d580e2b061c697e68f9502977680d6ad8e12b4f61e3e2a2252ce11428941f2a84b7f0a821cb8cc7699303bd4fec2247870562618fd8d6169072d9b33614,,6000,invalid input parameters, point is not in subgroup +1c2cb2b4504b19c7e073679432e625f2706b7c4728cd9bd3ce36579f4de2f3902c8605f723ac2f73baa15eac674f62ab,00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,6000,invalid input parameters, point is not in subgroup ,00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,6000, 1A87B0584CE92F4593D161480614F2989035225609F08058CCFA3D0F940FEBE31A2F3C951F6DADCC7EE9007DFF81504B0FCD6D7CF59996EFDC33D92BF7F9F8F60100000000000000000000000000000000,0x220cb1540fa85ba04d863dca86de9359a43ac9fc084aebb9f2a936d989abbb602ccdc6c020dd2cf78332132b3f1d1122391b515035623cd6f53d4aea24ea2466,6000, 1A87B0584CE92F4593D161480614F2989035225609F08058CCFA3D0F940FEBE31A2F3C951F6DADCC7EE9007DFF81504B0FCD6D7CF59996EFDC33D92BF7F9F8F60000000000000000000000000000000100000000000000000000000000000000,1051acb0700ec6d42a88215852d582efbaef31529b6fcbc3277b5c1b300f5cf0135b2394bb45ab04b8bd7611bd2dfe1de6a4e6e2ccea1ea1955f577cd66af85b,6000, -1c2cb2b4504b19c7e073679432e625f2706b7c4728cd9bd3ce36579f4de2f390,,6000,invalid input parameters, point is not on curve +1c2cb2b4504b19c7e073679432e625f2706b7c4728cd9bd3ce36579f4de2f390,,6000,invalid input parameters, point is not in subgroup 1c2cb2b4504b19c7e073679432e625f2706b7c4728cd9bd3ce36579f4de2f3902c8605f723ac2f73baa15eac674f62ab06c79809aa4a4be3391c4d41d5a6e62c,00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000,6000, From 551f2e54c7a5a8e7c560e64453b2e8a4f812be8b Mon Sep 17 00:00:00 2001 From: garyschulte Date: Tue, 13 Aug 2024 09:44:33 -0700 Subject: [PATCH 2/2] error message consistency Signed-off-by: garyschulte --- gnark/gnark-jni/gnark-eip-196.go | 5 ++--- .../gnark/AltBN128PairingPrecompiledContractLegacyTest.java | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/gnark/gnark-jni/gnark-eip-196.go b/gnark/gnark-jni/gnark-eip-196.go index 30a9632e..cd9c792b 100644 --- a/gnark/gnark-jni/gnark-eip-196.go +++ b/gnark/gnark-jni/gnark-eip-196.go @@ -20,7 +20,6 @@ import ( var ErrMalformedPointEIP196 = errors.New("invalid point encoding") var ErrInvalidInputPairingLengthEIP196 = errors.New("invalid input parameters, invalid input length for pairing") var ErrPointNotInFieldEIP196 = errors.New("point not in field") -var ErrPointOnCurveCheckFailedEIP196 = errors.New("point is not on curve") var ErrPointInSubgroupCheckFailedEIP196 = errors.New("point is not in subgroup") const ( @@ -282,7 +281,7 @@ func safeUnmarshalEIP196(g1 *bn254.G1Affine, input []byte, offset int) (error) { } err := g1.Y.SetBytesCanonical(pointBytes[32:64]) if (err == nil) { - if (!g1.IsOnCurve()) { + if (!g1.IsInSubGroup()) { return ErrPointInSubgroupCheckFailedEIP196 } return nil @@ -308,7 +307,7 @@ func safeUnmarshalG2EIP196(g2 *bn254.G2Affine, input []byte) (error) { g2.Y.A1.SetBytesCanonical(input[64:96]) g2.Y.A0.SetBytesCanonical(input[96:128]) if (!g2.IsInSubGroup()) { - return ErrPointOnCurveCheckFailedEIP196 + return ErrPointInSubgroupCheckFailedEIP196 } return nil diff --git a/gnark/src/test/java/org/hyperledger/besu/nativelib/gnark/AltBN128PairingPrecompiledContractLegacyTest.java b/gnark/src/test/java/org/hyperledger/besu/nativelib/gnark/AltBN128PairingPrecompiledContractLegacyTest.java index 48efa2c7..a9aa6c2d 100644 --- a/gnark/src/test/java/org/hyperledger/besu/nativelib/gnark/AltBN128PairingPrecompiledContractLegacyTest.java +++ b/gnark/src/test/java/org/hyperledger/besu/nativelib/gnark/AltBN128PairingPrecompiledContractLegacyTest.java @@ -130,7 +130,7 @@ public void compute_invalidPointsOutsideSubgroupG2() { // assert there is an error assertThat(errorLength.getValue()).isNotEqualTo(0); String errorStr = new String(error, 0, errorLength.getValue()); - assertThat(errorStr).isEqualTo("invalid input parameters, point is not on curve"); + assertThat(errorStr).isEqualTo("invalid input parameters, point is not in subgroup"); // assert there is no output assertThat(outputLength.getValue()).isEqualTo(0); }