-
Notifications
You must be signed in to change notification settings - Fork 999
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 Bitlist and Bitvector #1224
Conversation
Bool, Bit -> boolean, bit Fix simple-serialize.md
specs/core/1_custody-game.md
Outdated
@@ -272,6 +272,16 @@ def get_custody_chunk_count(crosslink: Crosslink) -> int: | |||
return crosslink_length * chunks_per_epoch | |||
``` | |||
|
|||
### `get_bitfield_bit` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this function, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not change this logic at the moment because that will likely change quite a bit anyway. I think in the future, we will not need it, but I wanted to keep the old implementation alive for now.
So the test that is currently failing is for the bridge to py-ssz. I think we probably can't get that to work until py-ssz has the new Bitvector and Bitlist classes. (Also, I noticed that at the moment, py-ssz does not seem to support the new max-length lists) |
See a5154da on the
Edit; ssz implementation still needs testing, we don't want every bit to be encoded as byte, and need alignment checks for non-multiple-of-8 bits in bytes. |
# Conflicts: # specs/core/0_beacon-chain.md
@protolambda: Can we avoid throwing an error with
|
@JustinDrake Containers expect named arguments, not bare arguments. I prefer named arguments, as it enables you to shorten the names for input variables, without being less descriptive where it matters. And it enables you to leave out unused arguments. If we really want, I can add support for unnamed args, but it feels error prone (things change, and phase 1 will keep changing for a while). |
@dankrad @JustinDrake Can you take a look at the more explicit wording for list-limits? It's important, since it is part of consensus (max operations in a block), and merkleization with padding is a subtle combined thing, possibly not familiar to some readers. |
Resolving the merge conflict now |
…ke bits rework from bitlistvector PR
test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py
Outdated
Show resolved
Hide resolved
test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py
Outdated
Show resolved
Hide resolved
test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py
Show resolved
Hide resolved
Great work @dankrad! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick questions around max length merkelization
Co-Authored-By: Danny Ryan <[email protected]>
…rocess_attestation.py Co-Authored-By: Danny Ryan <[email protected]>
Update test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py Co-Authored-By: Danny Ryan <[email protected]>
2a6a154
to
be04eb2
Compare
Fixed the last suggestion: removed unused import for code linter |
Replaces #1096 for Bitfields only
Bitvector
(static length Bitfield) andBitlist
(dynamic length Bitfield)Further fixes:
Bool
,Bit
->boolean
,bit
. This means that all basic SSZ types are now lowercase (bool
is not possible due to python type clash)