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

Add 3 new invalid test cases #2790

Merged
merged 11 commits into from
Jan 13, 2022
Merged

Add 3 new invalid test cases #2790

merged 11 commits into from
Jan 13, 2022

Conversation

asanso
Copy link
Contributor

@asanso asanso commented Jan 6, 2022

The 3 test cases;

  1. first offset starts wrongly into the fixed position
  2. last variable part overflowing the max size
  3. last variable part has a wrong number f bytes

Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always great to see more test coverage!

i am wondering how the two last tests you wrote differ in terms of SSZ functionality they are touching, they both seem to just add extra data to the end which should yield a deserialization error but not necessarily because the offsets are incorrect (and that is what this part of test gen code suggests it is doing

tests/generators/ssz_generic/ssz_container.py Outdated Show resolved Hide resolved
tests/generators/ssz_generic/ssz_container.py Outdated Show resolved Hide resolved
tests/generators/ssz_generic/ssz_container.py Outdated Show resolved Hide resolved
tests/generators/ssz_generic/ssz_container.py Outdated Show resolved Hide resolved
tests/generators/ssz_generic/ssz_container.py Outdated Show resolved Hide resolved
tests/generators/ssz_generic/ssz_container.py Outdated Show resolved Hide resolved
asanso and others added 5 commits January 6, 2022 19:06
Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
@asanso
Copy link
Contributor Author

asanso commented Jan 6, 2022

always great to see more test coverage!

i am wondering how the two last tests you wrote differ in terms of SSZ functionality they are touching, they both seem to just add extra data to the end which should yield a deserialization error but not necessarily because the offsets are incorrect (and that is what this part of test gen code suggests it is doing

Yes you are correct the last 2 tests are not related to invalid offset but are still invalid tests nevertheless, IMHO the more invalid case we have, the better.

Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice updates!

still want to understand your thinking more on structure

tests/generators/ssz_generic/ssz_container.py Outdated Show resolved Hide resolved
Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice thanks :)

@ralexstokes ralexstokes merged commit 50a63c4 into ethereum:dev Jan 13, 2022
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.

3 participants