diff --git a/protocol/synchronizer/synchronizer.go b/protocol/synchronizer/synchronizer.go index 728ca15e..510f4ed8 100644 --- a/protocol/synchronizer/synchronizer.go +++ b/protocol/synchronizer/synchronizer.go @@ -250,7 +250,7 @@ func (s *Synchronizer) OnNewView(newView hotstuff.NewViewMsg) { func (s *Synchronizer) advanceView(syncInfo hotstuff.SyncInfo) { s.logger.Debugf("advanceView: %v", syncInfo) - qc, view, timeout, err := s.timeoutRules.VerifySyncInfo(syncInfo) + qc, view, timeout, err := s.auth.VerifySyncInfo(syncInfo) if err != nil { s.logger.Infof("advanceView: Failed to verify sync info: %v", err) return diff --git a/protocol/synchronizer/synchronizer_test.go b/protocol/synchronizer/synchronizer_test.go index 506df213..88d384b3 100644 --- a/protocol/synchronizer/synchronizer_test.go +++ b/protocol/synchronizer/synchronizer_test.go @@ -176,33 +176,37 @@ func TestAdvanceView(t *testing.T) { }{ // four signers; quorum reached, advance view {name: "signers=4/Simple___/__/__/__", tr: S, qc: F, tc: F, ac: F, firstSignerIdx: 0, wantView: 1}, // empty syncInfo, should not advance view - {name: "signers=4/Simple___/__/__/AC", tr: S, qc: F, tc: F, ac: T, firstSignerIdx: 0, wantView: 1}, // simple timeout rule ignores aggregate timeout cert, will not advance view + {name: "signers=4/Simple___/__/__/AC", tr: S, qc: F, tc: F, ac: T, firstSignerIdx: 0, wantView: 2}, // advance view; simple timeout rule no longer ignores aggregate certs: cert.Authority.VerifySyncInfo() {name: "signers=4/Simple___/__/TC/__", tr: S, qc: F, tc: T, ac: F, firstSignerIdx: 0, wantView: 2}, {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}, {name: "signers=4/Aggregate/__/TC/__", tr: A, qc: F, tc: T, ac: F, firstSignerIdx: 0, wantView: 2}, {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/__/__", tr: A, qc: T, tc: F, ac: F, firstSignerIdx: 0, wantView: 2}, // advance view; aggregate timeout rule no longer ignores quorum certs: cert.Authority.VerifySyncInfo() {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___/__/__/AC", tr: S, qc: F, tc: F, ac: T, firstSignerIdx: 1, wantView: 2}, // advance view; simple timeout rule no longer ignores aggregate certs: cert.Authority.VerifySyncInfo() {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}, {name: "signers=3/Aggregate/__/TC/__", tr: A, qc: F, tc: T, ac: F, firstSignerIdx: 1, wantView: 2}, {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/__/__", tr: A, qc: T, tc: F, ac: F, firstSignerIdx: 1, wantView: 2}, // advance view; aggregate timeout rule no longer ignores quorum certs: cert.Authority.VerifySyncInfo() {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 @@ -211,6 +215,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}, @@ -218,6 +223,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 9e9b1270..96b84734 100644 --- a/protocol/synchronizer/timeoutrule_aggregate.go +++ b/protocol/synchronizer/timeoutrule_aggregate.go @@ -61,25 +61,5 @@ func (s *Aggregate) RemoteTimeoutRule(currentView, timeoutView hotstuff.View, ti return si, nil } -func (s *Aggregate) VerifySyncInfo(syncInfo hotstuff.SyncInfo) (qc *hotstuff.QuorumCert, view hotstuff.View, timeout bool, err error) { - 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 - } - return nil, view, timeout, nil // aggregate quorum certificate not present, so no high QC available -} +var _ TimeoutRuler = (*Aggregate)(nil) diff --git a/protocol/synchronizer/timeoutrule_simple.go b/protocol/synchronizer/timeoutrule_simple.go index 01667c51..7971e7c1 100644 --- a/protocol/synchronizer/timeoutrule_simple.go +++ b/protocol/synchronizer/timeoutrule_simple.go @@ -46,25 +46,4 @@ func (s *Simple) RemoteTimeoutRule(_, timeoutView hotstuff.View, timeouts []hots return hotstuff.NewSyncInfoWith(tc), nil } -func (s *Simple) VerifySyncInfo(syncInfo hotstuff.SyncInfo) (qc *hotstuff.QuorumCert, view hotstuff.View, timeout bool, err error) { - 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 -} +var _ TimeoutRuler = (*Simple)(nil) diff --git a/protocol/synchronizer/timeoutrules.go b/protocol/synchronizer/timeoutrules.go index c8fa7854..0c9379be 100644 --- a/protocol/synchronizer/timeoutrules.go +++ b/protocol/synchronizer/timeoutrules.go @@ -17,5 +17,4 @@ func NewTimeoutRuler(cfg *core.RuntimeConfig, auth *cert.Authority) TimeoutRuler type TimeoutRuler interface { LocalTimeoutRule(hotstuff.View, hotstuff.SyncInfo) (*hotstuff.TimeoutMsg, error) RemoteTimeoutRule(currentView, timeoutView hotstuff.View, timeouts []hotstuff.TimeoutMsg) (hotstuff.SyncInfo, error) - VerifySyncInfo(hotstuff.SyncInfo) (qc *hotstuff.QuorumCert, view hotstuff.View, timeout bool, err error) } diff --git a/security/cert/auth.go b/security/cert/auth.go index 7c21b01f..ce37c4aa 100644 --- a/security/cert/auth.go +++ b/security/cert/auth.go @@ -209,3 +209,42 @@ func (c *Authority) VerifyAnyQC(proposal *hotstuff.ProposeMsg) error { } return c.VerifyQuorumCert(qc) } + +// VerifySyncInfo verifies the sync info and returns the highest QC found (if any), +// the highest view, whether it was a timeout, and an error if verification failed. +func (c *Authority) VerifySyncInfo(syncInfo hotstuff.SyncInfo) (highQC *hotstuff.QuorumCert, view hotstuff.View, timeout bool, err error) { + if timeoutCert, haveTC := syncInfo.TC(); haveTC { + if err := c.VerifyTimeoutCert(timeoutCert); err != nil { + return nil, 0, timeout, fmt.Errorf("failed to verify timeout certificate: %w", err) + } + view = timeoutCert.View() + timeout = true + } + + if aggQC, haveAggQC := syncInfo.AggQC(); haveAggQC { + qc, err := c.VerifyAggregateQC(aggQC) + if err != nil { + return nil, 0, timeout, fmt.Errorf("failed to verify aggregate quorum certificate: %w", err) + } + view = max(view, aggQC.View()) + timeout = true // an aggregate QC represents a timeout + highQC = &qc + } + + if qc, haveQC := syncInfo.QC(); haveQC { + if err := c.VerifyQuorumCert(qc); err != nil { + return nil, 0, timeout, fmt.Errorf("failed to verify quorum certificate: %w", err) + } + view = max(view, qc.View()) + if qc.View() == view { + // QC's view is higher or equal: not a timeout + highQC = &qc + timeout = false + } else if highQC == nil { + // QC's view is lower, but there was no AggQC + highQC = &qc + } + } + + return highQC, view, timeout, nil +}