Do not verify block data when calculating rewards (#15819)

* Do not verify block data when calculating rewards

* remove `Get` from function names

* changelog <3

* do not verify sync committee sig in handler

* Revert "remove `Get` from function names"

This reverts commit 770a89d990.

* typo fix

---------

Co-authored-by: james-prysm <90280386+james-prysm@users.noreply.github.com>

beacon-chain/core/altair/block.go

@@ -49,13 +49,22 @@ func ProcessSyncAggregate(ctx context.Context, s state.BeaconState, sync *ethpb.
 	if err != nil {
 		return nil, 0, errors.Wrap(err, "could not filter sync committee votes")
 	}
-
 	if err := VerifySyncCommitteeSig(s, votedKeys, sync.SyncCommitteeSignature); err != nil {
 		return nil, 0, errors.Wrap(err, "could not verify sync committee signature")
 	}
 	return s, reward, nil
 }
 
+// ProcessSyncAggregateNoVerifySig processes the sync aggregate without verifying the sync committee signature.
+// This is useful in scenarios such as block reward calculation, where we can assume the data in the block is valid.
+func ProcessSyncAggregateNoVerifySig(ctx context.Context, s state.BeaconState, sync *ethpb.SyncAggregate) (state.BeaconState, uint64, error) {
+	s, _, reward, err := processSyncAggregate(ctx, s, sync)
+	if err != nil {
+		return nil, 0, errors.Wrap(err, "could not filter sync committee votes")
+	}
+	return s, reward, nil
+}
+
 // processSyncAggregate applies all the logic in the spec function `process_sync_aggregate` except
 // verifying the BLS signatures. It returns the modified beacons state, the list of validators'
 // public keys that voted (for future signature verification) and the proposer reward for including

beacon-chain/core/altair/block_test.go

@@ -53,9 +53,19 @@ func TestProcessSyncCommittee_PerfectParticipation(t *testing.T) {
 		SyncCommitteeSignature: aggregatedSig,
 	}
 
+	// Verify that ProcessSyncAggregateNoVerifySig and ProcessSyncAggregate have the same outcome.
+	beaconStateNoVerifySig := beaconState.Copy()
+	beaconStateNoVerifySig, rewardNoVerifySig, err := altair.ProcessSyncAggregateNoVerifySig(t.Context(), beaconStateNoVerifySig, syncAggregate)
+	require.NoError(t, err)
+	sszNoVerifySig, err := beaconStateNoVerifySig.MarshalSSZ()
+	require.NoError(t, err)
 	var reward uint64
 	beaconState, reward, err = altair.ProcessSyncAggregate(t.Context(), beaconState, syncAggregate)
 	require.NoError(t, err)
+	ssz, err := beaconState.MarshalSSZ()
+	require.NoError(t, err)
+	assert.DeepEqual(t, sszNoVerifySig, ssz, "States resulting from ProcessSyncAggregateNoVerifySig and ProcessSyncAggregate are not equal")
+	assert.Equal(t, rewardNoVerifySig, reward, "Rewards resulting from ProcessSyncAggregateNoVerifySig and ProcessSyncAggregate are not equal")
 	assert.Equal(t, uint64(72192), reward)
 
 	// Use a non-sync committee index to compare profitability.

beacon-chain/core/blocks/attester_slashing.go

@@ -55,6 +55,28 @@ func ProcessAttesterSlashings(
 	return beaconState, nil
 }
 
+// ProcessAttesterSlashingsNoVerify processes attester slashings without verifying them.
+// This is useful in scenarios such as block reward calculation, where we can assume the data
+// in the block is valid.
+func ProcessAttesterSlashingsNoVerify(
+	ctx context.Context,
+	beaconState state.BeaconState,
+	slashings []ethpb.AttSlashing,
+	exitInfo *validators.ExitInfo,
+) (state.BeaconState, error) {
+	if exitInfo == nil && len(slashings) > 0 {
+		return nil, errors.New("exit info required to process attester slashings")
+	}
+	var err error
+	for _, slashing := range slashings {
+		beaconState, err = ProcessAttesterSlashingNoVerify(ctx, beaconState, slashing, exitInfo)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return beaconState, nil
+}
+
 // ProcessAttesterSlashing processes individual attester slashing.
 func ProcessAttesterSlashing(
 	ctx context.Context,
@@ -68,6 +90,30 @@ func ProcessAttesterSlashing(
 	if err := VerifyAttesterSlashing(ctx, beaconState, slashing); err != nil {
 		return nil, errors.Wrap(err, "could not verify attester slashing")
 	}
+	return processAttesterSlashing(ctx, beaconState, slashing, exitInfo)
+}
+
+// ProcessAttesterSlashingNoVerify processes individual attester slashing without verifying it.
+// This is useful in scenarios such as block reward calculation, where we can assume the data
+// in the block is valid.
+func ProcessAttesterSlashingNoVerify(
+	ctx context.Context,
+	beaconState state.BeaconState,
+	slashing ethpb.AttSlashing,
+	exitInfo *validators.ExitInfo,
+) (state.BeaconState, error) {
+	if exitInfo == nil {
+		return nil, errors.New("exit info is required to process attester slashing")
+	}
+	return processAttesterSlashing(ctx, beaconState, slashing, exitInfo)
+}
+
+func processAttesterSlashing(
+	ctx context.Context,
+	beaconState state.BeaconState,
+	slashing ethpb.AttSlashing,
+	exitInfo *validators.ExitInfo,
+) (state.BeaconState, error) {
 	slashableIndices := SlashableAttesterIndices(slashing)
 	sort.SliceStable(slashableIndices, func(i, j int) bool {
 		return slashableIndices[i] < slashableIndices[j]

beacon-chain/core/blocks/attester_slashing_test.go

@@ -242,8 +242,18 @@ func TestProcessAttesterSlashings_AppliesCorrectStatus(t *testing.T) {
 			currentSlot := 2 * params.BeaconConfig().SlotsPerEpoch
 			require.NoError(t, tc.st.SetSlot(currentSlot))
 
+			// Verify that ProcessAttesterSlashingsNoVerify and ProcessAttesterSlashings have the same outcome.
+			stNoVerify := tc.st.Copy()
+			newStateNoVerify, err := blocks.ProcessAttesterSlashingsNoVerify(t.Context(), stNoVerify, []ethpb.AttSlashing{tc.slashing}, v.ExitInformation(stNoVerify))
+			require.NoError(t, err)
+			sszNoVerify, err := newStateNoVerify.MarshalSSZ()
+			require.NoError(t, err)
 			newState, err := blocks.ProcessAttesterSlashings(t.Context(), tc.st, []ethpb.AttSlashing{tc.slashing}, v.ExitInformation(tc.st))
 			require.NoError(t, err)
+			ssz, err := newState.MarshalSSZ()
+			require.NoError(t, err)
+			assert.DeepEqual(t, sszNoVerify, ssz, "States resulting from ProcessAttesterSlashingsNoVerify and ProcessAttesterSlashings are not equal")
+
 			newRegistry := newState.Validators()
 
 			// Given the intersection of slashable indices is [1], only validator

beacon-chain/core/blocks/proposer_slashing.go

@@ -64,6 +64,28 @@ func ProcessProposerSlashings(
 	return beaconState, nil
 }
 
+// ProcessProposerSlashingsNoVerify processes proposer slashings without verifying them.
+// This is useful in scenarios such as block reward calculation, where we can assume the data
+// in the block is valid.
+func ProcessProposerSlashingsNoVerify(
+	ctx context.Context,
+	beaconState state.BeaconState,
+	slashings []*ethpb.ProposerSlashing,
+	exitInfo *validators.ExitInfo,
+) (state.BeaconState, error) {
+	if exitInfo == nil && len(slashings) > 0 {
+		return nil, errors.New("exit info required to process proposer slashings")
+	}
+	var err error
+	for _, slashing := range slashings {
+		beaconState, err = ProcessProposerSlashingNoVerify(ctx, beaconState, slashing, exitInfo)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return beaconState, nil
+}
+
 // ProcessProposerSlashing processes individual proposer slashing.
 func ProcessProposerSlashing(
 	ctx context.Context,
@@ -71,16 +93,40 @@ func ProcessProposerSlashing(
 	slashing *ethpb.ProposerSlashing,
 	exitInfo *validators.ExitInfo,
 ) (state.BeaconState, error) {
-	var err error
 	if slashing == nil {
 		return nil, errors.New("nil proposer slashings in block body")
 	}
-	if err = VerifyProposerSlashing(beaconState, slashing); err != nil {
+	if err := VerifyProposerSlashing(beaconState, slashing); err != nil {
 		return nil, errors.Wrap(err, "could not verify proposer slashing")
 	}
+	return processProposerSlashing(ctx, beaconState, slashing, exitInfo)
+}
+
+// ProcessProposerSlashingNoVerify processes individual proposer slashing without verifying it.
+// This is useful in scenarios such as block reward calculation, where we can assume the data
+// in the block is valid.
+func ProcessProposerSlashingNoVerify(
+	ctx context.Context,
+	beaconState state.BeaconState,
+	slashing *ethpb.ProposerSlashing,
+	exitInfo *validators.ExitInfo,
+) (state.BeaconState, error) {
+	if slashing == nil {
+		return nil, errors.New("nil proposer slashings in block body")
+	}
+	return processProposerSlashing(ctx, beaconState, slashing, exitInfo)
+}
+
+func processProposerSlashing(
+	ctx context.Context,
+	beaconState state.BeaconState,
+	slashing *ethpb.ProposerSlashing,
+	exitInfo *validators.ExitInfo,
+) (state.BeaconState, error) {
 	if exitInfo == nil {
 		return nil, errors.New("exit info is required to process proposer slashing")
 	}
+	var err error
 	beaconState, err = validators.SlashValidator(ctx, beaconState, slashing.Header_1.Header.ProposerIndex, exitInfo)
 	if err != nil {
 		return nil, errors.Wrapf(err, "could not slash proposer index %d", slashing.Header_1.Header.ProposerIndex)

beacon-chain/core/blocks/proposer_slashing_test.go

@@ -172,8 +172,17 @@ func TestProcessProposerSlashings_AppliesCorrectStatus(t *testing.T) {
 	block := util.NewBeaconBlock()
 	block.Block.Body.ProposerSlashings = slashings
 
+	// Verify that ProcessProposerSlashingsNoVerify and ProcessProposerSlashings have the same outcome.
+	beaconStateNoVerify := beaconState.Copy()
+	newStateNoVerify, err := blocks.ProcessProposerSlashingsNoVerify(t.Context(), beaconStateNoVerify, block.Block.Body.ProposerSlashings, v.ExitInformation(beaconStateNoVerify))
+	require.NoError(t, err)
+	sszNoVerify, err := newStateNoVerify.MarshalSSZ()
+	require.NoError(t, err)
 	newState, err := blocks.ProcessProposerSlashings(t.Context(), beaconState, block.Block.Body.ProposerSlashings, v.ExitInformation(beaconState))
 	require.NoError(t, err)
+	ssz, err := newState.MarshalSSZ()
+	require.NoError(t, err)
+	assert.DeepEqual(t, sszNoVerify, ssz, "States resulting from ProcessProposerSlashingsNoVerify and ProcessProposerSlashings are not equal")
 
 	newStateVals := newState.Validators()
 	if newStateVals[1].ExitEpoch != beaconState.Validators()[1].ExitEpoch {

beacon-chain/rpc/eth/rewards/handlers.go

@@ -151,7 +151,7 @@ func (s *Server) SyncCommitteeRewards(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	_, proposerReward, err := altair.ProcessSyncAggregate(r.Context(), st, sa)
+	_, proposerReward, err := altair.ProcessSyncAggregateNoVerifySig(r.Context(), st, sa)
 	if err != nil {
 		httputil.HandleError(w, "Could not get sync aggregate rewards: "+err.Error(), http.StatusInternalServerError)
 		return

beacon-chain/rpc/eth/rewards/service.go

@@ -73,7 +73,7 @@ func (rs *BlockRewardService) GetBlockRewardsData(ctx context.Context, blk inter
 		// ExitInformation is expensive to compute, only do it if we need it.
 		exitInfo = validators.ExitInformation(st)
 	}
-	st, err = coreblocks.ProcessAttesterSlashings(ctx, st, blk.Body().AttesterSlashings(), exitInfo)
+	st, err = coreblocks.ProcessAttesterSlashingsNoVerify(ctx, st, blk.Body().AttesterSlashings(), exitInfo)
 	if err != nil {
 		return nil, &httputil.DefaultJsonError{
 			Message: "Could not get attester slashing rewards: " + err.Error(),
@@ -87,7 +87,7 @@ func (rs *BlockRewardService) GetBlockRewardsData(ctx context.Context, blk inter
 			Code:    http.StatusInternalServerError,
 		}
 	}
-	st, err = coreblocks.ProcessProposerSlashings(ctx, st, blk.Body().ProposerSlashings(), exitInfo)
+	st, err = coreblocks.ProcessProposerSlashingsNoVerify(ctx, st, blk.Body().ProposerSlashings(), exitInfo)
 	if err != nil {
 		return nil, &httputil.DefaultJsonError{
 			Message: "Could not get proposer slashing rewards: " + err.Error(),
@@ -109,7 +109,7 @@ func (rs *BlockRewardService) GetBlockRewardsData(ctx context.Context, blk inter
 		}
 	}
 	var syncCommitteeReward uint64
-	_, syncCommitteeReward, err = altair.ProcessSyncAggregate(ctx, st, sa)
+	_, syncCommitteeReward, err = altair.ProcessSyncAggregateNoVerifySig(ctx, st, sa)
 	if err != nil {
 		return nil, &httputil.DefaultJsonError{
 			Message: "Could not get sync aggregate rewards: " + err.Error(),

changelog/radek_rewards-no-verify.md

@@ -0,0 +1,3 @@
+### Changed
+
+- Do not verify block data when calculating rewards.