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

[WIP] SSZ update #32

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

jannikluhn
Copy link
Collaborator

This PR replaces the existing oudated SSZ test generators. It includes tests for all types defined in the 0.5 spec release, excluding aliases. There are no tests for invalid inputs or outputs.

It will likely take a while until we'd be able to merge this PR as

Therefore, I opened it mostly for reference and to give people a chance to review and/or complain if they find an error in the tests.

See this gist for the generated tests vectors.

@jannikluhn jannikluhn added the SSZ label Mar 28, 2019
@jannikluhn
Copy link
Collaborator Author

(At least) 15 of the 730 test are broken. I think all of them are related to this issue: ethereum/consensus-specs#854

Here's a list with filenames and line numbers:

deeply_nested_composite_types.yaml:387
deeply_nested_composite_types.yaml:428
deeply_nested_composite_types.yaml:867
deeply_nested_composite_types.yaml:1164
nested_composite_types.yaml:4256
nested_composite_types.yaml:4276
nested_composite_types.yaml:4323
nested_composite_types.yaml:5460
nested_composite_types.yaml:5965
nested_composite_types.yaml:6191
nested_composite_types.yaml:9191
nested_composite_types.yaml:9320
nested_composite_types.yaml:9463
nested_composite_types.yaml:9473
nested_composite_types.yaml:10121

@hwwhww
Copy link

hwwhww commented Mar 29, 2019

It's a little awkward that "Vector" is "Tuple" in spec v0.5.1 (https://github.com/ethereum/eth2.0-specs/blob/v0.5.1/specs/simple-serialize.md), and will be renamed in the next release. IMO it's okay but we should note that in the py-ssz and tests release.

@protolambda
Copy link

Concern here is that this uses a 2 months old SSZ package, and incompatible with the SSZ typing definitions used in the spec. See https://github.com/ethereum/eth2.0-specs/blob/d7d2eeadb3e8dad3fed51b243e9647c64c084136/specs/test_formats/ssz_generic/README.md and ethereum/consensus-specs#962 for context.

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.

None yet

3 participants