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 bitfield_bytes type error #1806

Merged
merged 2 commits into from
May 18, 2020
Merged

Fix bitfield_bytes type error #1806

merged 2 commits into from
May 18, 2020

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented May 13, 2020

Issue

Address #1787.

How did I fix it

Rename bitfield_bytesto pack_bits and make this function return BYTES_PER_CHUNK-byte chunks.

p.s. it seems the spec was misleading but the implementations are all good? /cc @protolambda to confirm.

@hwwhww hwwhww added the scope:SSZ Simple Serialize label May 13, 2020
@hwwhww hwwhww requested a review from protolambda May 13, 2020 08:33
* `mix_in_length(merkleize(pack(value), limit=chunk_count(type)), len(value))` if `value` is a list of basic objects.
* `mix_in_length(merkleize(bitfield_bytes(value), limit=chunk_count(type)), len(value))` if `value` is a bitlist.
* `mix_in_length(merkleize(pack_bits(value), limit=chunk_count(type)), len(value))` if `value` is a bitlist.
Copy link
Collaborator

Choose a reason for hiding this comment

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

mix_in_length(merkleize(pack(bitfield_bytes(value), limit=chunk_count(type)), len(value)) would also work. But being explicit here is not a bad idea.

ssz/simple-serialize.md Outdated Show resolved Hide resolved
@protolambda
Copy link
Collaborator

Also confirm that the spec was misleading here, good catch and fix. Thanks.

Co-authored-by: Diederik Loerakker <[email protected]>
@hwwhww hwwhww added this to the v0.12.0 milestone May 14, 2020
@hwwhww hwwhww requested a review from protolambda May 18, 2020 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:SSZ Simple Serialize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants