Skip to content

Commit 887f72c

Browse files
nootnoot
authored
fix(dot/network, lib/grandpa): fix handshake decoding and grandpa message handler sigabort (#1631)
1 parent ca99fbf commit 887f72c

File tree

9 files changed

+76
-74
lines changed

9 files changed

+76
-74
lines changed

‎Makefile

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ VERSION=latest
88
endif
99
FULLDOCKERNAME=$(COMPANY)/$(NAME):$(VERSION)
1010

11-
.PHONY: help lint test install build clean start docker gossamer
11+
.PHONY: help lint test install build clean start docker gossamer build-debug
1212
all: help
1313
help: Makefile
1414
@echo
@@ -83,9 +83,8 @@ build:
8383
GOBIN=$(PWD)/bin go run scripts/ci.go install
8484

8585
## debug: Builds application binary with debug flags and stores it in `./bin/gossamer`
86-
build-debug:
87-
@echo " > \033[32mBuilding binary...\033[0m "
88-
GOBIN=$(PWD)/bin go run scripts/ci.go install-debug
86+
build-debug: clean
87+
cd cmd/gossamer && go build -gcflags=all="-N -l" -o ../../bin/gossamer && cd ../..
8988

9089
## init: Initialise gossamer using the default genesis and toml configuration files
9190
init:

‎dot/network/notifications.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ type handshakeReader struct {
6363
type notificationsProtocol struct {
6464
protocolID protocol.ID
6565
getHandshake HandshakeGetter
66+
handshakeDecoder HandshakeDecoder
6667
handshakeValidator HandshakeValidator
6768

6869
inboundHandshakeData *sync.Map //map[peer.ID]*handshakeData
@@ -207,7 +208,6 @@ func (s *Service) createNotificationsMessageHandler(info *notificationsProtocol,
207208

208209
func (s *Service) sendData(peer peer.ID, hs Handshake, info *notificationsProtocol, msg NotificationsMessage) {
209210
if support, err := s.host.supportsProtocol(peer, info.protocolID); err != nil || !support {
210-
logger.Debug("the peer does not supports the protocol", "protocol", info.protocolID, "peer", peer, "err", err)
211211
return
212212
}
213213

@@ -248,12 +248,11 @@ func (s *Service) sendData(peer peer.ID, hs Handshake, info *notificationsProtoc
248248
_ = stream.Close()
249249
info.outboundHandshakeData.Delete(peer)
250250
return
251-
case hsResponse := <-s.readHandshake(stream, decodeBlockAnnounceHandshake):
251+
case hsResponse := <-s.readHandshake(stream, info.handshakeDecoder):
252252
hsTimer.Stop()
253253
if hsResponse.err != nil {
254254
logger.Trace("failed to read handshake", "protocol", info.protocolID, "peer", peer, "error", err)
255255
_ = stream.Close()
256-
257256
info.outboundHandshakeData.Delete(peer)
258257
return
259258
}

‎dot/network/service.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,7 @@ func (s *Service) RegisterNotificationsProtocol(sub protocol.ID,
424424
protocolID: protocolID,
425425
getHandshake: handshakeGetter,
426426
handshakeValidator: handshakeValidator,
427+
handshakeDecoder: handshakeDecoder,
427428
inboundHandshakeData: new(sync.Map),
428429
outboundHandshakeData: new(sync.Map),
429430
}

‎dot/state/block.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -389,17 +389,6 @@ func (bs *BlockState) GetFinalizedHeader(round, setID uint64) (*types.Header, er
389389

390390
// GetFinalizedHash gets the latest finalised block header
391391
func (bs *BlockState) GetFinalizedHash(round, setID uint64) (common.Hash, error) {
392-
// get current round
393-
r, err := bs.GetRound()
394-
if err != nil {
395-
return common.Hash{}, err
396-
}
397-
398-
// round that is being queried for has not yet finalised
399-
if round > r {
400-
return common.Hash{}, fmt.Errorf("round not yet finalised")
401-
}
402-
403392
h, err := bs.db.Get(finalizedHashKey(round, setID))
404393
if err != nil {
405394
return common.Hash{}, err

‎lib/babe/babe.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,8 +480,7 @@ func (b *Service) handleSlot(slotNum uint64) error {
480480

481481
block, err := b.buildBlock(parent, currentSlot)
482482
if err != nil {
483-
logger.Error("block authoring", "error", err)
484-
return nil
483+
return err
485484
}
486485

487486
err = b.storageState.StoreTrie(ts)

‎lib/grandpa/grandpa.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,10 @@ func (s *Service) attemptToFinalize() error {
563563
return ErrServicePaused
564564
}
565565

566+
if s.ctx.Err() != nil {
567+
return nil
568+
}
569+
566570
has, _ := s.blockState.HasFinalizedBlock(s.state.round, s.state.setID)
567571
if has {
568572
return nil // a block was finalised, seems like we missed some messages

‎lib/grandpa/message_handler.go

Lines changed: 5 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ func (h *MessageHandler) handleMessage(from peer.ID, m GrandpaMessage) (network.
5858
// send vote message to grandpa service
5959
h.grandpa.in <- vm
6060
}
61+
return nil, nil
6162
case commitType:
6263
if fm, ok := m.(*CommitMessage); ok {
6364
return h.handleCommitMessage(fm)
@@ -85,7 +86,7 @@ func (h *MessageHandler) handleMessage(from peer.ID, m GrandpaMessage) (network.
8586
}
8687

8788
func (h *MessageHandler) handleNeighbourMessage(from peer.ID, msg *NeighbourMessage) error {
88-
currFinalized, err := h.grandpa.blockState.GetFinalizedHeader(0, 0)
89+
currFinalized, err := h.blockState.GetFinalizedHeader(0, 0)
8990
if err != nil {
9091
return err
9192
}
@@ -97,7 +98,7 @@ func (h *MessageHandler) handleNeighbourMessage(from peer.ID, msg *NeighbourMess
9798

9899
// TODO; determine if there is some reason we don't receive justifications in responses near the head (usually),
99100
// and remove the following code if it's fixed.
100-
head, err := h.grandpa.blockState.BestBlockNumber()
101+
head, err := h.blockState.BestBlockNumber()
101102
if err != nil {
102103
return err
103104
}
@@ -136,7 +137,7 @@ func (h *MessageHandler) handleNeighbourMessage(from peer.ID, msg *NeighbourMess
136137
}
137138

138139
func (h *MessageHandler) handleCommitMessage(msg *CommitMessage) (*ConsensusMessage, error) {
139-
logger.Debug("received finalisation message", "round", msg.Round, "hash", msg.Vote.hash)
140+
logger.Debug("received finalisation message", "msg", msg)
140141

141142
if has, _ := h.blockState.HasFinalizedBlock(msg.Round, h.grandpa.state.setID); has {
142143
return nil, nil
@@ -259,49 +260,6 @@ func (h *MessageHandler) verifyCatchUpResponseCompletability(prevote, precommit
259260
return nil
260261
}
261262

262-
// decodeMessage decodes a network-level consensus message into a GRANDPA VoteMessage or CommitMessage
263-
func decodeMessage(msg *ConsensusMessage) (m GrandpaMessage, err error) {
264-
var (
265-
mi interface{}
266-
ok bool
267-
)
268-
269-
switch msg.Data[0] {
270-
case voteType:
271-
m = &VoteMessage{}
272-
_, err = scale.Decode(msg.Data[1:], m)
273-
case commitType:
274-
r := &bytes.Buffer{}
275-
_, _ = r.Write(msg.Data[1:])
276-
cm := &CommitMessage{}
277-
err = cm.Decode(r)
278-
m = cm
279-
case neighbourType:
280-
mi, err = scale.Decode(msg.Data[1:], &NeighbourMessage{})
281-
if m, ok = mi.(*NeighbourMessage); !ok {
282-
return nil, ErrInvalidMessageType
283-
}
284-
case catchUpRequestType:
285-
mi, err = scale.Decode(msg.Data[1:], &catchUpRequest{})
286-
if m, ok = mi.(*catchUpRequest); !ok {
287-
return nil, ErrInvalidMessageType
288-
}
289-
case catchUpResponseType:
290-
mi, err = scale.Decode(msg.Data[1:], &catchUpResponse{})
291-
if m, ok = mi.(*catchUpResponse); !ok {
292-
return nil, ErrInvalidMessageType
293-
}
294-
default:
295-
return nil, ErrInvalidMessageType
296-
}
297-
298-
if err != nil {
299-
return nil, err
300-
}
301-
302-
return m, nil
303-
}
304-
305263
func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) error {
306264
if len(fm.Precommits) != len(fm.AuthData) {
307265
return ErrPrecommitSignatureMismatch
@@ -327,7 +285,7 @@ func (h *MessageHandler) verifyCommitMessageJustification(fm *CommitMessage) err
327285

328286
// confirm total # signatures >= grandpa threshold
329287
if uint64(count) < h.grandpa.state.threshold() {
330-
logger.Error("minimum votes not met for finalisation message", "votes needed", h.grandpa.state.threshold(),
288+
logger.Debug("minimum votes not met for finalisation message", "votes needed", h.grandpa.state.threshold(),
331289
"votes received", len(fm.Precommits))
332290
return ErrMinVotesNotMet
333291
}

‎lib/grandpa/network.go

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package grandpa
1818

1919
import (
20+
"bytes"
2021
"fmt"
2122
"time"
2223

@@ -136,7 +137,6 @@ func (s *Service) decodeMessage(in []byte) (NotificationsMessage, error) {
136137

137138
func (s *Service) handleNetworkMessage(from peer.ID, msg NotificationsMessage) (bool, error) {
138139
if msg == nil {
139-
logger.Trace("received nil message, ignoring")
140140
return false, nil
141141
}
142142

@@ -145,8 +145,7 @@ func (s *Service) handleNetworkMessage(from peer.ID, msg NotificationsMessage) (
145145
return false, ErrInvalidMessageType
146146
}
147147

148-
if len(cm.Data) == 0 {
149-
logger.Trace("received message with nil data, ignoring")
148+
if len(cm.Data) < 2 {
150149
return false, nil
151150
}
152151

@@ -160,8 +159,14 @@ func (s *Service) handleNetworkMessage(from peer.ID, msg NotificationsMessage) (
160159
return false, err
161160
}
162161

163-
if resp != nil {
164-
s.network.SendMessage(resp)
162+
switch r := resp.(type) {
163+
case *ConsensusMessage:
164+
if r != nil {
165+
s.network.SendMessage(resp)
166+
}
167+
case nil:
168+
default:
169+
logger.Warn("unexpected type returned from message handler", "response", resp)
165170
}
166171

167172
if m.Type() == neighbourType || m.Type() == catchUpResponseType {
@@ -203,3 +208,46 @@ func (s *Service) sendNeighbourMessage() {
203208
s.network.SendMessage(cm)
204209
}
205210
}
211+
212+
// decodeMessage decodes a network-level consensus message into a GRANDPA VoteMessage or CommitMessage
213+
func decodeMessage(msg *ConsensusMessage) (m GrandpaMessage, err error) {
214+
var (
215+
mi interface{}
216+
ok bool
217+
)
218+
219+
switch msg.Data[0] {
220+
case voteType:
221+
m = &VoteMessage{}
222+
_, err = scale.Decode(msg.Data[1:], m)
223+
case commitType:
224+
r := &bytes.Buffer{}
225+
_, _ = r.Write(msg.Data[1:])
226+
cm := &CommitMessage{}
227+
err = cm.Decode(r)
228+
m = cm
229+
case neighbourType:
230+
mi, err = scale.Decode(msg.Data[1:], &NeighbourMessage{})
231+
if m, ok = mi.(*NeighbourMessage); !ok {
232+
return nil, ErrInvalidMessageType
233+
}
234+
case catchUpRequestType:
235+
mi, err = scale.Decode(msg.Data[1:], &catchUpRequest{})
236+
if m, ok = mi.(*catchUpRequest); !ok {
237+
return nil, ErrInvalidMessageType
238+
}
239+
case catchUpResponseType:
240+
mi, err = scale.Decode(msg.Data[1:], &catchUpResponse{})
241+
if m, ok = mi.(*catchUpResponse); !ok {
242+
return nil, ErrInvalidMessageType
243+
}
244+
default:
245+
return nil, ErrInvalidMessageType
246+
}
247+
248+
if err != nil {
249+
return nil, err
250+
}
251+
252+
return m, nil
253+
}

‎lib/grandpa/vote_message.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ func (s *Service) receiveMessages(cond func() bool) {
4242
logger.Trace("received vote message", "msg", msg)
4343
vm, ok := msg.(*VoteMessage)
4444
if !ok {
45-
logger.Trace("failed to cast message to VoteMessage")
4645
continue
4746
}
4847

@@ -52,7 +51,13 @@ func (s *Service) receiveMessages(cond func() bool) {
5251
continue
5352
}
5453

55-
logger.Debug("validated vote message", "vote", v, "round", vm.Round, "subround", vm.Message.Stage, "precommits", s.precommits)
54+
logger.Debug("validated vote message",
55+
"vote", v,
56+
"round", vm.Round,
57+
"subround", vm.Message.Stage,
58+
"prevotes", s.prevotes,
59+
"precommits", s.precommits,
60+
)
5661
case <-ctx.Done():
5762
logger.Trace("returning from receiveMessages")
5863
return

0 commit comments

Comments
 (0)