fix non monotonic views in sbft

sbft was not persisting view change messages which led to replicas
returning to previous views. Previously failing test included.

Change-Id: Ie9c9afd372c9a80f36da67fdbcb696336b9586c6
Signed-off-by: Marko Vukolic <mvu@zurich.ibm.com>

Author: Marko Vukolic

orderer/sbft/simplebft/simplebft.go

@@ -117,27 +117,37 @@ func New(id uint64, config *Config, sys System) (*SBFT, error) {
 	s.cur.executed = true
 	s.cur.checkpointDone = true
 	s.cur.timeout = dummyCanceller{}
+	s.activeView = true
+
+	svc := &Signed{}
+	if s.sys.Restore("viewchange", svc) {
+		vc := &ViewChange{}
+		err := proto.Unmarshal(svc.Data, vc)
+		if err != nil {
+			return nil, err
+		}
+		s.view = vc.View
+		s.replicaState[s.id].signedViewchange = svc
+		s.activeView = false
+	}
 
 	pp := &Preprepare{}
-	if s.sys.Restore("preprepare", pp) {
+	if s.sys.Restore("preprepare", pp) && pp.Seq.View >= s.view {
 		s.view = pp.Seq.View
+		s.activeView = true
 		if pp.Seq.Seq > s.seq() {
 			s.acceptPreprepare(pp)
 		}
 	}
 	c := &Subject{}
-	if s.sys.Restore("commit", c) && reflect.DeepEqual(c, &s.cur.subject) {
+	if s.sys.Restore("commit", c) && reflect.DeepEqual(c, &s.cur.subject) && c.Seq.View >= s.view {
 		s.cur.sentCommit = true
 	}
 	ex := &Subject{}
-	if s.sys.Restore("execute", ex) && reflect.DeepEqual(c, &s.cur.subject) {
+	if s.sys.Restore("execute", ex) && reflect.DeepEqual(c, &s.cur.subject) && ex.Seq.View >= s.view {
 		s.cur.executed = true
 	}
 
-	if s.seq() == 0 {
-		s.activeView = true
-	}
-
 	s.cancelViewChangeTimer()
 	return s, nil
 }

orderer/sbft/simplebft/simplebft_test.go

@@ -166,6 +166,41 @@ func TestN1(t *testing.T) {
 	}
 }
 
+func TestMonotonicViews(t *testing.T) {
+	N := uint64(4)
+	sys := newTestSystem(N)
+	var repls []*SBFT
+	var adapters []*testSystemAdapter
+	for i := uint64(0); i < N; i++ {
+		a := sys.NewAdapter(i)
+		s, err := New(i, &Config{N: N, F: 1, BatchDurationNsec: 2000000000, BatchSizeBytes: 10, RequestTimeoutNsec: 20000000000}, a)
+		if err != nil {
+			t.Fatal(err)
+		}
+		repls = append(repls, s)
+		adapters = append(adapters, a)
+	}
+
+	repls[0].sendViewChange()
+	sys.Run()
+
+	view := repls[0].view
+	testLog.Notice("TEST: Replica 0 is in view ", view)
+	testLog.Notice("TEST: restarting replica 0")
+	repls[0], _ = New(0, &Config{N: N, F: 1, BatchDurationNsec: 2000000000, BatchSizeBytes: 10, RequestTimeoutNsec: 20000000000}, adapters[0])
+	for _, a := range adapters {
+		if a.id != 0 {
+			a.receiver.Connection(0)
+			adapters[0].receiver.Connection(a.id)
+		}
+	}
+	sys.Run()
+
+	if repls[0].view != view {
+		t.Fatalf("Replica 0 must be in view %d, but is in view %d", view, repls[0].view)
+	}
+}
+
 func TestByzPrimary(t *testing.T) {
 	N := uint64(4)
 	sys := newTestSystem(N)

orderer/sbft/simplebft/viewchange.go

@@ -50,6 +50,8 @@ func (s *SBFT) sendViewChange() {
 	svc := s.sign(vc)
 	s.viewChangeTimer.Cancel()
 	s.cur.timeout.Cancel()
+
+	s.sys.Persist("viewchange", svc)
 	s.broadcast(&Msg{&Msg_ViewChange{svc}})
 
 	s.processNewView()