Refactor Sync Committee Rewards (#11696)

beacon-chain/core/altair/BUILD.bazel

@@ -59,6 +59,7 @@ go_test(
         "deposit_test.go",
         "epoch_precompute_test.go",
         "epoch_spec_test.go",
+        "exports_test.go",
         "reward_test.go",
         "sync_committee_test.go",
         "transition_test.go",

beacon-chain/core/altair/block.go

@@ -9,7 +9,6 @@ import (
 	p2pType "github.com/prysmaticlabs/prysm/v3/beacon-chain/p2p/types"
 	"github.com/prysmaticlabs/prysm/v3/beacon-chain/state"
 	"github.com/prysmaticlabs/prysm/v3/config/params"
-	types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives"
 	"github.com/prysmaticlabs/prysm/v3/crypto/bls"
 	"github.com/prysmaticlabs/prysm/v3/encoding/bytesutil"
 	ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1"
@@ -46,7 +45,7 @@ import (
 //	    else:
 //	        decrease_balance(state, participant_index, participant_reward)
 func ProcessSyncAggregate(ctx context.Context, s state.BeaconState, sync *ethpb.SyncAggregate) (state.BeaconState, error) {
-	votedKeys, votedIndices, didntVoteIndices, err := FilterSyncCommitteeVotes(s, sync)
+	s, votedKeys, err := processSyncAggregate(ctx, s, sync)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not filter sync committee votes")
 	}
@@ -54,50 +53,70 @@ func ProcessSyncAggregate(ctx context.Context, s state.BeaconState, sync *ethpb.
 	if err := VerifySyncCommitteeSig(s, votedKeys, sync.SyncCommitteeSignature); err != nil {
 		return nil, errors.Wrap(err, "could not verify sync committee signature")
 	}
-
-	return ApplySyncRewardsPenalties(ctx, s, votedIndices, didntVoteIndices)
+	return s, nil
 }
 
-// FilterSyncCommitteeVotes filters the validator public keys and indices for the ones that voted and didn't vote.
-func FilterSyncCommitteeVotes(s state.BeaconState, sync *ethpb.SyncAggregate) (
-	votedKeys []bls.PublicKey,
-	votedIndices []types.ValidatorIndex,
-	didntVoteIndices []types.ValidatorIndex,
-	err error) {
+// processSyncAggregate applies all the logic in the spec function `process_sync_aggregate` except
+// verifying the BLS signatures. It returns the modified beacons state and the list of validators'
+// public keys that voted, for future signature verification.
+func processSyncAggregate(ctx context.Context, s state.BeaconState, sync *ethpb.SyncAggregate) (
+	state.BeaconState,
+	[]bls.PublicKey,
+	error) {
 	currentSyncCommittee, err := s.CurrentSyncCommittee()
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 	if currentSyncCommittee == nil {
-		return nil, nil, nil, errors.New("nil current sync committee in state")
+		return nil, nil, errors.New("nil current sync committee in state")
 	}
 	committeeKeys := currentSyncCommittee.Pubkeys
 	if sync.SyncCommitteeBits.Len() > uint64(len(committeeKeys)) {
-		return nil, nil, nil, errors.New("bits length exceeds committee length")
+		return nil, nil, errors.New("bits length exceeds committee length")
 	}
-	votedKeys = make([]bls.PublicKey, 0, len(committeeKeys))
-	votedIndices = make([]types.ValidatorIndex, 0, len(committeeKeys))
-	didntVoteIndices = make([]types.ValidatorIndex, 0) // No allocation. Expect most votes.
+	votedKeys := make([]bls.PublicKey, 0, len(committeeKeys))
 
+	activeBalance, err := helpers.TotalActiveBalance(s)
+	if err != nil {
+		return nil, nil, err
+	}
+	proposerReward, participantReward, err := SyncRewards(activeBalance)
+	if err != nil {
+		return nil, nil, err
+	}
+	proposerIndex, err := helpers.BeaconProposerIndex(ctx, s)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	earnedProposerReward := uint64(0)
 	for i := uint64(0); i < sync.SyncCommitteeBits.Len(); i++ {
 		vIdx, exists := s.ValidatorIndexByPubkey(bytesutil.ToBytes48(committeeKeys[i]))
 		// Impossible scenario.
 		if !exists {
-			return nil, nil, nil, errors.New("validator public key does not exist in state")
+			return nil, nil, errors.New("validator public key does not exist in state")
 		}
 
 		if sync.SyncCommitteeBits.BitAt(i) {
 			pubKey, err := bls.PublicKeyFromBytes(committeeKeys[i])
 			if err != nil {
-				return nil, nil, nil, err
+				return nil, nil, err
 			}
 			votedKeys = append(votedKeys, pubKey)
-			votedIndices = append(votedIndices, vIdx)
+			if err := helpers.IncreaseBalance(s, vIdx, participantReward); err != nil {
+				return nil, nil, err
+			}
+			earnedProposerReward += proposerReward
 		} else {
-			didntVoteIndices = append(didntVoteIndices, vIdx)
+			if err := helpers.DecreaseBalance(s, vIdx, participantReward); err != nil {
+				return nil, nil, err
+			}
 		}
 	}
-	return
+	if err := helpers.IncreaseBalance(s, proposerIndex, earnedProposerReward); err != nil {
+		return nil, nil, err
+	}
+	return s, votedKeys, err
 }
 
 // VerifySyncCommitteeSig verifies sync committee signature `syncSig` is valid with respect to public keys `syncKeys`.
@@ -126,43 +145,6 @@ func VerifySyncCommitteeSig(s state.BeaconState, syncKeys []bls.PublicKey, syncS
 	return nil
 }
 
-// ApplySyncRewardsPenalties applies rewards and penalties for proposer and sync committee participants.
-func ApplySyncRewardsPenalties(ctx context.Context, s state.BeaconState, votedIndices, didntVoteIndices []types.ValidatorIndex) (state.BeaconState, error) {
-	activeBalance, err := helpers.TotalActiveBalance(s)
-	if err != nil {
-		return nil, err
-	}
-	proposerReward, participantReward, err := SyncRewards(activeBalance)
-	if err != nil {
-		return nil, err
-	}
-
-	// Apply sync committee rewards.
-	earnedProposerReward := uint64(0)
-	for _, index := range votedIndices {
-		if err := helpers.IncreaseBalance(s, index, participantReward); err != nil {
-			return nil, err
-		}
-		earnedProposerReward += proposerReward
-	}
-	// Apply proposer rewards.
-	proposerIndex, err := helpers.BeaconProposerIndex(ctx, s)
-	if err != nil {
-		return nil, err
-	}
-	if err := helpers.IncreaseBalance(s, proposerIndex, earnedProposerReward); err != nil {
-		return nil, err
-	}
-	// Apply sync committee penalties.
-	for _, index := range didntVoteIndices {
-		if err := helpers.DecreaseBalance(s, index, participantReward); err != nil {
-			return nil, err
-		}
-	}
-
-	return s, nil
-}
-
 // SyncRewards returns the proposer reward and the sync participant reward given the total active balance in state.
 func SyncRewards(activeBalance uint64) (proposerReward, participantReward uint64, err error) {
 	cfg := params.BeaconConfig()

beacon-chain/core/altair/block_test.go

@@ -168,7 +168,36 @@ func TestProcessSyncCommittee_MixParticipation_GoodSignature(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func TestProcessSyncCommittee_FilterSyncCommitteeVotes(t *testing.T) {
+// This is a regression test #11696
+func TestProcessSyncCommittee_DontPrecompute(t *testing.T) {
+	beaconState, _ := util.DeterministicGenesisStateAltair(t, params.BeaconConfig().MaxValidatorsPerCommittee)
+	require.NoError(t, beaconState.SetSlot(1))
+	committee, err := altair.NextSyncCommittee(context.Background(), beaconState)
+	require.NoError(t, err)
+	committeeKeys := committee.Pubkeys
+	committeeKeys[1] = committeeKeys[0]
+	require.NoError(t, beaconState.SetCurrentSyncCommittee(committee))
+	idx, ok := beaconState.ValidatorIndexByPubkey(bytesutil.ToBytes48(committeeKeys[0]))
+	require.Equal(t, true, ok)
+
+	syncBits := bitfield.NewBitvector512()
+	for i := range syncBits {
+		syncBits[i] = 0xFF
+	}
+	syncBits.SetBitAt(0, false)
+	syncAggregate := &ethpb.SyncAggregate{
+		SyncCommitteeBits: syncBits,
+	}
+	require.NoError(t, beaconState.UpdateBalancesAtIndex(idx, 0))
+	st, votedKeys, err := altair.ProcessSyncAggregateEported(context.Background(), beaconState, syncAggregate)
+	require.NoError(t, err)
+	require.Equal(t, 511, len(votedKeys))
+	require.DeepEqual(t, committeeKeys[0], votedKeys[0].Marshal())
+	balances := st.Balances()
+	require.Equal(t, uint64(988), balances[idx])
+}
+
+func TestProcessSyncCommittee_processSyncAggregate(t *testing.T) {
 	beaconState, _ := util.DeterministicGenesisStateAltair(t, params.BeaconConfig().MaxValidatorsPerCommittee)
 	require.NoError(t, beaconState.SetSlot(1))
 	committee, err := altair.NextSyncCommittee(context.Background(), beaconState)
@@ -183,25 +212,40 @@ func TestProcessSyncCommittee_FilterSyncCommitteeVotes(t *testing.T) {
 		SyncCommitteeBits: syncBits,
 	}
 
-	votedKeys, votedIndices, didntVoteIndices, err := altair.FilterSyncCommitteeVotes(beaconState, syncAggregate)
+	st, votedKeys, err := altair.ProcessSyncAggregateEported(context.Background(), beaconState, syncAggregate)
 	require.NoError(t, err)
 	votedMap := make(map[[fieldparams.BLSPubkeyLength]byte]bool)
 	for _, key := range votedKeys {
 		votedMap[bytesutil.ToBytes48(key.Marshal())] = true
 	}
 	require.Equal(t, int(syncBits.Len()/2), len(votedKeys))
-	require.Equal(t, int(syncBits.Len()/2), len(votedIndices))
-	require.Equal(t, int(syncBits.Len()/2), len(didntVoteIndices))
+
+	currentSyncCommittee, err := st.CurrentSyncCommittee()
+	require.NoError(t, err)
+	committeeKeys := currentSyncCommittee.Pubkeys
+	balances := st.Balances()
+
+	proposerIndex, err := helpers.BeaconProposerIndex(context.Background(), beaconState)
+	require.NoError(t, err)
 
 	for i := 0; i < len(syncBits); i++ {
 		if syncBits.BitAt(uint64(i)) {
-			pk := beaconState.PubkeyAtIndex(votedIndices[i])
+			pk := bytesutil.ToBytes48(committeeKeys[i])
 			require.DeepEqual(t, true, votedMap[pk])
+			idx, ok := st.ValidatorIndexByPubkey(pk)
+			require.Equal(t, true, ok)
+			require.Equal(t, uint64(32000000988), balances[idx])
 		} else {
-			pk := beaconState.PubkeyAtIndex(didntVoteIndices[i])
+			pk := bytesutil.ToBytes48(committeeKeys[i])
 			require.DeepEqual(t, false, votedMap[pk])
+			idx, ok := st.ValidatorIndexByPubkey(pk)
+			require.Equal(t, true, ok)
+			if idx != proposerIndex {
+				require.Equal(t, uint64(31999999012), balances[idx])
+			}
 		}
 	}
+	require.Equal(t, uint64(32000035108), balances[proposerIndex])
 }
 
 func Test_VerifySyncCommitteeSig(t *testing.T) {
@@ -240,22 +284,6 @@ func Test_VerifySyncCommitteeSig(t *testing.T) {
 	require.NoError(t, altair.VerifySyncCommitteeSig(beaconState, pks, aggregatedSig))
 }
 
-func Test_ApplySyncRewardsPenalties(t *testing.T) {
-	beaconState, _ := util.DeterministicGenesisStateAltair(t, params.BeaconConfig().MaxValidatorsPerCommittee)
-	beaconState, err := altair.ApplySyncRewardsPenalties(context.Background(), beaconState,
-		[]types.ValidatorIndex{0, 1}, // voted
-		[]types.ValidatorIndex{2, 3}) // didn't vote
-	require.NoError(t, err)
-	balances := beaconState.Balances()
-	require.Equal(t, uint64(32000000988), balances[0])
-	require.Equal(t, balances[0], balances[1])
-	require.Equal(t, uint64(31999999012), balances[2])
-	require.Equal(t, balances[2], balances[3])
-	proposerIndex, err := helpers.BeaconProposerIndex(context.Background(), beaconState)
-	require.NoError(t, err)
-	require.Equal(t, uint64(32000000282), balances[proposerIndex])
-}
-
 func Test_SyncRewards(t *testing.T) {
 	tests := []struct {
 		name                  string

beacon-chain/core/altair/exports_test.go

@@ -0,0 +1,3 @@
+package altair
+
+var ProcessSyncAggregateEported = processSyncAggregate