Do not verify block signature on block processing (#14820)

Verifying the block signature adds a batch and performs a full hash of
the block unnecessarily.

beacon-chain/blockchain/process_block_test.go

@@ -820,24 +820,6 @@ func TestOnBlock_NilBlock(t *testing.T) {
 	require.Equal(t, true, IsInvalidBlock(err))
 }
 
-func TestOnBlock_InvalidSignature(t *testing.T) {
-	service, tr := minimalTestService(t)
-	ctx := tr.ctx
-
-	gs, keys := util.DeterministicGenesisState(t, 32)
-	require.NoError(t, service.saveGenesisData(ctx, gs))
-
-	blk, err := util.GenerateFullBlock(gs, keys, util.DefaultBlockGenConfig(), 1)
-	require.NoError(t, err)
-	blk.Signature = []byte{'a'} // Mutate the signature.
-	wsb, err := consensusblocks.NewSignedBeaconBlock(blk)
-	require.NoError(t, err)
-	preState, err := service.getBlockPreState(ctx, wsb.Block())
-	require.NoError(t, err)
-	_, err = service.validateStateTransition(ctx, preState, wsb)
-	require.Equal(t, true, IsInvalidBlock(err))
-}
-
 func TestOnBlock_CallNewPayloadAndForkchoiceUpdated(t *testing.T) {
 	params.SetupTestConfigCleanup(t)
 	config := params.BeaconConfig()

beacon-chain/core/blocks/header_test.go

@@ -290,52 +290,3 @@ func TestProcessBlockHeader_OK(t *testing.T) {
 	}
 	assert.Equal(t, true, proto.Equal(nsh, expected), "Expected %v, received %v", expected, nsh)
 }
-
-func TestBlockSignatureSet_OK(t *testing.T) {
-	validators := make([]*ethpb.Validator, params.BeaconConfig().MinGenesisActiveValidatorCount)
-	for i := range validators {
-		validators[i] = &ethpb.Validator{
-			PublicKey:             make([]byte, 32),
-			WithdrawalCredentials: make([]byte, 32),
-			ExitEpoch:             params.BeaconConfig().FarFutureEpoch,
-			Slashed:               true,
-		}
-	}
-
-	state, err := util.NewBeaconState()
-	require.NoError(t, err)
-	require.NoError(t, state.SetValidators(validators))
-	require.NoError(t, state.SetSlot(10))
-	require.NoError(t, state.SetLatestBlockHeader(util.HydrateBeaconHeader(&ethpb.BeaconBlockHeader{
-		Slot:          9,
-		ProposerIndex: 0,
-	})))
-
-	latestBlockSignedRoot, err := state.LatestBlockHeader().HashTreeRoot()
-	require.NoError(t, err)
-
-	currentEpoch := time.CurrentEpoch(state)
-	priv, err := bls.RandKey()
-	require.NoError(t, err)
-	pID, err := helpers.BeaconProposerIndex(t.Context(), state)
-	require.NoError(t, err)
-	block := util.NewBeaconBlock()
-	block.Block.Slot = 10
-	block.Block.ProposerIndex = pID
-	block.Block.Body.RandaoReveal = bytesutil.PadTo([]byte{'A', 'B', 'C'}, 96)
-	block.Block.ParentRoot = latestBlockSignedRoot[:]
-	block.Signature, err = signing.ComputeDomainAndSign(state, currentEpoch, block.Block, params.BeaconConfig().DomainBeaconProposer, priv)
-	require.NoError(t, err)
-	proposerIdx, err := helpers.BeaconProposerIndex(t.Context(), state)
-	require.NoError(t, err)
-	validators[proposerIdx].Slashed = false
-	validators[proposerIdx].PublicKey = priv.PublicKey().Marshal()
-	err = state.UpdateValidatorAtIndex(proposerIdx, validators[proposerIdx])
-	require.NoError(t, err)
-	set, err := blocks.BlockSignatureBatch(state, block.Block.ProposerIndex, block.Signature, block.Block.HashTreeRoot)
-	require.NoError(t, err)
-
-	verified, err := set.Verify()
-	require.NoError(t, err)
-	assert.Equal(t, true, verified, "Block signature set returned a set which was unable to be verified")
-}

beacon-chain/core/blocks/signature.go

@@ -122,24 +122,6 @@ func VerifyBlockSignatureUsingCurrentFork(beaconState state.ReadOnlyBeaconState,
 	return nil
 }
 
-// BlockSignatureBatch retrieves the block signature batch from the provided block and its corresponding state.
-func BlockSignatureBatch(beaconState state.ReadOnlyBeaconState,
-	proposerIndex primitives.ValidatorIndex,
-	sig []byte,
-	rootFunc func() ([32]byte, error)) (*bls.SignatureBatch, error) {
-	currentEpoch := slots.ToEpoch(beaconState.Slot())
-	domain, err := signing.Domain(beaconState.Fork(), currentEpoch, params.BeaconConfig().DomainBeaconProposer, beaconState.GenesisValidatorsRoot())
-	if err != nil {
-		return nil, err
-	}
-	proposer, err := beaconState.ValidatorAtIndex(proposerIndex)
-	if err != nil {
-		return nil, err
-	}
-	proposerPubKey := proposer.PublicKey
-	return signing.BlockSignatureBatch(proposerPubKey, sig, domain, rootFunc)
-}
-
 // RandaoSignatureBatch retrieves the relevant randao specific signature batch object
 // from a block and its corresponding state.
 func RandaoSignatureBatch(

beacon-chain/core/transition/transition_no_verify_sig.go

@@ -182,12 +182,6 @@ func ProcessBlockNoVerifyAnySig(
 		return nil, nil, err
 	}
 
-	sig := signed.Signature()
-	bSet, err := b.BlockSignatureBatch(st, blk.ProposerIndex(), sig[:], blk.HashTreeRoot)
-	if err != nil {
-		tracing.AnnotateError(span, err)
-		return nil, nil, errors.Wrap(err, "could not retrieve block signature set")
-	}
 	randaoReveal := signed.Block().Body().RandaoReveal()
 	rSet, err := b.RandaoSignatureBatch(ctx, st, randaoReveal[:])
 	if err != nil {
@@ -201,7 +195,7 @@ func ProcessBlockNoVerifyAnySig(
 
 	// Merge beacon block, randao and attestations signatures into a set.
 	set := bls.NewSet()
-	set.Join(bSet).Join(rSet).Join(aSet)
+	set.Join(rSet).Join(aSet)
 
 	if blk.Version() >= version.Capella {
 		changes, err := signed.Block().Body().BLSToExecutionChanges()

beacon-chain/core/transition/transition_no_verify_sig_test.go

@@ -157,9 +157,8 @@ func TestProcessBlockNoVerify_SigSetContainsDescriptions(t *testing.T) {
 	set, _, err := transition.ProcessBlockNoVerifyAnySig(t.Context(), beaconState, wsb)
 	require.NoError(t, err)
 	assert.Equal(t, len(set.Signatures), len(set.Descriptions), "Signatures and descriptions do not match up")
-	assert.Equal(t, "block signature", set.Descriptions[0])
-	assert.Equal(t, "randao signature", set.Descriptions[1])
-	assert.Equal(t, "attestation signature", set.Descriptions[2])
+	assert.Equal(t, "randao signature", set.Descriptions[0])
+	assert.Equal(t, "attestation signature", set.Descriptions[1])
 }
 
 func TestProcessOperationsNoVerifyAttsSigs_OK(t *testing.T) {

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

@@ -14,6 +14,7 @@ import (
 	"github.com/OffchainLabs/prysm/v7/api"
 	"github.com/OffchainLabs/prysm/v7/api/server/structs"
 	"github.com/OffchainLabs/prysm/v7/beacon-chain/blockchain/kzg"
+	coreblocks "github.com/OffchainLabs/prysm/v7/beacon-chain/core/blocks"
 	corehelpers "github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers"
 	"github.com/OffchainLabs/prysm/v7/beacon-chain/core/transition"
 	"github.com/OffchainLabs/prysm/v7/beacon-chain/db/filters"
@@ -957,6 +958,13 @@ func (s *Server) validateConsensus(ctx context.Context, b *eth.GenericSignedBeac
 			}
 		}
 	}
+	blockRoot, err := blk.Block().HashTreeRoot()
+	if err != nil {
+		return errors.Wrap(err, "could not hash block")
+	}
+	if err := coreblocks.VerifyBlockSignatureUsingCurrentFork(parentState, blk, blockRoot); err != nil {
+		return errors.Wrap(err, "could not verify block signature")
+	}
 	_, err = transition.ExecuteStateTransition(ctx, parentState, blk)
 	if err != nil {
 		return errors.Wrap(err, "could not execute state transition")