Fix competing attestation check (#3305)

beacon-chain/blockchain/receive_attestation.go

@@ -1,6 +1,7 @@
 package blockchain
 
 import (
+	"bytes"
 	"context"
 	"encoding/hex"
 	"time"
@@ -82,9 +83,16 @@ func (c *ChainService) ReceiveAttestationNoPubsub(ctx context.Context, att *ethp
 	log.WithFields(logrus.Fields{
 		"headSlot": headBlk.Slot,
 		"headRoot": hex.EncodeToString(headRoot),
-	}).Debug("Finished applying fork choice")
+	}).Debug("Finished applying fork choice for attestation")
 
-	isCompetingAtts(att.Data.BeaconBlockRoot[:], headRoot)
+	// Skip checking for competing attestation's target roots at epoch boundary.
+	if helpers.IsEpochEnd(slot) {
+		targetRoot, err := helpers.BlockRoot(c.headState, att.Data.Target.Epoch)
+		if err != nil {
+			return errors.Wrapf(err, "could not get target root for epoch %d", att.Data.Target.Epoch)
+		}
+		isCompetingAtts(targetRoot, att.Data.Target.Root[:])
+	}
 
 	// Save head info after running fork choice.
 	if err := c.saveHead(ctx, headBlk, bytesutil.ToBytes32(headRoot)); err != nil {
@@ -118,3 +126,14 @@ func (c *ChainService) waitForAttInclDelay(ctx context.Context, a *ethpb.Attesta
 	time.Sleep(time.Until(timeToInclude))
 	return slot, nil
 }
+
+// This checks if the attestation is from a competing chain, emits warning and updates metrics.
+func isCompetingAtts(headTargetRoot []byte, attTargetRoot []byte) {
+	if !bytes.Equal(attTargetRoot, headTargetRoot) {
+		log.WithFields(logrus.Fields{
+			"attTargetRoot":  hex.EncodeToString(attTargetRoot),
+			"headTargetRoot": hex.EncodeToString(headTargetRoot),
+		}).Warn("target heads different from new attestation")
+		competingAtts.Inc()
+	}
+}

beacon-chain/blockchain/receive_block.go

@@ -1,6 +1,7 @@
 package blockchain
 
 import (
+	"bytes"
 	"context"
 	"encoding/hex"
 
@@ -81,7 +82,7 @@ func (c *ChainService) ReceiveBlockNoPubsub(ctx context.Context, block *ethpb.Be
 	log.WithFields(logrus.Fields{
 		"headSlot": headBlk.Slot,
 		"headRoot": hex.EncodeToString(headRoot),
-	}).Info("Finished fork choice")
+	}).Info("Finished applying fork choice for block")
 
 	isCompetingBlock(root[:], block.Slot, headRoot, headBlk.Slot)
 
@@ -147,3 +148,16 @@ func (c *ChainService) CleanupBlockOperations(ctx context.Context, block *ethpb.
 	}
 	return nil
 }
+
+// This checks if the block is from a competing chain, emits warning and updates metrics.
+func isCompetingBlock(root []byte, slot uint64, headRoot []byte, headSlot uint64) {
+	if !bytes.Equal(root[:], headRoot) {
+		log.WithFields(logrus.Fields{
+			"blkSlot":  slot,
+			"blkRoot":  hex.EncodeToString(root[:]),
+			"headSlot": headSlot,
+			"headRoot": hex.EncodeToString(headRoot),
+		}).Warn("Calculated head diffs from new block")
+		competingBlks.Inc()
+	}
+}

beacon-chain/blockchain/receive_block_test.go

@@ -112,7 +112,7 @@ func TestReceiveBlock_ProcessCorrectly(t *testing.T) {
 		t.Errorf("Block failed processing: %v", err)
 	}
 	testutil.AssertLogsContain(t, hook, "Finished state transition and updated fork choice store for block")
-	testutil.AssertLogsContain(t, hook, "Finished fork choice")
+	testutil.AssertLogsContain(t, hook, "Finished applying fork choice for block")
 }
 
 func TestReceiveBlockNoPubsubForkchoice_ProcessCorrectly(t *testing.T) {

beacon-chain/blockchain/service.go

@@ -4,7 +4,6 @@
 package blockchain
 
 import (
-	"bytes"
 	"context"
 	"encoding/hex"
 	"fmt"
@@ -269,27 +268,3 @@ func (c *ChainService) saveGenesisValidators(ctx context.Context, s *pb.BeaconSt
 	}
 	return nil
 }
-
-// This checks if the block is from a competing chain, emits warning and updates metrics.
-func isCompetingBlock(root []byte, slot uint64, headRoot []byte, headSlot uint64) {
-	if !bytes.Equal(root[:], headRoot) {
-		log.WithFields(logrus.Fields{
-			"blkSlot":  slot,
-			"blkRoot":  hex.EncodeToString(root[:]),
-			"headSlot": headSlot,
-			"headRoot": hex.EncodeToString(headRoot),
-		}).Warn("Calculated head diffs from new block")
-		competingBlks.Inc()
-	}
-}
-
-// This checks if the attestation is from a competing chain, emits warning and updates metrics.
-func isCompetingAtts(root []byte, headRoot []byte) {
-	if !bytes.Equal(root[:], headRoot) {
-		log.WithFields(logrus.Fields{
-			"attDataRoot": hex.EncodeToString(root[:]),
-			"headRoot":    hex.EncodeToString(headRoot),
-		}).Warn("Calculated head diffs from new attestation")
-		competingAtts.Inc()
-	}
-}

beacon-chain/p2p/discovery_test.go

@@ -64,7 +64,7 @@ func TestStartDiscV5_DiscoverAllPeers(t *testing.T) {
 
 	cfg := &Config{
 		BootstrapNodeAddr: bootNode.String(),
-		Encoding: "ssz",
+		Encoding:          "ssz",
 	}
 
 	var listeners []*discv5.Network

beacon-chain/p2p/monitoring.go

@@ -13,7 +13,7 @@ var (
 		Name: "p2p_topic_peer_count",
 		Help: "The number of peers subscribed to a topic",
 	},
-	[]string{"topic"})
+		[]string{"topic"})
 )
 
 func registerMetrics(s *Service) {
@@ -29,7 +29,6 @@ func registerMetrics(s *Service) {
 		log.WithError(err).Error("Failed to register metric")
 	}
 
-
 	// Metrics with labels, polled every 10s.
 	go func() {
 		for {
@@ -49,4 +48,4 @@ func updateP2PTopicPeerCount(s *Service) {
 		topic += s.Encoding().ProtocolSuffix()
 		p2pTopicPeerCount.WithLabelValues(topic).Set(float64(len(s.pubsub.ListPeers(topic))))
 	}
-}
+}

beacon-chain/p2p/options_test.go

@@ -30,7 +30,7 @@ func TestPrivateKeyLoading(t *testing.T) {
 	log.WithField("file", file.Name()).WithField("key", keyStr).Info("Wrote key to file")
 	cfg := &Config{
 		PrivateKey: file.Name(),
-		Encoding: "ssz",
+		Encoding:   "ssz",
 	}
 	pKey, err := privKey(cfg)
 	if err != nil {

beacon-chain/p2p/service_test.go

@@ -91,8 +91,8 @@ func TestService_Start_OnlyStartsOnce(t *testing.T) {
 	hook := logTest.NewGlobal()
 
 	cfg := &Config{
-		Port:    2000,
-		UDPPort: 2000,
+		Port:     2000,
+		UDPPort:  2000,
 		Encoding: "ssz",
 	}
 	s, _ := NewService(cfg)
@@ -126,7 +126,7 @@ func TestListenForNewNodes(t *testing.T) {
 
 	cfg := &Config{
 		BootstrapNodeAddr: bootNode.String(),
-		Encoding: "ssz",
+		Encoding:          "ssz",
 	}
 	var listeners []*discv5.Network
 	var hosts []host.Host

beacon-chain/rpc/proposer_server.go

@@ -138,11 +138,6 @@ func (ps *ProposerServer) ProposeBlock(ctx context.Context, blk *ethpb.BeaconBlo
 		}
 	}
 
-	log.WithFields(logrus.Fields{
-		"headRoot": fmt.Sprintf("%#x", bytesutil.Trunc(root[:])),
-		"headSlot": blk.Slot,
-	}).Info("Chain head block and state updated")
-
 	return &pb.ProposeResponse{BlockRoot: root[:]}, nil
 }