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 Freeze Spec Simplification Section - part 1 #2893

Merged
merged 5 commits into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions beacon-chain/core/blocks/block_operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,24 +551,29 @@ func ProcessAttestation(beaconState *pb.BeaconState, att *pb.Attestation, verify
//
// Spec pseudocode definition:
// def convert_to_indexed(state: BeaconState, attestation: Attestation) -> IndexedAttestation:
// """
// Convert ``attestation`` to (almost) indexed-verifiable form.
// """
// attesting_indices = get_attesting_indices(state, attestation.data, attestation.aggregation_bitfield)
// custody_bit_1_indices = get_attesting_indices(state, attestation.data, attestation.custody_bitfield)
// custody_bit_0_indices = [index for index in attesting_indices if index not in custody_bit_1_indices]
// return IndexedAttestation(
// custody_bit_0_indices=custody_bit_0_indices,
// custody_bit_1_indices=custody_bit_1_indices,
// data=attestation.data,
// signature=attestation.signature,
// )
// """
// Convert ``attestation`` to (almost) indexed-verifiable form.
// """
// attesting_indices = get_attesting_indices(state, attestation.data, attestation.aggregation_bitfield)
// custody_bit_1_indices = get_attesting_indices(state, attestation.data, attestation.custody_bitfield)
// assert custody_bit_1_indices.issubset(attesting_indices)
// custody_bit_0_indices = attesting_indices.difference(custody_bit_1_indices)
//
// return IndexedAttestation(
// custody_bit_0_indices=sorted(custody_bit_0_indices),
Copy link
Member Author

Choose a reason for hiding this comment

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

sort here is not needed and confirmed:
Screen Shot 2019-07-01 at 9 14 03 AM

// custody_bit_1_indices=sorted(custody_bit_1_indices),
// data=attestation.data,
// signature=attestation.signature,
// )
func ConvertToIndexed(state *pb.BeaconState, attestation *pb.Attestation) (*pb.IndexedAttestation, error) {
attIndices, err := helpers.AttestingIndices(state, attestation.Data, attestation.AggregationBits)
if err != nil {
return nil, fmt.Errorf("could not get attesting indices: %v", err)
}
cb1i, _ := helpers.AttestingIndices(state, attestation.Data, attestation.CustodyBits)
if !sliceutil.SubsetUint64(cb1i, attIndices) {
return nil, fmt.Errorf("%v is not a subset of %v", cb1i, attIndices)
}
cb1Map := make(map[uint64]bool)
for _, idx := range cb1i {
cb1Map[idx] = true
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/core/blocks/block_operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ func TestConvertToIndexed_OK(t *testing.T) {
aggregationBitfield: []byte{0x03},
custodyBitfield: []byte{0x03},
wantedCustodyBit0Indices: []uint64{},
wantedCustodyBit1Indices: []uint64{71, 127},
wantedCustodyBit1Indices: []uint64{127, 71},
},
}

Expand Down
33 changes: 13 additions & 20 deletions beacon-chain/core/helpers/committee.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ package helpers
import (
"errors"
"fmt"
"sort"

"github.com/prysmaticlabs/prysm/beacon-chain/cache"
"github.com/prysmaticlabs/prysm/beacon-chain/utils"
pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
Expand Down Expand Up @@ -140,19 +138,16 @@ func ComputeCommittee(
return shuffledIndices, nil
}

// AttestingIndices returns the attesting participants indices. We removed sorting because it's irrelevant
// in production, we don't need to reduce the surface of possible valid input.
// AttestingIndices returns the attesting participants indices from the attestation data.
//
// Spec pseudocode definition:
// def get_attesting_indices(state: BeaconState,
// attestation_data: AttestationData,
// bitfield: bytes) -> List[ValidatorIndex]:
// def get_attesting_indices(state: BeaconState, data: AttestationData, bitfield: bytes) -> Set[ValidatorIndex]:
// """
// Return the sorted attesting indices corresponding to ``attestation_data`` and ``bitfield``.
// Return the set of attesting indices corresponding to ``data`` and ``bitfield``.
// """
// committee = get_crosslink_committee(state, attestation_data.target_epoch, attestation_data.crosslink.shard)
// assert verify_bitfield(bitfield, len(committee))
// return sorted([index for i, index in enumerate(committee) if get_bitfield_bit(bitfield, i) == 0b1])
// return set(index for i, index in enumerate(committee) if get_bitfield_bit(bitfield, i) == 0b1)
func AttestingIndices(state *pb.BeaconState, data *pb.AttestationData, bitfield []byte) ([]uint64, error) {
committee, err := CrosslinkCommitteeAtEpoch(state, data.TargetEpoch, data.Crosslink.Shard)
if err != nil {
Expand All @@ -165,19 +160,17 @@ func AttestingIndices(state *pb.BeaconState, data *pb.AttestationData, bitfield
return nil, errors.New("bitfield is unable to be verified")
}

var attestingIndices []uint64
for i, index := range committee {
if mathutil.CeilDiv8(int(i)) > len(bitfield) {
continue
}
if bitutil.BitfieldBit(bitfield, int(i)) == 1 {
attestingIndices = append(attestingIndices, index)
indices := make([]uint64, 0, len(committee))
indicesSet := make(map[uint64]bool)
for i, idx := range committee {
if !indicesSet[idx] {
if mathutil.CeilDiv8(i) <= len(bitfield) && bitutil.BitfieldBit(bitfield, i) == 0x1 {
indices = append(indices, idx)
}
}
indicesSet[idx] = true
}
sort.SliceStable(attestingIndices, func(i, j int) bool {
return attestingIndices[i] < attestingIndices[j]
})
return attestingIndices, nil
return indices, nil
}

// VerifyBitfield validates a bitfield with a given committee size.
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/core/helpers/committee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func TestAttestationParticipants_NoCommitteeCache(t *testing.T) {
attestationSlot: 3,
stateSlot: 5,
bitfield: []byte{0x03},
wanted: []uint64{71, 127},
wanted: []uint64{127, 71},
},
{
attestationSlot: 2,
Expand All @@ -258,7 +258,7 @@ func TestAttestationParticipants_NoCommitteeCache(t *testing.T) {
attestationSlot: 11,
stateSlot: 10,
bitfield: []byte{0x03},
wanted: []uint64{68, 102},
wanted: []uint64{102, 68},
},
}

Expand Down
13 changes: 2 additions & 11 deletions beacon-chain/utils/shuffle.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ const positionWindowSize = int8(4)
const pivotViewSize = seedSize + roundSize
const totalSize = seedSize + roundSize + positionWindowSize

var maxShuffleListSize uint64 = 1 << 40

// SplitIndices splits a list into n pieces.
func SplitIndices(l []uint64, n uint64) [][]uint64 {
var divided [][]uint64
Expand Down Expand Up @@ -50,7 +48,6 @@ func UnShuffledIndex(index uint64, indexCount uint64, seed [32]byte) (uint64, er
// Return the shuffled validator index corresponding to ``seed`` (and ``index_count``).
// """
// assert index < index_count
// assert index_count <= 2**40
// # Swap or not (https://link.springer.com/content/pdf/10.1007%2F978-3-642-32009-5_1.pdf)
// # See the 'generalized domain' algorithm on page 3
// for round in range(SHUFFLE_ROUND_COUNT):
Expand All @@ -70,10 +67,7 @@ func innerShuffledIndex(index uint64, indexCount uint64, seed [32]byte, shuffle
return 0, fmt.Errorf("input index %d out of bounds: %d",
index, indexCount)
}
if indexCount > maxShuffleListSize {
return 0, fmt.Errorf("list size %d out of bounds",
indexCount)
}

rounds := uint8(params.BeaconConfig().ShuffleRoundCount)
round := uint8(0)
if !shuffle {
Expand Down Expand Up @@ -166,10 +160,7 @@ func innerShuffleList(input []uint64, seed [32]byte, shuffle bool) ([]uint64, er
if len(input) <= 1 {
return input, nil
}
if uint64(len(input)) > maxShuffleListSize {
return nil, fmt.Errorf("list size %d out of bounds",
len(input))
}

rounds := uint8(params.BeaconConfig().ShuffleRoundCount)
if rounds == 0 {
return input, nil
Expand Down
10 changes: 0 additions & 10 deletions beacon-chain/utils/shuffle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,6 @@ import (
"github.com/prysmaticlabs/prysm/shared/params"
)

func TestShuffleList_InvalidValidatorCount(t *testing.T) {
maxShuffleListSize = 20
list := make([]uint64, 21)
if _, err := ShuffleList(list, [32]byte{123, 125}); err == nil {
t.Error("Shuffle should have failed when validator count exceeds ModuloBias")
maxShuffleListSize = 1 << 40
}
maxShuffleListSize = 1 << 40
}

func TestShuffleList_OK(t *testing.T) {
var list1 []uint64
seed1 := [32]byte{1, 128, 12}
Expand Down
21 changes: 21 additions & 0 deletions shared/sliceutil/slice.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,26 @@
package sliceutil

// SubsetUint64 returns true if the first array is
// completely contained in the second array with time
// complexity of approximately o(n).
func SubsetUint64(a []uint64, b []uint64) bool {
Copy link
Member

Choose a reason for hiding this comment

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

Shortcut: return false if a is longer than b

set := make(map[uint64]uint64)
for _, v := range b {
set[v] += 1
}

for _, v := range a {
if count, found := set[v]; !found {
return false
} else if count < 1 {
return false
} else {
set[v] = count - 1
}
}
return true
}

// IntersectionUint64 of two uint64 slices with time
// complexity of approximately O(n) leveraging a map to
// check for element existence off by a constant factor
Expand Down
22 changes: 22 additions & 0 deletions shared/sliceutil/slice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,28 @@ import (
"testing"
)

func TestSubsetUint64(t *testing.T) {
testCases := []struct {
setA []uint64
setB []uint64
out bool
}{
{[]uint64{1}, []uint64{1, 2, 3, 4}, true},
{[]uint64{1, 2, 3, 4}, []uint64{1, 2, 3, 4}, true},
{[]uint64{1, 1}, []uint64{1, 2, 3, 4}, false},
{[]uint64{}, []uint64{1}, true},
{[]uint64{1}, []uint64{}, false},
{[]uint64{1, 2, 3, 4, 5}, []uint64{1, 2, 3, 4}, false},
}
for _, tt := range testCases {
result := SubsetUint64(tt.setA, tt.setB)
if result != tt.out {
t.Errorf("%v, got %v, want %v", tt.setA, result, tt.out)
}

}
}

func TestIntersectionUint64(t *testing.T) {
testCases := []struct {
setA []uint64
Expand Down