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

Clean up homogeneous composite sedes length pre-conditions #116

Open
hwwhww opened this issue Jun 3, 2020 · 3 comments
Open

Clean up homogeneous composite sedes length pre-conditions #116

hwwhww opened this issue Jun 3, 2020 · 3 comments

Comments

@hwwhww
Copy link
Contributor

hwwhww commented Jun 3, 2020

updated the issue: 2020, June 18th

What was wrong?

As ethereum/consensus-specs#1863 pointed out, it seems

py-ssz/ssz/sedes/basic.py

Lines 177 to 180 in 7e9c107

if max_length < 1:
raise ValueError(
f"(Maximum) length of homogenous composites must be at least 1, got {max_length}"
)

is only for Vector, and then override it in List. It made the illegal condition unclear.

How can it be fixed?

1. Use illegal condition max_length < 0 for HomogeneousProperCompositeSedes (wow it's so long)
2. Override vector types' (Vector and Bitvector) __init__() with illegal condition max_length < 1.

  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.
@booleanfunction
Copy link
Contributor

@hwwhww I think the __inti__()s for Bitvector and Vector have the override mentioned in 2. above? :)

class Bitvector(BitfieldCompositeSedes[BytesOrByteArray, bytes]):
def __init__(self, bit_count: int) -> None:
if bit_count <= 0:
raise TypeError("Bit count cannot be zero or negative")

class Vector(
HomogeneousProperCompositeSedes[
Sequence[TSerializableElement], Tuple[TDeserializedElement, ...]
]
):
def __init__(self, element_sedes: TSedes, length: int) -> None:
if length <= 0:
raise ValueError(f"Vectors must have a size of 1 or greater, got {length}")

@hwwhww
Copy link
Contributor Author

hwwhww commented Jun 18, 2020

@booleanfunction

You're right, sorry! Fixing the suggested proposal:

  1. Replace HomogeneousProperCompositeSedes.__init_() with abstractmethod. 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.

I think this is more clear than overriding?

@booleanfunction
Copy link
Contributor

@hwwhww that seems like a good way to do it :)

@hwwhww hwwhww changed the title 0-length List/Bitlist should be legal Clean up homogeneous composite sedes length pre-conditions Jun 18, 2020
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

No branches or pull requests

2 participants