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

Using hash_tree_root for DepositData #31

Merged
merged 3 commits into from
Apr 22, 2019
Merged

Using hash_tree_root for DepositData #31

merged 3 commits into from
Apr 22, 2019

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Apr 19, 2019

What was wrong?

Fix #29
Goes along with spec PR #924

How was it fixed?

  1. Per Using hash_tree_root for DepositData #29 (comment) suggestion, the API now is: def deposit(pubkey: bytes[48], withdrawal_credentials: bytes[32], signature: bytes[96])
  2. Add minimal_ssz.py in this repo to verify tree hashing, but I suppose we will either use updated py-ssz or move deposit contract back to eth2.0-specs.

Gas used of the first 10 deposits

Before

deposit transaction consumes 144374 gas
deposit transaction consumes 116018 gas
deposit transaction consumes 114448 gas
deposit transaction consumes 117662 gas
deposit transaction consumes 114448 gas
deposit transaction consumes 116092 gas
deposit transaction consumes 114458 gas
deposit transaction consumes 119370 gas
deposit transaction consumes 114448 gas
deposit transaction consumes 116028 gas

After

deposit transaction consumes 187771 gas
deposit transaction consumes 144415 gas
deposit transaction consumes 142781 gas
deposit transaction consumes 146059 gas
deposit transaction consumes 142781 gas
deposit transaction consumes 144425 gas
deposit transaction consumes 142791 gas
deposit transaction consumes 147703 gas
deposit transaction consumes 142781 gas
deposit transaction consumes 144425 gas

Increased ~28,000 gas :'(

Cute Animal Picture

IMG_1033

Copy link
Collaborator

@NIC619 NIC619 left a comment

Choose a reason for hiding this comment

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

LGTM!

tests/contracts/test_deposit.py Outdated Show resolved Hide resolved
@djrtwo djrtwo merged commit b529796 into dev Apr 22, 2019
@djrtwo djrtwo deleted the hash_tree_deposit_data branch April 22, 2019 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using hash_tree_root for DepositData
3 participants