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

fix: do not change hashTreeRoot() #393

Merged
merged 3 commits into from
Aug 16, 2024

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Aug 16, 2024

Motivation

Don't want to modify hashTreeRoot() in any ways

Description

  • ListValidatorTreeViewDU: return super.commit() if it's from hashTreeRoot()
  • Revert BranchNodeStruct because we compute and apply validator roots in ListValidatorTreeViewDU for batchHashTreeRoot() flow
  • Update to the original benchmark to make sure there is no performance regression
 BeaconState ViewDU partially modified tree vc=200000 numModified=100000
    ✓ BeaconState ViewDU batchHashTreeRoot vc=200000                      7.316355 ops/s    136.6801 ms/op        -         85 runs   20.4 s
    ✓ BeaconState ViewDU batchHashTreeRoot - commit step vc=200000        8.522365 ops/s    117.3383 ms/op        -        101 runs   20.3 s
    ✓ BeaconState ViewDU batchHashTreeRoot - hash step vc=200000          38.30311 ops/s    26.10754 ms/op        -         76 runs   20.3 s
    ✓ BeaconState ViewDU hashTreeRoot() vc=200000                         2.260320 ops/s    442.4152 ms/op        -         39 runs   20.7 s
    ✓ BeaconState ViewDU hashTreeRoot - commit step vc=200000             25.73512 ops/s    38.85740 ms/op        -        165 runs   20.2 s
    ✓ BeaconState ViewDU hashTreeRoot - validator tree creation vc=100    7.122690 ops/s    140.3964 ms/op        -         82 runs   20.9 s

this is the same to https://hackmd.io/zj9N5RIqQfCqYz8Y1Xc_hA

@github-actions github-actions bot added the ssz label Aug 16, 2024
@twoeths twoeths changed the title Te/unchanged hash tree root fix: do not change hashTreeRoot() Aug 16, 2024
@twoeths twoeths marked this pull request as ready for review August 16, 2024 08:15
@twoeths twoeths requested a review from a team as a code owner August 16, 2024 08:15
@twoeths twoeths merged commit 04ed553 into te/batch_hash_tree_root Aug 16, 2024
9 of 10 checks passed
@twoeths twoeths deleted the te/unchanged_hash_tree_root branch August 16, 2024 08:16
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.

1 participant