Add get_merkle_root

[hwwhww]

What was wrong?

Replace #1674
Fix #1673
It's #1674 without get_updated_merkle_accumulator

How was it fixed?

• Refactor eth._utils.merkle APIs
• Add get_merkle_root

Cute Animal Picture

[ChihChengLiang]

I might miss some historical discussion. But is Hashable what we want here?
According to typing documentation, Hashable is an alias to collections.abc.Hashable, which checks if an object has the abstract method __hash__. That is in the sense of Python hash rather then if a Serializable object could be hash_eth2-ed or tree-hashed.

[hwwhww]

Ohh I changed it to Hashable just for resolving the lint error with merkle utils.
eth_hash.auto.keccak takes Union[bytes, bytearray] as input. Could we just replace Hashable with Union[bytes, bytearray]? Ping @jannikluhn to confirm it. 🙏

[jannikluhn]

Agree to Hashable --> Union[bytes, bytearray] (even though bytearray doesn't seem to be used in py-evm much, so just bytes might be fine too).

[pipermerriam]

IIUC this is reversing the iterable. I think reversed(...) reads a lot more naturally.

[djrtwo]

In what context do we expect to use get_merkle_root vs calc_merkle_root?

[hwwhww]

@djrtwo this module was written for proof of custody a long time ago. calc_merkle_root was the old one, and get_merkle_root is what we need for the beacon chain spec.

• calc_merkle_root(items):
  • takes "items" - the items before being hashed
  • does a leaves = tuple(hash_eth2(item) for item in items) ( in calc_merkle_tree).
• get_merkle_root:
  • takes "leaves" - the hashed items
  • merkleizes these leaves. In our case, leaves is latest_block_roots.

Not sure if we will need calc_merkle_root in the future...

[djrtwo]

Right. So do we want to leave both in for now?
The naming and functionality are very similar and might cause confusion.

That said, I'm not too opposed to leaving in the old function..

[jannikluhn]

We don't have to use cast here, MerkleTree(tuple(...)) works as well (and mypy can check that the argument has the right type).

[jannikluhn]

This expression is getting quite long. Can we maybe split it up, e.g.:

reversed_tree = tuple(take(n_layers, iterate(_hash_layer, leaves)))
return MerkleTree(tuple(reversed(reversed_tree)))

[hwwhww]

@djrtwo
I renamed calc_merkle_root to get_merkle_root_from_items for distingushing. I think it's okay to leave it, for now.

@jannikluhn
Oh, just found that we missed tests/core/test_blob_utils.py in master branch (https://github.com/jannikluhn/py-evm/blob/048f0df597b56ae0af47df661e3572dcd02b1571/tests/core/test_blob_utils.py).

[djrtwo]

lgtm