v0.8 spec sync #77
v0.8 spec sync #77
Conversation
hwwhww
force-pushed
the
list_max_length
branch
4 times, most recently
from
25037b9 to
a9bd4fe
Compare
ssz/sedes/bitlist.py
Outdated
|
|
||
| class Bitlist(BaseCompositeSedes[BytesOrByteArray, bytes]): | ||
| def __init__(self, bit_count: int) -> None: | ||
| if bit_count < 0: |
There was a problem hiding this comment.
| if bit_count < 0: | |
| if bit_count < 1: |
as the new spec version now explicitly forbids zero-length vectors..
There was a problem hiding this comment.
looks like I was wrong, empty lists (but not vectors) should be fine.
ssz/sedes/bitlist.py
Outdated
| if len_value == 0: | ||
| return b'\x01' | ||
|
|
||
| as_bytearray = [0] * (len_value // 8 + 1) |
There was a problem hiding this comment.
is there a reason you aren't actually using a bytearray? I think it will give you some added type/value safety since it won't allow assignment of values outside of the 0-256 range.
as_bytearray = bytearray(len_value // 8 + 1)Also, won't len_value // 8 + 1 produce arrays with an extra element in the case that len_value is a clean multiple of 8?
There was a problem hiding this comment.
Right, I should have used bytearray, thanks!
Also, won't
len_value // 8 + 1produce arrays with an extra element in the case thatlen_valueis a clean multiple of8?
It is. https://github.com/ethereum/eth2.0-specs/blob/dev/specs/simple-serialize.md#bitlistn:
"Note that from the offset coding, the length (in bytes) of the bitlist is known. An additional leading 1 bit is added so that the length in bits will also be known."
ssz/sedes/bitlist.py
Outdated
|
|
||
| class Bitlist(BaseCompositeSedes[BytesOrByteArray, bytes]): | ||
| def __init__(self, bit_count: int) -> None: | ||
| if bit_count < 0: |
There was a problem hiding this comment.
looks like I was wrong, empty lists (but not vectors) should be fine.
Co-Authored-By: jannikluhn <jannik@brainbot.com>
|
@jannikluhn @pipermerriam @jannikluhn while the minimum length check is pending on the spec side discussion, we can go back to check it later after this PR. |
| @@ -142,20 +153,18 @@ def _deserialize_stream(self, stream: IO[bytes]) -> Iterable[TDeserialized]: | |||
| # Tree hashing | |||
| # | |||
| def hash_tree_root(self, value: Iterable[TSerializable]) -> bytes: | |||
There was a problem hiding this comment.
I was just thinking about input validation, to prevent people from accidentally hashing inputs that are too big. Don't think it's super necessary though, we can always add this later.
| for layer in range(chunk_depth, max_depth): | ||
| tmp_list[layer + 1] = hash_eth2(tmp_list[layer] + zerohashes[layer]) | ||
|
|
||
| return tmp_list[max_depth] |
There was a problem hiding this comment.
Yes, you're right. In the interest of time I'd be ok with keeping it as it is for now. We might have to revisit the implementation anyway when we're going to implement tree caching.
* convert bash scripts to py
* convert bash scripts to py
What was wrong?
Fix #76
How was it fixed?
merkle_minimal/zerohashesstyle Merkleization since withmax_length, we do needzerohashesapproach for saving computations of a lot of padding.max_lengthinListand addmix_in_lengthforList.hash_tree_root().BitvectorandBitlist:NOTE: Use spec patch Define Bitlist/Bitvector serialization using bytes, not bigints consensus-specs#1267 for
BitvectorandBitlistserialization. Should wait for Define Bitlist/Bitvector serialization using bytes, not bigints consensus-specs#1267 getting merged.BeaconState,BeaconBlockin pyspec side).Cute Animal Picture