From 63bc2366cf84d61b0f9db09bfcb17d7d4d3676f2 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 21 Feb 2024 15:51:29 +0100 Subject: [PATCH 1/7] go-bindings: pass large arrays by ref instead of value --- bindings/go/main.go | 16 ++++++++-------- bindings/go/main_test.go | 20 ++++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/bindings/go/main.go b/bindings/go/main.go index bdd5385b5..590c52016 100644 --- a/bindings/go/main.go +++ b/bindings/go/main.go @@ -200,14 +200,14 @@ BlobToKZGCommitment is the binding for: const Blob *blob, const KZGSettings *s); */ -func BlobToKZGCommitment(blob Blob) (KZGCommitment, error) { +func BlobToKZGCommitment(blob *Blob) (KZGCommitment, error) { if !loaded { panic("trusted setup isn't loaded") } commitment := KZGCommitment{} ret := C.blob_to_kzg_commitment( (*C.KZGCommitment)(unsafe.Pointer(&commitment)), - (*C.Blob)(unsafe.Pointer(&blob)), + (*C.Blob)(unsafe.Pointer(blob)), &settings) return commitment, makeErrorFromRet(ret) } @@ -222,7 +222,7 @@ ComputeKZGProof is the binding for: const Bytes32 *z_bytes, const KZGSettings *s); */ -func ComputeKZGProof(blob Blob, zBytes Bytes32) (KZGProof, Bytes32, error) { +func ComputeKZGProof(blob *Blob, zBytes Bytes32) (KZGProof, Bytes32, error) { if !loaded { panic("trusted setup isn't loaded") } @@ -231,7 +231,7 @@ func ComputeKZGProof(blob Blob, zBytes Bytes32) (KZGProof, Bytes32, error) { ret := C.compute_kzg_proof( (*C.KZGProof)(unsafe.Pointer(&proof)), (*C.Bytes32)(unsafe.Pointer(&y)), - (*C.Blob)(unsafe.Pointer(&blob)), + (*C.Blob)(unsafe.Pointer(blob)), (*C.Bytes32)(unsafe.Pointer(&zBytes)), &settings) return proof, y, makeErrorFromRet(ret) @@ -246,14 +246,14 @@ ComputeBlobKZGProof is the binding for: const Bytes48 *commitment_bytes, const KZGSettings *s); */ -func ComputeBlobKZGProof(blob Blob, commitmentBytes Bytes48) (KZGProof, error) { +func ComputeBlobKZGProof(blob *Blob, commitmentBytes Bytes48) (KZGProof, error) { if !loaded { panic("trusted setup isn't loaded") } proof := KZGProof{} ret := C.compute_blob_kzg_proof( (*C.KZGProof)(unsafe.Pointer(&proof)), - (*C.Blob)(unsafe.Pointer(&blob)), + (*C.Blob)(unsafe.Pointer(blob)), (*C.Bytes48)(unsafe.Pointer(&commitmentBytes)), &settings) return proof, makeErrorFromRet(ret) @@ -295,14 +295,14 @@ VerifyBlobKZGProof is the binding for: const Bytes48 *proof_bytes, const KZGSettings *s); */ -func VerifyBlobKZGProof(blob Blob, commitmentBytes, proofBytes Bytes48) (bool, error) { +func VerifyBlobKZGProof(blob *Blob, commitmentBytes, proofBytes Bytes48) (bool, error) { if !loaded { panic("trusted setup isn't loaded") } var result C.bool ret := C.verify_blob_kzg_proof( &result, - (*C.Blob)(unsafe.Pointer(&blob)), + (*C.Blob)(unsafe.Pointer(blob)), (*C.Bytes48)(unsafe.Pointer(&commitmentBytes)), (*C.Bytes48)(unsafe.Pointer(&proofBytes)), &settings) diff --git a/bindings/go/main_test.go b/bindings/go/main_test.go index 258883de4..79b325e0e 100644 --- a/bindings/go/main_test.go +++ b/bindings/go/main_test.go @@ -91,7 +91,7 @@ func TestBlobToKZGCommitment(t *testing.T) { return } - commitment, err := BlobToKZGCommitment(blob) + commitment, err := BlobToKZGCommitment(&blob) if err == nil { require.NotNil(t, test.Output) require.Equal(t, test.Output[:], commitment[:]) @@ -138,7 +138,7 @@ func TestComputeKZGProof(t *testing.T) { return } - proof, y, err := ComputeKZGProof(blob, z) + proof, y, err := ComputeKZGProof(&blob, z) if err == nil { require.NotNil(t, test.Output) var expectedProof Bytes48 @@ -192,7 +192,7 @@ func TestComputeBlobKZGProof(t *testing.T) { return } - proof, err := ComputeBlobKZGProof(blob, commitment) + proof, err := ComputeBlobKZGProof(&blob, commitment) if err == nil { require.NotNil(t, test.Output) require.Equal(t, test.Output[:], proof[:]) @@ -310,7 +310,7 @@ func TestVerifyBlobKZGProof(t *testing.T) { return } - valid, err := VerifyBlobKZGProof(blob, commitment, proof) + valid, err := VerifyBlobKZGProof(&blob, commitment, proof) if err == nil { require.NotNil(t, test.Output) require.Equal(t, *test.Output, valid) @@ -400,9 +400,9 @@ func Benchmark(b *testing.B) { fields := [length]Bytes32{} for i := 0; i < length; i++ { blob := getRandBlob(int64(i)) - commitment, err := BlobToKZGCommitment(blob) + commitment, err := BlobToKZGCommitment(&blob) require.NoError(b, err) - proof, err := ComputeBlobKZGProof(blob, Bytes48(commitment)) + proof, err := ComputeBlobKZGProof(&blob, Bytes48(commitment)) require.NoError(b, err) blobs[i] = blob @@ -413,19 +413,19 @@ func Benchmark(b *testing.B) { b.Run("BlobToKZGCommitment", func(b *testing.B) { for n := 0; n < b.N; n++ { - BlobToKZGCommitment(blobs[0]) + BlobToKZGCommitment(&blobs[0]) } }) b.Run("ComputeKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - ComputeKZGProof(blobs[0], fields[0]) + ComputeKZGProof(&blobs[0], fields[0]) } }) b.Run("ComputeBlobKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - ComputeBlobKZGProof(blobs[0], commitments[0]) + ComputeBlobKZGProof(&blobs[0], commitments[0]) } }) @@ -437,7 +437,7 @@ func Benchmark(b *testing.B) { b.Run("VerifyBlobKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - VerifyBlobKZGProof(blobs[0], commitments[0], proofs[0]) + VerifyBlobKZGProof(&blobs[0], commitments[0], proofs[0]) } }) From 4065f9d090c1fbfe2afa9c353a65bee889dcf287 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 22 Feb 2024 09:28:20 +0100 Subject: [PATCH 2/7] go: make return values by-ref Also make the code a bit more go-idiomatic, by returning nil when there is also an error returned. This commit also changes the error-check, to avoid map-lookups in the happy case. Also the map lookup was converted into a switch clause. The unmarshalling methods were changed to be less memory-intense, by decoding directly into the destination objects. --- bindings/go/main.go | 139 +++++++++++++++++++++++---------------- bindings/go/main_test.go | 62 ++++++++--------- 2 files changed, 114 insertions(+), 87 deletions(-) diff --git a/bindings/go/main.go b/bindings/go/main.go index 590c52016..7d85c92cb 100644 --- a/bindings/go/main.go +++ b/bindings/go/main.go @@ -6,10 +6,10 @@ package ckzg4844 import "C" import ( + "bytes" "encoding/hex" "errors" "fmt" - "strings" "unsafe" // So its functions are available during compilation. @@ -38,12 +38,6 @@ var ( ErrBadArgs = errors.New("bad arguments") ErrError = errors.New("unexpected error") ErrMalloc = errors.New("malloc failed") - errorMap = map[C.C_KZG_RET]error{ - C.C_KZG_OK: nil, - C.C_KZG_BADARGS: ErrBadArgs, - C.C_KZG_ERROR: ErrError, - C.C_KZG_MALLOC: ErrMalloc, - } ) /////////////////////////////////////////////////////////////////////////////// @@ -54,11 +48,17 @@ var ( // by the C library, into a proper Go error. If there is no error, this // will return nil. func makeErrorFromRet(ret C.C_KZG_RET) error { - err, ok := errorMap[ret] - if !ok { - panic(fmt.Sprintf("unexpected return value: %v", ret)) + switch ret { + case C.C_KZG_BADARGS: + return ErrBadArgs + case C.C_KZG_ERROR: + return ErrError + case C.C_KZG_MALLOC: + return ErrMalloc + case C.C_KZG_OK: + return nil } - return err + return fmt.Errorf("unexpected return value from c-library %v", ret) } /////////////////////////////////////////////////////////////////////////////// @@ -66,50 +66,53 @@ func makeErrorFromRet(ret C.C_KZG_RET) error { /////////////////////////////////////////////////////////////////////////////// func (b *Bytes32) UnmarshalText(input []byte) error { - inputStr := string(input) - if strings.HasPrefix(inputStr, "0x") { - inputStr = strings.TrimPrefix(inputStr, "0x") + if bytes.HasPrefix(input, []byte("0x")) { + input = input[2:] } - bytes, err := hex.DecodeString(inputStr) + if len(input) != 2*len(b) { + return ErrBadArgs + } + l, err := hex.Decode(b[:], input) if err != nil { return err } - if len(bytes) != len(b) { + if l != len(b) { return ErrBadArgs } - copy(b[:], bytes) return nil } func (b *Bytes48) UnmarshalText(input []byte) error { - inputStr := string(input) - if strings.HasPrefix(inputStr, "0x") { - inputStr = strings.TrimPrefix(inputStr, "0x") + if bytes.HasPrefix(input, []byte("0x")) { + input = input[2:] } - bytes, err := hex.DecodeString(inputStr) + if len(input) != 2*len(b) { + return ErrBadArgs + } + l, err := hex.Decode(b[:], input) if err != nil { return err } - if len(bytes) != len(b) { + if l != len(b) { return ErrBadArgs } - copy(b[:], bytes) return nil } func (b *Blob) UnmarshalText(input []byte) error { - inputStr := string(input) - if strings.HasPrefix(inputStr, "0x") { - inputStr = strings.TrimPrefix(inputStr, "0x") + if bytes.HasPrefix(input, []byte("0x")) { + input = input[2:] + } + if len(input) != 2*len(b) { + return ErrBadArgs } - bytes, err := hex.DecodeString(inputStr) + l, err := hex.Decode(b[:], input) if err != nil { return err } - if len(bytes) != len(b) { + if l != len(b) { return ErrBadArgs } - copy(b[:], bytes) return nil } @@ -147,6 +150,7 @@ func LoadTrustedSetup(g1Bytes, g2Bytes []byte) error { (C.size_t)(numG2Elements)) if ret == C.C_KZG_OK { loaded = true + return nil } return makeErrorFromRet(ret) } @@ -174,6 +178,7 @@ func LoadTrustedSetupFile(trustedSetupFile string) error { C.fclose(fp) if ret == C.C_KZG_OK { loaded = true + return nil } return makeErrorFromRet(ret) } @@ -200,16 +205,19 @@ BlobToKZGCommitment is the binding for: const Blob *blob, const KZGSettings *s); */ -func BlobToKZGCommitment(blob *Blob) (KZGCommitment, error) { +func BlobToKZGCommitment(blob *Blob) (*KZGCommitment, error) { if !loaded { panic("trusted setup isn't loaded") } - commitment := KZGCommitment{} + commitment := new(KZGCommitment) ret := C.blob_to_kzg_commitment( - (*C.KZGCommitment)(unsafe.Pointer(&commitment)), + (*C.KZGCommitment)(unsafe.Pointer(commitment)), (*C.Blob)(unsafe.Pointer(blob)), &settings) - return commitment, makeErrorFromRet(ret) + if ret != C.C_KZG_OK { + return nil, makeErrorFromRet(ret) + } + return commitment, nil } /* @@ -222,19 +230,22 @@ ComputeKZGProof is the binding for: const Bytes32 *z_bytes, const KZGSettings *s); */ -func ComputeKZGProof(blob *Blob, zBytes Bytes32) (KZGProof, Bytes32, error) { +func ComputeKZGProof(blob *Blob, zBytes *Bytes32) (*KZGProof, *Bytes32, error) { if !loaded { panic("trusted setup isn't loaded") } - proof := KZGProof{} - y := Bytes32{} + proof := new(KZGProof) + y := new(Bytes32) ret := C.compute_kzg_proof( - (*C.KZGProof)(unsafe.Pointer(&proof)), - (*C.Bytes32)(unsafe.Pointer(&y)), + (*C.KZGProof)(unsafe.Pointer(proof)), + (*C.Bytes32)(unsafe.Pointer(y)), (*C.Blob)(unsafe.Pointer(blob)), - (*C.Bytes32)(unsafe.Pointer(&zBytes)), + (*C.Bytes32)(unsafe.Pointer(zBytes)), &settings) - return proof, y, makeErrorFromRet(ret) + if ret != C.C_KZG_OK { + return nil, nil, makeErrorFromRet(ret) + } + return proof, y, nil } /* @@ -246,17 +257,21 @@ ComputeBlobKZGProof is the binding for: const Bytes48 *commitment_bytes, const KZGSettings *s); */ -func ComputeBlobKZGProof(blob *Blob, commitmentBytes Bytes48) (KZGProof, error) { +func ComputeBlobKZGProof(blob *Blob, commitmentBytes *Bytes48) (*KZGProof, error) { if !loaded { panic("trusted setup isn't loaded") } - proof := KZGProof{} + proof := new(KZGProof) ret := C.compute_blob_kzg_proof( - (*C.KZGProof)(unsafe.Pointer(&proof)), + (*C.KZGProof)(unsafe.Pointer(proof)), (*C.Blob)(unsafe.Pointer(blob)), - (*C.Bytes48)(unsafe.Pointer(&commitmentBytes)), + (*C.Bytes48)(unsafe.Pointer(commitmentBytes)), &settings) - return proof, makeErrorFromRet(ret) + + if ret != C.C_KZG_OK { + return nil, makeErrorFromRet(ret) + } + return proof, nil } /* @@ -270,19 +285,23 @@ VerifyKZGProof is the binding for: const Bytes48 *proof_bytes, const KZGSettings *s); */ -func VerifyKZGProof(commitmentBytes Bytes48, zBytes, yBytes Bytes32, proofBytes Bytes48) (bool, error) { +func VerifyKZGProof(commitmentBytes *Bytes48, zBytes, yBytes *Bytes32, proofBytes *Bytes48) (bool, error) { if !loaded { panic("trusted setup isn't loaded") } var result C.bool ret := C.verify_kzg_proof( &result, - (*C.Bytes48)(unsafe.Pointer(&commitmentBytes)), - (*C.Bytes32)(unsafe.Pointer(&zBytes)), - (*C.Bytes32)(unsafe.Pointer(&yBytes)), - (*C.Bytes48)(unsafe.Pointer(&proofBytes)), + (*C.Bytes48)(unsafe.Pointer(commitmentBytes)), + (*C.Bytes32)(unsafe.Pointer(zBytes)), + (*C.Bytes32)(unsafe.Pointer(yBytes)), + (*C.Bytes48)(unsafe.Pointer(proofBytes)), &settings) - return bool(result), makeErrorFromRet(ret) + + if ret != C.C_KZG_OK { + return false, makeErrorFromRet(ret) + } + return bool(result), nil } /* @@ -295,7 +314,7 @@ VerifyBlobKZGProof is the binding for: const Bytes48 *proof_bytes, const KZGSettings *s); */ -func VerifyBlobKZGProof(blob *Blob, commitmentBytes, proofBytes Bytes48) (bool, error) { +func VerifyBlobKZGProof(blob *Blob, commitmentBytes, proofBytes *Bytes48) (bool, error) { if !loaded { panic("trusted setup isn't loaded") } @@ -303,10 +322,14 @@ func VerifyBlobKZGProof(blob *Blob, commitmentBytes, proofBytes Bytes48) (bool, ret := C.verify_blob_kzg_proof( &result, (*C.Blob)(unsafe.Pointer(blob)), - (*C.Bytes48)(unsafe.Pointer(&commitmentBytes)), - (*C.Bytes48)(unsafe.Pointer(&proofBytes)), + (*C.Bytes48)(unsafe.Pointer(commitmentBytes)), + (*C.Bytes48)(unsafe.Pointer(proofBytes)), &settings) - return bool(result), makeErrorFromRet(ret) + + if ret != C.C_KZG_OK { + return false, makeErrorFromRet(ret) + } + return bool(result), nil } /* @@ -335,5 +358,9 @@ func VerifyBlobKZGProofBatch(blobs []Blob, commitmentsBytes, proofsBytes []Bytes *(**C.Bytes48)(unsafe.Pointer(&proofsBytes)), (C.size_t)(len(blobs)), &settings) - return bool(result), makeErrorFromRet(ret) + + if ret != C.C_KZG_OK { + return false, makeErrorFromRet(ret) + } + return bool(result), nil } diff --git a/bindings/go/main_test.go b/bindings/go/main_test.go index 79b325e0e..e90045a06 100644 --- a/bindings/go/main_test.go +++ b/bindings/go/main_test.go @@ -12,9 +12,9 @@ import ( ) func TestMain(m *testing.M) { - err := LoadTrustedSetupFile("../../src/trusted_setup.txt") - if err != nil { - panic("failed to load trusted setup") + + if err := LoadTrustedSetupFile("../../src/trusted_setup.txt"); err != nil { + panic(fmt.Sprintf("failed to load trusted setup: %v", err)) } defer FreeTrustedSetup() code := m.Run() @@ -40,13 +40,11 @@ func getRandFieldElement(seed int64) Bytes32 { return fieldElementBytes } -func getRandBlob(seed int64) Blob { - var blob Blob +func fillBlobRandom(blob *Blob, seed int64) { for i := 0; i < BytesPerBlob; i += BytesPerFieldElement { fieldElementBytes := getRandFieldElement(seed + int64(i)) copy(blob[i:i+BytesPerFieldElement], fieldElementBytes[:]) } - return blob } /////////////////////////////////////////////////////////////////////////////// @@ -84,14 +82,14 @@ func TestBlobToKZGCommitment(t *testing.T) { require.NoError(t, testFile.Close()) require.NoError(t, err) - var blob Blob + blob := new(Blob) err = blob.UnmarshalText([]byte(test.Input.Blob)) if err != nil { require.Nil(t, test.Output) return } - commitment, err := BlobToKZGCommitment(&blob) + commitment, err := BlobToKZGCommitment(blob) if err == nil { require.NotNil(t, test.Output) require.Equal(t, test.Output[:], commitment[:]) @@ -124,21 +122,21 @@ func TestComputeKZGProof(t *testing.T) { require.NoError(t, testFile.Close()) require.NoError(t, err) - var blob Blob + blob := new(Blob) err = blob.UnmarshalText([]byte(test.Input.Blob)) if err != nil { require.Nil(t, test.Output) return } - var z Bytes32 + var z = new(Bytes32) err = z.UnmarshalText([]byte(test.Input.Z)) if err != nil { require.Nil(t, test.Output) return } - proof, y, err := ComputeKZGProof(&blob, z) + proof, y, err := ComputeKZGProof(blob, z) if err == nil { require.NotNil(t, test.Output) var expectedProof Bytes48 @@ -178,21 +176,21 @@ func TestComputeBlobKZGProof(t *testing.T) { require.NoError(t, testFile.Close()) require.NoError(t, err) - var blob Blob + blob := new(Blob) err = blob.UnmarshalText([]byte(test.Input.Blob)) if err != nil { require.Nil(t, test.Output) return } - var commitment Bytes48 + var commitment = new(Bytes48) err = commitment.UnmarshalText([]byte(test.Input.Commitment)) if err != nil { require.Nil(t, test.Output) return } - proof, err := ComputeBlobKZGProof(&blob, commitment) + proof, err := ComputeBlobKZGProof(blob, commitment) if err == nil { require.NotNil(t, test.Output) require.Equal(t, test.Output[:], proof[:]) @@ -227,28 +225,28 @@ func TestVerifyKZGProof(t *testing.T) { require.NoError(t, testFile.Close()) require.NoError(t, err) - var commitment Bytes48 + var commitment = new(Bytes48) err = commitment.UnmarshalText([]byte(test.Input.Commitment)) if err != nil { require.Nil(t, test.Output) return } - var z Bytes32 + var z = new(Bytes32) err = z.UnmarshalText([]byte(test.Input.Z)) if err != nil { require.Nil(t, test.Output) return } - var y Bytes32 + var y = new(Bytes32) err = y.UnmarshalText([]byte(test.Input.Y)) if err != nil { require.Nil(t, test.Output) return } - var proof Bytes48 + var proof = new(Bytes48) err = proof.UnmarshalText([]byte(test.Input.Proof)) if err != nil { require.Nil(t, test.Output) @@ -289,28 +287,28 @@ func TestVerifyBlobKZGProof(t *testing.T) { require.NoError(t, testFile.Close()) require.NoError(t, err) - var blob Blob + var blob = new(Blob) err = blob.UnmarshalText([]byte(test.Input.Blob)) if err != nil { require.Nil(t, test.Output) return } - var commitment Bytes48 + var commitment = new(Bytes48) err = commitment.UnmarshalText([]byte(test.Input.Commitment)) if err != nil { require.Nil(t, test.Output) return } - var proof Bytes48 + var proof = new(Bytes48) err = proof.UnmarshalText([]byte(test.Input.Proof)) if err != nil { require.Nil(t, test.Output) return } - valid, err := VerifyBlobKZGProof(&blob, commitment, proof) + valid, err := VerifyBlobKZGProof(blob, commitment, proof) if err == nil { require.NotNil(t, test.Output) require.Equal(t, *test.Output, valid) @@ -397,18 +395,20 @@ func Benchmark(b *testing.B) { blobs := [length]Blob{} commitments := [length]Bytes48{} proofs := [length]Bytes48{} - fields := [length]Bytes32{} + fields := [length]*Bytes32{} for i := 0; i < length; i++ { - blob := getRandBlob(int64(i)) + var blob Blob + fillBlobRandom(&blob, int64(i)) commitment, err := BlobToKZGCommitment(&blob) require.NoError(b, err) - proof, err := ComputeBlobKZGProof(&blob, Bytes48(commitment)) + proof, err := ComputeBlobKZGProof(&blob, (*Bytes48)(commitment)) require.NoError(b, err) blobs[i] = blob - commitments[i] = Bytes48(commitment) - proofs[i] = Bytes48(proof) - fields[i] = getRandFieldElement(int64(i)) + commitments[i] = Bytes48(*commitment) + proofs[i] = Bytes48(*proof) + field := getRandFieldElement(int64(i)) + fields[i] = &field } b.Run("BlobToKZGCommitment", func(b *testing.B) { @@ -425,19 +425,19 @@ func Benchmark(b *testing.B) { b.Run("ComputeBlobKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - ComputeBlobKZGProof(&blobs[0], commitments[0]) + ComputeBlobKZGProof(&blobs[0], &commitments[0]) } }) b.Run("VerifyKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - VerifyKZGProof(commitments[0], fields[0], fields[1], proofs[0]) + VerifyKZGProof(&commitments[0], fields[0], fields[1], &proofs[0]) } }) b.Run("VerifyBlobKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - VerifyBlobKZGProof(&blobs[0], commitments[0], proofs[0]) + VerifyBlobKZGProof(&blobs[0], &commitments[0], &proofs[0]) } }) From 445461ef6cf65c648c864d4ae24ffeaf448d76e3 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 22 Feb 2024 08:16:22 -0600 Subject: [PATCH 3/7] Revert array of fields back to values in benchmarks I just found it weird that this was the only array which was converted to pointers. When looking at the benchmark calls, I thought those parameters were being handled differently but they are not. --- bindings/go/main_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bindings/go/main_test.go b/bindings/go/main_test.go index e90045a06..00753646d 100644 --- a/bindings/go/main_test.go +++ b/bindings/go/main_test.go @@ -395,7 +395,7 @@ func Benchmark(b *testing.B) { blobs := [length]Blob{} commitments := [length]Bytes48{} proofs := [length]Bytes48{} - fields := [length]*Bytes32{} + fields := [length]Bytes32{} for i := 0; i < length; i++ { var blob Blob fillBlobRandom(&blob, int64(i)) @@ -407,8 +407,7 @@ func Benchmark(b *testing.B) { blobs[i] = blob commitments[i] = Bytes48(*commitment) proofs[i] = Bytes48(*proof) - field := getRandFieldElement(int64(i)) - fields[i] = &field + fields[i] = getRandFieldElement(int64(i)) } b.Run("BlobToKZGCommitment", func(b *testing.B) { @@ -419,7 +418,7 @@ func Benchmark(b *testing.B) { b.Run("ComputeKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - ComputeKZGProof(&blobs[0], fields[0]) + ComputeKZGProof(&blobs[0], &fields[0]) } }) @@ -431,7 +430,7 @@ func Benchmark(b *testing.B) { b.Run("VerifyKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - VerifyKZGProof(&commitments[0], fields[0], fields[1], &proofs[0]) + VerifyKZGProof(&commitments[0], &fields[0], &fields[1], &proofs[0]) } }) From 4b7e233d3109d356e6208e464b876b416fca916e Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 22 Feb 2024 08:28:50 -0600 Subject: [PATCH 4/7] Check that arguments aren't nil --- bindings/go/main.go | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/bindings/go/main.go b/bindings/go/main.go index 7d85c92cb..0b2782fb0 100644 --- a/bindings/go/main.go +++ b/bindings/go/main.go @@ -140,6 +140,7 @@ func LoadTrustedSetup(g1Bytes, g2Bytes []byte) error { if len(g2Bytes)%C.BYTES_PER_G2 != 0 { panic(fmt.Sprintf("len(g2Bytes) is not a multiple of %v", C.BYTES_PER_G2)) } + numG1Elements := len(g1Bytes) / C.BYTES_PER_G1 numG2Elements := len(g2Bytes) / C.BYTES_PER_G2 ret := C.load_trusted_setup( @@ -148,6 +149,7 @@ func LoadTrustedSetup(g1Bytes, g2Bytes []byte) error { (C.size_t)(numG1Elements), *(**C.uint8_t)(unsafe.Pointer(&g2Bytes)), (C.size_t)(numG2Elements)) + if ret == C.C_KZG_OK { loaded = true return nil @@ -209,11 +211,16 @@ func BlobToKZGCommitment(blob *Blob) (*KZGCommitment, error) { if !loaded { panic("trusted setup isn't loaded") } + if blob == nil { + return nil, ErrBadArgs + } + commitment := new(KZGCommitment) ret := C.blob_to_kzg_commitment( (*C.KZGCommitment)(unsafe.Pointer(commitment)), (*C.Blob)(unsafe.Pointer(blob)), &settings) + if ret != C.C_KZG_OK { return nil, makeErrorFromRet(ret) } @@ -234,14 +241,18 @@ func ComputeKZGProof(blob *Blob, zBytes *Bytes32) (*KZGProof, *Bytes32, error) { if !loaded { panic("trusted setup isn't loaded") } - proof := new(KZGProof) - y := new(Bytes32) + if blob == nil || zBytes == nil { + return nil, nil, ErrBadArgs + } + + proof, y := new(KZGProof), new(Bytes32) ret := C.compute_kzg_proof( (*C.KZGProof)(unsafe.Pointer(proof)), (*C.Bytes32)(unsafe.Pointer(y)), (*C.Blob)(unsafe.Pointer(blob)), (*C.Bytes32)(unsafe.Pointer(zBytes)), &settings) + if ret != C.C_KZG_OK { return nil, nil, makeErrorFromRet(ret) } @@ -261,6 +272,10 @@ func ComputeBlobKZGProof(blob *Blob, commitmentBytes *Bytes48) (*KZGProof, error if !loaded { panic("trusted setup isn't loaded") } + if blob == nil || commitmentBytes == nil { + return nil, ErrBadArgs + } + proof := new(KZGProof) ret := C.compute_blob_kzg_proof( (*C.KZGProof)(unsafe.Pointer(proof)), @@ -289,6 +304,10 @@ func VerifyKZGProof(commitmentBytes *Bytes48, zBytes, yBytes *Bytes32, proofByte if !loaded { panic("trusted setup isn't loaded") } + if commitmentBytes == nil || zBytes == nil || yBytes == nil || proofBytes == nil { + return false, ErrBadArgs + } + var result C.bool ret := C.verify_kzg_proof( &result, @@ -318,6 +337,10 @@ func VerifyBlobKZGProof(blob *Blob, commitmentBytes, proofBytes *Bytes48) (bool, if !loaded { panic("trusted setup isn't loaded") } + if blob == nil || commitmentBytes == nil || proofBytes == nil { + return false, ErrBadArgs + } + var result C.bool ret := C.verify_blob_kzg_proof( &result, From c7d6ae9835920e2cf4d18e3162d69f57a2321bdf Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 22 Feb 2024 08:38:31 -0600 Subject: [PATCH 5/7] Move C_KZG_OK to top in makeErrorFromRet This is just a nit, but it matches the order defined in C. --- bindings/go/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/go/main.go b/bindings/go/main.go index 0b2782fb0..8892ad95c 100644 --- a/bindings/go/main.go +++ b/bindings/go/main.go @@ -49,14 +49,14 @@ var ( // will return nil. func makeErrorFromRet(ret C.C_KZG_RET) error { switch ret { + case C.C_KZG_OK: + return nil case C.C_KZG_BADARGS: return ErrBadArgs case C.C_KZG_ERROR: return ErrError case C.C_KZG_MALLOC: return ErrMalloc - case C.C_KZG_OK: - return nil } return fmt.Errorf("unexpected return value from c-library %v", ret) } From 8ed9355bc0d9ef11366dff3f33c3de6a8c80acf1 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 22 Feb 2024 15:22:29 -0600 Subject: [PATCH 6/7] Remove C_KZG_OK from makeErrorFromRet --- bindings/go/main.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/bindings/go/main.go b/bindings/go/main.go index 8892ad95c..72590523a 100644 --- a/bindings/go/main.go +++ b/bindings/go/main.go @@ -45,12 +45,10 @@ var ( /////////////////////////////////////////////////////////////////////////////// // makeErrorFromRet translates an (integral) return value, as reported -// by the C library, into a proper Go error. If there is no error, this -// will return nil. +// by the C library, into a proper Go error. This function should only be +// called when there is an error, not with C_KZG_OK. func makeErrorFromRet(ret C.C_KZG_RET) error { switch ret { - case C.C_KZG_OK: - return nil case C.C_KZG_BADARGS: return ErrBadArgs case C.C_KZG_ERROR: @@ -58,7 +56,7 @@ func makeErrorFromRet(ret C.C_KZG_RET) error { case C.C_KZG_MALLOC: return ErrMalloc } - return fmt.Errorf("unexpected return value from c-library %v", ret) + return fmt.Errorf("unexpected error from c-library: %v", ret) } /////////////////////////////////////////////////////////////////////////////// From c0abbd65c6e801fb8b403bf50d22a4039387478e Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Wed, 6 Mar 2024 14:50:06 +0100 Subject: [PATCH 7/7] bindings/go: only use pass-by-ref for blobs --- bindings/go/main.go | 69 +++++++++++++++++++--------------------- bindings/go/main_test.go | 30 ++++++++--------- 2 files changed, 47 insertions(+), 52 deletions(-) diff --git a/bindings/go/main.go b/bindings/go/main.go index 72590523a..bf344a1d5 100644 --- a/bindings/go/main.go +++ b/bindings/go/main.go @@ -138,7 +138,6 @@ func LoadTrustedSetup(g1Bytes, g2Bytes []byte) error { if len(g2Bytes)%C.BYTES_PER_G2 != 0 { panic(fmt.Sprintf("len(g2Bytes) is not a multiple of %v", C.BYTES_PER_G2)) } - numG1Elements := len(g1Bytes) / C.BYTES_PER_G1 numG2Elements := len(g2Bytes) / C.BYTES_PER_G2 ret := C.load_trusted_setup( @@ -147,7 +146,6 @@ func LoadTrustedSetup(g1Bytes, g2Bytes []byte) error { (C.size_t)(numG1Elements), *(**C.uint8_t)(unsafe.Pointer(&g2Bytes)), (C.size_t)(numG2Elements)) - if ret == C.C_KZG_OK { loaded = true return nil @@ -205,22 +203,22 @@ BlobToKZGCommitment is the binding for: const Blob *blob, const KZGSettings *s); */ -func BlobToKZGCommitment(blob *Blob) (*KZGCommitment, error) { +func BlobToKZGCommitment(blob *Blob) (KZGCommitment, error) { if !loaded { panic("trusted setup isn't loaded") } if blob == nil { - return nil, ErrBadArgs + return KZGCommitment{}, ErrBadArgs } - commitment := new(KZGCommitment) + var commitment KZGCommitment ret := C.blob_to_kzg_commitment( - (*C.KZGCommitment)(unsafe.Pointer(commitment)), + (*C.KZGCommitment)(unsafe.Pointer(&commitment)), (*C.Blob)(unsafe.Pointer(blob)), &settings) if ret != C.C_KZG_OK { - return nil, makeErrorFromRet(ret) + return KZGCommitment{}, makeErrorFromRet(ret) } return commitment, nil } @@ -235,24 +233,26 @@ ComputeKZGProof is the binding for: const Bytes32 *z_bytes, const KZGSettings *s); */ -func ComputeKZGProof(blob *Blob, zBytes *Bytes32) (*KZGProof, *Bytes32, error) { +func ComputeKZGProof(blob *Blob, zBytes Bytes32) (KZGProof, Bytes32, error) { if !loaded { panic("trusted setup isn't loaded") } - if blob == nil || zBytes == nil { - return nil, nil, ErrBadArgs + if blob == nil { + return KZGProof{}, Bytes32{}, ErrBadArgs } - - proof, y := new(KZGProof), new(Bytes32) + var ( + proof KZGProof + y Bytes32 + ) ret := C.compute_kzg_proof( - (*C.KZGProof)(unsafe.Pointer(proof)), - (*C.Bytes32)(unsafe.Pointer(y)), + (*C.KZGProof)(unsafe.Pointer(&proof)), + (*C.Bytes32)(unsafe.Pointer(&y)), (*C.Blob)(unsafe.Pointer(blob)), - (*C.Bytes32)(unsafe.Pointer(zBytes)), + (*C.Bytes32)(unsafe.Pointer(&zBytes)), &settings) if ret != C.C_KZG_OK { - return nil, nil, makeErrorFromRet(ret) + return KZGProof{}, Bytes32{}, makeErrorFromRet(ret) } return proof, y, nil } @@ -266,23 +266,22 @@ ComputeBlobKZGProof is the binding for: const Bytes48 *commitment_bytes, const KZGSettings *s); */ -func ComputeBlobKZGProof(blob *Blob, commitmentBytes *Bytes48) (*KZGProof, error) { +func ComputeBlobKZGProof(blob *Blob, commitmentBytes Bytes48) (KZGProof, error) { if !loaded { panic("trusted setup isn't loaded") } - if blob == nil || commitmentBytes == nil { - return nil, ErrBadArgs + if blob == nil { + return KZGProof{}, ErrBadArgs } - - proof := new(KZGProof) + var proof KZGProof ret := C.compute_blob_kzg_proof( - (*C.KZGProof)(unsafe.Pointer(proof)), + (*C.KZGProof)(unsafe.Pointer(&proof)), (*C.Blob)(unsafe.Pointer(blob)), - (*C.Bytes48)(unsafe.Pointer(commitmentBytes)), + (*C.Bytes48)(unsafe.Pointer(&commitmentBytes)), &settings) if ret != C.C_KZG_OK { - return nil, makeErrorFromRet(ret) + return KZGProof{}, makeErrorFromRet(ret) } return proof, nil } @@ -298,21 +297,17 @@ VerifyKZGProof is the binding for: const Bytes48 *proof_bytes, const KZGSettings *s); */ -func VerifyKZGProof(commitmentBytes *Bytes48, zBytes, yBytes *Bytes32, proofBytes *Bytes48) (bool, error) { +func VerifyKZGProof(commitmentBytes Bytes48, zBytes, yBytes Bytes32, proofBytes Bytes48) (bool, error) { if !loaded { panic("trusted setup isn't loaded") } - if commitmentBytes == nil || zBytes == nil || yBytes == nil || proofBytes == nil { - return false, ErrBadArgs - } - var result C.bool ret := C.verify_kzg_proof( &result, - (*C.Bytes48)(unsafe.Pointer(commitmentBytes)), - (*C.Bytes32)(unsafe.Pointer(zBytes)), - (*C.Bytes32)(unsafe.Pointer(yBytes)), - (*C.Bytes48)(unsafe.Pointer(proofBytes)), + (*C.Bytes48)(unsafe.Pointer(&commitmentBytes)), + (*C.Bytes32)(unsafe.Pointer(&zBytes)), + (*C.Bytes32)(unsafe.Pointer(&yBytes)), + (*C.Bytes48)(unsafe.Pointer(&proofBytes)), &settings) if ret != C.C_KZG_OK { @@ -331,11 +326,11 @@ VerifyBlobKZGProof is the binding for: const Bytes48 *proof_bytes, const KZGSettings *s); */ -func VerifyBlobKZGProof(blob *Blob, commitmentBytes, proofBytes *Bytes48) (bool, error) { +func VerifyBlobKZGProof(blob *Blob, commitmentBytes, proofBytes Bytes48) (bool, error) { if !loaded { panic("trusted setup isn't loaded") } - if blob == nil || commitmentBytes == nil || proofBytes == nil { + if blob == nil { return false, ErrBadArgs } @@ -343,8 +338,8 @@ func VerifyBlobKZGProof(blob *Blob, commitmentBytes, proofBytes *Bytes48) (bool, ret := C.verify_blob_kzg_proof( &result, (*C.Blob)(unsafe.Pointer(blob)), - (*C.Bytes48)(unsafe.Pointer(commitmentBytes)), - (*C.Bytes48)(unsafe.Pointer(proofBytes)), + (*C.Bytes48)(unsafe.Pointer(&commitmentBytes)), + (*C.Bytes48)(unsafe.Pointer(&proofBytes)), &settings) if ret != C.C_KZG_OK { diff --git a/bindings/go/main_test.go b/bindings/go/main_test.go index 00753646d..99d1f3bd2 100644 --- a/bindings/go/main_test.go +++ b/bindings/go/main_test.go @@ -129,7 +129,7 @@ func TestComputeKZGProof(t *testing.T) { return } - var z = new(Bytes32) + var z Bytes32 err = z.UnmarshalText([]byte(test.Input.Z)) if err != nil { require.Nil(t, test.Output) @@ -183,7 +183,7 @@ func TestComputeBlobKZGProof(t *testing.T) { return } - var commitment = new(Bytes48) + var commitment Bytes48 err = commitment.UnmarshalText([]byte(test.Input.Commitment)) if err != nil { require.Nil(t, test.Output) @@ -225,28 +225,28 @@ func TestVerifyKZGProof(t *testing.T) { require.NoError(t, testFile.Close()) require.NoError(t, err) - var commitment = new(Bytes48) + var commitment Bytes48 err = commitment.UnmarshalText([]byte(test.Input.Commitment)) if err != nil { require.Nil(t, test.Output) return } - var z = new(Bytes32) + var z Bytes32 err = z.UnmarshalText([]byte(test.Input.Z)) if err != nil { require.Nil(t, test.Output) return } - var y = new(Bytes32) + var y Bytes32 err = y.UnmarshalText([]byte(test.Input.Y)) if err != nil { require.Nil(t, test.Output) return } - var proof = new(Bytes48) + var proof Bytes48 err = proof.UnmarshalText([]byte(test.Input.Proof)) if err != nil { require.Nil(t, test.Output) @@ -294,14 +294,14 @@ func TestVerifyBlobKZGProof(t *testing.T) { return } - var commitment = new(Bytes48) + var commitment Bytes48 err = commitment.UnmarshalText([]byte(test.Input.Commitment)) if err != nil { require.Nil(t, test.Output) return } - var proof = new(Bytes48) + var proof Bytes48 err = proof.UnmarshalText([]byte(test.Input.Proof)) if err != nil { require.Nil(t, test.Output) @@ -401,12 +401,12 @@ func Benchmark(b *testing.B) { fillBlobRandom(&blob, int64(i)) commitment, err := BlobToKZGCommitment(&blob) require.NoError(b, err) - proof, err := ComputeBlobKZGProof(&blob, (*Bytes48)(commitment)) + proof, err := ComputeBlobKZGProof(&blob, Bytes48(commitment)) require.NoError(b, err) blobs[i] = blob - commitments[i] = Bytes48(*commitment) - proofs[i] = Bytes48(*proof) + commitments[i] = Bytes48(commitment) + proofs[i] = Bytes48(proof) fields[i] = getRandFieldElement(int64(i)) } @@ -418,25 +418,25 @@ func Benchmark(b *testing.B) { b.Run("ComputeKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - ComputeKZGProof(&blobs[0], &fields[0]) + ComputeKZGProof(&blobs[0], fields[0]) } }) b.Run("ComputeBlobKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - ComputeBlobKZGProof(&blobs[0], &commitments[0]) + ComputeBlobKZGProof(&blobs[0], commitments[0]) } }) b.Run("VerifyKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - VerifyKZGProof(&commitments[0], &fields[0], &fields[1], &proofs[0]) + VerifyKZGProof(commitments[0], fields[0], fields[1], proofs[0]) } }) b.Run("VerifyBlobKZGProof", func(b *testing.B) { for n := 0; n < b.N; n++ { - VerifyBlobKZGProof(&blobs[0], &commitments[0], &proofs[0]) + VerifyBlobKZGProof(&blobs[0], commitments[0], proofs[0]) } })