Checking PC validity performance improvement (#89)

* Checking PC validity performance improvement

core/ibft.go

@@ -1143,7 +1143,7 @@ func (i *IBFT) ExtendRoundTimeout(amount time.Duration) {
 // validPC verifies that the prepared certificate is valid
 func (i *IBFT) validPC(
 	certificate *proto.PreparedCertificate,
-	rLimit,
+	roundLimit,
 	height uint64,
 ) bool {
 	if certificate == nil {
@@ -1161,11 +1161,6 @@ func (i *IBFT) validPC(
 		certificate.PrepareMessages...,
 	)
 
-	// Make sure the senders are unique
-	if !messages.HasUniqueSenders(allMessages) {
-		return false
-	}
-
 	// Make sure there are at least Quorum (PP + P) messages
 	// hasQuorum directly since the messages are of different types
 	if !i.validatorManager.HasQuorum(convertMessageToAddressSet(allMessages)) {
@@ -1184,23 +1179,8 @@ func (i *IBFT) validPC(
 		}
 	}
 
-	// Make sure the proposal hashes match
-	if !messages.HaveSameProposalHash(allMessages) {
-		return false
-	}
-
-	// Make sure all the messages have a round number lower than rLimit
-	if !messages.AllHaveLowerRound(allMessages, rLimit) {
-		return false
-	}
-
-	// Make sure all the messages have the same height
-	if !messages.AllHaveSameHeight(allMessages, height) {
-		return false
-	}
-
-	// Make sure all have the same round
-	if !messages.AllHaveSameRound(allMessages) {
+	// Make sure the round, height and proposal hashes match and the senders are unique
+	if !messages.AreValidPCMessages(allMessages, height, roundLimit) {
 		return false
 	}
 

messages/helpers.go

@@ -127,7 +127,7 @@ func HasUniqueSenders(messages []*proto.Message) bool {
 		return false
 	}
 
-	senderMap := make(map[string]struct{})
+	senderMap := make(map[string]struct{}, len(messages))
 
 	for _, message := range messages {
 		key := string(message.From)
@@ -141,86 +141,63 @@ func HasUniqueSenders(messages []*proto.Message) bool {
 	return true
 }
 
-// HaveSameProposalHash checks if the messages have the same proposal hash
-func HaveSameProposalHash(messages []*proto.Message) bool {
+// AreValidPCMessages validates PreparedCertificate messages
+func AreValidPCMessages(messages []*proto.Message, height uint64, roundLimit uint64) bool {
 	if len(messages) < 1 {
 		return false
 	}
 
+	round := messages[0].View.Round
+	senderMap := make(map[string]struct{})
+
 	var hash []byte
 
 	for _, message := range messages {
-		var extractedHash []byte
-
-		switch message.Type {
-		case proto.MessageType_PREPREPARE:
-			extractedHash = ExtractProposalHash(message)
-		case proto.MessageType_PREPARE:
-			extractedHash = ExtractPrepareHash(message)
-		case proto.MessageType_COMMIT, proto.MessageType_ROUND_CHANGE:
+		// all messages must have the same height
+		if message.View.Height != height {
 			return false
-		default:
+		}
+
+		// all messages must have the same round that is not greater than round limit
+		if message.View.Round != round || message.View.Round >= roundLimit {
 			return false
 		}
 
+		// all messages must have the same proposal hash
+		extractedHash, ok := extractPCMessageHash(message)
 		if hash == nil {
 			// No previous hash for comparison,
 			// set the first one as the reference, as
 			// all of them need to be the same anyway
 			hash = extractedHash
 		}
 
-		if !bytes.Equal(hash, extractedHash) {
+		if !ok || !bytes.Equal(hash, extractedHash) {
 			return false
 		}
-	}
-
-	return true
-}
 
-// AllHaveLowerRound checks if all messages have the same round
-func AllHaveLowerRound(messages []*proto.Message, round uint64) bool {
-	if len(messages) < 1 {
-		return false
-	}
-
-	for _, message := range messages {
-		if message.View.Round >= round {
+		// all messages must have unique senders
+		key := string(message.From)
+		if _, exists := senderMap[key]; exists {
 			return false
 		}
-	}
-
-	return true
-}
-
-// AllHaveSameRound checks if all messages have the same round
-func AllHaveSameRound(messages []*proto.Message) bool {
-	if len(messages) < 1 {
-		return false
-	}
 
-	var round = messages[0].View.Round
-
-	for _, message := range messages {
-		if message.View.Round != round {
-			return false
-		}
+		senderMap[key] = struct{}{}
 	}
 
 	return true
 }
 
-// AllHaveSameHeight checks if all messages have the same height
-func AllHaveSameHeight(messages []*proto.Message, height uint64) bool {
-	if len(messages) < 1 {
-		return false
-	}
-
-	for _, message := range messages {
-		if message.View.Height != height {
-			return false
-		}
+// extractPCMessageHash extracts the hash from a PC message
+func extractPCMessageHash(message *proto.Message) ([]byte, bool) {
+	switch message.Type {
+	case proto.MessageType_PREPREPARE:
+		return ExtractProposalHash(message), true
+	case proto.MessageType_PREPARE:
+		return ExtractPrepareHash(message), true
+	case proto.MessageType_COMMIT, proto.MessageType_ROUND_CHANGE:
+		return nil, false
+	default:
+		return nil, false
 	}
-
-	return true
 }