Skip to content

Implement type spec tests #417

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

Draft
wants to merge 5 commits into
base: unstable
Choose a base branch
from

Conversation

Zacholme7
Copy link
Member

Implementation for the types spec tests. Gave myself a week for these and think it may be time to just clean this all up and call them good. This PR contains the work in #374 and #377 as everything got a bit our of line so I decided to just merge everything together. This is a giga PR so I can also try to break it up.

I would say the main use for these is to test our SSZ encoding/decoding, which we can now confirm is spec compliant. There really may even be an argument that we should only include the ssz encoding/decoding and consider what we can scrap.

I have also deemed a few to be skippable

  • The committee member tests as our types differ substantially here and it would be a couple hundred lines of just parsing for basic quorum validation. I can just write a unit tests if this is really needed
  • The beacon deposit data test. This generates ETH deposit data, but this is never actually even used in the client.
  • The duty tests. These are for their internal runner mapping which is an implementation detail, not spec level.
  • The share encoding test. We use a different structure for share data so it does not make sense to test this encoding.

There are also some tests with data issues that I have raised

  • The ssv message tests use message ids that should not be considered valid for the testing scenario. I have addresses this here
  • the ssz test uses data from a beacon block with mocked signatures which when we try to parse into BeaconBlock, fails with BLST_BAD_ENCODING. Raised the issue here

@Zacholme7
Copy link
Member Author

Currently blocked on #416 getting merged in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant