crypto/keccak: vendor in golang.org/x/crypto/sha3

[fjl]

The upstream libray has removed the assembly-based implementation of keccak. We need to maintain our own library to avoid a peformance regression.

[gballet]

Should both go.mod not be updated in order to remove the reference to the other package?

[fjl]

We use some other packages from golang.org/x/crypto still

[rvagg]

Since go-ethereum isn't the only project impacted by this (Filecoin is too, there's likely many more), perhaps a separate repo to maintain the extracted package would be good? It could even be a shared concern among those of us that depend on it so we can collectively maintain it.

[cuiweixie]

will this be merged?

[cuiweixie]

any update? fork golang.org/x/crypto/sha3 or using this vendor?

[fjl]

Sorry for the non-progress. I have considered it a bit more, and we could make another repository for this code. For most crypto code used by Ethereum, we vendored the implementation into the repo because it makes it easier to pin to a very specific version and perform security fixes. It seems less necessary to vendor it here for sha3 because it is very stable.

However, creating a fork repo would have the downside that someone has to be the maintainer of it. I'm not looking forward to being that person. I'd honestly be fine if go-ethereum just had its own copy that we never have to touch until Go changes the assembler interface again.

[fjl]

@Kubuxu @rvagg wdyt?

[rvagg]

@fjl I was about to pull the trigger on putting one in the filecoin-project org, it'd be more prominent if it lived in the ethereum org--you'd be more likely to have downstream users of it, and you'd get other contributors showing up to update things as interfaces break of Go versions need updating etc. Of course that creates its own burden. We have a bunch of automated tooling to keep various things like Go version, linting etc. updated that we'd plug in, but as you said, I wouldn't expect much churn for sha3. Happy to go either way on this but we're probably not vendoring it in to individual codebases because we have a few separate ones that'll need it.

[rvagg]

@fjl we set up a package with the code extracted, coupled up to our CI to keep it fresh: https://github.com/filecoin-project/go-keccak, you're welcome to use it if you like.
Drop in for us @ filecoin-project/lotus#13477 & filecoin-project/go-f3#1064 so far.

[lightclient]

After further discussion, we decided to vendor it into go-ethereum. Ultimately, keccak is an important dependency for us and we want to stay on top of it. Sorry @rvagg for being so indecisive.

--

Was able to verify that keccakf.go, keccakf_amd64.go, keccakf_amd64.s, and sha3.go are identical matches to golang.org/x/crypto@v0.43.0, the version right before the asm was removed.

$ gh api "repos/golang/crypto/contents/sha3/keccakf.go?ref=v0.43.0" --jq '.content' | base64 -d > /tmp/upstream_keccakf.go
$ gh api "repos/golang/crypto/contents/sha3/keccakf_amd64.go?ref=v0.43.0" --jq '.content' | base64 -d > /tmp/upstream_keccakf_amd64.go
$ gh api "repos/golang/crypto/contents/sha3/keccakf_amd64.s?ref=v0.43.0" --jq '.content' | base64 -d > /tmp/upstream_keccakf_amd64.s
$ gh api 'repos/golang/crypto/contents/sha3/sha3.go?ref=v0.43.0' --jq '.content' | base64 -d > /tmp/upstream_sha3.go

$ diff -u /tmp/upstream_keccakf_amd64.go crypto/keccak/keccakf_amd64.go
$ diff -u /tmp/upstream_keccakf_amd64.s crypto/keccak/keccakf_amd64.s
$ diff -u /tmp/upstream_legacy_keccakf.go crypto/keccak/keccakf.go
$ diff -u /tmp/upstream_sha3.go crypto/keccak/sha3.go

The hashes.go and sha3_test.go just consists of deletions of unrelated sha3 code.

$ gh api "repos/golang/crypto/contents/sha3/hashes.go?ref=v0.43.0" --jq '.content' | base64 -d > /tmp/upstream_hashes.go
$ gh api "repos/golang/crypto/contents/sha3/sha3_test.go?ref=v0.43.0" --jq '.content' | base64 -d > /tmp/upstream_sha3_test.go

$ diff -u /tmp/upstream_hashes.go crypto/keccak/hashes.go
$ diff -u /tmp/upstream_sha3_test.go crypto/keccak/sha3_test.go

You can check that the asm optimization is still working by running:

go test ./crypto/keccak/... -bench=. -benchmem -count=5 | tee bench_asm.txt
go test -tags=purego ./crypto/keccak/... -bench=. -benchmem -count=5 | tee bench_purego.txt
benchstat bench_purego.txt bench_asm.txt

And all internal uses were successfully ported to our vendored packaged.

$ grep -r 'golang.org/x/crypto/sha3' --include='*.go' .
$

[Sahil-4555]

Since we’re maintaining a vendored keccak now, has anyone looked at fastkeccak?
It seems to provide additional optimizations. Might be worth benchmarking against the current vendored sha3 implementation.