Closes #2241: Use Keccak-256 from golang.org/x/crypto/sha3 and mention explicitly

[jimenezrick]

First of all, apologies for this long PR.

These are the improvements I propose:

• Use latest version of golang.org/x/crypto/sha3 with a small patch to expose Keccak-256 and update the old copied code. The upstream version of sha3 is a bit faster.
• Rename all the misleading occurrences of SHA-3 to Keccak-256
• Remove some bits of dead/old code I saw while doing this

Before merging there are two things that need to be taken care:

• The golang.org/x/crypto fork (with just this modification: https://github.com/jimenezrick/crypto/commit/8802e1c340c41c8d290e6315609cfbcbdf1b6229) should be hosted somewhere, it could be the Ethereum Github: something like github.com/ethereum/go-crypto - that new import path could be used instead
• The Go code in https://github.com/ethereum/ethash needs to be updated remove references to: crypto.Sha3{,Hash}()

[robotally]

Vote	Count	Reviewers
👍	2	@fjl @obscuren
👎	0

Updated: Wed Feb 24 11:57:05 UTC 2016

[jimenezrick]

The builds will fail until the necessary update to ethash I mentioned before is applied.

[fjl]

This "unhelpful comment" is the Godoc synopsis of package common and should not be deleted.

[jimenezrick]

No problem, line preserved.

[fjl]

While I certainly appreciate the effort you took to clean up the code, there are a few issues with the PR that I would like to see resolved. The main issue is that we generally avoid vendoring a patched version of a dependency with godep, because it will break as soon as someone else updates the dependencies from upstream.

So far, we have treated crypto dependencies specially by vendoring them directly into our tree, for various reasons. I think it would be better to import the upstream changes into crypto/sha3 and then patch the version in our tree instead.

[jimenezrick]

Yeah, that sounds right. It's a bit of a mess to vendor in Go modified libraries. I can adapt the PR easily to do that instead.

[fjl]

For crypto/sha3, the last update from upstream happened in June 2015. Since then the only change has been to include the full text of the license in all files. Please make sure that they stay in place.

[fjl]

For the ethash issue, I think it might be better to preserve crypto.Sha3{,Hash} as an alias for now.
We can do a second pass and remove the alias later.

[fjl]

Ahh, right, one more thing: the commit convention in go-ethereum is to prefix commit messages with the package they modify, e.g. "bytes: remove dead code". If you do a change affecting a lot of packages, you can use prefix "all:". The renaming of both Sha3 and Sha3Hash can just be one commit.

[jimenezrick]

Roger that, cheers.

[fjl]

Thank you for contributing to go-ethereum.

[jimenezrick]

• I added an alias so that it doesn't break ethash
• Upstream sha3 code copied verbatim, including the LICENSE and PATENTS files and small modification to expose Keccak256
• Reorder of all the commits

@fjl thanks for the suggestions, it's way more clear now ;)

[codecov-io]

Current coverage is 50.76%

Merging #2242 into develop will increase coverage by +2.74% as of 1722c76

Powered by Codecov. Updated on successful CI builds.

[fjl]

👍

[obscuren]

Thanks for the PR @jimenezrick, appreciated. 👍