Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for Signing Root Mismatch When Submitting Proposals and Importing Proposals in Slashing Interchange #8085

Merged
merged 13 commits into from
Dec 11, 2020
60 changes: 0 additions & 60 deletions beacon-chain/state/stateutil/attestations.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,66 +84,6 @@ func marshalAttestationData(data *ethpb.AttestationData) []byte {
return enc
}

func attestationRoot(hasher htrutils.HashFn, att *ethpb.Attestation) ([32]byte, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep source was complaining of unused code

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)

Expand Down
25 changes: 17 additions & 8 deletions validator/client/propose.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Comment on lines +106 to +113
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something for another PR, but these should just be merged into 1 method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but out of scope. Will open an issue

log.WithFields(
blockLogFields(pubKey, b, sig),
).WithError(err).Error("Failed block slashing protection check")
Expand Down
64 changes: 35 additions & 29 deletions validator/client/propose_protect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, maybe for another PR. Would like some documentation on what this method does here.

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 {
Expand All @@ -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)
Expand All @@ -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()
Expand Down
46 changes: 30 additions & 16 deletions validator/client/propose_protect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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.
Expand All @@ -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")
}

Expand All @@ -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) {
Expand All @@ -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")
}
34 changes: 28 additions & 6 deletions validator/client/propose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion validator/client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading