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

Use hash_tree_root to generate shard block body root #1980

Merged
merged 3 commits into from
Jul 29, 2020

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Jul 22, 2020

Issue

Address #1973 and replace #1975

#1973 (comment): No problem for the proof of custody or MPC-friendliness.

How did I fix it

  • Use hash_tree_root to generate the root instead of get_block_data_merkle_root
  • Include the mix-in length into the proof (similar to deposit data Merkle proof)

Include the mix-in length to the proof.
@hwwhww hwwhww force-pushed the hwwhww/use-hash-tree-root-for-shard-body branch from eb27a34 to e7f070d Compare July 23, 2020 18:09
@hwwhww hwwhww changed the base branch from dev to enable-bls-citest July 23, 2020 18:10
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.

awesome! few minor comments and questions

specs/phase1/custody-game.md Outdated Show resolved Hide resolved
tests/core/pyspec/eth2spec/test/helpers/custody.py Outdated Show resolved Hide resolved
data_branch = build_proof(custody_data_block.get_backing().get_left(), chunk_index + 2**spec.CUSTODY_RESPONSE_DEPTH)
leaf_index = chunk_index + 2**spec.CUSTODY_RESPONSE_DEPTH
serialized_length = (len(custody_data_block)).to_bytes(32, 'little')
data_branch = build_proof(custody_data_block.get_backing().get_left(), leaf_index) + [serialized_length]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the backing from remerkleable?

What exactly does get_left() give you here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

afaik the get_right() is the mix-in length and the left side is the block data.
@protolambda 's example here: https://gist.github.com/protolambda/8384b1305613d064ba962214d08396f0

We can use eth2spec.utils.merkle_minimal.calc_merkle_tree_from_leaves and get_merkle_proof to create the proof too. Hmm, if we want to include proof generation in v-guide, using get_merkle_proof helper would be simpler. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the validator guide, I think we should be explicit and not use underlying library structure functions.
As for the spec tests, I'm happy to use the remerkleable backings because they are probably much more efficient

hwwhww added 2 commits July 28, 2020 21:51
* Quick fix the configurations

* Remove the unused `CUSTODY_RESPONSE_DEADLINE`
@hwwhww hwwhww mentioned this pull request Jul 29, 2020
@djrtwo djrtwo merged commit 1ca4c25 into enable-bls-citest Jul 29, 2020
@djrtwo djrtwo deleted the hwwhww/use-hash-tree-root-for-shard-body branch July 29, 2020 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants