chore: replace blake2b implementation by golang.org/x/crypto

[Jorropo]

In 2016-12-13 d8e61c69ab46ca38328da2f4995abaf93b252290 golang.org/x/crypto gained a blake2b AVX implementation.

There is also an AVX2 one.

It is faster than what we are using right now.

This commit allows for faster code and remove a dependency.

For future improvements (like NEON) /x/crypto seems more maintained (it isn't an archived repo).

benchmark              old ns/op     new ns/op     delta
BenchmarkSum128-12     252           140           -44.35%
BenchmarkSum1K-12      1221          986           -19.24%

benchmark              old MB/s     new MB/s     speedup
BenchmarkSum128-12     507.37       911.98       1.80x
BenchmarkSum1K-12      838.70       1038.43      1.24x

[marten-seemann]

This looks good to me, but I'll leave the final approval to someone who has more context with these hash functions.

[aschmahmann]

@Kubuxu any issues with merging?

@Jorropo not blocking here, but it'd be nice if there was some verification that the hashes from the different blake2 libraries are the same. AFAICT we don't have any here, either a Go test or adding to https://github.com/multiformats/multihash/ and bumping the dependency here would be great.

[marten-seemann]

not blocking here, but it'd be nice if there was some verification that the hashes from the different blake2 libraries are the same

That would need to live in a separate Go module though, otherwise the dependency would stay in go.mod. Or maybe a one-time test would be enough to convince us that the libraries are doing the same?

[Jorropo]

I'm surprised there is no blake2 tests, I'll add some, likely blake2's official test set.

[aschmahmann]

That would need to live in a separate Go module though, otherwise the dependency would stay in go.mod. Or maybe a one-time test would be enough to convince us that the libraries are doing the same?

I was thinking that if you did a PR with the tests separately from this one we could merge that, and then rebase this PR on top of master and make sure if passes. Since the tests would be hitting test vectors that are constant it'd be fine.

[Jorropo]

I dont think we actually need to check boths, blake2 is a spec if an implementation isnt following it its wrong and that likely a big security issue.

The tests can just compare to well known values.

[Kubuxu]

No issues from my side, I trust golang/x/crypto to implement Blake2 correctly.

[Jorropo]

/cc @aschmahmann I have added blake2b tests, tested both on the old and new implementation they both work.

RFM on my side

[Jorropo]

I trust golang/x/crypto to implement Blake2 correctly.

Fair enough, I belive the goal was more to ensure the new registering is done correctly.
Maybe one used to take bits now it takes byte sizes, or something weird like that.

[Jorropo]

@Kubuxu I have fixed the lint rule, could I pls get this merged so I can open the bump PR in go-ipfs ?

[Kubuxu]

Hey, tests seem to fail on the CI.
Sorry for the delay, I didn't know you were waiting on me.

[Jorropo]

@Kubuxu
Sorry for the tests I had fixed it locally it but forgot to push it.

I don't know if it's waiting on you, you are just the only one with merge permissions in the conversation afait.

[Jorropo]

Thx, sorry for pinging you Kubuxu, weird that Marten only show up as Contributor.