Modify hash_to_bls_field logic

[hwwhww]

Address #2980: replace hash_tree_root(x) with hash(ssz_serialize(x)).

[dankrad]

I understand that this improves performance by 2x. This is due to the fact that sha256 of 232 bytes hashes a total of 2 blocks, which is 432 bytes; if this were not the case, hash_tree_root would have about the same performance as a plain hash, but it is much more parallelizable.

But it would be important to know the baseline that we are comparing this to.

So far we have had a rule that we don't use serialization inside the spec (though the merge might break that rule already, haven't checked).

[hwwhww]

@dankrad @asn-d6

So far we have had a rule that we don't use serialization inside the spec (though the merge might break that rule already, haven't checked).

FYI (dank)sharding spec uses ssz_serialize() in process_sharded_data function.

Also, since hash_to_bls_field was copied from sharding spec, if we decide to apply this PR, we may want to update sharding spec function as well. And either way, we should update the function name to something like hash_to_bls_field_with_challenge_number due to the name conflict.

[ppopth]

I think it's a good idea to put why we must not use hash_tree_root in the comment in the code. (probably put the issue link in the comment)

[hwwhww]

@ppopth

I think it's fine to add comments, but not sure about adding the issue link since we never add issue links to specs before. My life depends on git blame a lot though. 😅

[asn-d6]

LGTM!

[asn-d6]

Merged this as is! Thanks for the patch @hwwhww . For more details on how this impacts performance see #2980 (comment)

WRT the sharding spec changes suggested in #2981 (comment), I think we should handle these in a different ticket. Both for the sake of progressing this more-urgent ticket faster, but also because I think there is gonna be a big banch of 4844 changes that should be folded in the sharding spec (since 4844 became executable).

WRT the comment suggestion in #2981 (comment), I think it's OK to leave it as is. I think comments about design choices that were not taken might confuse code readers who are usually interested in the design choices that were taken (also, as @hwwhww said, a git-blame should bring them here. Hey!).