Fix same deposits from same validator in same block (#4319)

* Update dict

* Test helper

* Regression test

* Comment

* Reset test cache

beacon-chain/core/blocks/block_operations.go

@@ -934,6 +934,7 @@ func ProcessDeposit(beaconState *pb.BeaconState, deposit *ethpb.Deposit, valInde
 			EffectiveBalance:           effectiveBalance,
 		})
 		beaconState.Balances = append(beaconState.Balances, amount)
+		valIndexMap[bytesutil.ToBytes48(pubKey)] = len(beaconState.Validators) - 1
 	} else {
 		beaconState = helpers.IncreaseBalance(beaconState, uint64(index), amount)
 	}

beacon-chain/core/blocks/block_operations_test.go

@@ -1343,6 +1343,47 @@ func TestValidateIndexedAttestation_AboveMaxLength(t *testing.T) {
 	}
 }
 
+func TestProcessDeposits_SameValidatorMultipleDepositsSameBlock(t *testing.T) {
+	// Same validator created 3 valid deposits within the same block
+	testutil.ResetCache()
+	dep, _, _ := testutil.DeterministicDepositsAndKeysSameValidator(3)
+	eth1Data, err := testutil.DeterministicEth1Data(len(dep))
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Log(dep)
+	block := &ethpb.BeaconBlock{
+		Body: &ethpb.BeaconBlockBody{
+			// 3 deposits from the same validator
+			Deposits: []*ethpb.Deposit{dep[0], dep[1], dep[2]},
+		},
+	}
+	registry := []*ethpb.Validator{
+		{
+			PublicKey:             []byte{1},
+			WithdrawalCredentials: []byte{1, 2, 3},
+		},
+	}
+	balances := []uint64{0}
+	beaconState := &pb.BeaconState{
+		Validators: registry,
+		Balances:   balances,
+		Eth1Data:   eth1Data,
+		Fork: &pb.Fork{
+			PreviousVersion: params.BeaconConfig().GenesisForkVersion,
+			CurrentVersion:  params.BeaconConfig().GenesisForkVersion,
+		},
+	}
+	newState, err := blocks.ProcessDeposits(context.Background(), beaconState, block.Body)
+	if err != nil {
+		t.Fatalf("Expected block deposits to process correctly, received: %v", err)
+	}
+
+	if len(newState.Validators) != 2 {
+		t.Errorf("Incorrect validator count. Wanted %d, got %d", 2, len(newState.Validators))
+	}
+}
+
 func TestProcessDeposits_MerkleBranchFailsVerification(t *testing.T) {
 	deposit := &ethpb.Deposit{
 		Data: &ethpb.Deposit_Data{

beacon-chain/core/helpers/committee.go

@@ -212,7 +212,7 @@ func CommitteeAssignments(state *pb.BeaconState, epoch uint64) (map[uint64]*Comm
 	if epoch > NextEpoch(state) {
 		return nil, nil, fmt.Errorf(
 			"epoch %d can't be greater than next epoch %d",
-			epoch, 
+			epoch,
 			NextEpoch(state),
 		)
 	}

beacon-chain/core/helpers/committee_test.go

@@ -394,7 +394,7 @@ func TestCommitteeAssignments_CanRetrieve(t *testing.T) {
 
 	state := &pb.BeaconState{
 		Validators:  validators,
-		Slot:        2*params.BeaconConfig().SlotsPerEpoch, // epoch 2
+		Slot:        2 * params.BeaconConfig().SlotsPerEpoch, // epoch 2
 		RandaoMixes: make([][]byte, params.BeaconConfig().EpochsPerHistoricalVector),
 	}
 

beacon-chain/rpc/validator/assignments_test.go

@@ -306,7 +306,7 @@ func BenchmarkCommitteeAssignment(b *testing.B) {
 	ctx := context.Background()
 
 	genesis := blk.NewGenesisBlock([]byte{})
-	depChainStart := uint64(8192*2)
+	depChainStart := uint64(8192 * 2)
 	deposits, _, _ := testutil.DeterministicDepositsAndKeys(depChainStart)
 	eth1Data, err := testutil.DeterministicEth1Data(len(deposits))
 	if err != nil {

shared/testutil/deposits.go

@@ -189,3 +189,79 @@ func ResetCache() {
 	privKeys = []*bls.SecretKey{}
 	cachedDeposits = []*ethpb.Deposit{}
 }
+
+// DeterministicDepositsAndKeysSameValidator returns the entered amount of deposits and secret keys.
+// of the same validator. This is for negative test cases such as same deposits from same validators in a block don't
+// result in duplicated validator indices.
+func DeterministicDepositsAndKeysSameValidator(numDeposits uint64) ([]*ethpb.Deposit, []*bls.SecretKey, error) {
+	lock.Lock()
+	defer lock.Unlock()
+	var err error
+
+	// Populate trie cache, if not initialized yet.
+	if trie == nil {
+		trie, err = trieutil.NewTrie(int(params.BeaconConfig().DepositContractTreeDepth))
+		if err != nil {
+			return nil, nil, errors.Wrap(err, "failed to create new trie")
+		}
+	}
+
+	// If more deposits requested than cached, generate more.
+	if numDeposits > uint64(len(cachedDeposits)) {
+		numExisting := uint64(len(cachedDeposits))
+		numRequired := numDeposits - uint64(len(cachedDeposits))
+		// Fetch the required number of keys.
+		secretKeys, publicKeys, err := interop.DeterministicallyGenerateKeys(numExisting, numRequired+1)
+		if err != nil {
+			return nil, nil, errors.Wrap(err, "could not create deterministic keys: ")
+		}
+		privKeys = append(privKeys, secretKeys[:len(secretKeys)-1]...)
+
+		// Create the new deposits and add them to the trie. Always use the first validator to create deposit
+		for i := uint64(0); i < numRequired; i++ {
+			withdrawalCreds := hashutil.Hash(publicKeys[1].Marshal())
+			withdrawalCreds[0] = params.BeaconConfig().BLSWithdrawalPrefixByte
+
+			depositData := &ethpb.Deposit_Data{
+				PublicKey:             publicKeys[1].Marshal(),
+				Amount:                params.BeaconConfig().MaxEffectiveBalance,
+				WithdrawalCredentials: withdrawalCreds[:],
+			}
+
+			domain := bls.ComputeDomain(params.BeaconConfig().DomainDeposit)
+			root, err := ssz.SigningRoot(depositData)
+			if err != nil {
+				return nil, nil, errors.Wrap(err, "could not get signing root of deposit data")
+			}
+			// Always use the same validator to sign
+			depositData.Signature = secretKeys[1].Sign(root[:], domain).Marshal()
+
+			deposit := &ethpb.Deposit{
+				Data: depositData,
+			}
+			cachedDeposits = append(cachedDeposits, deposit)
+
+			hashedDeposit, err := ssz.HashTreeRoot(deposit.Data)
+			if err != nil {
+				return nil, nil, errors.Wrap(err, "could not tree hash deposit data")
+			}
+
+			trie.Insert(hashedDeposit[:], int(numExisting+i))
+		}
+	}
+
+	depositTrie, _, err := DeterministicDepositTrie(int(numDeposits))
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "failed to create deposit trie")
+	}
+	requestedDeposits := cachedDeposits[:numDeposits]
+	for i := range requestedDeposits {
+		proof, err := depositTrie.MerkleProof(int(i))
+		if err != nil {
+			return nil, nil, errors.Wrap(err, "could not create merkle proof")
+		}
+		requestedDeposits[i].Proof = proof
+	}
+
+	return requestedDeposits, privKeys[0:numDeposits], nil
+}