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

Tree based spec SSZ implementation - remerkleable #1552

Merged
merged 1 commit into from
Jan 25, 2020
Merged

Conversation

protolambda
Copy link
Collaborator

@protolambda protolambda commented Jan 2, 2020

This mostly just deletes the old SSZ implementation from the spec.
And then if we link in this library: https://github.com/protolambda/remerkleable, it passes all spec tests.

The library is different from status quo in the following ways:

  • data-sharing at the core: every type is a view over a binary merkle tree.
  • supports typing over partial backings: just provide the branches of the tree that matter to the reads/writes on the object, and it works.
  • hash-function doesn't even show up in initial profiling, everything is cached in place
  • data accesses are more expensive, as it creates temporary views over subtrees. E.g. a validator view for each registry entry to get active validator indices.
    • Quadratic-complexity state accesses slow down things, even more than a minimal-configuration state hash-tree-root. Naively calling a function for every validator, that then calls a function for every validator, is a big slowdown. Normal eth2 clients wouldn't do this, but the spec does things like this in reward-computation.
  • For normal use (not the quadratic problem as described above), mainnet config is much more manageable for testing, state-hashes are fully cached.
  • Typing/functionality extensions and improvements over status quo spec implementation
  • Deserialization now fully supported.
  • Lots of other SSZ feature goodies from remerkleable to use in tooling.

And compared to py-ssz:

  • More readable types, pretty much the same syntax as status-quo spec.
  • Caching and data-sharing everything, with access to the binary tree (good for loading typed merkle proofs at 0 cost). As opposed to pyrsistent, the data-sharing library that py-ssz uses.
  • (de)serialization should be slightly faster, passing an stream around to read/write from.

PS: pymerkle is already taken as a name, pymerkles was a working name. Suggestions for a good name are welcome. Something that covers ssz / merkle partials better. Decided on a new name: remerkleable! A mix-up of "remarkable"/"re-merkle-able".

In addition, the new cached hash-tree-roots are very useful for memoization of spec functions, bring big performance improvements to the pyspec code.

@protolambda
Copy link
Collaborator Author

Need Python 3.8 for remerkleable typing features (as well as changes in phase 1 PR). Deposit tests are failing with 3.8 though, since vyper fails parsing when using the 3.8 AST code. Need to backport the PR fromt the vyper repo that fixed that.

@protolambda protolambda changed the title Tree based spec SSZ implementation [Proof Of Concept] Tree based spec SSZ implementation - remerkleable Jan 23, 2020
@protolambda
Copy link
Collaborator Author

Hmm pypi db index is slow, CI can't find v0.1.9 I just published. Will re-run after giving it some time to catch up

@protolambda protolambda marked this pull request as ready for review January 24, 2020 03:01
@protolambda
Copy link
Collaborator Author

This PR is ready for review.

Some final notes:

  • I checked the ssz_static generator output against that of the previous ssz solution, and it matched, except for some files that had differently generated random values. The generator is not perfect.
  • Although the memoization is a big performance gain, the memoization is backed by just dict now. I'll swap in a proper memoization cache in another PR.

specs/phase0/beacon-chain.md Outdated Show resolved Hide resolved
specs/phase1/beacon-chain.md Outdated Show resolved Hide resolved
tests/core/pyspec/eth2spec/debug/random_value.py Outdated Show resolved Hide resolved
tests/core/pyspec/eth2spec/test/helpers/custody.py Outdated Show resolved Hide resolved
tests/core/pyspec/eth2spec/test/utils.py Show resolved Hide resolved
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.

Looks good. Minor merge conflict with build_spec.py

@protolambda
Copy link
Collaborator Author

Just implemented ByteList, testing and preparing a new remerkleable release to include here.

@protolambda
Copy link
Collaborator Author

Reduced it all to one single commit 👍

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.

looks good! ready to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:SSZ Simple Serialize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants