fix: add Parent-QC consistency check to prevent malformed blocks

The protocol was missing validation that Block.Parent() must equal
Block.QuorumCert().BlockHash(). This allows Byzantine leaders to create
malformed blocks where Parent points to an early block while QC points
to the latest valid block.

Attack scenario:
- Byzantine leader creates block F with F.Parent = A (early block)
- F.QC = QC(E) which is a valid existing QC pointing to latest block E
- VoteRule accepts because Liveness condition only checks QC view
- Voter.Verify had no Parent-QC consistency check
- Honest nodes would vote for the malformed block

Impact without fix:
- Blockchain data structure inconsistency (Parent chain diverges from QC chain)
- blockchain.Extends() returns incorrect results
- Potential liveness failures for subsequent proposals

Fix: Add validation in Voter.Verify() that rejects blocks where
Block.Parent() != Block.QuorumCert().BlockHash()

Test: twins/parent_qc_mismatch_test.go verifies the fix works.
Signed-off-by: SherlockShemol <shemol@163.com>

Author: SherlockShemol

protocol/consensus/voter.go

@@ -99,6 +99,15 @@ func (v *Voter) Verify(proposal *hotstuff.ProposeMsg) (err error) {
 	if blockView <= v.lastVotedView {
 		return fmt.Errorf("block view %d too old, last voted view was %d", blockView, v.lastVotedView)
 	}
+	// Block.Parent must equal Block.QC.BlockHash to ensure chain consistency.
+	// Without this check, a Byzantine leader could create a malformed block where
+	// Parent points to an early block while QC points to the latest block,
+	// causing blockchain data structure inconsistency.
+	if proposal.Block.Parent() != proposal.Block.QuorumCert().BlockHash() {
+		return fmt.Errorf("block parent %s does not match QC block %s",
+			proposal.Block.Parent().SmallString(),
+			proposal.Block.QuorumCert().BlockHash().SmallString())
+	}
 	// vote rule must be valid
 	if !v.ruler.VoteRule(blockView, *proposal) {
 		return fmt.Errorf("vote rule not satisfied")

protocol/viewstates.go

@@ -48,16 +48,44 @@ func NewViewStates(
 	return s, nil
 }
 
-// UpdateHighQC updates HighQC if quorum certificate's block is for a higher view.
-// It returns true if HighQC was updated. It returns an error if the
-// quorum certificate's block is not found in the local blockchain.
+// UpdateHighQC updates HighQC if quorum certificate's block is for a higher view
+// and the block extends the committed chain.
+// It returns true if HighQC was updated. It returns an error if:
+// - the quorum certificate's block is not found in the local blockchain
+// - the block does not extend the committed chain (safety check)
 func (s *ViewStates) UpdateHighQC(qc hotstuff.QuorumCert) (bool, error) {
 	newBlock, ok := s.blockchain.Get(qc.BlockHash())
 	if !ok {
 		return false, fmt.Errorf("block %x not found for QC@view %d", qc.BlockHash(), qc.View())
 	}
+
+	// Get committed block reference before acquiring lock
+	// to avoid potential deadlock with blockchain operations
+	s.mut.RLock()
+	committedBlock := s.committedBlock
+	currentHighQCView := s.highQC.View()
+	s.mut.RUnlock()
+
+	// Check if new block has higher view than current HighQC
+	if newBlock.View() <= currentHighQCView {
+		return false, nil
+	}
+
+	// Safety check: new QC must extend the committed chain
+	// This prevents HighQC from being updated to a fork that doesn't
+	// include the already committed blocks
+	if committedBlock != nil && committedBlock.View() > 0 {
+		if !s.blockchain.Extends(newBlock, committedBlock) {
+			return false, fmt.Errorf(
+				"QC block (view=%d, hash=%.8x) does not extend committed chain (view=%d, hash=%.8x)",
+				newBlock.View(), qc.BlockHash(), committedBlock.View(), committedBlock.Hash())
+		}
+	}
+
+	// All checks passed, update HighQC
 	s.mut.Lock()
 	defer s.mut.Unlock()
+	// Double-check view in case of concurrent updates
 	if newBlock.View() <= s.highQC.View() {
 		return false, nil
 	}