core, trie: unify keccaking

[pmikolajczyk41]

Context

Keccak hashing algorithm can be used in two ways: (1) as a one-shot hash function (we are just interested in hash of the data that we already have in full) or (2) as a cryptographic sponge (for cases where we don't know all the data to be hashed yet, we need some intermediate hashes, etc.)

From the caller perspective, (1) is a completely stateless process, but (2) requires keeping some state.

The crypto package exposes Keccak256Hash() and Keccak256() functions realizing (1) and NewKeccakState() for (2).

Problem

There are ~20 places where we use stateful approach (2) for (1). In other words, we:

• use NewKeccakState() to create the sponge state
• (optionally) keep it as a field
• use lower-level methods on this object to compute the hash (Write(), Read(), Sum())
  whereas we could (and should) just use Keccak256() utility.

HOWEVER, on the other hand, we want to keep diff between our fork and upstream repo as small as possible.

Motivation

When running geth within ZK VM (like SP1) it is highly beneficial to outsource heavy computation (like Keccak) to the dedicated VM precompiles. By removing all unnecessarily stateful hashing and reusing the Keccak256Hash() and Keccak256() utilities, it is way easier to replace all keccak invocations than to worry that some of them will persist in this strange form.

Proposed changes

Use stateless keccaking only in those places that are accessible by replay.wasm: trie/ and core/.

Testing method

In the history of this branch (and its nitro companion) you will find that I added panic to every place with 'incorrect' keccaking. Then I was running nitro system tests from block_validator_test.go. This way I confirmed what @tsahee anticipated - the only required changes are in trie/ and core/.

---

pulled in by OffchainLabs/nitro#4128
closes NIT-4123
upstream ethereum/go-ethereum#33222 (in a bit extended version) was rejected as non-essential

[tsahee]

So.. after a lot more thinking and reading, I have yet another conclusion.
At least some of what they're currently doing is very intentional and could actualy improve performance for a native node. See for example this PR in geth that reduces allocations by introducing hashTo: ethereum/go-ethereum#30743

I think that using an existing hasher, and certainly existing buffers, could improve performance and we don't want to give up on that.

So what I suggest is something like:
create in crypto package a new function called NewLegacyKeccak256.

This function, in non-wasm mode will simply call sha3.NewLegacyKeccak256
In wasm mode, this will return a a special struct that implements crypto KeccakState interface, and just stores all written buffers internally on a write, and on a read calls Keccak256Hash on it's internal buffer.

Then, in places we need to - (including inside crypto package) - replace calls to sha3.NewLegacyKeccak256 with calls to crypto.NewLegacyKeccak256.

This will not change what happends in native mode, and will allow us to unify and use precompile in wasm mode.

[tsahee]

could we avoid these (and similar import-order-only) changes so they won't turn up in future merges?

[tsahee]

these as well