Move VerifySyncInfo from TimeoutRuler to cert.Authority

[meling]

Fixes #262

This moves the VerifySyncInfo logic from the simple and aggregate timeout rules into a common logic on the cert.Authority struct, along with many other crypto verification methods (also used by the VerifySyncInfo method). This also removes the method from the TimeoutRuler interface, reducing the interface to only two methods.

Since the Simple and Aggregate timeout rules now share a common VerifySyncInfo method, the tests no longer matched the previous expectation. However, I believe these changes match the expectations for advancing the view when we have only one of AC or QC.

[Copilot]

Pull Request Overview

This PR refactors the VerifySyncInfo method by moving it from the TimeoutRuler interface implementations (Simple and Aggregate timeout rulers) to the cert.Authority struct, consolidating crypto verification logic in a single location. This reduces the TimeoutRuler interface to only two methods and eliminates code duplication between the two timeout ruler implementations.

Key changes:

• Unified VerifySyncInfo implementation in cert.Authority that handles TC, AggQC, and QC verification
• Removed VerifySyncInfo from the TimeoutRuler interface and both implementation structs
• Updated test expectations to reflect new behavior where both Simple and Aggregate timeout rulers now handle all certificate types

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
security/cert/auth.go	Added unified VerifySyncInfo method that verifies TC, AggQC, and QC from SyncInfo and returns the highest QC
protocol/synchronizer/timeoutrules.go	Removed VerifySyncInfo from TimeoutRuler interface definition
protocol/synchronizer/timeoutrule_simple.go	Removed VerifySyncInfo implementation; added interface compliance check
protocol/synchronizer/timeoutrule_aggregate.go	Removed VerifySyncInfo implementation; added interface compliance check
protocol/synchronizer/synchronizer.go	Updated to call s.auth.VerifySyncInfo instead of s.timeoutRules.VerifySyncInfo
protocol/synchronizer/synchronizer_test.go	Updated test cases to reflect new behavior where Simple and Aggregate handle all certificate types; fixed typo "reacted" → "reached"; added test cases for QC/TC combinations

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The highQC selection logic doesn't properly handle the case when both AggQC and a plain QC are present, and the plain QC has a higher view than the AggQC's embedded QC.

Example scenario:

• AggQC.View() = 6, AggQC's embedded highQC.View() = 4
• Plain QC.View() = 5

Current behavior:

1. Line 229: view = max(0, 6) = 6, highQC = &qc (view 4 from AggQC)
2. Line 238: view = max(6, 5) = 6
3. Line 239: qc.View() == view evaluates to 5 == 6 (false)
4. Line 243: highQC == nil evaluates to false
5. Result: Returns AggQC's embedded QC (view 4)

Expected: Should return the plain QC (view 5) since it's higher than the AggQC's embedded QC (view 4). The highQC should represent the highest quorum certificate known, regardless of which certificate determines the view to advance to.

Suggested fix: When qc.View() != view and highQC != nil, compare qc.View() with highQC.View() and update highQC if the plain QC has a higher view:

} else if highQC == nil || qc.View() > highQC.View() {
    highQC = &qc
}

Suggested change

	} else if highQC == nil {
	// QC's view is lower, but there was no AggQC
	} else if highQC == nil || qc.View() > highQC.View() {
	// QC's view is lower, but there was no AggQC, or QC is higher than current highQC