From bfbfa39eb9cdaf44721a8a9d15f9137e4b30e190 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Tue, 17 Jun 2025 13:06:18 +0200 Subject: [PATCH 1/8] modify gnark marshal and unmarshal --- crypto/bn256/gnark/g1.go | 64 +++++++++++++++--- crypto/bn256/gnark/g2.go | 83 +++++++++++++++++++++--- crypto/bn256/gnark/native_format_test.go | 47 ++++++++++++++ 3 files changed, 178 insertions(+), 16 deletions(-) create mode 100644 crypto/bn256/gnark/native_format_test.go diff --git a/crypto/bn256/gnark/g1.go b/crypto/bn256/gnark/g1.go index 2f933dd5360..c0400a70466 100644 --- a/crypto/bn256/gnark/g1.go +++ b/crypto/bn256/gnark/g1.go @@ -1,6 +1,7 @@ package bn256 import ( + "errors" "math/big" "github.com/consensys/gnark-crypto/ecc/bn254" @@ -31,21 +32,68 @@ func (g *G1) ScalarMult(a *G1, scalar *big.Int) { // Unmarshal deserializes `buf` into `g` // -// Note: whether the deserialization is of a compressed -// or an uncompressed point, is encoded in the bytes. -// -// For our purpose, the point will always be serialized -// as uncompressed, ie 64 bytes. +// The input is expected to be in the EVM format: +// 64 bytes: [32-byte x coordinate][32-byte y coordinate] +// where each coordinate is in big-endian format. // // This method also checks whether the point is on the // curve and in the prime order subgroup. func (g *G1) Unmarshal(buf []byte) (int, error) { - return g.inner.SetBytes(buf) + if len(buf) < 64 { + return 0, errors.New("invalid G1 point size") + } + + // Check if both coordinates are zero (point at infinity) + isZero := true + for i := 0; i < 64; i++ { + if buf[i] != 0 { + isZero = false + break + } + } + if isZero { + g.inner.X.SetZero() + g.inner.Y.SetZero() + return 64, nil + } + + err := g.inner.X.SetBytesCanonical(buf[:32]) + if err != nil { + return 0, err + } + err = g.inner.Y.SetBytesCanonical(buf[32:64]) + if err != nil { + return 0, err + } + + if !g.inner.IsOnCurve() { + return 0, errors.New("point is not on curve") + } + if !g.inner.IsInSubGroup() { + return 0, errors.New("point is not in correct subgroup") + } + + return 64, nil } // Marshal serializes the point into a byte slice. // -// Note: The point is serialized as uncompressed. +// The output is in EVM format: 64 bytes total. +// [32-byte x coordinate][32-byte y coordinate] +// where each coordinate is a big-endian integer padded to 32 bytes. func (p *G1) Marshal() []byte { - return p.inner.Marshal() + output := make([]byte, 64) + + // Handle point at infinity + if p.inner.X.IsZero() && p.inner.Y.IsZero() { + return output + } + + xBytes := p.inner.X.Bytes() + copy(output[:32], xBytes[:]) + + yBytes := p.inner.Y.Bytes() + copy(output[32:64], yBytes[:]) + + return output } diff --git a/crypto/bn256/gnark/g2.go b/crypto/bn256/gnark/g2.go index 205373a5919..0fe4ffd00b2 100644 --- a/crypto/bn256/gnark/g2.go +++ b/crypto/bn256/gnark/g2.go @@ -1,6 +1,8 @@ package bn256 import ( + "errors" + "github.com/consensys/gnark-crypto/ecc/bn254" ) @@ -18,21 +20,86 @@ type G2 struct { // Unmarshal deserializes `buf` into `g` // -// Note: whether the deserialization is of a compressed -// or an uncompressed point, is encoded in the bytes. -// -// For our purpose, the point will always be serialized -// as uncompressed, ie 128 bytes. +// The input is expected to be in the EVM format: +// 128 bytes: [32-byte x.real][32-byte x.imag][32-byte y.real][32-byte y.imag] +// where each value is a big-endian integer. // // This method also checks whether the point is on the // curve and in the prime order subgroup. func (g *G2) Unmarshal(buf []byte) (int, error) { - return g.inner.SetBytes(buf) + if len(buf) < 128 { + return 0, errors.New("invalid G2 point size") + } + + // Check if all coordinates are zero (point at infinity) + isZero := true + for i := 0; i < 128; i++ { + if buf[i] != 0 { + isZero = false + break + } + } + if isZero { + g.inner.X.A0.SetZero() + g.inner.X.A1.SetZero() + g.inner.Y.A0.SetZero() + g.inner.Y.A1.SetZero() + return 128, nil + } + + err := g.inner.X.A0.SetBytesCanonical(buf[0:32]) + if err != nil { + return 0, err + } + err = g.inner.X.A1.SetBytesCanonical(buf[32:64]) + if err != nil { + return 0, err + } + err = g.inner.Y.A0.SetBytesCanonical(buf[64:96]) + if err != nil { + return 0, err + } + err = g.inner.Y.A1.SetBytesCanonical(buf[96:128]) + if err != nil { + return 0, err + } + + if !g.inner.IsOnCurve() { + return 0, errors.New("point is not on curve") + } + + if !g.inner.IsInSubGroup() { + return 0, errors.New("point is not in correct subgroup") + } + + return 128, nil } // Marshal serializes the point into a byte slice. // -// Note: The point is serialized as uncompressed. +// The output is in EVM format: 128 bytes total. +// [32-byte x.real][32-byte x.imag][32-byte y.real][32-byte y.imag] +// where each value is a big-endian integer padded to 32 bytes. func (g *G2) Marshal() []byte { - return g.inner.Marshal() + output := make([]byte, 128) + + // Handle point at infinity + if g.inner.X.A0.IsZero() && g.inner.X.A1.IsZero() && + g.inner.Y.A0.IsZero() && g.inner.Y.A1.IsZero() { + return output + } + + xA0Bytes := g.inner.X.A0.Bytes() + copy(output[:32], xA0Bytes[:]) + + xA1Bytes := g.inner.X.A1.Bytes() + copy(output[32:64], xA1Bytes[:]) + + yA0Bytes := g.inner.Y.A0.Bytes() + copy(output[64:96], yA0Bytes[:]) + + yA1Bytes := g.inner.Y.A1.Bytes() + copy(output[96:128], yA1Bytes[:]) + + return output } diff --git a/crypto/bn256/gnark/native_format_test.go b/crypto/bn256/gnark/native_format_test.go new file mode 100644 index 00000000000..238826ae699 --- /dev/null +++ b/crypto/bn256/gnark/native_format_test.go @@ -0,0 +1,47 @@ +package bn256 + +import ( + "testing" + + "github.com/consensys/gnark-crypto/ecc/bn254" +) + +func TestNativeGnarkFormatIncompatibility(t *testing.T) { + // Use official gnark serialization + _, _, g1Gen, _ := bn254.Generators() + wrongSer := g1Gen.Bytes() + + // This should fail since the evm serializes points in a different format + var evmG1 G1 + _, err := evmG1.Unmarshal(wrongSer[:]) + if err == nil { + t.Fatalf("Points serialized using the official bn254 serialization algorithm, should not work with the evm format") + } +} + +func TestSerRoundTrip(t *testing.T) { + _, _, g1Gen, g2Gen := bn254.Generators() + + expectedG1 := G1{ + inner: g1Gen, + } + bytesG1 := expectedG1.Marshal() + + expectedG2 := G2{ + inner: g2Gen, + } + bytesG2 := expectedG2.Marshal() + + var gotG1 G1 + gotG1.Unmarshal(bytesG1) + + var gotG2 G2 + gotG2.Unmarshal(bytesG2) + + if !expectedG1.inner.Equal(&gotG1.inner) { + t.Errorf("serialization roundtrip failed for G1") + } + if !expectedG2.inner.Equal(&gotG2.inner) { + t.Errorf("serialization roundtrip failed for G2") + } +} From 283f1a10d111ce6e98edbaf1c30b6812417536d4 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Tue, 17 Jun 2025 13:27:24 +0200 Subject: [PATCH 2/8] Apply suggestions from code review --- crypto/bn256/gnark/native_format_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crypto/bn256/gnark/native_format_test.go b/crypto/bn256/gnark/native_format_test.go index 238826ae699..a34395c3792 100644 --- a/crypto/bn256/gnark/native_format_test.go +++ b/crypto/bn256/gnark/native_format_test.go @@ -11,11 +11,10 @@ func TestNativeGnarkFormatIncompatibility(t *testing.T) { _, _, g1Gen, _ := bn254.Generators() wrongSer := g1Gen.Bytes() - // This should fail since the evm serializes points in a different format var evmG1 G1 _, err := evmG1.Unmarshal(wrongSer[:]) if err == nil { - t.Fatalf("Points serialized using the official bn254 serialization algorithm, should not work with the evm format") + t.Fatalf("points serialized using the official bn254 serialization algorithm, should not work with the evm format") } } From 103d398e488971f7a3d2d0208f840bc547423782 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Tue, 17 Jun 2025 15:09:08 +0200 Subject: [PATCH 3/8] appease felix --- crypto/bn256/gnark/g1.go | 19 ++++++++++++------- crypto/bn256/gnark/g2.go | 8 +------- crypto/bn256/gnark/native_format_test.go | 8 ++------ 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/crypto/bn256/gnark/g1.go b/crypto/bn256/gnark/g1.go index c0400a70466..50c25212bff 100644 --- a/crypto/bn256/gnark/g1.go +++ b/crypto/bn256/gnark/g1.go @@ -44,13 +44,7 @@ func (g *G1) Unmarshal(buf []byte) (int, error) { } // Check if both coordinates are zero (point at infinity) - isZero := true - for i := 0; i < 64; i++ { - if buf[i] != 0 { - isZero = false - break - } - } + isZero := allZeroes(buf[0:64]) if isZero { g.inner.X.SetZero() g.inner.Y.SetZero() @@ -97,3 +91,14 @@ func (p *G1) Marshal() []byte { return output } + +func allZeroes(buf []byte) bool { + isZero := true + for i := 0; i < len(buf); i++ { + if buf[i] != 0 { + isZero = false + break + } + } + return isZero +} diff --git a/crypto/bn256/gnark/g2.go b/crypto/bn256/gnark/g2.go index 0fe4ffd00b2..83a33c112ac 100644 --- a/crypto/bn256/gnark/g2.go +++ b/crypto/bn256/gnark/g2.go @@ -32,13 +32,7 @@ func (g *G2) Unmarshal(buf []byte) (int, error) { } // Check if all coordinates are zero (point at infinity) - isZero := true - for i := 0; i < 128; i++ { - if buf[i] != 0 { - isZero = false - break - } - } + isZero := allZeroes(buf[0:128]) if isZero { g.inner.X.A0.SetZero() g.inner.X.A1.SetZero() diff --git a/crypto/bn256/gnark/native_format_test.go b/crypto/bn256/gnark/native_format_test.go index a34395c3792..e2b67449321 100644 --- a/crypto/bn256/gnark/native_format_test.go +++ b/crypto/bn256/gnark/native_format_test.go @@ -21,14 +21,10 @@ func TestNativeGnarkFormatIncompatibility(t *testing.T) { func TestSerRoundTrip(t *testing.T) { _, _, g1Gen, g2Gen := bn254.Generators() - expectedG1 := G1{ - inner: g1Gen, - } + expectedG1 := G1{inner: g1Gen} bytesG1 := expectedG1.Marshal() - expectedG2 := G2{ - inner: g2Gen, - } + expectedG2 := G2{inner: g2Gen} bytesG2 := expectedG2.Marshal() var gotG1 G1 From 2f98aee459d883e1d9b7aada0f2c8d8f428a611c Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Tue, 17 Jun 2025 15:14:07 +0200 Subject: [PATCH 4/8] remove optimization --- crypto/bn256/gnark/g1.go | 5 ----- crypto/bn256/gnark/g2.go | 6 ------ 2 files changed, 11 deletions(-) diff --git a/crypto/bn256/gnark/g1.go b/crypto/bn256/gnark/g1.go index 50c25212bff..d6902ab3225 100644 --- a/crypto/bn256/gnark/g1.go +++ b/crypto/bn256/gnark/g1.go @@ -78,11 +78,6 @@ func (g *G1) Unmarshal(buf []byte) (int, error) { func (p *G1) Marshal() []byte { output := make([]byte, 64) - // Handle point at infinity - if p.inner.X.IsZero() && p.inner.Y.IsZero() { - return output - } - xBytes := p.inner.X.Bytes() copy(output[:32], xBytes[:]) diff --git a/crypto/bn256/gnark/g2.go b/crypto/bn256/gnark/g2.go index 83a33c112ac..619660f7719 100644 --- a/crypto/bn256/gnark/g2.go +++ b/crypto/bn256/gnark/g2.go @@ -77,12 +77,6 @@ func (g *G2) Unmarshal(buf []byte) (int, error) { func (g *G2) Marshal() []byte { output := make([]byte, 128) - // Handle point at infinity - if g.inner.X.A0.IsZero() && g.inner.X.A1.IsZero() && - g.inner.Y.A0.IsZero() && g.inner.Y.A1.IsZero() { - return output - } - xA0Bytes := g.inner.X.A0.Bytes() copy(output[:32], xA0Bytes[:]) From c83cdbb79a97e263558b631c10b36ad0d77ecadd Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Tue, 17 Jun 2025 15:16:58 +0200 Subject: [PATCH 5/8] shorter comment --- crypto/bn256/gnark/g2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/bn256/gnark/g2.go b/crypto/bn256/gnark/g2.go index 619660f7719..c7b0ad65d25 100644 --- a/crypto/bn256/gnark/g2.go +++ b/crypto/bn256/gnark/g2.go @@ -73,7 +73,7 @@ func (g *G2) Unmarshal(buf []byte) (int, error) { // // The output is in EVM format: 128 bytes total. // [32-byte x.real][32-byte x.imag][32-byte y.real][32-byte y.imag] -// where each value is a big-endian integer padded to 32 bytes. +// where each value is a big-endian integer. func (g *G2) Marshal() []byte { output := make([]byte, 128) From 5fc7fcdbf830e8cee5e73d9acb1d85c1b8cddd30 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Tue, 17 Jun 2025 15:19:24 +0200 Subject: [PATCH 6/8] less words -- appease felix even more --- crypto/bn256/gnark/g2.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/bn256/gnark/g2.go b/crypto/bn256/gnark/g2.go index c7b0ad65d25..9d9c2cf2af5 100644 --- a/crypto/bn256/gnark/g2.go +++ b/crypto/bn256/gnark/g2.go @@ -21,7 +21,7 @@ type G2 struct { // Unmarshal deserializes `buf` into `g` // // The input is expected to be in the EVM format: -// 128 bytes: [32-byte x.real][32-byte x.imag][32-byte y.real][32-byte y.imag] +// 128 bytes: [32-byte x.0][32-byte x.1][32-byte y.0][32-byte y.1] // where each value is a big-endian integer. // // This method also checks whether the point is on the @@ -72,7 +72,7 @@ func (g *G2) Unmarshal(buf []byte) (int, error) { // Marshal serializes the point into a byte slice. // // The output is in EVM format: 128 bytes total. -// [32-byte x.real][32-byte x.imag][32-byte y.real][32-byte y.imag] +// [32-byte x.0][32-byte x.1][32-byte y.0][32-byte y.1] // where each value is a big-endian integer. func (g *G2) Marshal() []byte { output := make([]byte, 128) From d2bcbab38d5d7f5ee0185a9cf4d27361be75791e Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Tue, 17 Jun 2025 15:49:56 +0200 Subject: [PATCH 7/8] Apply felix diff Co-authored-by: Felix Lange --- crypto/bn256/gnark/g1.go | 15 +++++---------- crypto/bn256/gnark/g2.go | 17 +++++------------ 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/crypto/bn256/gnark/g1.go b/crypto/bn256/gnark/g1.go index d6902ab3225..a4cd9bbd16f 100644 --- a/crypto/bn256/gnark/g1.go +++ b/crypto/bn256/gnark/g1.go @@ -51,12 +51,10 @@ func (g *G1) Unmarshal(buf []byte) (int, error) { return 64, nil } - err := g.inner.X.SetBytesCanonical(buf[:32]) - if err != nil { + if err := g.inner.X.SetBytesCanonical(buf[:32]); err != nil { return 0, err } - err = g.inner.Y.SetBytesCanonical(buf[32:64]) - if err != nil { + if err := g.inner.Y.SetBytesCanonical(buf[32:64]); err != nil { return 0, err } @@ -66,7 +64,6 @@ func (g *G1) Unmarshal(buf []byte) (int, error) { if !g.inner.IsInSubGroup() { return 0, errors.New("point is not in correct subgroup") } - return 64, nil } @@ -88,12 +85,10 @@ func (p *G1) Marshal() []byte { } func allZeroes(buf []byte) bool { - isZero := true - for i := 0; i < len(buf); i++ { + for i := range buf { if buf[i] != 0 { - isZero = false - break + return false } } - return isZero + return true } diff --git a/crypto/bn256/gnark/g2.go b/crypto/bn256/gnark/g2.go index 9d9c2cf2af5..ad91c065155 100644 --- a/crypto/bn256/gnark/g2.go +++ b/crypto/bn256/gnark/g2.go @@ -31,41 +31,34 @@ func (g *G2) Unmarshal(buf []byte) (int, error) { return 0, errors.New("invalid G2 point size") } - // Check if all coordinates are zero (point at infinity) isZero := allZeroes(buf[0:128]) if isZero { + // point at infinity g.inner.X.A0.SetZero() g.inner.X.A1.SetZero() g.inner.Y.A0.SetZero() g.inner.Y.A1.SetZero() return 128, nil } - - err := g.inner.X.A0.SetBytesCanonical(buf[0:32]) - if err != nil { + if err := g.inner.X.A0.SetBytesCanonical(buf[0:32]); err != nil { return 0, err } - err = g.inner.X.A1.SetBytesCanonical(buf[32:64]) - if err != nil { + if err := g.inner.X.A1.SetBytesCanonical(buf[32:64]); err != nil { return 0, err } - err = g.inner.Y.A0.SetBytesCanonical(buf[64:96]) - if err != nil { + if err := g.inner.Y.A0.SetBytesCanonical(buf[64:96]); err != nil { return 0, err } - err = g.inner.Y.A1.SetBytesCanonical(buf[96:128]) - if err != nil { + if err := g.inner.Y.A1.SetBytesCanonical(buf[96:128]); err != nil { return 0, err } if !g.inner.IsOnCurve() { return 0, errors.New("point is not on curve") } - if !g.inner.IsInSubGroup() { return 0, errors.New("point is not in correct subgroup") } - return 128, nil } From c8d92ec016c8d9a3de60f278ccc1d7372c5cc8e4 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Tue, 17 Jun 2025 15:56:56 +0200 Subject: [PATCH 8/8] appease felix, hes right next to me --- crypto/bn256/gnark/g1.go | 5 ++--- crypto/bn256/gnark/g2.go | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crypto/bn256/gnark/g1.go b/crypto/bn256/gnark/g1.go index a4cd9bbd16f..59e04cb247f 100644 --- a/crypto/bn256/gnark/g1.go +++ b/crypto/bn256/gnark/g1.go @@ -43,9 +43,8 @@ func (g *G1) Unmarshal(buf []byte) (int, error) { return 0, errors.New("invalid G1 point size") } - // Check if both coordinates are zero (point at infinity) - isZero := allZeroes(buf[0:64]) - if isZero { + if allZeroes(buf[:64]) { + // point at infinity g.inner.X.SetZero() g.inner.Y.SetZero() return 64, nil diff --git a/crypto/bn256/gnark/g2.go b/crypto/bn256/gnark/g2.go index ad91c065155..07452cc2d87 100644 --- a/crypto/bn256/gnark/g2.go +++ b/crypto/bn256/gnark/g2.go @@ -31,8 +31,7 @@ func (g *G2) Unmarshal(buf []byte) (int, error) { return 0, errors.New("invalid G2 point size") } - isZero := allZeroes(buf[0:128]) - if isZero { + if allZeroes(buf[:128]) { // point at infinity g.inner.X.A0.SetZero() g.inner.X.A1.SetZero()