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

pyspec-SSZ: lists-rework (enable static generalized indices) + fully python class based now. #1180

Merged
merged 75 commits into from
Jun 25, 2019

Conversation

vbuterin
Copy link
Contributor

@vbuterin vbuterin commented Jun 14, 2019

SSZ List type must specify a max length. This greatly simplifies generalized indices.

@vbuterin vbuterin requested a review from protolambda June 14, 2019 18:08
Copy link
Collaborator

@protolambda protolambda 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 👏 , but it's not quite complete yet. CC @hwwhww, this PR also does the step of typing of all integers. May want to merge together, or rebase one PR on the other PR.

test_libs/pyspec/eth2spec/utils/ssz/ssz_impl.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
@protolambda
Copy link
Collaborator

protolambda commented Jun 15, 2019

print(Vector[uint8, 7]())
print(Vector[uint8, 7](uint8(i) for i in range(7)))
print(List[uint8, 128](uint8(1), uint8(2), uint8(3)))

BigVector = Vector[uint8, 64]
foo = BigVector()
foo[3] = uint8(123)
print(foo)


class Foobar(Container):
    a: uint8
    b: uint32


x = Foobar()
print(x)

y = Bytes[123](b"foobar")
print(y)

print(BytesN[3](b"yay"))
print(BytesN[8](i ^ 0xa0 for i in range(8)))

out:

Vector[uint8, 7](0, 0, 0, 0, 0, 0, 0)
Vector[uint8, 7](0, 1, 2, 3, 4, 5, 6)
List[uint8, 128](1, 2, 3)
Vector[uint8, 64](0, 0, 0, 123, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)
Foobar
  a: 0
  b: 0
Bytes[123]: 666f6f626172
BytesN[3]: 796179
BytesN[8]: a0a1a2a3a4a5a6a7

Also:

  • can't instantiate a type with wrong amount of params or none at all
  • typ.default() is a thing now :)
  • everything can be checked with issubclass
  • easily declare parametrized type properties (just 3 clean lines to define/override a metaclass)

@protolambda
Copy link
Collaborator

test output for new merkleization:

def hash(v):
    return f"h({v})"

zerohashes = [f"Z({i})" for i in range(0, 32)]

... merkleization


for i in range(20):
    print(merkleize_chunks([f"{v}" for v in range(i)], 32))
h(h(h(h(h(Z(0)Z(0))Z(1))Z(2))Z(3))Z(4))
h(h(h(h(h(0Z(0))Z(1))Z(2))Z(3))Z(4))
h(h(h(h(h(01)Z(1))Z(2))Z(3))Z(4))
h(h(h(h(h(01)h(2Z(0)))Z(2))Z(3))Z(4))
h(h(h(h(h(01)h(23))Z(2))Z(3))Z(4))
h(h(h(h(h(01)h(23))h(h(4Z(0))Z(1)))Z(3))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(Z(0)Z(0))))Z(3))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(6Z(0))))Z(3))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))Z(3))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(8Z(0))Z(1))Z(2)))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(Z(0)Z(0)))Z(2)))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(10Z(0)))Z(2)))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(1011))h(h(Z(0)Z(0))Z(1))))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(1011))h(h(12Z(0))Z(1))))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(1011))h(h(1213)h(Z(0)Z(0)))))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(1011))h(h(1213)h(14Z(0)))))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(1011))h(h(1213)h(1415))))Z(4))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(1011))h(h(1213)h(1415))))h(h(h(h(16Z(0))Z(1))Z(2))Z(3)))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(1011))h(h(1213)h(1415))))h(h(h(h(1617)h(Z(0)Z(0)))Z(2))Z(3)))
h(h(h(h(h(01)h(23))h(h(45)h(67)))h(h(h(89)h(1011))h(h(1213)h(1415))))h(h(h(h(1617)h(18Z(0)))Z(2))Z(3)))

@protolambda
Copy link
Collaborator

@vbuterin Can I get a final review before updating the usages of these ssz types everywhere? (and fixing whatever small bugs I can find along the way)

@JustinDrake JustinDrake added general:presentation Presentation (as opposed to content) scope:SSZ Simple Serialize labels Jun 15, 2019
@protolambda
Copy link
Collaborator

  • updated spec builder to handle container field dependencies of new style types
  • implemented tests for typing + ssz implementation, new ssz seems to be working
  • blocker: NewType doesn't type check. Need proper classes to declare element types and field types. Since a NewType around an int isn't a subclass of SSZType, or has any default value. I know @hwwhww is implementing replacement of newtypes with classes. If we get that merged, we can continue this PR using those class based declarations.

@protolambda protolambda mentioned this pull request Jun 18, 2019
4 tasks
@protolambda
Copy link
Collaborator

protolambda commented Jun 18, 2019

  • Spec builder updated to deal with new types, cleanly handle dependencies
  • merged in dev, no conflicts anymore (thx @hwwhww for mypy PR)
  • conforms fully to mypy and flake8 linting
  • phase-0 compiles (tests start, crash on phase-0)
  • phase-1 compiles.
  • Todo: need to assign lengths to lists in phase-1, PLACEHOLDER for now.
  • run tests, see if test code needs to adapt new types somewhere
  • double check ssz serialization behavior by comparing ssz-static generator output with ssz-static output on Dev. (disabling hash-tree-root, since that behavior changed) See new checklist

@JustinDrake JustinDrake removed the general:presentation Presentation (as opposed to content) label Jun 19, 2019
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.

This is a beast!
Here's some review. need to dig into partials a bit more

specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_partials.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_typing.py Outdated Show resolved Hide resolved
@protolambda
Copy link
Collaborator

protolambda commented Jun 22, 2019

Ok, PR mostly ready, summary of last TODOs:

@protolambda
Copy link
Collaborator

Verified that:

  • objects of fixed-size have same hash-tree-root and serialization as in the dev branch
  • objects of variable size:
    • have same serialization (up to point where RNG changes due to var. length limiting, see below)
    • different hash-tree-root due to limit on lists
      The RNG diverges when the ssz_static generator passes transfers (limited to 0); it generates different values, since bigger are simply not allowed within the list-limit. So I couldn't compare the full data, but what I could compare matched expectations.

And we've tests for the new merkleization now, see test_libs/pyspec/eth2spec/utils/test_merkle_minimal.py

@protolambda
Copy link
Collaborator

Also, this PR introduces 230+59+119=408 lines of ssz/merkleization test-code, and includes a bunch of fixes + updated tests for finalization (thanks @djrtwo). I'm happy we are actually reducing lines in the spec util code, while improving in style and tests 🎉

@protolambda protolambda self-requested a review June 22, 2019 20:26
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_impl.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/ssz/ssz_impl.py Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
scripts/build_spec.py Outdated Show resolved Hide resolved
scripts/build_spec.py Outdated Show resolved Hide resolved
specs/core/1_shard-data-chains.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
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.

6 participants