fix acceptance of sbft new-view messages

per pbft protocol, new-view messages for old views must not be processed.

It is not clear if there was a bug, but with this changeset the
implementation is more streamlined and slightly optimized.

Change-Id: Idbd9e9fdb883516dcf142ef7bdf56794313c8352
Signed-off-by: Marko Vukolic <[email protected]>

Author: Marko Vukolic

orderer/sbft/simplebft/connection.go

@@ -75,7 +75,7 @@ func (s *SBFT) handleHello(h *Hello, src uint64) {
 		s.deliverBatch(h.Batch)
 	}
 
-	if h.NewView != nil {
+	if h.NewView != nil && s.view <= h.NewView.View {
 		if s.primaryIDView(h.NewView.View) != src {
 			log.Warningf("replica %d: invalid hello with new view from non-primary %d", s.id, src)
 			return
@@ -93,10 +93,8 @@ func (s *SBFT) handleHello(h *Hello, src uint64) {
 			return
 		}
 
-		if s.view <= h.NewView.View {
-			s.view = h.NewView.View
-			s.activeView = true
-		}
+		s.view = h.NewView.View
+		s.activeView = true
 
 		s.maybeDeliverUsingXset(h.NewView)
 	}

orderer/sbft/simplebft/newview.go

@@ -86,13 +86,18 @@ func (s *SBFT) checkNewViewSignatures(nv *NewView) ([]*ViewChange, error) {
 }
 
 func (s *SBFT) handleNewView(nv *NewView, src uint64) {
-	if src != s.primaryIDView(nv.View) {
-		log.Warningf("replica %d: invalid new view from %d for %d", s.id, src, nv.View)
+	if nv.View < s.view {
+		log.Debugf("replica %d: discarding old new view from %d for %d, we are in %d", s.id, src, nv.View, s.view)
 		return
 	}
 
-	if onv := s.replicaState[s.primaryIDView(nv.View)].newview; onv != nil && onv.View >= nv.View {
-		log.Debugf("replica %d: discarding duplicate new view for %d", s.id, nv.View)
+	if nv.View == s.view && s.activeView {
+		log.Debugf("replica %d: discarding new view from %d for %d, we are already active in %d", s.id, src, nv.View, s.view)
+		return
+	}
+
+	if src != s.primaryIDView(nv.View) {
+		log.Warningf("replica %d: invalid new view from %d for %d", s.id, src, nv.View)
 		return
 	}
 
@@ -135,11 +140,8 @@ func (s *SBFT) handleNewView(nv *NewView, src uint64) {
 	}
 
 	s.replicaState[s.primaryIDView(nv.View)].newview = nv
-
-	if nv.View > s.view {
-		s.view = nv.View
-		s.activeView = false
-	}
+	s.view = nv.View
+	s.activeView = false
 
 	s.processNewView()
 }