From ee52f5800c780dcd51959bbb32ddff76493ef0b3 Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Mon, 3 Nov 2025 23:37:53 +0100 Subject: [PATCH 1/4] feat: add HighestView and HighestViewAggQC methods to SyncInfo This adds two helper methods for view comparison between QC and TC and AggQC and TC. These can be used by VerifySyncInfo. --- types.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/types.go b/types.go index e3d78fab8..37c894b2a 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 { From bb188b2e51b508245a91dc7f9d81374bed256623 Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Mon, 3 Nov 2025 23:40:03 +0100 Subject: [PATCH 2/4] refactor: simplify VerifySyncInfo in Aggregate and Simple timeout rules This uses the new HighestView() methods to determine if the QC or TC has the highest view, and thus if timeout should be true. Note: Moving the initialization of view and timeout before the signature verification should be fine, since a signature failure will return err != nil, and users of the VerifySyncInfo must always check for err before using any other output. --- protocol/synchronizer/timeoutrule_aggregate.go | 10 +++------- protocol/synchronizer/timeoutrule_simple.go | 11 +++-------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/protocol/synchronizer/timeoutrule_aggregate.go b/protocol/synchronizer/timeoutrule_aggregate.go index 3d333da1f..8fe55d91d 100644 --- a/protocol/synchronizer/timeoutrule_aggregate.go +++ b/protocol/synchronizer/timeoutrule_aggregate.go @@ -62,23 +62,19 @@ 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 } 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 fcc6cb35f..e60414aec 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 From eec647e7ea9e56d2f7d5fe737ffc4e31d79df91b Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Tue, 4 Nov 2025 00:14:49 +0100 Subject: [PATCH 3/4] fix: added missing test cases in TestAdvanceView --- protocol/synchronizer/synchronizer_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/protocol/synchronizer/synchronizer_test.go b/protocol/synchronizer/synchronizer_test.go index a8b6516d5..750d9e28b 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 { From 6de681b3b732cd6f208bf95f828feb8b55cccad8 Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Fri, 14 Nov 2025 22:29:42 +0100 Subject: [PATCH 4/4] fix: VerifySyncInfo to verify QC absent aggregate QC If the SyncInfo is missing the AggregateQC, we now check if the SyncInfo has a quorum certificate and returns that instead. This corresponds to the existing logic on the master branch. --- protocol/synchronizer/timeoutrule_aggregate.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/protocol/synchronizer/timeoutrule_aggregate.go b/protocol/synchronizer/timeoutrule_aggregate.go index 8fe55d91d..883f38f35 100644 --- a/protocol/synchronizer/timeoutrule_aggregate.go +++ b/protocol/synchronizer/timeoutrule_aggregate.go @@ -76,6 +76,11 @@ func (s *Aggregate) VerifySyncInfo(syncInfo hotstuff.SyncInfo) (qc *hotstuff.Quo return nil, 0, timeout, fmt.Errorf("failed to verify aggregate quorum certificate: %w", err) } 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 }