AttestingIndices: Make beacon committee be an argument (#4284)

* make beacon committee be an argument
* remove state from ConvertToIndexed
* Merge branch 'master' into refactor-AttestingIndices-committee
* Merge branch 'master' into refactor-AttestingIndices-committee
* Merge branch 'master' into refactor-AttestingIndices-committee
* Merge refs/heads/master into refactor-AttestingIndices-committee

beacon-chain/blockchain/forkchoice/process_attestation.go

@@ -171,7 +171,11 @@ func (s *Store) saveCheckpointState(ctx context.Context, baseState *pb.BeaconSta
 
 // verifyAttestation validates input attestation is valid.
 func (s *Store) verifyAttestation(ctx context.Context, baseState *pb.BeaconState, a *ethpb.Attestation) (*ethpb.IndexedAttestation, error) {
-	indexedAtt, err := blocks.ConvertToIndexed(ctx, baseState, a)
+	committee, err := helpers.BeaconCommittee(baseState, a.Data.Slot, a.Data.CommitteeIndex)
+	if err != nil {
+		return nil, err
+	}
+	indexedAtt, err := blocks.ConvertToIndexed(ctx, a, committee)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not convert attestation to indexed attestation")
 	}

beacon-chain/blockchain/forkchoice/process_block.go

@@ -316,7 +316,11 @@ func (s *Store) updateBlockAttestationVote(ctx context.Context, att *ethpb.Attes
 	if baseState == nil {
 		return errors.New("no state found in db with attestation tgt root")
 	}
-	indexedAtt, err := blocks.ConvertToIndexed(ctx, baseState, att)
+	committee, err := helpers.BeaconCommittee(baseState, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		return err
+	}
+	indexedAtt, err := blocks.ConvertToIndexed(ctx, att, committee)
 	if err != nil {
 		return errors.Wrap(err, "could not convert attestation to indexed attestation")
 	}

beacon-chain/blockchain/forkchoice/process_block_test.go

@@ -141,7 +141,11 @@ func TestStore_UpdateBlockAttestationVote(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	indices, err := blocks.ConvertToIndexed(ctx, beaconState, att)
+	committee, err := helpers.BeaconCommittee(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	indices, err := blocks.ConvertToIndexed(ctx, att, committee)
 	if err != nil {
 		t.Fatal(err)
 	}

beacon-chain/core/blocks/block_operations.go

@@ -622,6 +622,10 @@ func ProcessAttestationNoVerify(ctx context.Context, beaconState *pb.BeaconState
 
 // ConvertToIndexed converts attestation to (almost) indexed-verifiable form.
 //
+// Note about spec pseudocode definition. The state was used by get_attesting_indices to determine
+// the attestation committee. Now that we provide this as an argument, we no longer need to provide
+// a state.
+//
 // Spec pseudocode definition:
 //   def get_indexed_attestation(state: BeaconState, attestation: Attestation) -> IndexedAttestation:
 //    """
@@ -638,16 +642,16 @@ func ProcessAttestationNoVerify(ctx context.Context, beaconState *pb.BeaconState
 //        data=attestation.data,
 //        signature=attestation.signature,
 //    )
-func ConvertToIndexed(ctx context.Context, state *pb.BeaconState, attestation *ethpb.Attestation) (*ethpb.IndexedAttestation, error) {
+func ConvertToIndexed(ctx context.Context, attestation *ethpb.Attestation, committee []uint64) (*ethpb.IndexedAttestation, error) {
 	ctx, span := trace.StartSpan(ctx, "core.ConvertToIndexed")
 	defer span.End()
 
-	attIndices, err := helpers.AttestingIndices(state, attestation.Data, attestation.AggregationBits)
+	attIndices, err := helpers.AttestingIndices(attestation.AggregationBits, committee)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not get attesting indices")
 	}
 
-	cb1i, err := helpers.AttestingIndices(state, attestation.Data, attestation.CustodyBits)
+	cb1i, err := helpers.AttestingIndices(attestation.CustodyBits, committee)
 	if err != nil {
 		return nil, err
 	}
@@ -820,7 +824,11 @@ func VerifyIndexedAttestation(ctx context.Context, beaconState *pb.BeaconState,
 // VerifyAttestation converts and attestation into an indexed attestation and verifies
 // the signature in that attestation.
 func VerifyAttestation(ctx context.Context, beaconState *pb.BeaconState, att *ethpb.Attestation) error {
-	indexedAtt, err := ConvertToIndexed(ctx, beaconState, att)
+	committee, err := helpers.BeaconCommittee(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		return err
+	}
+	indexedAtt, err := ConvertToIndexed(ctx, att, committee)
 	if err != nil {
 		return errors.Wrap(err, "could not convert to indexed attestation")
 	}

beacon-chain/core/blocks/block_operations_test.go

@@ -923,7 +923,11 @@ func TestProcessAttestations_OK(t *testing.T) {
 	beaconState.CurrentJustifiedCheckpoint.Root = []byte("hello-world")
 	beaconState.CurrentEpochAttestations = []*pb.PendingAttestation{}
 
-	attestingIndices, err := helpers.AttestingIndices(beaconState, att.Data, att.AggregationBits)
+	committee, err := helpers.BeaconCommittee(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	attestingIndices, err := helpers.AttestingIndices(att.AggregationBits, committee)
 	if err != nil {
 		t.Error(err)
 	}
@@ -978,7 +982,11 @@ func TestProcessAggregatedAttestation_OverlappingBits(t *testing.T) {
 	beaconState.CurrentJustifiedCheckpoint.Root = []byte("hello-world")
 	beaconState.CurrentEpochAttestations = []*pb.PendingAttestation{}
 
-	attestingIndices1, err := helpers.AttestingIndices(beaconState, att1.Data, att1.AggregationBits)
+	committee, err := helpers.BeaconCommittee(beaconState, att1.Data.Slot, att1.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	attestingIndices1, err := helpers.AttestingIndices(att1.AggregationBits, committee)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1008,7 +1016,11 @@ func TestProcessAggregatedAttestation_OverlappingBits(t *testing.T) {
 		CustodyBits:     custodyBits2,
 	}
 
-	attestingIndices2, err := helpers.AttestingIndices(beaconState, att2.Data, att2.AggregationBits)
+	committee, err = helpers.BeaconCommittee(beaconState, att2.Data.Slot, att2.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	attestingIndices2, err := helpers.AttestingIndices(att2.AggregationBits, committee)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1053,7 +1065,11 @@ func TestProcessAggregatedAttestation_NoOverlappingBits(t *testing.T) {
 	beaconState.CurrentJustifiedCheckpoint.Root = []byte("hello-world")
 	beaconState.CurrentEpochAttestations = []*pb.PendingAttestation{}
 
-	attestingIndices1, err := helpers.AttestingIndices(beaconState, att1.Data, att1.AggregationBits)
+	committee, err := helpers.BeaconCommittee(beaconState, att1.Data.Slot, att1.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	attestingIndices1, err := helpers.AttestingIndices(att1.AggregationBits, committee)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1082,7 +1098,11 @@ func TestProcessAggregatedAttestation_NoOverlappingBits(t *testing.T) {
 		CustodyBits:     custodyBits2,
 	}
 
-	attestingIndices2, err := helpers.AttestingIndices(beaconState, att2.Data, att2.AggregationBits)
+	committee, err = helpers.BeaconCommittee(beaconState, att2.Data.Slot, att2.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	attestingIndices2, err := helpers.AttestingIndices(att2.AggregationBits, committee)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1202,7 +1222,12 @@ func TestConvertToIndexed_OK(t *testing.T) {
 			Data:                attestation.Data,
 			Signature:           attestation.Signature,
 		}
-		ia, err := blocks.ConvertToIndexed(context.Background(), state, attestation)
+
+		committee, err := helpers.BeaconCommittee(state, attestation.Data.Slot, attestation.Data.CommitteeIndex)
+		if err != nil {
+			t.Error(err)
+		}
+		ia, err := blocks.ConvertToIndexed(context.Background(), attestation, committee)
 		if err != nil {
 			t.Errorf("failed to convert attestation to indexed attestation: %v", err)
 		}

beacon-chain/core/epoch/epoch_processing.go

@@ -340,7 +340,11 @@ func unslashedAttestingIndices(state *pb.BeaconState, atts []*pb.PendingAttestat
 	var setIndices []uint64
 	seen := make(map[uint64]bool)
 	for _, att := range atts {
-		attestingIndices, err := helpers.AttestingIndices(state, att.Data, att.AggregationBits)
+		committee, err := helpers.BeaconCommittee(state, att.Data.Slot, att.Data.CommitteeIndex)
+		if err != nil {
+			return nil, err
+		}
+		attestingIndices, err := helpers.AttestingIndices(att.AggregationBits, committee)
 		if err != nil {
 			return nil, errors.Wrap(err, "could not get attester indices")
 		}

beacon-chain/core/epoch/precompute/attestation.go

@@ -36,7 +36,11 @@ func ProcessAttestations(
 		}
 
 		// Get attested indices and update the pre computed fields for each attested validators.
-		indices, err := helpers.AttestingIndices(state, a.Data, a.AggregationBits)
+		committee, err := helpers.BeaconCommittee(state, a.Data.Slot, a.Data.CommitteeIndex)
+		if err != nil {
+			return nil, nil, err
+		}
+		indices, err := helpers.AttestingIndices(a.AggregationBits, committee)
 		if err != nil {
 			return nil, nil, err
 		}

beacon-chain/core/epoch/precompute/attestation_test.go

@@ -199,13 +199,21 @@ func TestProcessAttestations(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	indices, _ := helpers.AttestingIndices(beaconState, att1.Data, att1.AggregationBits)
+	committee, err := helpers.BeaconCommittee(beaconState, att1.Data.Slot, att1.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	indices, _ := helpers.AttestingIndices(att1.AggregationBits, committee)
 	for _, i := range indices {
 		if !vp[i].IsPrevEpochAttester {
 			t.Error("Not a prev epoch attester")
 		}
 	}
-	indices, _ = helpers.AttestingIndices(beaconState, att2.Data, att2.AggregationBits)
+	committee, err = helpers.BeaconCommittee(beaconState, att2.Data.Slot, att2.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	indices, _ = helpers.AttestingIndices(att2.AggregationBits, committee)
 	for _, i := range indices {
 		if !vp[i].IsPrevEpochAttester {
 			t.Error("Not a prev epoch attester")

beacon-chain/core/helpers/committee.go

@@ -128,7 +128,9 @@ func ComputeCommittee(
 	return shuffledIndices, nil
 }
 
-// AttestingIndices returns the attesting participants indices from the attestation data.
+// AttestingIndices returns the attesting participants indices from the attestation data. The
+// committee is provided as an argument rather than a direct implementation from the spec definition.
+// Having the committee as an argument allows for re-use of beacon committees when possible.
 //
 // Spec pseudocode definition:
 //   def get_attesting_indices(state: BeaconState,
@@ -139,12 +141,7 @@ func ComputeCommittee(
 //    """
 //    committee = get_beacon_committee(state, data.slot, data.index)
 //    return set(index for i, index in enumerate(committee) if bits[i])
-func AttestingIndices(state *pb.BeaconState, data *ethpb.AttestationData, bf bitfield.Bitfield) ([]uint64, error) {
+func AttestingIndices(bf bitfield.Bitfield, committee []uint64) ([]uint64, error) {
-	committee, err := BeaconCommittee(state, data.Slot, data.CommitteeIndex)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get committee")
-	}
-
 	indices := make([]uint64, 0, len(committee))
 	indicesSet := make(map[uint64]bool)
 	for i, idx := range committee {

beacon-chain/core/helpers/committee_test.go

@@ -7,7 +7,6 @@ import (
 	"testing"
 
 	"github.com/prysmaticlabs/go-bitfield"
-
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
 	pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
@@ -116,8 +115,11 @@ func TestAttestationParticipants_NoCommitteeCache(t *testing.T) {
 	for _, tt := range tests {
 		attestationData.Target = &ethpb.Checkpoint{Epoch: 0}
 		attestationData.Slot = tt.attestationSlot
-
+		committee, err := BeaconCommittee(state, tt.attestationSlot, 0 /* committee index */)
-		result, err := AttestingIndices(state, attestationData, tt.bitfield)
+		if err != nil {
+			t.Error(err)
+		}
+		result, err := AttestingIndices(tt.bitfield, committee)
 		if err != nil {
 			t.Errorf("Failed to get attestation participants: %v", err)
 		}
@@ -146,7 +148,11 @@ func TestAttestationParticipants_EmptyBitfield(t *testing.T) {
 	}
 	attestationData := &ethpb.AttestationData{Target: &ethpb.Checkpoint{}}
 
-	indices, err := AttestingIndices(state, attestationData, bitfield.NewBitlist(128))
+	committee, err := BeaconCommittee(state, attestationData.Slot, attestationData.CommitteeIndex)
+	if err != nil {
+		t.Fatal(err)
+	}
+	indices, err := AttestingIndices(bitfield.NewBitlist(128), committee)
 	if err != nil {
 		t.Fatalf("attesting indices failed: %v", err)
 	}

beacon-chain/core/state/transition_test.go

@@ -387,7 +387,11 @@ func TestProcessBlock_PassesProcessingConditions(t *testing.T) {
 		AggregationBits: aggBits,
 		CustodyBits:     custodyBits,
 	}
-	attestingIndices, err := helpers.AttestingIndices(beaconState, blockAtt.Data, blockAtt.AggregationBits)
+	committee, err := helpers.BeaconCommittee(beaconState, blockAtt.Data.Slot, blockAtt.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	attestingIndices, err := helpers.AttestingIndices(blockAtt.AggregationBits, committee)
 	if err != nil {
 		t.Error(err)
 	}
@@ -671,7 +675,12 @@ func TestProcessBlk_AttsBasedOnValidatorCount(t *testing.T) {
 			AggregationBits: aggBits,
 			CustodyBits:     custodyBits,
 		}
-		attestingIndices, err := helpers.AttestingIndices(s, att.Data, att.AggregationBits)
+
+		committee, err := helpers.BeaconCommittee(s, att.Data.Slot, att.Data.CommitteeIndex)
+		if err != nil {
+			t.Error(err)
+		}
+		attestingIndices, err := helpers.AttestingIndices(att.AggregationBits, committee)
 		if err != nil {
 			t.Error(err)
 		}

beacon-chain/operations/attestation_test.go

@@ -39,8 +39,11 @@ func TestHandleAttestation_Saves_NewAttestation(t *testing.T) {
 		AggregationBits: bitfield.Bitlist{0xCF, 0xC0, 0xC0, 0xC0, 0x01},
 		CustodyBits:     bitfield.Bitlist{0x00, 0x00, 0x00, 0x00, 0x01},
 	}
-
+	committee, err := helpers.BeaconCommittee(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
-	attestingIndices, err := helpers.AttestingIndices(beaconState, att.Data, att.AggregationBits)
+	if err != nil {
+		t.Fatal(err)
+	}
+	attestingIndices, err := helpers.AttestingIndices(att.AggregationBits, committee)
 	if err != nil {
 		t.Error(err)
 	}
@@ -350,7 +353,11 @@ func TestRetrieveAttestations_OK(t *testing.T) {
 		AggregationBits: aggBits,
 		CustodyBits:     custodyBits,
 	}
-	attestingIndices, err := helpers.AttestingIndices(beaconState, att.Data, att.AggregationBits)
+	committee, err := helpers.BeaconCommittee(beaconState, att.Data.Slot, att.Data.CommitteeIndex)
+	if err != nil {
+		t.Error(err)
+	}
+	attestingIndices, err := helpers.AttestingIndices(att.AggregationBits, committee)
 	if err != nil {
 		t.Error(err)
 	}

shared/testutil/block.go

@@ -266,11 +266,11 @@ func generateAttesterSlashings(
 		sig = privs[valIndex].Sign(dataRoot[:], domain)
 		att2.Signature = bls.AggregateSignatures([]*bls.Signature{sig}).Marshal()
 
-		indexedAtt1, err := blocks.ConvertToIndexed(context.Background(), bState, att1)
+		indexedAtt1, err := blocks.ConvertToIndexed(context.Background(), att1, committee)
 		if err != nil {
 			return nil, err
 		}
-		indexedAtt2, err := blocks.ConvertToIndexed(context.Background(), bState, att2)
+		indexedAtt2, err := blocks.ConvertToIndexed(context.Background(), att2, committee)
 		if err != nil {
 			return nil, err
 		}

tools/blocktree/main.go

@@ -95,7 +95,11 @@ func main() {
 		}
 		// Retrieve attestation indices
 		for _, att := range atts {
-			indices, err := helpers.AttestingIndices(state, att.Data, att.AggregationBits)
+			committee, err := helpers.BeaconCommittee(state, att.Data.Slot, att.Data.CommitteeIndex)
+			if err != nil {
+				panic(err)
+			}
+			indices, err := helpers.AttestingIndices(att.AggregationBits, committee)
 			if err != nil {
 				panic(err)
 			}