Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions internal/bft/viewchanger.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ type possibleProposal struct {
type proposalAndMetadata struct {
proposal *protos.Proposal
metadata *protos.ViewMetadata
prepared bool
}

// CheckInFlight checks if there is an in-flight proposal that needs to be decided on (because a node might decided on it already)
Expand All @@ -856,7 +857,7 @@ func CheckInFlight(messages map[uint64]*protos.ViewData, f int, quorum int, n ui
for _, vd := range messages {
if vd.GetInFlightProposal() == nil { // there is no in flight proposal here
noInFlightCount++
proposalsAndMetadata = append(proposalsAndMetadata, &proposalAndMetadata{nil, nil})
proposalsAndMetadata = append(proposalsAndMetadata, &proposalAndMetadata{nil, nil, false})
continue
}

Expand All @@ -869,7 +870,7 @@ func CheckInFlight(messages map[uint64]*protos.ViewData, f int, quorum int, n ui
return false, false, nil, fmt.Errorf("node was unable to unmarshal the in flight proposal metadata, error: %w", err)
}

proposalsAndMetadata = append(proposalsAndMetadata, &proposalAndMetadata{vd.GetInFlightProposal(), inFlightMetadata})
proposalsAndMetadata = append(proposalsAndMetadata, &proposalAndMetadata{vd.GetInFlightProposal(), inFlightMetadata, vd.GetInFlightPrepared()})

if inFlightMetadata.GetLatestSequence() != expectedSequence { // the in flight proposal sequence is not as expected
noInFlightCount++
Expand Down Expand Up @@ -913,7 +914,14 @@ func CheckInFlight(messages map[uint64]*protos.ViewData, f int, quorum int, n ui

if proto.Equal(prop.proposal, possible.proposal) {
possible.noArgument++
possible.preprepared++
// Only a node that actually prepared this proposal may back
// condition A2. A node that merely holds it in-flight has not
// committed to it, and counting it as prepared would let a
// view change decide an in-flight proposal that no quorum ever
// prepared.
if prop.prepared {
possible.preprepared++
}
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions internal/bft/viewchanger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1890,6 +1890,11 @@ func TestCheckInFlightWithProposal(t *testing.T) {
proposal: expectedProposal,
},
{
// Only messages[3] declares InFlightPrepared. Nodes that merely hold the
// proposal in-flight have not committed to it, so they must not back
// condition A2: 1 prepared is below f+1, and the view change falls back
// to condition B instead of deciding an in-flight proposal that no quorum
// ever prepared.
description: "all with expected in flight proposal, only one is prepared, other with different in flight",
mutateMessages: func(messages []*protos.ViewData) {
for _, msg := range messages {
Expand All @@ -1901,6 +1906,19 @@ func TestCheckInFlightWithProposal(t *testing.T) {
messages[3].InFlightPrepared = true
},
ok: true,
no: true,
proposal: nil,
},
{
description: "f+1 nodes prepared the expected in flight, the rest only hold it",
mutateMessages: func(messages []*protos.ViewData) {
for _, msg := range messages {
msg.InFlightProposal = expectedProposal
}
messages[1].InFlightPrepared = true
messages[2].InFlightPrepared = true
},
ok: true,
no: false,
proposal: expectedProposal,
},
Expand Down
Loading