Skip to content

Commit ab67f34

Browse files
corecodeSimon Schubert
corecode
authored and
Simon Schubert
committed
sbft: rework new view null requests
Previously we would use our own concept of LastBatch() for the null request batch prevHash. This was unsound and replicas that were in view change for one or more requests could not verify the correctness of this hash. Instead we now base our choice of prevHash on the now properly reported most recent checkpoint. Change-Id: Ia6794113672d64700b05ed10cac8e81539428f5b Signed-off-by: Simon Schubert <[email protected]>
1 parent 061020b commit ab67f34

8 files changed

+193
-141
lines changed

‎orderer/sbft/simplebft/connection.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (s *SBFT) handleHello(h *Hello, src uint64) {
8282
return
8383
}
8484

85-
_, ok := s.makeXset(vcs)
85+
_, _, ok := s.makeXset(vcs)
8686
if !ok {
8787
log.Warningf("invalid hello new view xset from %d", src)
8888
return

‎orderer/sbft/simplebft/newview.go

+30-29
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package simplebft
1818

1919
import (
20+
"bytes"
2021
"fmt"
2122
"reflect"
2223
)
@@ -36,23 +37,20 @@ func (s *SBFT) maybeSendNewView() {
3637
}
3738
}
3839

39-
xset, ok := s.makeXset(vcs)
40+
xset, newBatch, ok := s.makeXset(vcs)
4041
if !ok {
4142
log.Debug("xset not yet sufficient")
4243
return
4344
}
4445

4546
var batch *Batch
46-
if xset.Digest != nil {
47-
if reflect.DeepEqual(s.cur.subject.Digest, xset.Digest) {
48-
batch = s.cur.preprep.Batch
49-
} else {
50-
log.Warningf("forfeiting primary - do not have request in store for %d %x", xset.Seq.Seq, xset.Digest)
51-
xset = nil
52-
}
47+
if newBatch != nil {
48+
batch = newBatch
49+
} else if reflect.DeepEqual(s.cur.subject.Digest, xset.Digest) {
50+
batch = s.cur.preprep.Batch
5351
} else {
54-
batch = s.makeBatch(xset.Seq.Seq, s.sys.LastBatch().Hash(), nil)
55-
xset.Digest = batch.Hash()
52+
log.Warningf("forfeiting primary - do not have request in store for %d %x", xset.Seq.Seq, xset.Digest)
53+
xset = nil
5654
}
5755

5856
nv := &NewView{
@@ -73,12 +71,13 @@ func (s *SBFT) checkNewViewSignatures(nv *NewView) ([]*ViewChange, error) {
7371
vc := &ViewChange{}
7472
err := s.checkSig(svc, vcsrc, vc)
7573
if err == nil {
74+
_, err = s.checkBatch(vc.Checkpoint, false, true)
7675
if vc.View != nv.View {
7776
err = fmt.Errorf("view does not match")
7877
}
7978
}
8079
if err != nil {
81-
return nil, err
80+
return nil, fmt.Errorf("viewchange from %d: %s", vcsrc, err)
8281
}
8382
vcs = append(vcs, vc)
8483
}
@@ -101,27 +100,31 @@ func (s *SBFT) handleNewView(nv *NewView, src uint64) {
101100
if err != nil {
102101
log.Warningf("invalid new view from %d: %s", src, err)
103102
s.sendViewChange()
103+
return
104104
}
105105

106-
xset, ok := s.makeXset(vcs)
107-
if xset.Digest == nil {
108-
// null request special treatment
109-
xset.Digest = s.makeBatch(nv.Xset.Seq.Seq, s.sys.LastBatch().Hash(), nil).Hash()
110-
}
111-
112-
if !ok || !reflect.DeepEqual(nv.Xset, xset) {
113-
log.Warningf("invalid new view from %d: xset incorrect: %v, %v", src, nv.Xset, xset)
106+
if nv.Batch == nil {
107+
log.Warningf("invalid new view from %d: no batch attached", src)
114108
s.sendViewChange()
115109
return
116110
}
117111

118-
if nv.Batch == nil {
119-
log.Warningf("invalid new view from %d: batch empty", src)
112+
xset, newBatch, ok := s.makeXset(vcs)
113+
114+
if !ok || !reflect.DeepEqual(nv.Xset, xset) {
115+
log.Warningf("invalid new view from %d: xset incorrect: %v, %v", src, nv.Xset, xset)
120116
s.sendViewChange()
121117
return
122118
}
123119

124-
if !reflect.DeepEqual(hash(nv.Batch.Header), nv.Xset.Digest) {
120+
if nv.Xset == nil {
121+
if !bytes.Equal(nv.Batch.Hash(), newBatch.Hash()) {
122+
log.Warningf("invalid new view from %d: new batch for null request does not match: %x, %x, %v",
123+
src, nv.Batch.Hash(), newBatch.Hash(), nv)
124+
s.sendViewChange()
125+
return
126+
}
127+
} else if !bytes.Equal(nv.Batch.Hash(), nv.Xset.Digest) {
125128
log.Warningf("invalid new view from %d: batch head hash does not match xset: %x, %x, %v",
126129
src, hash(nv.Batch.Header), nv.Xset.Digest, nv)
127130
s.sendViewChange()
@@ -151,17 +154,15 @@ func (s *SBFT) processNewView() {
151154
return
152155
}
153156

154-
nextSeq := s.nextSeq()
155-
if *nv.Xset.Seq != nextSeq {
156-
log.Infof("we are outdated")
157-
return
158-
}
157+
s.activeView = true
158+
s.discardBacklog(s.primaryID())
159159

160160
pp := &Preprepare{
161-
Seq: nv.Xset.Seq,
161+
Seq: &SeqView{Seq: nv.Batch.DecodeHeader().Seq, View: s.view},
162162
Batch: nv.Batch,
163163
}
164164

165-
s.activeView = true
166165
s.handleCheckedPreprepare(pp)
166+
167+
s.processBacklog()
167168
}

‎orderer/sbft/simplebft/newview_test.go

+69-34
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,24 @@ func TestXsetNoByz(t *testing.T) {
2929
Pset: nil,
3030
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")},
3131
&Subject{&SeqView{2, 2}, []byte("val2")}},
32-
Executed: 1,
32+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
3333
},
3434
&ViewChange{
35-
View: 3,
36-
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
37-
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
38-
Executed: 1,
35+
View: 3,
36+
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
37+
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
38+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
3939
},
4040
&ViewChange{
4141
View: 3,
4242
Pset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
4343
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")},
4444
&Subject{&SeqView{2, 2}, []byte("val2")}},
45-
Executed: 1,
45+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
4646
},
4747
}
4848

49-
xset, ok := s.makeXset(vcs)
49+
xset, _, ok := s.makeXset(vcs)
5050
if !ok {
5151
t.Fatal("no xset")
5252
}
@@ -56,31 +56,66 @@ func TestXsetNoByz(t *testing.T) {
5656
}
5757
}
5858

59+
func TestXsetNoNew(t *testing.T) {
60+
s := &SBFT{config: Config{N: 4, F: 1}, view: 3}
61+
prev := s.makeBatch(2, []byte("prev"), nil)
62+
vcs := []*ViewChange{
63+
&ViewChange{
64+
View: 3,
65+
Pset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
66+
Qset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
67+
Checkpoint: prev,
68+
},
69+
&ViewChange{
70+
View: 3,
71+
Pset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
72+
Qset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
73+
Checkpoint: prev,
74+
},
75+
&ViewChange{
76+
View: 3,
77+
Pset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
78+
Qset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
79+
Checkpoint: prev,
80+
},
81+
}
82+
83+
_, newBatch, ok := s.makeXset(vcs)
84+
if !ok {
85+
t.Fatal("no xset")
86+
}
87+
88+
expect := s.makeBatch(3, prev.Hash(), nil)
89+
if !reflect.DeepEqual(newBatch, expect) {
90+
t.Errorf("batches don't match: %v, %v", newBatch.DecodeHeader(), expect.DecodeHeader())
91+
}
92+
}
93+
5994
func TestXsetByz0(t *testing.T) {
6095
s := &SBFT{config: Config{N: 4, F: 1}, view: 3}
6196
vcs := []*ViewChange{
6297
&ViewChange{
63-
View: 3,
64-
Pset: nil,
65-
Qset: nil,
66-
Executed: 1,
98+
View: 3,
99+
Pset: nil,
100+
Qset: nil,
101+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
67102
},
68103
&ViewChange{
69-
View: 3,
70-
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
71-
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
72-
Executed: 1,
104+
View: 3,
105+
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
106+
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
107+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
73108
},
74109
&ViewChange{
75110
View: 3,
76111
Pset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
77112
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")},
78113
&Subject{&SeqView{2, 2}, []byte("val2")}},
79-
Executed: 1,
114+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
80115
},
81116
}
82117

83-
xset, ok := s.makeXset(vcs)
118+
xset, _, ok := s.makeXset(vcs)
84119
if ok {
85120
t.Error("should not have received an xset")
86121
}
@@ -90,10 +125,10 @@ func TestXsetByz0(t *testing.T) {
90125
Pset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
91126
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")},
92127
&Subject{&SeqView{2, 2}, []byte("val2")}},
93-
Executed: 2,
128+
Checkpoint: s.makeBatch(2, []byte("prev"), nil),
94129
})
95130

