From afdd74c1fbab4f13971d935ee094b9dc63266d77 Mon Sep 17 00:00:00 2001 From: Rongjian Lan Date: Wed, 14 Oct 2020 18:17:33 -0700 Subject: [PATCH] fix comments --- cmd/harmony/bls.go | 17 ++--------------- consensus/checks.go | 16 ++++++---------- consensus/leader.go | 2 +- consensus/quorum/one-node-staked-vote.go | 3 +++ multibls/multibls.go | 18 ++++++++++++++++++ 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/cmd/harmony/bls.go b/cmd/harmony/bls.go index 6f0615630..66d893161 100644 --- a/cmd/harmony/bls.go +++ b/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) { diff --git a/consensus/checks.go b/consensus/checks.go index db39c9256..51f830447 100644 --- a/consensus/checks.go +++ b/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] diff --git a/consensus/leader.go b/consensus/leader.go index 186de393d..1a0f374ad 100644 --- a/consensus/leader.go +++ b/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") diff --git a/consensus/quorum/one-node-staked-vote.go b/consensus/quorum/one-node-staked-vote.go index 65c6edfdf..7b79f4de8 100644 --- a/consensus/quorum/one-node-staked-vote.go +++ b/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) } diff --git a/multibls/multibls.go b/multibls/multibls.go index 5c4568fa8..dc09d9a86 100644 --- a/multibls/multibls.go +++ b/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))