fix(dot/network, lib/grandpa): fix handshake decoding and grandpa mes…

…sage handler sigabort (ChainSafe#1631)

Makefile

@@ -8,7 +8,7 @@ VERSION=latest
 endif
 FULLDOCKERNAME=$(COMPANY)/$(NAME):$(VERSION)
 
-.PHONY: help lint test install build clean start docker gossamer
+.PHONY: help lint test install build clean start docker gossamer build-debug
 all: help
 help: Makefile
 	@echo
@@ -83,9 +83,8 @@ build:
 	GOBIN=$(PWD)/bin go run scripts/ci.go install
 
 ## debug: Builds application binary with debug flags and stores it in `./bin/gossamer`
-build-debug:
-	@echo "  >  \033[32mBuilding binary...\033[0m "
-	GOBIN=$(PWD)/bin go run scripts/ci.go install-debug
+build-debug: clean
+	cd cmd/gossamer && go build -gcflags=all="-N -l" -o ../../bin/gossamer && cd ../..
 
 ## init: Initialise gossamer using the default genesis and toml configuration files
 init:

dot/network/notifications.go

@@ -63,6 +63,7 @@ type handshakeReader struct {
 type notificationsProtocol struct {
 	protocolID         protocol.ID
 	getHandshake       HandshakeGetter
+	handshakeDecoder   HandshakeDecoder
 	handshakeValidator HandshakeValidator
 
 	inboundHandshakeData  *sync.Map //map[peer.ID]*handshakeData
@@ -207,7 +208,6 @@ func (s *Service) createNotificationsMessageHandler(info *notificationsProtocol,
 
 func (s *Service) sendData(peer peer.ID, hs Handshake, info *notificationsProtocol, msg NotificationsMessage) {
 	if support, err := s.host.supportsProtocol(peer, info.protocolID); err != nil || !support {
-		logger.Debug("the peer does not supports the protocol", "protocol", info.protocolID, "peer", peer, "err", err)
 		return
 	}
 
@@ -248,12 +248,11 @@ func (s *Service) sendData(peer peer.ID, hs Handshake, info *notificationsProtoc
 			_ = stream.Close()
 			info.outboundHandshakeData.Delete(peer)
 			return
-		case hsResponse := <-s.readHandshake(stream, decodeBlockAnnounceHandshake):
+		case hsResponse := <-s.readHandshake(stream, info.handshakeDecoder):
 			hsTimer.Stop()
 			if hsResponse.err != nil {
 				logger.Trace("failed to read handshake", "protocol", info.protocolID, "peer", peer, "error", err)
 				_ = stream.Close()
-
 				info.outboundHandshakeData.Delete(peer)
 				return
 			}

dot/network/service.go

@@ -424,6 +424,7 @@ func (s *Service) RegisterNotificationsProtocol(sub protocol.ID,
 		protocolID:            protocolID,
 		getHandshake:          handshakeGetter,
 		handshakeValidator:    handshakeValidator,
+		handshakeDecoder:      handshakeDecoder,
 		inboundHandshakeData:  new(sync.Map),
 		outboundHandshakeData: new(sync.Map),
 	}

dot/state/block.go

@@ -389,17 +389,6 @@ func (bs *BlockState) GetFinalizedHeader(round, setID uint64) (*types.Header, er
 
 // GetFinalizedHash gets the latest finalised block header
 func (bs *BlockState) GetFinalizedHash(round, setID uint64) (common.Hash, error) {
-	// get current round
-	r, err := bs.GetRound()
-	if err != nil {
-		return common.Hash{}, err
-	}
-
-	// round that is being queried for has not yet finalised
-	if round > r {
-		return common.Hash{}, fmt.Errorf("round not yet finalised")
-	}
-
 	h, err := bs.db.Get(finalizedHashKey(round, setID))
 	if err != nil {
 		return common.Hash{}, err

lib/babe/babe.go

@@ -480,8 +480,7 @@ func (b *Service) handleSlot(slotNum uint64) error {
 
 	block, err := b.buildBlock(parent, currentSlot)
 	if err != nil {
-		logger.Error("block authoring", "error", err)
-		return nil
+		return err
 	}
 
 	err = b.storageState.StoreTrie(ts)

lib/grandpa/grandpa.go

@@ -563,6 +563,10 @@ func (s *Service) attemptToFinalize() error {
 		return ErrServicePaused
 	}
 
+	if s.ctx.Err() != nil {
+		return nil
+	}
+
 	has, _ := s.blockState.HasFinalizedBlock(s.state.round, s.state.setID)
 	if has {
 		return nil // a block was finalised, seems like we missed some messages

lib/grandpa/message_handler.go

@@ -58,6 +58,7 @@ func (h *MessageHandler) handleMessage(from peer.ID, m GrandpaMessage) (network.
 			// send vote message to grandpa service
 			h.grandpa.in <- vm
 		}
+		return nil, nil
 	case commitType:
 		if fm, ok := m.(*CommitMessage); ok {
 			return h.handleCommitMessage(fm)
@@ -85,7 +86,7 @@ func (h *MessageHandler) handleMessage(from peer.ID, m GrandpaMessage) (network.
 }
 
 func (h *MessageHandler) handleNeighbourMessage(from peer.ID, msg *NeighbourMessage) error {
-	currFinalized, err := h.grandpa.blockState.GetFinalizedHeader(0, 0)
+	currFinalized, err := h.blockState.GetFinalizedHeader(0, 0)
 	if err != nil {
 		return err
 	}
@@ -97,7 +98,7 @@ func (h *MessageHandler) handleNeighbourMessage(from peer.ID, msg *NeighbourMess
 
 	// TODO; determine if there is some reason we don't receive justifications in responses near the head (usually),
 	// and remove the following code if it's fixed.
-	head, err := h.grandpa.blockState.BestBlockNumber()
+	head, err := h.blockState.BestBlockNumber()
 	if err != nil {
 		return err
 	}
@@ -136,7 +137,7 @@ func (h *MessageHandler) handleNeighbourMessage(from peer.ID, msg *NeighbourMess
 }
 
 func (h *MessageHandler) handleCommitMessage(msg *CommitMessage) (*ConsensusMessage, error) {
-	logger.Debug("received finalisation message", "round", msg.Round, "hash", msg.Vote.hash)
+	logger.Debug("received finalisation message", "msg", msg)
 
 	if has, _ := h.blockState.HasFinalizedBlock(msg.Round, h.grandpa.state.setID); has {
 		return nil, nil
@@ -259,49 +260,6 @@ func (h *MessageHandler) verifyCatchUpResponseCompletability(prevote, precommit
 	return nil
 }
 
-// decodeMessage decodes a network-level consensus message into a GRANDPA VoteMessage or CommitMessage
-func decodeMessage(msg *ConsensusMessage) (m GrandpaMessage, err error) {
-	var (
-		mi interface{}
-		ok bool
-	)
-
-	switch msg.Data[0] {
-	case voteType:
-		m = &VoteMessage{}
-		_, err = scale.Decode(msg.Data[1:], m)
-	case commitType:
-		r := &bytes.Buffer{}
-		_, _ = r.Write(msg.Data[1:])
-		cm := &CommitMessage{}
-		err = cm.Decode(r)
-		m = cm
-	case neighbourType:
-		mi, err = scale.Decode(msg.Data[1:], &NeighbourMessage{})
-		if m, ok = mi.(*NeighbourMessage); !ok {
-			return nil, ErrInvalidMessageType
-		}
-	case catchUpRequestType:
-		mi, err = scale.Decode(msg.Data[1:], &catchUpRequest{})
-		if m, ok = mi.(*catchUpRequest); !ok {
-			return nil, ErrInvalidMessageType
-		}
-	case catchUpResponseType:
-		mi, err = scale.Decode(msg.Data[1:], &catchUpResponse{})
-		if m, ok = mi.(*catchUpResponse); !ok {
-			return nil, ErrInvalidMessageType
-		}
-	default:
-		return nil, ErrInvalidMessageType
-	}
-
-	if err != nil {
-		return nil, err
-	}
-
-	return m, nil
-}
-
 func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) error {
 	if len(fm.Precommits) != len(fm.AuthData) {
 		return ErrPrecommitSignatureMismatch
@@ -327,7 +285,7 @@ func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) err
 
 	// confirm total # signatures >= grandpa threshold
 	if uint64(count) < h.grandpa.state.threshold() {
-		logger.Error("minimum votes not met for finalisation message", "votes needed", h.grandpa.state.threshold(),
+		logger.Debug("minimum votes not met for finalisation message", "votes needed", h.grandpa.state.threshold(),
 			"votes received", len(fm.Precommits))
 		return ErrMinVotesNotMet
 	}

lib/grandpa/network.go

@@ -17,6 +17,7 @@
 package grandpa
 
 import (
+	"bytes"
 	"fmt"
 	"time"
 
@@ -136,7 +137,6 @@ func (s *Service) decodeMessage(in []byte) (NotificationsMessage, error) {
 
 func (s *Service) handleNetworkMessage(from peer.ID, msg NotificationsMessage) (bool, error) {
 	if msg == nil {
-		logger.Trace("received nil message, ignoring")
 		return false, nil
 	}
 
@@ -145,8 +145,7 @@ func (s *Service) handleNetworkMessage(from peer.ID, msg NotificationsMessage) (
 		return false, ErrInvalidMessageType
 	}
 
-	if len(cm.Data) == 0 {
-		logger.Trace("received message with nil data, ignoring")
+	if len(cm.Data) < 2 {
 		return false, nil
 	}
 
@@ -160,8 +159,14 @@ func (s *Service) handleNetworkMessage(from peer.ID, msg NotificationsMessage) (
 		return false, err
 	}
 
-	if resp != nil {
-		s.network.SendMessage(resp)
+	switch r := resp.(type) {
+	case *ConsensusMessage:
+		if r != nil {
+			s.network.SendMessage(resp)
+		}
+	case nil:
+	default:
+		logger.Warn("unexpected type returned from message handler", "response", resp)
 	}
 
 	if m.Type() == neighbourType || m.Type() == catchUpResponseType {
@@ -203,3 +208,46 @@ func (s *Service) sendNeighbourMessage() {
 		s.network.SendMessage(cm)
 	}
 }
+
+// decodeMessage decodes a network-level consensus message into a GRANDPA VoteMessage or CommitMessage
+func decodeMessage(msg *ConsensusMessage) (m GrandpaMessage, err error) {
+	var (
+		mi interface{}
+		ok bool
+	)
+
+	switch msg.Data[0] {
+	case voteType:
+		m = &VoteMessage{}
+		_, err = scale.Decode(msg.Data[1:], m)
+	case commitType:
+		r := &bytes.Buffer{}
+		_, _ = r.Write(msg.Data[1:])
+		cm := &CommitMessage{}
+		err = cm.Decode(r)
+		m = cm
+	case neighbourType:
+		mi, err = scale.Decode(msg.Data[1:], &NeighbourMessage{})
+		if m, ok = mi.(*NeighbourMessage); !ok {
+			return nil, ErrInvalidMessageType
+		}
+	case catchUpRequestType:
+		mi, err = scale.Decode(msg.Data[1:], &catchUpRequest{})
+		if m, ok = mi.(*catchUpRequest); !ok {
+			return nil, ErrInvalidMessageType
+		}
+	case catchUpResponseType:
+		mi, err = scale.Decode(msg.Data[1:], &catchUpResponse{})
+		if m, ok = mi.(*catchUpResponse); !ok {
+			return nil, ErrInvalidMessageType
+		}
+	default:
+		return nil, ErrInvalidMessageType
+	}
+
+	if err != nil {
+		return nil, err
+	}
+
+	return m, nil
+}

lib/grandpa/vote_message.go

@@ -42,7 +42,6 @@ func (s *Service) receiveMessages(cond func() bool) {
 				logger.Trace("received vote message", "msg", msg)
 				vm, ok := msg.(*VoteMessage)
 				if !ok {
-					logger.Trace("failed to cast message to VoteMessage")
 					continue
 				}
 
@@ -52,7 +51,13 @@ func (s *Service) receiveMessages(cond func() bool) {
 					continue
 				}
 
-				logger.Debug("validated vote message", "vote", v, "round", vm.Round, "subround", vm.Message.Stage, "precommits", s.precommits)
+				logger.Debug("validated vote message",
+					"vote", v,
+					"round", vm.Round,
+					"subround", vm.Message.Stage,
+					"prevotes", s.prevotes,
+					"precommits", s.precommits,
+				)
 			case <-ctx.Done():
 				logger.Trace("returning from receiveMessages")
 				return