Align Attestation Fields in State/Block With Spec #1072
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rauljordan
commented
Dec 11, 2018
| // TODO(#781): Pending refactor from the spec. | ||
| _ = slot | ||
| _ = crossLinkRecords | ||
| // for _, attestation := range pendingAttestations { |
There was a problem hiding this comment.
Not entirely sure what to do here other than commenting out the bulk of this function, as it will break and the inner logic will be inherently different due to spec changes. Ideas @terenc3t @prestonvanloon?
rauljordan
commented
Dec 11, 2018
| @@ -78,9 +78,9 @@ func GetShardAndCommitteesForSlot(shardCommittees []*pb.ShardAndCommitteeArray, | |||
|
|
|||
| // AreAttesterBitfieldsValid validates that the length of the attester bitfield matches the attester indices | |||
| // defined in the Crystallized State. | |||
| func AreAttesterBitfieldsValid(attestation *pb.AggregatedAttestation, attesterIndices []uint32) bool { | |||
| func AreAttesterBitfieldsValid(participationBitfield []byte, attesterIndices []uint32) bool { | |||
There was a problem hiding this comment.
It's best to pass the actual item a function needs rather than a full struct when it only accesses one or two pieces of a struct, so changed it appropriately
|
Closing this as we will be using the generated protos as our canonical type definitions |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #781
Description
This PR replaces the deprecated proto field for
repeated AggregatedAttestation pending_attestationsin the BeaconState by arepeated PendingAttestationRecord latest_attestationsand changes all instances in thecore/package for tests to pass and the system to build correctly.Additionally, we deprecate a beacon blocks'
repeated AggregatedAttestationand replace them byrepeated Attestation attestationswithin the block body appropriately. After this PR, it will be possible to easily deprecate all other instances ofAggregatedAttestationacross the repo correctly.