fix(dot/netwok): check for duplicate message earlier

dot/network/block_announce.go

@@ -69,11 +69,14 @@ func (bm *BlockAnnounceMessage) Decode(in []byte) error {
 }
 
 // Hash returns the hash of the BlockAnnounceMessage
-func (bm *BlockAnnounceMessage) Hash() common.Hash {
+func (bm *BlockAnnounceMessage) Hash() (common.Hash, error) {
 	// scale encode each extrinsic
-	encMsg, _ := bm.Encode()
-	hash, _ := common.Blake2bHash(encMsg)
-	return hash
+	encMsg, err := bm.Encode()
+	if err != nil {
+		return common.Hash{}, err
+	}
+
+	return common.Blake2bHash(encMsg)
 }
 
 // IsHandshake returns false
@@ -145,8 +148,8 @@ func (*BlockAnnounceHandshake) Type() byte {
 }
 
 // Hash ...
-func (*BlockAnnounceHandshake) Hash() common.Hash {
-	return common.Hash{}
+func (*BlockAnnounceHandshake) Hash() (common.Hash, error) {
+	return common.Hash{}, nil
 }
 
 // IsHandshake returns true
@@ -174,7 +177,7 @@ func (s *Service) validateBlockAnnounceHandshake(from peer.ID, hs Handshake) err
 		return errors.New("invalid handshake type")
 	}
 
-	if bhs.GenesisHash != s.blockState.GenesisHash() {
+	if !bhs.GenesisHash.Equal(s.blockState.GenesisHash()) {
 		s.host.cm.peerSetHandler.ReportPeer(peerset.ReputationChange{
 			Value:  peerset.GenesisMismatch,
 			Reason: peerset.GenesisMismatchReason,

dot/network/gossip.go

@@ -4,6 +4,7 @@
 package network
 
 import (
+	"fmt"
 	"sync"
 
 	"github.com/ChainSafe/gossamer/internal/log"
@@ -24,13 +25,17 @@ func newGossip() *gossip {
 }
 
 // hasSeen broadcasts messages that have not been seen
-func (g *gossip) hasSeen(msg NotificationsMessage) bool {
+func (g *gossip) hasSeen(msg NotificationsMessage) (bool, error) {
 	// check if message has not been seen
-	if seen, ok := g.seen.Load(msg.Hash()); !ok || !seen.(bool) {
+	msgHash, err := msg.Hash()
+	if err != nil {
+		return false, fmt.Errorf("could not hash notification message: %w", err)
+	}
+	if seen, ok := g.seen.Load(msgHash); !ok || !seen.(bool) {
 		// set message to has been seen
-		g.seen.Store(msg.Hash(), true)
-		return false
+		g.seen.Store(msgHash, true)
+		return false, nil
 	}
 
-	return true
+	return true, nil
 }

dot/network/gossip_test.go

@@ -89,23 +89,26 @@ func TestGossip(t *testing.T) {
 
 	time.Sleep(TestMessageTimeout)
 
-	if hasSeenB, ok := nodeB.gossip.seen.Load(announceMessage.Hash()); !ok || hasSeenB.(bool) == false {
+	hash, err := announceMessage.Hash()
+	require.NoError(t, err)
+
+	if hasSeenB, ok := nodeB.gossip.seen.Load(hash); !ok || hasSeenB.(bool) == false {
 		t.Error(
 			"node B did not receive block request message from node A",
 			"\nreceived:", hasSeenB,
 			"\nexpected:", true,
 		)
 	}
 
-	if hasSeenC, ok := nodeC.gossip.seen.Load(announceMessage.Hash()); !ok || hasSeenC.(bool) == false {
+	if hasSeenC, ok := nodeC.gossip.seen.Load(hash); !ok || hasSeenC.(bool) == false {
 		t.Error(
 			"node C did not receive block request message from node B",
 			"\nreceived:", hasSeenC,
 			"\nexpected:", true,
 		)
 	}
 
-	if hasSeenA, ok := nodeA.gossip.seen.Load(announceMessage.Hash()); !ok || hasSeenA.(bool) == false {
+	if hasSeenA, ok := nodeA.gossip.seen.Load(hash); !ok || hasSeenA.(bool) == false {
 		t.Error(
 			"node A did not receive block request message from node C",
 			"\nreceived:", hasSeenA,

dot/network/message.go

@@ -36,7 +36,7 @@ type Message interface {
 type NotificationsMessage interface {
 	Message
 	Type() byte
-	Hash() common.Hash
+	Hash() (common.Hash, error)
 	IsHandshake() bool
 }
 
@@ -389,11 +389,13 @@ func (cm *ConsensusMessage) Decode(in []byte) error {
 }
 
 // Hash returns the Hash of ConsensusMessage
-func (cm *ConsensusMessage) Hash() common.Hash {
+func (cm *ConsensusMessage) Hash() (common.Hash, error) {
 	// scale encode each extrinsic
-	encMsg, _ := cm.Encode()
-	hash, _ := common.Blake2bHash(encMsg)
-	return hash
+	encMsg, err := cm.Encode()
+	if err != nil {
+		return common.Hash{}, err
+	}
+	return common.Blake2bHash(encMsg)
 }
 
 // IsHandshake returns false

dot/network/message_cache.go

@@ -55,6 +55,7 @@ func (m *messageCache) put(peer peer.ID, msg NotificationsMessage) (bool, error)
 func (m *messageCache) exists(peer peer.ID, msg NotificationsMessage) bool {
 	key, err := generateCacheKey(peer, msg)
 	if err != nil {
+		logger.Errorf("could not generate cache key: %w", err)
 		return false
 	}
 
@@ -67,7 +68,12 @@ func generateCacheKey(peer peer.ID, msg NotificationsMessage) ([]byte, error) {
 		return nil, errors.New("cache does not support handshake messages")
 	}
 
-	peerMsgHash, err := common.Blake2bHash(append([]byte(peer), msg.Hash().ToBytes()...))
+	msgHash, err := msg.Hash()
+	if err != nil {
+		return nil, err
+	}
+
+	peerMsgHash, err := common.Blake2bHash(append([]byte(peer), msgHash.ToBytes()...))
 	if err != nil {
 		return nil, err
 	}

dot/network/notifications.go

@@ -136,6 +136,20 @@ func (s *Service) createNotificationsMessageHandler(
 			return fmt.Errorf("%w: expected %T but got %T", errMessageTypeNotValid, (NotificationsMessage)(nil), msg)
 		}
 
+		hasSeen, err := s.gossip.hasSeen(msg)
+		if err != nil {
+			return fmt.Errorf("could not check if message was seen before: %w", err)
+		}
+
+		if hasSeen {
+			// report peer if we get duplicate gossip message.
+			s.host.cm.peerSetHandler.ReportPeer(peerset.ReputationChange{
+				Value:  peerset.DuplicateGossipValue,
+				Reason: peerset.DuplicateGossipReason,
+			}, peer)
+			return nil
+		}
+
 		if msg.IsHandshake() {
 			logger.Tracef("received handshake on notifications sub-protocol %s from peer %s, message is: %s",
 				info.protocolID, stream.Conn().RemotePeer(), msg)
@@ -207,16 +221,7 @@ func (s *Service) createNotificationsMessageHandler(
 			return nil
 		}
 
-		if !s.gossip.hasSeen(msg) {
-			s.broadcastExcluding(info, peer, msg)
-			return nil
-		}
-
-		// report peer if we get duplicate gossip message.
-		s.host.cm.peerSetHandler.ReportPeer(peerset.ReputationChange{
-			Value:  peerset.DuplicateGossipValue,
-			Reason: peerset.DuplicateGossipReason,
-		}, peer)
+		s.broadcastExcluding(info, peer, msg)
 		return nil
 	}
 }
@@ -238,7 +243,14 @@ func (s *Service) sendData(peer peer.ID, hs Handshake, info *notificationsProtoc
 		return
 	}
 
-	if support, err := s.host.supportsProtocol(peer, info.protocolID); err != nil || !support {
+	support, err := s.host.supportsProtocol(peer, info.protocolID)
+	if err != nil {
+		logger.Criticalf("could not check if protocol %s is supported by peer %s: %s", info.protocolID, peer, err)
+	} else if !support {
+		logger.Criticalf("protocol %s is not supported by peer %s", info.protocolID, peer)
+	}
+
+	if err != nil || !support {
 		s.host.cm.peerSetHandler.ReportPeer(peerset.ReputationChange{
 			Value:  peerset.BadProtocolValue,
 			Reason: peerset.BadProtocolReason,
@@ -319,7 +331,7 @@ func (s *Service) sendHandshake(peer peer.ID, hs Handshake, info *notificationsP
 		peer, info.protocolID, hs)
 	stream, err := s.host.send(peer, info.protocolID, hs)
 	if err != nil {
-		logger.Tracef("failed to send message to peer %s: %s", peer, err)
+		logger.Tracef("failed to send handshake to peer %s: %s", peer, err)
 		// don't need to close the stream here, as it's nil!
 		return nil, err
 	}
@@ -345,7 +357,7 @@ func (s *Service) sendHandshake(peer peer.ID, hs Handshake, info *notificationsP
 		}
 
 		if hsResponse.err != nil {
-			logger.Tracef("failed to read handshake from peer %s using protocol %s: %s", peer, info.protocolID, err)
+			logger.Tracef("failed to read handshake from peer %s using protocol %s: %s", peer, info.protocolID, hsResponse.err)
 			closeOutboundStream(info, peer, stream)
 			return nil, hsResponse.err
 		}

dot/network/transaction.go

@@ -54,10 +54,12 @@ func (tm *TransactionMessage) Decode(in []byte) error {
 }
 
 // Hash returns the hash of the TransactionMessage
-func (tm *TransactionMessage) Hash() common.Hash {
-	encMsg, _ := tm.Encode()
-	hash, _ := common.Blake2bHash(encMsg)
-	return hash
+func (tm *TransactionMessage) Hash() (common.Hash, error) {
+	encMsg, err := tm.Encode()
+	if err != nil {
+		return common.Hash{}, nil
+	}
+	return common.Blake2bHash(encMsg)
 }
 
 // IsHandshake returns false
@@ -93,8 +95,8 @@ func (*transactionHandshake) Type() byte {
 }
 
 // Hash ...
-func (*transactionHandshake) Hash() common.Hash {
-	return common.Hash{}
+func (*transactionHandshake) Hash() (common.Hash, error) {
+	return common.Hash{}, nil
 }
 
 // IsHandshake returns true
@@ -129,6 +131,7 @@ func (s *Service) startTxnBatchProcessing(txnBatchCh chan *BatchMessage, slotDur
 				case txnMsg := <-txnBatchCh:
 					propagate, err := s.handleTransactionMessage(txnMsg.peer, txnMsg.msg)
 					if err != nil {
+						logger.Warnf("could not handle transaction message: %s ", err)
 						s.host.closeProtocolStream(protocolID, txnMsg.peer)
 						continue
 					}
@@ -137,7 +140,16 @@ func (s *Service) startTxnBatchProcessing(txnBatchCh chan *BatchMessage, slotDur
 						continue
 					}
 
-					if !s.gossip.hasSeen(txnMsg.msg) {
+					// TODO: Check if s.gossip.hasSeen should be moved before handleTransactionMessage.
+					// That we could avoid handling the transactions again, which we would have already seen.
+
+					hasSeen, err := s.gossip.hasSeen(txnMsg.msg)
+					if err != nil {
+						s.host.closeProtocolStream(protocolID, txnMsg.peer)
+						logger.Debugf("could not check if message was seen before: %s", err)
+						continue
+					}
+					if !hasSeen {
 						s.broadcastExcluding(s.notificationsProtocols[TransactionMsgType], txnMsg.peer, txnMsg.msg)
 					}
 				}

dot/peerset/handler.go

@@ -77,6 +77,10 @@ func (h *Handler) RemovePeer(setID int, peers ...peer.ID) {
 
 // ReportPeer reports ReputationChange according to the peer behaviour.
 func (h *Handler) ReportPeer(rep ReputationChange, peers ...peer.ID) {
+	for _, pid := range peers {
+		logger.Debugf("reporting reputation change of %d to peer %s, reason: %s", rep.Value, pid, rep.Reason)
+	}
+
 	h.actionQueue <- action{
 		actionCall: reportPeer,
 		reputation: rep,

dot/state/block.go

@@ -164,6 +164,7 @@ func headerKey(hash common.Hash) []byte {
 }
 
 // headerHashKey = headerHashPrefix + num (uint64 big endian)
+// TODO: Shouldn't we change this block number to uint32?
 func headerHashKey(number uint64) []byte {
 	return append(headerHashPrefix, encodeBlockNumber(number)...)
 }

dot/types/block_data.go

@@ -28,7 +28,7 @@ func NewEmptyBlockData() *BlockData {
 
 // Number returns the block header number.
 func (bd *BlockData) Number() uint {
-	return bd.Header.Number
+	return uint(bd.Header.Number)
 }
 
 func (bd *BlockData) String() string {

go.mod

@@ -58,7 +58,7 @@ require (
 	github.com/btcsuite/btcd v0.22.0-beta // indirect
 	github.com/cespare/xxhash v1.1.0 // indirect
 	github.com/cespare/xxhash/v2 v2.1.2 // indirect
-	github.com/cpuguy83/go-md2man/v2 v2.0.0 // indirect
+	github.com/cpuguy83/go-md2man/v2 v2.0.1 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/davidlazar/go-crypto v0.0.0-20200604182044-b73af7476f6c // indirect
 	github.com/deckarep/golang-set v1.8.0 // indirect
@@ -130,7 +130,7 @@ require (
 	github.com/libp2p/go-yamux/v2 v2.2.0 // indirect
 	github.com/libp2p/zeroconf/v2 v2.1.0 // indirect
 	github.com/marten-seemann/tcp v0.0.0-20210406111302-dfbc87cc63fd // indirect
-	github.com/mattn/go-colorable v0.1.9 // indirect
+	github.com/mattn/go-colorable v0.1.12 // indirect
 	github.com/mattn/go-isatty v0.0.14 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
 	github.com/miekg/dns v1.1.43 // indirect
@@ -159,9 +159,8 @@ require (
 	github.com/prometheus/common v0.32.1 // indirect
 	github.com/prometheus/procfs v0.7.3 // indirect
 	github.com/rs/cors v1.7.0 // indirect
-	github.com/russross/blackfriday/v2 v2.0.1 // indirect
+	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/shirou/gopsutil v3.21.4-0.20210419000835-c7a38de76ee5+incompatible // indirect
-	github.com/shurcooL/sanitized_anchor_name v1.0.0 // indirect
 	github.com/spacemonkeygo/spacelog v0.0.0-20180420211403-2296661a0572 // indirect
 	github.com/stretchr/objx v0.1.1 // indirect
 	github.com/tklauser/go-sysconf v0.3.5 // indirect
@@ -178,8 +177,9 @@ require (
 	go.uber.org/zap v1.19.0 // indirect
 	golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2 // indirect
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
-	golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 // indirect
+	golang.org/x/sys v0.0.0-20220319134239-a9b59b0215f8 // indirect
 	golang.org/x/text v0.3.7 // indirect
+	golang.org/x/tools v0.1.9 // indirect
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
 	google.golang.org/appengine v1.6.6 // indirect
 	gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce // indirect