diff --git a/protocol/synchronizer/synchronizer_test.go b/protocol/synchronizer/synchronizer_test.go index a8b6516d..750d9e28 100644 --- a/protocol/synchronizer/synchronizer_test.go +++ b/protocol/synchronizer/synchronizer_test.go @@ -180,6 +180,7 @@ func TestAdvanceView(t *testing.T) { {name: "signers=4/Simple___/__/TC/AC", tr: S, qc: F, tc: T, ac: T, firstSignerIdx: 0, wantView: 2}, {name: "signers=4/Simple___/QC/__/__", tr: S, qc: T, tc: F, ac: F, firstSignerIdx: 0, wantView: 2}, {name: "signers=4/Simple___/QC/__/AC", tr: S, qc: T, tc: F, ac: T, firstSignerIdx: 0, wantView: 2}, + {name: "signers=4/Simple___/QC/TC/__", tr: S, qc: T, tc: T, ac: F, firstSignerIdx: 0, wantView: 2}, {name: "signers=4/Simple___/QC/TC/AC", tr: S, qc: T, tc: T, ac: T, firstSignerIdx: 0, wantView: 2}, {name: "signers=4/Aggregate/__/__/__", tr: A, qc: F, tc: F, ac: F, firstSignerIdx: 0, wantView: 1}, // empty syncInfo, should not advance view {name: "signers=4/Aggregate/__/__/AC", tr: A, qc: F, tc: F, ac: T, firstSignerIdx: 0, wantView: 2}, @@ -187,14 +188,16 @@ func TestAdvanceView(t *testing.T) { {name: "signers=4/Aggregate/__/TC/AC", tr: A, qc: F, tc: T, ac: T, firstSignerIdx: 0, wantView: 2}, {name: "signers=4/Aggregate/QC/__/__", tr: A, qc: T, tc: F, ac: F, firstSignerIdx: 0, wantView: 1}, // aggregate timeout rule ignores quorum cert, will not advance view {name: "signers=4/Aggregate/QC/__/AC", tr: A, qc: T, tc: F, ac: T, firstSignerIdx: 0, wantView: 2}, + {name: "signers=4/Aggregate/QC/TC/__", tr: A, qc: T, tc: T, ac: F, firstSignerIdx: 0, wantView: 2}, {name: "signers=4/Aggregate/QC/TC/AC", tr: A, qc: T, tc: T, ac: T, firstSignerIdx: 0, wantView: 2}, - // three signers; quorum reacted, advance view + // three signers; quorum reached, advance view {name: "signers=3/Simple___/__/__/__", tr: S, qc: F, tc: F, ac: F, firstSignerIdx: 1, wantView: 1}, // empty syncInfo, should not advance view {name: "signers=3/Simple___/__/__/AC", tr: S, qc: F, tc: F, ac: T, firstSignerIdx: 1, wantView: 1}, // simple timeout rule ignores aggregate timeout cert, will not advance view {name: "signers=3/Simple___/__/TC/__", tr: S, qc: F, tc: T, ac: F, firstSignerIdx: 1, wantView: 2}, {name: "signers=3/Simple___/__/TC/AC", tr: S, qc: F, tc: T, ac: T, firstSignerIdx: 1, wantView: 2}, {name: "signers=3/Simple___/QC/__/__", tr: S, qc: T, tc: F, ac: F, firstSignerIdx: 1, wantView: 2}, {name: "signers=3/Simple___/QC/__/AC", tr: S, qc: T, tc: F, ac: T, firstSignerIdx: 1, wantView: 2}, + {name: "signers=3/Simple___/QC/TC/__", tr: S, qc: T, tc: T, ac: F, firstSignerIdx: 1, wantView: 2}, {name: "signers=3/Simple___/QC/TC/AC", tr: S, qc: T, tc: T, ac: T, firstSignerIdx: 1, wantView: 2}, {name: "signers=3/Aggregate/__/__/__", tr: A, qc: F, tc: F, ac: F, firstSignerIdx: 1, wantView: 1}, // empty syncInfo, should not advance view {name: "signers=3/Aggregate/__/__/AC", tr: A, qc: F, tc: F, ac: T, firstSignerIdx: 1, wantView: 2}, @@ -202,6 +205,7 @@ func TestAdvanceView(t *testing.T) { {name: "signers=3/Aggregate/__/TC/AC", tr: A, qc: F, tc: T, ac: T, firstSignerIdx: 1, wantView: 2}, {name: "signers=3/Aggregate/QC/__/__", tr: A, qc: T, tc: F, ac: F, firstSignerIdx: 1, wantView: 1}, // aggregate timeout rule ignores quorum cert, will not advance view {name: "signers=3/Aggregate/QC/__/AC", tr: A, qc: T, tc: F, ac: T, firstSignerIdx: 1, wantView: 2}, + {name: "signers=3/Aggregate/QC/TC/__", tr: A, qc: T, tc: T, ac: F, firstSignerIdx: 1, wantView: 2}, {name: "signers=3/Aggregate/QC/TC/AC", tr: A, qc: T, tc: T, ac: T, firstSignerIdx: 1, wantView: 2}, // only two signers; no quorum reached, should not advance view {name: "signers=2/Simple___/__/__/__", tr: S, qc: F, tc: F, ac: F, firstSignerIdx: 2, wantView: 1}, // empty syncInfo, should not advance view @@ -210,6 +214,7 @@ func TestAdvanceView(t *testing.T) { {name: "signers=2/Simple___/__/TC/AC", tr: S, qc: F, tc: T, ac: T, firstSignerIdx: 2, wantView: 1}, {name: "signers=2/Simple___/QC/__/__", tr: S, qc: T, tc: F, ac: F, firstSignerIdx: 2, wantView: 1}, {name: "signers=2/Simple___/QC/__/AC", tr: S, qc: T, tc: F, ac: T, firstSignerIdx: 2, wantView: 1}, + {name: "signers=2/Simple___/QC/TC/__", tr: S, qc: T, tc: T, ac: F, firstSignerIdx: 2, wantView: 1}, {name: "signers=2/Simple___/QC/TC/AC", tr: S, qc: T, tc: T, ac: T, firstSignerIdx: 2, wantView: 1}, {name: "signers=2/Aggregate/__/__/__", tr: A, qc: F, tc: F, ac: F, firstSignerIdx: 2, wantView: 1}, // empty syncInfo, should not advance view {name: "signers=2/Aggregate/__/__/AC", tr: A, qc: F, tc: F, ac: T, firstSignerIdx: 2, wantView: 1}, @@ -217,6 +222,7 @@ func TestAdvanceView(t *testing.T) { {name: "signers=2/Aggregate/__/TC/AC", tr: A, qc: F, tc: T, ac: T, firstSignerIdx: 2, wantView: 1}, {name: "signers=2/Aggregate/QC/__/__", tr: A, qc: T, tc: F, ac: F, firstSignerIdx: 2, wantView: 1}, {name: "signers=2/Aggregate/QC/__/AC", tr: A, qc: T, tc: F, ac: T, firstSignerIdx: 2, wantView: 1}, + {name: "signers=2/Aggregate/QC/TC/__", tr: A, qc: T, tc: T, ac: F, firstSignerIdx: 2, wantView: 1}, {name: "signers=2/Aggregate/QC/TC/AC", tr: A, qc: T, tc: T, ac: T, firstSignerIdx: 2, wantView: 1}, } for _, tt := range tests { diff --git a/protocol/synchronizer/timeoutrule_aggregate.go b/protocol/synchronizer/timeoutrule_aggregate.go index 3d333da1..883f38f3 100644 --- a/protocol/synchronizer/timeoutrule_aggregate.go +++ b/protocol/synchronizer/timeoutrule_aggregate.go @@ -62,24 +62,25 @@ func (s *Aggregate) RemoteTimeoutRule(currentView, timeoutView hotstuff.View, ti } func (s *Aggregate) VerifySyncInfo(syncInfo hotstuff.SyncInfo) (qc *hotstuff.QuorumCert, view hotstuff.View, timeout bool, err error) { + // get the highest view of the sync info's AggQC and TC + view, timeout = syncInfo.HighestViewAggQC() + if timeoutCert, haveTC := syncInfo.TC(); haveTC { if err := s.auth.VerifyTimeoutCert(timeoutCert); err != nil { return nil, 0, timeout, fmt.Errorf("failed to verify timeout certificate: %w", err) } - view = timeoutCert.View() - timeout = true } - if aggQC, haveQC := syncInfo.AggQC(); haveQC { highQC, err := s.auth.VerifyAggregateQC(aggQC) if err != nil { return nil, 0, timeout, fmt.Errorf("failed to verify aggregate quorum certificate: %w", err) } - if aggQC.View() >= view { - view = aggQC.View() - timeout = true - } return &highQC, view, timeout, nil + } else if qc, haveQC := syncInfo.QC(); haveQC { + if err := s.auth.VerifyQuorumCert(qc); err != nil { + return nil, 0, timeout, fmt.Errorf("failed to verify quorum certificate: %w", err) + } + return &qc, view, timeout, nil } return nil, view, timeout, nil // aggregate quorum certificate not present, so no high QC available } diff --git a/protocol/synchronizer/timeoutrule_simple.go b/protocol/synchronizer/timeoutrule_simple.go index fcc6cb35..e60414ae 100644 --- a/protocol/synchronizer/timeoutrule_simple.go +++ b/protocol/synchronizer/timeoutrule_simple.go @@ -47,23 +47,18 @@ func (s *Simple) RemoteTimeoutRule(_, timeoutView hotstuff.View, timeouts []hots } func (s *Simple) VerifySyncInfo(syncInfo hotstuff.SyncInfo) (qc *hotstuff.QuorumCert, view hotstuff.View, timeout bool, err error) { + // get the highest view of the sync info's QC and TC + view, timeout = syncInfo.HighestView() + if timeoutCert, haveTC := syncInfo.TC(); haveTC { if err := s.auth.VerifyTimeoutCert(timeoutCert); err != nil { return nil, 0, timeout, fmt.Errorf("failed to verify timeout certificate: %w", err) } - view = timeoutCert.View() - timeout = true } - if quorumCert, haveQC := syncInfo.QC(); haveQC { if err := s.auth.VerifyQuorumCert(quorumCert); err != nil { return nil, 0, timeout, fmt.Errorf("failed to verify quorum certificate: %w", err) } - // if there is both a TC and a QC, we use the QC if its view is greater or equal to the TC. - if quorumCert.View() >= view { - view = quorumCert.View() - timeout = false - } return &quorumCert, view, timeout, nil } return nil, view, timeout, nil // quorum certificate not present, so no high QC available diff --git a/types.go b/types.go index e3d78fab..37c894b2 100644 --- a/types.go +++ b/types.go @@ -177,6 +177,42 @@ func (si *SyncInfo) SetAggQC(aggQC AggregateQC) { si.aggQC = &aggQC } +// HighestView returns the highest view among the QC and TC, +// and whether it came from a timeout. +func (si *SyncInfo) HighestView() (view View, timeout bool) { + switch { + case si.qc != nil && si.tc != nil: + if si.qc.View() >= si.tc.View() { + return si.qc.View(), false + } + return si.tc.View(), true + case si.qc != nil: + return si.qc.View(), false + case si.tc != nil: + return si.tc.View(), true + } + // no QC or TC present + return 0, false +} + +// HighestViewAggQC returns the highest view among the AggQC and TC, +// and whether it came from a timeout. +func (si *SyncInfo) HighestViewAggQC() (view View, timeout bool) { + switch { + case si.aggQC != nil && si.tc != nil: + if si.aggQC.View() >= si.tc.View() { + return si.aggQC.View(), true // aggQC is always from timeouts + } + return si.tc.View(), true + case si.aggQC != nil: + return si.aggQC.View(), true + case si.tc != nil: + return si.tc.View(), true + } + // no AggQC or TC present + return 0, false +} + // QC returns the quorum certificate, if present. func (si SyncInfo) QC() (_ QuorumCert, _ bool) { if si.qc != nil {