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

SSZ static testing [blocked by #960] #962

Merged
merged 7 commits into from
Apr 19, 2019
Merged

SSZ static testing [blocked by #960] #962

merged 7 commits into from
Apr 19, 2019

Conversation

protolambda
Copy link
Collaborator

@protolambda protolambda commented Apr 18, 2019

See /specs/test_formats/ssz_static/README.md and /specs/test_formats/ssz_generic/README.md to learn about the two types of SSZ testing.

SSZ itself is very much the same. It's just that the clients build towards efficient SSZ deserialization/serialization for their phase-0 structs, and generic SSZ would not cover this code.

Also, the generic SSZ test-suite was build on by:

But these are based on the older SSZ (afaik). Production-first clients have a high need to just test spec-SSZ for their ETH-2-phase0 types.

Generic SSZ would require to:

  • standardize a SSZ type format
  • write a type parser in every client, and an encoder for the test-gen
  • make clients instantiate these dynamic types. Something you wouldn't do in Go or Rust. Instead, a generic Container/Vector/List object would be used, but this makes things less efficient, and introduces more overhead in the SSZ encoder/decoder.
    I do see generic-SSZ be a thing in the future, but for now we really just need testing for the ETH-2.0 types, that every client is looking to serialize/deserialize anyway.

The introduced tests:

For every type pulled from the phase-0 spec (dynamic, generator does not need to be updated), create a bunch of random instances (we use the SSZ typing to fill it with random data), and output test vectors for it.
The randomization has different modes, and a "chaos" switch to randomly change the mode during filling of the SSZ instance with random data.
(Note: some modes are more like edge-cases, e.g. all zeroes, and will only output 1 case, unless in chaos-mode and things are mixed up)

The test-format itself is documented in the ssz_static test-format docs included in the PR.

It produces roughly ~10K test-cases, split in different suites. Most are for the minimal testing-configuration. And a few random ones are generated for the mainnet-configuration (larger objects, requires more space).


Note: this PR is branched of the ssz-impl-fixes (PR #960), to make use of the new fixed spec-conformant SSZ implementation. Wait for that PR to merge.

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. Agreed with static/generic split for now
Just a few minor comments

Curious -- how large is the ~10k test case output?

test_libs/pyspec/eth2spec/debug/random_value.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/debug/random_value.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/debug/random_value.py Outdated Show resolved Hide resolved
test_libs/pyspec/eth2spec/utils/minimal_ssz.py Outdated Show resolved Hide resolved
@protolambda
Copy link
Collaborator Author

@djrtwo Still relatively small compared to the Eth 1.0 tests repo. Just 67MB, fine with git LFS.

du -sh yaml_tests/ssz_static
67M     yaml_tests/ssz_static

@protolambda
Copy link
Collaborator Author

Rebased on dev to solve merge conflicts (this PR was based of my previous SSZ-PR, which still had some changes / rebases before being submitted). Also applied suggestions from Danny 👍

@djrtwo
Copy link
Contributor

djrtwo commented Apr 19, 2019

lgtm

@djrtwo djrtwo merged commit 1c86c87 into dev Apr 19, 2019
@djrtwo djrtwo deleted the ssz-static-testing branch April 19, 2019 03:10
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