Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid Hash <> hash() type and func confusion #1478

Merged
merged 2 commits into from
Nov 12, 2019
Merged

Avoid Hash <> hash() type and func confusion #1478

merged 2 commits into from
Nov 12, 2019

Conversation

protolambda
Copy link
Contributor

Why:

  • hash() is the SHA-256 of empty input. Hash() is the default instantiation of the Bytes32 alias, i.e. 32 zeroed bytes.
  • Hash(*input*) casts the input to the Bytes32 alias (not changing the contents!). Whereas hash(*input*) hashes the input to a Hash (with SHA-256).

Changelog (no functional changes):

  • remove Hash alias of Bytes32
  • introduce Root alias of Bytes32
  • change roots to Root
  • change non-roots to Bytes32

Difficult choices:

  • a Bytes32 inside a proof is technically also a combination of nodes. But not necessarily so for the very bottom sibling. So not a Root or Hash
    • Also, Hash is especially confusing, as a leaf item is packed into 32 bytes but not hashed.
  • There are a bunch of hashes that are not SSZ roots that we start calling Bytes32 now:
    • Eth1 block hash
    • Randao mix / seeds
    • withdrawal_credentials

I am opposed to making a separation between SigningRoot and HashRoot: the roots should be typed by the (summary) type they represent, not the function that is used.

signing_root already is a hack to make the same object definition work for multiple purposes. But really signatures shouldn't be part of the tree, unless intended so. A SSZ type simply cannot be represented by the same immutable cached (each node) tree, covering both signing-root and hash-tree-root, as the tree structure is dfifferent.
And since we are using signing_root and hash_tree_root pretty much exclusively, I would prefer to just do away with signing_root. Maybe in favor of a way to represent a container with a signature as a serialization-only thing.

Open for suggestions/comments.

@protolambda protolambda added the general:presentation Presentation (as opposed to content) label Nov 12, 2019
@ralexstokes
Copy link
Member

changes look sound @protolambda -- can you elaborate on what you mean by "do away with signing_root"?

@protolambda
Copy link
Contributor Author

@ralexstokes As in completely remove it. With more type definitions it's technically not necessary. And it's difficult to fit one type that can be merkleized in two different forms onto a single binary tree.

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one minor change

test_libs/pyspec/eth2spec/test/sanity/test_blocks.py Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:presentation Presentation (as opposed to content)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants