Skip to content

Commit

Permalink
dbft: fix the deadlock reason in four-good-nodes setup
Browse files Browse the repository at this point in the history
Finally, the reason of the following real-life liveness lock in the four nodes
scenario is found:
```
rmState |-> ( 0 :> [type |-> "cv", view |-> 1] @@
  1 :> [type |-> "cv", view |-> 1] @@
  2 :> [type |-> "commitSent", view |-> 0] @@
  3 :> [type |-> "commitSent", view |-> 1] )
```
The issue was discovered and fixed in the TLA+ model initially, see the
roman-khimov#2.

The decision on preparation payloads rejection/acceptance should be based on
the number of Commits accepted durnig the whole set of rounds, not during
the current round only.

See also the C# source code, it doesn't have such problem:
https://github.com/neo-project/neo-modules/blob/d00d90b9c27b3d0c3c57e9ca1f560a09975df241/src/DBFTPlugin/Consensus/ConsensusContext.cs#L223
  • Loading branch information
AnnaShaleva committed Mar 6, 2023
1 parent 39e3e95 commit e90389d
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@ type Context struct {

// PreparationPayloads stores consensus Prepare* payloads for the current epoch.
PreparationPayloads []payload.ConsensusPayload
// CommitPayloads stores consensus Commit payloads for the current epoch.
// CommitPayloads stores consensus Commit payloads sent throughout all epochs. It
// is assumed that valid Commit payload can only be sent once by a single node per
// the whole set of consensus epochs for particular block. Invalid commit payloads
// are kicked off this list immediately (if PrepareRequest was received for the
// current round, so it's possible to verify Commit against it) or stored till
// the corresponding PrepareRequest receiving.
CommitPayloads []payload.ConsensusPayload
// ChangeViewPayloads stores consensus ChangeView payloads for the current epoch.
ChangeViewPayloads []payload.ConsensusPayload
Expand Down Expand Up @@ -101,7 +106,8 @@ func (c *Context) IsBackup() bool {
// WatchOnly returns true iff node takes no active part in consensus.
func (c *Context) WatchOnly() bool { return c.MyIndex < 0 || c.Config.WatchOnly() }

// CountCommitted returns number of received Commit messages for the current epoch.
// CountCommitted returns number of received Commit messages not only for the current
// epoch but also for any other epoch.
func (c *Context) CountCommitted() (count int) {
for i := range c.CommitPayloads {
if c.CommitPayloads[i] != nil {
Expand Down Expand Up @@ -135,7 +141,8 @@ func (c *Context) ResponseSent() bool {
return !c.WatchOnly() && c.PreparationPayloads[c.MyIndex] != nil
}

// CommitSent returns true iff Commit message was sent for the current epoch.
// CommitSent returns true iff Commit message was sent for the current epoch
// assuming that the node can't go further than current epoch after commit was sent.
func (c *Context) CommitSent() bool {
return !c.WatchOnly() && c.CommitPayloads[c.MyIndex] != nil
}
Expand Down Expand Up @@ -203,7 +210,9 @@ func (c *Context) reset(view byte, ts uint64) {

n := len(c.Validators)
c.ChangeViewPayloads = make([]payload.ConsensusPayload, n)
c.CommitPayloads = make([]payload.ConsensusPayload, n)
if view == 0 {
c.CommitPayloads = make([]payload.ConsensusPayload, n)
}
c.PreparationPayloads = make([]payload.ConsensusPayload, n)

c.Transactions = make(map[util.Uint256]block.Transaction)
Expand Down

0 comments on commit e90389d

Please sign in to comment.