96-
xset, ok = s.makeXset(vcs)
131+
xset, _, ok = s.makeXset(vcs)
97132
if !ok {
98133
t.Error("no xset")
99134
}
@@ -106,39 +141,39 @@ func TestXsetByz2(t *testing.T) {
106141
s := &SBFT{config: Config{N: 4, F: 1}, view: 3}
107142
vcs := []*ViewChange{
108143
&ViewChange{
109-
View: 3,
110-
Pset: nil,
111-
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
112-
Executed: 1,
144+
View: 3,
145+
Pset: nil,
146+
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
147+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
113148
},
114149
&ViewChange{
115-
View: 3,
116-
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
117-
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
118-
Executed: 1,
150+
View: 3,
151+
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
152+
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
153+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
119154
},
120155
&ViewChange{
121156
View: 3,
122157
Pset: []*Subject{&Subject{&SeqView{2, 2}, []byte("val2")}},
123158
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")},
124159
&Subject{&SeqView{2, 2}, []byte("val2")}},
125-
Executed: 1,
160+
Checkpoint: s.makeBatch(1, []byte("prev"), nil),
126161
},
127162
}
128163

129-
xset, ok := s.makeXset(vcs)
164+
xset, _, ok := s.makeXset(vcs)
130165
if ok {
131166
t.Error("should not have received an xset")
132167
}
133168

134169
vcs = append(vcs, &ViewChange{
135-
View: 3,
136-
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
137-
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
138-
Executed: 2,
170+
View: 3,
171+
Pset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
172+
Qset: []*Subject{&Subject{&SeqView{1, 2}, []byte("val1")}},
173+
Checkpoint: s.makeBatch(2, []byte("prev"), nil),
139174
})
140175

141-
xset, ok = s.makeXset(vcs)
176+
xset, _, ok = s.makeXset(vcs)
142177
if !ok {
143178
t.Error("no xset")
144179
}

0 commit comments

Comments
 (0)