coinbase / kryptology

Apache License 2.0
855 stars 125 forks source link

Release 1.5.4 broken: `bls12-381/fp.go:53:2: undefined: mul` #30

Closed Olshansk closed 2 years ago

Olshansk commented 2 years ago

Seems like there is an issue with the bls12-381 package in the 1.5.4 release.

Using go get github.com/coinbase/kryptology@v1.5.3 in my working directory works, while go get github.com/coinbase/kryptology@v1.5.4 does not and results in the same errors as when I run the tests (see the image below).

I believe it is related to the fact that bls12-381/arithmetic_decl.go uses the package package bls12381.

According to https://go.dev/blog/package-names, package names should either be camelCase or snake_case and not be hyphenated. If it is hyphenated, the package name should replace - with _.

I spent about 40 minutes trying to simply rename everything, but the issue is a little more complex because of conflicting name dependencies and autogenerated vs source code, so I've decided to open this issue instead.

Screen Shot 2022-01-11 at 12 15 17 PM

mikelodder7 commented 2 years ago

I know we updated to go 1.17 between releases but I haven't seen that error before. I'll take a look to see if I can reproduce it.

Olshansk commented 2 years ago

@mikelodder7 Have you had a chance to try this out? Simply running the tests still fails for me.

mikelodder7 commented 2 years ago

I’m going to review in depth later this week. Hopefully can reproduce and adjust. Thanks for keeping on it

corverroos commented 2 years ago

I'm facing the same issue. It seems to be build tag related when running on arm64 M1 Mac, for the package to build you need to specify --tags=generic

pakkage: github.com/coinbase/kryptology/pkg/core/curves/native/bls12-381 tagged files:

git rev-parse HEAD

71ffd4cbf01951cd0ee056fc7b45b13ffb178330

go version

go version go1.17.6 darwin/arm64

go test ./...

github.com/coinbase/kryptology/pkg/core/curves/native/bls12-381 [github.com/coinbase/kryptology/pkg/core/curves/native/bls12-381.test]

pkg/core/curves/native/bls12-381/field_element.go:180:2: undefined: neg

...

go test -tags=generic ./...

ok github.com/coinbase/kryptology/cmd/benchcomp 0.382s

ok github.com/coinbase/kryptology/internal 0.603s

--- FAIL: Test_Membership (0.02s)

witness_test.go:70:

Error Trace: witness_test.go:70

Error: Received unexpected error:

invalid result

Test: Test_Membership

GOARCH=amd64 go test ./...

Only one failure and one timeout

--- FAIL: TestAugSigningG1 (0.02s)

tiny_bls_sig_aug_test.go:111: Aug Verify succeeded when it should've failed.

FAIL

panic: test timed out after 10m0s

FAIL github.com/coinbase/kryptology/pkg/tecdsa/gg20/participant 602.228s

mikelodder7 commented 2 years ago

Should be fixed in #49