Skip to content

Commit

Permalink
fix non monotonic views in sbft
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Marko Vukolic committed Dec 13, 2016
1 parent fe16a6d commit 44d5564
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 7 deletions.
24 changes: 17 additions & 7 deletions orderer/sbft/simplebft/simplebft.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
35 changes: 35 additions & 0 deletions orderer/sbft/simplebft/simplebft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions orderer/sbft/simplebft/viewchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 44d5564

Please sign in to comment.