Check for Signing Root Mismatch When Submitting Proposals and Importi…

…ng Proposals in Slashing Interchange (#8085)

* flexible signing root

* add test

* add tests

* fix test

* Preston's comments

* res tests

* ensure we consider the case for minimum proposals

* pass test

* tests passing

* rem unused code

beacon-chain/state/stateutil/attestations.go

@@ -84,66 +84,6 @@ func marshalAttestationData(data *ethpb.AttestationData) []byte {
 	return enc
 }
 
-func attestationRoot(hasher htrutils.HashFn, att *ethpb.Attestation) ([32]byte, error) {
-	fieldRoots := make([][32]byte, 3)
-
-	// Bitfield.
-	aggregationRoot, err := htrutils.BitlistRoot(hasher, att.AggregationBits, params.BeaconConfig().MaxValidatorsPerCommittee)
-	if err != nil {
-		return [32]byte{}, err
-	}
-	fieldRoots[0] = aggregationRoot
-
-	dataRoot, err := attestationDataRoot(hasher, att.Data)
-	if err != nil {
-		return [32]byte{}, err
-	}
-	fieldRoots[1] = dataRoot
-
-	signatureBuf := bytesutil.ToBytes96(att.Signature)
-	packedSig, err := htrutils.Pack([][]byte{signatureBuf[:]})
-	if err != nil {
-		return [32]byte{}, err
-	}
-	sigRoot, err := htrutils.BitwiseMerkleize(hasher, packedSig, uint64(len(packedSig)), uint64(len(packedSig)))
-	if err != nil {
-		return [32]byte{}, err
-	}
-	fieldRoots[2] = sigRoot
-	return htrutils.BitwiseMerkleizeArrays(hasher, fieldRoots, uint64(len(fieldRoots)), uint64(len(fieldRoots)))
-}
-
-func blockAttestationRoot(atts []*ethpb.Attestation) ([32]byte, error) {
-	hasher := hashutil.CustomSHA256Hasher()
-	roots := make([][]byte, len(atts))
-	for i := 0; i < len(atts); i++ {
-		pendingRoot, err := attestationRoot(hasher, atts[i])
-		if err != nil {
-			return [32]byte{}, errors.Wrap(err, "could not attestation merkleization")
-		}
-		roots[i] = pendingRoot[:]
-	}
-
-	attsRootsRoot, err := htrutils.BitwiseMerkleize(
-		hasher,
-		roots,
-		uint64(len(roots)),
-		params.BeaconConfig().MaxAttestations,
-	)
-	if err != nil {
-		return [32]byte{}, errors.Wrap(err, "could not compute block attestations merkleization")
-	}
-	attsLenBuf := new(bytes.Buffer)
-	if err := binary.Write(attsLenBuf, binary.LittleEndian, uint64(len(atts))); err != nil {
-		return [32]byte{}, errors.Wrap(err, "could not marshal epoch attestations length")
-	}
-	// We need to mix in the length of the slice.
-	attsLenRoot := make([]byte, 32)
-	copy(attsLenRoot, attsLenBuf.Bytes())
-	res := htrutils.MixInLength(attsRootsRoot, attsLenRoot)
-	return res, nil
-}
-
 func attestationDataRoot(hasher htrutils.HashFn, data *ethpb.AttestationData) ([32]byte, error) {
 	fieldRoots := make([][]byte, 5)
 

validator/client/propose.go

@@ -80,13 +80,6 @@ func (v *validator) ProposeBlock(ctx context.Context, slot uint64, pubKey [48]by
 		return
 	}
 
-	if err := v.preBlockSignValidations(ctx, pubKey, b); err != nil {
-		log.WithFields(
-			blockLogFields(pubKey, b, nil),
-		).WithError(err).Error("Failed block slashing protection check")
-		return
-	}
-
 	// Sign returned block from beacon node
 	sig, domain, err := v.signBlock(ctx, pubKey, epoch, b)
 	if err != nil {
@@ -101,7 +94,23 @@ func (v *validator) ProposeBlock(ctx context.Context, slot uint64, pubKey [48]by
 		Signature: sig,
 	}
 
-	if err := v.postBlockSignUpdate(ctx, pubKey, blk, domain); err != nil {
+	signingRoot, err := helpers.ComputeSigningRoot(b, domain.SignatureDomain)
+	if err != nil {
+		if v.emitAccountMetrics {
+			ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc()
+		}
+		log.WithError(err).Error("Failed to compute signing root for block")
+		return
+	}
+
+	if err := v.preBlockSignValidations(ctx, pubKey, b, signingRoot); err != nil {
+		log.WithFields(
+			blockLogFields(pubKey, b, nil),
+		).WithError(err).Error("Failed block slashing protection check")
+		return
+	}
+
+	if err := v.postBlockSignUpdate(ctx, pubKey, blk, signingRoot); err != nil {
 		log.WithFields(
 			blockLogFields(pubKey, b, sig),
 		).WithError(err).Error("Failed block slashing protection check")

validator/client/propose_protect.go

@@ -6,50 +6,58 @@ import (
 
 	"github.com/pkg/errors"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
-	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
 	"github.com/prysmaticlabs/prysm/shared/blockutil"
 	"github.com/prysmaticlabs/prysm/shared/featureconfig"
+	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/sirupsen/logrus"
 )
 
 var failedPreBlockSignLocalErr = "attempted to sign a double proposal, block rejected by local protection"
 var failedPreBlockSignExternalErr = "attempted a double proposal, block rejected by remote slashing protection"
 var failedPostBlockSignErr = "made a double proposal, considered slashable by remote slashing protection"
 
-func (v *validator) preBlockSignValidations(ctx context.Context, pubKey [48]byte, block *ethpb.BeaconBlock) error {
+func (v *validator) preBlockSignValidations(
+	ctx context.Context, pubKey [48]byte, block *ethpb.BeaconBlock, signingRoot [32]byte,
+) error {
 	fmtKey := fmt.Sprintf("%#x", pubKey[:])
 
-	// Based on EIP3076, validator should refuse to sign any proposal with slot less
-	// than or equal to the minimum signed proposal present in the DB for that public key.
-	lowestSignedProposalSlot, exists, err := v.db.LowestSignedProposal(ctx, pubKey)
-	if err != nil {
-		return err
-	}
-	if exists && lowestSignedProposalSlot >= block.Slot {
-		return fmt.Errorf(
-			"could not sign block with slot <= lowest signed slot in db, lowest signed slot: %d >= block slot: %d",
-			lowestSignedProposalSlot,
-			block.Slot,
-		)
-	}
-
-	_, exists, err = v.db.ProposalHistoryForSlot(ctx, pubKey, block.Slot)
+	prevSigningRoot, proposalAtSlotExists, err := v.db.ProposalHistoryForSlot(ctx, pubKey, block.Slot)
 	if err != nil {
 		if v.emitAccountMetrics {
 			ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc()
 		}
 		return errors.Wrap(err, "failed to get proposal history")
 	}
-	// If a proposal exists in our history for the slot, we assume it is slashable.
-	// TODO(#7848): Add a more sophisticated strategy where if we indeed have the signing root,
-	// only blocks that have a conflicting signing root with a historical proposal are slashable.
-	if exists {
+
+	lowestSignedProposalSlot, lowestProposalExists, err := v.db.LowestSignedProposal(ctx, pubKey)
+	if err != nil {
+		return err
+	}
+
+	// If a proposal exists in our history for the slot, we check the following:
+	// If the signing root is empty (zero hash), then we consider it slashable. If signing root is not empty,
+	// we check if it is different than the incoming block's signing root. If that is the case,
+	// we consider that proposal slashable.
+	signingRootIsDifferent := prevSigningRoot == params.BeaconConfig().ZeroHash || prevSigningRoot != signingRoot
+	if proposalAtSlotExists && signingRootIsDifferent {
 		if v.emitAccountMetrics {
 			ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc()
 		}
 		return errors.New(failedPreBlockSignLocalErr)
 	}
 
+	// Based on EIP3076, validator should refuse to sign any proposal with slot less
+	// than or equal to the minimum signed proposal present in the DB for that public key.
+	// In the case the slot of the incoming block is equal to the minimum signed proposal, we
+	// then also check the signing root is different.
+	if lowestProposalExists && signingRootIsDifferent && lowestSignedProposalSlot >= block.Slot {
+		return fmt.Errorf(
+			"could not sign block with slot <= lowest signed slot in db, lowest signed slot: %d >= block slot: %d",
+			lowestSignedProposalSlot,
+			block.Slot,
+		)
+	}
+
 	if featureconfig.Get().SlasherProtection && v.protector != nil {
 		blockHdr, err := blockutil.BeaconBlockHeaderFromBlock(block)
 		if err != nil {
@@ -66,7 +74,12 @@ func (v *validator) preBlockSignValidations(ctx context.Context, pubKey [48]byte
 	return nil
 }
 
-func (v *validator) postBlockSignUpdate(ctx context.Context, pubKey [48]byte, block *ethpb.SignedBeaconBlock, domain *ethpb.DomainResponse) error {
+func (v *validator) postBlockSignUpdate(
+	ctx context.Context,
+	pubKey [48]byte,
+	block *ethpb.SignedBeaconBlock,
+	signingRoot [32]byte,
+) error {
 	fmtKey := fmt.Sprintf("%#x", pubKey[:])
 	if featureconfig.Get().SlasherProtection && v.protector != nil {
 		sbh, err := blockutil.SignedBeaconBlockHeaderFromBlock(block)
@@ -84,13 +97,6 @@ func (v *validator) postBlockSignUpdate(ctx context.Context, pubKey [48]byte, bl
 			return fmt.Errorf(failedPostBlockSignErr)
 		}
 	}
-	signingRoot, err := helpers.ComputeSigningRoot(block.Block, domain.SignatureDomain)
-	if err != nil {
-		if v.emitAccountMetrics {
-			ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc()
-		}
-		return errors.Wrap(err, "failed to compute signing root for block")
-	}
 	if err := v.db.SaveProposalHistoryForSlot(ctx, pubKey, block.Block.Slot, signingRoot[:]); err != nil {
 		if v.emitAccountMetrics {
 			ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc()

validator/client/propose_protect_test.go

@@ -30,25 +30,34 @@ func TestPreBlockSignLocalValidation_PreventsLowerThanMinProposal(t *testing.T)
 		Slot:          lowestSignedSlot - 1,
 		ProposerIndex: 0,
 	}
-	err = validator.preBlockSignValidations(context.Background(), pubKeyBytes, block)
+	err = validator.preBlockSignValidations(context.Background(), pubKeyBytes, block, [32]byte{4})
 	require.ErrorContains(t, "could not sign block with slot <= lowest signed", err)
 
 	// We expect the same block with a slot equal to the lowest
-	// signed slot to fail validation.
+	// signed slot to pass validation if signing roots are equal.
 	block = &ethpb.BeaconBlock{
 		Slot:          lowestSignedSlot,
 		ProposerIndex: 0,
 	}
-	err = validator.preBlockSignValidations(context.Background(), pubKeyBytes, block)
-	require.ErrorContains(t, "could not sign block with slot <= lowest signed", err)
+	err = validator.preBlockSignValidations(context.Background(), pubKeyBytes, block, [32]byte{1})
+	require.NoError(t, err)
+
+	// We expect the same block with a slot equal to the lowest
+	// signed slot to fail validation if signing roots are different.
+	block = &ethpb.BeaconBlock{
+		Slot:          lowestSignedSlot,
+		ProposerIndex: 0,
+	}
+	err = validator.preBlockSignValidations(context.Background(), pubKeyBytes, block, [32]byte{4})
+	require.ErrorContains(t, failedPreBlockSignLocalErr, err)
 
 	// We expect the same block with a slot > than the lowest
 	// signed slot to pass validation.
 	block = &ethpb.BeaconBlock{
 		Slot:          lowestSignedSlot + 1,
 		ProposerIndex: 0,
 	}
-	err = validator.preBlockSignValidations(context.Background(), pubKeyBytes, block)
+	err = validator.preBlockSignValidations(context.Background(), pubKeyBytes, block, [32]byte{3})
 	require.NoError(t, err)
 }
 
@@ -74,13 +83,18 @@ func TestPreBlockSignLocalValidation(t *testing.T) {
 	require.NoError(t, err)
 
 	// We save a proposal at slot 10 with a dummy signing root.
-	err = validator.db.SaveProposalHistoryForSlot(ctx, pubKeyBytes, 10, []byte{1})
+	dummySigningRoot := [32]byte{1}
+	err = validator.db.SaveProposalHistoryForSlot(ctx, pubKeyBytes, 10, dummySigningRoot[:])
 	require.NoError(t, err)
 	pubKey := [48]byte{}
 	copy(pubKey[:], validatorKey.PublicKey().Marshal())
 
-	// We expect the same block sent out should return slashable error.
-	err = validator.preBlockSignValidations(context.Background(), pubKey, block)
+	// We expect the same block sent out with the same root should not be slasahble.
+	err = validator.preBlockSignValidations(context.Background(), pubKey, block, dummySigningRoot)
+	require.NoError(t, err)
+
+	// We expect the same block sent out with a different signing root should be slasahble.
+	err = validator.preBlockSignValidations(context.Background(), pubKey, block, [32]byte{2})
 	require.ErrorContains(t, failedPreBlockSignLocalErr, err)
 
 	// We save a proposal at slot 11 with a nil signing root.
@@ -90,13 +104,13 @@ func TestPreBlockSignLocalValidation(t *testing.T) {
 
 	// We expect the same block sent out should return slashable error even
 	// if we had a nil signing root stored in the database.
-	err = validator.preBlockSignValidations(context.Background(), pubKey, block)
+	err = validator.preBlockSignValidations(context.Background(), pubKey, block, [32]byte{2})
 	require.ErrorContains(t, failedPreBlockSignLocalErr, err)
 
 	// A block with a different slot for which we do not have a proposing history
 	// should not be failing validation.
 	block.Slot = 9
-	err = validator.preBlockSignValidations(context.Background(), pubKey, block)
+	err = validator.preBlockSignValidations(context.Background(), pubKey, block, [32]byte{3})
 	require.NoError(t, err, "Expected allowed block not to throw error")
 }
 
@@ -115,11 +129,11 @@ func TestPreBlockSignValidation(t *testing.T) {
 	block.Block.Slot = 10
 	mockProtector := &mockSlasher.MockProtector{AllowBlock: false}
 	validator.protector = mockProtector
-	err := validator.preBlockSignValidations(context.Background(), pubKey, block.Block)
+	err := validator.preBlockSignValidations(context.Background(), pubKey, block.Block, [32]byte{2})
 	require.ErrorContains(t, failedPreBlockSignExternalErr, err)
 	mockProtector.AllowBlock = true
-	err = validator.preBlockSignValidations(context.Background(), pubKey, block.Block)
-	require.NoError(t, err, "Expected allowed attestation not to throw error")
+	err = validator.preBlockSignValidations(context.Background(), pubKey, block.Block, [32]byte{2})
+	require.NoError(t, err, "Expected allowed block not to throw error")
 }
 
 func TestPostBlockSignUpdate(t *testing.T) {
@@ -137,9 +151,9 @@ func TestPostBlockSignUpdate(t *testing.T) {
 	emptyBlock.Block.ProposerIndex = 0
 	mockProtector := &mockSlasher.MockProtector{AllowBlock: false}
 	validator.protector = mockProtector
-	err := validator.postBlockSignUpdate(context.Background(), pubKey, emptyBlock, nil)
+	err := validator.postBlockSignUpdate(context.Background(), pubKey, emptyBlock, [32]byte{})
 	require.ErrorContains(t, failedPostBlockSignErr, err, "Expected error when post signature update is detected as slashable")
 	mockProtector.AllowBlock = true
-	err = validator.postBlockSignUpdate(context.Background(), pubKey, emptyBlock, &ethpb.DomainResponse{SignatureDomain: make([]byte, 32)})
-	require.NoError(t, err, "Expected allowed attestation not to throw error")
+	err = validator.postBlockSignUpdate(context.Background(), pubKey, emptyBlock, [32]byte{})
+	require.NoError(t, err, "Expected allowed block not to throw error")
 }

validator/client/propose_test.go

@@ -189,8 +189,9 @@ func TestProposeBlock_BlocksDoubleProposal(t *testing.T) {
 	pubKey := [48]byte{}
 	copy(pubKey[:], validatorKey.PublicKey().Marshal())
 
+	dummyRoot := [32]byte{}
 	// Save a dummy proposal history at slot 0.
-	err := validator.db.SaveProposalHistoryForSlot(context.Background(), pubKey, 0, []byte{})
+	err := validator.db.SaveProposalHistoryForSlot(context.Background(), pubKey, 0, dummyRoot[:])
 	require.NoError(t, err)
 
 	m.validatorClient.EXPECT().DomainData(
@@ -204,12 +205,22 @@ func TestProposeBlock_BlocksDoubleProposal(t *testing.T) {
 	m.validatorClient.EXPECT().GetBlock(
 		gomock.Any(), // ctx
 		gomock.Any(),
-	).Times(2).Return(testBlock.Block, nil /*err*/)
+	).Return(testBlock.Block, nil /*err*/)
+
+	secondTestBlock := testutil.NewBeaconBlock()
+	secondTestBlock.Block.Slot = slot
+	graffiti := [32]byte{}
+	copy(graffiti[:], "someothergraffiti")
+	secondTestBlock.Block.Body.Graffiti = graffiti[:]
+	m.validatorClient.EXPECT().GetBlock(
+		gomock.Any(), // ctx
+		gomock.Any(),
+	).Return(secondTestBlock.Block, nil /*err*/)
 
 	m.validatorClient.EXPECT().DomainData(
 		gomock.Any(), // ctx
 		gomock.Any(), //epoch
-	).Return(&ethpb.DomainResponse{SignatureDomain: make([]byte, 32)}, nil /*err*/)
+	).Times(2).Return(&ethpb.DomainResponse{SignatureDomain: make([]byte, 32)}, nil /*err*/)
 
 	m.validatorClient.EXPECT().ProposeBlock(
 		gomock.Any(), // ctx
@@ -230,8 +241,9 @@ func TestProposeBlock_BlocksDoubleProposal_After54KEpochs(t *testing.T) {
 	pubKey := [48]byte{}
 	copy(pubKey[:], validatorKey.PublicKey().Marshal())
 
+	dummyRoot := [32]byte{}
 	// Save a dummy proposal history at slot 0.
-	err := validator.db.SaveProposalHistoryForSlot(context.Background(), pubKey, 0, []byte{})
+	err := validator.db.SaveProposalHistoryForSlot(context.Background(), pubKey, 0, dummyRoot[:])
 	require.NoError(t, err)
 
 	m.validatorClient.EXPECT().DomainData(
@@ -245,12 +257,22 @@ func TestProposeBlock_BlocksDoubleProposal_After54KEpochs(t *testing.T) {
 	m.validatorClient.EXPECT().GetBlock(
 		gomock.Any(), // ctx
 		gomock.Any(),
-	).Times(2).Return(testBlock.Block, nil /*err*/)
+	).Return(testBlock.Block, nil /*err*/)
+
+	secondTestBlock := testutil.NewBeaconBlock()
+	secondTestBlock.Block.Slot = farFuture
+	graffiti := [32]byte{}
+	copy(graffiti[:], "someothergraffiti")
+	secondTestBlock.Block.Body.Graffiti = graffiti[:]
+	m.validatorClient.EXPECT().GetBlock(
+		gomock.Any(), // ctx
+		gomock.Any(),
+	).Return(secondTestBlock.Block, nil /*err*/)
 
 	m.validatorClient.EXPECT().DomainData(
 		gomock.Any(), // ctx
 		gomock.Any(), //epoch
-	).Return(&ethpb.DomainResponse{SignatureDomain: make([]byte, 32)}, nil /*err*/)
+	).Times(2).Return(&ethpb.DomainResponse{SignatureDomain: make([]byte, 32)}, nil /*err*/)
 
 	m.validatorClient.EXPECT().ProposeBlock(
 		gomock.Any(), // ctx

validator/client/validator.go

@@ -58,7 +58,6 @@ type validator struct {
 	attLogsLock                        sync.Mutex
 	aggregatedSlotCommitteeIDCacheLock sync.Mutex
 	prevBalanceLock                    sync.RWMutex
-	attesterHistoryByPubKeyLock        sync.RWMutex
 	slashableKeysLock                  sync.RWMutex
 	walletInitializedFeed              *event.Feed
 	genesisTime                        uint64