fix comments

cmd/harmony/bls.go

@@ -5,8 +5,6 @@ import (
 	"os"
 	"sync"
 
-	"github.com/harmony-one/harmony/crypto/bls"
-
 	"github.com/harmony-one/harmony/internal/blsgen"
 	nodeconfig "github.com/harmony-one/harmony/internal/configs/node"
 	"github.com/harmony-one/harmony/multibls"
@@ -28,18 +26,7 @@ func setupConsensusKeys(hc harmonyConfig, config *nodeconfig.ConfigType) multibl
 		}
 	})
 
-	// Dedup private keys
-	uniqueKeys := map[bls.SerializedPublicKey]struct{}{}
-	deduped := multibls.PrivateKeys{}
-	for _, priKey := range multiBLSPriKey {
-		if _, ok := uniqueKeys[priKey.Pub.Bytes]; ok {
-			continue
-		}
-		uniqueKeys[priKey.Pub.Bytes] = struct{}{}
-		deduped = append(deduped, priKey)
-	}
-
-	config.ConsensusPriKey = deduped
+	config.ConsensusPriKey = multiBLSPriKey
 	return multiBLSPriKey.GetPublicKeys()
 }
 
@@ -58,7 +45,7 @@ func loadBLSKeys(raw blsConfig) (multibls.PrivateKeys, error) {
 	if len(keys) > raw.MaxKeys {
 		return nil, fmt.Errorf("bls keys exceed maximum count %v", raw.MaxKeys)
 	}
-	return keys, err
+	return keys.Dedup(), err
 }
 
 func parseBLSLoadingConfig(raw blsConfig) (blsgen.Config, error) {

consensus/checks.go

@@ -60,10 +60,8 @@ func (consensus *Consensus) isRightBlockNumAndViewID(recvMsg *FBFTMessage,
 ) bool {
 	if recvMsg.ViewID != consensus.GetCurBlockViewID() || recvMsg.BlockNum != consensus.blockNum {
 		consensus.getLogger().Debug().
-			Uint64("MsgViewID", recvMsg.ViewID).
-			Uint64("MsgBlockNum", recvMsg.BlockNum).
 			Uint64("blockNum", consensus.blockNum).
-			Interface("ValidatorPubKey", recvMsg.SenderPubkeys).
+			Str("recvMsg", recvMsg.String()).
 			Msg("BlockNum/viewID not match")
 		return false
 	}
@@ -76,8 +74,9 @@ func (consensus *Consensus) onAnnounceSanityChecks(recvMsg *FBFTMessage) bool {
 	)
 	if len(logMsgs) > 0 {
 		if len(logMsgs[0].SenderPubkeys) != 1 || len(recvMsg.SenderPubkeys) != 1 {
-			consensus.getLogger().Debug().
-				Interface("signers", recvMsg.SenderPubkeys).
+			consensus.getLogger().Warn().
+				Str("logMsgs[0]", logMsgs[0].String()).
+				Str("recvMsg", recvMsg.String()).
 				Msg("[OnAnnounce] Announce message have 0 or more than 1 signers")
 			return false
 		}
@@ -86,10 +85,7 @@ func (consensus *Consensus) onAnnounceSanityChecks(recvMsg *FBFTMessage) bool {
 			consensus.getLogger().Debug().
 				Str("logMsgSenderKey", logMsgs[0].SenderPubkeys[0].Bytes.Hex()).
 				Str("logMsgBlockHash", logMsgs[0].BlockHash.Hex()).
-				Str("recvMsg.SenderPubkeys", recvMsg.SenderPubkeys[0].Bytes.Hex()).
-				Uint64("recvMsg.BlockNum", recvMsg.BlockNum).
-				Uint64("recvMsg.ViewID", recvMsg.ViewID).
-				Str("recvMsgBlockHash", recvMsg.BlockHash.Hex()).
+				Str("recvMsg", recvMsg.String()).
 				Str("LeaderKey", consensus.LeaderPubKey.Bytes.Hex()).
 				Msg("[OnAnnounce] Leader is malicious")
 			if consensus.IsViewChangingMode() {
@@ -198,7 +194,7 @@ func (consensus *Consensus) onViewChangeSanityCheck(recvMsg *FBFTMessage) bool {
 	}
 
 	if len(recvMsg.SenderPubkeys) != 1 {
-		consensus.getLogger().Error().Msg("[onViewChange] zero or multiple signers in view change message.")
+		consensus.getLogger().Error().Msg("[onViewChangeSanityCheck] zero or multiple signers in view change message.")
 		return false
 	}
 	senderKey := recvMsg.SenderPubkeys[0]

consensus/leader.go

@@ -222,7 +222,7 @@ func (consensus *Consensus) onCommit(msg *msg_pb.Message) {
 
 	// Verify the signature on commitPayload is correct
 	logger := consensus.getLogger().With().
-		Interface("validatorPubKey", recvMsg.SenderPubkeys).
+		Str("recvMsg", recvMsg.String()).
 		Int64("numReceivedSoFar", signerCount).Logger()
 
 	logger.Debug().Msg("[OnCommit] Received new commit message")

consensus/quorum/one-node-staked-vote.go

@@ -72,6 +72,9 @@ func (v *stakedVoteWeight) AddNewVote(
 		if i == 0 {
 			signerAddr = voter.EarningAccount
 		} else {
+			// Aggregated signature should not contain signatures from keys belonging to different accounts,
+			// to avoid malicious node catching other people's signatures and merge with their own to cause problems.
+			// Harmony nodes are excluded from this rule.
 			if bytes.Compare(signerAddr.Bytes(), voter.EarningAccount[:]) != 0 && !voter.IsHarmonyNode {
 				return nil, errors.Errorf("Multiple signer accounts used in multi-sig: %x, %x", signerAddr.Bytes(), voter.EarningAccount)
 			}

multibls/multibls.go

@@ -3,6 +3,8 @@ package multibls
 import (
 	"strings"
 
+	"github.com/harmony-one/harmony/internal/utils"
+
 	bls_core "github.com/harmony-one/bls/ffi/go/bls"
 	"github.com/harmony-one/harmony/crypto/bls"
 )
@@ -45,6 +47,22 @@ func (multiKey PrivateKeys) GetPublicKeys() PublicKeys {
 	return pubKeys
 }
 
+// Dedup will return a new list of dedupped private keys.
+// This func won't modify the original slice.
+func (multiKey PrivateKeys) Dedup() PrivateKeys {
+	uniqueKeys := make(map[bls.SerializedPublicKey]struct{})
+	deduped := make(PrivateKeys, 0, len(multiKey))
+	for _, priKey := range multiKey {
+		if _, ok := uniqueKeys[priKey.Pub.Bytes]; ok {
+			utils.Logger().Warn().Str("PubKey", priKey.Pub.Bytes.Hex()).Msg("Duplicate private key ignored!")
+			continue
+		}
+		uniqueKeys[priKey.Pub.Bytes] = struct{}{}
+		deduped = append(deduped, priKey)
+	}
+	return deduped
+}
+
 // GetPrivateKeys creates a multibls PrivateKeys using bls.SecretKey
 func GetPrivateKeys(secretKeys ...*bls_core.SecretKey) PrivateKeys {
 	keys := make(PrivateKeys, 0, len(secretKeys))