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

Update to frozen spec ❄️ (v0.8.1) #444

Merged
merged 64 commits into from Jul 30, 2019
Merged

Update to frozen spec ❄️ (v0.8.1) #444

merged 64 commits into from Jul 30, 2019

Conversation

@michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Jul 18, 2019

Proposed Changes

  • Update types and state_processing to v0.8.1 of the spec.
  • Update beacon_node, validator_client and other top-level crates
  • Update the EF tests and get them to pass

Related Issues

Still to do

  • Genesis tests #452
michaelsproul and others added 30 commits Jul 17, 2019
Fixed a typo in the implementation of ssz::Decode for BitVector, which caused it
to be considered variable length!
Allow computing the difference of two bitfields of different lengths.
SSZ benches relied upon fixed_len_vec -- it is easier to just delete
them and rebuild them later (when necessary)
In the case where we tried to access the committee of a shard that didn't have a committee in the
current epoch, we were accessing elements beyond the end of the shuffling vector and panicking! This
commit adds a check to make the failure safe and explicit.
There was a bug in our implementation of get_indexed_attestation whereby
incorrect "committee indices" were used to index into the custody bitfield. The
bug was only observable in the case where some bits of the custody bitfield were
set to 1. The implementation has been simplified to remove the bug, and a test
added.
* Remove redundant max operation checks.
* Always supply both messages when checking attestation signatures -- allowing
  verification of an attestation with no signatures.
* Swap the order of the fork and domain constant in `get_domain`, to match
  the spec.
@michaelsproul michaelsproul marked this pull request as ready for review Jul 26, 2019
Copy link
Member

@paulhauner paulhauner left a comment

I've been through all of these files (phew).

It's looking good so far. There's one set of tests I want to add back in.

Next steps are to resolve these comments then try and merge master in. I'll take care of it from here (unless anyone wants to input, of course).


if !verify_bitfield_length(&bitfield, committee.committee.len()) {
/* TODO(freeze): re-enable this?
Copy link
Member

@paulhauner paulhauner Jul 29, 2019

Choose a reason for hiding this comment

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

Commented-out-boi

tests/ef_tests/src/cases/operations_transfer.rs Outdated Show resolved Hide resolved
eth2/utils/tree_hash_derive/tests/tests.rs Show resolved Hide resolved
@paulhauner paulhauner merged commit a236003 into master Jul 30, 2019
4 checks passed
@paulhauner paulhauner deleted the v0.8 branch Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants