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

Fix homogeneous sequence boundary #117

Merged
merged 3 commits into from Jun 18, 2020
Merged

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Jun 18, 2020

What was wrong?

Address issue #116

How was it fixed?

  1. Remove HomogeneousProperCompositeSedes.__init_()
  2. Update vector types' (Vector and Bitvector) __init__() with illegal condition max_length < 1.
  3. Add list types' (List and Bitlist) __init__() with illegal condition max_length < 0.
  4. Use ValueError for these exceptions.
  5. Add tests: test_homogeneous_sequence_length_boundary and test_bitfield_length_boundary

Cute Animal Picture

animal-5302420_640

/cc @booleanfunction @saltiniroberto

ssz/sedes/list.py Outdated Show resolved Hide resolved
@@ -28,9 +28,10 @@ class Vector(
]
):
def __init__(self, element_sedes: TSedes, length: int) -> None:
if length <= 0:
if length < 1:
Copy link

Choose a reason for hiding this comment

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

Why the change here? Given that we are dealing with int, <= 0 is equivalent to < 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's equivalent. I wanted to make it more clear and follow the error message more tightly.

ssz/sedes/list.py Outdated Show resolved Hide resolved
@hwwhww hwwhww requested a review from pipermerriam June 18, 2020 15:40
@hwwhww hwwhww merged commit 36f3406 into ethereum:master Jun 18, 2020
carver added a commit that referenced this pull request Aug 18, 2022
carver added a commit that referenced this pull request Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants